| Message ID | 20240206132931.38376-2-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | hw/arm: Implement new machine mps3-an536 (Cortex-R52 MPS3 AN536 FPGA image) | expand |
On 2/6/24 23:29, Peter Maydell wrote: > We support two different encodings for the AArch32 IMPDEF > CBAR register -- older cores like the Cortex A9, A7, A15 > have this at 4, c15, c0, 0; newer cores like the > Cortex A35, A53, A57 and A72 have it at 1 c15 c0 0. > > When we implemented this we picked which encoding to > use based on whether the CPU set ARM_FEATURE_AARCH64. > However this isn't right for three cases: > * the qemu-system-arm 'max' CPU, which is supposed to be > a variant on a Cortex-A57; it ought to use the same > encoding the A57 does and which the AArch64 'max' > exposes to AArch32 guest code > * the Cortex-R52, which is AArch32-only but has the CBAR > at the newer encoding (and where we incorrectly are > not yet setting ARM_FEATURE_CBAR_RO anyway) > * any possible future support for other v8 AArch32 > only CPUs, or for supporting "boot the CPU into > AArch32 mode" on our existing cores like the A57 etc > > Make the decision of the encoding be based on whether > the CPU implements the ARM_FEATURE_V8 flag instead. > > This changes the behaviour only for the qemu-system-arm > '-cpu max'. We don't expect anybody to be relying on the > old behaviour because: > * it's not what the real hardware Cortex-A57 does > (and that's what our ID register claims we are) Not even that, because max resets MIDR. Anyway, Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~
On Tue, 6 Feb 2024 at 20:34, Richard Henderson <richard.henderson@linaro.org> wrote: > > On 2/6/24 23:29, Peter Maydell wrote: > > We support two different encodings for the AArch32 IMPDEF > > CBAR register -- older cores like the Cortex A9, A7, A15 > > have this at 4, c15, c0, 0; newer cores like the > > Cortex A35, A53, A57 and A72 have it at 1 c15 c0 0. > > > > When we implemented this we picked which encoding to > > use based on whether the CPU set ARM_FEATURE_AARCH64. > > However this isn't right for three cases: > > * the qemu-system-arm 'max' CPU, which is supposed to be > > a variant on a Cortex-A57; it ought to use the same > > encoding the A57 does and which the AArch64 'max' > > exposes to AArch32 guest code > > * the Cortex-R52, which is AArch32-only but has the CBAR > > at the newer encoding (and where we incorrectly are > > not yet setting ARM_FEATURE_CBAR_RO anyway) > > * any possible future support for other v8 AArch32 > > only CPUs, or for supporting "boot the CPU into > > AArch32 mode" on our existing cores like the A57 etc > > > > Make the decision of the encoding be based on whether > > the CPU implements the ARM_FEATURE_V8 flag instead. > > > > This changes the behaviour only for the qemu-system-arm > > '-cpu max'. We don't expect anybody to be relying on the > > old behaviour because: > > * it's not what the real hardware Cortex-A57 does > > (and that's what our ID register claims we are) > > Not even that, because max resets MIDR. qemu-system-aarch64 max does (in aarch64_max_tcg_initfn(), yes; but qemu-system-arm max is set up in arm_max_initfn() in cpu32.c, and that sets cpu->midr = 0x411fd070 (which is the same as A57's MIDR)... > Anyway, > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> thanks -- PMM
diff --git a/target/arm/helper.c b/target/arm/helper.c index 945d8571a61..2a2659aade2 100644 --- a/target/arm/helper.c +++ b/target/arm/helper.c @@ -9519,7 +9519,7 @@ void register_cp_regs_for_features(ARMCPU *cpu) * AArch64 cores we might need to add a specific feature flag * to indicate cores with "flavour 2" CBAR. */ - if (arm_feature(env, ARM_FEATURE_AARCH64)) { + if (arm_feature(env, ARM_FEATURE_V8)) { /* 32 bit view is [31:18] 0...0 [43:32]. */ uint32_t cbar32 = (extract64(cpu->reset_cbar, 18, 14) << 18) | extract64(cpu->reset_cbar, 32, 12);
We support two different encodings for the AArch32 IMPDEF CBAR register -- older cores like the Cortex A9, A7, A15 have this at 4, c15, c0, 0; newer cores like the Cortex A35, A53, A57 and A72 have it at 1 c15 c0 0. When we implemented this we picked which encoding to use based on whether the CPU set ARM_FEATURE_AARCH64. However this isn't right for three cases: * the qemu-system-arm 'max' CPU, which is supposed to be a variant on a Cortex-A57; it ought to use the same encoding the A57 does and which the AArch64 'max' exposes to AArch32 guest code * the Cortex-R52, which is AArch32-only but has the CBAR at the newer encoding (and where we incorrectly are not yet setting ARM_FEATURE_CBAR_RO anyway) * any possible future support for other v8 AArch32 only CPUs, or for supporting "boot the CPU into AArch32 mode" on our existing cores like the A57 etc Make the decision of the encoding be based on whether the CPU implements the ARM_FEATURE_V8 flag instead. This changes the behaviour only for the qemu-system-arm '-cpu max'. We don't expect anybody to be relying on the old behaviour because: * it's not what the real hardware Cortex-A57 does (and that's what our ID register claims we are) * we don't implement the memory-mapped GICv3 support which is the only thing that exists at the peripheral base address pointed to by the register Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target/arm/helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)