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

[Gustavo Romero]

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