diff mbox

[v6,28/37] target-arm: Don't expose wildcard ID register definitions for ARMv8

Message ID 1397146536-30116-29-git-send-email-peter.maydell@linaro.org
State New
Headers show

Commit Message

Peter Maydell April 10, 2014, 4:15 p.m. UTC
In ARMv8 the 32 bit coprocessor ID register space is tidied up to
remove the wildcarded aliases of the MIDR and the RAZ behaviour
for the unassigned space where crm = 3..7. Make sure we don't
expose thes wildcards for v8 cores. This means we need to have
a specific implementation for REVIDR, an IMPDEF register which
may be the same as the MIDR (and which we always implement as such).

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

Comments

Peter Crosthwaite April 14, 2014, 6:27 a.m. UTC | #1
On Fri, Apr 11, 2014 at 2:15 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> In ARMv8 the 32 bit coprocessor ID register space is tidied up to
> remove the wildcarded aliases of the MIDR and the RAZ behaviour
> for the unassigned space where crm = 3..7. Make sure we don't
> expose thes wildcards for v8 cores. This means we need to have
> a specific implementation for REVIDR, an IMPDEF register which
> may be the same as the MIDR (and which we always implement as such).
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target-arm/helper.c | 60 +++++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 42 insertions(+), 18 deletions(-)
>
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index 398c8f5..20952c3 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -2258,8 +2258,9 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>       * be read-only (ie write causes UNDEF exception).
>       */
>      {
> -        ARMCPRegInfo id_cp_reginfo[] = {
> -            /* Note that the MIDR isn't a simple constant register because
> +        ARMCPRegInfo id_old_midr_cp_reginfo[] = {

"old" is fairly ambiguous and tends to change definition quickly. Can
you be more specific with:

id_pre_v8_midr_cp_reginfo

> +            /* Pre-v8 MIDR space.

And then this is self documented.

Otherwise,

Reviewed-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>

> +             * Note that the MIDR isn't a simple constant register because
>               * of the TI925 behaviour where writes to another register can
>               * cause the MIDR value to change.
>               *
> @@ -2273,22 +2274,6 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>                .writefn = arm_cp_write_ignore, .raw_writefn = raw_write,
>                .fieldoffset = offsetof(CPUARMState, cp15.c0_cpuid),
>                .type = ARM_CP_OVERRIDE },
> -            { .name = "MIDR_EL1", .state = ARM_CP_STATE_AA64,
> -              .opc0 = 3, .opc1 = 0, .opc2 = 0, .crn = 0, .crm = 0,
> -              .access = PL1_R, .resetvalue = cpu->midr, .type = ARM_CP_CONST },
> -            { .name = "CTR",
> -              .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 1,
> -              .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = cpu->ctr },
> -            { .name = "CTR_EL0", .state = ARM_CP_STATE_AA64,
> -              .opc0 = 3, .opc1 = 3, .opc2 = 1, .crn = 0, .crm = 0,
> -              .access = PL0_R, .accessfn = ctr_el0_access,
> -              .type = ARM_CP_CONST, .resetvalue = cpu->ctr },
> -            { .name = "TCMTR",
> -              .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 2,
> -              .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0 },
> -            { .name = "TLBTR",
> -              .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 3,
> -              .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0 },
>              /* crn = 0 op1 = 0 crm = 3..7 : currently unassigned; we RAZ. */
>              { .name = "DUMMY",
>                .cp = 15, .crn = 0, .crm = 3, .opc1 = 0, .opc2 = CP_ANY,
> @@ -2307,6 +2292,37 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>                .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0 },
>              REGINFO_SENTINEL
>          };
> +        ARMCPRegInfo id_v8_midr_cp_reginfo[] = {
> +            /* v8 MIDR -- the wildcard isn't necessary, and nor is the
> +             * variable-MIDR TI925 behaviour. Instead we have a single
> +             * (strictly speaking IMPDEF) alias of the MIDR, REVIDR.
> +             */
> +            { .name = "MIDR_EL1", .state = ARM_CP_STATE_BOTH,
> +              .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 0, .opc2 = 0,
> +              .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = cpu->midr },
> +            { .name = "REVIDR_EL1", .state = ARM_CP_STATE_BOTH,
> +              .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 0, .opc2 = 6,
> +              .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = cpu->midr },
> +            REGINFO_SENTINEL
> +        };
> +        ARMCPRegInfo id_cp_reginfo[] = {
> +            /* These are common to v8 and pre-v8 */
> +            { .name = "CTR",
> +              .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 1,
> +              .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = cpu->ctr },
> +            { .name = "CTR_EL0", .state = ARM_CP_STATE_AA64,
> +              .opc0 = 3, .opc1 = 3, .opc2 = 1, .crn = 0, .crm = 0,
> +              .access = PL0_R, .accessfn = ctr_el0_access,
> +              .type = ARM_CP_CONST, .resetvalue = cpu->ctr },
> +            /* TCMTR and TLBTR exist in v8 but have no 64-bit versions */
> +            { .name = "TCMTR",
> +              .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 2,
> +              .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0 },
> +            { .name = "TLBTR",
> +              .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 3,
> +              .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0 },
> +            REGINFO_SENTINEL
> +        };
>          ARMCPRegInfo crn0_wi_reginfo = {
>              .name = "CRN0_WI", .cp = 15, .crn = 0, .crm = CP_ANY,
>              .opc1 = CP_ANY, .opc2 = CP_ANY, .access = PL1_W,
> @@ -2321,10 +2337,18 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>               * UNDEF.
>               */
>              define_one_arm_cp_reg(cpu, &crn0_wi_reginfo);
> +            for (r = id_old_midr_cp_reginfo; r->type != ARM_CP_SENTINEL; r++) {
> +                r->access = PL1_RW;
> +            }
>              for (r = id_cp_reginfo; r->type != ARM_CP_SENTINEL; r++) {
>                  r->access = PL1_RW;
>              }
>          }
> +        if (arm_feature(env, ARM_FEATURE_V8)) {
> +            define_arm_cp_regs(cpu, id_v8_midr_cp_reginfo);
> +        } else {
> +            define_arm_cp_regs(cpu, id_old_midr_cp_reginfo);
> +        }
>          define_arm_cp_regs(cpu, id_cp_reginfo);
>      }
>
> --
> 1.9.1
>
>
diff mbox

Patch

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 398c8f5..20952c3 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2258,8 +2258,9 @@  void register_cp_regs_for_features(ARMCPU *cpu)
      * be read-only (ie write causes UNDEF exception).
      */
     {
-        ARMCPRegInfo id_cp_reginfo[] = {
-            /* Note that the MIDR isn't a simple constant register because
+        ARMCPRegInfo id_old_midr_cp_reginfo[] = {
+            /* Pre-v8 MIDR space.
+             * Note that the MIDR isn't a simple constant register because
              * of the TI925 behaviour where writes to another register can
              * cause the MIDR value to change.
              *
@@ -2273,22 +2274,6 @@  void register_cp_regs_for_features(ARMCPU *cpu)
               .writefn = arm_cp_write_ignore, .raw_writefn = raw_write,
               .fieldoffset = offsetof(CPUARMState, cp15.c0_cpuid),
               .type = ARM_CP_OVERRIDE },
-            { .name = "MIDR_EL1", .state = ARM_CP_STATE_AA64,
-              .opc0 = 3, .opc1 = 0, .opc2 = 0, .crn = 0, .crm = 0,
-              .access = PL1_R, .resetvalue = cpu->midr, .type = ARM_CP_CONST },
-            { .name = "CTR",
-              .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 1,
-              .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = cpu->ctr },
-            { .name = "CTR_EL0", .state = ARM_CP_STATE_AA64,
-              .opc0 = 3, .opc1 = 3, .opc2 = 1, .crn = 0, .crm = 0,
-              .access = PL0_R, .accessfn = ctr_el0_access,
-              .type = ARM_CP_CONST, .resetvalue = cpu->ctr },
-            { .name = "TCMTR",
-              .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 2,
-              .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0 },
-            { .name = "TLBTR",
-              .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 3,
-              .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0 },
             /* crn = 0 op1 = 0 crm = 3..7 : currently unassigned; we RAZ. */
             { .name = "DUMMY",
               .cp = 15, .crn = 0, .crm = 3, .opc1 = 0, .opc2 = CP_ANY,
@@ -2307,6 +2292,37 @@  void register_cp_regs_for_features(ARMCPU *cpu)
               .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0 },
             REGINFO_SENTINEL
         };
+        ARMCPRegInfo id_v8_midr_cp_reginfo[] = {
+            /* v8 MIDR -- the wildcard isn't necessary, and nor is the
+             * variable-MIDR TI925 behaviour. Instead we have a single
+             * (strictly speaking IMPDEF) alias of the MIDR, REVIDR.
+             */
+            { .name = "MIDR_EL1", .state = ARM_CP_STATE_BOTH,
+              .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 0, .opc2 = 0,
+              .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = cpu->midr },
+            { .name = "REVIDR_EL1", .state = ARM_CP_STATE_BOTH,
+              .opc0 = 3, .opc1 = 0, .crn = 0, .crm = 0, .opc2 = 6,
+              .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = cpu->midr },
+            REGINFO_SENTINEL
+        };
+        ARMCPRegInfo id_cp_reginfo[] = {
+            /* These are common to v8 and pre-v8 */
+            { .name = "CTR",
+              .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 1,
+              .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = cpu->ctr },
+            { .name = "CTR_EL0", .state = ARM_CP_STATE_AA64,
+              .opc0 = 3, .opc1 = 3, .opc2 = 1, .crn = 0, .crm = 0,
+              .access = PL0_R, .accessfn = ctr_el0_access,
+              .type = ARM_CP_CONST, .resetvalue = cpu->ctr },
+            /* TCMTR and TLBTR exist in v8 but have no 64-bit versions */
+            { .name = "TCMTR",
+              .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 2,
+              .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0 },
+            { .name = "TLBTR",
+              .cp = 15, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 3,
+              .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0 },
+            REGINFO_SENTINEL
+        };
         ARMCPRegInfo crn0_wi_reginfo = {
             .name = "CRN0_WI", .cp = 15, .crn = 0, .crm = CP_ANY,
             .opc1 = CP_ANY, .opc2 = CP_ANY, .access = PL1_W,
@@ -2321,10 +2337,18 @@  void register_cp_regs_for_features(ARMCPU *cpu)
              * UNDEF.
              */
             define_one_arm_cp_reg(cpu, &crn0_wi_reginfo);
+            for (r = id_old_midr_cp_reginfo; r->type != ARM_CP_SENTINEL; r++) {
+                r->access = PL1_RW;
+            }
             for (r = id_cp_reginfo; r->type != ARM_CP_SENTINEL; r++) {
                 r->access = PL1_RW;
             }
         }
+        if (arm_feature(env, ARM_FEATURE_V8)) {
+            define_arm_cp_regs(cpu, id_v8_midr_cp_reginfo);
+        } else {
+            define_arm_cp_regs(cpu, id_old_midr_cp_reginfo);
+        }
         define_arm_cp_regs(cpu, id_cp_reginfo);
     }