qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V5 0/4] Basic ASID2 support
@ 2025-12-04 18:04 Jim MacArthur
  2025-12-04 18:04 ` [PATCH 1/4] target/arm: Enable ID_AA64MMFR4_EL1 register Jim MacArthur
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Jim MacArthur @ 2025-12-04 18:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: Jim MacArthur

Thanks to Gustavo Romero for reviews.

Changes in v5:
- Patch 2:
  - TLB flush when A2/FNG0/FNG1 could be written to.
- Patch 4:
  - SPDX License identifier moved to first line.

Jim MacArthur (4):
  target/arm: Enable ID_AA64MMFR4_EL1 register
  target/arm: Allow writes to FNG1, FNG0, A2
  target/arm/tcg/cpu64.c: Enable ASID2 for cpu_max
  tests: Add test for ASID2 and write/read of feature bits

 docs/system/arm/emulation.rst    |  1 +
 target/arm/cpu-features.h        |  7 +++
 target/arm/cpu-sysregs.h.inc     |  1 +
 target/arm/helper.c              | 22 ++++++++-
 target/arm/tcg/cpu64.c           |  4 ++
 tests/tcg/aarch64/system/asid2.c | 76 ++++++++++++++++++++++++++++++++
 6 files changed, 109 insertions(+), 2 deletions(-)
 create mode 100644 tests/tcg/aarch64/system/asid2.c

-- 
2.43.0



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/4] target/arm: Enable ID_AA64MMFR4_EL1 register
  2025-12-04 18:04 [PATCH V5 0/4] Basic ASID2 support Jim MacArthur
@ 2025-12-04 18:04 ` Jim MacArthur
  2025-12-04 18:04 ` [PATCH 2/4] target/arm: Allow writes to FNG1, FNG0, A2 Jim MacArthur
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Jim MacArthur @ 2025-12-04 18:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: Jim MacArthur

Signed-off-by: Jim MacArthur <jim.macarthur@linaro.org>
---
 target/arm/cpu-sysregs.h.inc | 1 +
 target/arm/helper.c          | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/target/arm/cpu-sysregs.h.inc b/target/arm/cpu-sysregs.h.inc
index 2bb2861c62..2ba49d8478 100644
--- a/target/arm/cpu-sysregs.h.inc
+++ b/target/arm/cpu-sysregs.h.inc
@@ -14,6 +14,7 @@ DEF(ID_AA64MMFR0_EL1, 3, 0, 0, 7, 0)
 DEF(ID_AA64MMFR1_EL1, 3, 0, 0, 7, 1)
 DEF(ID_AA64MMFR2_EL1, 3, 0, 0, 7, 2)
 DEF(ID_AA64MMFR3_EL1, 3, 0, 0, 7, 3)
+DEF(ID_AA64MMFR4_EL1, 3, 0, 0, 7, 4)
 DEF(ID_PFR0_EL1, 3, 0, 0, 1, 0)
 DEF(ID_PFR1_EL1, 3, 0, 0, 1, 1)
 DEF(ID_DFR0_EL1, 3, 0, 0, 1, 2)
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 27ebc6f29b..c20334fa65 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6566,11 +6566,11 @@ void register_cp_regs_for_features(ARMCPU *cpu)
               .access = PL1_R, .type = ARM_CP_CONST,
               .accessfn = access_aa64_tid3,
               .resetvalue = GET_IDREG(isar, ID_AA64MMFR3) },
-            { .name = "ID_AA64MMFR4_EL1_RESERVED", .state = ARM_CP_STATE_AA64,
+            { .name = "ID_AA64MMFR4_EL1", .state = ARM_CP_STATE_AA64,
               .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 7, .opc2 = 4,
               .access = PL1_R, .type = ARM_CP_CONST,
               .accessfn = access_aa64_tid3,
-              .resetvalue = 0 },
+              .resetvalue = GET_IDREG(isar, ID_AA64MMFR4) },
             { .name = "ID_AA64MMFR5_EL1_RESERVED", .state = ARM_CP_STATE_AA64,
               .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 7, .opc2 = 5,
               .access = PL1_R, .type = ARM_CP_CONST,
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/4] target/arm: Allow writes to FNG1, FNG0, A2
  2025-12-04 18:04 [PATCH V5 0/4] Basic ASID2 support Jim MacArthur
  2025-12-04 18:04 ` [PATCH 1/4] target/arm: Enable ID_AA64MMFR4_EL1 register Jim MacArthur
@ 2025-12-04 18:04 ` Jim MacArthur
  2025-12-05 15:30   ` Richard Henderson
  2025-12-04 18:04 ` [PATCH 3/4] target/arm/tcg/cpu64.c: Enable ASID2 for cpu_max Jim MacArthur
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Jim MacArthur @ 2025-12-04 18:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: Jim MacArthur

This just allows read/write of three feature bits. ASID is still
ignored. Any writes to TTBR0_EL0 and TTBR1_EL0, including changing
the ASID, will still cause a complete flush of the TLB.

Signed-off-by: Jim MacArthur <jim.macarthur@linaro.org>
---
 target/arm/cpu-features.h |  7 +++++++
 target/arm/helper.c       | 18 ++++++++++++++++++
 2 files changed, 25 insertions(+)

diff --git a/target/arm/cpu-features.h b/target/arm/cpu-features.h
index 579fa8f8f4..d56bda9ce0 100644
--- a/target/arm/cpu-features.h
+++ b/target/arm/cpu-features.h
@@ -346,6 +346,8 @@ FIELD(ID_AA64MMFR3, SDERR, 52, 4)
 FIELD(ID_AA64MMFR3, ADERR, 56, 4)
 FIELD(ID_AA64MMFR3, SPEC_FPACC, 60, 4)
 
+FIELD(ID_AA64MMFR4, ASID2, 8, 4)
+
 FIELD(ID_AA64DFR0, DEBUGVER, 0, 4)
 FIELD(ID_AA64DFR0, TRACEVER, 4, 4)
 FIELD(ID_AA64DFR0, PMUVER, 8, 4)
@@ -1369,6 +1371,11 @@ static inline bool isar_feature_aa64_aie(const ARMISARegisters *id)
     return FIELD_EX64_IDREG(id, ID_AA64MMFR3, AIE) != 0;
 }
 
+static inline bool isar_feature_aa64_asid2(const ARMISARegisters *id)
+{
+    return FIELD_EX64_IDREG(id, ID_AA64MMFR4, ASID2) != 0;
+}
+
 static inline bool isar_feature_aa64_mec(const ARMISARegisters *id)
 {
     return FIELD_EX64_IDREG(id, ID_AA64MMFR3, MEC) != 0;
diff --git a/target/arm/helper.c b/target/arm/helper.c
index c20334fa65..ecb31b058c 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -6095,6 +6095,7 @@ static void tcr2_el1_write(CPUARMState *env, const ARMCPRegInfo *ri,
 {
     ARMCPU *cpu = env_archcpu(env);
     uint64_t valid_mask = 0;
+    bool require_flush = false;
 
     if (cpu_isar_feature(aa64_s1pie, cpu)) {
         valid_mask |= TCR2_PIE;
@@ -6102,8 +6103,16 @@ static void tcr2_el1_write(CPUARMState *env, const ARMCPRegInfo *ri,
     if (cpu_isar_feature(aa64_aie, cpu)) {
         valid_mask |= TCR2_AIE;
     }
+    if (cpu_isar_feature(aa64_asid2, cpu)) {
+        valid_mask |= TCR2_FNG1 | TCR2_FNG0 | TCR2_A2;
+        require_flush = true;
+    }
     value &= valid_mask;
     raw_write(env, ri, value);
+
+    if (require_flush) {
+        tlb_flush(CPU(cpu));
+    }
 }
 
 static void tcr2_el2_write(CPUARMState *env, const ARMCPRegInfo *ri,
@@ -6111,6 +6120,7 @@ static void tcr2_el2_write(CPUARMState *env, const ARMCPRegInfo *ri,
 {
     ARMCPU *cpu = env_archcpu(env);
     uint64_t valid_mask = 0;
+    bool require_flush = false;
 
     if (cpu_isar_feature(aa64_s1pie, cpu)) {
         valid_mask |= TCR2_PIE;
@@ -6121,8 +6131,16 @@ static void tcr2_el2_write(CPUARMState *env, const ARMCPRegInfo *ri,
     if (cpu_isar_feature(aa64_mec, cpu)) {
         valid_mask |= TCR2_AMEC0 | TCR2_AMEC1;
     }
+    if (cpu_isar_feature(aa64_asid2, cpu)) {
+        valid_mask |= TCR2_FNG1 | TCR2_FNG0 | TCR2_A2;
+        require_flush = true;
+    }
     value &= valid_mask;
     raw_write(env, ri, value);
+
+    if (require_flush) {
+        tlb_flush(CPU(cpu));
+    }
 }
 
 static const ARMCPRegInfo tcr2_reginfo[] = {
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/4] target/arm/tcg/cpu64.c: Enable ASID2 for cpu_max
  2025-12-04 18:04 [PATCH V5 0/4] Basic ASID2 support Jim MacArthur
  2025-12-04 18:04 ` [PATCH 1/4] target/arm: Enable ID_AA64MMFR4_EL1 register Jim MacArthur
  2025-12-04 18:04 ` [PATCH 2/4] target/arm: Allow writes to FNG1, FNG0, A2 Jim MacArthur
@ 2025-12-04 18:04 ` Jim MacArthur
  2025-12-04 18:04 ` [PATCH 4/4] tests: Add test for ASID2 and write/read of feature bits Jim MacArthur
  2025-12-04 18:30 ` [PATCH V5 0/4] Basic ASID2 support Alex Bennée
  4 siblings, 0 replies; 9+ messages in thread
From: Jim MacArthur @ 2025-12-04 18:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: Jim MacArthur

docs/system/arm/emulation.rst: Add ASID2

Signed-off-by: Jim MacArthur <jim.macarthur@linaro.org>
---
 docs/system/arm/emulation.rst | 1 +
 target/arm/tcg/cpu64.c        | 4 ++++
 2 files changed, 5 insertions(+)

diff --git a/docs/system/arm/emulation.rst b/docs/system/arm/emulation.rst
index 31a5878a8f..3f30ea5a30 100644
--- a/docs/system/arm/emulation.rst
+++ b/docs/system/arm/emulation.rst
@@ -24,6 +24,7 @@ the following architecture extensions:
 - FEAT_AIE (Memory Attribute Index Enhancement)
 - FEAT_Armv9_Crypto (Armv9 Cryptographic Extension)
 - FEAT_ASID16 (16 bit ASID)
+- FEAT_ASID2 (Concurrent use of two ASIDs)
 - FEAT_ATS1A (Address Translation operations that ignore stage 1 permissions)
 - FEAT_BBM at level 2 (Translation table break-before-make levels)
 - FEAT_BF16 (AArch64 BFloat16 instructions)
diff --git a/target/arm/tcg/cpu64.c b/target/arm/tcg/cpu64.c
index 6871956382..ef4c0c8d73 100644
--- a/target/arm/tcg/cpu64.c
+++ b/target/arm/tcg/cpu64.c
@@ -1334,6 +1334,10 @@ void aarch64_max_tcg_initfn(Object *obj)
     t = FIELD_DP64(t, ID_AA64MMFR3, AIE, 1);      /* FEAT_AIE */
     SET_IDREG(isar, ID_AA64MMFR3, t);
 
+    t = GET_IDREG(isar, ID_AA64MMFR4);
+    t = FIELD_DP64(t, ID_AA64MMFR4, ASID2, 1);    /* FEAT_ASID2 */
+    SET_IDREG(isar, ID_AA64MMFR4, t);
+
     t = GET_IDREG(isar, ID_AA64ZFR0);
     t = FIELD_DP64(t, ID_AA64ZFR0, SVEVER, 2);    /* FEAT_SVE2p1 */
     t = FIELD_DP64(t, ID_AA64ZFR0, AES, 2);       /* FEAT_SVE_PMULL128 */
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/4] tests: Add test for ASID2 and write/read of feature bits
  2025-12-04 18:04 [PATCH V5 0/4] Basic ASID2 support Jim MacArthur
                   ` (2 preceding siblings ...)
  2025-12-04 18:04 ` [PATCH 3/4] target/arm/tcg/cpu64.c: Enable ASID2 for cpu_max Jim MacArthur
@ 2025-12-04 18:04 ` Jim MacArthur
  2025-12-04 18:30 ` [PATCH V5 0/4] Basic ASID2 support Alex Bennée
  4 siblings, 0 replies; 9+ messages in thread
From: Jim MacArthur @ 2025-12-04 18:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: Jim MacArthur

Test for presence of ASID2; if it is, check FNG1, FNG0, and A2 are
writable, and read value shows the update. If not present, check these
read as RES0.

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

diff --git a/tests/tcg/aarch64/system/asid2.c b/tests/tcg/aarch64/system/asid2.c
new file mode 100644
index 0000000000..7d5466af34
--- /dev/null
+++ b/tests/tcg/aarch64/system/asid2.c
@@ -0,0 +1,76 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ *
+ * ASID2 Feature presence and enabled TCR2_EL1 bits test
+ *
+ * Copyright (c) 2025 Linaro Ltd
+ *
+ */
+
+#include <stdint.h>
+#include <minilib.h>
+
+#define ID_AA64MMFR3_EL1 "S3_0_C0_C7_3"
+#define ID_AA64MMFR4_EL1 "S3_0_C0_C7_4"
+#define TCR2_EL1 "S3_0_C2_C0_3"
+
+int main()
+{
+    /*
+     * Test for presence of ASID2 and three feature bits enabled by it:
+     * https://developer.arm.com/documentation/109697/2025_09/Feature-descriptions/The-Armv9-5-architecture-extension
+     * Bits added are FNG1, FNG0, and A2. These should be RES0 if A2 is
+     * not enabled and read as the written value if A2 is enabled.
+     */
+
+    uint64_t out;
+    uint64_t idreg3;
+    uint64_t idreg4;
+    int tcr2_present;
+    int asid2_present;
+
+    /* Mask is FNG1, FNG0, and A2 */
+    const uint64_t feature_mask = (1ULL << 18 | 1ULL << 17 | 1ULL << 16);
+    const uint64_t in = feature_mask;
+
+    asm("mrs %[idreg3], " ID_AA64MMFR3_EL1 "\n\t"
+        : [idreg3] "=r" (idreg3));
+
+    tcr2_present = ((idreg3 & 0xF) != 0);
+
+    if (!tcr2_present) {
+        ml_printf("TCR2 is not present, cannot perform test");
+        return 0;
+    }
+
+    asm("mrs %[idreg4], " ID_AA64MMFR4_EL1 "\n\t"
+        : [idreg4] "=r" (idreg4));
+
+    asid2_present = ((idreg4 & 0xF00) != 0);
+
+    asm("msr " TCR2_EL1 ", %[x0]\n\t"
+        "mrs %[x1], " TCR2_EL1 "\n\t"
+        : [x1] "=r" (out)
+        : [x0] "r" (in));
+
+    if (asid2_present) {
+        if ((out & feature_mask) == in) {
+            ml_printf("OK\n");
+            return 0;
+        } else {
+            ml_printf("FAIL: ASID2 present, but read value %lx != "
+                      "written value %lx\n",
+                      out & feature_mask, in);
+            return 1;
+        }
+    } else {
+        if (out == 0) {
+            ml_printf("TCR2_EL1 reads as RES0 as expected\n");
+            return 0;
+        } else {
+            ml_printf("FAIL: ASID2, missing but read value %lx != 0\n",
+                      out & feature_mask, in);
+            return 1;
+        }
+    }
+}
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH V5 0/4] Basic ASID2 support
  2025-12-04 18:04 [PATCH V5 0/4] Basic ASID2 support Jim MacArthur
                   ` (3 preceding siblings ...)
  2025-12-04 18:04 ` [PATCH 4/4] tests: Add test for ASID2 and write/read of feature bits Jim MacArthur
@ 2025-12-04 18:30 ` Alex Bennée
  4 siblings, 0 replies; 9+ messages in thread
From: Alex Bennée @ 2025-12-04 18:30 UTC (permalink / raw)
  To: Jim MacArthur; +Cc: qemu-devel

Jim MacArthur <jim.macarthur@linaro.org> writes:

> Thanks to Gustavo Romero for reviews.
>
> Changes in v5:
> - Patch 2:
>   - TLB flush when A2/FNG0/FNG1 could be written to.
> - Patch 4:
>   - SPDX License identifier moved to first line.

I think you missed picking up the Reviewed-by tags. I used to do this by
hand but using a tool like b4 makes it a lot easier. See:

  https://qemu.readthedocs.io/en/v10.0.3/devel/submitting-a-patch.html#proper-use-of-reviewed-by-tags-can-aid-review

Also its worth adding to your summary what patches remain un-reviewed or
indeed noting they have all now been reviewed. It makes the maintainers
job easier when eyeballing the cover letter.

>
> Jim MacArthur (4):
>   target/arm: Enable ID_AA64MMFR4_EL1 register
>   target/arm: Allow writes to FNG1, FNG0, A2
>   target/arm/tcg/cpu64.c: Enable ASID2 for cpu_max
>   tests: Add test for ASID2 and write/read of feature bits
>
>  docs/system/arm/emulation.rst    |  1 +
>  target/arm/cpu-features.h        |  7 +++
>  target/arm/cpu-sysregs.h.inc     |  1 +
>  target/arm/helper.c              | 22 ++++++++-
>  target/arm/tcg/cpu64.c           |  4 ++
>  tests/tcg/aarch64/system/asid2.c | 76 ++++++++++++++++++++++++++++++++
>  6 files changed, 109 insertions(+), 2 deletions(-)
>  create mode 100644 tests/tcg/aarch64/system/asid2.c

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/4] target/arm: Allow writes to FNG1, FNG0, A2
  2025-12-04 18:04 ` [PATCH 2/4] target/arm: Allow writes to FNG1, FNG0, A2 Jim MacArthur
@ 2025-12-05 15:30   ` Richard Henderson
  2025-12-09 15:04     ` Jim MacArthur
  0 siblings, 1 reply; 9+ messages in thread
From: Richard Henderson @ 2025-12-05 15:30 UTC (permalink / raw)
  To: qemu-devel

On 12/4/25 12:04, Jim MacArthur wrote:
> @@ -6121,8 +6131,16 @@ static void tcr2_el2_write(CPUARMState *env, const ARMCPRegInfo *ri,
>       if (cpu_isar_feature(aa64_mec, cpu)) {
>           valid_mask |= TCR2_AMEC0 | TCR2_AMEC1;
>       }
> +    if (cpu_isar_feature(aa64_asid2, cpu)) {
> +        valid_mask |= TCR2_FNG1 | TCR2_FNG0 | TCR2_A2;
> +        require_flush = true;
> +    }
>       value &= valid_mask;
>       raw_write(env, ri, value);
> +
> +    if (require_flush) {
> +        tlb_flush(CPU(cpu));
> +    }

Just because A2 is valid doesn't mean the A2 bit changed.

Compare, for instance, vmsa_ttbr_write, where we notice if the ASID has changed before 
performing the flush.

Note as well that we don't need to flush all tlbs.  In tcr2_el1_write we know that we are 
only affecting the EL1&0 regime (alle1_tlbmask).  In tcr2_el2_write, we know that we are 
only affecting the EL2&0 regime (see the E2H part of vae2_tlbmask).


r~


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/4] target/arm: Allow writes to FNG1, FNG0, A2
  2025-12-05 15:30   ` Richard Henderson
@ 2025-12-09 15:04     ` Jim MacArthur
  2025-12-09 15:39       ` Richard Henderson
  0 siblings, 1 reply; 9+ messages in thread
From: Jim MacArthur @ 2025-12-09 15:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: richard.henderson

On 12/5/25 15:30, Richard Henderson wrote:
> On 12/4/25 12:04, Jim MacArthur wrote:
>> @@ -6121,8 +6131,16 @@ static void tcr2_el2_write(CPUARMState *env, 
>> const ARMCPRegInfo *ri,
>>       if (cpu_isar_feature(aa64_mec, cpu)) {
>>           valid_mask |= TCR2_AMEC0 | TCR2_AMEC1;
>>       }
>> +    if (cpu_isar_feature(aa64_asid2, cpu)) {
>> +        valid_mask |= TCR2_FNG1 | TCR2_FNG0 | TCR2_A2;
>> +        require_flush = true;
>> +    }
>>       value &= valid_mask;
>>       raw_write(env, ri, value);
>> +
>> +    if (require_flush) {
>> +        tlb_flush(CPU(cpu));
>> +    }
>
> Just because A2 is valid doesn't mean the A2 bit changed.
>
> Compare, for instance, vmsa_ttbr_write, where we notice if the ASID 
> has changed before performing the flush.
>
> Note as well that we don't need to flush all tlbs.  In tcr2_el1_write 
> we know that we are only affecting the EL1&0 regime (alle1_tlbmask).  
> In tcr2_el2_write, we know that we are only affecting the EL2&0 regime 
> (see the E2H part of vae2_tlbmask).
>
>
> r~
>

Before I make a full patch series, can I check this looks correct?

In tcr2_el1_write:

     if (cpu_isar_feature(aa64_asid2, cpu)) {
         uint64_t asid_nonglobal_flags = TCR2_FNG1 | TCR2_FNG0 | TCR2_A2;
         valid_mask |= asid_nonglobal_flags;
         require_flush = ((raw_read(env, ri) ^ value) & 
asid_nonglobal_flags) != 0;
     }
     value &= valid_mask;
     raw_write(env, ri, value);

     if (require_flush) {
         tlb_flush_by_mmuidx(CPU(cpu), alle1_tlbmask(env));
     }

And then in tcr_el2_write, the same check but flushing by: 
ARMMMUIdxBit_E20_2 |ARMMMUIdxBit_E20_2_PAN | ARMMMUIdxBit_E20_2_GCS | 
ARMMMUIdxBit_E20_0 | ARMMMUIdxBit_E20_0_GCS, as used in 
vmsa_tcr_ttbr_el2_write. This could be factored out into a constant 
function like alle1_tlbmask.

Jim



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/4] target/arm: Allow writes to FNG1, FNG0, A2
  2025-12-09 15:04     ` Jim MacArthur
@ 2025-12-09 15:39       ` Richard Henderson
  0 siblings, 0 replies; 9+ messages in thread
From: Richard Henderson @ 2025-12-09 15:39 UTC (permalink / raw)
  To: Jim MacArthur, qemu-devel

On 12/9/25 09:04, Jim MacArthur wrote:
> On 12/5/25 15:30, Richard Henderson wrote:
>> On 12/4/25 12:04, Jim MacArthur wrote:
>>> @@ -6121,8 +6131,16 @@ static void tcr2_el2_write(CPUARMState *env, const ARMCPRegInfo 
>>> *ri,
>>>       if (cpu_isar_feature(aa64_mec, cpu)) {
>>>           valid_mask |= TCR2_AMEC0 | TCR2_AMEC1;
>>>       }
>>> +    if (cpu_isar_feature(aa64_asid2, cpu)) {
>>> +        valid_mask |= TCR2_FNG1 | TCR2_FNG0 | TCR2_A2;
>>> +        require_flush = true;
>>> +    }
>>>       value &= valid_mask;
>>>       raw_write(env, ri, value);
>>> +
>>> +    if (require_flush) {
>>> +        tlb_flush(CPU(cpu));
>>> +    }
>>
>> Just because A2 is valid doesn't mean the A2 bit changed.
>>
>> Compare, for instance, vmsa_ttbr_write, where we notice if the ASID has changed before 
>> performing the flush.
>>
>> Note as well that we don't need to flush all tlbs.  In tcr2_el1_write we know that we 
>> are only affecting the EL1&0 regime (alle1_tlbmask). In tcr2_el2_write, we know that we 
>> are only affecting the EL2&0 regime (see the E2H part of vae2_tlbmask).
>>
>>
>> r~
>>
> 
> Before I make a full patch series, can I check this looks correct?
> 
> In tcr2_el1_write:
> 
>      if (cpu_isar_feature(aa64_asid2, cpu)) {
>          uint64_t asid_nonglobal_flags = TCR2_FNG1 | TCR2_FNG0 | TCR2_A2;
>          valid_mask |= asid_nonglobal_flags;
>          require_flush = ((raw_read(env, ri) ^ value) & asid_nonglobal_flags) != 0;
>      }
>      value &= valid_mask;
>      raw_write(env, ri, value);
> 
>      if (require_flush) {
>          tlb_flush_by_mmuidx(CPU(cpu), alle1_tlbmask(env));
>      }
> 
> And then in tcr_el2_write, the same check but flushing by: ARMMMUIdxBit_E20_2 | 
> ARMMMUIdxBit_E20_2_PAN | ARMMMUIdxBit_E20_2_GCS | ARMMMUIdxBit_E20_0 | 
> ARMMMUIdxBit_E20_0_GCS, as used in vmsa_tcr_ttbr_el2_write. This could be factored out 
> into a constant function like alle1_tlbmask.

You don't actually need the require_flush boolean. You could just as well perform the 
flush immediately -- there's nothing about tlb_flush that requires the raw_write to happen 
first.

The FNG[01] bits don't affect ASID selection, so you don't need to flush when they change, 
only when the A2 bit changes.


r~


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-12-09 15:40 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-04 18:04 [PATCH V5 0/4] Basic ASID2 support Jim MacArthur
2025-12-04 18:04 ` [PATCH 1/4] target/arm: Enable ID_AA64MMFR4_EL1 register Jim MacArthur
2025-12-04 18:04 ` [PATCH 2/4] target/arm: Allow writes to FNG1, FNG0, A2 Jim MacArthur
2025-12-05 15:30   ` Richard Henderson
2025-12-09 15:04     ` Jim MacArthur
2025-12-09 15:39       ` Richard Henderson
2025-12-04 18:04 ` [PATCH 3/4] target/arm/tcg/cpu64.c: Enable ASID2 for cpu_max Jim MacArthur
2025-12-04 18:04 ` [PATCH 4/4] tests: Add test for ASID2 and write/read of feature bits Jim MacArthur
2025-12-04 18:30 ` [PATCH V5 0/4] Basic ASID2 support Alex Bennée

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).