Re: [PATCH v5 19/49] KVM: arm64: gic-v5: Add KVM_VGIC_V5_ADDR_TYPE_IRS to UAPI

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The patch defines the UAPI constant KVM_VGIC_V5_ADDR_TYPE_IRS for userspace to specify the IRS location, but provides no corresponding kernel implementation to handle it.
- [Low] The newly added UAPI macro KVM_VGIC_V5_IRS_SIZE uses the kernel-internal macro SZ_64K, which is not exported to userspace and will cause compilation errors.
--

--- Patch [19]: [PATCH v5 19/49] KVM: arm64: gic-v5: Add KVM_VGIC_V5_ADDR_TYPE_IRS to UAPI ---
commit 2deebb3b97ce3e2f88f0cdf0452104edbd0a7ea6
Author: Sascha Bischoff <[email protected]>

KVM: arm64: gic-v5: Add KVM_VGIC_V5_ADDR_TYPE_IRS to UAPI

This patch adds the UAPI definitions for the GICv5 IRS address type and size 
to the arm64 KVM headers. It reserves 2x64kB in the GPA space for the IRS 
CONFIG_FRAME and SETLPI_FRAME.

> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index 1c13bfa2d38aa..d1b2ca317f586 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
[ ... ]
> @@ -97,6 +97,11 @@ struct kvm_regs {
>  #define KVM_VGIC_V3_REDIST_SIZE		(2 * SZ_64K)
>  #define KVM_VGIC_V3_ITS_SIZE		(2 * SZ_64K)
>  
> +/* Supported VGICv5 address types  */
> +#define KVM_VGIC_V5_ADDR_TYPE_IRS	6
> +

[Severity: High]
Does this patch provide the kernel implementation to handle
KVM_VGIC_V5_ADDR_TYPE_IRS?

If userspace attempts to use this UAPI definition with the
KVM_DEV_ARM_VGIC_GRP_ADDR attribute group, vgic_v5_set_attr() explicitly
returns -ENXIO for that group:

arch/arm64/kvm/vgic/vgic-kvm-device.c:vgic_v5_set_attr() {
	switch (attr->group) {
	case KVM_DEV_ARM_VGIC_GRP_ADDR:
	case KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS:
	case KVM_DEV_ARM_VGIC_GRP_NR_IRQS:
		return -ENXIO;
...
}

Similarly, kvm_vgic_addr() lacks a case for KVM_VGIC_V5_ADDR_TYPE_IRS and
would reject it with -ENODEV:

arch/arm64/kvm/vgic/vgic-kvm-device.c:kvm_vgic_addr() {
...
	default:
		r = -ENODEV;
	}
...
}

Without handling in the kernel, userspace cannot configure the base address
of the GICv5 IRS, which renders the GICv5 IRS emulation feature unusable.

> +#define KVM_VGIC_V5_IRS_SIZE		(2 * SZ_64K)
> +

[Severity: Low]
Does this newly added UAPI definition rely on the kernel-internal macro
SZ_64K?

Since <linux/sizes.h> is not exported to userspace, userspace programs
attempting to use KVM_VGIC_V5_IRS_SIZE will fail to compile with an
undeclared identifier error for SZ_64K.

While this pattern already exists in the file for GICv3 sizes, introducing a
new instance perpetuates the UAPI breakage.

>  #define KVM_ARM_VCPU_POWER_OFF		0 /* CPU is started in OFF state */
>  #define KVM_ARM_VCPU_EL1_32BIT		1 /* CPU running a 32bit VM */
>  #define KVM_ARM_VCPU_PSCI_0_2		2 /* CPU uses PSCI v0.2 */
> diff --git a/tools/arch/arm64/include/uapi/asm/kvm.h b/tools/arch/arm64/include/uapi/asm/kvm.h
> index 1c13bfa2d38aa..d1b2ca317f586 100644
> --- a/tools/arch/arm64/include/uapi/asm/kvm.h
> +++ b/tools/arch/arm64/include/uapi/asm/kvm.h
[ ... ]
> @@ -97,6 +97,11 @@ struct kvm_regs {
>  #define KVM_VGIC_V3_REDIST_SIZE		(2 * SZ_64K)
>  #define KVM_VGIC_V3_ITS_SIZE		(2 * SZ_64K)
>  
> +/* Supported VGICv5 address types  */
> +#define KVM_VGIC_V5_ADDR_TYPE_IRS	6
> +
> +#define KVM_VGIC_V5_IRS_SIZE		(2 * SZ_64K)
> +

[Severity: Low]
Are the same SZ_64K and missing implementation issues applicable to this
mirrored tools UAPI header?

>  #define KVM_ARM_VCPU_POWER_OFF		0 /* CPU is started in OFF state */
>  #define KVM_ARM_VCPU_EL1_32BIT		1 /* CPU running a 32bit VM */
>  #define KVM_ARM_VCPU_PSCI_0_2		2 /* CPU uses PSCI v0.2 */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=19