| Message ID | 1487850673-26455-3-git-send-email-vijay.kilari@gmail.com |
|---|---|
| State | New |
| Headers | show |
On 23 February 2017 at 11:51, <vijay.kilari@gmail.com> wrote: > From: Vijaya Kumar K <Vijaya.Kumar@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.Kumar@cavium.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> thanks -- PMM
Hi, On 23/02/2017 12:51, vijay.kilari@gmail.com wrote: > From: Vijaya Kumar K <Vijaya.Kumar@cavium.com> > > 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 <Vijaya.Kumar@cavium.com> > --- > 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. Besides Reviewed-by: Eric Auger <eric.auger@redhat.com> Eric > + > +static bool icc_sre_el1_reg_needed(void *opaque) > +{ > + GICv3CPUState *cs = opaque; > + > + return cs->icc_sre_el1 != 7; > +} > + > +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 +132,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]; >
On 24 February 2017 at 17:53, Auger Eric <eric.auger@redhat.com> wrote: > Hi, > > On 23/02/2017 12:51, vijay.kilari@gmail.com wrote: >> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com> >> >> 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 <Vijaya.Kumar@cavium.com> >> --- >> 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
Hi Peter, On 24/02/2017 18:57, Peter Maydell wrote: > On 24 February 2017 at 17:53, Auger Eric <eric.auger@redhat.com> wrote: >> Hi, >> >> On 23/02/2017 12:51, vijay.kilari@gmail.com wrote: >>> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com> >>> >>> 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 <Vijaya.Kumar@cavium.com> >>> --- >>> 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 >
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; +} + +static bool icc_sre_el1_reg_needed(void *opaque) +{ + GICv3CPUState *cs = opaque; + + return cs->icc_sre_el1 != 7; +} + +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 +132,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];