Re: [PATCH v5 22/49] KVM: arm64: gic-v5: Register the IRS IODEV

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] GICv5 SPI injection via IRQFD and KVM_IRQ_ROUTING_IRQCHIP is broken due to missing UAPI INTID translation and incomplete vgic_get_irq() support.
- [High] PMU IRQ configuration is broken for GICv5 VMs due to missing UAPI INTID translation.
--

--- Patch [22]: [PATCH v5 22/49] KVM: arm64: gic-v5: Register the IRS IODEV ---
commit 1f7c421def2656a75d9f4c6c3c9bc8b90309dbb8
Author: Sascha Bischoff <[email protected]>
Subject: KVM: arm64: gic-v5: Register the IRS IODEV

This commit registers the GICv5 IRS IODEV during vgic_map_resources. It
also exposes the IRS address configuration to userspace and allows setting
the number of SPIs handled by the GICv5 implementation.

> diff --git a/arch/arm64/kvm/vgic/vgic-kvm-device.c b/arch/arm64/kvm/vgic/vgic-kvm-device.c
> index 90be99443df3b..8c2c6886af4e3 100644
> --- a/arch/arm64/kvm/vgic/vgic-kvm-device.c
> +++ b/arch/arm64/kvm/vgic/vgic-kvm-device.c

[ ... ]

> @@ -224,31 +232,48 @@ static int vgic_set_common_attr(struct kvm_device *dev,

[ ... ]

> +		} else {
> +			/*
> +			 * GICv5 reports a number of SPIs, not a total number of
> +			 * interrupts. Require a multiple of 32 SPIs.
> +			 */
> +			if (val < VGIC_V5_DEFAULT_NR_SPIS ||
> +			    val > VGIC_V5_MAX_NR_SPIS ||
> +			    (val & 31))
> +				return -EINVAL;
> +
> +			mutex_lock(&dev->kvm->arch.config_lock);
> +			if (vgic_initialized(dev->kvm) || dev->kvm->arch.vgic.nr_spis)
> +				ret = -EBUSY;
> +			else
> +				dev->kvm->arch.vgic.nr_spis = val;
> +			mutex_unlock(&dev->kvm->arch.config_lock);
> +		}
>  
>  		return ret;
>  	}

[Severity: High]
Does this leave PMU IRQ configuration broken for GICv5 VMs?

Since the UAPI INTID lacks the GICv5 internal hardware type bits, the
PMU configuration fails unconditionally during validation. When userspace
configures the PMU IRQ using the KVM_ARM_VCPU_PMU_V3_IRQ attribute (passing
a standard PPI), the check fails:

arch/arm64/kvm/pmu-emul.c:kvm_arm_pmu_v3_set_attr() {
    ...
    if (!(irq_is_ppi(vcpu->kvm, irq) || irq_is_spi(vcpu->kvm, irq)))
        return -EINVAL;
    ...
}

Because irq_is_ppi() and irq_is_spi() require the GICV5_HWIRQ_TYPE bits
that are missing from the raw integer, will this cause PMU setup to fail
with -EINVAL for GICv5 VMs?

> diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
> index 59c6e6e766166..e03e820e43ff0 100644
> --- a/include/kvm/arm_vgic.h
> +++ b/include/kvm/arm_vgic.h
> @@ -31,7 +31,12 @@
>  #define VGIC_MAX_SPI		1019
>  #define VGIC_MAX_RESERVED	1023
>  #define VGIC_MIN_LPI		8192
> -#define KVM_IRQCHIP_NUM_PINS	(1020 - 32)
> +
> +/*
> + * The common IRQ routing table must accommodate all GIC types. GICv2 and
> + * GICv3 need 988 pins, while GICv5 needs 1024 pins.
> + */
> +#define KVM_IRQCHIP_NUM_PINS	1024

[Severity: High]
Does expanding the IRQ routing table to 1024 pins expose a gap in GICv5
SPI injection support?

While this expands the pins for GICv5, the underlying injection mechanisms
do not translate the UAPI INTID. For example, in vgic_irqfd_set_irq():

arch/arm64/kvm/vgic/vgic-irqfd.c:vgic_irqfd_set_irq() {
    ...
    unsigned int spi_id = e->irqchip.pin + VGIC_NR_PRIVATE_IRQS;
    ...
    if (!vgic_valid_spi(kvm, spi_id))
        return -EINVAL;
    ...
}

This calculates spi_id without the required GICV5_HWIRQ_TYPE_SPI bits,
causing vgic_valid_spi() to fail for GICv5.

Additionally, vgic_get_irq() hardcodes a NULL return for GICv5 SPIs,
which blocks injection entirely:

arch/arm64/kvm/vgic/vgic.c:vgic_get_irq() {
    ...
    if (vgic_is_v5(kvm))
        return NULL;
    ...
}

Will this prevent guests using device pass-through or virtio devices
that rely on SPIs from receiving interrupts via irqfd/eventfd?

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