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(-)

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);
-- 
2.34.1

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