[PATCH 0/3] FEAT_RME_GDI initial work

[PATCH 0/3] FEAT_RME_GDI initial work

[Jim MacArthur]

Adds FEAT_RME_GDI, some extra cases to arm_granule_protection_check,
and a very basic TCG test to check the prerequisites of FEAT_RME_GDI:
FEAT_RME and FEAT_RME_GPC2.

FEAT_RME_GDI is not enabled yet as no functional support has been
added; it can easily be enabled in cpu_arm_set_rme in future.

In the future we should correctly set bits in the fault address
registers such as PFAR_EL2, but I don't see any apparent support for
these in the existing GPT mechanism. Testing this also gets tricky as
I think this would have to be done from EL3 (e.g. by modifying
test_rme_virt.py)

Signed-off-by: Jim MacArthur <jim.macarthur@linaro.org>
---
Jim MacArthur (3):
      target/arm/cpu-features.c: Add RMEGDI to AA64MMFR4
      target/arm/ptw.c: Add two new spaces to the granule protection case
      tests/tcg/aarch64/system/rme_gdi.c: Very basic test of GDI

 target/arm/cpu-features.h          |  1 +
 target/arm/ptw.c                   |  5 ++++
 tests/tcg/aarch64/system/rme_gdi.c | 56 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 62 insertions(+)
---
base-commit: da6c4fe60fee30dd77267764d55b38af9cb89d4b
change-id: 20260416-jmac-feat_rme_gdi-71c2d2dd6770

Best regards,
-- 
Jim MacArthur <jim.macarthur@linaro.org>

[PATCH 1/3] target/arm/cpu-features.c: Add RMEGDI to AA64MMFR4

[Jim MacArthur]

Signed-off-by: Jim MacArthur <jim.macarthur@linaro.org>
---
 target/arm/cpu-features.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/arm/cpu-features.h b/target/arm/cpu-features.h
index b683c9551a..3f2a9a0c36 100644
--- a/target/arm/cpu-features.h
+++ b/target/arm/cpu-features.h
@@ -348,6 +348,7 @@ FIELD(ID_AA64MMFR3, SPEC_FPACC, 60, 4)
 
 FIELD(ID_AA64MMFR4, ASID2, 8, 4)
 FIELD(ID_AA64MMFR4, E2H0, 24, 4)
+FIELD(ID_AA64MMFR4, RMEGDI, 28, 4)
 
 FIELD(ID_AA64DFR0, DEBUGVER, 0, 4)
 FIELD(ID_AA64DFR0, TRACEVER, 4, 4)

-- 
2.43.0

Re: [PATCH 1/3] target/arm/cpu-features.c: Add RMEGDI to AA64MMFR4

[Richard Henderson]

On 4/16/26 21:12, Jim MacArthur wrote:
> Signed-off-by: Jim MacArthur <jim.macarthur@linaro.org>
> ---
>   target/arm/cpu-features.h | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/target/arm/cpu-features.h b/target/arm/cpu-features.h
> index b683c9551a..3f2a9a0c36 100644
> --- a/target/arm/cpu-features.h
> +++ b/target/arm/cpu-features.h
> @@ -348,6 +348,7 @@ FIELD(ID_AA64MMFR3, SPEC_FPACC, 60, 4)
>   
>   FIELD(ID_AA64MMFR4, ASID2, 8, 4)
>   FIELD(ID_AA64MMFR4, E2H0, 24, 4)
> +FIELD(ID_AA64MMFR4, RMEGDI, 28, 4)
>   
>   FIELD(ID_AA64DFR0, DEBUGVER, 0, 4)
>   FIELD(ID_AA64DFR0, TRACEVER, 4, 4)
> 

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

It's often worthwhile to simply sync the entire register with the docs.
The current M.a.a document has 10 fields here.


r~

Re: [PATCH 1/3] target/arm/cpu-features.c: Add RMEGDI to AA64MMFR4

[Peter Maydell]

On Thu, 16 Apr 2026 at 23:15, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 4/16/26 21:12, Jim MacArthur wrote:
> > Signed-off-by: Jim MacArthur <jim.macarthur@linaro.org>
> > ---
> >   target/arm/cpu-features.h | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/target/arm/cpu-features.h b/target/arm/cpu-features.h
> > index b683c9551a..3f2a9a0c36 100644
> > --- a/target/arm/cpu-features.h
> > +++ b/target/arm/cpu-features.h
> > @@ -348,6 +348,7 @@ FIELD(ID_AA64MMFR3, SPEC_FPACC, 60, 4)
> >
> >   FIELD(ID_AA64MMFR4, ASID2, 8, 4)
> >   FIELD(ID_AA64MMFR4, E2H0, 24, 4)
> > +FIELD(ID_AA64MMFR4, RMEGDI, 28, 4)
> >
> >   FIELD(ID_AA64DFR0, DEBUGVER, 0, 4)
> >   FIELD(ID_AA64DFR0, TRACEVER, 4, 4)
> >
>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
>
> It's often worthwhile to simply sync the entire register with the docs.
> The current M.a.a document has 10 fields here.

FYI, as of last week the current version of the Arm ARM is M.b :-)

-- PMM

[PATCH 2/3] target/arm/ptw.c: Add two new spaces to the granule protection case

[Jim MacArthur]

This makes no functional difference, it just marks out two cases for
future use.

Signed-off-by: Jim MacArthur <jim.macarthur@linaro.org>
---
 target/arm/ptw.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 7b993bb5b3..d4b63a365a 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -510,6 +510,11 @@ bool arm_granule_protection_check(ARMGranuleProtectionConfig config,
         break;
     case 0b1111: /* all access */
         return true;
+    case 0b0100: /* system agent only */
+    case 0b0101: /* non-secure protected */
+        /* System Agent and Non-secure Protected are GDI extensions. */
+        /* No processing element should have access to these. */
+        goto fault_walk;
     case 0b1000: /* secure */
         if (!config.support_sel2) {
             goto fault_walk;

-- 
2.43.0

Re: [PATCH 2/3] target/arm/ptw.c: Add two new spaces to the granule protection case

[Richard Henderson]

On 4/16/26 21:12, Jim MacArthur wrote:
> This makes no functional difference, it just marks out two cases for
> future use.
> 
> Signed-off-by: Jim MacArthur <jim.macarthur@linaro.org>
> ---
>   target/arm/ptw.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/target/arm/ptw.c b/target/arm/ptw.c
> index 7b993bb5b3..d4b63a365a 100644
> --- a/target/arm/ptw.c
> +++ b/target/arm/ptw.c
> @@ -510,6 +510,11 @@ bool arm_granule_protection_check(ARMGranuleProtectionConfig config,
>           break;
>       case 0b1111: /* all access */
>           return true;
> +    case 0b0100: /* system agent only */
> +    case 0b0101: /* non-secure protected */
> +        /* System Agent and Non-secure Protected are GDI extensions. */
> +        /* No processing element should have access to these. */
> +        goto fault_walk;
>       case 0b1000: /* secure */
>           if (!config.support_sel2) {
>               goto fault_walk;
> 

There's no real point to this patch, since we get the same effect from the default case.

It should be trivial to "properly" implement these two cases for the CPU, since

     R_QCLPTA:  PE is not permitted to access memory within the NSP
     or the SA PA spaces, from any Security state.

Access to these spaces is only allowed from external devices via the SMMU.

The only effect on the PE is to check the various GPCCR fields (SA, NSP, NA6, NA7) to 
transform them from one type of fault to another: fault_walk -> fault_fail (i.e. break).

There will have to be some modifications at a later date to actually allow these cases for 
the SMMU, but at this point none of the SMMU patches to use GPC are upstream.  So it's not 
worth worrying about at this time.


r~

Re: [PATCH 2/3] target/arm/ptw.c: Add two new spaces to the granule protection case

[Jim MacArthur]

On Fri, Apr 17, 2026 at 08:36:52AM +1000, Richard Henderson wrote:
> On 4/16/26 21:12, Jim MacArthur wrote:
> > This makes no functional difference, it just marks out two cases for
> > future use.
> > 
> > Signed-off-by: Jim MacArthur <jim.macarthur@linaro.org>
> > ---
> >   target/arm/ptw.c | 5 +++++
> >   1 file changed, 5 insertions(+)
> > 
> > diff --git a/target/arm/ptw.c b/target/arm/ptw.c
> > index 7b993bb5b3..d4b63a365a 100644
> > --- a/target/arm/ptw.c
> > +++ b/target/arm/ptw.c
> > @@ -510,6 +510,11 @@ bool arm_granule_protection_check(ARMGranuleProtectionConfig config,
> >           break;
> >       case 0b1111: /* all access */
> >           return true;
> > +    case 0b0100: /* system agent only */
> > +    case 0b0101: /* non-secure protected */
> > +        /* System Agent and Non-secure Protected are GDI extensions. */
> > +        /* No processing element should have access to these. */
> > +        goto fault_walk;
> >       case 0b1000: /* secure */
> >           if (!config.support_sel2) {
> >               goto fault_walk;
> > 
> 
> There's no real point to this patch, since we get the same effect from the default case.
> 
> It should be trivial to "properly" implement these two cases for the CPU, since
> 
>     R_QCLPTA:  PE is not permitted to access memory within the NSP
>     or the SA PA spaces, from any Security state.
> 
> Access to these spaces is only allowed from external devices via the SMMU.
> 
> The only effect on the PE is to check the various GPCCR fields (SA, NSP,
> NA6, NA7) to transform them from one type of fault to another: fault_walk ->
> fault_fail (i.e. break).

The thing that held me back here was the mapping from 'Reserved' in the table in D9.6.5 to fault_walk, which wasn't clear in the documentation, and hence leaving the behaviour unchanged. However, I would agree that we can map (GPCCR.SA=1, gpi=0x0x100) to fault_fail and so on and leave the (SA=0, 0b0100) case to what it would have been before (fault_walk).

Thanks for your feedback,

Jim

[PATCH 3/3] tests/tcg/aarch64/system/rme_gdi.c: Very basic test of GDI

[Jim MacArthur]

Simply tests GDI's prerequisites; that if GDI is enabled then
so are FEAT_RME and FEAT_RME_GPC2.

Signed-off-by: Jim MacArthur <jim.macarthur@linaro.org>
---
 tests/tcg/aarch64/system/rme_gdi.c | 56 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/tests/tcg/aarch64/system/rme_gdi.c b/tests/tcg/aarch64/system/rme_gdi.c
new file mode 100644
index 0000000000..0921b791bd
--- /dev/null
+++ b/tests/tcg/aarch64/system/rme_gdi.c
@@ -0,0 +1,56 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ *
+ * FEAT_RME_GDI Feature presence and enabled bits test
+ *
+ * Copyright (c) 2026 Linaro Ltd
+ *
+ */
+
+#include <stdint.h>
+#include <minilib.h>
+
+#define ID_AA64PFR0_EL1 "S3_0_C0_C4_0"
+#define ID_AA64MMFR4_EL1 "S3_0_C0_C7_4"
+
+int main()
+{
+    uint64_t mmfr4;
+    uint64_t pfr0;
+    int rme_status;
+    int rmegdi_status;
+
+    asm("mrs %[pfr0], " ID_AA64PFR0_EL1 "\n\t"
+        : [pfr0] "=r" (pfr0));
+
+    /* rme_status is 1 for RME, 2 for RME + GPC2, 3 for RME+GPC3 */
+    rme_status = (pfr0 >> 52) & 0xF;
+
+    asm("mrs %[mmfr4], " ID_AA64MMFR4_EL1 "\n\t"
+        : [mmfr4] "=r" (mmfr4));
+
+    rmegdi_status = ((mmfr4 >> 28) & 0xF);
+
+    switch (rmegdi_status) {
+    case 0:
+        ml_printf("SKIP: GDI not implemented\n");
+        return 0;
+    case 1:
+        /* GDI is implemented, so continue testing */
+        break;
+    default:
+        ml_printf("FAIL: GDI status is %d, only values 0 and 1 are defined\n",
+                  rmegdi_status);
+        return 1;
+    }
+
+    if (rmegdi_status != 0) {
+        /* Check FEAT_RME and FEAT_RME_GPC2 also present */
+        if (rme_status < 2) {
+            ml_printf("FAIL: GDI is %d, but RME is %d; RME should be >= 2\n",
+                      rmegdi_status, rme_status);
+            return 1;
+        }
+    }
+    return 0;
+}

-- 
2.43.0