[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 03/15] tests/tcg/aarch64: add system boot.S
From: |
Alex Bennée |
Subject: |
Re: [Qemu-devel] [PATCH v5 03/15] tests/tcg/aarch64: add system boot.S |
Date: |
Wed, 01 May 2019 15:57:53 +0100 |
User-agent: |
mu4e 1.3.1; emacs 26.1 |
Richard Henderson <address@hidden> writes:
> 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?
Sure.
>
>> + /* 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?
Ok.
>
>> + /* 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.
Yeah we can do better here. I mainly went with what libgloss had setup
because I was finding it hard to get find a nice summary of the various
page table formats. I want big enough that I don't have to futz around
create multiple page entries and ideally have some fault-able regions as
well.
>
>> + /* 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
lol, I guess the page table was being crapped over....
>
> Also, there's tab/space confusion all through this file.
> IMO, this is assembly, so it *should* be tabs.
That will probably be my editor getting confused because .S implies cpp
>
>> @@ -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.
OK
>
>
> r~
--
Alex Bennée