Re: [PATCH] target/arm: add support for FEAT_MTE_TAGGED_FAR

[Gustavo Romero <gustavo.romero@linaro.org>]

Hi Gabriel,

Thanks for your contribution.

On 11/4/25 21:50, Gabriel Brookman wrote:
> FEAT_MTE_TAGGED_FAR is a feature required for MTE4. The feature
> guarantees that the full address (including tag bits) is reported after
> a SEGV_MTESERR, and advertises itself in the ID_AA64PFR2_EL1 system
> register. QEMU was already reporting the full address, so this commit
> simply advertises the feature by setting that register, and unsets the
> register if MTE is disabled.
> 
> Signed-off-by: Gabriel Brookman <brookmangabriel@gmail.com>
> ---
> This patch is the first step toward implementing ARM's Enhanced Memory
> Tagging Extension (MTE4). MTE4 guarantees the presence of several
> subfeatures: FEAT_MTE_CANONICAL_TAGS, FEAT_MTE_TAGGED_FAR,
> FEAT_MTE_STORE_ONLY, FEAT_MTE_NO_ADDRESS_TAGS, and FEAT_MTE_PERM,
> none of which are currently implemented in QEMU.
> 
> According to the ARM ARM, the presence of any of these features (except
> FEAT_MTE_PERM) implies the presence of all the others. For simplicity
> and ease of review, I plan to introduce them one at a time. This first
> patch focuses on FEAT_MTE_TAGGED_FAR.

I think it's ok to add these "subfeatures" separately.

You need another patch to add FEAT_MTE_TAGGED_FAR to:

docs/system/arm/emulation.rst

Also, please adjust the title to something:

target/arm: Advertise FEAT_MTE_TAGGED_FAR for -cpu max

instead of "add support...", I think it's a better description for
the changes done here.


> FEAT_MTE_TAGGED_FAR guarantees that the full fault address (including
> tag bits) is reported after a SEGV_MTESERR, and exposes itself in the
> ID_AA64PFR2_EL1 register. QEMU already reports the full address in this
> case, so this change only advertises the feature by setting the
> appropriate field in ID_AA64PFR2_EL1. The field is cleared when MTE
> support is disabled or rolled back to instruction-only.
> 
> Testing:
> - Verified in system mode that the MTEFAR field in ID_AA64PFR2_EL1
> is set to 1 when running with mte=on and cleared with mte=off.

If you want to add a test like that it's good, yeah, but we don't usually
test the features this way.


> - Verified in user mode test that SEGV_MTESERR faults report the full
> tagged address as expected.

Yeah, that would be good if you can add a test like that. Maybe use
as a starting point some test in the mte-[1-8].c tests.

  > I didn’t include these checks as formal tests since the functionality is
> simple, but I can add them in follow-up versions if reviewers prefer.
> 
> Follow-up patches will implement the remaining MTE4 subfeatures listed
> above.
> 
> Thanks,
> Gabriel Brookman
> ---
>   target/arm/cpu.c       | 4 +++-
>   target/arm/tcg/cpu64.c | 4 ++++
>   2 files changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 39292fb9bc..804e70b235 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ -2020,6 +2020,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
>            */
>           if (tcg_enabled() && cpu->tag_memory == NULL) {
>               FIELD_DP64_IDREG(isar, ID_AA64PFR1, MTE, 1);
> +            FIELD_DP64_IDREG(isar, ID_AA64PFR2, MTEFAR, 0);
>           }
>   
>           /*
> @@ -2027,7 +2028,8 @@ static void arm_cpu_realizefn(DeviceState *dev, Error **errp)
>            * enabled on the guest (i.e mte=off), clear guest's MTE bits."
>            */
>           if (kvm_enabled() && !cpu->kvm_mte) {
> -                FIELD_DP64_IDREG(isar, ID_AA64PFR1, MTE, 0);
> +            FIELD_DP64_IDREG(isar, ID_AA64PFR1, MTE, 0);
> +            FIELD_DP64_IDREG(isar, ID_AA64PFR2, MTEFAR, 0);
>           }
>   #endif
>       }
> diff --git a/target/arm/tcg/cpu64.c b/target/arm/tcg/cpu64.c
> index 6871956382..27f0b43256 100644
> --- a/target/arm/tcg/cpu64.c
> +++ b/target/arm/tcg/cpu64.c
> @@ -1283,6 +1283,10 @@ void aarch64_max_tcg_initfn(Object *obj)
>       t = FIELD_DP64(t, ID_AA64PFR1, GCS, 1);       /* FEAT_GCS */
>       SET_IDREG(isar, ID_AA64PFR1, t);
>   
> +    t = GET_IDREG(isar, ID_AA64PFR2);
> +    t = FIELD_DP64(t, ID_AA64PFR2, MTEFAR, 1);    /* FEAT_MTE_TAGGED_FAR */
> +    SET_IDREG(isar, ID_AA64PFR2, t);
> +
>       t = GET_IDREG(isar, ID_AA64MMFR0);
>       t = FIELD_DP64(t, ID_AA64MMFR0, PARANGE, 6); /* FEAT_LPA: 52 bits */
>       t = FIELD_DP64(t, ID_AA64MMFR0, TGRAN16, 1);   /* 16k pages supported */

The changes in code above look good to me.


Cheers,
Gustavo