Re: [PATCH v4 34/48] KVM: arm64: gic-v5: Add VGICv5 IST save/restore UAPI

On Fri, 2026-07-24 at 11:46 +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 3 potential
> issue(s) to consider:
> - [Medium] The KVM implementation for the newly added VGICv5 IST
> attribute group is entirely missing.
> - [Low] The ARM-specific `struct kvm_vgic_v5_ist` is added to the
> cross-architecture `include/uapi/linux/kvm.h` instead of the
> architecture-specific `arch/arm64/include/uapi/asm/kvm.h`, polluting
> the global UAPI namespace and causing a tools header desync.
> - [Low] The new `KVM_DEV_ARM_VGIC_GRP_IST` device attribute group and
> its descriptor structure are completely undocumented.
> --
> 
> --- Patch [34]: [PATCH v4 34/48] KVM: arm64: gic-v5: Add VGICv5 IST
> save/restore UAPI ---
> commit 18a475f214feec673055a712a5b6d844aa9edb1a
> Author: Sascha Bischoff <[email protected]>
> 
> KVM: arm64: gic-v5: Add VGICv5 IST save/restore UAPI
>     
> Add the VGICv5 IST device attribute group used by userspace to save
> and restore the IRS Interrupt State Tables. It introduces the
> kvm_vgic_v5_ist descriptor structure for SPI and LPI IST buffers.
> 
> > diff --git a/arch/arm64/include/uapi/asm/kvm.h
> > b/arch/arm64/include/uapi/asm/kvm.h
> > index 710a0d267347d..1b9bbeab18a4e 100644
> > --- a/arch/arm64/include/uapi/asm/kvm.h
> > +++ b/arch/arm64/include/uapi/asm/kvm.h
> > @@ -423,6 +423,7 @@ enum {
> >  #define KVM_DEV_ARM_VGIC_GRP_ITS_REGS 8
> >  #define KVM_DEV_ARM_VGIC_GRP_MAINT_IRQ  9
> >  #define KVM_DEV_ARM_VGIC_GRP_IRS_REGS	10
> > +#define KVM_DEV_ARM_VGIC_GRP_IST	11
> 
> [Severity: Medium]
> The commit message states this attribute group is used by userspace
> to save
> and restore the IRS Interrupt State Tables. Is the backend
> implementation for
> this new UAPI intentionally omitted from this patch?

Yes. Coming in one of the next ones.

> 
> Without the corresponding KVM VGIC device ops in the codebase to
> parse the
> attribute, this UAPI currently appears unusable.
> 
> [Severity: Low]
> Is there a missing documentation update for this new guest-visible
> feature?
> 
> Kernel guidelines typically expect new UAPI additions to be
> explicitly
> documented. This patch omits updating the KVM device documentation to
> explain
> the usage semantics, expected alignment, and lifecycle constraints
> for the
> VGICv5 IST attribute.

This is quite extensively documented in an explicit documentation
patches for this specific UAPI.

> 
> >  #define KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT	10
> >  #define KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_MASK \
> >  			(0x3fffffULL <<
> > KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT)
> > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> > index 419011097fa8e..df6356fb45b08 100644
> > --- a/include/uapi/linux/kvm.h
> > +++ b/include/uapi/linux/kvm.h
> > @@ -1191,6 +1191,13 @@ struct kvm_device_attr {
> >  	__u64	addr;		/* userspace address of attr data
> > */
> >  };
> >  
> > +struct kvm_vgic_v5_ist {
> 
> [Severity: Low]
> Should this ARM-specific structure be placed in the architecture-
> specific
> header arch/arm64/include/uapi/asm/kvm.h instead of the global
> cross-architecture include/uapi/linux/kvm.h? 
> 
> Additionally, while the patch updates the tools copy of the arm64
> UAPI
> header, it appears to miss syncing the tools copy of the global
> header
> (tools/include/uapi/linux/kvm.h). Does this omission lead to a tools
> header
> desync warning?

Ah, yes. I did put that into the wrong header. Have moved it to the
arm-specific header.

> 
> > +	__u64	spi_ist_addr;
> > +	__u64	spi_ist_size;
> > +	__u64	lpi_ist_addr;
> > +	__u64	lpi_ist_size;
> > +};
> > +
> >  #define  KVM_DEV_VFIO_FILE			1
> 
> [ ... ]
> 
Thanks,
Sascha