[Qemu-devel] [RFC 0/2] target-arm: Adding Cortex-R4F support

[Qemu-devel] [RFC 0/2] target-arm: Adding Cortex-R4F support

[Andreas Färber]

Hello Peter,

I've been looking into adding support for Cortex-R4F.

1) Currently, -cpu is used to look up a Main ID Register value and to base
feature decisions on that. This doesn't work for Cortex-R4 and Cortex-R4F,
which have an identical MIDR but only -R4F has the FPU.
Re-checking the model string, while ugly, does the trick. Comments?

2) The R4/R4F TRM says "It implements the ARMv7R architecture, and includes
Thumb-2 technology" - how to incur the pre-v7 feature bits in addition to
the easy _V7 and _THUMB2? I.e. where is it documented (in ARMv7-A/R TRM?)
whether or not this should include _V4T, _V6K, etc.?
If V7 were to always imply the same set of features, I would expect it
alongside VAPA. (Could use some comments either way.)

3) How to handle processor revisions? The only two available TRMs for R4F
seem to be r1p3 and r1p4. ARM_CPUID_CORTEXA9 seems to use r0p0, with r2p0
being the oldest available TRM atm.
Apart from the actual MIDR define and its binding to -cpu name, this also
affects the FPSID register and gets a little ugly with non-linear mappings.
The ARM1136 seems a particularly bad example, will try to post a cleanup.

Regards,
Andreas

Andreas Färber (2):
  target-arm: Prepare support for Cortex-R4
  target-arm: Add support for Cortex-R4F

 target-arm/cpu.h    |    1 +
 target-arm/helper.c |   49 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+), 0 deletions(-)

-- 
1.7.3.4

[Qemu-devel] [RFC 1/2] target-arm: Prepare support for Cortex-R4

[Andreas Färber]

Glue "cortex-r4" to r1p4, the latest available TRM.

Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 target-arm/cpu.h    |    1 +
 target-arm/helper.c |   14 ++++++++++++++
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 6ab780d..f0a40b0 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -425,6 +425,7 @@ void cpu_arm_set_cp_io(CPUARMState *env, int cpnum,
 #define ARM_CPUID_ARM11MPCORE 0x410fb022
 #define ARM_CPUID_CORTEXA8    0x410fc080
 #define ARM_CPUID_CORTEXA9    0x410fc090
+#define ARM_CPUID_CORTEXR4_R1P4 0x411FC144
 #define ARM_CPUID_CORTEXM3    0x410fc231
 #define ARM_CPUID_ANY         0xffffffff
 
diff --git a/target-arm/helper.c b/target-arm/helper.c
index e2428eb..21be805 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -41,6 +41,12 @@ static uint32_t arm1176_cp15_c0_c1[8] =
 static uint32_t arm1176_cp15_c0_c2[8] =
 { 0x0140011, 0x12002111, 0x11231121, 0x01102131, 0x01141, 0, 0, 0 };
 
+static uint32_t cortexr4_cp15_c0_c1[8] =
+{ 0x0131, 0x001, 0x010400, 0x0, 0x0210030, 0x00000000, 0x01200000, 0x0211 };
+
+static uint32_t cortexr4_cp15_c0_c2[8] =
+{ 0x1101111, 0x13112111, 0x21232131, 0x01112131, 0x0010142, 0x0, 0, 0 };
+
 static uint32_t cpu_arm_find_by_name(const char *name);
 
 static inline void set_feature(CPUARMState *env, int feature)
@@ -186,6 +192,13 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
         env->cp15.c0_ccsid[1] = 0x200fe015; /* 16k L1 icache. */
         env->cp15.c1_sys = 0x00c50078;
         break;
+    case ARM_CPUID_CORTEXR4_R1P4:
+        /* TODO other features */
+        set_feature(env, ARM_FEATURE_THUMB2);
+        set_feature(env, ARM_FEATURE_V7);
+        memcpy(env->cp15.c0_c1, cortexr4_cp15_c0_c1, 8 * sizeof(uint32_t));
+        memcpy(env->cp15.c0_c2, cortexr4_cp15_c0_c2, 8 * sizeof(uint32_t));
+        break;
     case ARM_CPUID_CORTEXM3:
         set_feature(env, ARM_FEATURE_V4T);
         set_feature(env, ARM_FEATURE_V5);
@@ -424,6 +437,7 @@ static const struct arm_cpu_t arm_cpu_names[] = {
     { ARM_CPUID_CORTEXM3, "cortex-m3"},
     { ARM_CPUID_CORTEXA8, "cortex-a8"},
     { ARM_CPUID_CORTEXA9, "cortex-a9"},
+    { ARM_CPUID_CORTEXR4_R1P4, "cortex-r4"},
     { ARM_CPUID_TI925T, "ti925t" },
     { ARM_CPUID_PXA250, "pxa250" },
     { ARM_CPUID_SA1100,    "sa1100" },
-- 
1.7.3.4

[Qemu-devel] [RFC 2/2] target-arm: Add support for Cortex-R4F

[Andreas Färber]

All CPU-dependent initializations are currently done based on MIDR.
Cortex-R4F shares the MIDR with Cortex-R4 though. Therefore consider the
CPU model string, too (which is not cleared on reset).

Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 target-arm/helper.c |   35 +++++++++++++++++++++++++++++++++++
 1 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 21be805..2273492 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -196,6 +196,40 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
         /* TODO other features */
         set_feature(env, ARM_FEATURE_THUMB2);
         set_feature(env, ARM_FEATURE_V7);
+        if (strcmp(env->cpu_model_str, "cortex-r4f") == 0) {
+            uint8_t r = (id >> 20) & 0xf;
+            uint8_t p = id & 0xf;
+            uint8_t rev = 0;
+            set_feature(env, ARM_FEATURE_VFP);
+            set_feature(env, ARM_FEATURE_VFP3);
+            /* TODO VFPv3-D16 */
+            /* Calculate FPSID value matching to MIDR */
+            if (r == 1) {
+                switch (p) {
+                case 0:
+                    rev = 0x3;
+                    break;
+                case 1:
+                    rev = 0x4;
+                    break;
+                case 2:
+                    rev = 0x6;
+                    break;
+                case 3:
+                    rev = 0x7;
+                    break;
+                case 4:
+                    rev = 0x8;
+                    break;
+                }
+            }
+            if (rev == 0) {
+                cpu_abort(env,
+                          "Cortex-R4F r%" PRIu8 "p%" PRIu8 " unsupported",
+                          r, p);
+            }
+            env->vfp.xregs[ARM_VFP_FPSID] = 0x41023140 | (rev & 0xf);
+        }
         memcpy(env->cp15.c0_c1, cortexr4_cp15_c0_c1, 8 * sizeof(uint32_t));
         memcpy(env->cp15.c0_c2, cortexr4_cp15_c0_c2, 8 * sizeof(uint32_t));
         break;
@@ -438,6 +472,7 @@ static const struct arm_cpu_t arm_cpu_names[] = {
     { ARM_CPUID_CORTEXA8, "cortex-a8"},
     { ARM_CPUID_CORTEXA9, "cortex-a9"},
     { ARM_CPUID_CORTEXR4_R1P4, "cortex-r4"},
+    { ARM_CPUID_CORTEXR4_R1P4, "cortex-r4f"},
     { ARM_CPUID_TI925T, "ti925t" },
     { ARM_CPUID_PXA250, "pxa250" },
     { ARM_CPUID_SA1100,    "sa1100" },
-- 
1.7.3.4

Re: [Qemu-devel] [RFC 0/2] target-arm: Adding Cortex-R4F support

[Peter Maydell]

On 2 October 2011 19:56, Andreas Färber <andreas.faerber@web.de> wrote:
> I've been looking into adding support for Cortex-R4F.

Ooh, that will be the first R profile core. In particular the only
other non-M-profile PMSA core we support is the 946 which was a v5
core, so you'll need to check that we actually implement v7 PMSA.
(The M3 support means we probably have the functionality, but not
the cp15 interface for obvious reasons.)

> 1) Currently, -cpu is used to look up a Main ID Register value and to base
> feature decisions on that. This doesn't work for Cortex-R4 and Cortex-R4F,
> which have an identical MIDR but only -R4F has the FPU.
> Re-checking the model string, while ugly, does the trick. Comments?

That is indeed kind of ugly. I think if CPUID value isn't a unique value
for the things we pass to -cpu then we shouldn't treat it as one. More
generally, it would be nice to be able to say "I want a Cortex-A9
but I only want the no-neon VFPv3D16 variant". (I think some of the
other targets already have syntax for this.)

Currently the approach is to say "you only get one variant of the
processor, and it's the one with all the bells and whistles enabled".
That would imply that '-cpu cortex-r4' gives you one with an FPU.
If we're going to move away from that model we should do it for
all cores.

> 2) The R4/R4F TRM says "It implements the ARMv7R architecture, and includes
> Thumb-2 technology" - how to incur the pre-v7 feature bits in addition to
> the easy _V7 and _THUMB2? I.e. where is it documented (in ARMv7-A/R TRM?)
> whether or not this should include _V4T, _V6K, etc.?
> If V7 were to always imply the same set of features, I would expect it
> alongside VAPA. (Could use some comments either way.)

The feature switches in qemu generally line up with architectural
features, but the canonical reference for what they do is qemu's
source code, not the TRM. That said, the reason why V7 doesn't
auto-imply V4/V5/V6 is purely historical -- we should clean the
code up so that it sets these features like that rather than having
all the CPUs set the feature flags individually.

R4F is going to want V4T, V5, V6, V7, THUMB2, MPU, VFP3, VFP_FP16
and DIV [although since R profile has UDIV and SDIV only in Thumb
mode and I've just sent patches that make the DIV feature enable
it for ARM mode too we'll need to sort that out].

> 3) How to handle processor revisions? The only two available TRMs for R4F
> seem to be r1p3 and r1p4. ARM_CPUID_CORTEXA9 seems to use r0p0, with r2p0
> being the oldest available TRM atm.
> Apart from the actual MIDR define and its binding to -cpu name, this also
> affects the FPSID register and gets a little ugly with non-linear mappings.
> The ARM1136 seems a particularly bad example, will try to post a cleanup.

I think that (1) the bare CPU name should be the most recent rev of the
core that QEMU knows about [and that we should be happy to change qemu
to move up to supporting newer revisions]
(2) we should really resist implementing (or retaining the implementation
of) older revisions of a core because mostly they're not really very
interesting
(3) where we do support more than one revision we should do it with
the same 'specify a feature' syntax rather than with different core
cpu names.

(Anybody want to argue with (1) ?)

1136 is particularly awful because the qemu cpu names are so misleading:
"arm1136" is an r1p0 which is actually later than the "arm1136_r2" which
is an r0p2. And we have way too many revs of the PXA270 IMHO. I guess
we're stuck with them for command line compatibility, though :-(

-- PMM

Re: [Qemu-devel] [RFC 0/2] target-arm: Adding Cortex-R4F support

[Peter Maydell]

On 2 October 2011 22:44, Peter Maydell <peter.maydell@linaro.org> wrote:
> R4F is going to want V4T, V5, V6, V7, THUMB2, MPU, VFP3, VFP_FP16
> and DIV [although since R profile has UDIV and SDIV only in Thumb
> mode and I've just sent patches that make the DIV feature enable
> it for ARM mode too we'll need to sort that out].

Sorry, that's not quite right. VFP_FP16 is the half-precision extension,
and R4 does not have it. (I was getting confused with VFPv3-D16, which
we don't currently have a feature switch for. We probably need one.)

-- PMM

Re: [Qemu-devel] [RFC 0/2] target-arm: Adding Cortex-R4F support

[Andreas Färber]

Am 02.10.2011 23:44, schrieb Peter Maydell:
> On 2 October 2011 19:56, Andreas Färber <andreas.faerber@web.de> wrote:
>> I've been looking into adding support for Cortex-R4F.
> 
> Ooh, that will be the first R profile core. In particular the only
> other non-M-profile PMSA core we support is the 946 which was a v5
> core,

Yeah, I rarely pick the easy tasks. :)

>> 1) Currently, -cpu is used to look up a Main ID Register value and to base
>> feature decisions on that. This doesn't work for Cortex-R4 and Cortex-R4F,
>> which have an identical MIDR but only -R4F has the FPU.
>> Re-checking the model string, while ugly, does the trick. Comments?
> 
> That is indeed kind of ugly. I think if CPUID value isn't a unique value
> for the things we pass to -cpu then we shouldn't treat it as one.

For the reset, the MIDR is read, then the memset() is performed and
cpu_reset_model_id() is called with the previously read MIDR value,
which the function then writes into the register first thing. I'd
suggest to move that out into cpu_reset(), drop the id parameter and
switch on the register instead (only other use is cpu_abort()).

> More
> generally, it would be nice to be able to say "I want a Cortex-A9
> but I only want the no-neon VFPv3D16 variant". (I think some of the
> other targets already have syntax for this.)

Coming from a ppc background, we have a whole matrix of processors with
fixed features but I'm not aware of an arch where we opt-in/out
processor core features.

> Currently the approach is to say "you only get one variant of the
> processor, and it's the one with all the bells and whistles enabled".
> That would imply that '-cpu cortex-r4' gives you one with an FPU.

I'll go with cortex-r4f then.

> I think that (1) the bare CPU name should be the most recent rev of the
> core that QEMU knows about [and that we should be happy to change qemu
> to move up to supporting newer revisions]

> (Anybody want to argue with (1) ?)

I concur that an easy-to-type -cpu should provide the latest and
greatest features. Features hidden will not get much exposure. But if a
revision noticeably changes behavior, I guess we should remain command
line compatible.

Andreas

Re: [Qemu-devel] [RFC 0/2] target-arm: Adding Cortex-R4F support

[Peter Maydell]

On 6 October 2011 11:16, Andreas Färber <andreas.faerber@web.de> wrote:
> Am 02.10.2011 23:44, schrieb Peter Maydell:
>> On 2 October 2011 19:56, Andreas Färber <andreas.faerber@web.de> wrote:
>>> 1) Currently, -cpu is used to look up a Main ID Register value and to base
>>> feature decisions on that. This doesn't work for Cortex-R4 and Cortex-R4F,
>>> which have an identical MIDR but only -R4F has the FPU.
>>> Re-checking the model string, while ugly, does the trick. Comments?
>>
>> That is indeed kind of ugly. I think if CPUID value isn't a unique value
>> for the things we pass to -cpu then we shouldn't treat it as one.
>
> For the reset, the MIDR is read, then the memset() is performed and
> cpu_reset_model_id() is called with the previously read MIDR value,
> which the function then writes into the register first thing. I'd
> suggest to move that out into cpu_reset(), drop the id parameter and
> switch on the register instead (only other use is cpu_abort()).

If we're shuffling code around we should probably be doing something like:
 * in cpu_arm_init() look at the model string and set feature switches,
   ID register values, etc
 * in reset, don't reset ID registers (they're constant, after all),
   and [as with the rest of the code] behave based only on cpuid and
   feature switches

>> More
>> generally, it would be nice to be able to say "I want a Cortex-A9
>> but I only want the no-neon VFPv3D16 variant". (I think some of the
>> other targets already have syntax for this.)
>
> Coming from a ppc background, we have a whole matrix of processors with
> fixed features but I'm not aware of an arch where we opt-in/out
> processor core features.

target-i386 seems to have some code for handling syntax like this
(you seem to be able to say -cpu pentium,-fpu for instance).

>> I think that (1) the bare CPU name should be the most recent rev of the
>> core that QEMU knows about [and that we should be happy to change qemu
>> to move up to supporting newer revisions]
>
>> (Anybody want to argue with (1) ?)
>
> I concur that an easy-to-type -cpu should provide the latest and
> greatest features. Features hidden will not get much exposure. But if a
> revision noticeably changes behavior, I guess we should remain command
> line compatible.

Depends what you mean by "noticeably". User space will basically never
notice or care, typically. The kernel does care occasionally. I think
I'd rather have "cortex-foo" do the right thing for the vast majority
of users who don't care whether they get r2 or r3, rather than be stuck
with it meaning r1 because that's what we happened to model first and
there was some minor incompatible change between r1 and r2. I don't think
there's a position between "cortex-foo is always the most recent rev
we model" and "user must always specify rXpY" which doesn't lead you
into weird and confusing UI inconsistencies between CPUs.

-- PMM

[Qemu-devel] [RFC] target-arm: Preserve CPUID over CPU reset

[Andreas Färber]

Previously the CPUID register was set in cpu_arm_init() based on -cpu
model. The CPU was then reset, requiring to save the CPUID and restore
it afterwards.

Change the storage location of c0_cpuid so that it does not get cleared.

OMAP appears to be special in that the CPUID can be switched between
TI915T and TI925T on write. Therefore preserve cpu_reset_model_id() in
slightly simplified form, allowing OMAP to reset the CPUID to TI925.

Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Paul Brook <paul@codesourcery.com>
Cc: Andrzej Zaborowski <balrogg@gmail.com>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 hw/armv7m_nvic.c     |    2 +-
 target-arm/cpu.h     |    4 ++--
 target-arm/helper.c  |   24 ++++++++++--------------
 target-arm/machine.c |    4 ++--
 4 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/hw/armv7m_nvic.c b/hw/armv7m_nvic.c
index bf8c3c5..e029537 100644
--- a/hw/armv7m_nvic.c
+++ b/hw/armv7m_nvic.c
@@ -151,7 +151,7 @@ static uint32_t nvic_readl(void *opaque, uint32_t offset)
     case 0x1c: /* SysTick Calibration Value.  */
         return 10000;
     case 0xd00: /* CPUID Base.  */
-        return cpu_single_env->cp15.c0_cpuid;
+        return ARM_CPUID(cpu_single_env);
     case 0xd04: /* Interrypt Control State.  */
         /* VECTACTIVE */
         val = s->gic.running_irq[0];
diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 6ab780d..1623a2a 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -106,7 +106,6 @@ typedef struct CPUARMState {
 
     /* System control coprocessor (cp15) */
     struct {
-        uint32_t c0_cpuid;
         uint32_t c0_cachetype;
         uint32_t c0_ccsid[16]; /* Cache size.  */
         uint32_t c0_clid; /* Cache level.  */
@@ -228,6 +227,7 @@ typedef struct CPUARMState {
     } cp[15];
     void *nvic;
     const struct arm_boot_info *boot_info;
+    uint32_t cp15_c0_cpuid;
 } CPUARMState;
 
 CPUARMState *cpu_arm_init(const char *cpu_model);
@@ -398,7 +398,7 @@ void cpu_arm_set_cp_io(CPUARMState *env, int cpnum,
    conventional cores (ie. Application or Realtime profile).  */
 
 #define IS_M(env) arm_feature(env, ARM_FEATURE_M)
-#define ARM_CPUID(env) (env->cp15.c0_cpuid)
+#define ARM_CPUID(env) (env->cp15_c0_cpuid)
 
 #define ARM_CPUID_ARM1026     0x4106a262
 #define ARM_CPUID_ARM926      0x41069265
diff --git a/target-arm/helper.c b/target-arm/helper.c
index e2428eb..74e423d 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -48,10 +48,9 @@ static inline void set_feature(CPUARMState *env, int feature)
     env->features |= 1u << feature;
 }
 
-static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
+static void cpu_reset_model_id(CPUARMState *env)
 {
-    env->cp15.c0_cpuid = id;
-    switch (id) {
+    switch (env->cp15_c0_cpuid) {
     case ARM_CPUID_ARM926:
         set_feature(env, ARM_FEATURE_V4T);
         set_feature(env, ARM_FEATURE_V5);
@@ -214,7 +213,7 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
     case ARM_CPUID_TI925T:
         set_feature(env, ARM_FEATURE_V4T);
         set_feature(env, ARM_FEATURE_OMAPCP);
-        env->cp15.c0_cpuid = ARM_CPUID_TI925T; /* Depends on wiring.  */
+        env->cp15_c0_cpuid = ARM_CPUID_TI925T; /* Depends on wiring.  */
         env->cp15.c0_cachetype = 0x5109149;
         env->cp15.c1_sys = 0x00000070;
         env->cp15.c15_i_max = 0x000;
@@ -253,7 +252,7 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
         env->cp15.c1_sys = 0x00000070;
         break;
     default:
-        cpu_abort(env, "Bad CPU ID: %x\n", id);
+        cpu_abort(env, "Bad CPU ID: %x\n", env->cp15_c0_cpuid);
         break;
     }
 
@@ -265,17 +264,14 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
 
 void cpu_reset(CPUARMState *env)
 {
-    uint32_t id;
-
     if (qemu_loglevel_mask(CPU_LOG_RESET)) {
         qemu_log("CPU Reset (CPU %d)\n", env->cpu_index);
         log_cpu_state(env, 0);
     }
 
-    id = env->cp15.c0_cpuid;
     memset(env, 0, offsetof(CPUARMState, breakpoints));
-    if (id)
-        cpu_reset_model_id(env, id);
+    if (env->cp15_c0_cpuid)
+        cpu_reset_model_id(env);
 #if defined (CONFIG_USER_ONLY)
     env->uncached_cpsr = ARM_CPU_MODE_USR;
     /* For user mode we must enable access to coprocessors */
@@ -311,7 +307,7 @@ void cpu_reset(CPUARMState *env)
     /* v7 performance monitor control register: same implementor
      * field as main ID register, and we implement no event counters.
      */
-    env->cp15.c9_pmcr = (id & 0xff000000);
+    env->cp15.c9_pmcr = (env->cp15_c0_cpuid & 0xff000000);
 #endif
     set_flush_to_zero(1, &env->vfp.standard_fp_status);
     set_flush_inputs_to_zero(1, &env->vfp.standard_fp_status);
@@ -392,7 +388,7 @@ CPUARMState *cpu_arm_init(const char *cpu_model)
     }
 
     env->cpu_model_str = cpu_model;
-    env->cp15.c0_cpuid = id;
+    env->cp15_c0_cpuid = id;
     cpu_reset(env);
     if (arm_feature(env, ARM_FEATURE_NEON)) {
         gdb_register_coprocessor(env, vfp_gdb_get_reg, vfp_gdb_set_reg,
@@ -1756,7 +1752,7 @@ void HELPER(set_cp15)(CPUState *env, uint32_t insn, uint32_t val)
                 break;
             case 1: /* Set TI925T configuration.  */
                 env->cp15.c15_ticonfig = val & 0xe7;
-                env->cp15.c0_cpuid = (val & (1 << 5)) ? /* OS_TYPE bit */
+                env->cp15_c0_cpuid = (val & (1 << 5)) ? /* OS_TYPE bit */
                         ARM_CPUID_TI915T : ARM_CPUID_TI925T;
                 break;
             case 2: /* Set I_max.  */
@@ -1801,7 +1797,7 @@ uint32_t HELPER(get_cp15)(CPUState *env, uint32_t insn)
             case 0:
                 switch (op2) {
                 case 0: /* Device ID.  */
-                    return env->cp15.c0_cpuid;
+                    return env->cp15_c0_cpuid;
                 case 1: /* Cache Type.  */
 		    return env->cp15.c0_cachetype;
                 case 2: /* TCM status.  */
diff --git a/target-arm/machine.c b/target-arm/machine.c
index 7d4fc54..ed457a5 100644
--- a/target-arm/machine.c
+++ b/target-arm/machine.c
@@ -20,7 +20,7 @@ void cpu_save(QEMUFile *f, void *opaque)
         qemu_put_be32(f, env->usr_regs[i]);
         qemu_put_be32(f, env->fiq_regs[i]);
     }
-    qemu_put_be32(f, env->cp15.c0_cpuid);
+    qemu_put_be32(f, env->cp15_c0_cpuid);
     qemu_put_be32(f, env->cp15.c0_cachetype);
     qemu_put_be32(f, env->cp15.c0_cssel);
     qemu_put_be32(f, env->cp15.c1_sys);
@@ -134,7 +134,7 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
         env->usr_regs[i] = qemu_get_be32(f);
         env->fiq_regs[i] = qemu_get_be32(f);
     }
-    env->cp15.c0_cpuid = qemu_get_be32(f);
+    env->cp15_c0_cpuid = qemu_get_be32(f);
     env->cp15.c0_cachetype = qemu_get_be32(f);
     env->cp15.c0_cssel = qemu_get_be32(f);
     env->cp15.c1_sys = qemu_get_be32(f);
-- 
1.7.7

[Qemu-devel] [RFC post-1.0 0/5] Inference of ARM features

[Andreas Färber]

Hello Peter,

Here's a rebased version of my ARM feature inference series. It's based on
my CPUID preservation patch, which hasn't received any comments yet.

As demonstrated by the two trailing patches, this simplifies adding a new
cpu model significantly.

These inference rules are based on your comments for Cortex-R4F and by
analyzing the existing CPUs, so be careful to check that for whether there
are any exceptions to the rule.

Regards,

Andreas


Andreas Färber (5):
  target-arm: Infer ARMv4T feature
  target-arm: Infer ARMv5 feature
  target-arm: Infer ARMv6 feature
  target-arm: Prepare support for Cortex-R4
  target-arm: Add support for Cortex-R4F

 target-arm/cpu.h    |    1 +
 target-arm/helper.c |   79 ++++++++++++++++++++++++++++++++++++---------------
 2 files changed, 57 insertions(+), 23 deletions(-)

-- 
1.7.7

[Qemu-devel] [RFC 1/5] target-arm: Infer ARMv4T feature

[Andreas Färber]

ARMv5 => ARMv4T

Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 target-arm/helper.c |   15 +++------------
 1 files changed, 3 insertions(+), 12 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 3daf2f2..0d7635b 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -52,7 +52,6 @@ static void cpu_reset_model_id(CPUARMState *env)
 {
     switch (env->cp15_c0_cpuid) {
     case ARM_CPUID_ARM926:
-        set_feature(env, ARM_FEATURE_V4T);
         set_feature(env, ARM_FEATURE_V5);
         set_feature(env, ARM_FEATURE_VFP);
         env->vfp.xregs[ARM_VFP_FPSID] = 0x41011090;
@@ -60,14 +59,12 @@ static void cpu_reset_model_id(CPUARMState *env)
         env->cp15.c1_sys = 0x00090078;
         break;
     case ARM_CPUID_ARM946:
-        set_feature(env, ARM_FEATURE_V4T);
         set_feature(env, ARM_FEATURE_V5);
         set_feature(env, ARM_FEATURE_MPU);
         env->cp15.c0_cachetype = 0x0f004006;
         env->cp15.c1_sys = 0x00000078;
         break;
     case ARM_CPUID_ARM1026:
-        set_feature(env, ARM_FEATURE_V4T);
         set_feature(env, ARM_FEATURE_V5);
         set_feature(env, ARM_FEATURE_VFP);
         set_feature(env, ARM_FEATURE_AUXCR);
@@ -84,7 +81,6 @@ static void cpu_reset_model_id(CPUARMState *env)
          * older core than plain "arm1136". In particular this does not
          * have the v6K features.
          */
-        set_feature(env, ARM_FEATURE_V4T);
         set_feature(env, ARM_FEATURE_V5);
         set_feature(env, ARM_FEATURE_V6);
         set_feature(env, ARM_FEATURE_VFP);
@@ -102,7 +98,6 @@ static void cpu_reset_model_id(CPUARMState *env)
         env->cp15.c1_sys = 0x00050078;
         break;
     case ARM_CPUID_ARM1176:
-        set_feature(env, ARM_FEATURE_V4T);
         set_feature(env, ARM_FEATURE_V5);
         set_feature(env, ARM_FEATURE_V6);
         set_feature(env, ARM_FEATURE_V6K);
@@ -118,7 +113,6 @@ static void cpu_reset_model_id(CPUARMState *env)
         env->cp15.c1_sys = 0x00050078;
         break;
     case ARM_CPUID_ARM11MPCORE:
-        set_feature(env, ARM_FEATURE_V4T);
         set_feature(env, ARM_FEATURE_V5);
         set_feature(env, ARM_FEATURE_V6);
         set_feature(env, ARM_FEATURE_V6K);
@@ -133,7 +127,6 @@ static void cpu_reset_model_id(CPUARMState *env)
         env->cp15.c0_cachetype = 0x1dd20d2;
         break;
     case ARM_CPUID_CORTEXA8:
-        set_feature(env, ARM_FEATURE_V4T);
         set_feature(env, ARM_FEATURE_V5);
         set_feature(env, ARM_FEATURE_V6);
         set_feature(env, ARM_FEATURE_V6K);
@@ -157,7 +150,6 @@ static void cpu_reset_model_id(CPUARMState *env)
         env->cp15.c1_sys = 0x00c50078;
         break;
     case ARM_CPUID_CORTEXA9:
-        set_feature(env, ARM_FEATURE_V4T);
         set_feature(env, ARM_FEATURE_V5);
         set_feature(env, ARM_FEATURE_V6);
         set_feature(env, ARM_FEATURE_V6K);
@@ -186,7 +178,6 @@ static void cpu_reset_model_id(CPUARMState *env)
         env->cp15.c1_sys = 0x00c50078;
         break;
     case ARM_CPUID_CORTEXM3:
-        set_feature(env, ARM_FEATURE_V4T);
         set_feature(env, ARM_FEATURE_V5);
         set_feature(env, ARM_FEATURE_V6);
         set_feature(env, ARM_FEATURE_THUMB2);
@@ -195,7 +186,6 @@ static void cpu_reset_model_id(CPUARMState *env)
         set_feature(env, ARM_FEATURE_THUMB_DIV);
         break;
     case ARM_CPUID_ANY: /* For userspace emulation.  */
-        set_feature(env, ARM_FEATURE_V4T);
         set_feature(env, ARM_FEATURE_V5);
         set_feature(env, ARM_FEATURE_V6);
         set_feature(env, ARM_FEATURE_V6K);
@@ -225,7 +215,6 @@ static void cpu_reset_model_id(CPUARMState *env)
     case ARM_CPUID_PXA260:
     case ARM_CPUID_PXA261:
     case ARM_CPUID_PXA262:
-        set_feature(env, ARM_FEATURE_V4T);
         set_feature(env, ARM_FEATURE_V5);
         set_feature(env, ARM_FEATURE_XSCALE);
         /* JTAG_ID is ((id << 28) | 0x09265013) */
@@ -238,7 +227,6 @@ static void cpu_reset_model_id(CPUARMState *env)
     case ARM_CPUID_PXA270_B1:
     case ARM_CPUID_PXA270_C0:
     case ARM_CPUID_PXA270_C5:
-        set_feature(env, ARM_FEATURE_V4T);
         set_feature(env, ARM_FEATURE_V5);
         set_feature(env, ARM_FEATURE_XSCALE);
         /* JTAG_ID is ((id << 28) | 0x09265013) */
@@ -261,6 +249,9 @@ static void cpu_reset_model_id(CPUARMState *env)
     if (arm_feature(env, ARM_FEATURE_V7)) {
         set_feature(env, ARM_FEATURE_VAPA);
     }
+    if (arm_feature(env, ARM_FEATURE_V5)) {
+        set_feature(env, ARM_FEATURE_V4T);
+    }
     if (arm_feature(env, ARM_FEATURE_ARM_DIV)) {
         set_feature(env, ARM_FEATURE_THUMB_DIV);
     }
-- 
1.7.7

[Qemu-devel] [RFC 2/5] target-arm: Infer ARMv5 feature

[Andreas Färber]

ARMv6 => ARMv5

Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 target-arm/helper.c |   10 +++-------
 1 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 0d7635b..bd6173d 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -81,7 +81,6 @@ static void cpu_reset_model_id(CPUARMState *env)
          * older core than plain "arm1136". In particular this does not
          * have the v6K features.
          */
-        set_feature(env, ARM_FEATURE_V5);
         set_feature(env, ARM_FEATURE_V6);
         set_feature(env, ARM_FEATURE_VFP);
         set_feature(env, ARM_FEATURE_AUXCR);
@@ -98,7 +97,6 @@ static void cpu_reset_model_id(CPUARMState *env)
         env->cp15.c1_sys = 0x00050078;
         break;
     case ARM_CPUID_ARM1176:
-        set_feature(env, ARM_FEATURE_V5);
         set_feature(env, ARM_FEATURE_V6);
         set_feature(env, ARM_FEATURE_V6K);
         set_feature(env, ARM_FEATURE_VFP);
@@ -113,7 +111,6 @@ static void cpu_reset_model_id(CPUARMState *env)
         env->cp15.c1_sys = 0x00050078;
         break;
     case ARM_CPUID_ARM11MPCORE:
-        set_feature(env, ARM_FEATURE_V5);
         set_feature(env, ARM_FEATURE_V6);
         set_feature(env, ARM_FEATURE_V6K);
         set_feature(env, ARM_FEATURE_VFP);
@@ -127,7 +124,6 @@ static void cpu_reset_model_id(CPUARMState *env)
         env->cp15.c0_cachetype = 0x1dd20d2;
         break;
     case ARM_CPUID_CORTEXA8:
-        set_feature(env, ARM_FEATURE_V5);
         set_feature(env, ARM_FEATURE_V6);
         set_feature(env, ARM_FEATURE_V6K);
         set_feature(env, ARM_FEATURE_V7);
@@ -150,7 +146,6 @@ static void cpu_reset_model_id(CPUARMState *env)
         env->cp15.c1_sys = 0x00c50078;
         break;
     case ARM_CPUID_CORTEXA9:
-        set_feature(env, ARM_FEATURE_V5);
         set_feature(env, ARM_FEATURE_V6);
         set_feature(env, ARM_FEATURE_V6K);
         set_feature(env, ARM_FEATURE_V7);
@@ -178,7 +173,6 @@ static void cpu_reset_model_id(CPUARMState *env)
         env->cp15.c1_sys = 0x00c50078;
         break;
     case ARM_CPUID_CORTEXM3:
-        set_feature(env, ARM_FEATURE_V5);
         set_feature(env, ARM_FEATURE_V6);
         set_feature(env, ARM_FEATURE_THUMB2);
         set_feature(env, ARM_FEATURE_V7);
@@ -186,7 +180,6 @@ static void cpu_reset_model_id(CPUARMState *env)
         set_feature(env, ARM_FEATURE_THUMB_DIV);
         break;
     case ARM_CPUID_ANY: /* For userspace emulation.  */
-        set_feature(env, ARM_FEATURE_V5);
         set_feature(env, ARM_FEATURE_V6);
         set_feature(env, ARM_FEATURE_V6K);
         set_feature(env, ARM_FEATURE_V7);
@@ -249,6 +242,9 @@ static void cpu_reset_model_id(CPUARMState *env)
     if (arm_feature(env, ARM_FEATURE_V7)) {
         set_feature(env, ARM_FEATURE_VAPA);
     }
+    if (arm_feature(env, ARM_FEATURE_V6)) {
+        set_feature(env, ARM_FEATURE_V5);
+    }
     if (arm_feature(env, ARM_FEATURE_V5)) {
         set_feature(env, ARM_FEATURE_V4T);
     }
-- 
1.7.7

[Qemu-devel] [RFC 3/5] target-arm: Infer ARMv6 feature

[Andreas Färber]

ARMv7 => ARMv6

Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 target-arm/helper.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index bd6173d..3de0781 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -124,7 +124,6 @@ static void cpu_reset_model_id(CPUARMState *env)
         env->cp15.c0_cachetype = 0x1dd20d2;
         break;
     case ARM_CPUID_CORTEXA8:
-        set_feature(env, ARM_FEATURE_V6);
         set_feature(env, ARM_FEATURE_V6K);
         set_feature(env, ARM_FEATURE_V7);
         set_feature(env, ARM_FEATURE_AUXCR);
@@ -146,7 +145,6 @@ static void cpu_reset_model_id(CPUARMState *env)
         env->cp15.c1_sys = 0x00c50078;
         break;
     case ARM_CPUID_CORTEXA9:
-        set_feature(env, ARM_FEATURE_V6);
         set_feature(env, ARM_FEATURE_V6K);
         set_feature(env, ARM_FEATURE_V7);
         set_feature(env, ARM_FEATURE_AUXCR);
@@ -173,14 +171,12 @@ static void cpu_reset_model_id(CPUARMState *env)
         env->cp15.c1_sys = 0x00c50078;
         break;
     case ARM_CPUID_CORTEXM3:
-        set_feature(env, ARM_FEATURE_V6);
         set_feature(env, ARM_FEATURE_THUMB2);
         set_feature(env, ARM_FEATURE_V7);
         set_feature(env, ARM_FEATURE_M);
         set_feature(env, ARM_FEATURE_THUMB_DIV);
         break;
     case ARM_CPUID_ANY: /* For userspace emulation.  */
-        set_feature(env, ARM_FEATURE_V6);
         set_feature(env, ARM_FEATURE_V6K);
         set_feature(env, ARM_FEATURE_V7);
         set_feature(env, ARM_FEATURE_THUMB2);
@@ -241,6 +237,7 @@ static void cpu_reset_model_id(CPUARMState *env)
     /* Some features automatically imply others: */
     if (arm_feature(env, ARM_FEATURE_V7)) {
         set_feature(env, ARM_FEATURE_VAPA);
+        set_feature(env, ARM_FEATURE_V6);
     }
     if (arm_feature(env, ARM_FEATURE_V6)) {
         set_feature(env, ARM_FEATURE_V5);
-- 
1.7.7

[Qemu-devel] [FYI 4/5] target-arm: Prepare support for Cortex-R4

[Andreas Färber]

Glue "cortex-r4" to r1p4, the latest available TRM.
Set MPU and Thumb division feature bit.

Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 target-arm/cpu.h    |    1 +
 target-arm/helper.c |   15 +++++++++++++++
 2 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index c38dfd5..44868e6 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -427,6 +427,7 @@ void cpu_arm_set_cp_io(CPUARMState *env, int cpnum,
 #define ARM_CPUID_ARM11MPCORE 0x410fb022
 #define ARM_CPUID_CORTEXA8    0x410fc080
 #define ARM_CPUID_CORTEXA9    0x410fc090
+#define ARM_CPUID_CORTEXR4_R1P4 0x411FC144
 #define ARM_CPUID_CORTEXM3    0x410fc231
 #define ARM_CPUID_ANY         0xffffffff
 
diff --git a/target-arm/helper.c b/target-arm/helper.c
index 3de0781..4836762 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -41,6 +41,12 @@ static uint32_t arm1176_cp15_c0_c1[8] =
 static uint32_t arm1176_cp15_c0_c2[8] =
 { 0x0140011, 0x12002111, 0x11231121, 0x01102131, 0x01141, 0, 0, 0 };
 
+static uint32_t cortexr4_cp15_c0_c1[8] =
+{ 0x0131, 0x001, 0x010400, 0x0, 0x0210030, 0x00000000, 0x01200000, 0x0211 };
+
+static uint32_t cortexr4_cp15_c0_c2[8] =
+{ 0x1101111, 0x13112111, 0x21232131, 0x01112131, 0x0010142, 0x0, 0, 0 };
+
 static uint32_t cpu_arm_find_by_name(const char *name);
 
 static inline void set_feature(CPUARMState *env, int feature)
@@ -170,6 +176,14 @@ static void cpu_reset_model_id(CPUARMState *env)
         env->cp15.c0_ccsid[1] = 0x200fe015; /* 16k L1 icache. */
         env->cp15.c1_sys = 0x00c50078;
         break;
+    case ARM_CPUID_CORTEXR4_R1P4:
+        set_feature(env, ARM_FEATURE_V7);
+        set_feature(env, ARM_FEATURE_THUMB2);
+        set_feature(env, ARM_FEATURE_THUMB_DIV);
+        set_feature(env, ARM_FEATURE_MPU);
+        memcpy(env->cp15.c0_c1, cortexr4_cp15_c0_c1, 8 * sizeof(uint32_t));
+        memcpy(env->cp15.c0_c2, cortexr4_cp15_c0_c2, 8 * sizeof(uint32_t));
+        break;
     case ARM_CPUID_CORTEXM3:
         set_feature(env, ARM_FEATURE_THUMB2);
         set_feature(env, ARM_FEATURE_V7);
@@ -408,6 +422,7 @@ static const struct arm_cpu_t arm_cpu_names[] = {
     { ARM_CPUID_CORTEXM3, "cortex-m3"},
     { ARM_CPUID_CORTEXA8, "cortex-a8"},
     { ARM_CPUID_CORTEXA9, "cortex-a9"},
+    { ARM_CPUID_CORTEXR4_R1P4, "cortex-r4"},
     { ARM_CPUID_TI925T, "ti925t" },
     { ARM_CPUID_PXA250, "pxa250" },
     { ARM_CPUID_SA1100,    "sa1100" },
-- 
1.7.7

[Qemu-devel] [FYI 5/5] target-arm: Add support for Cortex-R4F

[Andreas Färber]

Since no clean distinction between R4 and R4F can be made yet,
default -cpu cortex-r4 to Cortex-R4F.

Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 target-arm/helper.c |   34 ++++++++++++++++++++++++++++++++++
 1 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/target-arm/helper.c b/target-arm/helper.c
index 4836762..15853e0 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -181,6 +181,40 @@ static void cpu_reset_model_id(CPUARMState *env)
         set_feature(env, ARM_FEATURE_THUMB2);
         set_feature(env, ARM_FEATURE_THUMB_DIV);
         set_feature(env, ARM_FEATURE_MPU);
+        {
+            static const struct {
+                uint8_t r;
+                uint8_t p;
+                uint8_t value;
+            } fpsid_revs[] = {
+                { 1, 0, 0x3 },
+                { 1, 1, 0x4 },
+                { 1, 2, 0x6 },
+                { 1, 3, 0x7 },
+                { 1, 4, 0x8 },
+                {}
+            };
+            uint8_t r = (ARM_CPUID(env) >> 20) & 0xf;
+            uint8_t p = ARM_CPUID(env) & 0xf;
+            uint8_t rev = 0;
+            int i;
+            set_feature(env, ARM_FEATURE_VFP);
+            set_feature(env, ARM_FEATURE_VFP3);
+            /* TODO VFPv3-D16 */
+            /* Calculate FPSID value matching to MIDR */
+            for (i = 0; fpsid_revs[i].r != 0; i++) {
+                if (fpsid_revs[i].r == r && fpsid_revs[i].p == p) {
+                    rev = fpsid_revs[i].value;
+                    break;
+                }
+            }
+            if (rev == 0) {
+                cpu_abort(env,
+                          "Cortex-R4F r%" PRIu8 "p%" PRIu8 " unsupported",
+                          r, p);
+            }
+            env->vfp.xregs[ARM_VFP_FPSID] = 0x41023140 | (rev & 0xf);
+        }
         memcpy(env->cp15.c0_c1, cortexr4_cp15_c0_c1, 8 * sizeof(uint32_t));
         memcpy(env->cp15.c0_c2, cortexr4_cp15_c0_c2, 8 * sizeof(uint32_t));
         break;
-- 
1.7.7

Re: [Qemu-devel] [FYI 5/5] target-arm: Add support for Cortex-R4F

[Peter Maydell]

On 10 November 2011 10:32, Andreas Färber <andreas.faerber@web.de> wrote:
> +        {
> +            static const struct {
> +                uint8_t r;
> +                uint8_t p;
> +                uint8_t value;
> +            } fpsid_revs[] = {
> +                { 1, 0, 0x3 },
> +                { 1, 1, 0x4 },
> +                { 1, 2, 0x6 },
> +                { 1, 3, 0x7 },
> +                { 1, 4, 0x8 },
> +                {}
> +            };
> +            uint8_t r = (ARM_CPUID(env) >> 20) & 0xf;
> +            uint8_t p = ARM_CPUID(env) & 0xf;
> +            uint8_t rev = 0;
> +            int i;
> +            set_feature(env, ARM_FEATURE_VFP);
> +            set_feature(env, ARM_FEATURE_VFP3);
> +            /* TODO VFPv3-D16 */
> +            /* Calculate FPSID value matching to MIDR */
> +            for (i = 0; fpsid_revs[i].r != 0; i++) {
> +                if (fpsid_revs[i].r == r && fpsid_revs[i].p == p) {
> +                    rev = fpsid_revs[i].value;
> +                    break;
> +                }
> +            }
> +            if (rev == 0) {
> +                cpu_abort(env,
> +                          "Cortex-R4F r%" PRIu8 "p%" PRIu8 " unsupported",
> +                          r, p);
> +            }
> +            env->vfp.xregs[ARM_VFP_FPSID] = 0x41023140 | (rev & 0xf);
> +        }

This seems a bit "beyond the call of duty" since we don't try to do
it for any other CPUs. I would just set the FPSID to a fixed value...

-- PMM

Re: [Qemu-devel] [RFC post-1.0 0/5] Inference of ARM features

[Peter Maydell]

On 10 November 2011 10:31, Andreas Färber <andreas.faerber@web.de> wrote:
> Here's a rebased version of my ARM feature inference series. It's based on
> my CPUID preservation patch, which hasn't received any comments yet.

Oops. I'll go back and dig that out of the archive.

> These inference rules are based on your comments for Cortex-R4F and by
> analyzing the existing CPUs, so be careful to check that for whether there
> are any exceptions to the rule.

The inference rules look good. Some others you can do:
  V7 implies V6K  (this turns on V6K for M3 but that's safe because V6K
        only affects code which isn't reachable for M profile cores)
  V6K implies V6
  V7 implies THUMB2
  VFP4 implies VFP3
  VFP3 implies VFP
  M implies THUMB_DIV
  V6 implies AUXCR

-- PMM

Re: [Qemu-devel] [RFC] target-arm: Preserve CPUID over CPU reset

[Peter Maydell]

On 22 October 2011 12:00, Andreas Färber <andreas.faerber@web.de> wrote:
> Previously the CPUID register was set in cpu_arm_init() based on -cpu
> model. The CPU was then reset, requiring to save the CPUID and restore
> it afterwards.
>
> Change the storage location of c0_cpuid so that it does not get cleared.

I think this is the wrong approach. c0_cpuid should not be handled
any differently from any of the other read-only cp15 registers...

> OMAP appears to be special in that the CPUID can be switched between
> TI915T and TI925T on write. Therefore preserve cpu_reset_model_id() in
> slightly simplified form, allowing OMAP to reset the CPUID to TI925.

...and indeed it's not even purely r/o on those cores.

I don't think we should be doing things based on c0_cpuid. At the
moment we use it for:
 * determining whether we should implement the MPIDR (v7 or 11mpcore)
    -- we should define and use a feature bit instead
 * identifying what to return as the ACTLR value
    -- we should handle this like the other implementation-specific
       cp15 registers instead
 * code in cpu_reset_model_id which sets feature bits and various
   default register values
    -- see below

For cp15 registers I think we need to move to something more like
how target-ppc handles SPRs -- a registration routine of some
kind so each implementation can provide the cp15 registers it
requires and we don't get tangled up in huge nested switch statements
or splitting of "is this register present/ro/whatever?" between
translate.c and helper.c. If we do that then obviously "reset value"
is one of the things you feed in when you register the register,
and we only do the registration once from cpu_init, not at reset.

So cpu_arm_init should pick a base model (defining mostly a set
of cp15 regs and feature bits) and the user can also tweak the
feature bits (cf target-i386 +/-foo syntax), and reset no longer
needs to care at all what "cpu model" the user specified.

-- PMM

[Qemu-devel] [PATCH] target-arm: Tidy up ARM1136 CPUID naming

[Andreas Färber]

-cpu arm1136-r2 is commented to in fact be ARM1136 r0p2, whereas
-cpu arm1136 seems to be ARM1136 r1p3 according to the MIDR value.

The CPUID values contain major and minor revision numbers (rnpn) and
are never used with a mask, so are specific to the chosen revision.
Rename the CPUID preprocessor defines to reflect this, but leave the
CPU model names unchanged for command line compatibility.

Cc: Andrzej Zaborowski <andrew@openedhand.com>
Cc: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Andreas Färber <andreas.faerber@web.de>
---
 target-arm/cpu.h    |    4 ++--
 target-arm/helper.c |   12 ++++++------
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/target-arm/cpu.h b/target-arm/cpu.h
index 6ab780d..783989f 100644
--- a/target-arm/cpu.h
+++ b/target-arm/cpu.h
@@ -419,8 +419,8 @@ void cpu_arm_set_cp_io(CPUARMState *env, int cpnum,
 #define ARM_CPUID_PXA270_B1   0x69054113
 #define ARM_CPUID_PXA270_C0   0x69054114
 #define ARM_CPUID_PXA270_C5   0x69054117
-#define ARM_CPUID_ARM1136     0x4117b363
-#define ARM_CPUID_ARM1136_R2  0x4107b362
+#define ARM_CPUID_ARM1136_R1P3 0x4117b363
+#define ARM_CPUID_ARM1136_R0P2 0x4107b362
 #define ARM_CPUID_ARM1176     0x410fb767
 #define ARM_CPUID_ARM11MPCORE 0x410fb022
 #define ARM_CPUID_CORTEXA8    0x410fc080
diff --git a/target-arm/helper.c b/target-arm/helper.c
index e2428eb..0d342ba 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -76,11 +76,11 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
         env->cp15.c0_cachetype = 0x1dd20d2;
         env->cp15.c1_sys = 0x00090078;
         break;
-    case ARM_CPUID_ARM1136:
+    case ARM_CPUID_ARM1136_R1P3:
         /* This is the 1136 r1, which is a v6K core */
         set_feature(env, ARM_FEATURE_V6K);
         /* Fall through */
-    case ARM_CPUID_ARM1136_R2:
+    case ARM_CPUID_ARM1136_R0P2:
         /* What qemu calls "arm1136_r2" is actually the 1136 r0p2, ie an
          * older core than plain "arm1136". In particular this does not
          * have the v6K features.
@@ -417,8 +417,8 @@ static const struct arm_cpu_t arm_cpu_names[] = {
     { ARM_CPUID_ARM926, "arm926"},
     { ARM_CPUID_ARM946, "arm946"},
     { ARM_CPUID_ARM1026, "arm1026"},
-    { ARM_CPUID_ARM1136, "arm1136"},
-    { ARM_CPUID_ARM1136_R2, "arm1136-r2"},
+    { ARM_CPUID_ARM1136_R1P3, "arm1136" },
+    { ARM_CPUID_ARM1136_R0P2, "arm1136-r2" },
     { ARM_CPUID_ARM1176, "arm1176"},
     { ARM_CPUID_ARM11MPCORE, "arm11mpcore"},
     { ARM_CPUID_CORTEXM3, "cortex-m3"},
@@ -1886,8 +1886,8 @@ uint32_t HELPER(get_cp15)(CPUState *env, uint32_t insn)
             switch (ARM_CPUID(env)) {
             case ARM_CPUID_ARM1026:
                 return 1;
-            case ARM_CPUID_ARM1136:
-            case ARM_CPUID_ARM1136_R2:
+            case ARM_CPUID_ARM1136_R1P3:
+            case ARM_CPUID_ARM1136_R0P2:
             case ARM_CPUID_ARM1176:
                 return 7;
             case ARM_CPUID_ARM11MPCORE:
-- 
1.7.3.4

Re: [Qemu-devel] [PATCH] target-arm: Tidy up ARM1136 CPUID naming

[Andreas Färber]

Am 03.10.2011 12:32, schrieb Andreas Färber:
> -cpu arm1136-r2 is commented to in fact be ARM1136 r0p2, whereas
> -cpu arm1136 seems to be ARM1136 r1p3 according to the MIDR value.
> 
> The CPUID values contain major and minor revision numbers (rnpn) and
> are never used with a mask, so are specific to the chosen revision.
> Rename the CPUID preprocessor defines to reflect this, but leave the
> CPU model names unchanged for command line compatibility.
> 
> Cc: Andrzej Zaborowski <andrew@openedhand.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Andreas Färber <andreas.faerber@web.de>
> ---

Ping?

Andreas

>  target-arm/cpu.h    |    4 ++--
>  target-arm/helper.c |   12 ++++++------
>  2 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index 6ab780d..783989f 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -419,8 +419,8 @@ void cpu_arm_set_cp_io(CPUARMState *env, int cpnum,
>  #define ARM_CPUID_PXA270_B1   0x69054113
>  #define ARM_CPUID_PXA270_C0   0x69054114
>  #define ARM_CPUID_PXA270_C5   0x69054117
> -#define ARM_CPUID_ARM1136     0x4117b363
> -#define ARM_CPUID_ARM1136_R2  0x4107b362
> +#define ARM_CPUID_ARM1136_R1P3 0x4117b363
> +#define ARM_CPUID_ARM1136_R0P2 0x4107b362
>  #define ARM_CPUID_ARM1176     0x410fb767
>  #define ARM_CPUID_ARM11MPCORE 0x410fb022
>  #define ARM_CPUID_CORTEXA8    0x410fc080
> diff --git a/target-arm/helper.c b/target-arm/helper.c
> index e2428eb..0d342ba 100644
> --- a/target-arm/helper.c
> +++ b/target-arm/helper.c
> @@ -76,11 +76,11 @@ static void cpu_reset_model_id(CPUARMState *env, uint32_t id)
>          env->cp15.c0_cachetype = 0x1dd20d2;
>          env->cp15.c1_sys = 0x00090078;
>          break;
> -    case ARM_CPUID_ARM1136:
> +    case ARM_CPUID_ARM1136_R1P3:
>          /* This is the 1136 r1, which is a v6K core */
>          set_feature(env, ARM_FEATURE_V6K);
>          /* Fall through */
> -    case ARM_CPUID_ARM1136_R2:
> +    case ARM_CPUID_ARM1136_R0P2:
>          /* What qemu calls "arm1136_r2" is actually the 1136 r0p2, ie an
>           * older core than plain "arm1136". In particular this does not
>           * have the v6K features.
> @@ -417,8 +417,8 @@ static const struct arm_cpu_t arm_cpu_names[] = {
>      { ARM_CPUID_ARM926, "arm926"},
>      { ARM_CPUID_ARM946, "arm946"},
>      { ARM_CPUID_ARM1026, "arm1026"},
> -    { ARM_CPUID_ARM1136, "arm1136"},
> -    { ARM_CPUID_ARM1136_R2, "arm1136-r2"},
> +    { ARM_CPUID_ARM1136_R1P3, "arm1136" },
> +    { ARM_CPUID_ARM1136_R0P2, "arm1136-r2" },
>      { ARM_CPUID_ARM1176, "arm1176"},
>      { ARM_CPUID_ARM11MPCORE, "arm11mpcore"},
>      { ARM_CPUID_CORTEXM3, "cortex-m3"},
> @@ -1886,8 +1886,8 @@ uint32_t HELPER(get_cp15)(CPUState *env, uint32_t insn)
>              switch (ARM_CPUID(env)) {
>              case ARM_CPUID_ARM1026:
>                  return 1;
> -            case ARM_CPUID_ARM1136:
> -            case ARM_CPUID_ARM1136_R2:
> +            case ARM_CPUID_ARM1136_R1P3:
> +            case ARM_CPUID_ARM1136_R0P2:
>              case ARM_CPUID_ARM1176:
>                  return 7;
>              case ARM_CPUID_ARM11MPCORE:

Re: [Qemu-devel] [PATCH] target-arm: Tidy up ARM1136 CPUID naming

[Peter Maydell]

On 3 October 2011 11:32, Andreas Färber <andreas.faerber@web.de> wrote:
> -#define ARM_CPUID_ARM1136     0x4117b363
> -#define ARM_CPUID_ARM1136_R2  0x4107b362
> +#define ARM_CPUID_ARM1136_R1P3 0x4117b363
> +#define ARM_CPUID_ARM1136_R0P2 0x4107b362

I don't think the patchlevels are important enough to
memorialise in the constant names. The important
distinction in behaviour is between the r0 and r1, so
I think that ARM1136_R0 vs _R1 would be better.

-- PMM

Re: [Qemu-devel] [PATCH] target-arm: Tidy up ARM1136 CPUID naming

[Andreas Färber]

Am 22.10.2011 12:20, schrieb Peter Maydell:
> On 3 October 2011 11:32, Andreas Färber <andreas.faerber@web.de> wrote:
>> -#define ARM_CPUID_ARM1136     0x4117b363
>> -#define ARM_CPUID_ARM1136_R2  0x4107b362
>> +#define ARM_CPUID_ARM1136_R1P3 0x4117b363
>> +#define ARM_CPUID_ARM1136_R0P2 0x4107b362
> 
> I don't think the patchlevels are important enough to
> memorialise in the constant names. The important
> distinction in behaviour is between the r0 and r1, so
> I think that ARM1136_R0 vs _R1 would be better.

Would you be okay if we do the following?

#define ARM_CPUID_ARM1136_R0 ARM_CPUID_ARM1136_R0P2
#define ARM_CPUID_ARM1136_R1 ARM_CPUID_ARM1136_R1P3

My point is that the number is actually hardcoded in there, whatever we
name the constant. ARM1136 or ARM1136_R0 gives the impression of a more
generic value.

Masking would be the only way to have generic code there and I still
haven't figured out how to do that sensibly without breaking up the
whole switch. For now I have a patch cooking for CPUID preservation that
I'll submit shortly.

Andreas

Re: [Qemu-devel] [PATCH] target-arm: Tidy up ARM1136 CPUID naming

[Peter Maydell]

On 22 October 2011 11:33, Andreas Färber <andreas.faerber@web.de> wrote:
> Am 22.10.2011 12:20, schrieb Peter Maydell:
>> On 3 October 2011 11:32, Andreas Färber <andreas.faerber@web.de> wrote:
>>> -#define ARM_CPUID_ARM1136     0x4117b363
>>> -#define ARM_CPUID_ARM1136_R2  0x4107b362
>>> +#define ARM_CPUID_ARM1136_R1P3 0x4117b363
>>> +#define ARM_CPUID_ARM1136_R0P2 0x4107b362
>>
>> I don't think the patchlevels are important enough to
>> memorialise in the constant names. The important
>> distinction in behaviour is between the r0 and r1, so
>> I think that ARM1136_R0 vs _R1 would be better.
>
> Would you be okay if we do the following?
>
> #define ARM_CPUID_ARM1136_R0 ARM_CPUID_ARM1136_R0P2
> #define ARM_CPUID_ARM1136_R1 ARM_CPUID_ARM1136_R1P3

Not sure that makes it any better.

> My point is that the number is actually hardcoded in there, whatever we
> name the constant.

I think that's really the problem...

-- PMM