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.rom...@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)
}
