From: David Woodhouse <[email protected]> Plane twins are published with a plain store to common->vcpus[] plus an smp_wmb() that pairs with online_vcpus, not with twin initialisation, while the lockless readers grown across the series (the twin-kick paths, the Hyper-V TLB-flush and stimer twin lookups, the SEND_IPI target lookup, vcpu_for_each_plane) read the slots with a mixture of READ_ONCE and bare loads. x86-TSO hides the lot, but the protocol is unannotated generic code: KCSAN reports it, and nothing stops a reordered publication from exposing a half-initialised twin to a kick.
Publish the slot with smp_store_release(), make every lockless reader use READ_ONCE (vcpu_for_each_plane included), and document the pairing at the iterator. Mutex-serialised lookups (the in-kernel VTL switch runs under the shared vCPU mutex that twin creation also takes) stay plain. Also document KVM_REQUEST_PER_PLANE's two scopes at its definition: the fan-out to twins happens in kvm_make_vcpu_request(), while a direct kvm_make_request() marks only the twin it was given - a distinction a future direct-raise caller would otherwise trip over. No functional change on x86. Signed-off-by: David Woodhouse <[email protected]> Assisted-by: LLM --- arch/x86/kvm/hyperv.c | 6 +++--- include/linux/kvm_host.h | 13 ++++++++++++- virt/kvm/kvm_main.c | 8 +++++++- 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c index dbe1b83c1f28..8abc61f46608 100644 --- a/arch/x86/kvm/hyperv.c +++ b/arch/x86/kvm/hyperv.c @@ -612,7 +612,7 @@ static void stimer_mark_pending(struct kvm_vcpu_hv_stimer *stimer, int p; for (p = 0; p < vcpu->plane_level; p++) { - twin = vcpu->common->vcpus[p]; + twin = READ_ONCE(vcpu->common->vcpus[p]); if (twin) { kvm_make_request(KVM_REQ_UNBLOCK, twin); kvm_vcpu_kick(twin); @@ -2370,7 +2370,7 @@ static void hv_tlb_flush_enqueue_all_planes(struct kvm_vcpu *v, int p; for (p = 0; p < KVM_MAX_PLANES; p++) { - twin = v->common->vcpus[p]; + twin = READ_ONCE(v->common->vcpus[p]); if (!twin || !to_hv_vcpu(twin)) continue; fifo = kvm_hv_get_tlb_flush_fifo(twin, false); @@ -2601,7 +2601,7 @@ static void kvm_hv_send_ipi_to_many(struct kvm *kvm, u32 vector, * VTL): deliver into that plane's APIC. */ if (target_plane) { - target = vcpu->common->vcpus[target_plane]; + target = READ_ONCE(vcpu->common->vcpus[target_plane]); if (!target) continue; } diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 6a1ac96d64ba..4d98de4c7523 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -159,6 +159,12 @@ static inline bool kvm_is_error_gpa(gpa_t gpa) #define KVM_REQUEST_NO_WAKEUP BIT(8) #define KVM_REQUEST_WAIT BIT(9) #define KVM_REQUEST_NO_ACTION BIT(10) +/* + * A KVM_REQUEST_PER_PLANE request has two scopes: raised through + * kvm_make_vcpu_request() (kicks, mass requests) it fans out to every + * plane twin of the vCPU, while a direct kvm_make_request() marks only + * the twin it was given. A direct caller must pick the twin it means. + */ #define KVM_REQUEST_PER_PLANE BIT(11) /* * Architecture-independent vcpu->requests bit members @@ -400,9 +406,14 @@ struct kvm_vcpu_common { struct kvm_vcpu_arch_common arch; }; +/* + * The slot read pairs with the smp_store_release() publication in + * kvm_vcpu_finish_common() for iterators running outside the locks that + * serialise twin creation. + */ #define vcpu_for_each_plane(common, i, v) \ for ((i) = 0; (i) < KVM_MAX_PLANES; ++(i)) \ - if (((v) = common->vcpus[(i)]) != NULL) + if (((v) = READ_ONCE(common->vcpus[(i)])) != NULL) /* Tracked per plane-VCPU - used for deciding which plane-vcpu to run */ enum kvm_vcpu_state { diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index f0b474b0547d..6031ba9a6667 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -563,7 +563,13 @@ static int kvm_vcpu_init_common(struct kvm_vcpu *vcpu, struct kvm *kvm, unsigned static void kvm_vcpu_finish_common(struct kvm_vcpu *vcpu) { WARN_ON(vcpu->common->vcpus[vcpu->plane_level] != NULL); - vcpu->common->vcpus[vcpu->plane_level] = vcpu; + /* + * Publish with release semantics so the lockless twin readers + * (the kick paths, vcpu_for_each_plane, the hyperv twin lookups) + * observe a fully initialised vCPU through their READ_ONCE of the + * slot; the plain store relied on x86-TSO. + */ + smp_store_release(&vcpu->common->vcpus[vcpu->plane_level], vcpu); smp_wmb(); if (vcpu->plane_level == 0) { /* -- 2.50.1
