Re: [Qemu-arm] [PATCH v9 2/5] hw/intc/arm_gicv3_kvm: Add ICC_SRE_EL1 register to vmstate

[Peter Maydell]

On 24 February 2017 at 17:53, Auger Eric <address@hidden> wrote:
> Hi,
>
> On 23/02/2017 12:51, address@hidden wrote:
>> From: Vijaya Kumar K <address@hidden>
>>
>> To Save and Restore ICC_SRE_EL1 register introduce vmstate
>> subsection and load only if non-zero.
> != 7
>
>> Also initialize icc_sre_el1 with to 0x7 in pre_load
>> function.
>>
>> Signed-off-by: Vijaya Kumar K <address@hidden>
>> ---
>>  hw/intc/arm_gicv3_common.c         | 36 ++++++++++++++++++++++++++++++++++++
>>  include/hw/intc/arm_gicv3_common.h |  1 +
>>  2 files changed, 37 insertions(+)
>>
>> diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
>> index 16b9b0f..5b0e456 100644
>> --- a/hw/intc/arm_gicv3_common.c
>> +++ b/hw/intc/arm_gicv3_common.c
>> @@ -70,6 +70,38 @@ static const VMStateDescription vmstate_gicv3_cpu_virt = {
>>      }
>>  };
>>
>> +static int icc_sre_el1_reg_pre_load(void *opaque)
>> +{
>> +    GICv3CPUState *cs = opaque;
>> +
>> +   /*
>> +    * If the sre_el1 subsection is not transferred this
>> +    * means SRE_EL1 is 0x7 (which might not be the same as
>> +    * our reset value).
>> +    */
>> +    cs->icc_sre_el1 = 0x7;
>> +    return 0;
>> +}
> As Peter asked before I don't really get why we need the pre_load
> function here.

No, it's correct. As the comment says, the reset value
of icc_sre_el1 might not be 7 (it is right now, but it's
less of a trap for future changes to not assume it).
So the way migration works is that we use pre-load on the destination
end to set the value to 0x7, then if the source end transfers the data
we get the transferred value, otherwise we end up with the default
value as set in the pre-load function.

thanks
-- PMM