[PATCH 0/6] target/arm: Fix v8 AArch32 RAZ ID regs; implement FEAT_ETS

[PATCH 0/6] target/arm: Fix v8 AArch32 RAZ ID regs; implement FEAT_ETS

[Peter Maydell]

The main aim of this patchset is to implement FEAT_ETS.
FEAT_ETS provides tighter guarantees on some memory orderings
involving translation table walks that permit guest code to
skip the context-synchronization event they would otherwise
need to do after a TLB maintenance operation. QEMU already
provides the tighter guarantees this feature requires, so
all we need to do is advertise it in the ID registers...

...except that it turns out that for AArch32 this is done
in ID_MMFR5, which is a new-in-v8.6 register that we don't
implement yet. So we need to provide it. And while I was
doing that I noticed that we accidentally forgot to
implement a big chunk of the "reserved for future ID
registers, must RAZ" cp15 space for v8 AArch32. So the
big bit of the patchset is sorting that out :-)

thanks
-- PMM

Peter Maydell (6):
  target/arm: Make cpregs 0, c0, c{3-15}, {0-7} correctly RAZ in v8
  target/arm: Sort KVM reads of AArch32 ID registers into encoding order
  target/arm: Implement ID_MMFR5
  target/arm: Implement ID_DFR1
  target/arm: Advertise FEAT_ETS for '-cpu max'
  target/arm: Add missing space in comment

 docs/system/arm/emulation.rst |  1 +
 target/arm/cpu.h              |  2 +
 target/arm/cpu64.c            |  1 +
 target/arm/cpu_tcg.c          |  6 ++-
 target/arm/helper.c           | 69 +++++++++++++++++++++++++++++++----
 target/arm/kvm64.c            |  8 +++-
 6 files changed, 77 insertions(+), 10 deletions(-)

-- 
2.25.1

[PATCH 1/6] target/arm: Make cpregs 0, c0, c{3-15}, {0-7} correctly RAZ in v8

[Peter Maydell]

In the AArch32 ID register scheme, coprocessor registers with
encoding cp15, 0, c0, c{0-7}, {0-7} are all in the space covered by
what in v6 and v7 was called the "CPUID scheme", and are supposed to
RAZ if they're not allocated to a specific ID register.  For our
pre-v8 CPUs we get this right, because the regdefs in
id_pre_v8_midr_cp_reginfo[] cover these RAZ requirements.  However
for v8 we failed to put in the necessary patterns to cover this, so
we end up UNDEFing on everything we didn't have an ID register for.
This is a problem because in Armv8 some encodings in 0, c0, c3, {0-7}
are now being used for new ID registers, and guests might thus start
trying to read them.  (We already have one of these: ID_PFR2.)

For v8 CPUs, we already have regdefs for 0, c0, c{0-2}, {0-7} (that
is, the space is completely allocated with no reserved spaces).  Add
entries to v8_idregs[] covering 0, c0, c3, {0-7}:
 * c3, {0-2} is the reserved AArch32 space corresponding to the
   AArch64 MVFR[012]_EL1
 * c3, {3,5,6,7} are reserved RAZ for both AArch32 and AArch64
   (in fact some of these are given defined meanings in Armv8.6,
   but we don't implement them yet)
 * c3, 4 is ID_PFR2 (already defined)

We then programmatically add RAZ patterns for AArch32 for
0, c0, c{4..15}, {0-7}:
 * c4-c7 are unused, and not shared with AArch64 (these
   are the encodings corresponding to where the AArch64
   specific ID registers live in the system register space)
 * c8-c15 weren't required to RAZ in v6/v7, but v8 extends
   the AArch32 reserved-should-RAZ space to cover these;
   the equivalent area of the AArch64 sysreg space is not
   defined as must-RAZ

Note that the architecture allows some registers in this space
to return an UNKNOWN value; we always return 0.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/helper.c | 65 +++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 60 insertions(+), 5 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index d7bc467a2a5..c171770b035 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -7345,11 +7345,16 @@ void register_cp_regs_for_features(ARMCPU *cpu)
         define_arm_cp_regs(cpu, not_v7_cp_reginfo);
     }
     if (arm_feature(env, ARM_FEATURE_V8)) {
-        /* AArch64 ID registers, which all have impdef reset values.
+        /*
+         * v8 ID registers, which all have impdef reset values.
          * Note that within the ID register ranges the unused slots
          * must all RAZ, not UNDEF; future architecture versions may
          * define new registers here.
+         * ID registers which are AArch64 views of the AArch32 ID registers
+         * which already existed in v6 and v7 are handled elsewhere,
+         * in v6_idregs[].
          */
+        int i;
         ARMCPRegInfo v8_idregs[] = {
             /*
              * ID_AA64PFR0_EL1 is not a plain ARM_CP_CONST in system
@@ -7539,7 +7544,34 @@ void register_cp_regs_for_features(ARMCPU *cpu)
               .access = PL1_R, .type = ARM_CP_CONST,
               .accessfn = access_aa64_tid3,
               .resetvalue = cpu->isar.mvfr2 },
-            { .name = "MVFR3_EL1_RESERVED", .state = ARM_CP_STATE_AA64,
+            /*
+             * "0, c0, c3, {0,1,2}" are the encodings corresponding to
+             * AArch64 MVFR[012]_EL1. Define the STATE_AA32 encoding
+             * as RAZ, since it is in the "reserved for future ID
+             * registers, RAZ" part of the AArch32 encoding space.
+             */
+            { .name = "RES_0_C0_C3_0", .state = ARM_CP_STATE_AA32,
+              .cp = 15, .opc1 = 0, .crn = 0, .crm = 3, .opc2 = 0,
+              .access = PL1_R, .type = ARM_CP_CONST,
+              .accessfn = access_aa64_tid3,
+              .resetvalue = 0 },
+            { .name = "RES_0_C0_C3_1", .state = ARM_CP_STATE_AA32,
+              .cp = 15, .opc1 = 0, .crn = 0, .crm = 3, .opc2 = 1,
+              .access = PL1_R, .type = ARM_CP_CONST,
+              .accessfn = access_aa64_tid3,
+              .resetvalue = 0 },
+            { .name = "RES_0_C0_C3_2", .state = ARM_CP_STATE_AA32,
+              .cp = 15, .opc1 = 0, .crn = 0, .crm = 3, .opc2 = 2,
+              .access = PL1_R, .type = ARM_CP_CONST,
+              .accessfn = access_aa64_tid3,
+              .resetvalue = 0 },
+            /*
+             * Other encodings in "0, c0, c3, ..." are STATE_BOTH because
+             * they're also RAZ for AArch64, and in v8 are gradually
+             * being filled with AArch64-view-of-AArch32-ID-register
+             * for new ID registers.
+             */
+            { .name = "RES_0_C0_C3_3", .state = ARM_CP_STATE_BOTH,
               .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 3, .opc2 = 3,
               .access = PL1_R, .type = ARM_CP_CONST,
               .accessfn = access_aa64_tid3,
@@ -7549,17 +7581,17 @@ void register_cp_regs_for_features(ARMCPU *cpu)
               .access = PL1_R, .type = ARM_CP_CONST,
               .accessfn = access_aa64_tid3,
               .resetvalue = cpu->isar.id_pfr2 },
-            { .name = "MVFR5_EL1_RESERVED", .state = ARM_CP_STATE_AA64,
+            { .name = "RES_0_C0_C3_5", .state = ARM_CP_STATE_BOTH,
               .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 3, .opc2 = 5,
               .access = PL1_R, .type = ARM_CP_CONST,
               .accessfn = access_aa64_tid3,
               .resetvalue = 0 },
-            { .name = "MVFR6_EL1_RESERVED", .state = ARM_CP_STATE_AA64,
+            { .name = "RES_0_C0_C3_6", .state = ARM_CP_STATE_BOTH,
               .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 3, .opc2 = 6,
               .access = PL1_R, .type = ARM_CP_CONST,
               .accessfn = access_aa64_tid3,
               .resetvalue = 0 },
-            { .name = "MVFR7_EL1_RESERVED", .state = ARM_CP_STATE_AA64,
+            { .name = "RES_0_C0_C3_7", .state = ARM_CP_STATE_BOTH,
               .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 3, .opc2 = 7,
               .access = PL1_R, .type = ARM_CP_CONST,
               .accessfn = access_aa64_tid3,
@@ -7625,6 +7657,29 @@ void register_cp_regs_for_features(ARMCPU *cpu)
         }
         define_arm_cp_regs(cpu, v8_idregs);
         define_arm_cp_regs(cpu, v8_cp_reginfo);
+
+        for (i = 4; i < 16; i++) {
+            /*
+             * Encodings in "0, c0, {c4-c7}, {0-7}" are RAZ for AArch32.
+             * For pre-v8 cores there are RAZ patterns for these in
+             * id_pre_v8_midr_cp_reginfo[]; for v8 we do that here.
+             * v8 extends the "must RAZ" part of the ID register space
+             * to also cover c0, 0, c{8-15}, {0-7}.
+             * These are STATE_AA32 because in the AArch64 sysreg space
+             * c4-c7 is where the AArch64 ID registers live (and we've
+             * already defined those in v8_idregs[]), and c8-c15 are not
+             * "must RAZ" for AArch64.
+             */
+            g_autofree char *name = g_strdup_printf("RES_0_C0_C%d_X", i);
+            ARMCPRegInfo v8_aa32_raz_idregs = {
+                .name = name,
+                .state = ARM_CP_STATE_AA32,
+                .cp = 15, .opc1 = 0, .crn = 0, .crm = i, .opc2 = CP_ANY,
+                .access = PL1_R, .type = ARM_CP_CONST,
+                .accessfn = access_aa64_tid3,
+                .resetvalue = 0 };
+            define_one_arm_cp_reg(cpu, &v8_aa32_raz_idregs);
+        }
     }
 
     /*
-- 
2.25.1

Re: [PATCH 1/6] target/arm: Make cpregs 0, c0, c{3-15}, {0-7} correctly RAZ in v8

[Richard Henderson]

On 8/19/22 04:00, Peter Maydell wrote:
> In the AArch32 ID register scheme, coprocessor registers with
> encoding cp15, 0, c0, c{0-7}, {0-7} are all in the space covered by
> what in v6 and v7 was called the "CPUID scheme", and are supposed to
> RAZ if they're not allocated to a specific ID register.  For our
> pre-v8 CPUs we get this right, because the regdefs in
> id_pre_v8_midr_cp_reginfo[] cover these RAZ requirements.  However
> for v8 we failed to put in the necessary patterns to cover this, so
> we end up UNDEFing on everything we didn't have an ID register for.
> This is a problem because in Armv8 some encodings in 0, c0, c3, {0-7}
> are now being used for new ID registers, and guests might thus start
> trying to read them.  (We already have one of these: ID_PFR2.)
> 
> For v8 CPUs, we already have regdefs for 0, c0, c{0-2}, {0-7} (that
> is, the space is completely allocated with no reserved spaces).  Add
> entries to v8_idregs[] covering 0, c0, c3, {0-7}:
>   * c3, {0-2} is the reserved AArch32 space corresponding to the
>     AArch64 MVFR[012]_EL1
>   * c3, {3,5,6,7} are reserved RAZ for both AArch32 and AArch64
>     (in fact some of these are given defined meanings in Armv8.6,
>     but we don't implement them yet)
>   * c3, 4 is ID_PFR2 (already defined)
> 
> We then programmatically add RAZ patterns for AArch32 for
> 0, c0, c{4..15}, {0-7}:
>   * c4-c7 are unused, and not shared with AArch64 (these
>     are the encodings corresponding to where the AArch64
>     specific ID registers live in the system register space)
>   * c8-c15 weren't required to RAZ in v6/v7, but v8 extends
>     the AArch32 reserved-should-RAZ space to cover these;
>     the equivalent area of the AArch64 sysreg space is not
>     defined as must-RAZ
> 
> Note that the architecture allows some registers in this space
> to return an UNKNOWN value; we always return 0.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   target/arm/helper.c | 65 +++++++++++++++++++++++++++++++++++++++++----
>   1 file changed, 60 insertions(+), 5 deletions(-)

This is the thing at the top of H.a page G7-8990, yeah?

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


r~

Re: [PATCH 1/6] target/arm: Make cpregs 0, c0, c{3-15}, {0-7} correctly RAZ in v8

[Peter Maydell]

On Sat, 20 Aug 2022 at 03:37, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 8/19/22 04:00, Peter Maydell wrote:
> > In the AArch32 ID register scheme, coprocessor registers with
> > encoding cp15, 0, c0, c{0-7}, {0-7} are all in the space covered by
> > what in v6 and v7 was called the "CPUID scheme", and are supposed to
> > RAZ if they're not allocated to a specific ID register.  For our
> > pre-v8 CPUs we get this right, because the regdefs in
> > id_pre_v8_midr_cp_reginfo[] cover these RAZ requirements.  However
> > for v8 we failed to put in the necessary patterns to cover this, so
> > we end up UNDEFing on everything we didn't have an ID register for.
> > This is a problem because in Armv8 some encodings in 0, c0, c3, {0-7}
> > are now being used for new ID registers, and guests might thus start
> > trying to read them.  (We already have one of these: ID_PFR2.)
> >
> > For v8 CPUs, we already have regdefs for 0, c0, c{0-2}, {0-7} (that
> > is, the space is completely allocated with no reserved spaces).  Add
> > entries to v8_idregs[] covering 0, c0, c3, {0-7}:
> >   * c3, {0-2} is the reserved AArch32 space corresponding to the
> >     AArch64 MVFR[012]_EL1
> >   * c3, {3,5,6,7} are reserved RAZ for both AArch32 and AArch64
> >     (in fact some of these are given defined meanings in Armv8.6,
> >     but we don't implement them yet)
> >   * c3, 4 is ID_PFR2 (already defined)
> >
> > We then programmatically add RAZ patterns for AArch32 for
> > 0, c0, c{4..15}, {0-7}:
> >   * c4-c7 are unused, and not shared with AArch64 (these
> >     are the encodings corresponding to where the AArch64
> >     specific ID registers live in the system register space)
> >   * c8-c15 weren't required to RAZ in v6/v7, but v8 extends
> >     the AArch32 reserved-should-RAZ space to cover these;
> >     the equivalent area of the AArch64 sysreg space is not
> >     defined as must-RAZ
> >
> > Note that the architecture allows some registers in this space
> > to return an UNKNOWN value; we always return 0.
> >
> > Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> > ---
> >   target/arm/helper.c | 65 +++++++++++++++++++++++++++++++++++++++++----
> >   1 file changed, 60 insertions(+), 5 deletions(-)
>
> This is the thing at the top of H.a page G7-8990, yeah?

Yes, that's the one.

-- PMM

[PATCH 2/6] target/arm: Sort KVM reads of AArch32 ID registers into encoding order

[Peter Maydell]

The code that reads the AArch32 ID registers from KVM in
kvm_arm_get_host_cpu_features() does so almost but not quite in
encoding order.  Move the read of ID_PFR2 down so it's really in
encoding order.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/kvm64.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 9b9dd46d782..84c4c85f405 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -608,8 +608,6 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
                               ARM64_SYS_REG(3, 0, 0, 1, 0));
         err |= read_sys_reg32(fdarray[2], &ahcf->isar.id_pfr1,
                               ARM64_SYS_REG(3, 0, 0, 1, 1));
-        err |= read_sys_reg32(fdarray[2], &ahcf->isar.id_pfr2,
-                              ARM64_SYS_REG(3, 0, 0, 3, 4));
         err |= read_sys_reg32(fdarray[2], &ahcf->isar.id_dfr0,
                               ARM64_SYS_REG(3, 0, 0, 1, 2));
         err |= read_sys_reg32(fdarray[2], &ahcf->isar.id_mmfr0,
@@ -643,6 +641,8 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
                               ARM64_SYS_REG(3, 0, 0, 3, 1));
         err |= read_sys_reg32(fdarray[2], &ahcf->isar.mvfr2,
                               ARM64_SYS_REG(3, 0, 0, 3, 2));
+        err |= read_sys_reg32(fdarray[2], &ahcf->isar.id_pfr2,
+                              ARM64_SYS_REG(3, 0, 0, 3, 4));
 
         /*
          * DBGDIDR is a bit complicated because the kernel doesn't
-- 
2.25.1

Re: [PATCH 2/6] target/arm: Sort KVM reads of AArch32 ID registers into encoding order

[Richard Henderson]

On 8/19/22 04:00, Peter Maydell wrote:
> The code that reads the AArch32 ID registers from KVM in
> kvm_arm_get_host_cpu_features() does so almost but not quite in
> encoding order.  Move the read of ID_PFR2 down so it's really in
> encoding order.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   target/arm/kvm64.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

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

r~

[PATCH 3/6] target/arm: Implement ID_MMFR5

[Peter Maydell]

In Armv8.6 a new AArch32 ID register ID_MMFR5 is defined.
Implement this; we want to be able to use it to report to
the guest that we implement FEAT_ETS.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/cpu.h    | 1 +
 target/arm/helper.c | 4 ++--
 target/arm/kvm64.c  | 2 ++
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 5168e3d837e..fcc5927587e 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -975,6 +975,7 @@ struct ArchCPU {
         uint32_t id_mmfr2;
         uint32_t id_mmfr3;
         uint32_t id_mmfr4;
+        uint32_t id_mmfr5;
         uint32_t id_pfr0;
         uint32_t id_pfr1;
         uint32_t id_pfr2;
diff --git a/target/arm/helper.c b/target/arm/helper.c
index c171770b035..07378519259 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -7586,11 +7586,11 @@ void register_cp_regs_for_features(ARMCPU *cpu)
               .access = PL1_R, .type = ARM_CP_CONST,
               .accessfn = access_aa64_tid3,
               .resetvalue = 0 },
-            { .name = "RES_0_C0_C3_6", .state = ARM_CP_STATE_BOTH,
+            { .name = "ID_MMFR5", .state = ARM_CP_STATE_BOTH,
               .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 3, .opc2 = 6,
               .access = PL1_R, .type = ARM_CP_CONST,
               .accessfn = access_aa64_tid3,
-              .resetvalue = 0 },
+              .resetvalue = cpu->isar.id_mmfr5 },
             { .name = "RES_0_C0_C3_7", .state = ARM_CP_STATE_BOTH,
               .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 3, .opc2 = 7,
               .access = PL1_R, .type = ARM_CP_CONST,
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 84c4c85f405..2d737c443eb 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -643,6 +643,8 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
                               ARM64_SYS_REG(3, 0, 0, 3, 2));
         err |= read_sys_reg32(fdarray[2], &ahcf->isar.id_pfr2,
                               ARM64_SYS_REG(3, 0, 0, 3, 4));
+        err |= read_sys_reg32(fdarray[2], &ahcf->isar.id_mmfr5,
+                              ARM64_SYS_REG(3, 0, 0, 3, 6));
 
         /*
          * DBGDIDR is a bit complicated because the kernel doesn't
-- 
2.25.1

Re: [PATCH 3/6] target/arm: Implement ID_MMFR5

[Richard Henderson]

On 8/19/22 04:00, Peter Maydell wrote:
> In Armv8.6 a new AArch32 ID register ID_MMFR5 is defined.
> Implement this; we want to be able to use it to report to
> the guest that we implement FEAT_ETS.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   target/arm/cpu.h    | 1 +
>   target/arm/helper.c | 4 ++--
>   target/arm/kvm64.c  | 2 ++
>   3 files changed, 5 i

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

r~

[PATCH 4/6] target/arm: Implement ID_DFR1

[Peter Maydell]

In Armv8.6, a new AArch32 ID register ID_DFR1 is defined; implement
it. We don't have any CPUs with features that they need to advertise
here yet, but plumbing in the ID register gives it the right name
when debugging and will help in future when we do add a CPU that
has non-zero ID_DFR1 fields.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/cpu.h    | 1 +
 target/arm/helper.c | 4 ++--
 target/arm/kvm64.c  | 2 ++
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index fcc5927587e..fa24ce9f96b 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -983,6 +983,7 @@ struct ArchCPU {
         uint32_t mvfr1;
         uint32_t mvfr2;
         uint32_t id_dfr0;
+        uint32_t id_dfr1;
         uint32_t dbgdidr;
         uint32_t dbgdevid;
         uint32_t dbgdevid1;
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 07378519259..7ff03f1a4ba 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -7581,11 +7581,11 @@ void register_cp_regs_for_features(ARMCPU *cpu)
               .access = PL1_R, .type = ARM_CP_CONST,
               .accessfn = access_aa64_tid3,
               .resetvalue = cpu->isar.id_pfr2 },
-            { .name = "RES_0_C0_C3_5", .state = ARM_CP_STATE_BOTH,
+            { .name = "ID_DFR1", .state = ARM_CP_STATE_BOTH,
               .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 3, .opc2 = 5,
               .access = PL1_R, .type = ARM_CP_CONST,
               .accessfn = access_aa64_tid3,
-              .resetvalue = 0 },
+              .resetvalue = cpu->isar.id_dfr1 },
             { .name = "ID_MMFR5", .state = ARM_CP_STATE_BOTH,
               .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 3, .opc2 = 6,
               .access = PL1_R, .type = ARM_CP_CONST,
diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 2d737c443eb..1197253d12f 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -643,6 +643,8 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf)
                               ARM64_SYS_REG(3, 0, 0, 3, 2));
         err |= read_sys_reg32(fdarray[2], &ahcf->isar.id_pfr2,
                               ARM64_SYS_REG(3, 0, 0, 3, 4));
+        err |= read_sys_reg32(fdarray[2], &ahcf->isar.id_dfr1,
+                              ARM64_SYS_REG(3, 0, 0, 3, 5));
         err |= read_sys_reg32(fdarray[2], &ahcf->isar.id_mmfr5,
                               ARM64_SYS_REG(3, 0, 0, 3, 6));
 
-- 
2.25.1

Re: [PATCH 4/6] target/arm: Implement ID_DFR1

[Richard Henderson]

On 8/19/22 04:00, Peter Maydell wrote:
> In Armv8.6, a new AArch32 ID register ID_DFR1 is defined; implement
> it. We don't have any CPUs with features that they need to advertise
> here yet, but plumbing in the ID register gives it the right name
> when debugging and will help in future when we do add a CPU that
> has non-zero ID_DFR1 fields.
> 
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   target/arm/cpu.h    | 1 +
>   target/arm/helper.c | 4 ++--
>   target/arm/kvm64.c  | 2 ++
>   3 files changed, 5 insertions(+), 2 deletions(-)

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


r~

[PATCH 5/6] target/arm: Advertise FEAT_ETS for '-cpu max'

[Peter Maydell]

The architectural feature FEAT_ETS (Enhanced Translation
Synchronization) is a set of tightened guarantees about memory
ordering involving translation table walks:

 * if memory access RW1 is ordered-before memory access RW2 then it
   is also ordered-before any translation table walk generated by RW2
   that generates a translation fault, address size fault or access
   fault

 * TLB maintenance on non-exec-permission translations is guaranteed
   complete after a DSB (ie it does not need the context
   synchronization event that you have to have if you don’t have
   FEAT_ETS)

For QEMU’s implementation we don’t reorder translation table walk
accesses, and we guarantee to finish the TLB maintenance as soon as
the TLB op is done (the tlb_flush functions will complete at the end
of the TLB, and TLB ops always end the TB because they’re sysreg
writes).

So we’re already compliant and all we need to do is say so in the ID
registers for the 'max' CPU.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 docs/system/arm/emulation.rst | 1 +
 target/arm/cpu64.c            | 1 +
 target/arm/cpu_tcg.c          | 4 ++++
 3 files changed, 6 insertions(+)

diff --git a/docs/system/arm/emulation.rst b/docs/system/arm/emulation.rst
index 8e494c8bea5..811358fd0a0 100644
--- a/docs/system/arm/emulation.rst
+++ b/docs/system/arm/emulation.rst
@@ -24,6 +24,7 @@ the following architecture extensions:
 - FEAT_Debugv8p4 (Debug changes for v8.4)
 - FEAT_DotProd (Advanced SIMD dot product instructions)
 - FEAT_DoubleFault (Double Fault Extension)
+- FEAT_ETS (Enhanced Translation Synchronization)
 - FEAT_FCMA (Floating-point complex number instructions)
 - FEAT_FHM (Floating-point half-precision multiplication instructions)
 - FEAT_FP16 (Half-precision floating-point data processing)
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c
index 78e27f778ac..6d65248f29c 100644
--- a/target/arm/cpu64.c
+++ b/target/arm/cpu64.c
@@ -1043,6 +1043,7 @@ static void aarch64_max_initfn(Object *obj)
     t = FIELD_DP64(t, ID_AA64MMFR1, LO, 1);       /* FEAT_LOR */
     t = FIELD_DP64(t, ID_AA64MMFR1, PAN, 2);      /* FEAT_PAN2 */
     t = FIELD_DP64(t, ID_AA64MMFR1, XNX, 1);      /* FEAT_XNX */
+    t = FIELD_DP64(t, ID_AA64MMFR1, ETS, 1);      /* FEAT_ETS */
     t = FIELD_DP64(t, ID_AA64MMFR1, HCX, 1);      /* FEAT_HCX */
     cpu->isar.id_aa64mmfr1 = t;
 
diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c
index 3099b38e32b..f63f8cdd954 100644
--- a/target/arm/cpu_tcg.c
+++ b/target/arm/cpu_tcg.c
@@ -67,6 +67,10 @@ void aa32_max_features(ARMCPU *cpu)
     t = FIELD_DP32(t, ID_MMFR4, XNX, 1);          /* FEAT_XNX*/
     cpu->isar.id_mmfr4 = t;
 
+    t = cpu->isar.id_mmfr5;
+    t = FIELD_DP32(t, ID_MMFR5, ETS, 1);          /* FEAT_ETS */
+    cpu->isar.id_mmfr5 = t;
+
     t = cpu->isar.id_pfr0;
     t = FIELD_DP32(t, ID_PFR0, CSV2, 2);          /* FEAT_CVS2 */
     t = FIELD_DP32(t, ID_PFR0, DIT, 1);           /* FEAT_DIT */
-- 
2.25.1

Re: [PATCH 5/6] target/arm: Advertise FEAT_ETS for '-cpu max'

[Richard Henderson]

On 8/19/22 04:00, Peter Maydell wrote:
> The architectural feature FEAT_ETS (Enhanced Translation
> Synchronization) is a set of tightened guarantees about memory
> ordering involving translation table walks:
> 
>   * if memory access RW1 is ordered-before memory access RW2 then it
>     is also ordered-before any translation table walk generated by RW2
>     that generates a translation fault, address size fault or access
>     fault
> 
>   * TLB maintenance on non-exec-permission translations is guaranteed
>     complete after a DSB (ie it does not need the context
>     synchronization event that you have to have if you don’t have
>     FEAT_ETS)
> 
> For QEMU’s implementation we don’t reorder translation table walk
> accesses, and we guarantee to finish the TLB maintenance as soon as
> the TLB op is done (the tlb_flush functions will complete at the end
> of the TLB, and TLB ops always end the TB because they’re sysreg

First TLB on this line should be TB.

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


r~

[PATCH 6/6] target/arm: Add missing space in comment

[Peter Maydell]

Fix a missing space before a comment terminator.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/cpu_tcg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/arm/cpu_tcg.c b/target/arm/cpu_tcg.c
index f63f8cdd954..b714c61d940 100644
--- a/target/arm/cpu_tcg.c
+++ b/target/arm/cpu_tcg.c
@@ -64,7 +64,7 @@ void aa32_max_features(ARMCPU *cpu)
     t = FIELD_DP32(t, ID_MMFR4, HPDS, 1);         /* FEAT_AA32HPD */
     t = FIELD_DP32(t, ID_MMFR4, AC2, 1);          /* ACTLR2, HACTLR2 */
     t = FIELD_DP32(t, ID_MMFR4, CNP, 1);          /* FEAT_TTCNP */
-    t = FIELD_DP32(t, ID_MMFR4, XNX, 1);          /* FEAT_XNX*/
+    t = FIELD_DP32(t, ID_MMFR4, XNX, 1);          /* FEAT_XNX */
     cpu->isar.id_mmfr4 = t;
 
     t = cpu->isar.id_mmfr5;
-- 
2.25.1

Re: [PATCH 6/6] target/arm: Add missing space in comment

[Richard Henderson]

On 8/19/22 04:00, Peter Maydell wrote:
> Fix a missing space before a comment terminator.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   target/arm/cpu_tcg.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

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


r~

Re: [PATCH 0/6] target/arm: Fix v8 AArch32 RAZ ID regs; implement FEAT_ETS

[Richard Henderson]

On 8/19/22 04:00, Peter Maydell wrote:
> The main aim of this patchset is to implement FEAT_ETS.
> FEAT_ETS provides tighter guarantees on some memory orderings
> involving translation table walks that permit guest code to
> skip the context-synchronization event they would otherwise
> need to do after a TLB maintenance operation. QEMU already
> provides the tighter guarantees this feature requires, so
> all we need to do is advertise it in the ID registers...
> 
> ...except that it turns out that for AArch32 this is done
> in ID_MMFR5, which is a new-in-v8.6 register that we don't
> implement yet. So we need to provide it. And while I was
> doing that I noticed that we accidentally forgot to
> implement a big chunk of the "reserved for future ID
> registers, must RAZ" cp15 space for v8 AArch32. So the
> big bit of the patchset is sorting that out :-)

Thanks, queued to target-arm.next.


r~