| Message ID | 1487850673-26455-6-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> > > Reset CPU interface registers of GICv3 when CPU is reset. > For this, ARMCPRegInfo struct is registered with one ICC > register whose resetfn is called when cpu is reset. > > All the ICC registers are reset under one single register > reset function instead of calling resetfn for each ICC > register. > > Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com> > --- > hw/intc/arm_gicv3_kvm.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 60 insertions(+) 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> > > Reset CPU interface registers of GICv3 when CPU is reset. > For this, ARMCPRegInfo struct is registered with one ICC > register whose resetfn is called when cpu is reset. > > All the ICC registers are reset under one single register > reset function instead of calling resetfn for each ICC > register. > > Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com> Reviewed-by: Eric Auger <eric.auger@redhat.com> Eric > --- > hw/intc/arm_gicv3_kvm.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 60 insertions(+) > > diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c > index cda1af4..81f0403 100644 > --- a/hw/intc/arm_gicv3_kvm.c > +++ b/hw/intc/arm_gicv3_kvm.c > @@ -604,6 +604,32 @@ static void kvm_arm_gicv3_get(GICv3State *s) > } > } > > +static void arm_gicv3_icc_reset(CPUARMState *env, const ARMCPRegInfo *ri) > +{ > + ARMCPU *cpu; > + GICv3State *s; > + GICv3CPUState *c; > + > + c = (GICv3CPUState *)env->gicv3state; > + 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; > + c->icc_bpr[GICV3_G1NS] = GIC_MIN_BPR; > + > + c->icc_sre_el1 = 0x7; > + memset(c->icc_apr, 0, sizeof(c->icc_apr)); > + memset(c->icc_igrpen, 0, sizeof(c->icc_igrpen)); > +} > + > static void kvm_arm_gicv3_reset(DeviceState *dev) > { > GICv3State *s = ARM_GICV3_COMMON(dev); > @@ -621,6 +647,34 @@ static void kvm_arm_gicv3_reset(DeviceState *dev) > kvm_arm_gicv3_put(s); > } > > +/* > + * CPU interface registers of GIC needs to be reset on CPU reset. > + * For the calling arm_gicv3_icc_reset() on CPU reset, we register > + * below ARMCPRegInfo. As we reset the whole cpu interface under single > + * register reset, we define only one register of CPU interface instead > + * of defining all the registers. > + */ > +static const ARMCPRegInfo gicv3_cpuif_reginfo[] = { > + { .name = "ICC_CTLR_EL1", .state = ARM_CP_STATE_BOTH, > + .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 12, .opc2 = 4, > + /* > + * If ARM_CP_NOP is used, resetfn is not called, > + * So ARM_CP_NO_RAW is appropriate type. > + */ > + .type = ARM_CP_NO_RAW, > + .access = PL1_RW, > + .readfn = arm_cp_read_zero, > + .writefn = arm_cp_write_ignore, > + /* > + * We hang the whole cpu interface reset routine off here > + * rather than parcelling it out into one little function > + * per register > + */ > + .resetfn = arm_gicv3_icc_reset, > + }, > + REGINFO_SENTINEL > +}; > + > static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp) > { > GICv3State *s = KVM_ARM_GICV3(dev); > @@ -644,6 +698,12 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp) > > gicv3_init_irqs_and_mmio(s, kvm_arm_gicv3_set_irq, NULL); > > + for (i = 0; i < s->num_cpu; i++) { > + ARMCPU *cpu = ARM_CPU(qemu_get_cpu(i)); > + > + define_arm_cp_regs(cpu, gicv3_cpuif_reginfo); > + } > + > /* Try to create the device via the device control API */ > s->dev_fd = kvm_create_device(kvm_state, KVM_DEV_TYPE_ARM_VGIC_V3, false); > if (s->dev_fd < 0) { >
On 02/23/2017 07:37 PM, Peter Maydell wrote: > On 23 February 2017 at 11:51, <vijay.kilari@gmail.com> wrote: >> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com> >> >> Reset CPU interface registers of GICv3 when CPU is reset. >> For this, ARMCPRegInfo struct is registered with one ICC >> register whose resetfn is called when cpu is reset. >> >> All the ICC registers are reset under one single register >> reset function instead of calling resetfn for each ICC >> register. >> >> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com> >> --- >> hw/intc/arm_gicv3_kvm.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 60 insertions(+) > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> This patch breaks execution on 4.4 (SLES12 SP2) for me: $ ./aarch64-softmmu/qemu-system-aarch64 -nographic -M virt,gic-version=host -enable-kvm -cpu host -kernel /boot/Image qemu-system-aarch64: KVM_GET_DEVICE_ATTR failed: No such device or address Group 6 attr 0x000000000000c664 Aborted (core dumped) If I revert it, it works again. Is that device attr something only newer kernels received? In that case, it needs to be guarded by a capability check. Alex
Hi Alex, On 28/03/2017 19:24, Alexander Graf wrote: > On 02/23/2017 07:37 PM, Peter Maydell wrote: >> On 23 February 2017 at 11:51, <vijay.kilari@gmail.com> wrote: >>> From: Vijaya Kumar K <Vijaya.Kumar@cavium.com> >>> >>> Reset CPU interface registers of GICv3 when CPU is reset. >>> For this, ARMCPRegInfo struct is registered with one ICC >>> register whose resetfn is called when cpu is reset. >>> >>> All the ICC registers are reset under one single register >>> reset function instead of calling resetfn for each ICC >>> register. >>> >>> Signed-off-by: Vijaya Kumar K <Vijaya.Kumar@cavium.com> >>> --- >>> hw/intc/arm_gicv3_kvm.c | 60 >>> +++++++++++++++++++++++++++++++++++++++++++++++++ >>> 1 file changed, 60 insertions(+) >> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> > > This patch breaks execution on 4.4 (SLES12 SP2) for me: > > $ ./aarch64-softmmu/qemu-system-aarch64 -nographic -M > virt,gic-version=host -enable-kvm -cpu host -kernel /boot/Image > qemu-system-aarch64: KVM_GET_DEVICE_ATTR failed: No such device or address > Group 6 attr 0x000000000000c664 > Aborted (core dumped) > > If I revert it, it works again. Is that device attr something only newer > kernels received? In that case, it needs to be guarded by a capability > check. The patch just posted should fix it: [PATCH v2] hw/intc/arm_gicv3_kvm: Check KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS in reset Thanks Eric > > > Alex >
diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c index cda1af4..81f0403 100644 --- a/hw/intc/arm_gicv3_kvm.c +++ b/hw/intc/arm_gicv3_kvm.c @@ -604,6 +604,32 @@ static void kvm_arm_gicv3_get(GICv3State *s) } } +static void arm_gicv3_icc_reset(CPUARMState *env, const ARMCPRegInfo *ri) +{ + ARMCPU *cpu; + GICv3State *s; + GICv3CPUState *c; + + c = (GICv3CPUState *)env->gicv3state; + 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; + c->icc_bpr[GICV3_G1NS] = GIC_MIN_BPR; + + c->icc_sre_el1 = 0x7; + memset(c->icc_apr, 0, sizeof(c->icc_apr)); + memset(c->icc_igrpen, 0, sizeof(c->icc_igrpen)); +} + static void kvm_arm_gicv3_reset(DeviceState *dev) { GICv3State *s = ARM_GICV3_COMMON(dev); @@ -621,6 +647,34 @@ static void kvm_arm_gicv3_reset(DeviceState *dev) kvm_arm_gicv3_put(s); } +/* + * CPU interface registers of GIC needs to be reset on CPU reset. + * For the calling arm_gicv3_icc_reset() on CPU reset, we register + * below ARMCPRegInfo. As we reset the whole cpu interface under single + * register reset, we define only one register of CPU interface instead + * of defining all the registers. + */ +static const ARMCPRegInfo gicv3_cpuif_reginfo[] = { + { .name = "ICC_CTLR_EL1", .state = ARM_CP_STATE_BOTH, + .opc0 = 3, .opc1 = 0, .crn = 12, .crm = 12, .opc2 = 4, + /* + * If ARM_CP_NOP is used, resetfn is not called, + * So ARM_CP_NO_RAW is appropriate type. + */ + .type = ARM_CP_NO_RAW, + .access = PL1_RW, + .readfn = arm_cp_read_zero, + .writefn = arm_cp_write_ignore, + /* + * We hang the whole cpu interface reset routine off here + * rather than parcelling it out into one little function + * per register + */ + .resetfn = arm_gicv3_icc_reset, + }, + REGINFO_SENTINEL +}; + static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp) { GICv3State *s = KVM_ARM_GICV3(dev); @@ -644,6 +698,12 @@ static void kvm_arm_gicv3_realize(DeviceState *dev, Error **errp) gicv3_init_irqs_and_mmio(s, kvm_arm_gicv3_set_irq, NULL); + for (i = 0; i < s->num_cpu; i++) { + ARMCPU *cpu = ARM_CPU(qemu_get_cpu(i)); + + define_arm_cp_regs(cpu, gicv3_cpuif_reginfo); + } + /* Try to create the device via the device control API */ s->dev_fd = kvm_create_device(kvm_state, KVM_DEV_TYPE_ARM_VGIC_V3, false); if (s->dev_fd < 0) {