Re: [Qemu-devel] [PATCH v1 13/22] target-arm: Register EL2 versions of ELR and SPSR

[Peter Maydell]

On 7 May 2014 07:02, Peter Crosthwaite <address@hidden> wrote:
> On Tue, May 6, 2014 at 4:08 PM, Edgar E. Iglesias
> <address@hidden> wrote:
>> From: "Edgar E. Iglesias" <address@hidden>
>>
>> Signed-off-by: Edgar E. Iglesias <address@hidden>
>> ---
>>  target-arm/helper.c | 17 +++++++++++++++++
>>  1 file changed, 17 insertions(+)
>>
>> diff --git a/target-arm/helper.c b/target-arm/helper.c
>> index ba1830d..8efc340 100644
>> --- a/target-arm/helper.c
>> +++ b/target-arm/helper.c
>> @@ -2078,6 +2078,19 @@ static const ARMCPRegInfo v8_cp_reginfo[] = {
>>      REGINFO_SENTINEL
>>  };
>>
>> +static const ARMCPRegInfo v8_el2_cp_reginfo[] = {
>> +    { .name = "ELR_EL2", .state = ARM_CP_STATE_AA64,
>> +      .type = ARM_CP_NO_MIGRATE,
>> +      .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 0, .opc2 = 1,
>> +      .access = PL2_RW,
>> +      .fieldoffset = offsetof(CPUARMState, elr_el[ELR_EL_IDX(2)]) },
>> +    { .name = "SPSR_EL2", .state = ARM_CP_STATE_AA64,
>> +      .type = ARM_CP_NO_MIGRATE,
>> +      .opc0 = 3, .opc1 = 4, .crn = 4, .crm = 0, .opc2 = 0,
>> +      .access = PL2_RW, .fieldoffset = offsetof(CPUARMState, 
>> banked_spsr[6]) },
>> +    REGINFO_SENTINEL
>> +};
>> +
>>  static void sctlr_write(CPUARMState *env, const ARMCPRegInfo *ri,
>>                          uint64_t value)
>>  {
>> @@ -2321,6 +2334,10 @@ void register_cp_regs_for_features(ARMCPU *cpu)
>>          define_arm_cp_regs(cpu, v8_idregs);
>>          define_arm_cp_regs(cpu, v8_cp_reginfo);
>>          define_aarch64_debug_regs(cpu);
>> +
>> +        if (arm_feature(env, ARM_FEATURE_EL2)) {
>> +            define_arm_cp_regs(cpu, v8_el2_cp_reginfo);
>> +        }
>
> I think this should be outside the if ARM_FEATURE_V8 for consistency.
> None of the other per-feature CP register defs are nested within the
> ifferry for their ARM version. Detecting the invalid combination of
> ARM_FEATURE_EL2 and pre V8 should probably be an assertion done in
> arm_cpu_realizefn().

ARM_FEATURE_EL2 + pre-V8 should be valid, that seems the obvious
way to indicate "v7 CPU with virt extensions".

As it happens registering these reginfo on a pre-v8 CPU is
harmless -- they'll just never be accessible.

thanks
-- PMM