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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing hardware idle check before issuing VPE commands in `vgic_v5_irs_set_up_vpe()` causes state corruption on timeout recovery.
--

commit 0e86d7c766cc2f59e9782a84658b0e7ed678597a
Author: Sascha Bischoff <Sascha.Bischoff@arm.com>
Subject: KVM: arm64: gic-v5: Implement VPE IRS MMIO Ops

This commit introduces interfaces to validate and configure VPEs via the 
host's IRS MMIO interface. It also extends the VPE doorbell commands with 
VPE_MAKE_VALID.

> diff --git a/arch/arm64/kvm/vgic/vgic-v5.c b/arch/arm64/kvm/vgic/vgic-v5.c
> index fd64ea0173b2f..6d7d9e6ed9dbf 100644
> --- a/arch/arm64/kvm/vgic/vgic-v5.c
> +++ b/arch/arm64/kvm/vgic/vgic-v5.c

[ ... ]

> @@ -332,10 +344,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;
> +
> +	/* 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);
> +
> +	/* Wait for the VPE to be marked valid in the VPET */
> +	ret = vgic_v5_irs_wait_for_vm_op();
> +	if (ret)
> +		return ret;
> +
> +	selr = FIELD_PREP(GICV5_IRS_VPE_SELR_VPE_ID, vpe_id) |
> +	       FIELD_PREP(GICV5_IRS_VPE_SELR_VM_ID, vm_id) |
> +	       GICV5_IRS_VPE_SELR_S;
> +	irs_writeq_relaxed(selr, GICV5_IRS_VPE_SELR);

[Severity: High]
Is it possible to clobber a still-busy VPE engine here if a previous
operation timed out?

While vgic_v5_irs_set_up_vpe() checks vgic_v5_irs_wait_for_vm_op() at the
start of the function, there does not appear to be a hardware idle check for
the VPE operation unit (like vgic_v5_irs_wait_for_vpe_op()) before writing
to GICV5_IRS_VPE_SELR.

If an earlier call to vgic_v5_irs_set_up_vpe() timed out waiting for a VPE
operation and released global_irs_lock on the error path, a subsequent call
could find the VM operation unit idle but the VPE operation unit still busy.
Writing to GICV5_IRS_VPE_SELR in this state could corrupt the hardware state.

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