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

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

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);

>       /DISCARD/ : {
>           *(.ARM.attributes)
>       }