| Message ID | 1317637971-19296-1-git-send-email-andreas.faerber@web.de |
|---|---|
| State | New |
| Headers | show |
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:
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
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
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
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:
-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(-)