diff mbox

[RFC,v4,08/20] target-arm: Store cp15 c0_c1 and c0_c2 in ARMCPUClass

Message ID 1331398436-20761-9-git-send-email-afaerber@suse.de
State New
Headers show

Commit Message

Andreas Färber March 10, 2012, 4:53 p.m. UTC 
For now set them in the reset function.

Signed-off-by: Andreas Färber <afaerber@suse.de>
Cc: Peter Maydell <peter.maydell@linaro.org>
---
 target-arm/cpu-qom.h |    2 +
 target-arm/cpu.c     |   63 ++++++++++++++++++++++++++++++++++++++++++++++++++
 target-arm/helper.c  |   51 ----------------------------------------
 3 files changed, 65 insertions(+), 51 deletions(-)

Comments

Paul Brook March 15, 2012, 7:08 p.m. UTC | #1 
> For now set them in the reset function.

> +    /* TODO Move these into arm_cpu_initfn() once no longer zeroed above.*/
> +    memcpy(env->cp15.c0_c1, klass->cp15.c0_c1, 8 * sizeof(uint32_t));
> +    memcpy(env->cp15.c0_c2, klass->cp15.c0_c2, 8 * sizeof(uint32_t)); +

Why bother copying them into the CPU state?  These are readonly, so anything 
that needs them should be able to use the value straight from the class 
definitions.

Paul
Peter Maydell March 15, 2012, 7:20 p.m. UTC | #2 
On 15 March 2012 19:08, Paul Brook <paul@codesourcery.com> wrote:
>> For now set them in the reset function.
>
>> +    /* TODO Move these into arm_cpu_initfn() once no longer zeroed above.*/
>> +    memcpy(env->cp15.c0_c1, klass->cp15.c0_c1, 8 * sizeof(uint32_t));
>> +    memcpy(env->cp15.c0_c2, klass->cp15.c0_c2, 8 * sizeof(uint32_t)); +
>
> Why bother copying them into the CPU state?  These are readonly, so anything
> that needs them should be able to use the value straight from the class
> definitions.

In my (hugely delayed) cp15 rework attempt these probably go away anyway
in favour of having each CPU register a pile of registers along the lines of

    { .name = "ID_PFR0", .cp = 15, .crn = 0, .crm = 1, .opc1 = 0, .opc2 = 0,
      .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0x00001231 },
    { .name = "ID_PFR1", .cp = 15, .crn = 0, .crm = 1, .opc1 = 0, .opc2 = 1,
      .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0x00000011 },
etc.

I'm not sure there's any need to retain the CPUState (or equivalent) fields
for them at that point.

-- PMM
Alexey Starikovskiy March 15, 2012, 7:29 p.m. UTC | #3 
On Thu, Mar 15, 2012 at 11:20 PM, Peter Maydell
<peter.maydell@linaro.org> wrote:
> On 15 March 2012 19:08, Paul Brook <paul@codesourcery.com> wrote:
>>> For now set them in the reset function.
>>
>>> +    /* TODO Move these into arm_cpu_initfn() once no longer zeroed above.*/
>>> +    memcpy(env->cp15.c0_c1, klass->cp15.c0_c1, 8 * sizeof(uint32_t));
>>> +    memcpy(env->cp15.c0_c2, klass->cp15.c0_c2, 8 * sizeof(uint32_t)); +
>>
>> Why bother copying them into the CPU state?  These are readonly, so anything
>> that needs them should be able to use the value straight from the class
>> definitions.
>
> In my (hugely delayed) cp15 rework attempt these probably go away anyway
> in favour of having each CPU register a pile of registers along the lines of
>
>    { .name = "ID_PFR0", .cp = 15, .crn = 0, .crm = 1, .opc1 = 0, .opc2 = 0,
>      .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0x00001231 },
>    { .name = "ID_PFR1", .cp = 15, .crn = 0, .crm = 1, .opc1 = 0, .opc2 = 1,
>      .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0x00000011 },
> etc.
>
> I'm not sure there's any need to retain the CPUState (or equivalent) fields
> for them at that point.
>
> -- PMM
>
Peter, with the reserved bits in most registers it might make sense to
have resetvalue in two
masks -- ones and zeros, so that you can prevent changing of those bits.

Alex.
Peter Maydell March 15, 2012, 7:42 p.m. UTC | #4 
On 15 March 2012 19:29, Alexey Starikovskiy <aystarik@gmail.com> wrote:
> On Thu, Mar 15, 2012 at 11:20 PM, Peter Maydell
> <peter.maydell@linaro.org> wrote:
>> On 15 March 2012 19:08, Paul Brook <paul@codesourcery.com> wrote:
>>>> For now set them in the reset function.
>>>
>>>> +    /* TODO Move these into arm_cpu_initfn() once no longer zeroed above.*/
>>>> +    memcpy(env->cp15.c0_c1, klass->cp15.c0_c1, 8 * sizeof(uint32_t));
>>>> +    memcpy(env->cp15.c0_c2, klass->cp15.c0_c2, 8 * sizeof(uint32_t)); +
>>>
>>> Why bother copying them into the CPU state?  These are readonly, so anything
>>> that needs them should be able to use the value straight from the class
>>> definitions.
>>
>> In my (hugely delayed) cp15 rework attempt these probably go away anyway
>> in favour of having each CPU register a pile of registers along the lines of
>>
>>    { .name = "ID_PFR0", .cp = 15, .crn = 0, .crm = 1, .opc1 = 0, .opc2 = 0,
>>      .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0x00001231 },
>>    { .name = "ID_PFR1", .cp = 15, .crn = 0, .crm = 1, .opc1 = 0, .opc2 = 1,
>>      .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = 0x00000011 },
>> etc.
>>
>> I'm not sure there's any need to retain the CPUState (or equivalent) fields
>> for them at that point.
>>
>> -- PMM
>>
> Peter, with the reserved bits in most registers it might make sense to
> have resetvalue in two
> masks -- ones and zeros, so that you can prevent changing of those bits.

Well, in this case it's totally read only. Mostly QEMU at the moment doesn't
enforce selectively read-only bits (we probably should!) so I haven't
put in read-only-masks in my conversion so far, but it's an obvious
possibility.

(Current status:
http://git.linaro.org/gitweb?p=people/pmaydell/qemu-arm.git;a=shortlog;h=refs/heads/cp15-rework
still TODO:
 cp15 crn=0 conversion ;
 drop special case of mcrr in favour of explicitly implementing
 block cache ops registers ;
 reset handling of SCTLR still relies on cpu_reset_model_id ;
 arrangement of define_arm_cp_regs() calls could probably be improved.
I'm also toying with the idea of just having each CPU we support go
through and register its own cp15 registers in a big long list (ie
no attempt at factoring out "these are registers all v7 cores have",
"these are registers for feature X", etc). That sounds kind of weird
but it might help in separating out changes to migration state for
CPU A from those for CPU B.)

-- PMM
diff mbox

Patch

diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h
index 6c97337..0148d18 100644
--- a/target-arm/cpu-qom.h
+++ b/target-arm/cpu-qom.h
@@ -48,6 +48,8 @@  typedef struct ARMCPUClass {
 
     struct {
         uint32_t c0_cpuid;
+        uint32_t c0_c1[8];
+        uint32_t c0_c2[8];
     } cp15;
 
     uint32_t features;
diff --git a/target-arm/cpu.c b/target-arm/cpu.c
index f4c05d8..74be400 100644
--- a/target-arm/cpu.c
+++ b/target-arm/cpu.c
@@ -45,6 +45,10 @@  static void arm_cpu_reset(CPUState *c)
     env->cp15.c0_cpuid = id;
     env->cp15.c15_config_base_address = tmp;
 
+    /* TODO Move these into arm_cpu_initfn() once no longer zeroed above. */
+    memcpy(env->cp15.c0_c1, klass->cp15.c0_c1, 8 * sizeof(uint32_t));
+    memcpy(env->cp15.c0_c2, klass->cp15.c0_c2, 8 * sizeof(uint32_t));
+
 #if defined(CONFIG_USER_ONLY)
     env->uncached_cpsr = ARM_CPU_MODE_USR;
     /* For user mode we must enable access to coprocessors */
@@ -165,6 +169,8 @@  static inline void unset_class_feature(ARMCPUClass *klass, int feature)
 typedef struct ARMCPUInfo {
     const char *name;
     uint32_t id;
+    uint32_t cp15_c0_c1[8];
+    uint32_t cp15_c0_c2[8];
     uint32_t features;
     void (*class_init)(ARMCPUClass *klass, const struct ARMCPUInfo *info);
 } ARMCPUInfo;
@@ -177,6 +183,13 @@  static void arm1136_r0_class_init(ARMCPUClass *k, const ARMCPUInfo *info)
 
     k->features = r1_class->features;
     unset_class_feature(k, ARM_FEATURE_V6K);
+
+    /* These ID register values are correct for 1136 but may be wrong
+     * for 1136_r2 (in particular r0p2 does not actually implement most
+     * of the ID registers).
+     */
+    memcpy(k->cp15.c0_c1, r1_class->cp15.c0_c1, 8 * sizeof(uint32_t));
+    memcpy(k->cp15.c0_c2, r1_class->cp15.c0_c2, 8 * sizeof(uint32_t));
 }
 
 static void ti925t_reset(CPUState *c)
@@ -246,12 +259,28 @@  static const ARMCPUInfo arm_cpus[] = {
     {
         .name = "arm1136",
         .id = 0x4117b363,
+        .cp15_c0_c1 = {
+            0x111, 0x1, 0x2, 0x3,
+            0x01130003, 0x10030302, 0x01222110, 0
+        },
+        .cp15_c0_c2 = {
+            0x00140011, 0x12002111, 0x11231111, 0x01102131,
+            0x141, 0, 0, 0
+        },
         .features = ARM_FEATURE(V6) |
                     ARM_FEATURE(VFP),
     },
     {
         .name = "arm1176",
         .id = 0x410fb767,
+        .cp15_c0_c1 = {
+            0x111, 0x11, 0x33, 0,
+            0x01130003, 0x10030302, 0x01222100, 0
+        },
+        .cp15_c0_c2 = {
+            0x0140011, 0x12002111, 0x11231121, 0x01102131,
+            0x01141, 0, 0, 0
+        },
         .features = ARM_FEATURE(V6K) |
                     ARM_FEATURE(VFP) |
                     ARM_FEATURE(VAPA),
@@ -259,6 +288,14 @@  static const ARMCPUInfo arm_cpus[] = {
     {
         .name = "arm11mpcore",
         .id = 0x410fb022,
+        .cp15_c0_c1 = {
+            0x111, 0x1, 0, 0x2,
+            0x01100103, 0x10020302, 0x01222000, 0
+        },
+        .cp15_c0_c2 = {
+            0x00100011, 0x12002111, 0x11221011, 0x01102131,
+            0x141, 0, 0, 0
+        },
         .features = ARM_FEATURE(V6K) |
                     ARM_FEATURE(VFP) |
                     ARM_FEATURE(VAPA),
@@ -272,6 +309,14 @@  static const ARMCPUInfo arm_cpus[] = {
     {
         .name = "cortex-a8",
         .id = 0x410fc080,
+        .cp15_c0_c1 = {
+            0x1031, 0x11, 0x400, 0,
+            0x31100003, 0x20000000, 0x01202000, 0x11
+        },
+        .cp15_c0_c2 = {
+            0x00101111, 0x12112111, 0x21232031, 0x11112131,
+            0x00111142, 0, 0, 0
+        },
         .features = ARM_FEATURE(V7) |
                     ARM_FEATURE(VFP3) |
                     ARM_FEATURE(NEON) |
@@ -280,6 +325,14 @@  static const ARMCPUInfo arm_cpus[] = {
     {
         .name = "cortex-a9",
         .id = 0x410fc090,
+        .cp15_c0_c1 = {
+            0x1031, 0x11, 0x000, 0,
+            0x00100103, 0x20000000, 0x01230000, 0x00002111
+        },
+        .cp15_c0_c2 = {
+            0x00101111, 0x13112111, 0x21232041, 0x11112131,
+            0x00111142, 0, 0, 0
+        },
         .features = ARM_FEATURE(V7) |
                     ARM_FEATURE(VFP3) |
                     ARM_FEATURE(VFP_FP16) |
@@ -294,6 +347,14 @@  static const ARMCPUInfo arm_cpus[] = {
     {
         .name = "cortex-a15",
         .id = 0x412fc0f1,
+        .cp15_c0_c1 = {
+            0x00001131, 0x00011011, 0x02010555, 0x00000000,
+            0x10201105, 0x20000000, 0x01240000, 0x02102211
+        },
+        .cp15_c0_c2 = {
+            0x02101110, 0x13112111, 0x21232041, 0x11112131,
+            0x10011142, 0, 0, 0
+        },
         .features = ARM_FEATURE(V7) |
                     ARM_FEATURE(VFP4) |
                     ARM_FEATURE(VFP_FP16) |
@@ -412,6 +473,8 @@  static void arm_cpu_class_init(ObjectClass *klass, void *data)
     cpu_class->reset = arm_cpu_reset;
 
     k->cp15.c0_cpuid = info->id;
+    memcpy(k->cp15.c0_c1, info->cp15_c0_c1, 8 * sizeof(uint32_t));
+    memcpy(k->cp15.c0_c2, info->cp15_c0_c2, 8 * sizeof(uint32_t));
     k->features = info->features;
 
     if (info->class_init != NULL) {
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 59a9812..c0cfa17 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -5,45 +5,6 @@ 
 #include "sysemu.h"
 #include "cpu-qom.h"
 
-static uint32_t cortexa15_cp15_c0_c1[8] = {
-    0x00001131, 0x00011011, 0x02010555, 0x00000000,
-    0x10201105, 0x20000000, 0x01240000, 0x02102211
-};
-
-static uint32_t cortexa15_cp15_c0_c2[8] = {
-    0x02101110, 0x13112111, 0x21232041, 0x11112131, 0x10011142, 0, 0, 0
-};
-
-static uint32_t cortexa9_cp15_c0_c1[8] =
-{ 0x1031, 0x11, 0x000, 0, 0x00100103, 0x20000000, 0x01230000, 0x00002111 };
-
-static uint32_t cortexa9_cp15_c0_c2[8] =
-{ 0x00101111, 0x13112111, 0x21232041, 0x11112131, 0x00111142, 0, 0, 0 };
-
-static uint32_t cortexa8_cp15_c0_c1[8] =
-{ 0x1031, 0x11, 0x400, 0, 0x31100003, 0x20000000, 0x01202000, 0x11 };
-
-static uint32_t cortexa8_cp15_c0_c2[8] =
-{ 0x00101111, 0x12112111, 0x21232031, 0x11112131, 0x00111142, 0, 0, 0 };
-
-static uint32_t mpcore_cp15_c0_c1[8] =
-{ 0x111, 0x1, 0, 0x2, 0x01100103, 0x10020302, 0x01222000, 0 };
-
-static uint32_t mpcore_cp15_c0_c2[8] =
-{ 0x00100011, 0x12002111, 0x11221011, 0x01102131, 0x141, 0, 0, 0 };
-
-static uint32_t arm1136_cp15_c0_c1[8] =
-{ 0x111, 0x1, 0x2, 0x3, 0x01130003, 0x10030302, 0x01222110, 0 };
-
-static uint32_t arm1136_cp15_c0_c2[8] =
-{ 0x00140011, 0x12002111, 0x11231111, 0x01102131, 0x141, 0, 0, 0 };
-
-static uint32_t arm1176_cp15_c0_c1[8] =
-{ 0x111, 0x11, 0x33, 0, 0x01130003, 0x10030302, 0x01222100, 0 };
-
-static uint32_t arm1176_cp15_c0_c2[8] =
-{ 0x0140011, 0x12002111, 0x11231121, 0x01102131, 0x01141, 0, 0, 0 };
-
 static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
 {
     switch (id) {
@@ -76,8 +37,6 @@  static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
         env->vfp.xregs[ARM_VFP_FPSID] = 0x410120b4;
         env->vfp.xregs[ARM_VFP_MVFR0] = 0x11111111;
         env->vfp.xregs[ARM_VFP_MVFR1] = 0x00000000;
-        memcpy(env->cp15.c0_c1, arm1136_cp15_c0_c1, 8 * sizeof(uint32_t));
-        memcpy(env->cp15.c0_c2, arm1136_cp15_c0_c2, 8 * sizeof(uint32_t));
         env->cp15.c0_cachetype = 0x1dd20d2;
         env->cp15.c1_sys = 0x00050078;
         break;
@@ -85,8 +44,6 @@  static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
         env->vfp.xregs[ARM_VFP_FPSID] = 0x410120b5;
         env->vfp.xregs[ARM_VFP_MVFR0] = 0x11111111;
         env->vfp.xregs[ARM_VFP_MVFR1] = 0x00000000;
-        memcpy(env->cp15.c0_c1, arm1176_cp15_c0_c1, 8 * sizeof(uint32_t));
-        memcpy(env->cp15.c0_c2, arm1176_cp15_c0_c2, 8 * sizeof(uint32_t));
         env->cp15.c0_cachetype = 0x1dd20d2;
         env->cp15.c1_sys = 0x00050078;
         break;
@@ -94,16 +51,12 @@  static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
         env->vfp.xregs[ARM_VFP_FPSID] = 0x410120b4;
         env->vfp.xregs[ARM_VFP_MVFR0] = 0x11111111;
         env->vfp.xregs[ARM_VFP_MVFR1] = 0x00000000;
-        memcpy(env->cp15.c0_c1, mpcore_cp15_c0_c1, 8 * sizeof(uint32_t));
-        memcpy(env->cp15.c0_c2, mpcore_cp15_c0_c2, 8 * sizeof(uint32_t));
         env->cp15.c0_cachetype = 0x1dd20d2;
         break;
     case ARM_CPUID_CORTEXA8:
         env->vfp.xregs[ARM_VFP_FPSID] = 0x410330c0;
         env->vfp.xregs[ARM_VFP_MVFR0] = 0x11110222;
         env->vfp.xregs[ARM_VFP_MVFR1] = 0x00011100;
-        memcpy(env->cp15.c0_c1, cortexa8_cp15_c0_c1, 8 * sizeof(uint32_t));
-        memcpy(env->cp15.c0_c2, cortexa8_cp15_c0_c2, 8 * sizeof(uint32_t));
         env->cp15.c0_cachetype = 0x82048004;
         env->cp15.c0_clid = (1 << 27) | (2 << 24) | 3;
         env->cp15.c0_ccsid[0] = 0xe007e01a; /* 16k L1 dcache. */
@@ -115,8 +68,6 @@  static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
         env->vfp.xregs[ARM_VFP_FPSID] = 0x41033090;
         env->vfp.xregs[ARM_VFP_MVFR0] = 0x11110222;
         env->vfp.xregs[ARM_VFP_MVFR1] = 0x01111111;
-        memcpy(env->cp15.c0_c1, cortexa9_cp15_c0_c1, 8 * sizeof(uint32_t));
-        memcpy(env->cp15.c0_c2, cortexa9_cp15_c0_c2, 8 * sizeof(uint32_t));
         env->cp15.c0_cachetype = 0x80038003;
         env->cp15.c0_clid = (1 << 27) | (1 << 24) | 3;
         env->cp15.c0_ccsid[0] = 0xe00fe015; /* 16k L1 dcache. */
@@ -127,8 +78,6 @@  static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
         env->vfp.xregs[ARM_VFP_FPSID] = 0x410430f0;
         env->vfp.xregs[ARM_VFP_MVFR0] = 0x10110222;
         env->vfp.xregs[ARM_VFP_MVFR1] = 0x11111111;
-        memcpy(env->cp15.c0_c1, cortexa15_cp15_c0_c1, 8 * sizeof(uint32_t));
-        memcpy(env->cp15.c0_c2, cortexa15_cp15_c0_c2, 8 * sizeof(uint32_t));
         env->cp15.c0_cachetype = 0x8444c004;
         env->cp15.c0_clid = 0x0a200023;
         env->cp15.c0_ccsid[0] = 0x701fe00a; /* 32K L1 dcache */