mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: David Woodhouse <dwmw2@infradead.org>
To: kvm@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
	rcu@vger.kernel.org, linux-rt-devel@lists.linux.dev,
	Paolo Bonzini <pbonzini@redhat.com>,
	Sean Christopherson <seanjc@google.com>,
	Paul Durrant <paul@xen.org>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	Fred Griffoul <fgriffo@amazon.co.uk>,
	"Paul E . McKenney" <paulmck@kernel.org>,
	Kunwu Chan <kunwu.chan@linux.dev>,
	Kunwu Chan <kunwu.chan@gmail.com>,
	syzbot+208f7f3e5f59c11aeb90@syzkaller.appspotmail.com,
	David Woodhouse <dwmw@amazon.co.uk>
Subject: [PATCH v2 08/20] KVM: pfncache: Use atomic SRCU for readers instead of a rwlock
Date: Thu,  5 Nov 2026 02:00:50 +0000	[thread overview]
Message-ID: <20261105020104.2507083-9-dwmw2@infradead.org> (raw)
In-Reply-To: <20261105020104.2507083-1-dwmw2@infradead.org>

From: David Woodhouse <dwmw@amazon.co.uk>

Replace the per-cache rwlock with a dedicated per-VM atomic SRCU
domain (kvm->gpc_srcu) for the read side. Readers now run under
srcu_read_lock_atomic() alone: preemption disabled and non-blocking
enforced, which every GPC read-side section already satisfied as a
former rwlock critical section. It works in any context (including
hardirq and sched-out paths) and never fails or spins, eliminating
the read_trylock() contortions in the atomic paths, the double-lock
dance with the lockdep subclass hack in the runstate update, and the
PREEMPT_RT problems inherent to taking an rwlock in those contexts.
In particular, kvm_xen_set_evtchn_fast() is called from hardirq
context (timer callback, kvm_arch_set_irq_inatomic()), where taking
gpc->lock is a sleeping-lock-in-atomic-context bug on PREEMPT_RT.

The mutators' reader drains use synchronize_srcu_atomic(), which
spins for the bounded readers instead of sleeping: no path waiting
out GPC readers ever schedules. That satisfies the OOM-reaper
non-blocking requirement in the invalidation walk, whose wait must
happen even on unblockable ranges — GPC readers never allocate, take
mmap_lock, or sleep, so waiting them out cannot deadlock against
reclaim. SRCU rather than plain RCU so that wait covers only this
VM's GPC readers, not a grace period across every pCPU in the
system; the read-side cost is a this_cpu_inc() and a barrier.

Without gpc->lock, the refresh can no longer rely on holding it to
make its final invalidation check and its publish atomic with
respect to the mmu_notifier walk, and can no longer treat "GPC_VALID
already clear" as "no readers remain". Three state bits in a single
atomic word replace it: GPC_BECOMING_VALID lets the walk veto an
in-flight refresh's pending publish (the publish is a cmpxchg which
fails if the bit was cleared, closing the window between the
refresh's last check and its publish); GPC_INVALIDATING records that
a grace period for the previously published mapping is still owed:
whoever clears GPC_VALID leaves the marker standing until their
grace-period wait completes, so that a later mutator — or a later
mmu_notifier walk, whose return is what lets the primary MMU free
the page — finding GPC_VALID already clear still performs a wait of
its own rather than trusting one it cannot see finish. The comments
in pfncache.c and kvm_types.h carry the full protocol.

As before, a refresh which resolves to the same uHVA in the same
memslot leaves the cache valid and undisturbed, skipping the grace
period: routine host memory management (page migration, NUMA
balancing, KSM) usually re-backs the same uHVA, and a grace period
in the reader's refresh path for each such event would add seconds
to a guest boot.

Not suitable for stable backport despite the Fixes tag (it depends
on the atomic SRCU reader flavour, new in this cycle); the tag
documents where the PREEMPT_RT sleeping-lock problem was introduced.

Fixes: 14243b387137 ("KVM: x86/xen: Add KVM_IRQ_ROUTING_XEN_EVTCHN and event channel delivery")
Reported-by: syzbot+208f7f3e5f59c11aeb90@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=208f7f3e5f59c11aeb90
Reported-by: syzbot+919877893c9d28162dc2@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=919877893c9d28162dc2
Tested-by: syzbot+919877893c9d28162dc2@syzkaller.appspotmail.com
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 arch/x86/kvm/x86.c        |  10 +-
 arch/x86/kvm/xen.c        | 146 ++++++------
 include/linux/kvm_host.h  |  26 ++-
 include/linux/kvm_types.h |  37 ++-
 include/linux/srcu.h      |   7 +
 virt/kvm/kvm_main.c       |   9 +
 virt/kvm/pfncache.c       | 481 ++++++++++++++++++++++++++++++--------
 7 files changed, 528 insertions(+), 188 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 3c26c0eb9865..f5f3459ed4a5 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1747,18 +1747,18 @@ static void kvm_setup_guest_pvclock(struct pvclock_vcpu_time_info *ref_hv_clock,
 {
 	struct pvclock_vcpu_time_info *guest_hv_clock;
 	struct pvclock_vcpu_time_info hv_clock;
-	unsigned long flags;
+	int idx;
 
 	memcpy(&hv_clock, ref_hv_clock, sizeof(hv_clock));
 
-	read_lock_irqsave(&gpc->lock, flags);
+	idx = srcu_read_lock_atomic(&vcpu->kvm->gpc_srcu);
 	while (!kvm_gpc_check(gpc, offset + sizeof(*guest_hv_clock))) {
-		read_unlock_irqrestore(&gpc->lock, flags);
+		srcu_read_unlock_atomic(&vcpu->kvm->gpc_srcu, idx);
 
 		if (kvm_gpc_refresh(gpc, offset + sizeof(*guest_hv_clock)))
 			return;
 
-		read_lock_irqsave(&gpc->lock, flags);
+		idx = srcu_read_lock_atomic(&vcpu->kvm->gpc_srcu);
 	}
 
 	guest_hv_clock = (void *)(gpc->khva + offset);
@@ -1783,7 +1783,7 @@ static void kvm_setup_guest_pvclock(struct pvclock_vcpu_time_info *ref_hv_clock,
 	guest_hv_clock->version = ++hv_clock.version;
 
 	kvm_gpc_mark_dirty_in_slot(gpc);
-	read_unlock_irqrestore(&gpc->lock, flags);
+	srcu_read_unlock_atomic(&vcpu->kvm->gpc_srcu, idx);
 
 	trace_kvm_pvclock_update(vcpu->vcpu_id, &hv_clock);
 }
diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index e54988522b8b..13e287bc17bb 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -46,16 +46,17 @@ static int kvm_xen_shared_info_init(struct kvm *kvm)
 	u64 wall_nsec;
 	int ret = 0;
 	int idx = srcu_read_lock(&kvm->srcu);
+	int gpc_idx;
 
-	read_lock_irq(&gpc->lock);
+	gpc_idx = srcu_read_lock_atomic(&kvm->gpc_srcu);
 	while (!kvm_gpc_check(gpc, PAGE_SIZE)) {
-		read_unlock_irq(&gpc->lock);
+		srcu_read_unlock_atomic(&kvm->gpc_srcu, gpc_idx);
 
 		ret = kvm_gpc_refresh(gpc, PAGE_SIZE);
 		if (ret)
 			goto out;
 
-		read_lock_irq(&gpc->lock);
+		gpc_idx = srcu_read_lock_atomic(&kvm->gpc_srcu);
 	}
 
 	/*
@@ -98,7 +99,7 @@ static int kvm_xen_shared_info_init(struct kvm *kvm)
 	smp_wmb();
 
 	wc->version = wc_version + 1;
-	read_unlock_irq(&gpc->lock);
+	srcu_read_unlock_atomic(&kvm->gpc_srcu, gpc_idx);
 
 out:
 	srcu_read_unlock(&kvm->srcu, idx);
@@ -155,22 +156,21 @@ static int xen_get_guest_pvclock(struct kvm_vcpu *vcpu,
 				 struct gfn_to_pfn_cache *gpc,
 				 unsigned int offset)
 {
-	unsigned long flags;
-	int r;
+	int idx, r;
 
-	read_lock_irqsave(&gpc->lock, flags);
+	idx = srcu_read_lock_atomic(&vcpu->kvm->gpc_srcu);
 	while (!kvm_gpc_check(gpc, offset + sizeof(*hv_clock))) {
-		read_unlock_irqrestore(&gpc->lock, flags);
+		srcu_read_unlock_atomic(&vcpu->kvm->gpc_srcu, idx);
 
 		r = kvm_gpc_refresh(gpc, offset + sizeof(*hv_clock));
 		if (r)
 			return r;
 
-		read_lock_irqsave(&gpc->lock, flags);
+		idx = srcu_read_lock_atomic(&vcpu->kvm->gpc_srcu);
 	}
 
 	memcpy(hv_clock, gpc->khva + offset, sizeof(*hv_clock));
-	read_unlock_irqrestore(&gpc->lock, flags);
+	srcu_read_unlock_atomic(&vcpu->kvm->gpc_srcu, idx);
 
 	/*
 	 * Sanity check TSC shift+multiplier to verify the guest's view of time
@@ -325,12 +325,12 @@ static void kvm_xen_update_runstate_guest(struct kvm_vcpu *v, bool atomic)
 	struct gfn_to_pfn_cache *gpc2 = &vx->runstate2_cache;
 	size_t user_len, user_len1, user_len2;
 	struct vcpu_runstate_info rs;
-	unsigned long flags;
 	size_t times_ofs;
 	uint8_t *update_bit = NULL;
 	uint64_t entry_time;
 	uint64_t *rs_times;
 	int *rs_state;
+	int idx;
 
 	/*
 	 * The only difference between 32-bit and 64-bit versions of the
@@ -417,20 +417,12 @@ static void kvm_xen_update_runstate_guest(struct kvm_vcpu *v, bool atomic)
 
  retry:
 	/*
-	 * Attempt to obtain the GPC lock on *both* (if there are two)
-	 * gfn_to_pfn caches that cover the region.
+	 * Check *both* (if there are two) gfn_to_pfn caches that cover
+	 * the region, under a single SRCU read-side critical section.
 	 */
-	if (atomic) {
-		local_irq_save(flags);
-		if (!read_trylock(&gpc1->lock)) {
-			local_irq_restore(flags);
-			return;
-		}
-	} else {
-		read_lock_irqsave(&gpc1->lock, flags);
-	}
+	idx = srcu_read_lock_atomic(&v->kvm->gpc_srcu);
 	while (!kvm_gpc_check(gpc1, user_len1)) {
-		read_unlock_irqrestore(&gpc1->lock, flags);
+		srcu_read_unlock_atomic(&v->kvm->gpc_srcu, idx);
 
 		/* When invoked from kvm_sched_out() we cannot sleep */
 		if (atomic)
@@ -439,7 +431,7 @@ static void kvm_xen_update_runstate_guest(struct kvm_vcpu *v, bool atomic)
 		if (kvm_gpc_refresh(gpc1, user_len1))
 			return;
 
-		read_lock_irqsave(&gpc1->lock, flags);
+		idx = srcu_read_lock_atomic(&v->kvm->gpc_srcu);
 	}
 
 	if (likely(!user_len2)) {
@@ -459,24 +451,11 @@ static void kvm_xen_update_runstate_guest(struct kvm_vcpu *v, bool atomic)
 	} else {
 		/*
 		 * The guest's runstate_info is split across two pages and we
-		 * need to hold and validate both GPCs simultaneously. We can
-		 * declare a lock ordering GPC1 > GPC2 because nothing else
-		 * takes them more than one at a time. Set a subclass on the
-		 * gpc1 lock to make lockdep shut up about it.
+		 * need to validate both GPCs simultaneously. They are both
+		 * covered by the single SRCU read-side critical section above.
 		 */
-		lock_set_subclass(&gpc1->lock.dep_map, 1, _THIS_IP_);
-		if (atomic) {
-			if (!read_trylock(&gpc2->lock)) {
-				read_unlock_irqrestore(&gpc1->lock, flags);
-				return;
-			}
-		} else {
-			read_lock(&gpc2->lock);
-		}
-
 		if (!kvm_gpc_check(gpc2, user_len2)) {
-			read_unlock(&gpc2->lock);
-			read_unlock_irqrestore(&gpc1->lock, flags);
+			srcu_read_unlock_atomic(&v->kvm->gpc_srcu, idx);
 
 			/* When invoked from kvm_sched_out() we cannot sleep */
 			if (atomic)
@@ -575,13 +554,11 @@ static void kvm_xen_update_runstate_guest(struct kvm_vcpu *v, bool atomic)
 		smp_wmb();
 	}
 
-	if (user_len2) {
+	if (user_len2)
 		kvm_gpc_mark_dirty_in_slot(gpc2);
-		read_unlock(&gpc2->lock);
-	}
 
 	kvm_gpc_mark_dirty_in_slot(gpc1);
-	read_unlock_irqrestore(&gpc1->lock, flags);
+	srcu_read_unlock_atomic(&v->kvm->gpc_srcu, idx);
 }
 
 void kvm_xen_update_runstate(struct kvm_vcpu *v, int state)
@@ -656,7 +633,7 @@ void kvm_xen_inject_pending_events(struct kvm_vcpu *v)
 {
 	unsigned long evtchn_pending_sel = READ_ONCE(v->arch.xen.evtchn_pending_sel);
 	struct gfn_to_pfn_cache *gpc = &v->arch.xen.vcpu_info_cache;
-	unsigned long flags;
+	int idx;
 
 	if (!evtchn_pending_sel)
 		return;
@@ -666,14 +643,14 @@ void kvm_xen_inject_pending_events(struct kvm_vcpu *v)
 	 * does anyway. Page it in and retry the instruction. We're just a
 	 * little more honest about it.
 	 */
-	read_lock_irqsave(&gpc->lock, flags);
+	idx = srcu_read_lock_atomic(&v->kvm->gpc_srcu);
 	while (!kvm_gpc_check(gpc, sizeof(struct vcpu_info))) {
-		read_unlock_irqrestore(&gpc->lock, flags);
+		srcu_read_unlock_atomic(&v->kvm->gpc_srcu, idx);
 
 		if (kvm_gpc_refresh(gpc, sizeof(struct vcpu_info)))
 			return;
 
-		read_lock_irqsave(&gpc->lock, flags);
+		idx = srcu_read_lock_atomic(&v->kvm->gpc_srcu);
 	}
 
 	/* Now gpc->khva is a valid kernel address for the vcpu_info */
@@ -706,7 +683,7 @@ void kvm_xen_inject_pending_events(struct kvm_vcpu *v)
 		WRITE_ONCE(vi->evtchn_upcall_pending, 1);
 	}
 
-	read_unlock_irqrestore(&gpc->lock, flags);
+	srcu_read_unlock_atomic(&v->kvm->gpc_srcu, idx);
 
 	/* For the per-vCPU lapic vector, deliver it as MSI. */
 	if (v->arch.xen.upcall_vector)
@@ -716,8 +693,8 @@ void kvm_xen_inject_pending_events(struct kvm_vcpu *v)
 int __kvm_xen_has_interrupt(struct kvm_vcpu *v)
 {
 	struct gfn_to_pfn_cache *gpc = &v->arch.xen.vcpu_info_cache;
-	unsigned long flags;
 	u8 rc = 0;
+	int idx;
 
 	/*
 	 * If the global upcall vector (HVMIRQ_callback_vector) is set and
@@ -742,9 +719,9 @@ int __kvm_xen_has_interrupt(struct kvm_vcpu *v)
 	 */
 	guard(srcu)(&v->kvm->srcu);
 
-	read_lock_irqsave(&gpc->lock, flags);
+	idx = srcu_read_lock_atomic(&v->kvm->gpc_srcu);
 	while (!kvm_gpc_check(gpc, sizeof(struct vcpu_info))) {
-		read_unlock_irqrestore(&gpc->lock, flags);
+		srcu_read_unlock_atomic(&v->kvm->gpc_srcu, idx);
 
 		/*
 		 * This function gets called from kvm_vcpu_block() after setting the
@@ -764,11 +741,11 @@ int __kvm_xen_has_interrupt(struct kvm_vcpu *v)
 			 */
 			return 0;
 		}
-		read_lock_irqsave(&gpc->lock, flags);
+		idx = srcu_read_lock_atomic(&v->kvm->gpc_srcu);
 	}
 
 	rc = ((struct vcpu_info *)gpc->khva)->evtchn_upcall_pending;
-	read_unlock_irqrestore(&gpc->lock, flags);
+	srcu_read_unlock_atomic(&v->kvm->gpc_srcu, idx);
 	return rc;
 }
 
@@ -803,6 +780,13 @@ int kvm_xen_hvm_set_attr(struct kvm *kvm, struct kvm_xen_hvm_attr *data)
 
 		mutex_lock(&kvm->arch.xen.xen_lock);
 
+		/*
+		 * The cache's own readers hold kvm->gpc_srcu, but activating
+		 * it resolves the gfn through kvm_memslots(), which is
+		 * protected by kvm->srcu. (Holding kvm->srcu across the
+		 * deactivation is also fine: its grace-period wait is on the
+		 * gpc_srcu domain, not this one.)
+		 */
 		idx = srcu_read_lock(&kvm->srcu);
 
 		if (data->type == KVM_XEN_ATTR_TYPE_SHARED_INFO) {
@@ -937,6 +921,14 @@ int kvm_xen_vcpu_set_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
 	int idx, r = -ENOENT;
 
 	mutex_lock(&vcpu->kvm->arch.xen.xen_lock);
+
+	/*
+	 * Activating the caches below resolves gfns through kvm_memslots(),
+	 * and the runstate cases read it via kvm_gpc_check(); both are
+	 * protected by kvm->srcu (the caches' readers hold kvm->gpc_srcu,
+	 * which does not cover the memslots, and whose grace-period waits
+	 * are safe to perform with kvm->srcu held).
+	 */
 	idx = srcu_read_lock(&vcpu->kvm->srcu);
 
 	switch (data->type) {
@@ -1532,14 +1524,13 @@ static bool wait_pending_event(struct kvm_vcpu *vcpu, bool has_64bit_shinfo,
 	struct kvm *kvm = vcpu->kvm;
 	struct gfn_to_pfn_cache *gpc = &kvm->arch.xen.shinfo_cache;
 	unsigned long *pending_bits;
-	unsigned long flags;
 	bool ret = true;
-	int idx, i;
+	int idx, gpc_idx, i;
 
 	idx = srcu_read_lock(&kvm->srcu);
-	read_lock_irqsave(&gpc->lock, flags);
+	gpc_idx = srcu_read_lock_atomic(&kvm->gpc_srcu);
 	if (!kvm_gpc_check(gpc, PAGE_SIZE))
-		goto out_rcu;
+		goto out_gpc_srcu;
 
 	ret = false;
 	if (has_64bit_shinfo) {
@@ -1557,8 +1548,8 @@ static bool wait_pending_event(struct kvm_vcpu *vcpu, bool has_64bit_shinfo,
 		}
 	}
 
- out_rcu:
-	read_unlock_irqrestore(&gpc->lock, flags);
+ out_gpc_srcu:
+	srcu_read_unlock_atomic(&kvm->gpc_srcu, gpc_idx);
 	srcu_read_unlock(&kvm->srcu, idx);
 
 	return ret;
@@ -1883,6 +1874,15 @@ static void kvm_xen_check_poller(struct kvm_vcpu *vcpu, int port)
 	}
 }
 
+/*
+ * Called with kvm->srcu held, as kvm_gpc_check() consults the memslots.
+ *
+ * The caller passes its own has_64bit_shinfo snapshot, from which it also
+ * derived port_word_bit; re-reading the mode here could race with a
+ * concurrent mode flip and pair a 64-bit bit index with the compat
+ * evtchn_pending_sel offset (or vice versa), writing outside the intended
+ * field within the guest's vcpu_info.
+ */
 static void __kvm_xen_set_evtchn_fast(struct kvm_vcpu *vcpu, int port_word_bit,
 				      bool has_64bit_shinfo)
 {
@@ -1890,15 +1890,15 @@ static void __kvm_xen_set_evtchn_fast(struct kvm_vcpu *vcpu, int port_word_bit,
 	unsigned long vi_pending_sel_ofs;
 	bool inject_upcall = false;
 	bool kick_vcpu = false;
-	unsigned long flags;
 	bool old;
+	int idx;
 
 	vi_pending_sel_ofs = has_64bit_shinfo ?
 		offsetof(struct vcpu_info, evtchn_pending_sel) :
 		offsetof(struct compat_vcpu_info, evtchn_pending_sel);
 
 	/* Now switch to the vCPU's vcpu_info to set the index and pending_sel */
-	read_lock_irqsave(&gpc->lock, flags);
+	idx = srcu_read_lock_atomic(&vcpu->kvm->gpc_srcu);
 	if (!kvm_gpc_check(gpc, sizeof(struct vcpu_info))) {
 		/*
 		 * Could not access the vcpu_info. Set the bit in-kernel and
@@ -1940,14 +1940,17 @@ static void __kvm_xen_set_evtchn_fast(struct kvm_vcpu *vcpu, int port_word_bit,
 	}
 
 out_unlock:
-	read_unlock_irqrestore(&gpc->lock, flags);
+	srcu_read_unlock_atomic(&vcpu->kvm->gpc_srcu, idx);
 
 	/*
 	 * Deliver the upcall or kick the vCPU only after dropping the GPC
 	 * read lock. Both paths end up in kvm_vcpu_kick(), and the MSI
 	 * delivery also walks the APIC map and takes APIC locks; none of
-	 * that wants to be nested inside the GPC read lock, which a
-	 * concurrent invalidation may be waiting to take for write.
+	 * that wants to be nested inside the GPC read-side critical
+	 * section, which must be no longer than the accesses to gpc->khva
+	 * above. Invalidation waits for a grace period, so holding the
+	 * read lock across the kick would extend how long a memory
+	 * invalidation is blocked.
 	 */
 	if (!kick_vcpu)
 		return;
@@ -1982,9 +1985,8 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm)
 	bool has_64bit_shinfo = kvm_xen_has_64bit_shinfo(kvm);
 	unsigned long *pending_bits, *mask_bits;
 	struct kvm_vcpu *vcpu;
-	unsigned long flags;
 	int port_word_bit;
-	int vcpu_idx, idx, rc;
+	int vcpu_idx, idx, gpc_idx, rc;
 
 	vcpu_idx = READ_ONCE(xe->vcpu_idx);
 	if (vcpu_idx >= 0)
@@ -2003,9 +2005,9 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm)
 
 	idx = srcu_read_lock(&kvm->srcu);
 
-	read_lock_irqsave(&gpc->lock, flags);
+	gpc_idx = srcu_read_lock_atomic(&kvm->gpc_srcu);
 	if (!kvm_gpc_check(gpc, PAGE_SIZE))
-		goto out_unlock;
+		goto out_gpc_srcu;
 
 	if (has_64bit_shinfo) {
 		struct shared_info *shinfo = gpc->khva;
@@ -2037,12 +2039,12 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm)
 		rc = 1; /* Delivered to the bitmap in shared_info. */
 	}
 
- out_unlock:
-	read_unlock_irqrestore(&gpc->lock, flags);
+ out_gpc_srcu:
+	srcu_read_unlock_atomic(&kvm->gpc_srcu, gpc_idx);
 
 	/*
 	 * Both of these can kick the vCPU, so keep them outside the GPC
-	 * read lock; an invalidation may be waiting to take it for write.
+	 * read-side critical section; invalidation waits for a grace period.
 	 */
 	if (rc == -ENOTCONN)
 		kvm_xen_check_poller(vcpu, xe->port);
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index cadd0318400e..3ed0b8c9c102 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -798,6 +798,16 @@ struct kvm {
 	/* For management / invalidation of gfn_to_pfn_caches */
 	spinlock_t gpc_lock;
 	struct list_head gpc_list;
+	/*
+	 * Protects the contents of active gfn_to_pfn_caches. Readers hold it
+	 * across kvm_gpc_check() and any subsequent access to gpc->khva; the
+	 * invalidation and refresh paths clear GPC_VALID in gpc->state and
+	 * then wait for a grace period before mutating anything a reader
+	 * might be looking at.
+	 * A dedicated domain (rather than kvm->srcu) keeps those waits from
+	 * being lengthened by unrelated memslot readers.
+	 */
+	struct srcu_struct gpc_srcu;
 
 	/*
 	 * created_vcpus is protected by kvm->lock, and is incremented
@@ -1520,12 +1530,9 @@ int kvm_gpc_activate_hva(struct gfn_to_pfn_cache *gpc, unsigned long hva, unsign
  * @return:	   %true if the cache is still valid and the address matches.
  *		   %false if the cache is not valid.
  *
- * Callers outside IN_GUEST_MODE context should hold a read lock on @gpc->lock
- * while calling this function, and then continue to hold the lock until the
- * access is complete.
- *
- * Callers in IN_GUEST_MODE may do so without locking, although they should
- * still hold a read lock on kvm->scru for the memslot checks.
+ * Callers must hold a read lock on @gpc->kvm->gpc_srcu across this function
+ * and any subsequent access to the target page, and must hold a read lock on
+ * kvm->srcu for the memslot checks.
  */
 bool kvm_gpc_check(struct gfn_to_pfn_cache *gpc, unsigned long len);
 
@@ -1542,8 +1549,8 @@ bool kvm_gpc_check(struct gfn_to_pfn_cache *gpc, unsigned long len);
  * This will attempt to refresh a gfn_to_pfn_cache. Note that a successful
  * return from this function does not mean the page can be immediately
  * accessed because it may have raced with an invalidation. Callers must
- * still lock and check the cache status, as this function does not return
- * with the lock still held to permit access.
+ * still check the cache status under a kvm->gpc_srcu read lock, via
+ * kvm_gpc_check(), before accessing the target page.
  */
 int kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, unsigned long len);
 
@@ -1984,7 +1991,8 @@ static inline bool kvm_is_gpa_in_memslot(struct kvm *kvm, gpa_t gpa)
 
 static inline void kvm_gpc_mark_dirty_in_slot(struct gfn_to_pfn_cache *gpc)
 {
-	lockdep_assert_held(&gpc->lock);
+	RCU_LOCKDEP_WARN(!srcu_read_lock_held(&gpc->kvm->gpc_srcu),
+			 "kvm_gpc_mark_dirty_in_slot() without kvm->gpc_srcu read lock");
 
 	if (!gpc->memslot || gpc->never_dirty)
 		return;
diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h
index e850adc3f47e..22393f97266a 100644
--- a/include/linux/kvm_types.h
+++ b/include/linux/kvm_types.h
@@ -88,15 +88,48 @@ struct gfn_to_pfn_cache {
 	struct kvm_memory_slot *memslot;
 	struct kvm *kvm;
 	struct list_head list;
-	rwlock_t lock;
 	struct mutex refresh_lock;
 	void *khva;
 	kvm_pfn_t pfn;
 	bool active;
-	bool valid;
+	/*
+	 * GPC_VALID, GPC_BECOMING_VALID and GPC_INVALIDATING live in a
+	 * single atomic word so that a refresh can publish
+	 * (BECOMING_VALID => VALID) with a single conditional RMW, an
+	 * invalidation can veto that publish by clearing both of those bits
+	 * (setting INVALIDATING in the same operation if VALID was set, to
+	 * record that its grace period is still owed), and a mutator can
+	 * consume the whole state with one xchg to learn whether it must
+	 * wait for readers before touching anything they see. See the
+	 * comments in pfncache.c.
+	 *
+	 * 'active' deliberately stays separate: the invalidation and
+	 * mutation paths operate on the word without having to care about
+	 * racing with deactivation, which is ordered by refresh_lock
+	 * instead.
+	 */
+	atomic_t state;
 	bool never_dirty;
 };
 
+/* The cache is valid: pfn and khva may be used by readers. */
+#define GPC_VALID		0x1
+/*
+ * A refresh is in flight and intends to publish. Set before the uhva is
+ * resolved, so an invalidation of that uhva can veto the publish even
+ * though the cache is not yet valid.
+ */
+#define GPC_BECOMING_VALID	0x2
+/*
+ * GPC_VALID has been cleared but the grace period for readers of the
+ * previously published mapping has not yet completed. Nothing may mutate
+ * reader-visible fields (pfn, khva, uhva, gpa, memslot) until whoever
+ * next needs to do so has waited it out. Set by whichever party clears
+ * GPC_VALID; cleared after a synchronize_srcu() which began after the
+ * VALID clear.
+ */
+#define GPC_INVALIDATING	0x4
+
 #ifdef KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE
 /*
  * Memory caches are used to preallocate memory ahead of various MMU flows,
diff --git a/include/linux/srcu.h b/include/linux/srcu.h
index 1a8a465a5650..f7a6bbcabc4e 100644
--- a/include/linux/srcu.h
+++ b/include/linux/srcu.h
@@ -706,6 +706,13 @@ DEFINE_LOCK_GUARD_1(srcu, struct srcu_struct,
 DECLARE_LOCK_GUARD_1_ATTRS(srcu, __acquires_shared(_T), __releases_shared(*(struct srcu_struct **)_T))
 #define class_srcu_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(srcu, _T)
 
+DEFINE_LOCK_GUARD_1(srcu_atomic, struct srcu_struct,
+		    _T->idx = srcu_read_lock_atomic(_T->lock),
+		    srcu_read_unlock_atomic(_T->lock, _T->idx),
+		    int idx)
+DECLARE_LOCK_GUARD_1_ATTRS(srcu_atomic, __acquires_shared(_T), __releases_shared(*(struct srcu_struct **)_T))
+#define class_srcu_atomic_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(srcu_atomic, _T)
+
 DEFINE_LOCK_GUARD_1(srcu_fast, struct srcu_struct,
 		    _T->scp = srcu_read_lock_fast(_T->lock),
 		    srcu_read_unlock_fast(_T->lock, _T->scp),
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index cf4cb586f140..a3b4ca1ae88d 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1160,6 +1160,8 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname)
 		goto out_err_no_srcu;
 	if (init_srcu_struct(&kvm->irq_srcu))
 		goto out_err_no_irq_srcu;
+	if (init_srcu_struct_atomic(&kvm->gpc_srcu))
+		goto out_err_no_gpc_srcu;
 
 	r = kvm_init_irq_routing(kvm);
 	if (r)
@@ -1240,6 +1242,8 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname)
 		kfree(kvm_get_bus_for_destruction(kvm, i));
 	kvm_free_irq_routing(kvm);
 out_err_no_irq_routing:
+	cleanup_srcu_struct(&kvm->gpc_srcu);
+out_err_no_gpc_srcu:
 	cleanup_srcu_struct(&kvm->irq_srcu);
 out_err_no_irq_srcu:
 	cleanup_srcu_struct(&kvm->srcu);
@@ -1316,6 +1320,11 @@ static void kvm_destroy_vm(struct kvm *kvm)
 		kvm_free_memslots(kvm, &kvm->__memslots[i][0]);
 		kvm_free_memslots(kvm, &kvm->__memslots[i][1]);
 	}
+	/*
+	 * All gfn_to_pfn_caches have been deactivated by kvm_arch_destroy_vm()
+	 * above, so there can be no readers or pending invalidations left.
+	 */
+	cleanup_srcu_struct(&kvm->gpc_srcu);
 	cleanup_srcu_struct(&kvm->irq_srcu);
 	srcu_barrier(&kvm->srcu);
 	cleanup_srcu_struct(&kvm->srcu);
diff --git a/virt/kvm/pfncache.c b/virt/kvm/pfncache.c
index 17a30da00d40..1c98b7bdbf09 100644
--- a/virt/kvm/pfncache.c
+++ b/virt/kvm/pfncache.c
@@ -26,35 +26,149 @@ void gfn_to_pfn_cache_invalidate_start(struct kvm *kvm, unsigned long start,
 				       unsigned long end)
 {
 	struct gfn_to_pfn_cache *gpc;
+	bool cleared = false;
 
 	spin_lock(&kvm->gpc_lock);
+
+	/*
+	 * Order the caller's earlier mn_active_invalidate_count increment
+	 * (and gpc_invalidate_seq bump, for range_end) before the state
+	 * reads below. This is a store->load ordering, which neither the
+	 * unlock/lock the increment travelled through (RCtso) nor the
+	 * acquire on the state read provides generically. It pairs with
+	 * smp_mb__after_atomic() in hva_to_pfn_retry(): whichever side's
+	 * full barrier executes second guarantees that either this walk
+	 * observes GPC_BECOMING_VALID and vetoes the pending publish, or
+	 * the refresh observes the elevated count / bumped seq and
+	 * retries — closing the store-buffering window in which each
+	 * side misses the other and a stale pfn gets published. (On x86
+	 * both sides' locked RMW operations already provide this; generic
+	 * code must not rely on that.)
+	 */
+	smp_mb__after_spinlock();
+
 	list_for_each_entry(gpc, &kvm->gpc_list, list) {
-		read_lock_irq(&gpc->lock);
+		/*
+		 * A cache covers only a single page, keyed by its uhva, so
+		 * there is no length to consider.
+		 *
+		 * The acquire pairs with the release-publish in
+		 * hva_to_pfn_retry() (for GPC_VALID) and with the
+		 * smp_mb__before_atomic() before the GPC_BECOMING_VALID
+		 * announce (for that bit): whichever is seen, the uhva read
+		 * below is guaranteed to see the value stored before it. A
+		 * stale uhva paired with a fresh state bit could otherwise
+		 * cause a cache whose new uhva is in the invalidated range
+		 * to be skipped.
+		 *
+		 * uhva is read once, with READ_ONCE(), and only after the
+		 * state (the acquire provides that ordering): it is written
+		 * (with WRITE_ONCE()) by __kvm_gpc_refresh() without holding
+		 * gpc_lock. Note the refresh only mutates uhva while the
+		 * state word is 0 — after its xchg, before its announce —
+		 * so this walk cannot observe a half-updated cache: if
+		 * either state bit is visible, the uhva is current.
+		 *
+		 * Keying on GPC_BECOMING_VALID as well as GPC_VALID means a
+		 * cache whose refresh is in flight is *not* skipped: its
+		 * uhva is already known at that point, and clearing the bit
+		 * vetoes the pending publish. Testing only GPC_VALID would
+		 * leave the window between the refresh's last invalidation
+		 * check and its publish unprotected. (For an invalidation
+		 * which begins before that check, this veto is belt and
+		 * braces: the mn_active_invalidate_count / gpc_invalidate_seq
+		 * check makes the refresh retry anyway. The window after
+		 * that check has only the veto.)
+		 *
+		 * Keying on GPC_INVALIDATING too means a cache whose
+		 * VALID-clearing party is still mid-grace-period is not
+		 * skipped either: see the comment at the 'cleared' update
+		 * below.
+		 */
+		unsigned long gpc_uhva;
+		int old, new;
 
-		/* Only a single page so no need to care about length */
-		if (gpc->valid && !is_error_noslot_pfn(gpc->pfn) &&
-		    gpc->uhva >= start && gpc->uhva < end) {
-			read_unlock_irq(&gpc->lock);
+		old = atomic_read_acquire(&gpc->state);
+		if (!(old & (GPC_VALID | GPC_BECOMING_VALID | GPC_INVALIDATING)))
+			continue;
 
+		gpc_uhva = READ_ONCE(gpc->uhva);
+		if (gpc_uhva >= start && gpc_uhva < end) {
 			/*
-			 * There is a small window here where the cache could
-			 * be modified, and invalidation would no longer be
-			 * necessary. Hence check again whether invalidation
-			 * is still necessary once the write lock has been
-			 * acquired.
+			 * Clear VALID and BECOMING_VALID; if VALID was set,
+			 * also set INVALIDATING so that anyone else who wants
+			 * to mutate reader-visible fields before our grace
+			 * period below completes knows to wait for one of
+			 * their own. (A cache which was merely becoming
+			 * valid has no readers: the veto alone suffices.)
+			 *
+			 * 'old' is seeded from the read above; if the state
+			 * changed since (e.g. the refresh published), the
+			 * cmpxchg fails, reloads it, and goes around.
 			 */
+			do {
+				new = old & ~(GPC_VALID | GPC_BECOMING_VALID);
+				if (old & GPC_VALID)
+					new |= GPC_INVALIDATING;
+			} while (!atomic_try_cmpxchg(&gpc->state, &old, new));
 
-			write_lock_irq(&gpc->lock);
-			if (gpc->valid && !is_error_noslot_pfn(gpc->pfn) &&
-			    gpc->uhva >= start && gpc->uhva < end)
-				gpc->valid = false;
-			write_unlock_irq(&gpc->lock);
-			continue;
+			/*
+			 * Wait below not only when this walk itself cleared
+			 * GPC_VALID, but also when GPC_INVALIDATING alone
+			 * was found: the party which cleared GPC_VALID (a
+			 * concurrent walk, or a refresh/deactivation mid
+			 * grace-period — they keep the marker up until their
+			 * wait completes) may still be inside its own
+			 * synchronize, with readers outstanding on the old
+			 * mapping. Those readers are covered by *its* grace
+			 * period, which this walk's caller does not wait
+			 * for; it must begin (and finish) one of its own
+			 * before the caller frees the page.
+			 */
+			if (old & (GPC_VALID | GPC_INVALIDATING))
+				cleared = true;
 		}
-
-		read_unlock_irq(&gpc->lock);
 	}
 	spin_unlock(&kvm->gpc_lock);
+
+	/*
+	 * Readers may still be using the old mapping, having sampled
+	 * GPC_VALID before it was cleared. Wait for them all to drain
+	 * before the caller proceeds to zap the page tables; like a TLB
+	 * shootdown, this guarantees no access via the stale mapping
+	 * once the invalidation completes.
+	 *
+	 * This wait is safe even on unblockable ranges (the OOM reaper):
+	 * the constraint there is not "no sleeping" but "no blocking on
+	 * anything which may itself depend on memory allocation to make
+	 * progress" (see the reasoning in commit 312364f3534c and its
+	 * discussion). A GPC read-side critical section has no such
+	 * dependency: readers never allocate, never take mmap_lock, and
+	 * never sleep.
+	 *
+	 * The atomic variant: this stalls progress of the primary MMU
+	 * operation which triggered the invalidation, and the readers being
+	 * drained are bounded, short, non-sleeping sections, so the wait
+	 * spins (preemption disabled) rather than sleeping — and never
+	 * queues a callback, so it depends on no allocation at all. That
+	 * satisfies even the strictest reading of the OOM-reaper
+	 * requirement; it is also the only wait primitive permitted on a
+	 * domain whose readers are the atomic flavor.
+	 */
+	if (cleared)
+		synchronize_srcu_atomic(&kvm->gpc_srcu);
+
+	/*
+	 * Note the GPC_INVALIDATING markers set above are deliberately NOT
+	 * cleared here: this walk dropped gpc_lock before waiting, so it
+	 * cannot tell whether the marker it would clear is still the one
+	 * it set (consider a republish-and-reinvalidate cycle completing
+	 * entirely within this synchronize_srcu()). Leaving it standing is
+	 * merely conservative: a later walk over the same uhva pays a
+	 * redundant grace period, and the next mutation of the cache (a
+	 * refresh or deactivation, which performs its own wait) retires
+	 * it. Readers ignore the bit.
+	 */
 }
 
 static bool kvm_gpc_is_valid_len(gpa_t gpa, unsigned long uhva,
@@ -74,14 +188,31 @@ bool kvm_gpc_check(struct gfn_to_pfn_cache *gpc, unsigned long len)
 {
 	struct kvm_memslots *slots = kvm_memslots(gpc->kvm);
 
-	if (!gpc->active)
+	RCU_LOCKDEP_WARN(!srcu_read_lock_held(&gpc->kvm->gpc_srcu),
+			 "kvm_gpc_check() without kvm->gpc_srcu read lock");
+
+	/*
+	 * Check valid *first*. The acquire pairs with the release-publish
+	 * in hva_to_pfn_retry(), so every field read below — and any use
+	 * of gpc->khva by the caller — is guaranteed to be from the
+	 * published generation, not a stale value reordered from before
+	 * the publish. The fields are then stable for the remainder of
+	 * the RCU read-side critical section, because every mutator
+	 * clears valid and waits a full grace period before changing
+	 * anything.
+	 */
+	if (!(atomic_read_acquire(&gpc->state) & GPC_VALID))
+		return false;
+
+	if (!READ_ONCE(gpc->active))
 		return false;
 
 	/*
 	 * If the page was cached from a memslot, make sure the memslots have
 	 * not been re-configured.
 	 */
-	if (!kvm_is_error_gpa(gpc->gpa) && gpc->generation != slots->generation)
+	if (!kvm_is_error_gpa(gpc->gpa) &&
+	    READ_ONCE(gpc->generation) != slots->generation)
 		return false;
 
 	if (kvm_is_error_hva(gpc->uhva))
@@ -90,9 +221,6 @@ bool kvm_gpc_check(struct gfn_to_pfn_cache *gpc, unsigned long len)
 	if (!kvm_gpc_is_valid_len(gpc->gpa, gpc->uhva, len))
 		return false;
 
-	if (!gpc->valid)
-		return false;
-
 	return true;
 }
 
@@ -134,8 +262,8 @@ static inline bool mmu_notifier_retry_cache(struct kvm *kvm, unsigned long gpc_s
 	 * is elevated.
 	 *
 	 * Note, it does not matter that mn_active_invalidate_count
-	 * is not protected by gpc->lock.  It is guaranteed to
-	 * be elevated before the mmu_notifier acquires gpc->lock, and
+	 * is not protected by any lock the refresher holds.  It is
+	 * guaranteed to be elevated before the mmu_notifier walk, and
 	 * isn't dropped until after gpc_invalidate_seq is updated.
 	 */
 	if (kvm->mn_active_invalidate_count)
@@ -156,10 +284,13 @@ static kvm_pfn_t hva_to_pfn_retry(struct gfn_to_pfn_cache *gpc)
 {
 	/* Note, the new page offset may be different than the old! */
 	void *old_khva = (void *)PAGE_ALIGN_DOWN((uintptr_t)gpc->khva);
+	void *old_khva_exact = gpc->khva;
+	kvm_pfn_t old_pfn = gpc->pfn;
 	kvm_pfn_t new_pfn = KVM_PFN_ERR_FAULT;
 	void *new_khva = NULL;
 	unsigned long gpc_seq;
 	struct page *page;
+	int state;
 
 	struct kvm_follow_pfn kfp = {
 		.slot = gpc->memslot,
@@ -171,21 +302,42 @@ static kvm_pfn_t hva_to_pfn_retry(struct gfn_to_pfn_cache *gpc)
 
 	lockdep_assert_held(&gpc->refresh_lock);
 
-	lockdep_assert_held_write(&gpc->lock);
-
-	/*
-	 * Invalidate the cache prior to dropping gpc->lock, the gpa=>uhva
-	 * assets have already been updated and so a concurrent check() from a
-	 * different task may not fail the gpa/uhva/generation checks.
-	 */
-	gpc->valid = false;
+	WARN_ON_ONCE(atomic_read(&gpc->state) & GPC_VALID);
 
 	do {
+		/*
+		 * Announce the intent to publish before the uhva is resolved
+		 * and before any lock is dropped. From here on an
+		 * invalidation of this uhva will clear the bit, and the
+		 * cmpxchg below will then fail and send us around again.
+		 * This is what makes the check-and-publish atomic with
+		 * respect to the mmu_notifier walk without holding a lock
+		 * across it.
+		 *
+		 * The barrier orders the caller's gpc->uhva store before the
+		 * bit becoming visible (atomic_or() alone is unordered, and
+		 * x86's lock-or being a full barrier must not be relied on
+		 * in generic code). It pairs with the atomic_read_acquire()
+		 * in gfn_to_pfn_cache_invalidate_start(): a walker which
+		 * observes the bit observes the uhva this refresh is
+		 * resolving, and so aims its veto correctly.
+		 */
+		smp_mb__before_atomic();
+		atomic_or(GPC_BECOMING_VALID, &gpc->state);
+
+		/*
+		 * Order the announce store before the gpc_invalidate_seq
+		 * and mn_active_invalidate_count reads which follow
+		 * (store->load; atomic_or() alone is unordered). Pairs
+		 * with smp_mb__after_spinlock() in
+		 * gfn_to_pfn_cache_invalidate_start(); see the comment
+		 * there for the store-buffering window this closes.
+		 */
+		smp_mb__after_atomic();
+
 		gpc_seq = gpc->kvm->gpc_invalidate_seq;
 		smp_rmb();
 
-		write_unlock_irq(&gpc->lock);
-
 		/*
 		 * If the previous iteration "failed" due to an mmu_notifier
 		 * event, release the pfn and unmap the kernel virtual address
@@ -213,9 +365,15 @@ static kvm_pfn_t hva_to_pfn_retry(struct gfn_to_pfn_cache *gpc)
 		/*
 		 * Obtain a new kernel mapping if KVM itself will access the
 		 * pfn.  Note, kmap() and memremap() can both sleep, so this
-		 * too must be done outside of gpc->lock!
+		 * can sleep, which is fine: this path holds no spinning locks.
 		 */
-		if (new_pfn == gpc->pfn)
+		/*
+		 * Compare against the *published* pfn captured at entry, via
+		 * a local: gpc->pfn must not be consulted mid-loop. (A
+		 * previous iteration's vetoed publish attempt must not be
+		 * able to leak its doomed pfn into this comparison.)
+		 */
+		if (new_pfn == old_pfn)
 			new_khva = old_khva;
 		else
 			new_khva = gpc_map(new_pfn);
@@ -225,18 +383,39 @@ static kvm_pfn_t hva_to_pfn_retry(struct gfn_to_pfn_cache *gpc)
 			goto out_error;
 		}
 
-		write_lock_irq(&gpc->lock);
+		if (mmu_notifier_retry_cache(gpc->kvm, gpc_seq))
+			continue;
+
+		gpc->pfn = new_pfn;
+		gpc->khva = new_khva + offset_in_page(gpc->uhva);
 
 		/*
-		 * Other tasks must wait for _this_ refresh to complete before
-		 * attempting to refresh.
+		 * Publish: BECOMING_VALID => VALID, but only if the bit is
+		 * still set. If an invalidation of this uhva landed at any
+		 * point since it was set — including in the window after the
+		 * retry check just above — the bit is gone, the cmpxchg fails
+		 * and we go around again. The release ordering pairs with the
+		 * atomic_read_acquire() in kvm_gpc_check() so the pfn/khva
+		 * stores are visible before GPC_VALID is. (The stores must
+		 * precede the VALID publish; a reader which observes VALID
+		 * must observe the fields it covers.)
 		 */
-		WARN_ON_ONCE(gpc->valid);
-	} while (mmu_notifier_retry_cache(gpc->kvm, gpc_seq));
+		state = GPC_BECOMING_VALID;
+		if (atomic_try_cmpxchg_release(&gpc->state, &state, GPC_VALID))
+			break;
 
-	gpc->valid = true;
-	gpc->pfn = new_pfn;
-	gpc->khva = new_khva + offset_in_page(gpc->uhva);
+		/*
+		 * Vetoed: an invalidation cleared GPC_BECOMING_VALID after
+		 * the retry check above. No reader ever saw the values just
+		 * stored (VALID was never set for them), but the loop's own
+		 * cleanup and aliasing logic must not see them either —
+		 * restore the published values. This runs under refresh_lock
+		 * with the cache unpublished, so it is single-writer plain
+		 * stores.
+		 */
+		gpc->pfn = old_pfn;
+		gpc->khva = old_khva_exact;
+	} while (1);
 
 	/*
 	 * Put the reference to the _new_ page.  The page is now tracked by the
@@ -248,8 +427,19 @@ static kvm_pfn_t hva_to_pfn_retry(struct gfn_to_pfn_cache *gpc)
 	return 0;
 
 out_error:
-	write_lock_irq(&gpc->lock);
-
+	/*
+	 * Withdraw the announcement: no publish is coming. Leaving the bit
+	 * set would be harmless — an invalidation walk would merely veto a
+	 * publish which was never going to happen, and the next refresh or
+	 * deactivation starts by xchg'ing the whole state word to zero —
+	 * but clearing it here keeps the invariant that GPC_BECOMING_VALID
+	 * means a refresh is between announce and publish. Only VALID can
+	 * have been set here by anyone else, and only by our own publish
+	 * (which didn't happen), and INVALIDATING only by whoever clears
+	 * VALID; the sole concurrent writer is an invalidation walk also
+	 * clearing this bit, so a plain andnot cannot lose an update.
+	 */
+	atomic_andnot(GPC_BECOMING_VALID, &gpc->state);
 	return -EFAULT;
 }
 
@@ -259,7 +449,7 @@ static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned l
 	bool unmap_old = false;
 	unsigned long old_uhva;
 	kvm_pfn_t old_pfn;
-	bool hva_change = false;
+	bool must_drain;
 	void *old_khva;
 	int ret;
 
@@ -269,13 +459,103 @@ static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned l
 
 	lockdep_assert_held(&gpc->refresh_lock);
 
-	write_lock_irq(&gpc->lock);
+	if (!gpc->active)
+		return -EINVAL;
 
-	if (!gpc->active) {
-		ret = -EINVAL;
-		goto out_unlock;
+	/*
+	 * Resolve the target uHVA (and memslot, for a GPA-based cache) before
+	 * deciding whether anything needs to be invalidated at all.
+	 *
+	 * If the cache is still valid and this refresh resolves to exactly the
+	 * same uHVA, in the same memslot, for the same GPA, then nothing which
+	 * a reader can observe is changing: the gPA => uHVA translation is
+	 * unchanged, and GPC_VALID being set is precisely the assertion that
+	 * the second stage (uHVA => PFN, and the kernel mapping of it) is
+	 * still good. No mapping is retired, so there is nothing for a grace
+	 * period to wait for. At most the memslot generation needs updating,
+	 * which no reader consults except via kvm_gpc_check() itself.
+	 *
+	 * All of the following must hold to take this shortcut:
+	 *
+	 *  - GPC_VALID: the PFN and its kernel mapping are still good.
+	 *  - the resolved uHVA is unchanged. kvm_gpc_check() validates
+	 *    'offset + len <= PAGE_SIZE' from gpc->uhva/gpa on the reader's
+	 *    behalf, and the reader then accesses gpc->khva for 'len' bytes
+	 *    with no recheck; moving the offset under a live reader would let
+	 *    it run off the end of the page.
+	 *  - the memslot is unchanged. kvm_gpc_mark_dirty_in_slot() uses
+	 *    gpc->memslot, so replacing it under a reader could mark the wrong
+	 *    slot dirty and thus lose a dirty page for live migration.
+	 *  - the gPA is unchanged, as it provides the gfn for dirty tracking.
+	 */
+	if (atomic_read(&gpc->state) & GPC_VALID) {
+		struct kvm_memory_slot *new_slot = gpc->memslot;
+		unsigned long new_uhva = KVM_HVA_ERR_BAD;
+		u64 new_generation = gpc->generation;
+
+		if (kvm_is_error_gpa(gpa)) {
+			new_uhva = uhva;
+		} else {
+			struct kvm_memslots *slots = kvm_memslots(gpc->kvm);
+			gfn_t gfn = gpa_to_gfn(gpa);
+
+			new_generation = slots->generation;
+			new_slot = __gfn_to_memslot(slots, gfn);
+			if (new_slot)
+				new_uhva = gfn_to_hva_memslot(new_slot, gfn) +
+					   offset_in_page(gpa);
+		}
+
+		if (!kvm_is_error_hva(new_uhva) && new_uhva == gpc->uhva &&
+		    new_slot == gpc->memslot && gpa == gpc->gpa) {
+			/*
+			 * Nothing to invalidate. A concurrent reader may be
+			 * using the cache right now and can safely continue
+			 * to do so; only the memslot generation, which is
+			 * read solely by kvm_gpc_check(), may need updating.
+			 */
+			if (new_generation != gpc->generation)
+				WRITE_ONCE(gpc->generation, new_generation);
+
+			return 0;
+		}
 	}
 
+	/*
+	 * Take the cache invalid and wait for all current readers to
+	 * drain before mutating anything they might be looking at. Once
+	 * the grace period has elapsed, this task (serialized by
+	 * refresh_lock) owns all the cache fields exclusively: readers
+	 * check valid (with an acquire load) inside their SRCU read-side
+	 * critical sections and back off. This mirrors what a TLB
+	 * shootdown does for the hardware page tables.
+	 *
+	 * The clear and the test are one operation on purpose, and the wait
+	 * triggers on GPC_INVALIDATING as well as GPC_VALID: an mmu_notifier
+	 * invalidation which cleared GPC_VALID a moment ago is not
+	 * necessarily finished — its grace period may still be running, with
+	 * readers which sampled GPC_VALID before the clear still
+	 * dereferencing gpc->khva. GPC_INVALIDATING is set by whoever clears
+	 * GPC_VALID precisely so that this path knows those readers may
+	 * exist. Our own grace-period wait below necessarily begins after
+	 * that clear, so waiting here covers them; only then may
+	 * reader-visible fields (uhva, gpa, memslot, and later pfn/khva) be
+	 * mutated or the old mapping torn down.
+	 *
+	 * The xchg leaves GPC_INVALIDATING *standing* (rather than clearing
+	 * the word outright) for the duration of the wait, for the same
+	 * reason it is tested: an mmu_notifier walk which arrives while
+	 * this grace period is still running must not read a zero state,
+	 * skip the cache, and let its caller free the page under readers
+	 * only this wait covers. The marker comes down only once the drain
+	 * is complete and the readers it advertised cannot exist.
+	 */
+	must_drain = atomic_xchg(&gpc->state, GPC_INVALIDATING) &
+		     (GPC_VALID | GPC_INVALIDATING);
+	if (must_drain)
+		synchronize_srcu_atomic(&gpc->kvm->gpc_srcu);
+	atomic_set(&gpc->state, 0);
+
 	old_pfn = gpc->pfn;
 	old_khva = (void *)PAGE_ALIGN_DOWN((uintptr_t)gpc->khva);
 	old_uhva = PAGE_ALIGN_DOWN(gpc->uhva);
@@ -285,10 +565,14 @@ static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned l
 
 		gpc->gpa = INVALID_GPA;
 		gpc->memslot = NULL;
-		gpc->uhva = PAGE_ALIGN_DOWN(uhva);
-
-		if (gpc->uhva != old_uhva)
-			hva_change = true;
+		/*
+		 * uhva stores are WRITE_ONCE(): the invalidation walks read
+		 * it without holding refresh_lock. This mutation window is
+		 * safe because the state word is 0 here (cleared by the
+		 * xchg above, announced again only in hva_to_pfn_retry()
+		 * after these stores), so no walk will match this cache.
+		 */
+		WRITE_ONCE(gpc->uhva, PAGE_ALIGN_DOWN(uhva));
 	} else {
 		struct kvm_memslots *slots = kvm_memslots(gpc->kvm);
 
@@ -301,43 +585,31 @@ static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned l
 			gpc->gpa = gpa;
 			gpc->generation = slots->generation;
 			gpc->memslot = __gfn_to_memslot(slots, gfn);
-			gpc->uhva = gfn_to_hva_memslot(gpc->memslot, gfn);
+			WRITE_ONCE(gpc->uhva, gfn_to_hva_memslot(gpc->memslot, gfn));
 
 			if (kvm_is_error_hva(gpc->uhva)) {
 				ret = -EFAULT;
 				goto out;
 			}
-
-			/*
-			 * Even if the GPA and/or the memslot generation changed, the
-			 * HVA may still be the same.
-			 */
-			if (gpc->uhva != old_uhva)
-				hva_change = true;
 		} else {
-			gpc->uhva = old_uhva;
+			WRITE_ONCE(gpc->uhva, old_uhva);
 		}
 	}
 
 	/* Note: the offset must be correct before calling hva_to_pfn_retry() */
-	gpc->uhva += page_offset;
+	WRITE_ONCE(gpc->uhva, gpc->uhva + page_offset);
 
 	/*
-	 * If the userspace HVA changed or the PFN was already invalid,
-	 * drop the lock and do the HVA to PFN lookup again.
+	 * Always redo the HVA to PFN lookup: an invalidation of our uhva
+	 * may have raced with (or followed) the valid-clearing above, in
+	 * which case the notifier walk skipped this cache (valid was
+	 * already false) and the old pfn may already be stale. The
+	 * gpc_invalidate_seq check in hva_to_pfn_retry() is what detects
+	 * that race, so the lookup path is the only safe way to publish.
+	 * If the mapping is in fact unchanged, hva_to_pfn_retry() reuses
+	 * the existing kernel mapping for the same pfn.
 	 */
-	if (!gpc->valid || hva_change) {
-		ret = hva_to_pfn_retry(gpc);
-	} else {
-		/*
-		 * If the HVA→PFN mapping was already valid, don't unmap it.
-		 * But do update gpc->khva because the offset within the page
-		 * may have changed.
-		 */
-		gpc->khva = old_khva + page_offset;
-		ret = 0;
-		goto out_unlock;
-	}
+	ret = hva_to_pfn_retry(gpc);
 
  out:
 	/*
@@ -346,17 +618,12 @@ static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned l
 	 * valid, leave it as is.
 	 */
 	if (ret) {
-		gpc->valid = false;
+		WARN_ON_ONCE(atomic_read(&gpc->state) & GPC_VALID);
 		gpc->pfn = KVM_PFN_ERR_FAULT;
 		gpc->khva = NULL;
 	}
 
-	/* Detect a pfn change before dropping the lock! */
 	unmap_old = (old_pfn != gpc->pfn);
-
-out_unlock:
-	write_unlock_irq(&gpc->lock);
-
 	if (unmap_old)
 		gpc_unmap(old_pfn, old_khva);
 
@@ -385,14 +652,14 @@ int kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, unsigned long len)
 void __kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct kvm *kvm,
 		    bool never_dirty)
 {
-	rwlock_init(&gpc->lock);
 	mutex_init(&gpc->refresh_lock);
 
 	gpc->kvm = kvm;
 	gpc->pfn = KVM_PFN_ERR_FAULT;
 	gpc->gpa = INVALID_GPA;
 	gpc->uhva = KVM_HVA_ERR_BAD;
-	gpc->active = gpc->valid = false;
+	gpc->active = false;
+	atomic_set(&gpc->state, 0);
 	gpc->never_dirty = never_dirty;
 }
 
@@ -407,7 +674,7 @@ static int __kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned
 	guard(mutex)(&gpc->refresh_lock);
 
 	if (!gpc->active) {
-		if (KVM_BUG_ON(gpc->valid, kvm))
+		if (KVM_BUG_ON(atomic_read(&gpc->state) & GPC_VALID, kvm))
 			return -EIO;
 
 		spin_lock(&kvm->gpc_lock);
@@ -417,11 +684,11 @@ static int __kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned
 		/*
 		 * Activate the cache after adding it to the list, a concurrent
 		 * refresh must not establish a mapping until the cache is
-		 * reachable by mmu_notifier events.
+		 * reachable by mmu_notifier events. (Refreshes are serialized
+		 * by refresh_lock, which we hold; the store ordering matters
+		 * only against the notifier walk, which holds gpc_lock.)
 		 */
-		write_lock_irq(&gpc->lock);
-		gpc->active = true;
-		write_unlock_irq(&gpc->lock);
+		WRITE_ONCE(gpc->active, true);
 	}
 	return __kvm_gpc_refresh(gpc, gpa, uhva);
 }
@@ -456,13 +723,28 @@ void kvm_gpc_deactivate(struct gfn_to_pfn_cache *gpc)
 
 	if (gpc->active) {
 		/*
-		 * Deactivate the cache before removing it from the list, KVM
-		 * must stall mmu_notifier events until all users go away, i.e.
-		 * until gpc->lock is dropped and refresh is guaranteed to fail.
+		 * Mark the cache inactive and invalid, and wait for all
+		 * current readers to drain, before tearing down the mapping
+		 * they may have been using. Refreshes are excluded by
+		 * refresh_lock, which we hold.
+		 */
+		WRITE_ONCE(gpc->active, false);
+
+		/*
+		 * As in __kvm_gpc_refresh(): clear and test in one
+		 * operation, waiting on GPC_INVALIDATING as well as
+		 * GPC_VALID. An invalidation which cleared GPC_VALID first
+		 * may still be inside its own synchronize_srcu() with
+		 * readers outstanding — it left GPC_INVALIDATING set to say
+		 * so — and the unmap below must not run under those readers.
+		 * And, also as there, leave GPC_INVALIDATING standing until
+		 * the drain completes, so that an mmu_notifier walk arriving
+		 * mid-wait does not read a zero state and skip the cache.
 		 */
-		write_lock_irq(&gpc->lock);
-		gpc->active = false;
-		gpc->valid = false;
+		if (atomic_xchg(&gpc->state, GPC_INVALIDATING) &
+		    (GPC_VALID | GPC_INVALIDATING))
+			synchronize_srcu_atomic(&kvm->gpc_srcu);
+		atomic_set(&gpc->state, 0);
 
 		/*
 		 * Leave the GPA => uHVA cache intact, it's protected by the
@@ -475,7 +757,6 @@ void kvm_gpc_deactivate(struct gfn_to_pfn_cache *gpc)
 
 		old_pfn = gpc->pfn;
 		gpc->pfn = KVM_PFN_ERR_FAULT;
-		write_unlock_irq(&gpc->lock);
 
 		spin_lock(&kvm->gpc_lock);
 		list_del(&gpc->list);
-- 
2.43.0


  parent reply	other threads:[~2026-11-05  2:01 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-05  2:00 [PATCH v2 00/20] KVM: Use atomic SRCU for gfn-to-pfn cache, reinstate guest mode for x86 nesting David Woodhouse
2026-11-05  2:00 ` [PATCH v2 01/20] KVM: x86/xen: Fill the hypercall page with a single guest write David Woodhouse
2026-11-05  2:00 ` [PATCH v2 02/20] KVM: x86/xen: Add KVM_XEN_VCPU_ATTR_TYPE_WRITE_HYPERCALL_PAGE David Woodhouse
2026-11-05  2:00 ` [PATCH v2 03/20] KVM: x86/xen: Extract delivery of event to vCPU into a separate helper David Woodhouse
2026-11-05  2:00 ` [PATCH v2 04/20] KVM: x86/xen: Explicitly tag "shared info" page as never being dirty tracked David Woodhouse
2026-11-05  2:00 ` [PATCH v2 05/20] KVM: x86/xen: Don't dirty track "vCPU info" page David Woodhouse
2026-11-05  2:00 ` [PATCH v2 06/20] KVM: x86: Request the guest TLB flush from record_steal_time() David Woodhouse
2026-11-05  2:00 ` [PATCH v2 07/20] KVM: selftests: Actually deactivate the HVA-based shared_info cache in xen_shinfo_test David Woodhouse
2026-11-05  2:00 ` David Woodhouse [this message]
2026-11-05  2:00 ` [PATCH v2 09/20] KVM: x86: Use gfn_to_pfn_cache for steal time / preempted status David Woodhouse
2026-11-05  2:00 ` [PATCH v2 10/20] KVM: pfncache: Add guest-mode pinning (GUEST_USES_PFN successor) David Woodhouse
2026-11-05  2:29   ` sashiko-bot
2026-11-05  2:00 ` [PATCH v2 11/20] KVM: nVMX: Implement cache for L1 MSR bitmap David Woodhouse
2026-11-05  2:00 ` [PATCH v2 12/20] KVM: nVMX: Use pinned pfncache for L1 APIC virtualization pages David Woodhouse
2026-11-05  2:33   ` sashiko-bot
2026-11-05  2:00 ` [PATCH v2 13/20] KVM: x86: Post KVM_REQ_GET_NESTED_STATE_PAGES on memslot updates David Woodhouse
2026-11-05  2:00 ` [PATCH v2 14/20] KVM: selftests: Add nested VMX APIC cache invalidation test David Woodhouse
2026-11-05  2:25   ` sashiko-bot
2026-11-05  2:00 ` [PATCH v2 15/20] KVM: nSVM: Use a gfn_to_pfn_cache for the vmcb12 page David Woodhouse
2026-11-05  2:00 ` [PATCH v2 16/20] KVM: nSVM: Cache L1's MSR permissions map pages David Woodhouse
2026-11-05  2:00 ` [PATCH v2 17/20] KVM: nSVM: Cache L1's IO " David Woodhouse
2026-11-05  2:01 ` [PATCH v2 18/20] KVM: selftests: Add nested transition benchmark David Woodhouse
2026-11-05  2:01 ` [PATCH v2 19/20] KVM: selftests: Add Xen evtchn vs memslot-update race test David Woodhouse
2026-11-05  2:01 ` [PATCH v2 20/20] KVM: selftests: Add Xen evtchn vs mmu_notifier race test for HVA caches David Woodhouse
2026-11-05  2:28   ` sashiko-bot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20261105020104.2507083-9-dwmw2@infradead.org \
    --to=dwmw2@infradead.org \
    --cc=dwmw@amazon.co.uk \
    --cc=fgriffo@amazon.co.uk \
    --cc=kunwu.chan@gmail.com \
    --cc=kunwu.chan@linux.dev \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-rt-devel@lists.linux.dev \
    --cc=paul@xen.org \
    --cc=paulmck@kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=rcu@vger.kernel.org \
    --cc=seanjc@google.com \
    --cc=syzbot+208f7f3e5f59c11aeb90@syzkaller.appspotmail.com \
    --cc=vkuznets@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®