[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] target-arm: Add VBAR support to ARM1176 CPUs

From: Cédric Le Goater
Subject: Re: [Qemu-devel] [PATCH] target-arm: Add VBAR support to ARM1176 CPUs
Date: Thu, 24 Nov 2016 15:29:06 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0

On 09/05/2016 04:39 PM, Peter Maydell wrote:
> On 23 August 2016 at 10:59, Cédric Le Goater <address@hidden> wrote:
>> ARM1176 CPUs support the Vector Base Address Register but currently,
>> qemu only supports VBAR on ARMv7 CPUs. Fix this by adding a new
>> feature ARM_FEATURE_VBAR which is used for ARMv7 and ARM1176 CPUs.
>> Signed-off-by: Cédric Le Goater <address@hidden>
> This looks mostly good; one question below.

Sorry for the late answer. I was drilling down through PPC with 
the initial support of the pnv machine. 

For 2.9, I would like to send a couple of fixes for the Aspeed  
machines, this is one of them and I feel I didn't choose the 
easiest path to enter the ARM world :)

>> Index: qemu-aspeed.git/target-arm/cpu.h
>> ===================================================================
>> --- qemu-aspeed.git.orig/target-arm/cpu.h
>> +++ qemu-aspeed.git/target-arm/cpu.h
>> @@ -1129,6 +1129,7 @@ enum arm_features {
>>      ARM_FEATURE_V8_SHA256, /* implements SHA256 part of v8 Crypto 
>> Extensions */
>>      ARM_FEATURE_V8_PMULL, /* implements PMULL part of v8 Crypto Extensions 
>> */
>>      ARM_FEATURE_THUMB_DSP, /* DSP insns supported in the Thumb encodings */
>> +    ARM_FEATURE_VBAR, /* has cp15 VBAR (ARM1176) */
> you don't need the bit in brackets, I think (it's not complete
> anyway, since v7+ also has this).


>>  };
>>  static inline int arm_feature(CPUARMState *env, int feature)
>> Index: qemu-aspeed.git/target-arm/cpu.c
>> ===================================================================
>> --- qemu-aspeed.git.orig/target-arm/cpu.c
>> +++ qemu-aspeed.git/target-arm/cpu.c
>> @@ -584,6 +584,7 @@ static void arm_cpu_realizefn(DeviceStat
>>          set_feature(env, ARM_FEATURE_LPAE);
>>      }
>>      if (arm_feature(env, ARM_FEATURE_V7)) {
>> +        set_feature(env, ARM_FEATURE_VBAR);
>>          set_feature(env, ARM_FEATURE_VAPA);
>>          set_feature(env, ARM_FEATURE_THUMB2);
>>          set_feature(env, ARM_FEATURE_MPIDR);
>> @@ -867,6 +868,7 @@ static void arm1176_initfn(Object *obj)
>>      cpu->dtb_compatible = "arm,arm1176";
>>      set_feature(&cpu->env, ARM_FEATURE_V6K);
>> +    set_feature(&cpu->env, ARM_FEATURE_VBAR);
>>      set_feature(&cpu->env, ARM_FEATURE_VFP);
>>      set_feature(&cpu->env, ARM_FEATURE_VAPA);
>>      set_feature(&cpu->env, ARM_FEATURE_DUMMY_C15_REGS);
> Is it sufficient to set ARM_FEATURE_VBAR in the realizefn
> if FEATURE_V7 or FEATURE_EL3? (watch out that realizefn
> may change the value of the FEATURE_EL3 bit partway down
> so you'd need to do the check there) ?

It looks like it, yes. 

The addition of the VBAR only depends on ARM_FEATURE_VBAR 
bit which is set for 1176 and V7. I followed the pattern 
used by ARM_FEATURE_V6K. FEATURE_EL3 does not come in play
but may be it should indeed, see below.

> We implement VBAR in v7-without-EL3 even though architecturally
> it should only exist in v7-with-EL3 because we have some
> legacy board models which we implement as without-EL3 but
> where the guest (Linux) assumes it's running on a with-EL3
> CPU in NS mode. I'd rather we stick to the architectural
> definition for 1176 if we can rather than expanding the
> "things we do which aren't architectural" set if there's
> no legacy config that requires it. (If it is necessary
> to define it for 1176 always then that's OK, I'd just
> prefer not to unless we know we have to.)

According to the ARM spec, 1176 is a v6 with a couple of 
extensions, among which v6k and TrustZone. So it has Secure 
and Non-Secure VBAR and MVBAR registers.

Looking at :


May be, we should instead introduce a new ARMv6Z architecture 
for the 1176 cpu which would represent a ARMv6 + TrustZone ?

But I am a bit confused with this EL3 feature. 1176 does not 
have EL3 to my understanding (I am beyond my ARM skills there).
So why is it part of the feature list in arm1176_initfn ? 
Is that a way to define the MVBAR and SCR regs ? 

Should we define VBAR under el3_cp_reginfo and not under 
v7_cp_reginfo then ? 

> We should probably also have a brief comment noting that
> we define VBAR always in v7, even though architecturally
> it doesn't exist in non-EL3 configs, for the benefit of
> legacy board models.

ok. I can add that.



> (In v8 VBAR is required whether EL3 is implemented or not.)
> thanks
> -- PMM

reply via email to

[Prev in Thread] Current Thread [Next in Thread]