Re: [PATCH v4 13/48] KVM: arm64: gic-v5: Implement VPE IRS MMIO Ops

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing REQ_DB (Request Doorbell) flag when configuring the VPE doorbell, contradicting the explicit intent to force doorbell generation.
- [High] Missing VPE engine idle check at the beginning of `vgic_v5_irs_set_up_vpe`, risking command corruption if the engine is busy.
--

--- Patch [13]: [PATCH v4 13/48] KVM: arm64: gic-v5: Implement VPE IRS MMIO Ops ---
commit 92182082105efc7200f554f8b53732d80e3ea711
Author: Sascha Bischoff <Sascha.Bischoff@arm.com>

KVM: arm64: gic-v5: Implement VPE IRS MMIO Ops

This commit introduces interfaces to validate and configure VPEs via the
host's IRS. It extends the existing doorbell-based commands by adding
VPE_MAKE_VALID.

> diff --git a/arch/arm64/kvm/vgic/vgic-v5.c b/arch/arm64/kvm/vgic/vgic-v5.c
> index 21252bd11d96f..826c2284a1453 100644
> --- a/arch/arm64/kvm/vgic/vgic-v5.c
> +++ b/arch/arm64/kvm/vgic/vgic-v5.c

[ ... ]

> @@ -333,10 +345,73 @@ static int vgic_v5_irs_set_vist_invalid(u16 vm_id, bool spi_ist)
>  	return __vgic_v5_irs_update_vist_validity(vm_id, spi_ist, true);
>  }
>  
> +static int vgic_v5_irs_set_up_vpe(u16 vm_id, u16 vpe_id,
> +				  irq_hw_number_t db_hwirq)
> +{
> +	u64 vmap_vper, dbr, selr;
> +	u32 statusr, cr0;
> +	int ret;
> +
> +	guard(raw_spinlock_irqsave)(&global_irs_lock);
> +
> +	/* Make sure that we are idle to begin with */
> +	ret = vgic_v5_irs_wait_for_vm_op();
> +	if (ret)
> +		return ret;

[Severity: High]
Does this code also need to verify that the VPE engine is idle?

If a previous operation timed out and returned an error, the function
could exit early and leave the hardware engine busy. When called again
for the next VPE, the VM engine check above would pass, but writing to
GICV5_IRS_VPE_SELR below could corrupt the command if the VPE engine is
still busy.

> +
> +	/* Mark the VPE as valid */
> +	vmap_vper = FIELD_PREP(GICV5_IRS_VMAP_VPER_VPE_ID, vpe_id) |
> +		    FIELD_PREP(GICV5_IRS_VMAP_VPER_VM_ID, vm_id) |
> +		    GICV5_IRS_VMAP_VPER_M;
> +	irs_writeq_relaxed(vmap_vper, GICV5_IRS_VMAP_VPER);

[ ... ]

> +	/*
> +	 * The VPE has not yet run. Therefore, make sure that all interrupts
> +	 * will generate a doorbell.
> +	 */
> +	dbr = FIELD_PREP(GICV5_IRS_VPE_DBR_INTID, db_hwirq) |
> +	      GICV5_IRS_VPE_DBR_DBV;
> +	irs_writeq_relaxed(dbr, GICV5_IRS_VPE_DBR);

[Severity: High]
Is the GICV5_IRS_VPE_DBR_REQ_DB flag missing here?

The comment explicitly states the intent is to ensure all interrupts will
generate a doorbell. Without the request doorbell flag, could the GIC hardware
fail to generate doorbells for interrupts that become pending before the VPE
first runs?

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