Re: [Qemu-devel] [PATCH] target/arm: fix CBAR register for AArch64 CPUs

[Luc Michel]

On 9/17/19 12:56 PM, Peter Maydell wrote:
> On Tue, 17 Sep 2019 at 09:43, Luc Michel <address@hidden> wrote:
>>
>> On 9/13/19 9:26 AM, Luc Michel wrote:
>>> Hi Peter,
>>>
>>> On 9/12/19 6:03 PM, Peter Maydell wrote:
>>>> I think we need to check through the TRMs to confirm which CPUs use
>>>> which format for the CBAR, and have a different feature bit for the
>>>> newer format/sysreg encoding, so we can provide the right sysregs for
>>>> the right cores.
>>> I checked all the AArch64 Cortex's TRMs, for those having a PERIPHBASE
>>> signal and CBAR register (namely Cortex-A53, 57, 72, 73), they all match
>>> the mapping I put in this patch, so I think we don't need to split the
>>> CBAR feature further. I believe more recent Cortex's address the GIC
>>> using coprocessor registers, and CBAR does not exist in those ones.
>>
>> Hi Peter,
>>
>> Do you want me to re-roll this patch with some modifications?
> 
> No, that's OK. Thanks for checking the TRMs. I think what I'll
> do is squash in this comment to the patch:
> 
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 755aa18a2dc..bc1130d989d 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -6733,6 +6733,19 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>      }
> 
>      if (arm_feature(env, ARM_FEATURE_CBAR)) {
> +        /*
> +         * CBAR is IMPDEF, but common on Arm Cortex-A implementations.
> +         * There are two flavours:
> +         *  (1) older 32-bit only cores have a simple 32-bit CBAR
> +         *  (2) 64-bit cores have a 64-bit CBAR visible to AArch64, plus a
> +         *      32-bit register visible to AArch32 at a different encoding
> +         *      to the "flavour 1" register and with the bits rearranged to
> +         *      be able to squash a 64-bit address into the 32-bit view.
> +         * We distinguish the two via the ARM_FEATURE_AARCH64 flag, but
> +         * in future if we support AArch32-only configs of some of the
> +         * 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)) {
>              /* 32 bit view is [31:18] 0...0 [43:32]. */
>              uint32_t cbar32 = (extract64(cpu->reset_cbar, 18, 14) << 18)
> 
> 
> and apply it to target-arm.next, if that's OK with you.
Yes sure!

Thanks.

-- 
Luc

> 
> thanks
> -- PMM
>