Hi Peter,
> From: [email protected] <qemu-
> [email protected]> On Behalf Of Peter
> Maydell
> Sent: Tuesday, October 14, 2025 11:25 AM
> To: [email protected]
>
> Currently in arm_gicv3_icc_reset() we read the kernel's value of
> ICC_CTLR_EL1 as part of resetting the CPU interface. This mostly works, but
> we're actually breaking an assumption the kernel makes that userspace only
> accesses the in-kernel GIC data when the VM is totally paused, which may
> not be the case if a single vCPU is being reset. The effect is that it's
> possible
> that the read attempt returns EBUSY.
>
> Avoid this by reading the kernel's value of the reset ICC_CTLR_EL1 once in
> device realize. This brings ICC_CTLR_EL1 into line with the other cpuif
> registers, where we assume we know what the kernel is resetting them to
> and just update QEMU's data structures in arm_gicv3_icc_reset().
>
> Signed-off-by: Peter Maydell <[email protected]>
> ---
> I've only tested this fairly lightly, but it seems to work.
> Salil, does this fix the EBUSY issues you were seeing ?
Let me try this and get back to you. Also, just to let you know that -EBUSY
can return from other places as well. Please check my reply in the other
mail-chain.
Many thanks!
Best regards
Salil.
>
> include/hw/intc/arm_gicv3_common.h | 3 ++
> hw/intc/arm_gicv3_kvm.c | 49 +++++++++++++++++++++---------
> 2 files changed, 38 insertions(+), 14 deletions(-)
>
> diff --git a/include/hw/intc/arm_gicv3_common.h
> b/include/hw/intc/arm_gicv3_common.h
> index 38aa1961c50..61d51915e07 100644
> --- a/include/hw/intc/arm_gicv3_common.h
> +++ b/include/hw/intc/arm_gicv3_common.h
> @@ -166,6 +166,9 @@ struct GICv3CPUState {
> uint64_t icc_igrpen[3];
> uint64_t icc_ctlr_el3;
>
> + /* For KVM, cached copy of the kernel reset value of ICC_CTLR_EL1 */
> + uint64_t kvm_reset_icc_ctlr_el1;
> +
> /* Virtualization control interface */
> uint64_t ich_apr[3][4]; /* ich_apr[GICV3_G1][x] never used */
> uint64_t ich_hcr_el2;
> diff --git a/hw/intc/arm_gicv3_kvm.c b/hw/intc/arm_gicv3_kvm.c index
> 9829e2146da..b95e6ea057a 100644
> --- a/hw/intc/arm_gicv3_kvm.c
> +++ b/hw/intc/arm_gicv3_kvm.c
> @@ -666,11 +666,24 @@ static void kvm_arm_gicv3_get(GICv3State *s)
>
> static void arm_gicv3_icc_reset(CPUARMState *env, const ARMCPRegInfo
> *ri) {
> - GICv3State *s;
> - GICv3CPUState *c;
> + GICv3CPUState *c = (GICv3CPUState *)env->gicv3state;
>
> - c = (GICv3CPUState *)env->gicv3state;
> - s = c->gic;
> + /*
> + * This function is called when each vcpu resets. The kernel
> + * API for the GIC assumes that it is only to be used when the
> + * whole VM is paused, so if we attempt to read the kernel's
> + * reset values here we might get EBUSY failures.
> + * So instead we assume we know what the kernel's reset values
> + * are (mostly zeroes) and only update the QEMU state struct
> + * fields. The exception is that we do need to know the kernel's
> + * idea of the ICC_CTLR_EL1 reset value, so we cache that at
> + * device realize time.
> + *
> + * This makes these sysregs different from the usual CPU ones,
> + * which can be validly read and written when only the single
> + * vcpu they apply to is paused, and where (in target/arm code)
> + * we read the reset values out of the kernel on every reset.
> + */
>
> c->icc_pmr_el1 = 0;
> /*
> @@ -691,16 +704,8 @@ static void arm_gicv3_icc_reset(CPUARMState *env,
> const ARMCPRegInfo *ri)
> memset(c->icc_apr, 0, sizeof(c->icc_apr));
> memset(c->icc_igrpen, 0, sizeof(c->icc_igrpen));
>
> - if (s->migration_blocker) {
> - 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, c->gicr_typer),
> - &c->icc_ctlr_el1[GICV3_NS], false, &error_abort);
> -
> - c->icc_ctlr_el1[GICV3_S] = c->icc_ctlr_el1[GICV3_NS];
> + c->icc_ctlr_el1[GICV3_NS] = c->kvm_reset_icc_ctlr_el1;
> + c->icc_ctlr_el1[GICV3_S] = c->kvm_reset_icc_ctlr_el1;
> }
>
> static void kvm_arm_gicv3_reset_hold(Object *obj, ResetType type) @@ -
> 939,6 +944,22 @@ static void kvm_arm_gicv3_realize(DeviceState *dev,
> Error **errp)
> kvm_arm_gicv3_notifier,
> MIG_MODE_CPR_TRANSFER);
> }
> +
> + /*
> + * Now we can read the kernel's initial value of ICC_CTLR_EL1, which
> + * we will need if a CPU interface is reset. If the kernel is ancient
> + * and doesn't support writing the GIC state then we don't need to
> + * care what reset does to QEMU's data structures.
> + */
> + if (!s->migration_blocker) {
> + for (i = 0; i < s->num_cpu; i++) {
> + GICv3CPUState *c = &s->cpu[i];
> +
> + kvm_device_access(s->dev_fd,
> KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS,
> + KVM_VGIC_ATTR(ICC_CTLR_EL1, c->gicr_typer),
> + &c->kvm_reset_icc_ctlr_el1, false,
> &error_abort);
> + }
> + }
> }
>
> static void kvm_arm_gicv3_class_init(ObjectClass *klass, const void *data)
> --
> 2.43.0
>
