Re: [PATCH v2 15/17] KVM: arm64: Reject host power-on of a vCPU that EL2 holds powered off

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Race condition between EL2 and Host PSCI handling leads to state desynchronization and guest failure (INTERNAL_FAILURE)
--

commit 80fb3e3ea2b949c35be53322315a43ff86b0f3ea
Author: Fuad Tabba <fuad.tabba@linux.dev>

KVM: arm64: Reject host power-on of a vCPU that EL2 holds powered off

A protected vCPU's power state belongs to EL2, which changes it only
on the guest's own PSCI calls. KVM_SET_MP_STATE(RUNNABLE) on a vCPU
EL2 holds powered off changes the host's copy alone, and the guest's
next CPU_ON of that vCPU then fails: mp_state is no longer STOPPED, so
the host returns ALREADY_ON, which EL2 turns into INTERNAL_FAILURE.
SUSPENDED has the same effect.

[ ... ]

> diff --git a/arch/arm64/kvm/psci.c b/arch/arm64/kvm/psci.c
> index e3db84400d1f8..208289a08099f 100644
> --- a/arch/arm64/kvm/psci.c
> +++ b/arch/arm64/kvm/psci.c
> @@ -94,6 +94,9 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
>  	 */
>  	smp_wmb();
>  

[Severity: High]
This is a pre-existing issue, but can a concurrent CPU_OFF and CPU_ON cause
state desynchronization between EL2 and the host?

In kvm_psci_vcpu_on(), there is an early return path before this hunk:

	spin_lock(&vcpu->arch.mp_state_lock);
	if (!kvm_arm_vcpu_stopped(vcpu)) {
		if (kvm_psci_version(source_vcpu) != KVM_ARM_PSCI_0_1)
			ret = PSCI_RET_ALREADY_ON;
		else
			ret = PSCI_RET_INVALID_PARAMS;

		goto out_unlock;
	}

If vCPU 0 executes CPU_OFF, EL2 transitions it to OFF and exits to the host.
Concurrently, if vCPU 1 executes CPU_ON targeting vCPU 0, EL2 transitions it
to ON_PENDING and exits to the host.

If the host thread for vCPU 1 acquires mp_state_lock before the host thread
for vCPU 0, the host sees mp_state as RUNNABLE, rejects the CPU_ON, and
returns ALREADY_ON. EL2 then turns this into an INTERNAL_FAILURE, which leads
to the permanent loss of a vCPU.

Taking this early return path also skips the newly introduced
pkvm_powered_off = false update, compromising the accuracy of the tracking.

Could this early return condition in kvm_psci_vcpu_on() be updated to properly
account for pending EL2 state transitions?

> +	/* EL2 has committed the protected vCPU to ON_PENDING to get here. */
> +	vcpu->arch.pkvm_powered_off = false;
> +
>  	WRITE_ONCE(vcpu->arch.mp_state.mp_state, KVM_MP_STATE_RUNNABLE);
>  	kvm_vcpu_wake_up(vcpu);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907070002.3333525-1-fuad.tabba@linux.dev?part=15