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

[email protected]
Newsgroups org.kernel.vger.kvm,dev.linux.lists.kvmarm
Message-ID <[email protected]>
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 <[email protected]>

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?

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=48
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.