[RFC PATCH 32/55] KVM: planes: Keep current_vcpu in sync in vcpu_load()

[David Woodhouse]

From: David Woodhouse <dwmw@xxxxxxxxxxxx>

The preempt notifiers save and restore common->current_vcpu
(kvm_sched_out()/kvm_sched_in()), but vcpu_load() does not update it.
kvm_vcpu_ioctl_set_cpuid() swaps the loaded vCPU to each twin around
kvm_vcpu_after_set_cpuid(), a window that is preemptible and sleeps
(kvm_pmu_refresh, mmu_lock): a reschedule there arch-put the
already-put caller a second time, never saved the twin, and reloaded
the caller's VMCS, so the vendor callback wrote the twin's controls
into the caller's VMCS and the trailing vcpu_put() unloaded a
non-resident vCPU, corrupting the per-pCPU loaded_vmcs tracking. Any
KVM_SET_CPUID2 on a multi-plane VM reaches this; ioctls taken on a
plane-N fd while current_vcpu pointed at another plane had the same
exposure, as do the kick paths that now read current_vcpu->cpu.

Set common->current_vcpu in vcpu_load(), making every load path (run
loop, vcpu ioctls, the cpuid twin swap) agree with what the preempt
notifiers will operate on. kvm_vcpu_select_plane() keeps its own
assignment; both stores use WRITE_ONCE to pair with the lock-free
readers in the kick paths.

Found by Sashiko review.

Signed-off-by: David Woodhouse <dwmw@xxxxxxxxxxxx>
Assisted-by: LLM
---
 virt/kvm/kvm_main.c | 31 ++++++++++++++++++++++++++++++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index fa639a659138..d37961cc5235 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -165,6 +165,16 @@ void vcpu_load(struct kvm_vcpu *vcpu)
 {
 	int cpu = get_cpu();
 
+	/*
+	 * The preempt notifiers operate on common->current_vcpu, so it
+	 * must name the plane vCPU being loaded here: an ioctl taken on
+	 * another plane's fd than the one that ran last would otherwise
+	 * have a preemption arch-put the stale twin (whose state is not
+	 * resident) and arch-load it back over the state this load makes
+	 * resident.  The kick paths also read it to find the running
+	 * twin's pCPU.
+	 */
+	WRITE_ONCE(vcpu->common->current_vcpu, vcpu);
 	__this_cpu_write(kvm_running_vcpu, vcpu->common);
 	preempt_notifier_register(&vcpu->common->preempt_notifier);
 	kvm_arch_vcpu_load(vcpu, cpu);
@@ -4719,7 +4729,26 @@ static int kvm_plane_ioctl_create_vcpu(struct kvm_plane *plane, unsigned long id
 
 	kvm_vcpu_init(vcpu, kvm, id);
 
+	/*
+	 * A non-zero plane shares 'common' with a plane-0 twin which may
+	 * already be running.  kvm_arch_vcpu_create() calls vcpu_load(),
+	 * which publishes the half-created vCPU through
+	 * common->current_vcpu and registers the shared preempt notifier,
+	 * so it must not run concurrently with a twin's KVM_RUN: hold the
+	 * shared vCPU mutex across it.  Drop it before kvm->lock is taken
+	 * below, to respect the kvm->lock -> vcpu mutex ordering.
+	 */
+	if (plane->level > 0) {
+		r = mutex_lock_killable(kvm_vcpu_mutex(vcpu));
+		if (r)
+			goto vcpu_free_common;
+	}
+
 	r = kvm_arch_vcpu_create(vcpu);
+
+	if (plane->level > 0)
+		kvm_vcpu_unlock(vcpu);
+
 	if (r)
 		goto vcpu_free_common;
 
@@ -5427,7 +5456,7 @@ struct kvm_vcpu *kvm_vcpu_select_plane(struct kvm_vcpu *vcpu)
 		ret->plane_state = RUNNABLE;
 	}
 
-	common->current_vcpu = ret;
+	WRITE_ONCE(common->current_vcpu, ret);
 
 	common->plane_switch = false;
 
-- 
2.50.1