diff mbox series

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

Message ID 20220819110052.2942289-2-peter.maydell@linaro.org (mailing list archive)
State New, archived
Headers show
Series target/arm: Fix v8 AArch32 RAZ ID regs; implement FEAT_ETS | expand

Commit Message

Peter Maydell Aug. 19, 2022, 11 a.m. UTC
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(-)

Comments

Richard Henderson Aug. 20, 2022, 2:37 a.m. UTC | #1
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~
Peter Maydell Aug. 22, 2022, 8:48 a.m. UTC | #2
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
diff mbox series

Patch

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);
+        }
     }
 
     /*