Extend MTE gdbstub tests to also run in system mode (share tests between
user mode and system mode). The tests will only run if a version of GDB
that supports MTE on baremetal is available in the test environment and
if available compiler supports the 'memtag' flag
(-march=armv8.5-a+memtag).
For the tests running in system mode, a page that supports MTE ops. is
necessary. Therefore, an MTE-enabled page is made available (mapped) in
the third 2 MB chunk of the second 1 GB space in the flat mapping set in
boot.S. A new binary, mte.S, is also introduced for the tests. It links
against boot.S and is executed by QEMU in system mode.
Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
---
configure | 5 +
tests/tcg/aarch64/Makefile.softmmu-target | 49 +++++++++-
tests/tcg/aarch64/Makefile.target | 3 +-
tests/tcg/aarch64/gdbstub/test-mte.py | 71 +++++++++-----
tests/tcg/aarch64/system/boot.S | 11 +++
tests/tcg/aarch64/system/kernel.ld | 7 ++
tests/tcg/aarch64/system/mte.S | 109 ++++++++++++++++++++++
7 files changed, 227 insertions(+), 28 deletions(-)
create mode 100644 tests/tcg/aarch64/system/mte.S
diff --git a/configure b/configure
index d08b71f14b..40186d865e 100755
--- a/configure
+++ b/configure
@@ -1679,6 +1679,11 @@ for target in $target_list; do
echo "GDB_HAS_MTE=y" >> $config_target_mak
fi
+ if test "${gdb_arches#*aarch64}" != "$gdb_arches" && version_ge $gdb_version 16.0; then
+ # GDB has to support MTE in baremetal to allow debugging MTE in QEMU system mode
+ echo "GDB_SUPPORTS_MTE_IN_BAREMETAL=y" >> $config_target_mak
+ fi
+
echo "run-tcg-tests-$target: $qemu\$(EXESUF)" >> Makefile.prereqs
tcg_tests_targets="$tcg_tests_targets $target"
fi
diff --git a/tests/tcg/aarch64/Makefile.softmmu-target b/tests/tcg/aarch64/Makefile.softmmu-target
index 139e04d15f..59ee662cda 100644
--- a/tests/tcg/aarch64/Makefile.softmmu-target
+++ b/tests/tcg/aarch64/Makefile.softmmu-target
@@ -2,14 +2,22 @@
# Aarch64 system tests
#
-AARCH64_SYSTEM_SRC=$(SRC_PATH)/tests/tcg/aarch64/system
+AARCH64_SRC=$(SRC_PATH)/tests/tcg/aarch64
+AARCH64_SYSTEM_SRC=$(AARCH64_SRC)/system
+
VPATH+=$(AARCH64_SYSTEM_SRC)
# These objects provide the basic boot code and helper functions for all tests
CRT_OBJS=boot.o
-AARCH64_TEST_SRCS=$(wildcard $(AARCH64_SYSTEM_SRC)/*.c)
-AARCH64_TESTS = $(patsubst $(AARCH64_SYSTEM_SRC)/%.c, %, $(AARCH64_TEST_SRCS))
+AARCH64_TEST_C_SRCS_=$(wildcard $(AARCH64_SYSTEM_SRC)/*.c)
+AARCH64_TEST_S_SRCS=$(AARCH64_SYSTEM_SRC)/mte.S
+
+AARCH64_C_TESTS = $(patsubst $(AARCH64_SYSTEM_SRC)/%.c, %, $(AARCH64_TEST_C_SRCS))
+AARCH64_S_TESTS = $(patsubst $(AARCH64_SYSTEM_SRC)/%.S, %, $(AARCH64_TEST_S_SRCS))
+
+AARCH64_TESTS = $(AARCH64_C_TESTS)
+AARCH64_TESTS += $(AARCH64_S_TESTS)
CRT_PATH=$(AARCH64_SYSTEM_SRC)
LINK_SCRIPT=$(AARCH64_SYSTEM_SRC)/kernel.ld
@@ -21,7 +29,8 @@ LDFLAGS+=-static -nostdlib $(CRT_OBJS) $(MINILIB_OBJS) -lgcc
config-cc.mak: Makefile
$(quiet-@)( \
- $(call cc-option,-march=armv8.3-a, CROSS_CC_HAS_ARMV8_3)) 3> config-cc.mak
+ $(call cc-option,-march=armv8.3-a, CROSS_CC_HAS_ARMV8_3); \
+ $(call cc-option,-march=armv8.5-a+memtag, CROSS_CC_HAS_ARMV8_MTE)) 3> config-cc.mak
-include config-cc.mak
# building head blobs
@@ -88,3 +97,35 @@ pauth-3:
run-pauth-3:
$(call skip-test, "RUN of pauth-3", "not built")
endif
+
+ifneq ($(CROSS_CC_HAS_ARMV8_MTE),)
+QEMU_MTE_ENABLED_MACHINE=-M virt,mte=on -cpu max -display none
+QEMU_OPTS_WITH_MTE_ON = $(QEMU_MTE_ENABLED_MACHINE) $(QEMU_BASE_ARGS) -kernel
+mte: CFLAGS+=-march=armv8.5-a+memtag
+mte: mte.S $(LINK_SCRIPT) $(CRT_OBJS) $(MINILIB_OBJS)
+ $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS)
+
+run-mte: QEMU_OPTS=$(QEMU_OPTS_WITH_MTE_ON)
+run-mte: mte
+
+ifeq ($(GDB_SUPPORTS_MTE_IN_BAREMETAL),y)
+run-gdbstub-mte: QEMU_OPTS=$(QEMU_OPTS_WITH_MTE_ON)
+run-gdbstub-mte: mte
+ $(call run-test, $@, $(GDB_SCRIPT) \
+ --output run-gdbstub-mte.out \
+ --gdb $(GDB) \
+ --qemu $(QEMU) --qargs "-chardev null$(COMMA)id=output $(QEMU_OPTS)" \
+ --bin $< --test $(AARCH64_SRC)/gdbstub/test-mte.py -- --mode=system, \
+ gdbstub MTE support)
+
+EXTRA_RUNS += run-gdbstub-mte
+else # !GDB_SUPPORTS_MTE_IN_BAREMETAL
+run-gdbstub-mte:
+ $(call skip-test "RUN of gdbstub-mte", "GDB does not support MTE in baremetal!")
+endif
+else # !CROSS_CC_HAS_ARMV8_MTE
+mte:
+ $(call skip-test, "BUILD of $@", "missing compiler support")
+run-mte:
+ $(call skip-test, "RUN of mte", "not build")
+endif
diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target
index 8cc62eb456..9efe2f81ad 100644
--- a/tests/tcg/aarch64/Makefile.target
+++ b/tests/tcg/aarch64/Makefile.target
@@ -138,7 +138,8 @@ run-gdbstub-mte: mte-8
$(call run-test, $@, $(GDB_SCRIPT) \
--gdb $(GDB) \
--qemu $(QEMU) --qargs "$(QEMU_OPTS)" \
- --bin $< --test $(AARCH64_SRC)/gdbstub/test-mte.py, \
+ --bin $< --test $(AARCH64_SRC)/gdbstub/test-mte.py \
+ -- --mode=user, \
gdbstub MTE support)
EXTRA_RUNS += run-gdbstub-mte
diff --git a/tests/tcg/aarch64/gdbstub/test-mte.py b/tests/tcg/aarch64/gdbstub/test-mte.py
index 66f9c25f8a..a4cae6caa0 100644
--- a/tests/tcg/aarch64/gdbstub/test-mte.py
+++ b/tests/tcg/aarch64/gdbstub/test-mte.py
@@ -1,21 +1,26 @@
from __future__ import print_function
#
# Test GDB memory-tag commands that exercise the stubs for the qIsAddressTagged,
-# qMemTag, and QMemTag packets. Logical tag-only commands rely on local
-# operations, hence don't exercise any stub.
+# qMemTag, and QMemTag packets, which are used for manipulating allocation tags.
+# Logical tags-related commands rely on local operations, hence don't exercise
+# any stub and so are not used in this test.
#
-# The test consists in breaking just after a atag() call (which sets the
-# allocation tag -- see mte-8.c for details) and setting/getting tags in
-# different memory locations and ranges starting at the address of the array
-# 'a'.
+# The test consists in breaking just after a tag is set in a specific memory
+# chunk, and then using the GDB 'memory-tagging' subcommands to set/get tags in
+# different memory locations and ranges in the MTE-enabled memory chunk.
#
# This is launched via tests/guest-debug/run-test.py
#
-import gdb
+try:
+ import gdb
+except ModuleNotFoundError:
+ from sys import exit
+ exit("This script must be launched via tests/guest-debug/run-test.py!")
import re
-from test_gdbstub import main, report
+from sys import argv
+from test_gdbstub import arg_parser, main, report
PATTERN_0 = "Memory tags for address 0x[0-9a-f]+ match \\(0x[0-9a-f]+\\)."
@@ -23,12 +28,32 @@
def run_test():
- gdb.execute("break 95", False, True)
+ p = arg_parser(prog="test-mte.py", description="TCG MTE tests.")
+ p.add_argument("--mode", help="Run test for QEMU system or user mode.",
+ required=True, choices=['system','user'])
+
+ args = p.parse_args(args=argv)
+
+ if args.mode == "system":
+ # Break address: where to break before performing the tests
+ # See mte.S for details about this label.
+ ba = "main_end"
+ # Tagged address: the start of the MTE-enabled memory chunk to be tested
+ # 'tagged_addr' (x1) is a pointer to the MTE-enabled page. See mte.S.
+ ta = "$x1"
+ else: # mode="user"
+ # Line 95 in mte-8.c
+ ba = "95"
+ # 'a' array. See mte-8.c
+ ta = "a"
+
+ gdb.execute(f"break {ba}", False, True)
gdb.execute("continue", False, True)
+
try:
- # Test if we can check correctly that the allocation tag for
- # array 'a' matches the logical tag after atag() is called.
- co = gdb.execute("memory-tag check a", False, True)
+ # Test if we can check correctly that the allocation tag for the address
+ # in {ta} matches the logical tag in {ta}.
+ co = gdb.execute(f"memory-tag check {ta}", False, True)
tags_match = re.findall(PATTERN_0, co, re.MULTILINE)
if tags_match:
report(True, f"{tags_match[0]}")
@@ -39,20 +64,20 @@ def run_test():
# tags rely on local operation and so don't exercise any stub.
# Set the allocation tag for the first granule (16 bytes) of
- # address starting at 'a' address to a known value, i.e. 0x04.
- gdb.execute("memory-tag set-allocation-tag a 1 04", False, True)
+ # address starting at {ta} address to a known value, i.e. 0x04.
+ gdb.execute(f"memory-tag set-allocation-tag {ta} 1 04", False, True)
# Then set the allocation tag for the second granule to a known
# value, i.e. 0x06. This tests that contiguous tag granules are
- # set correct and don't run over each other.
- gdb.execute("memory-tag set-allocation-tag a+16 1 06", False, True)
+ # set correctly and don't run over each other.
+ gdb.execute(f"memory-tag set-allocation-tag {ta}+16 1 06", False, True)
# Read the known values back and check if they remain the same.
- co = gdb.execute("memory-tag print-allocation-tag a", False, True)
+ co = gdb.execute(f"memory-tag print-allocation-tag {ta}", False, True)
first_tag = re.match(PATTERN_1, co)[1]
- co = gdb.execute("memory-tag print-allocation-tag a+16", False, True)
+ co = gdb.execute(f"memory-tag print-allocation-tag {ta}+16", False, True)
second_tag = re.match(PATTERN_1, co)[1]
if first_tag == "0x4" and second_tag == "0x6":
@@ -61,15 +86,15 @@ def run_test():
report(False, "Can't set/print allocation tags!")
# Now test fill pattern by setting a whole page with a pattern.
- gdb.execute("memory-tag set-allocation-tag a 4096 0a0b", False, True)
+ gdb.execute(f"memory-tag set-allocation-tag {ta} 4096 0a0b", False, True)
# And read back the tags of the last two granules in page so
# we also test if the pattern is set correctly up to the end of
# the page.
- co = gdb.execute("memory-tag print-allocation-tag a+4096-32", False, True)
+ co = gdb.execute(f"memory-tag print-allocation-tag {ta}+4096-32", False, True)
tag = re.match(PATTERN_1, co)[1]
- co = gdb.execute("memory-tag print-allocation-tag a+4096-16", False, True)
+ co = gdb.execute(f"memory-tag print-allocation-tag {ta}+4096-16", False, True)
last_tag = re.match(PATTERN_1, co)[1]
if tag == "0xa" and last_tag == "0xb":
@@ -78,8 +103,8 @@ def run_test():
report(False, "Fill pattern failed!")
except gdb.error:
- # This usually happens because a GDB version that does not
- # support memory tagging was used to run the test.
+ # This usually happens because a GDB version that does not support
+ # memory tagging was used to run the test.
report(False, "'memory-tag' command failed!")
diff --git a/tests/tcg/aarch64/system/boot.S b/tests/tcg/aarch64/system/boot.S
index 501685d0ec..4eb1b35b88 100644
--- a/tests/tcg/aarch64/system/boot.S
+++ b/tests/tcg/aarch64/system/boot.S
@@ -135,6 +135,17 @@ __start:
orr x1, x1, x3
str x1, [x2] /* 2nd 2mb (.data & .bss)*/
+ /* Third block: at 'mte_page', set in kernel.ld */
+ 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)
+ /* attr(AF, NX, block, AttrIndx=Attr1) */
+ ldr x3, =(3 << 53) | 0x401 | (1 << 2)
+ orr x1, x1, x3
+ str x1, [x2]
+
/* Setup/enable the MMU. */
/*
diff --git a/tests/tcg/aarch64/system/kernel.ld b/tests/tcg/aarch64/system/kernel.ld
index 7b3a76dcbf..46f1092522 100644
--- a/tests/tcg/aarch64/system/kernel.ld
+++ b/tests/tcg/aarch64/system/kernel.ld
@@ -18,6 +18,13 @@ SECTIONS
.bss : {
*(.bss)
}
+ /*
+ * Align the MTE page to the next 2mb boundary (i.e., the third 2mb chunk
+ * starting from 1gb) by setting the address for symbol 'mte_page', which 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 = ALIGN(1 << 22);
/DISCARD/ : {
*(.ARM.attributes)
}
diff --git a/tests/tcg/aarch64/system/mte.S b/tests/tcg/aarch64/system/mte.S
new file mode 100644
index 0000000000..b611240a95
--- /dev/null
+++ b/tests/tcg/aarch64/system/mte.S
@@ -0,0 +1,109 @@
+/*
+ * Code to help test the MTE gdbstubs in system mode.
+ *
+ * Copyright (c) 2024 Linaro Limited
+ *
+ * Author: Gustavo Romero <gustavo.romero@linaro.org>
+ *
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+
+#define addr x0 /* Ptr to the start of the MTE-enabled page. */
+#define tagged_addr x1 /* 'addr' ptr with a random-generated tag added. */
+#define tmp0 x2 /* Scratch register. */
+#define tmp1 x3 /* Scratch register. */
+#define tmp2 x4 /* Scratch register. */
+#define tmp3 x5 /* Sctatch register. */
+
+ .file "mte.S"
+
+ .text
+ .align 4
+
+ .globl main
+ .type main, @function
+
+main:
+ /*
+ * Set MAIR_EL1 (Memory Attribute Index Register). In boot.S, the
+ * attribute index for .mte_page is set to point to MAILR_EL field Attr1
+ * (AttrIndx=Attr1), so set Attr1 as Tagged Normal (MTE) to enable MTE
+ * on this page.
+ *
+ * Attr1 = 0xF0 => Tagged Normal (MTE)
+ */
+ mrs tmp0, mair_el1
+ orr tmp0, tmp0, (0xF0 << 8)
+ msr mair_el1, tmp0
+
+ /*
+ * Set TCR_EL1 (Translation Control Registers) to ignore the top byte
+ * in the translated addresses so it can be used to keep the tags.
+ *
+ * TBI0[37] = 0b1 => Top Byte ignored and used for tagged addresses
+ */
+ mrs tmp1, tcr_el1
+ orr tmp1, tmp1, (1 << 37)
+ msr tcr_el1, tmp1
+
+ /*
+ * Set SCTLR_EL1 (System Control Register) to enable the use of MTE
+ * insns., like stg & friends, and to enable synchronous exception in
+ * case of a tag mismatch, i.e., when the logical tag in 'tagged_addr'
+ * is different from the allocation tag related to 'addr' address.
+ *
+ * ATA[43] = 0b1 => Enable access to allocation tags at EL1
+ * TCF[41:40] = 0b01 => Tag Check Faults cause a synchronous exception
+ *
+ */
+ mrs tmp2, sctlr_el1
+ mov tmp3, (1 << 43) | (1 << 40)
+ orr tmp2, tmp2, tmp3
+ msr sctlr_el1, tmp2
+
+ isb
+
+ /*
+ * MTE-enabled page resides at the 3rd 2MB chunk in the second 1GB
+ * block, i.e., at 0x40400000 address. See .mte_page section in boot.S
+ * and kernel.ld (where the address is effectively computed).
+ *
+ * Load .mte_page address into 'addr' register.
+ */
+ adrp addr, mte_page
+ add addr, addr, :lo12:mte_page
+
+ /*
+ * Set GCR for random tag generation. 0xA5 is just a random value to set
+ * GCR != 0 so the tag generated by 'irg' insn. is not zero, which is
+ * more interesting for the tests than when tag is zero.
+ */
+ mov tmp0, 0xA5
+ msr gcr_el1, tmp0
+
+ /*
+ * Generate a logical tag, add it to 'addr' address and put it into
+ * 'tagged_addr'.
+ */
+ irg tagged_addr, addr
+
+ /*
+ * Store the generated tag to memory region pointed to by 'addr', i.e.
+ * set the allocation tag for granule at 'addr'. The tag is extracted
+ * by stg from tagged_addr pointer.
+ */
+ stg tagged_addr, [addr]
+
+ /*
+ * Store a random value (0xdeadbeef) to tagged_addr address. This must
+ * not cause any Tag Check Fault since logical tag in tagged_addr and
+ * allocation tag associated with the memory pointed by tagged_addr are
+ * set the same, otherwise something is off and the test fails -- an
+ * exception is generated.
+ */
+ ldr tmp1, =0xdeadbeef
+ str tmp1, [tagged_addr]
+
+ /* This label is used by GDB Python script test-mte.py. */
+main_end:
+ ret
--
2.34.1Hi Gustavo,
On 25/8/24 16:52, Gustavo Romero wrote:
> Extend MTE gdbstub tests to also run in system mode (share tests between
> user mode and system mode). The tests will only run if a version of GDB
> that supports MTE on baremetal is available in the test environment and
> if available compiler supports the 'memtag' flag
> (-march=armv8.5-a+memtag).
>
> For the tests running in system mode, a page that supports MTE ops. is
> necessary. Therefore, an MTE-enabled page is made available (mapped) in
> the third 2 MB chunk of the second 1 GB space in the flat mapping set in
> boot.S. A new binary, mte.S, is also introduced for the tests. It links
> against boot.S and is executed by QEMU in system mode.
>
> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> ---
> configure | 5 +
> tests/tcg/aarch64/Makefile.softmmu-target | 49 +++++++++-
> tests/tcg/aarch64/Makefile.target | 3 +-
> tests/tcg/aarch64/gdbstub/test-mte.py | 71 +++++++++-----
> tests/tcg/aarch64/system/boot.S | 11 +++
> tests/tcg/aarch64/system/kernel.ld | 7 ++
> tests/tcg/aarch64/system/mte.S | 109 ++++++++++++++++++++++
> 7 files changed, 227 insertions(+), 28 deletions(-)
> create mode 100644 tests/tcg/aarch64/system/mte.S
> diff --git a/tests/tcg/aarch64/system/kernel.ld b/tests/tcg/aarch64/system/kernel.ld
> index 7b3a76dcbf..46f1092522 100644
> --- a/tests/tcg/aarch64/system/kernel.ld
> +++ b/tests/tcg/aarch64/system/kernel.ld
> @@ -18,6 +18,13 @@ SECTIONS
> .bss : {
> *(.bss)
> }
> + /*
> + * Align the MTE page to the next 2mb boundary (i.e., the third 2mb chunk
> + * starting from 1gb) by setting the address for symbol 'mte_page', which 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 = ALIGN(1 << 22);
Comment says 2MiB but you use 4MiB.
Matter of taste, 2MiB is easier to review as:
mte_page = ALIGN(2 << 20);
> /DISCARD/ : {
> *(.ARM.attributes)
> }
Hi Phil!
On 8/26/24 3:10 AM, Philippe Mathieu-Daudé wrote:
> Hi Gustavo,
>
> On 25/8/24 16:52, Gustavo Romero wrote:
>> Extend MTE gdbstub tests to also run in system mode (share tests between
>> user mode and system mode). The tests will only run if a version of GDB
>> that supports MTE on baremetal is available in the test environment and
>> if available compiler supports the 'memtag' flag
>> (-march=armv8.5-a+memtag).
>>
>> For the tests running in system mode, a page that supports MTE ops. is
>> necessary. Therefore, an MTE-enabled page is made available (mapped) in
>> the third 2 MB chunk of the second 1 GB space in the flat mapping set in
>> boot.S. A new binary, mte.S, is also introduced for the tests. It links
>> against boot.S and is executed by QEMU in system mode.
>>
>> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
>> ---
>> configure | 5 +
>> tests/tcg/aarch64/Makefile.softmmu-target | 49 +++++++++-
>> tests/tcg/aarch64/Makefile.target | 3 +-
>> tests/tcg/aarch64/gdbstub/test-mte.py | 71 +++++++++-----
>> tests/tcg/aarch64/system/boot.S | 11 +++
>> tests/tcg/aarch64/system/kernel.ld | 7 ++
>> tests/tcg/aarch64/system/mte.S | 109 ++++++++++++++++++++++
>> 7 files changed, 227 insertions(+), 28 deletions(-)
>> create mode 100644 tests/tcg/aarch64/system/mte.S
>
>
>> diff --git a/tests/tcg/aarch64/system/kernel.ld b/tests/tcg/aarch64/system/kernel.ld
>> index 7b3a76dcbf..46f1092522 100644
>> --- a/tests/tcg/aarch64/system/kernel.ld
>> +++ b/tests/tcg/aarch64/system/kernel.ld
>> @@ -18,6 +18,13 @@ SECTIONS
>> .bss : {
>> *(.bss)
>> }
>> + /*
>> + * Align the MTE page to the next 2mb boundary (i.e., the third 2mb chunk
>> + * starting from 1gb) by setting the address for symbol 'mte_page', which 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 = ALIGN(1 << 22);
>
> Comment says 2MiB but you use 4MiB.
> Matter of taste, 2MiB is easier to review as:
>
> mte_page = ALIGN(2 << 20);
>
This is incorrect. Aligning here at 2MiB will clash with r/w data section. I tried to
clarify it when I wrote "[...] third 2mb chunk starting from 1gb". The memory layout
so is:
0 ---- 1GiB range: avoid/skipped
1GiB ---- 1GiB+2MiB (1st 2MiB chunk): text
1GiB+2MiB ---- 1GiB+4MiB (2nd 2MiB chunk): data
1GiB+4MIB ---- 1GiB+8MiB (3rd 2MiB chunk): MTE-enabled page
ALIGN implicitly uses "." (current position), so we're defining consecutive sections
here starting from 1GiB.
All the other parts of the code uses the 1 << n form, which I prefer, specially when
reading asm code, so I prefer 1 << 22 instead of 4 << 20.
But apparently my comment failed miserable to explain the situation here, so I'm
happy to change the wording to make it clearer if you have any suggestion.
Cheers,
Gustavo
>> /DISCARD/ : {
>> *(.ARM.attributes)
>> }
>
Hi Phil,
On 8/27/24 9:42 AM, Gustavo Romero wrote:
> Hi Phil!
>
> On 8/26/24 3:10 AM, Philippe Mathieu-Daudé wrote:
>> Hi Gustavo,
>>
>> On 25/8/24 16:52, Gustavo Romero wrote:
>>> Extend MTE gdbstub tests to also run in system mode (share tests between
>>> user mode and system mode). The tests will only run if a version of GDB
>>> that supports MTE on baremetal is available in the test environment and
>>> if available compiler supports the 'memtag' flag
>>> (-march=armv8.5-a+memtag).
>>>
>>> For the tests running in system mode, a page that supports MTE ops. is
>>> necessary. Therefore, an MTE-enabled page is made available (mapped) in
>>> the third 2 MB chunk of the second 1 GB space in the flat mapping set in
>>> boot.S. A new binary, mte.S, is also introduced for the tests. It links
>>> against boot.S and is executed by QEMU in system mode.
>>>
>>> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
>>> ---
>>> configure | 5 +
>>> tests/tcg/aarch64/Makefile.softmmu-target | 49 +++++++++-
>>> tests/tcg/aarch64/Makefile.target | 3 +-
>>> tests/tcg/aarch64/gdbstub/test-mte.py | 71 +++++++++-----
>>> tests/tcg/aarch64/system/boot.S | 11 +++
>>> tests/tcg/aarch64/system/kernel.ld | 7 ++
>>> tests/tcg/aarch64/system/mte.S | 109 ++++++++++++++++++++++
>>> 7 files changed, 227 insertions(+), 28 deletions(-)
>>> create mode 100644 tests/tcg/aarch64/system/mte.S
>>
>>
>>> diff --git a/tests/tcg/aarch64/system/kernel.ld b/tests/tcg/aarch64/system/kernel.ld
>>> index 7b3a76dcbf..46f1092522 100644
>>> --- a/tests/tcg/aarch64/system/kernel.ld
>>> +++ b/tests/tcg/aarch64/system/kernel.ld
>>> @@ -18,6 +18,13 @@ SECTIONS
>>> .bss : {
>>> *(.bss)
>>> }
>>> + /*
>>> + * Align the MTE page to the next 2mb boundary (i.e., the third 2mb chunk
>>> + * starting from 1gb) by setting the address for symbol 'mte_page', which 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 = ALIGN(1 << 22);
>>
>> Comment says 2MiB but you use 4MiB.
>> Matter of taste, 2MiB is easier to review as:
>>
>> mte_page = ALIGN(2 << 20);
>>
>
> This is incorrect. Aligning here at 2MiB will clash with r/w data section. I tried to
> clarify it when I wrote "[...] third 2mb chunk starting from 1gb". The memory layout
> so is:
>
> 0 ---- 1GiB range: avoid/skipped
> 1GiB ---- 1GiB+2MiB (1st 2MiB chunk): text
> 1GiB+2MiB ---- 1GiB+4MiB (2nd 2MiB chunk): data
> 1GiB+4MIB ---- 1GiB+8MiB (3rd 2MiB chunk): MTE-enabled page
>
> ALIGN implicitly uses "." (current position), so we're defining consecutive sections
> here starting from 1GiB.
>
> All the other parts of the code uses the 1 << n form, which I prefer, specially when
> reading asm code, so I prefer 1 << 22 instead of 4 << 20.
Alex suggested using the M suffix to make it more clear, so how about:
diff --git a/tests/tcg/aarch64/system/kernel.ld b/tests/tcg/aarch64/system/kernel.ld
index 46f1092522..3a28412b2f 100644
--- a/tests/tcg/aarch64/system/kernel.ld
+++ b/tests/tcg/aarch64/system/kernel.ld
@@ -2,16 +2,18 @@ ENTRY(__start)
SECTIONS
{
- /* virt machine, RAM starts at 1gb */
+ /* Skip first 1 GiB on virt machine: RAM starts at 1 GiB. */
. = (1 << 30);
+ /* Align text to first 2 MiB. */
+ . = ALIGN(0 * 2M);
.text : {
*(.text)
}
.rodata : {
*(.rodata)
}
- /* align r/w section to next 2mb */
- . = ALIGN(1 << 21);
+ /* Align r/w section to next 2 MiB. */
+ . = ALIGN(1 * 2M);
.data : {
*(.data)
}
@@ -19,12 +21,12 @@ SECTIONS
*(.bss)
}
/*
- * Align the MTE page to the next 2mb boundary (i.e., the third 2mb chunk
- * starting from 1gb) by setting the address for symbol 'mte_page', which is
- * used in boot.S to setup the PTE and in the mte.S test as the address that
- * the MTE instructions operate on.
+ * Align the MTE page to the next 2 MiB boundary (i.e., the third 2 MiB
+ * chunk starting from 1 GiB) by setting the address for symbol 'mte_page',
+ * which 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 = ALIGN(1 << 22);
+ mte_page = ALIGN(2 * 2M);
/DISCARD/ : {
*(.ARM.attributes)
}
Cheers,
Gustavo
> But apparently my comment failed miserable to explain the situation here, so I'm
> happy to change the wording to make it clearer if you have any suggestion.
>
>
> Cheers,
> Gustavo
>
>>> /DISCARD/ : {
>>> *(.ARM.attributes)
>>> }
>>
On 8/28/24 04:01, Gustavo Romero wrote:
> SECTIONS
> {
> - /* virt machine, RAM starts at 1gb */
> + /* Skip first 1 GiB on virt machine: RAM starts at 1 GiB. */
> . = (1 << 30);
Better is to use
MEMORY {
RAM (rwx) : ORIGIN = 1 << 30, LENGTH = 16M
}
(or whatever minimum length seems reasonable).
Since there is only one memory region, it will be used by default and no further markup is
required.
> + /* Align text to first 2 MiB. */
> + . = ALIGN(0 * 2M);
This is pointless, of course: ALIGN(0) does nothing.
> @@ -19,12 +21,12 @@ SECTIONS
> *(.bss)
> }
> /*
> - * Align the MTE page to the next 2mb boundary (i.e., the third 2mb chunk
> - * starting from 1gb) by setting the address for symbol 'mte_page', which is
> - * used in boot.S to setup the PTE and in the mte.S test as the address that
> - * the MTE instructions operate on.
> + * Align the MTE page to the next 2 MiB boundary (i.e., the third 2 MiB
> + * chunk starting from 1 GiB) by setting the address for symbol 'mte_page',
> + * which 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 = ALIGN(1 << 22);
> + mte_page = ALIGN(2 * 2M);
This does not do what you think it does.
It aligns to the next 4M boundary, not the next 2M boundary.
r~
Hi Richard,
On 8/28/24 9:43 PM, Richard Henderson wrote:
> On 8/28/24 04:01, Gustavo Romero wrote:
>> SECTIONS
>> {
>> - /* virt machine, RAM starts at 1gb */
>> + /* Skip first 1 GiB on virt machine: RAM starts at 1 GiB. */
>> . = (1 << 30);
>
> Better is to use
>
> MEMORY {
> RAM (rwx) : ORIGIN = 1 << 30, LENGTH = 16M
> }
>
> (or whatever minimum length seems reasonable).
>
> Since there is only one memory region, it will be used by default and no further markup is required.
>
>> + /* Align text to first 2 MiB. */
>> + . = ALIGN(0 * 2M);
>
> This is pointless, of course: ALIGN(0) does nothing.
>
>> @@ -19,12 +21,12 @@ SECTIONS
>> *(.bss)
>> }
>> /*
>> - * Align the MTE page to the next 2mb boundary (i.e., the third 2mb chunk
>> - * starting from 1gb) by setting the address for symbol 'mte_page', which is
>> - * used in boot.S to setup the PTE and in the mte.S test as the address that
>> - * the MTE instructions operate on.
>> + * Align the MTE page to the next 2 MiB boundary (i.e., the third 2 MiB
>> + * chunk starting from 1 GiB) by setting the address for symbol 'mte_page',
>> + * which 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 = ALIGN(1 << 22);
>> + mte_page = ALIGN(2 * 2M);
>
> This does not do what you think it does.
> It aligns to the next 4M boundary, not the next 2M boundary.
I see now. This works because by chance ALIGN(2M) == ALIGN(4M).
So, using the 'M' suffix, I understand this is correct (and works):
(diff against the my patch)
index 46f1092522..823d47f7e7 100644
--- a/tests/tcg/aarch64/system/kernel.ld
+++ b/tests/tcg/aarch64/system/kernel.ld
@@ -11,7 +11,7 @@ SECTIONS
*(.rodata)
}
/* align r/w section to next 2mb */
- . = ALIGN(1 << 21);
+ . = ALIGN(2M);
.data : {
*(.data)
}
@@ -24,7 +24,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 = ALIGN(2M);
/DISCARD/ : {
*(.ARM.attributes)
}
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?
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
Cheers,
Gustavo
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.
r~
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
© 2016 - 2024 Red Hat, Inc.