Re: [PATCH v4 37/48] Documentation: KVM: Add KVM_DEV_ARM_VGIC_GRP_IRS_REGS to VGICv5 docs

On Fri, 2026-07-24 at 12:19 +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider:
> 
> New issues:
> - [Low] The documentation for KVM_DEV_ARM_VGIC_GRP_IRS_REGS omits
> that writes will return -EBUSY if a VCPU has run.
> 
> Pre-existing issues:
> - [High] SPI configuration (edge/level) is permanently lost during VM
> migration because KVM ignores userspace writes to
> `GICV5_IRS_SPI_CFGR` and fails to serialize the configuration into
> the Interrupt State Table (IST).
> --
> 
> --- Patch [37]: [PATCH v4 37/48] Documentation: KVM: Add
> KVM_DEV_ARM_VGIC_GRP_IRS_REGS to VGICv5 docs ---
> commit 6c9776f9e0496eb6c3e26fe3876764cc1148cc8a
> Author: Sascha Bischoff <[email protected]>
> 
> Documentation: KVM: Add KVM_DEV_ARM_VGIC_GRP_IRS_REGS to VGICv5 docs
> 
> Document the KVM_DEV_ARM_VGIC_GRP_IRS_REGS attribute group used to
> read and write the virtual IRS's MMIO register state. This provides a
> GICv5-specific interface for state that is conceptually similar to
> the
> VGICv3 ITS register interface, but uses IRS terminology instead of
> ITS.
> 
> > diff --git a/Documentation/virt/kvm/devices/arm-vgic-v5.rst
> > b/Documentation/virt/kvm/devices/arm-vgic-v5.rst
> > index 50d8030e2c804..c411d2195c6a0 100644
> > --- a/Documentation/virt/kvm/devices/arm-vgic-v5.rst
> > +++ b/Documentation/virt/kvm/devices/arm-vgic-v5.rst
> > @@ -149,3 +149,41 @@ Groups:
> 
> [ ... ]
> 
> > +      For registers without dedicated userspace accessors, getting
> > or setting a
> > +      register uses the same emulated MMIO handlers as guest
> > reads/writes.
> > +      Dedicated userspace accessors may instead save or restore
> > migration state
> > +      without triggering guest-visible side effects. For example,
> > restoring
> > +      IRS_IST_BASER only restores the emulated register state; any
> > host LPI IST
> > +      allocation based on the restored IRS_IST_CFGR and
> > IRS_IST_BASER state
> > +      happens when KVM_DEV_ARM_VGIC_GRP_IST is restored.
> 
> [Severity: High]
> This is a pre-existing issue, but does VM migration permanently lose
> the
> edge/level configuration for all SPIs because of how
> GICV5_IRS_SPI_CFGR is
> handled?
> 
> The userspace write accessor explicitly ignores writes to the SPI
> configuration register, making manual restoration impossible:

Nope. We restore the ISTs which explicitly contain this information. We
extract it from them as part of restoring them.

> 
> arch/arm64/kvm/vgic/vgic-irs-v5.c:vgic_v5_mmio_uaccess_write_irs() {
>     ...
> 	case GICV5_IRS_SPI_CFGR:
> 		break;
>     ...
> }
> 
> Additionally, the IST is copied to userspace directly from hardware
> memory
> without synthesizing KVM's software-tracked irq->config into the HM
> bit:

Because that should already be present!

> 
> arch/arm64/kvm/vgic/vgic-v5-tables.c:vgic_v5_save_linear_ist() {
>     ...
> 		h_iste = READ_ONCE(*h_iste_addr);
> 		ret = put_user(h_iste, uaddr);
>     ...
> }
> 
> Since the configuration is lost during the IST save and cannot be
> restored
> via GICV5_IRS_SPI_CFGR, are level-triggered interrupts restored
> incorrectly
> as edge-triggered?

No.

> 
> > +
> > +  Errors:
> > +
> > +    ======= 
> > =================================================================
> > +    -ENXIO   Offset does not correspond to any supported register
> > +    -EFAULT  Invalid user pointer for attr->addr
> > +    -EINVAL  Offset is not 32-bit aligned for 32-bit MMIO
> > registers, not
> > +             64-bit aligned for 64-bit registers, or the supplied
> > register
> > +             value is not compatible with the configured VGICv5
> > IRS state
> > +    -EBUSY   VGIC is not initialized, or one or more VCPUs are
> > running
> > +    ======= 
> > =================================================================
> 
> [Severity: Low]
> Does this error description miss the case where a VCPU has already
> run?
> 
> Looking at vgic_v5_attr_regs_access() in arch/arm64/kvm/vgic/vgic-
> kvm-device.c:
> 
> 	if (is_write && vgic_v5_vm_has_run_once(dev->kvm)) {
> 		ret = -EBUSY;
> 		goto out;
> 	}
> 
> Should the documentation also mention that writes are rejected with -
> EBUSY
> if attempted after a VCPU has run?
> 

I've added that case into the docs.

Thanks,
Sascha