Re: [PATCH v3 16/40] KVM: arm64: gic-v5: Request doorbells when VPEs enter WFI

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Use of `WARN_ON()` wrapping the fallible `irq_set_affinity()` API on a guest-driven WFI path allows a malicious guest to trigger a host denial of service.
- [High] Off-by-one error when computing the Doorbell Priority Mask (DBPM) causes the hardware to miss doorbell signals for valid pending interrupts.
--

commit a810e1cc5f94c1f25b127b180419f6917b92c803
Author: Sascha Bischoff <Sascha.Bischoff@arm.com>
Subject: KVM: arm64: gic-v5: Request doorbells when VPEs enter WFI

This commit adds support for requesting GICv5 VPE doorbells when a vCPU
enters WFI and the VPE becomes non-resident. It sets the doorbell priority
mask to avoid unnecessary wakeups. It also routes the doorbell interrupt to
the current CPU and updates the pending check to observe the doorbell
fired state.

> diff --git a/arch/arm64/kvm/vgic/vgic-v5.c b/arch/arm64/kvm/vgic/vgic-v5.c
> index 9c383b6864270..6e976162ec3c2 100644
> --- a/arch/arm64/kvm/vgic/vgic-v5.c
> +++ b/arch/arm64/kvm/vgic/vgic-v5.c
> @@ -1094,6 +1094,46 @@ void vgic_v5_put(struct kvm_vcpu *vcpu)
>  	kvm_call_hyp(__vgic_v5_save_apr, cpu_if);
>  
>  	cpu_if->vgic_contextr = 0;
> +	if (vcpu_get_flag(vcpu, IN_WFI)) {
> +		u32 priority_mask;
> +		int dbpm;
> +
> +		/*
> +		 * Find the virtual running priority and use this to calculate
> +		 * the doorbell priority mask. We combine the highest active
> +		 * priority and the CPU's priority mask. The guest can't handle
> +		 * interrupts with priorities less than or equal to the virtual
> +		 * running priority, so there's literally no point in waking the
> +		 * guest for these.
> +		 *
> +		 * The priority needs to be higher than the mask to signal, so
> +		 * pick the next higher priority (subtract 1).
> +		 */
> +		priority_mask = vgic_v5_get_effective_priority_mask(vcpu);
> +
> +		/*
> +		 * Request a doorbell *unless* the priority is 0, indicating
> +		 * that no interrupt can wake the CPU up.
> +		 */
> +		if (priority_mask) {
> +			int db_irq = vgic_v5_vpe_db(vcpu);
> +			struct irq_data *d = irq_get_irq_data(db_irq);
> +			const struct cpumask *aff = irq_data_get_effective_affinity_mask(d);
> +			int cpu = smp_processor_id();
> +
> +			dbpm = priority_mask - 1;
> +			cpu_if->vgic_contextr = FIELD_PREP(ICH_CONTEXTR_EL2_DB, 1) |
> +						FIELD_PREP(ICH_CONTEXTR_EL2_DBPM, dbpm);

[Severity: High]
Does this subtraction configure the mask too strictly? Since the hardware
already evaluates using a strictly-less-than check (Priority < DBPM), would
setting it to priority_mask - 1 cause the hardware to miss doorbell signals
for interrupts arriving at the lowest unmasked priority level?

> +
> +			/*
> +			 * Make the doorbell affine to this CPU, if it isn't
> +			 * already. Actively check the cpumask first as it is
> +			 * cheaper than changing the affinity every time.
> +			 */
> +			if (!cpumask_test_cpu(cpu, aff))
> +				WARN_ON(irq_set_affinity(db_irq, cpumask_of(cpu)));

[Severity: High]
Should we handle the failure of irq_set_affinity() here without using a
WARN_ON()?

Because this is a fallible API and the WFI path is driven directly by the
guest, a malicious guest could execute WFI in a tight loop to trigger this
warning continuously. This could lead to a host denial of service either
through severe log spam or a panic if panic_on_warn is enabled.

> +		}
> +	}
>  
>  	kvm_call_hyp(__vgic_v5_make_non_resident, cpu_if);
>

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