diff mbox

[v7,RESEND,2/5] hw/intc/arm_gicv3_kvm: Add ICC_SRE_EL1 register to vmstate

Message ID 1485879782-6075-3-git-send-email-vijay.kilari@gmail.com
State New
Headers show

Commit Message

Vijay Kilari Jan. 31, 2017, 4:22 p.m. UTC 
From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>

To Save and Restore ICC_SRE_EL1 register Add ICC_SRE_EL1 register
to vmstate and GICv3CPUState struct.

Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
---
 hw/intc/arm_gicv3_common.c         | 1 +
 include/hw/intc/arm_gicv3_common.h | 1 +
 2 files changed, 2 insertions(+)

Comments

Peter Maydell Feb. 7, 2017, 2:39 p.m. UTC | #1 
On 31 January 2017 at 16:22,  <vijay.kilari@gmail.com> wrote:
> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>
> To Save and Restore ICC_SRE_EL1 register Add ICC_SRE_EL1 register
> to vmstate and GICv3CPUState struct.
>
> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
> ---
>  hw/intc/arm_gicv3_common.c         | 1 +
>  include/hw/intc/arm_gicv3_common.h | 1 +
>  2 files changed, 2 insertions(+)
>
> diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
> index 0f8c4b8..f3245d9 100644
> --- a/hw/intc/arm_gicv3_common.c
> +++ b/hw/intc/arm_gicv3_common.c
> @@ -68,6 +68,7 @@ static const VMStateDescription vmstate_gicv3_cpu = {
>          VMSTATE_UINT32(gicr_igrpmodr0, GICv3CPUState),
>          VMSTATE_UINT32(gicr_nsacr, GICv3CPUState),
>          VMSTATE_UINT8_ARRAY(gicr_ipriorityr, GICv3CPUState, GIC_INTERNAL),
> +        VMSTATE_UINT64(icc_sre_el1, GICv3CPUState),
>          VMSTATE_UINT64_ARRAY(icc_ctlr_el1, GICv3CPUState, 2),
>          VMSTATE_UINT64(icc_pmr_el1, GICv3CPUState),
>          VMSTATE_UINT64_ARRAY(icc_bpr, GICv3CPUState, 3),
> diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h
> index 341a311..183c7f8 100644
> --- a/include/hw/intc/arm_gicv3_common.h
> +++ b/include/hw/intc/arm_gicv3_common.h
> @@ -166,6 +166,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];

This breaks migration compatibility for TCG using GICv3; you
need to do something here with a VMState subsection so
the new register is only transferred if it's non-zero.

thanks
-- PMM
Vijay Kilari Feb. 13, 2017, 12:17 p.m. UTC | #2 
On Tue, Feb 7, 2017 at 8:09 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 31 January 2017 at 16:22,  <vijay.kilari@gmail.com> wrote:
>> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>
>> To Save and Restore ICC_SRE_EL1 register Add ICC_SRE_EL1 register
>> to vmstate and GICv3CPUState struct.
>>
>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>> ---
>>  hw/intc/arm_gicv3_common.c         | 1 +
>>  include/hw/intc/arm_gicv3_common.h | 1 +
>>  2 files changed, 2 insertions(+)
>>
>> diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
>> index 0f8c4b8..f3245d9 100644
>> --- a/hw/intc/arm_gicv3_common.c
>> +++ b/hw/intc/arm_gicv3_common.c
>> @@ -68,6 +68,7 @@ static const VMStateDescription vmstate_gicv3_cpu = {
>>          VMSTATE_UINT32(gicr_igrpmodr0, GICv3CPUState),
>>          VMSTATE_UINT32(gicr_nsacr, GICv3CPUState),
>>          VMSTATE_UINT8_ARRAY(gicr_ipriorityr, GICv3CPUState, GIC_INTERNAL),
>> +        VMSTATE_UINT64(icc_sre_el1, GICv3CPUState),
>>          VMSTATE_UINT64_ARRAY(icc_ctlr_el1, GICv3CPUState, 2),
>>          VMSTATE_UINT64(icc_pmr_el1, GICv3CPUState),
>>          VMSTATE_UINT64_ARRAY(icc_bpr, GICv3CPUState, 3),
>> diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h
>> index 341a311..183c7f8 100644
>> --- a/include/hw/intc/arm_gicv3_common.h
>> +++ b/include/hw/intc/arm_gicv3_common.h
>> @@ -166,6 +166,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];
>
> This breaks migration compatibility for TCG using GICv3; you
> need to do something here with a VMState subsection so
> the new register is only transferred if it's non-zero.

So, you mean to put a check in kvm_arm_gicv3_put() and
kvm_arm_gicv3_get() to check for non-zero value?
icc_sre_el1 is always non-zero reset to 0xf in TCG and 0x7 in KVM mode.

>
> thanks
> -- PMM
Eric Auger Feb. 17, 2017, 9 a.m. UTC | #3 
Hi Vijaya,

On 13/02/2017 13:17, Vijay Kilari wrote:
> On Tue, Feb 7, 2017 at 8:09 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 31 January 2017 at 16:22,  <vijay.kilari@gmail.com> wrote:
>>> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>>
>>> To Save and Restore ICC_SRE_EL1 register Add ICC_SRE_EL1 register
>>> to vmstate and GICv3CPUState struct.
>>>
>>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>> ---
>>>  hw/intc/arm_gicv3_common.c         | 1 +
>>>  include/hw/intc/arm_gicv3_common.h | 1 +
>>>  2 files changed, 2 insertions(+)
>>>
>>> diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
>>> index 0f8c4b8..f3245d9 100644
>>> --- a/hw/intc/arm_gicv3_common.c
>>> +++ b/hw/intc/arm_gicv3_common.c
>>> @@ -68,6 +68,7 @@ static const VMStateDescription vmstate_gicv3_cpu = {
>>>          VMSTATE_UINT32(gicr_igrpmodr0, GICv3CPUState),
>>>          VMSTATE_UINT32(gicr_nsacr, GICv3CPUState),
>>>          VMSTATE_UINT8_ARRAY(gicr_ipriorityr, GICv3CPUState, GIC_INTERNAL),
>>> +        VMSTATE_UINT64(icc_sre_el1, GICv3CPUState),
>>>          VMSTATE_UINT64_ARRAY(icc_ctlr_el1, GICv3CPUState, 2),
>>>          VMSTATE_UINT64(icc_pmr_el1, GICv3CPUState),
>>>          VMSTATE_UINT64_ARRAY(icc_bpr, GICv3CPUState, 3),
>>> diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h
>>> index 341a311..183c7f8 100644
>>> --- a/include/hw/intc/arm_gicv3_common.h
>>> +++ b/include/hw/intc/arm_gicv3_common.h
>>> @@ -166,6 +166,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];
>>
>> This breaks migration compatibility for TCG using GICv3; you
>> need to do something here with a VMState subsection so
>> the new register is only transferred if it's non-zero.
> 
> So, you mean to put a check in kvm_arm_gicv3_put() and
> kvm_arm_gicv3_get() to check for non-zero value?
> icc_sre_el1 is always non-zero reset to 0xf in TCG and 0x7 in KVM mode.
In hw/intc/arm_gicv3_cpuif.c we have
    { .name = "ICC_SRE_EL1", .state = ARM_CP_STATE_BOTH,
	../..
      .resetvalue = 0x7,
    },
where did you find the TCG reset value equal to 0xF? I am not able to
find it.

Thanks

Eric

> 
>>
>> thanks
>> -- PMM
Vijay Kilari Feb. 17, 2017, 9:17 a.m. UTC | #4 
On Fri, Feb 17, 2017 at 2:30 PM, Auger Eric <eric.auger@redhat.com> wrote:
> Hi Vijaya,
>
> On 13/02/2017 13:17, Vijay Kilari wrote:
>> On Tue, Feb 7, 2017 at 8:09 PM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> On 31 January 2017 at 16:22,  <vijay.kilari@gmail.com> wrote:
>>>> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>>>
>>>> To Save and Restore ICC_SRE_EL1 register Add ICC_SRE_EL1 register
>>>> to vmstate and GICv3CPUState struct.
>>>>
>>>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com>
>>>> ---
>>>>  hw/intc/arm_gicv3_common.c         | 1 +
>>>>  include/hw/intc/arm_gicv3_common.h | 1 +
>>>>  2 files changed, 2 insertions(+)
>>>>
>>>> diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
>>>> index 0f8c4b8..f3245d9 100644
>>>> --- a/hw/intc/arm_gicv3_common.c
>>>> +++ b/hw/intc/arm_gicv3_common.c
>>>> @@ -68,6 +68,7 @@ static const VMStateDescription vmstate_gicv3_cpu = {
>>>>          VMSTATE_UINT32(gicr_igrpmodr0, GICv3CPUState),
>>>>          VMSTATE_UINT32(gicr_nsacr, GICv3CPUState),
>>>>          VMSTATE_UINT8_ARRAY(gicr_ipriorityr, GICv3CPUState, GIC_INTERNAL),
>>>> +        VMSTATE_UINT64(icc_sre_el1, GICv3CPUState),
>>>>          VMSTATE_UINT64_ARRAY(icc_ctlr_el1, GICv3CPUState, 2),
>>>>          VMSTATE_UINT64(icc_pmr_el1, GICv3CPUState),
>>>>          VMSTATE_UINT64_ARRAY(icc_bpr, GICv3CPUState, 3),
>>>> diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h
>>>> index 341a311..183c7f8 100644
>>>> --- a/include/hw/intc/arm_gicv3_common.h
>>>> +++ b/include/hw/intc/arm_gicv3_common.h
>>>> @@ -166,6 +166,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];
>>>
>>> This breaks migration compatibility for TCG using GICv3; you
>>> need to do something here with a VMState subsection so
>>> the new register is only transferred if it's non-zero.
>>
>> So, you mean to put a check in kvm_arm_gicv3_put() and
>> kvm_arm_gicv3_get() to check for non-zero value?
>> icc_sre_el1 is always non-zero reset to 0xf in TCG and 0x7 in KVM mode.
> In hw/intc/arm_gicv3_cpuif.c we have
>     { .name = "ICC_SRE_EL1", .state = ARM_CP_STATE_BOTH,
>         ../..
>       .resetvalue = 0x7,
>     },
> where did you find the TCG reset value equal to 0xF? I am not able to
> find it.

Sorry, I have referred to ICC_SRE_EL2/3. 0x7 is correct
diff mbox

Patch

diff --git a/hw/intc/arm_gicv3_common.c b/hw/intc/arm_gicv3_common.c
index 0f8c4b8..f3245d9 100644
--- a/hw/intc/arm_gicv3_common.c
+++ b/hw/intc/arm_gicv3_common.c
@@ -68,6 +68,7 @@  static const VMStateDescription vmstate_gicv3_cpu = {
         VMSTATE_UINT32(gicr_igrpmodr0, GICv3CPUState),
         VMSTATE_UINT32(gicr_nsacr, GICv3CPUState),
         VMSTATE_UINT8_ARRAY(gicr_ipriorityr, GICv3CPUState, GIC_INTERNAL),
+        VMSTATE_UINT64(icc_sre_el1, GICv3CPUState),
         VMSTATE_UINT64_ARRAY(icc_ctlr_el1, GICv3CPUState, 2),
         VMSTATE_UINT64(icc_pmr_el1, GICv3CPUState),
         VMSTATE_UINT64_ARRAY(icc_bpr, GICv3CPUState, 3),
diff --git a/include/hw/intc/arm_gicv3_common.h b/include/hw/intc/arm_gicv3_common.h
index 341a311..183c7f8 100644
--- a/include/hw/intc/arm_gicv3_common.h
+++ b/include/hw/intc/arm_gicv3_common.h
@@ -166,6 +166,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];