All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.