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

[Auger Eric]

Hi Peter,

On 24/02/2017 18:57, Peter Maydell wrote:
> 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.

OK that clarifies.

Thanks!

Eric
> 
> thanks
> -- PMM
>