Re: [PATCH v6 13/49] 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:
- [Medium] Missing defensive idle check for VPE operations before initiating VPE commands.
--

commit 81503b7ea1c4c11dba3bab36283047725b89c0a9
Author: Sascha Bischoff <Sascha.Bischoff@arm.com>

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

This commit introduces interfaces to make VPEs valid and configures them
via the host's IRS interface. It extends the existing VPE-doorbell-based
commands with VPE_MAKE_VALID.

> diff --git a/arch/arm64/kvm/vgic/vgic-v5.c b/arch/arm64/kvm/vgic/vgic-v5.c
> --- a/arch/arm64/kvm/vgic/vgic-v5.c
> +++ b/arch/arm64/kvm/vgic/vgic-v5.c

[ ... ]

> @@ -308,10 +320,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;
> +
> +	lockdep_assert_held(&vgic_v5_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: Medium]
Is a defensive idle check missing before we initiate this VPE command?

This function ensures the VM interface is idle at the top using
vgic_v5_irs_wait_for_vm_op(), but it does not do the same for the VPE
interface before writing to GICV5_IRS_VPE_SELR.

If a prior VPE operation timed out (for example, if
vgic_v5_irs_wait_for_vpe_op() failed in a previous call), the function
returns and vgic_v5_irs_lock is released. A subsequent call to
vgic_v5_irs_set_up_vpe() would then write to GICV5_IRS_VPE_SELR while the
hardware interface is still busy.

Writing to the VPE interface while it is active could corrupt the state
machine or result in unpredictable behavior. Should there be a preceding
check that the vpe_op hardware interface is actually idle?

> +
> +	ret = vgic_v5_irs_wait_for_vpe_op();
> +	if (ret)
> +		return ret;

[ ... ]

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