Re: [PATCH v3 4/4] tests/tcg/aarch64: Extend MTE gdbstub tests to system mode

[Gustavo Romero <gustavo.romero@linaro.org>]

Hi Richard,

On 8/29/24 7:16 PM, Richard Henderson wrote:
> On 8/30/24 06:13, Gustavo Romero wrote:
>>    1 .text         00001e60  0000000040001000  0000000040001000  00011000  2**12
>>    4 .data         00012000  0000000040200000  0000000040200000  00020000  2**12
>> 0000000040400000 g       .data    0000000000000000 mte_page
>>
>>
>> I was not able to make the MEMORY command work. I tried:
>>
>> index 46f1092522..dc39518a16 100644
>> --- a/tests/tcg/aarch64/system/kernel.ld
>> +++ b/tests/tcg/aarch64/system/kernel.ld
>> @@ -1,9 +1,13 @@
>>   ENTRY(__start)
>>
>> +MEMORY
>> +{
>> +    ram (rwx) : ORIGIN = 1 << 30, LENGTH = 16M
>> +}
>> +
>>   SECTIONS
>>   {
>>       /* virt machine, RAM starts at 1gb */
>> -    . = (1 << 30);
>>       .text : {
>>           *(.text)
>>       }
>> @@ -11,7 +15,7 @@ SECTIONS
>>           *(.rodata)
>>       }
>>       /* align r/w section to next 2mb */
>> -    . = ALIGN(1 << 21);
>> +    . = ALIGN(2M);
>>       .data : {
>>           *(.data)
>>       }
>> @@ -24,7 +28,7 @@ SECTIONS
>>        * used in boot.S to setup the PTE and in the mte.S test as the address that
>>        * the MTE instructions operate on.
>>        */
>> -    mte_page = ALIGN(1 << 22);
>> +    mte_page = ((1 << 30) + 4M);
>>       /DISCARD/ : {
>>           *(.ARM.attributes)
>>       }
>>
>> But it didn't work because data section is placed in the wrong place:
>>
>>    1 .text         00001e60  0000000040001000  0000000040001000  00011000  2**12
>>    4 .data         00012000  0000000040003000  0000000040003000  00013000  2**12
>> 0000000040400000 g       *ABS*    0000000000000000 mte_page
>>
>> Do you know why?
> 
> This is where using '.' gets tricky.
> For .data to be in the correct place, we need to place the ALIGN on .data:
> 
>    .data : ALIGN(2M) {
>       ...
>    }
> 
>> I also had to use 'mte_page = ((1 << 30) + 4M);' for the mte_page symbol because
>> the current location (.) seems to reset to zero for the sections, hence if I do:
>>
>> +    mte_page = ALIGN(2M);
>>
>> I get:
>>
>> 0000000040200000 g       .data    0000000000000000 mte_page
> 
> Right.  That's a consequence of not placing the symbol within a section for layout.
> 
> I suppose all this is a bit academic. The current method using '.' is sufficient, but it feels slapdash.
> 
> Thinking about this more, the proper way to use MEMORY is to describe the page tables that we're setting up:
> 
> MEMORY {
>    TXT (rx) : ORIGIN = 1 << 30, LENGTH = 2M
>    DAT (rw) : ORIGIN = (1 << 30) + 2M, LENGTH = 2M
>    TAG (rw) : ORIGIN = (1 << 30) + 4M, LENGTH = 2M
> }
> 
> SECTIONS {
>    .text : {
>      *(.text)
>      *(.rodata)
>    } >TXT
>    .data : {
>      *(.data)
>      *(.bss)
>    } >DAT
>    .tag : {
>      mte_page = .
>    } >TAG
> }
> 
> Or something close to that.

Thanks, that looks nice. So, the final version for v4 is:

ENTRY(__start)

MEMORY {
     /* On virt machine RAM starts at 1 GiB. */

     /* Align text and rodata to the 1st 2 MiB chunk. */
     TXT (rx) : ORIGIN = 1 << 30, LENGTH = 2M
     /* Align r/w data to the 2nd 2 MiB chunk. */
     DAT (rw) : ORIGIN = (1 << 30) + 2M, LENGTH = 2M
     /* Align the MTE-enabled page to the 3rd 2 MiB chunk. */
     TAG (rw) : ORIGIN = (1 << 30) + 4M, LENGTH = 2M
}

SECTIONS {
     .text : {
         *(.text)
         *(.rodata)
     } >TXT
     .data : {
         *(.data)
         *(.bss)
     } >DAT
     .tag : {
         /*
          * Symbol 'mte_page' is used in boot.S to setup the PTE and in the mte.S
          * test as the address that the MTE instructions operate on.
          */
         mte_page = .;
     } >TAG
     /DISCARD/ : {
         *(.ARM.attributes)
     }
}


@Phil Are you good with it?


Cheers,
Gustavo