* [Qemu-devel] [PATCH] hw/intc/arm_gicv3_kvm: Check KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS in reset
@ 2017-03-28 13:58 Eric Auger
2017-03-28 14:02 ` Auger Eric
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Eric Auger @ 2017-03-28 13:58 UTC (permalink / raw)
To: eric.auger.pro, eric.auger, peter.maydell, qemu-arm, qemu-devel
Cc: vijay.kilari, Vijaya.Kumar, drjones
KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS needs to be checked before
attempting to read ICC_CTLR_EL1; otherwise kernel versions not
exposing this kvm device group will be incompatible with qemu 2.9.
Fixes: 07a5628 ("hw/intc/arm_gicv3_kvm: Reset GICv3 cpu interface registers")
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Reported-by: Prakash B <bjsprakash.linux@gmail.com>
---
I understand the ICC_CTLR_EL1 state only is used in the put() function
which is used for migration
---
hw/intc/arm_gicv3_kvm.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)
diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
index 81f0403..4c3a88e 100644
--- a/hw/intc/arm_gicv3_kvm.c
+++ b/hw/intc/arm_gicv3_kvm.c
@@ -614,12 +614,6 @@ static void arm_gicv3_icc_reset(CPUARMState *env, const ARMCPRegInfo *ri)
s = c->gic;
cpu = ARM_CPU(c->cpu);
- /* Initialize to actual HW supported configuration */
- kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS,
- KVM_VGIC_ATTR(ICC_CTLR_EL1, cpu->mp_affinity),
- &c->icc_ctlr_el1[GICV3_NS], false);
-
- c->icc_ctlr_el1[GICV3_S] = c->icc_ctlr_el1[GICV3_NS];
c->icc_pmr_el1 = 0;
c->icc_bpr[GICV3_G0] = GIC_MIN_BPR;
c->icc_bpr[GICV3_G1] = GIC_MIN_BPR;
@@ -628,6 +622,17 @@ static void arm_gicv3_icc_reset(CPUARMState *env, const ARMCPRegInfo *ri)
c->icc_sre_el1 = 0x7;
memset(c->icc_apr, 0, sizeof(c->icc_apr));
memset(c->icc_igrpen, 0, sizeof(c->icc_igrpen));
+
+ if (!kvm_device_check_attr(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS,
+ KVM_VGIC_ATTR(ICC_CTLR_EL1, cpu->mp_affinity))) {
+ return;
+ }
+ /* Initialize to actual HW supported configuration */
+ kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS,
+ KVM_VGIC_ATTR(ICC_CTLR_EL1, cpu->mp_affinity),
+ &c->icc_ctlr_el1[GICV3_NS], false);
+
+ c->icc_ctlr_el1[GICV3_S] = c->icc_ctlr_el1[GICV3_NS];
}
static void kvm_arm_gicv3_reset(DeviceState *dev)
--
2.5.5
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH] hw/intc/arm_gicv3_kvm: Check KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS in reset
2017-03-28 13:58 [Qemu-devel] [PATCH] hw/intc/arm_gicv3_kvm: Check KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS in reset Eric Auger
@ 2017-03-28 14:02 ` Auger Eric
2017-03-28 15:08 ` Vijay Kilari
2017-03-30 5:32 ` Prakash B
2 siblings, 0 replies; 5+ messages in thread
From: Auger Eric @ 2017-03-28 14:02 UTC (permalink / raw)
To: eric.auger.pro, peter.maydell, qemu-arm, qemu-devel
Cc: vijay.kilari, Vijaya.Kumar, drjones, Prakash B
Adding Prakash B in cc too, sorry.
Vijaya, please let me know if I missed something in your original patch.
I tested GICv3 KVM save/restore with v4.11-rc4 and Prakash B use case
with 4.10 kernel.
Thanks
Eric
On 28/03/2017 15:58, Eric Auger wrote:
> KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS needs to be checked before
> attempting to read ICC_CTLR_EL1; otherwise kernel versions not
> exposing this kvm device group will be incompatible with qemu 2.9.
>
> Fixes: 07a5628 ("hw/intc/arm_gicv3_kvm: Reset GICv3 cpu interface registers")
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Reported-by: Prakash B <bjsprakash.linux@gmail.com>
>
> ---
>
> I understand the ICC_CTLR_EL1 state only is used in the put() function
> which is used for migration
> ---
> hw/intc/arm_gicv3_kvm.c | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
> index 81f0403..4c3a88e 100644
> --- a/hw/intc/arm_gicv3_kvm.c
> +++ b/hw/intc/arm_gicv3_kvm.c
> @@ -614,12 +614,6 @@ static void arm_gicv3_icc_reset(CPUARMState *env, const ARMCPRegInfo *ri)
> s = c->gic;
> cpu = ARM_CPU(c->cpu);
>
> - /* Initialize to actual HW supported configuration */
> - kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS,
> - KVM_VGIC_ATTR(ICC_CTLR_EL1, cpu->mp_affinity),
> - &c->icc_ctlr_el1[GICV3_NS], false);
> -
> - c->icc_ctlr_el1[GICV3_S] = c->icc_ctlr_el1[GICV3_NS];
> c->icc_pmr_el1 = 0;
> c->icc_bpr[GICV3_G0] = GIC_MIN_BPR;
> c->icc_bpr[GICV3_G1] = GIC_MIN_BPR;
> @@ -628,6 +622,17 @@ static void arm_gicv3_icc_reset(CPUARMState *env, const ARMCPRegInfo *ri)
> c->icc_sre_el1 = 0x7;
> memset(c->icc_apr, 0, sizeof(c->icc_apr));
> memset(c->icc_igrpen, 0, sizeof(c->icc_igrpen));
> +
> + if (!kvm_device_check_attr(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS,
> + KVM_VGIC_ATTR(ICC_CTLR_EL1, cpu->mp_affinity))) {
> + return;
> + }
> + /* Initialize to actual HW supported configuration */
> + kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS,
> + KVM_VGIC_ATTR(ICC_CTLR_EL1, cpu->mp_affinity),
> + &c->icc_ctlr_el1[GICV3_NS], false);
> +
> + c->icc_ctlr_el1[GICV3_S] = c->icc_ctlr_el1[GICV3_NS];
> }
>
> static void kvm_arm_gicv3_reset(DeviceState *dev)
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH] hw/intc/arm_gicv3_kvm: Check KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS in reset
2017-03-28 13:58 [Qemu-devel] [PATCH] hw/intc/arm_gicv3_kvm: Check KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS in reset Eric Auger
2017-03-28 14:02 ` Auger Eric
@ 2017-03-28 15:08 ` Vijay Kilari
2017-03-28 15:43 ` Auger Eric
2017-03-30 5:32 ` Prakash B
2 siblings, 1 reply; 5+ messages in thread
From: Vijay Kilari @ 2017-03-28 15:08 UTC (permalink / raw)
To: Eric Auger
Cc: eric.auger.pro, Peter Maydell, qemu-arm, QEMU Developers, Kumar,
Vijaya, drjones
Hi Eric,
On Tue, Mar 28, 2017 at 7:28 PM, Eric Auger <eric.auger@redhat.com> wrote:
> KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS needs to be checked before
> attempting to read ICC_CTLR_EL1; otherwise kernel versions not
> exposing this kvm device group will be incompatible with qemu 2.9.
>
> Fixes: 07a5628 ("hw/intc/arm_gicv3_kvm: Reset GICv3 cpu interface registers")
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Reported-by: Prakash B <bjsprakash.linux@gmail.com>
>
> ---
>
> I understand the ICC_CTLR_EL1 state only is used in the put() function
> which is used for migration
> ---
> hw/intc/arm_gicv3_kvm.c | 17 +++++++++++------
> 1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
> index 81f0403..4c3a88e 100644
> --- a/hw/intc/arm_gicv3_kvm.c
> +++ b/hw/intc/arm_gicv3_kvm.c
> @@ -614,12 +614,6 @@ static void arm_gicv3_icc_reset(CPUARMState *env, const ARMCPRegInfo *ri)
> s = c->gic;
> cpu = ARM_CPU(c->cpu);
>
> - /* Initialize to actual HW supported configuration */
> - kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS,
> - KVM_VGIC_ATTR(ICC_CTLR_EL1, cpu->mp_affinity),
> - &c->icc_ctlr_el1[GICV3_NS], false);
> -
> - c->icc_ctlr_el1[GICV3_S] = c->icc_ctlr_el1[GICV3_NS];
> c->icc_pmr_el1 = 0;
> c->icc_bpr[GICV3_G0] = GIC_MIN_BPR;
> c->icc_bpr[GICV3_G1] = GIC_MIN_BPR;
> @@ -628,6 +622,17 @@ static void arm_gicv3_icc_reset(CPUARMState *env, const ARMCPRegInfo *ri)
> c->icc_sre_el1 = 0x7;
> memset(c->icc_apr, 0, sizeof(c->icc_apr));
> memset(c->icc_igrpen, 0, sizeof(c->icc_igrpen));
> +
> + if (!kvm_device_check_attr(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS,
> + KVM_VGIC_ATTR(ICC_CTLR_EL1, cpu->mp_affinity))) {
> + return;
> + }
Can't we use gicv3 migration blocker provided if it is set before this reset.
> + /* Initialize to actual HW supported configuration */
> + kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS,
> + KVM_VGIC_ATTR(ICC_CTLR_EL1, cpu->mp_affinity),
> + &c->icc_ctlr_el1[GICV3_NS], false);
> +
> + c->icc_ctlr_el1[GICV3_S] = c->icc_ctlr_el1[GICV3_NS];
> }
>
> static void kvm_arm_gicv3_reset(DeviceState *dev)
> --
> 2.5.5
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [Qemu-devel] [PATCH] hw/intc/arm_gicv3_kvm: Check KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS in reset
2017-03-28 15:08 ` Vijay Kilari
@ 2017-03-28 15:43 ` Auger Eric
0 siblings, 0 replies; 5+ messages in thread
From: Auger Eric @ 2017-03-28 15:43 UTC (permalink / raw)
To: Vijay Kilari
Cc: Peter Maydell, drjones, QEMU Developers, Kumar, Vijaya, qemu-arm,
eric.auger.pro
Hi Vijay,
On 28/03/2017 17:08, Vijay Kilari wrote:
> Hi Eric,
>
> On Tue, Mar 28, 2017 at 7:28 PM, Eric Auger <eric.auger@redhat.com> wrote:
>> KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS needs to be checked before
>> attempting to read ICC_CTLR_EL1; otherwise kernel versions not
>> exposing this kvm device group will be incompatible with qemu 2.9.
>>
>> Fixes: 07a5628 ("hw/intc/arm_gicv3_kvm: Reset GICv3 cpu interface registers")
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>> Reported-by: Prakash B <bjsprakash.linux@gmail.com>
>>
>> ---
>>
>> I understand the ICC_CTLR_EL1 state only is used in the put() function
>> which is used for migration
>> ---
>> hw/intc/arm_gicv3_kvm.c | 17 +++++++++++------
>> 1 file changed, 11 insertions(+), 6 deletions(-)
>>
>> diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c
>> index 81f0403..4c3a88e 100644
>> --- a/hw/intc/arm_gicv3_kvm.c
>> +++ b/hw/intc/arm_gicv3_kvm.c
>> @@ -614,12 +614,6 @@ static void arm_gicv3_icc_reset(CPUARMState *env, const ARMCPRegInfo *ri)
>> s = c->gic;
>> cpu = ARM_CPU(c->cpu);
>>
>> - /* Initialize to actual HW supported configuration */
>> - kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS,
>> - KVM_VGIC_ATTR(ICC_CTLR_EL1, cpu->mp_affinity),
>> - &c->icc_ctlr_el1[GICV3_NS], false);
>> -
>> - c->icc_ctlr_el1[GICV3_S] = c->icc_ctlr_el1[GICV3_NS];
>> c->icc_pmr_el1 = 0;
>> c->icc_bpr[GICV3_G0] = GIC_MIN_BPR;
>> c->icc_bpr[GICV3_G1] = GIC_MIN_BPR;
>> @@ -628,6 +622,17 @@ static void arm_gicv3_icc_reset(CPUARMState *env, const ARMCPRegInfo *ri)
>> c->icc_sre_el1 = 0x7;
>> memset(c->icc_apr, 0, sizeof(c->icc_apr));
>> memset(c->icc_igrpen, 0, sizeof(c->icc_igrpen));
>> +
>> + if (!kvm_device_check_attr(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS,
>> + KVM_VGIC_ATTR(ICC_CTLR_EL1, cpu->mp_affinity))) {
>> + return;
>> + }
>
> Can't we use gicv3 migration blocker provided if it is set before this reset.
yes we could. I hesitated but eventually found the relationship between
the blocker and this kvm device group access was maybe not obvious.
As preferred.
Thanks
Eric
>
>> + /* Initialize to actual HW supported configuration */
>> + kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS,
>> + KVM_VGIC_ATTR(ICC_CTLR_EL1, cpu->mp_affinity),
>> + &c->icc_ctlr_el1[GICV3_NS], false);
>> +
>> + c->icc_ctlr_el1[GICV3_S] = c->icc_ctlr_el1[GICV3_NS];
>> }
>>
>> static void kvm_arm_gicv3_reset(DeviceState *dev)
>> --
>> 2.5.5
>>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH] hw/intc/arm_gicv3_kvm: Check KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS in reset
2017-03-28 13:58 [Qemu-devel] [PATCH] hw/intc/arm_gicv3_kvm: Check KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS in reset Eric Auger
2017-03-28 14:02 ` Auger Eric
2017-03-28 15:08 ` Vijay Kilari
@ 2017-03-30 5:32 ` Prakash B
2 siblings, 0 replies; 5+ messages in thread
From: Prakash B @ 2017-03-30 5:32 UTC (permalink / raw)
To: Eric Auger
Cc: eric.auger.pro, Peter Maydell, qemu-arm, qemu-devel, drjones,
vijay.kilari, Vijaya.Kumar, bprakash, Prakash B
On Tue, Mar 28, 2017 at 7:28 PM, Eric Auger <eric.auger@redhat.com> wrote:
> KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS needs to be checked before
> attempting to read ICC_CTLR_EL1; otherwise kernel versions not
> exposing this kvm device group will be incompatible with qemu 2.9.
>
> Fixes: 07a5628 ("hw/intc/arm_gicv3_kvm: Reset GICv3 cpu interface registers")
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Reported-by: Prakash B <bjsprakash.linux@gmail.com>
>
> ---
>
Thank Eric,
Verified compatibility for host kernel versions 4.4, 4.10 and
4.11-rc3, guest booted fine with these.
Tested-by: Prakash, Brahmajyosyula <Brahmajyosyula.Prakash@cavium.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-03-30 5:32 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-28 13:58 [Qemu-devel] [PATCH] hw/intc/arm_gicv3_kvm: Check KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS in reset Eric Auger
2017-03-28 14:02 ` Auger Eric
2017-03-28 15:08 ` Vijay Kilari
2017-03-28 15:43 ` Auger Eric
2017-03-30 5:32 ` Prakash B
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.