Hi Richard,

On 7/25/24 8:17 PM, Richard Henderson wrote:
On 7/23/24 02:07, Gustavo Romero wrote:
  def run_test():
-    gdb.execute("break 95", False, True)
+    if mode == "system":
+        # Break address: where to break before performing the tests
+        # Addresss is the last insn. before 'main' returns. See mte.c
+        ba = "*main+52"

Ugly.  You can add labels in your inline asm block instead.

I forgot about that possibility. Thanks. This motivated me to
convert mte.c to mte.S. I think there is no benefit in keeping
the test in C with a bunch of inline asm blocks. Please see v2.


diff --git a/tests/tcg/aarch64/system/boot.S b/tests/tcg/aarch64/system/boot.S
index 501685d0ec..a12393d00b 100644
--- a/tests/tcg/aarch64/system/boot.S
+++ b/tests/tcg/aarch64/system/boot.S
@@ -135,11 +135,22 @@ __start:
      orr    x1, x1, x3
      str    x1, [x2]            /* 2nd 2mb (.data & .bss)*/
+        /* Third block: .mte_page */
+    adrp    x1, .mte_page
+    add    x1, x1, :lo12:.mte_page
+    bic    x1, x1, #(1 << 21) - 1
+    and     x4, x1, x5
+    add    x2, x0, x4, lsr #(21 - 3)
+    ldr    x3, =(3 << 53) | 0x401 | 1 << 2    /* attr(AF, NX, block, 
AttrIndx=Attr1) */
+    orr    x1, x1, x3
+    str    x1, [x2]
+
      /* Setup/enable the MMU.  */
      /*
       * TCR_EL1 - Translation Control Registers
       *
+     * TBI0[37] = 0b1 => Top Byte ignored and used for tagged addresses
       * IPS[34:32] = 40-bit PA, 1TB
       * TG0[14:15] = b00 => 4kb granuale
       * ORGN0[11:10] = Outer: Normal, WB Read-Alloc No Write-Alloc Cacheable
@@ -152,16 +163,22 @@ __start:
       * with at least 1gb range to see RAM. So we start with a
       * level 1 lookup.
       */
-    ldr    x0, = (2 << 32) | 25 | (3 << 10) | (3 << 8)
+    ldr    x0, = (1 << 37) | (2 << 32) | 25 | (3 << 10) | (3 << 8)
      msr    tcr_el1, x0
-    mov    x0, #0xee            /* Inner/outer cacheable WB */
+    /*
+     * Attr0: Normal, Inner/outer cacheable WB
+     * Attr1: Tagged Normal (MTE)
+     */
+    mov    x0, #0xf0ee

Up to here, I think we're fine, no matter the emulated cpu model.

      msr    mair_el1, x0
      isb
      /*
       * SCTLR_EL1 - System Control Register
       *
+     * ATA[43] = 1 = enable access to allocation tags at EL1
+     * TCF[40] = 1 = Tag Check Faults cause a synchronous exception
       * WXN[19] = 0 = no effect, Write does not imply XN (execute never)
       * I[12] = Instruction cachability control
       * SA[3] = SP alignment check
@@ -169,7 +186,8 @@ __start:
       * M[0] = 1, enable stage 1 address translation for EL0/1
       */
      mrs    x0, sctlr_el1
-    ldr    x1, =0x100d            /* bits I(12) SA(3) C(2) M(0) */
+    /* Bits set: ATA(43) TCF(40) I(12) SA(3) C(2) M(0) */
+    ldr    x1, =(0x100d | 1 << 43 | 1 << 40)

But here, it's only legal to run this modified boot.S on -cpu max.
We should check for MTE enabled before setting those, or
set them elsewhere, e.g. in main of the specific MTE test.

Right. I move all these bits for MTE to mte.S, under main function,
and just left the page table setting in boot.S, since we need to
set it before the MMU is turned on.


@@ -239,3 +257,5 @@ ttb_stage2:
  stack:
      .space 65536, 0
  stack_end:
+
+    .section .mte_page

Why?

diff --git a/tests/tcg/aarch64/system/kernel.ld 
b/tests/tcg/aarch64/system/kernel.ld
index 7b3a76dcbf..7c00c1c378 100644
--- a/tests/tcg/aarch64/system/kernel.ld
+++ b/tests/tcg/aarch64/system/kernel.ld
@@ -18,6 +18,11 @@ SECTIONS
      .bss : {
          *(.bss)
      }
+    /* align MTE section to next (third) 2mb */
+    . = ALIGN(1 << 22);
+    .mte : {
+        *(.mte_page)
+    }

Why?

      /DISCARD/ : {
          *(.ARM.attributes)
      }
diff --git a/tests/tcg/aarch64/system/mte.c b/tests/tcg/aarch64/system/mte.c
new file mode 100644
index 0000000000..58a5ac31ff
--- /dev/null
+++ b/tests/tcg/aarch64/system/mte.c
@@ -0,0 +1,40 @@
+#include <inttypes.h>
+
+int main(void)
+{
+    uint8_t *addr;
+
+    /*
+     * Third 2MB chunk in the second 1GB block.
+     * See .mte_page section in kernel.ld.
+     */
+    addr = (void *)((1UL << 30) | (1UL << 22));

... because you're not using .mte_page here, just computing it.

.mte_page is used in boot.S for setting the page table entry, in:

        /* Third block: .mte_page */
        adrp    x1, .mte_page
        add     x1, x1, :lo12:.mte_page
        [...]

That's why it's being computed in kernel.ld. That said, you're
right that it's better to also tie 'addr' to it instead of having
a hard-coded value.

I'm using the .mte_page address in the new mte.S test in v2, so
now the value for 'addr' used in the test is tied to that computed
value.


+
+    asm (
+        /*
+         * Set GCR for randon tag generation. 0xA5 is just a random value to 
set

random

+         * GCR != 0 so the tag generated by 'irg' is not zero.
+         */
+        "ldr x1, =0xA5;"
+        "msr gcr_el1, x1;"

I think it might be easier to split the asm:

   asm volatile("msr gcr_el1, %0" : : "r"(0xA5));

hmm, all the tests are built with optimizations disabled,
so no need to use volatile? Anyways, in v2 I'm using mov
instead of ldr, and since it's .S, no more inline asm.


+
+         /* Generate a logical tag and put it in 'addr' pointer. */
+         "irg %[addr], %[addr];"

   asm("irg %0,%0" : "+r"(addr));

+
+         /*
+          * Store the generated tag to memory region pointed to by 'addr', i.e.
+          * set the allocation tag for the memory region.
+          */
+         "stg %[addr], [%[addr]];"

Storing addr into addr is a titch confusing, clearer with zero?

   asm("stg xzr,[%0]" : : "r"(addr))

This an invalid insn. xzr reg. can't be used as operand 1 in 'stg',
because register 31 is encoded as sp, not xzr.

However, I don't want to zero the allocation tag. Actually, it's
the opposite, I want it != 0, that's why I set gcr_el1 != 0: to
get a non-zero tag from 'irg'.

I agree it's a tad confusing, but the thing is, 'stg' expects a
tagged address in the source operand, not a "pure" tag, hence the
use of 'addr' in operand 1. But I think it could be:

stg tagged_addr, [addr]

In v2 I put a comment that stg actually extracts the tag to be
stored in [addr] from the address in 'tagged_addr'


+
+         /*
+          * Store a random value (0xdeadbeef) to *addr. This must not cause any
+          * Tag Check Fault since logical and allocation tags are set the same.
+          */
+     "ldr x1, =0xdeadbeef;"
+     "str x1, [x0];"

Where does x0 come from?  Certainly not "addr"...
Can you use "addr" directly in the gdb script?

Yep, it should be 'addr' instead of x0 here. It's a leftover.
Fixed in v2. Thanks.


Cheers,
Gustavo

Reply via email to