Re: [PATCH v4 46/48] KVM: selftests: Add VGICv5 SPI injection tests

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Writes to GICv5 control-plane system registers are missing the mandatory immediate isb() synchronization.
--

--- Patch [46]: [PATCH v4 46/48] KVM: selftests: Add VGICv5 SPI injection tests ---
commit 194658b92777c0e76e825f04069b3e506ee164f3
Author: Sascha Bischoff <Sascha.Bischoff@arm.com>

KVM: selftests: Add VGICv5 SPI injection tests

Add VGICv5 SPI delivery coverage through KVM_IRQ_LINE. SPIs are
configured through the virtual IRS and delivered through the GICv5 CPU
interface, so the test needs to exercise both the userspace injection
path and the guest-side IRS programming path.

> diff --git a/tools/testing/selftests/kvm/arm64/vgic_v5.c b/tools/testing/selftests/kvm/arm64/vgic_v5.c
> index 22ae726b17be8..cec994a77c993 100644
> --- a/tools/testing/selftests/kvm/arm64/vgic_v5.c
> +++ b/tools/testing/selftests/kvm/arm64/vgic_v5.c
[ ... ]
> @@ -41,7 +44,71 @@ static struct kvm_vgic_v5_ist vgic_v5_ist_attr(void *spi_ist, size_t spi_size,
>  	};
>  }
>  
> +static void gicv5_setup_and_enable_hwirq(u64 hwirq, u32 target_vpe)
> +{
> +	u64 val;
> +
> +	val = hwirq | FIELD_PREP(GICV5_GIC_CDPRI_PRIORITY_MASK,
> +				 GICV5_IRQ_DEFAULT_PRI);
> +	gic_insn(val, CDPRI);
> +
> +	val = hwirq | FIELD_PREP(GICV5_GIC_CDAFF_IAFFID_MASK, target_vpe);
> +	gic_insn(val, CDAFF);
> +
> +	gic_insn(hwirq, CDEN);

[Severity: Medium]
Are isb() barriers required after these control-plane system register writes?

Unless these specific GICv5 CD registers are architecturally defined as
self-synchronizing, ARM64 requires an immediate isb() after every
control-plane system register write to ensure the CPU pipeline observes
the new configuration state.

> +}
[ ... ]
> +static void guest_spi_irq_handler(struct ex_regs *regs)
> +{
> +	bool valid;
> +	u32 hwirq;
> +	u64 ia;
> +
> +	ia = gicr_insn(CDIA);
> +	valid = GICV5_GICR_CDIA_VALID(ia);
> +
> +	if (!valid)
> +		return;
> +
> +	gsb_ack();
> +	isb();
> +
> +	hwirq = FIELD_GET(GICV5_GICR_CDIA_INTID, ia);
> +
> +	GUEST_ASSERT_EQ(hwirq, gicv5_spi_hwirq(READ_ONCE(spi_line_expected)));
> +
> +	gic_insn(hwirq, CDDI);
> +	gic_insn(0, CDEOI);

[Severity: Medium]
Should these system register writes also be followed by isb() barriers?

Similar to the above, without explicit synchronization, the pipeline
might not immediately observe the updated state before the exception
handler completes.

> +
> +	GUEST_DONE();
> +}

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