Re: [PATCH v4 23/48] KVM: arm64: gic-v5: Set IRICHPPIDIS based on IRS enable state

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Spurious VPE doorbells and infinite wakeup loops due to unpreserved IRICHPPIDIS state.
- [Low] Missing READ_ONCE() on vgic.enabled causes a data race.
--

--- Patch [23]: [PATCH v4 23/48] KVM: arm64: gic-v5: Set IRICHPPIDIS based on IRS enable state ---
commit 40fbad6b3a2b1617d4bc4b16c3a6b176e7be5146
Author: Sascha Bischoff <Sascha.Bischoff@arm.com>

KVM: arm64: gic-v5: Set IRICHPPIDIS based on IRS enable state

The GICv5 ICH_CONTEXTR_EL2 has the IRICHPPIDIS field, which allows the
hypervisor to enable/disable the HPPI selection for SPIs and
LPIs. This can be used to emulate the guest enabling/disabling the
IRS. Therefore, make the state of this controlled by the IRS enable
state. Thus, SPIs and LPIs can't be delivered to the guest, until it
enables the emulated IRS, which matches the behaviour of the real
hardware.

> diff --git a/arch/arm64/kvm/vgic/vgic-v5.c b/arch/arm64/kvm/vgic/vgic-v5.c
> index 4042d468431b1..7957b9da95e7c 100644
> --- a/arch/arm64/kvm/vgic/vgic-v5.c
> +++ b/arch/arm64/kvm/vgic/vgic-v5.c
> @@ -1066,6 +1066,7 @@ void vgic_v5_flush_ppi_state(struct kvm_vcpu *vcpu)
>  
>  void vgic_v5_load(struct kvm_vcpu *vcpu)
>  {
> +	bool irichppidis = !vcpu->kvm->arch.vgic.enabled;

[Severity: Low]
Is a READ_ONCE() missing here?

The vgic.enabled flag is updated concurrently by the IRS MMIO handler using
WRITE_ONCE(), so reading it locklessly here might trigger KCSAN data race
warnings.

>  	struct vgic_v5_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v5;
>  	u16 vm = vgic_v5_vm_id(vcpu->kvm);
>  	u16 vpe = vgic_v5_vpe_id(vcpu);
> @@ -1082,6 +1083,7 @@ void vgic_v5_load(struct kvm_vcpu *vcpu)
>  	kvm_call_hyp(__vgic_v5_restore_vmcr_apr, cpu_if);
>  
>  	cpu_if->vgic_contextr = FIELD_PREP(ICH_CONTEXTR_EL2_V, true) |
> +				FIELD_PREP(ICH_CONTEXTR_EL2_IRICHPPIDIS, irichppidis) |

[Severity: High]
Will this IRICHPPIDIS state get lost when the VCPU enters WFI?

When the guest executes WFI, KVM calls vgic_v5_put(). It currently clears
vgic_contextr before setting the doorbell bits:

void vgic_v5_put(struct kvm_vcpu *vcpu)
{
	...
	cpu_if->vgic_contextr = 0;
	if (vcpu_get_flag(vcpu, IN_WFI)) {
		...
		cpu_if->vgic_contextr = FIELD_PREP(ICH_CONTEXTR_EL2_DB, 1) |
					FIELD_PREP(ICH_CONTEXTR_EL2_DBPM, dbpm);
	...
}

If the guest disabled the IRS (so IRICHPPIDIS should be 1) and enters WFI
while an SPI or LPI is pending, the hardware will see IRICHPPIDIS=0 and DB=1.

Could this cause the hardware to inadvertently evaluate the interrupt and fire
the doorbell, waking the guest?

If the guest then resumes from WFI, sees no pending interrupts, and executes
WFI again, wouldn't this result in an infinite wakeup loop?

>  				FIELD_PREP(ICH_CONTEXTR_EL2_VPE, vpe) |
>  				FIELD_PREP(ICH_CONTEXTR_EL2_VM, vm);
>

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