Re: [PATCH v5 48/49] KVM: selftests: Add VGICv5 IST save/restore coverage

[Sascha Bischoff <Sascha.Bischoff@arm.com>]

On Fri, 2026-08-07 at 17:50 +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> 
> Pre-existing issues:
> - [High] Missing Context Synchronization Event (ISB) after control-
> plane system register writes.
> --
> 
> --- Patch [48]: [PATCH v5 48/49] KVM: selftests: Add VGICv5 IST
> save/restore coverage ---
> commit 3f2c979e78365e70421bcaaf39f350d8b4a83086
> Author: Sascha Bischoff <Sascha.Bischoff@arm.com>
> 
> KVM: selftests: Add VGICv5 IST save/restore coverage
> 
> Add end-to-end VGICv5 IST save/restore coverage. The test verifies
> that
> interrupt state saved from the ISTs can be restored into a fresh VM
> before
> first run and then consumed by the guest.
> 
> > diff --git a/tools/testing/selftests/kvm/arm64/vgic_v5.c
> > b/tools/testing/selftests/kvm/arm64/vgic_v5.c
> > index 71bda5661c74e..47e147bc14b87 100644
> > --- a/tools/testing/selftests/kvm/arm64/vgic_v5.c
> > +++ b/tools/testing/selftests/kvm/arm64/vgic_v5.c
> [ ... ]
> > @@ -359,6 +406,165 @@ static void guest_lpi_code(void)
> [ ... ]
> > +static void guest_ist_restore_code(void)
> > +{
> > +	local_irq_disable();
> > +
> > +	gicv5_cpu_enable_interrupts();
> > +
> > +	if (!guest_get_vcpuid())
> 
> [Severity: High]
> This is a pre-existing issue, but should
> gicv5_cpu_enable_interrupts()
> issue an isb() before it returns?
> 
> Looking at the implementation of gicv5_cpu_enable_interrupts() in
> both the
> host kernel driver (drivers/irqchip/irq-gic-v5.c) and the selftest
> helper
> (tools/testing/selftests/kvm/include/arm64/gic_v5.h), it writes to
> control
> plane system registers like SYS_ICC_CR0_EL1 without an explicit
> context
> synchronization event.
> 
> In this new test code, we perform a conditional branch based on
> guest_get_vcpuid() immediately after returning from the function.
> Without an
> isb(), could this conditional instruction be executed against an
> architecturally undefined pipeline state?

As we've discussed relatively often at this point, this is not an
issue. Also, we're not in an undefined pipeline state.

> 
> > +		gicv5_set_pcr(VGIC_V5_IST_PRIO_HIGH);
> > +
> > +	GUEST_SYNC(GUEST_CMD_IS_READY);
> > +
> > +	local_irq_enable();
> > +
> > +	while (1)
> > +		wfi();
> > +}
> 

Thanks,
Sascha