| Message ID | 1433247661-2555-1-git-send-email-serge.fdrv@gmail.com |
|---|---|
| State | New |
| Headers | show |
Hi, On 2015/6/2 20:21, Sergey Fedorov wrote: > According to ARM Cortex-A57 TRM, REVIDR reset value should be zero. So > let REVIDR reset value be specified by CPU model and fix it for > Cortex-A57. > Also need to fix it for Cortex-A53? > Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com> > --- > target-arm/cpu-qom.h | 1 + > target-arm/cpu64.c | 1 + > target-arm/helper.c | 2 +- > 3 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h > index ed5a644..c80381d 100644 > --- a/target-arm/cpu-qom.h > +++ b/target-arm/cpu-qom.h > @@ -127,6 +127,7 @@ typedef struct ARMCPU { > * prefix means a constant register. > */ > uint32_t midr; > + uint32_t revidr; > uint32_t reset_fpsid; > uint32_t mvfr0; > uint32_t mvfr1; > diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c > index bf7dd68..e330160 100644 > --- a/target-arm/cpu64.c > +++ b/target-arm/cpu64.c > @@ -110,6 +110,7 @@ static void aarch64_a57_initfn(Object *obj) > set_feature(&cpu->env, ARM_FEATURE_CRC); > cpu->kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A57; > cpu->midr = 0x411fd070; > + cpu->revidr = 0x00000000; > cpu->reset_fpsid = 0x41034070; > cpu->mvfr0 = 0x10110222; > cpu->mvfr1 = 0x12111111; > diff --git a/target-arm/helper.c b/target-arm/helper.c > index b2b377a..89940d6 100644 > --- a/target-arm/helper.c > +++ b/target-arm/helper.c > @@ -3363,7 +3363,7 @@ void register_cp_regs_for_features(ARMCPU *cpu) > .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 }, > + .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = cpu->revidr }, > REGINFO_SENTINEL > }; > ARMCPRegInfo id_cp_reginfo[] = { >
On 2 June 2015 at 13:51, Shannon Zhao <shannon.zhao@linaro.org> wrote: > Hi, > > On 2015/6/2 20:21, Sergey Fedorov wrote: >> According to ARM Cortex-A57 TRM, REVIDR reset value should be zero. So >> let REVIDR reset value be specified by CPU model and fix it for >> Cortex-A57. >> > > Also need to fix it for Cortex-A53? Yes, please. Also you need to update the comment: /* 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. */ since the REVIDR isn't an alias of the MIDR any more. NB: a bug that's been on my todo list for ages is that the comment is incorrect about the wildcard being unnecessary -- this was a misreading of the ARM ARM by me when I wrote that code. v8 *does* retain the aliases of the MIDR at any unoccupied opc2 values in the 0, c0, c0, x space, and we should provide the wildcard encoding here. thanks -- PMM
On 2 June 2015 at 14:18, Peter Maydell <peter.maydell@linaro.org> wrote: > NB: a bug that's been on my todo list for ages is that the comment > is incorrect about the wildcard being unnecessary -- this was a > misreading of the ARM ARM by me when I wrote that code. v8 *does* > retain the aliases of the MIDR at any unoccupied opc2 values > in the 0, c0, c0, x space, and we should provide the wildcard > encoding here. ...*but* this only applies (as far as I can tell) to v8 AArch32; v8 AArch64 MIDR has no aliases. -- PMM
On 2 June 2015 at 13:21, Sergey Fedorov <serge.fdrv@gmail.com> wrote: > According to ARM Cortex-A57 TRM, REVIDR reset value should be zero. So > let REVIDR reset value be specified by CPU model and fix it for > Cortex-A57. > > Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com> > -- Do you have guest software that actually cares about the REVIDR? What's it doing with the value? thanks -- PMM
On 02.06.2015 16:28, Peter Maydell wrote: > On 2 June 2015 at 14:18, Peter Maydell <peter.maydell@linaro.org> wrote: >> NB: a bug that's been on my todo list for ages is that the comment >> is incorrect about the wildcard being unnecessary -- this was a >> misreading of the ARM ARM by me when I wrote that code. v8 *does* >> retain the aliases of the MIDR at any unoccupied opc2 values >> in the 0, c0, c0, x space, and we should provide the wildcard >> encoding here. > ...*but* this only applies (as far as I can tell) to v8 AArch32; > v8 AArch64 MIDR has no aliases. > > -- PMM OK, I'm going to fix that as well. Regards, Sergey
On 02.06.2015 16:54, Peter Maydell wrote: > On 2 June 2015 at 13:21, Sergey Fedorov <serge.fdrv@gmail.com> wrote: >> According to ARM Cortex-A57 TRM, REVIDR reset value should be zero. So >> let REVIDR reset value be specified by CPU model and fix it for >> Cortex-A57. >> >> Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com> >> -- > Do you have guest software that actually cares about the REVIDR? > What's it doing with the value? > > thanks > -- PMM Linux kernel reads this register. I just saw a strange value and checked it against TRM. Regards, Sergey
diff --git a/target-arm/cpu-qom.h b/target-arm/cpu-qom.h index ed5a644..c80381d 100644 --- a/target-arm/cpu-qom.h +++ b/target-arm/cpu-qom.h @@ -127,6 +127,7 @@ typedef struct ARMCPU { * prefix means a constant register. */ uint32_t midr; + uint32_t revidr; uint32_t reset_fpsid; uint32_t mvfr0; uint32_t mvfr1; diff --git a/target-arm/cpu64.c b/target-arm/cpu64.c index bf7dd68..e330160 100644 --- a/target-arm/cpu64.c +++ b/target-arm/cpu64.c @@ -110,6 +110,7 @@ static void aarch64_a57_initfn(Object *obj) set_feature(&cpu->env, ARM_FEATURE_CRC); cpu->kvm_target = QEMU_KVM_ARM_TARGET_CORTEX_A57; cpu->midr = 0x411fd070; + cpu->revidr = 0x00000000; cpu->reset_fpsid = 0x41034070; cpu->mvfr0 = 0x10110222; cpu->mvfr1 = 0x12111111; diff --git a/target-arm/helper.c b/target-arm/helper.c index b2b377a..89940d6 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -3363,7 +3363,7 @@ void register_cp_regs_for_features(ARMCPU *cpu) .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 }, + .access = PL1_R, .type = ARM_CP_CONST, .resetvalue = cpu->revidr }, REGINFO_SENTINEL }; ARMCPRegInfo id_cp_reginfo[] = {
According to ARM Cortex-A57 TRM, REVIDR reset value should be zero. So let REVIDR reset value be specified by CPU model and fix it for Cortex-A57. Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com> --- target-arm/cpu-qom.h | 1 + target-arm/cpu64.c | 1 + target-arm/helper.c | 2 +- 3 files changed, 3 insertions(+), 1 deletion(-)