Re: [Qemu-arm] [PATCH 01/20] target/arm: Implement ARMv8M's PMSAv8 registers

[Peter Maydell]

On 5 September 2017 at 20:16, Philippe Mathieu-Daudé <address@hidden> wrote:
> Hi Peter,
>
>
> On 08/22/2017 12:08 PM, Peter Maydell wrote:
>>   diff --git a/target/arm/machine.c b/target/arm/machine.c
>> index 3193b00..05e2909 100644
>> --- a/target/arm/machine.c
>> +++ b/target/arm/machine.c
>> @@ -159,7 +159,8 @@ static bool pmsav7_needed(void *opaque)
>>       CPUARMState *env = &cpu->env;
>>         return arm_feature(env, ARM_FEATURE_PMSA) &&
>> -           arm_feature(env, ARM_FEATURE_V7);
>> +           arm_feature(env, ARM_FEATURE_V7) &&
>> +           !arm_feature(env, ARM_FEATURE_V8);
>>   }
>>     static bool pmsav7_rgnr_vmstate_validate(void *opaque, int version_id)
>> @@ -209,6 +210,31 @@ static const VMStateDescription vmstate_pmsav7_rnr =
>> {
>>       }
>>   };
>>   +static bool pmsav8_needed(void *opaque)
>> +{
>> +    ARMCPU *cpu = opaque;
>> +    CPUARMState *env = &cpu->env;
>> +
>> +    return arm_feature(env, ARM_FEATURE_PMSA) &&
>> +        arm_feature(env, ARM_FEATURE_V8);
>> +}
>> +
>> +static const VMStateDescription vmstate_pmsav8 = {
>> +    .name = "cpu/pmsav8",
>> +    .version_id = 1,
>> +    .minimum_version_id = 1,
>> +    .needed = pmsav8_needed,
>> +    .fields = (VMStateField[]) {
>> +        VMSTATE_VARRAY_UINT32(env.pmsav8.rbar, ARMCPU, pmsav7_dregion, 0,
>> +                              vmstate_info_uint32, uint32_t),
>> +        VMSTATE_VARRAY_UINT32(env.pmsav8.rlar, ARMCPU, pmsav7_dregion, 0,
>> +                              vmstate_info_uint32, uint32_t),
>> +        VMSTATE_UINT32(env.pmsav8.mair0, ARMCPU),
>> +        VMSTATE_UINT32(env.pmsav8.mair1, ARMCPU),
>> +        VMSTATE_END_OF_LIST()
>> +    }
>> +};
>> +
>>   static int get_cpsr(QEMUFile *f, void *opaque, size_t size,
>>                       VMStateField *field)
>>   {
>>
>
> maybe you plan to add migration between version 22 and 23 in a later patch,
> else
>
> ...
> const VMStateDescription vmstate_arm_cpu = {
>     ...
>     .subsections = (const VMStateDescription*[]) {
>         ...
>         &vmstate_pmsav8,
>
> do not forget this ^

Whoops, yes, I forgot that bit. I'm surprised the compiler
didn't complain about the unused variables...

There's no need to worry about migration compat in any
of these v8M-specific vmstate structs, because right now
there are no QEMU CPUs with v8M enabled and so the
vmstate struct can't be used. That means we can freely
add fields to them in later patches without having to
bump version numbers or otherwise keep compatibility.
Once the patch which adds a cortex-m33 CPU lands in
master the rules will change and we'll need to be more
careful.


thanks
-- PMM