[RFC PATCH 31/55] KVM: planes: Kick the running twin's pCPU, not the request target's

[David Woodhouse]

From: David Woodhouse <dwmw@xxxxxxxxxxxx>

The remote kick paths (kvm_make_vcpus_request_mask(),
kvm_make_all_cpus_request(), __kvm_vcpu_kick()) read
READ_ONCE(vcpu->cpu) of the plane-0 vCPU. But vcpu->cpu is per plane
vCPU — kvm_arch_vcpu_load() refreshes only the loaded twin's copy —
while vcpu->mode is shared in kvm_vcpu_common. When the vCPU thread
has migrated pCPUs while a non-zero plane was loaded, the shared mode
reads IN_GUEST_MODE but the IPI goes to plane 0's stale pCPU: a flush
hypercall returns HV_STATUS_SUCCESS while the twin keeps running on
stale translations, breaking the TLFS guarantee that the flush covers
the whole VP.

Read the pCPU from common->current_vcpu, whose cpu field every
kvm_arch_vcpu_load() refreshes, in both kick paths. Also read the twin
array with READ_ONCE in the per-plane request fan-out and correct the
comment there: mode and wait are shared with the common, the pCPU is
not.

Found by Sashiko review.

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

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index ae998cef3c36..fa639a659138 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -225,16 +225,16 @@ static void kvm_make_vcpu_request(struct kvm_vcpu *vcpu, unsigned int req,
 		 * A per-plane request raised on "the vCPU" (TLB flush after
 		 * a host-side zap, obsolete-roots reload, ...) concerns
 		 * every plane: fan it out to the twins, or the plane that
-		 * runs next consumes a bit the others still need.  Mode,
-		 * pCPU and wait are shared with the common, so the kick
-		 * logic below covers the twins too.
+		 * runs next consumes a bit the others still need.  Mode and
+		 * wait live in the common and the kick below targets the
+		 * running twin's pCPU, so it covers the twins too.
 		 */
 		if (req & KVM_REQUEST_PER_PLANE) {
 			struct kvm_vcpu *twin;
 			int p;
 
 			for (p = 0; p < KVM_MAX_PLANES; p++) {
-				twin = vcpu->common->vcpus[p];
+				twin = READ_ONCE(vcpu->common->vcpus[p]);
 				if (twin && twin != vcpu)
 					__kvm_make_request(req, twin);
 			}
@@ -255,7 +255,14 @@ static void kvm_make_vcpu_request(struct kvm_vcpu *vcpu, unsigned int req,
 	 * finalized. See kvm_vcpu_kick() for more details on handling requests.
 	 */
 	if (kvm_request_needs_ipi(vcpu, req)) {
-		cpu = READ_ONCE(vcpu->cpu);
+		/*
+		 * vcpu->cpu is per plane vCPU and kvm_arch_vcpu_load()
+		 * refreshes only the loaded twin's, so this plane's copy
+		 * can point at a pCPU the thread has long left while a
+		 * twin runs elsewhere.  The mode just checked is shared;
+		 * the IPI must go where the running twin actually is.
+		 */
+		cpu = READ_ONCE(READ_ONCE(vcpu->common->current_vcpu)->cpu);
 		if (cpu != -1 && cpu != current_cpu)
 			__cpumask_set_cpu(cpu, tmp);
 	}
@@ -4334,7 +4341,13 @@ void __kvm_vcpu_kick(struct kvm_vcpu *vcpu, bool wait)
 	 * vCPU also requires it to leave IN_GUEST_MODE.
 	 */
 	if (kvm_arch_vcpu_should_kick(vcpu)) {
-		cpu = READ_ONCE(vcpu->cpu);
+		/*
+		 * Target the running twin's pCPU: this plane's vcpu->cpu
+		 * goes stale whenever the thread migrates while another
+		 * plane is loaded, and the IN_GUEST_MODE just observed is
+		 * the (shared) mode of whichever twin runs.
+		 */
+		cpu = READ_ONCE(READ_ONCE(vcpu->common->current_vcpu)->cpu);
 		if (cpu != me && (unsigned int)cpu < nr_cpu_ids && cpu_online(cpu)) {
 			/*
 			 * Use a reschedule IPI to kick the vCPU if the caller
-- 
2.50.1