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

[sashiko-bot@kernel.org]

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 <Sascha.Bischoff@arm.com>

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?

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.

>  #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?

> +	__u64	spi_ist_addr;
> +	__u64	spi_ist_size;
> +	__u64	lpi_ist_addr;
> +	__u64	lpi_ist_size;
> +};
> +
>  #define  KVM_DEV_VFIO_FILE			1

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724104819.1296803-1-sascha.bischoff@arm.com?part=34