Re: [Qemu-devel] [PATCH v9 5/5] hw/intc/arm_gicv3_kvm: Reset GICv3 cpu interface registers

[Auger Eric]

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,   wrote:
>>> From: Vijaya Kumar K 
>>>
>>> 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 
>>> ---
>>>   hw/intc/arm_gicv3_kvm.c | 60
>>> +
>>>   1 file changed, 60 insertions(+)
>> Reviewed-by: Peter Maydell 
> 
> 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 0xc664
> 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
> 

Re: [Qemu-devel] [PATCH v9 5/5] hw/intc/arm_gicv3_kvm: Reset GICv3 cpu interface registers

[Alexander Graf]

On 02/23/2017 07:37 PM, Peter Maydell wrote:

On 23 February 2017 at 11:51,   wrote:

From: Vijaya Kumar K 

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 
---
  hw/intc/arm_gicv3_kvm.c | 60 +
  1 file changed, 60 insertions(+)

Reviewed-by: Peter Maydell 


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 0xc664
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

Re: [Qemu-devel] [PATCH v9 5/5] hw/intc/arm_gicv3_kvm: Reset GICv3 cpu interface registers

[Auger Eric]

Hi

On 23/02/2017 12:51, vijay.kil...@gmail.com wrote:
> From: Vijaya Kumar K 
> 
> 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 
Reviewed-by: Eric Auger 

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),
> +  >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) {
> 

Re: [Qemu-devel] [PATCH v9 5/5] hw/intc/arm_gicv3_kvm: Reset GICv3 cpu interface registers

[Peter Maydell]

On 23 February 2017 at 11:51,   wrote:
> From: Vijaya Kumar K 
>
> 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 
> ---
>  hw/intc/arm_gicv3_kvm.c | 60 
> +
>  1 file changed, 60 insertions(+)

Reviewed-by: Peter Maydell 

thanks
-- PMM

[Qemu-devel] [PATCH v9 5/5] hw/intc/arm_gicv3_kvm: Reset GICv3 cpu interface registers

[vijay . kilari]

From: Vijaya Kumar K 

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 
---
 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),
+  >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) {
-- 
1.9.1