| Message ID | 1490709513-7180-1-git-send-email-eric.auger@redhat.com |
|---|---|
| State | New |
| Headers | show |
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>
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)
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(-)