Re: [Qemu-arm] [Qemu-devel] [PATCH v5 03/15] tests/tcg/aarch64: add system boot.S

[Richard Henderson]

On 4/30/19 9:52 AM, Alex Bennée wrote:
> +.error:
> +     .string "Terminated by exception.\n"

Put it in .rodata just because we can?

> +        /* Page table setup (identity mapping).  */
> +     adrp    x0, ttb
> +     add     x0, x0, :lo12:ttb

You are in control of the layout of the executable,
and adr has a 1MB range.  Why use adrp+add?

> +        /* Create some (big) pages */
> +     adr     x1, .                           /* phys address */
> +     bic     x1, x1, #(1 << 30) - 1          /* 1GB block alignment */

Do you really want 1GB pages?  You'll pretty much only be able to test valid
memory operations with that.  Which is also true until there's something other
than an exit for the exception vector... but ya know what I mean.

> +        /* Setup some stack space and enter the test code.
> +         * Assume everthing except the return value is garbage when we
> +      * return, we won't need it.
> +         */
> +     adrp    x0, stack
> +     add     x0, x0, :lo12:stack
> +        mov      sp, x0

You need a pointer to the end of the stack, not the beginning.
Again, I think this could be just

        adr     sp, stack_end

Also, there's tab/space confusion all through this file.
IMO, this is assembly, so it *should* be tabs.

> @@ -0,0 +1,22 @@
> +ENTRY(__start)
> +
> +SECTIONS
> +{
> +    /* virt machine, RAM starts at 1gb */
> +    . = (1 << 30);
> +    .text : {
> +        *(.text)
> +    }
> +    .data : {
> +        *(.data)
> +    }
> +    .rodata : {
> +        *(.rodata)
> +    }

If you ever wanted to make this read-only, swap .rodata before .data, so that
it's next to .text.


r~