hw/intc/arm_gicv3_kvm.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)
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
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) >
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 >
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 >> >
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>
© 2016 - 2024 Red Hat, Inc.