Hi Peter,
On Fri, Feb 17, 2017 at 7:25 PM, Peter Maydell <peter.mayd...@linaro.org> wrote:
> On 17 February 2017 at 06:31, <vijay.kil...@gmail.com> wrote:
>> From: Vijaya Kumar K <vijaya.ku...@cavium.com>
>>
>> To Save and Restore ICC_SRE_EL1 register introduce vmstate
>> subsection and load only if non-zero.
>> Also initialize icc_sre_el1 with to 0x7 in pre_load
>> function.
>>
>> Signed-off-by: Vijaya Kumar K <vijaya.ku...@cavium.com>
>> ---
>> hw/intc/arm_gicv3_common.c | 32 ++++++++++++++++++++++++++++++++
>> include/hw/intc/arm_gicv3_common.h | 1 +
>> 2 files changed, 33 insertions(+)
>>
>> diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
>> index 16b9b0f..e62480e 100644
>> --- a/hw/intc/arm_gicv3_common.c
>> +++ b/hw/intc/arm_gicv3_common.c
>> @@ -70,6 +70,34 @@ static const VMStateDescription vmstate_gicv3_cpu_virt = {
>> }
>> };
>>
>> +static int icc_sre_el1_reg_pre_load(void *opaque)
>> +{
>> + GICv3CPUState *cs = opaque;
>> +
>> + /* By default enable SRE and disable IRQ & FIQ bypass. */
>> + cs->icc_sre_el1 = 0x7;
>
> Why do we need the pre_load function? I would have
> expected that reset would have given us these defaults
> already.
>
>> + return 0;
>> +}
>> +
>> +static bool icc_sre_el1_reg_needed(void *opaque)
>> +{
>> + GICv3CPUState *cs = opaque;
>> +
>> + return cs->icc_sre_el1 != 0;
>
> I expected this to say "we need to transfer the value if
> it isn't 0x7" (since the current situation of migration
> is "we assume that the value is 0x7").
>
> Something should probably fail inbound migration for TCG
> if the value isn't 0x7, for that matter.
>
> Is there a situation where KVM might allow a value other
> than 0x7?
In KVM, the SRE_EL1 value is 0x1. During save, value
read from KVM is 0x1 though we reset to 0x7.
>
>> +}
>> +
>> +const VMStateDescription vmstate_gicv3_cpu_sre_el1 = {
>> + .name = "arm_gicv3_cpu/sre_el1",
>> + .version_id = 1,
>> + .minimum_version_id = 1,
>> + .pre_load = icc_sre_el1_reg_pre_load,
>> + .needed = icc_sre_el1_reg_needed,
>> + .fields = (VMStateField[]) {
>> + VMSTATE_UINT64(icc_sre_el1, GICv3CPUState),
>> + VMSTATE_END_OF_LIST()
>> + }
>> +};
>> +
>> static const VMStateDescription vmstate_gicv3_cpu = {
>> .name = "arm_gicv3_cpu",
>> .version_id = 1,
>> @@ -100,6 +128,10 @@ static const VMStateDescription vmstate_gicv3_cpu = {
>> .subsections = (const VMStateDescription * []) {
>> &vmstate_gicv3_cpu_virt,
>> NULL
>> + },
>> + .subsections = (const VMStateDescription * []) {
>> + &vmstate_gicv3_cpu_sre_el1,
>> + NULL
>> }
>> };
>>
>> diff --git a/include/hw/intc/arm_gicv3_common.h
>> b/include/hw/intc/arm_gicv3_common.h
>> index 4156051..bccdfe1 100644
>> --- a/include/hw/intc/arm_gicv3_common.h
>> +++ b/include/hw/intc/arm_gicv3_common.h
>> @@ -172,6 +172,7 @@ struct GICv3CPUState {
>> uint8_t gicr_ipriorityr[GIC_INTERNAL];
>>
>> /* CPU interface */
>> + uint64_t icc_sre_el1;
>> uint64_t icc_ctlr_el1[2];
>> uint64_t icc_pmr_el1;
>> uint64_t icc_bpr[3];
>> --
>> 1.9.1
>
> thanks
> -- PMM
