[PATCH v2 00/11] KVM: x86/xen: Don't drop event deliveries which race memslot updates

[PATCH v2 00/11] KVM: x86/xen: Don't drop event deliveries which race memslot updates

[David Woodhouse]

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

Sashiko analysis and torture testing of the upcoming conversion of the
gfn_to_pfn_cache from rwlock to atomic SRCU¹ highlighted a pre-existing
problem: refreshing a cache can fail while a memslot is briefly marked
KVM_MEMSLOT_INVALID during a memslot update. The Xen event channel and
timer delivery paths silently dropped events when they hit that window.

Return -EAGAIN from __kvm_gpc_refresh() to distinguish the transient
case from a genuinely unbacked GFN, and add a retry loop for the common
case callers which can just wait for the memslot update to complete, as
well as dealing with the corner cases which can't:

 - The Xen attribute ioctls wait, so userspace never sees the
   transient state.

 - Timer delivery leaves ->arch.xen.timer_pending set and retries from
   kvm_xen_inject_timer_irqs() on the next vCPU entry.

 - The guest's own EVTCHNOP_send hypercall runs under vcpu_run's
   kvm->srcu read section — the very reader the memslot update is
   waiting for — so it cannot wait. Leave the hypercall uncompleted
   without advancing RIP, so the guest re-executes it. This is how
   Xen itself restarts preempted hypercalls.

Patch 3 steps the hypercall page MSR write out of the vCPU's kvm->srcu
read section around xen_lock, so an ioctl waiting out a memslot update
while holding that lock cannot deadlock it.

Patches 7-10 are pulled forward from the SRCU series because they stand
alone:

 - Three from Sean², extracting vcpu_info delivery into a separate
   helper and taking the shared_info and vcpu_info pages out of dirty
   tracking. Rebased: his series had first converted the GPC locks to
   read_trylock(), which this one doesn't carry.

 - Requesting the guest TLB flush from record_steal_time() itself, as
   a standalone cleanup in preparation for converting that to GPC.

The final patch adds a selftest racing a guest EVTCHNOP_send loop
against a tight memslot delete/recreate loop. Tested with that and
xen_shinfo_test; an instrumented run showed 10 of its 4000 sends
hitting the update window and re-executing.

Based on kvm/next (d4b7fb647204).

v2: https://git.infradead.org/?p=users/dwmw2/linux.git;a=shortlog;h=refs/heads/gpc-eagain-v2
 - Take kvm->srcu around the kvm_gpc_check() loop and the wallclock
   write in kvm_xen_shared_info_init(), which had lost it.
 - New patch 3: don't block on xen_lock within the vCPU's kvm->srcu
   read section (three-way deadlock against a waiting ioctl and a
   memslot update). Document the rule at the lock's definition.
 - On -EINVAL (deactivated shared_info, or an invalid port), clear
   timer_pending but keep timer_expires: a halted vCPU would
   otherwise spin aborting its own block, and the event is still
   reported as pending through KVM_XEN_VCPU_GET_ATTR.
 - Drop the wait argument from kvm_xen_inject_timer_irqs(); neither
   caller may wait.
 - Extend the KVM_GET_DIRTY_LOG note in api.rst to the vcpu_info area.
 - Teach lockdep that the wait functions as a synchronize_srcu(), so
   it reports any lock held across it which a kvm->srcu read section
   can take.
 - Comment fixes; drop a leftover cosmetic hunk.

Thanks to the Sashiko review bot for finding three of those on v1.

v1: https://git.infradead.org/?p=users/dwmw2/linux.git;a=shortlog;h=refs/heads/gpc-eagain
    https://lore.kernel.org/kvm/20261013204824.454208-1-dwmw2@infradead.org/

¹ https://git.infradead.org/?p=users/dwmw2/linux.git;a=shortlog;h=refs/heads/gpc-srcu-v2
² https://lore.kernel.org/kvm/20260529165114.748639-1-seanjc@google.com/

David Woodhouse (8):
  KVM: pfncache: Return -EAGAIN for a lookup which hits an invalid memslot
  KVM: pfncache: Add a 'wait' argument to the refresh and activate functions
  KVM: x86/xen: Don't block on xen_lock within the vCPU's kvm->srcu section
  KVM: x86/xen: Wait out memslot updates in the Xen attribute ioctls
  KVM: x86/xen: Don't lose the timer event when delivery fails transiently
  KVM: x86/xen: Retry event channel delivery which races a memslot update
  KVM: x86: Request the guest TLB flush from record_steal_time()
  KVM: selftests: Add Xen evtchn vs memslot-update race test

Sean Christopherson (3):
  KVM: x86/xen: Extract delivery of event to vCPU into a separate helper
  KVM: x86/xen: Explicitly tag "shared info" page as never being dirty tracked
  KVM: x86/xen: Don't dirty track "vCPU info" page

 Documentation/virt/kvm/api.rst                |   5 +-
 arch/x86/include/asm/kvm_host.h               |   4 +
 arch/x86/kvm/msrs.c                           |   2 +-
 arch/x86/kvm/x86.c                            |  28 +-
 arch/x86/kvm/xen.c                            | 341 ++++++++++++------
 include/linux/kvm_host.h                      |  27 +-
 include/linux/kvm_types.h                     |   1 +
 tools/testing/selftests/kvm/Makefile.kvm      |   1 +
 .../kvm/x86/xen_evtchn_memslot_race_test.c    | 178 +++++++++
 .../selftests/kvm/x86/xen_shinfo_test.c       |  11 +-
 virt/kvm/pfncache.c                           | 137 +++++--
 11 files changed, 564 insertions(+), 171 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/x86/xen_evtchn_memslot_race_test.c

base-commit: d4b7fb647204f0c81dfeae2d1a708e4d858e0c94
-- 
2.43.0

[PATCH v2 01/11] KVM: pfncache: Return -EAGAIN for a lookup which hits an invalid memslot

[David Woodhouse]

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

A memslot being deleted or moved is present in the active array but
flagged KVM_MEMSLOT_INVALID — a transient state which exists only
within a single KVM_SET_USER_MEMORY_REGION call and is announced by
a further generation bump when it resolves. gfn_to_hva_memslot()
collapses that case and a genuinely unbacked gfn into one error, and
the pfncache refresh reported both as -EFAULT.

Distinguish them: return -EAGAIN when the slot exists but is
invalid, mirroring what RET_PF_RETRY does for ordinary guest faults
on an invalid slot. Existing users don't *need* to change; most
will simply resolve again at the next refresh. But the distinct
error code *allows* a caller to retry.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: LLM
---
 virt/kvm/pfncache.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/virt/kvm/pfncache.c b/virt/kvm/pfncache.c
index 3659686b97c2..17aaebd76db8 100644
--- a/virt/kvm/pfncache.c
+++ b/virt/kvm/pfncache.c
@@ -304,7 +304,22 @@ static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned l
 			gpc->uhva = gfn_to_hva_memslot(gpc->memslot, gfn);
 
 			if (kvm_is_error_hva(gpc->uhva)) {
-				ret = -EFAULT;
+				/*
+				 * A slot being deleted or moved is present
+				 * but flagged invalid; the lookup failure is
+				 * transient, resolved when the update
+				 * completes (or reverts) and announced by a
+				 * new memslot generation. Distinguish that
+				 * from a genuinely unbacked gfn so that the
+				 * caller may retry rather than report an
+				 * error, as RET_PF_RETRY does for ordinary
+				 * guest faults.
+				 */
+				if (gpc->memslot &&
+				    (gpc->memslot->flags & KVM_MEMSLOT_INVALID))
+					ret = -EAGAIN;
+				else
+					ret = -EFAULT;
 				goto out;
 			}
 
-- 
2.43.0

[PATCH v2 02/11] KVM: pfncache: Add a 'wait' argument to the refresh and activate functions

[David Woodhouse]

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

A caller which does not hold kvm->srcu may now pass wait=true to
kvm_gpc_refresh() or kvm_gpc_activate() to wait out a concurrent
memslot update instead of receiving -EAGAIN, looping with
cond_resched() until the lookup resolves or a fatal signal is
pending. (An HVA-based cache never consults the memslots, so
kvm_gpc_activate_hva() keeps its signature.)

The refresh now takes its own kvm->srcu read lock for each attempt, so
callers no longer need to hold it. Holding it with wait=true would
deadlock — the wait would block the memslot update from completing —
as would taking any lock held across the wait within a kvm->srcu read
section. The wait therefore functions as a synchronize_srcu() to
lockdep, via srcu_lock_sync(), making both cycles reportable.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: LLM
---
 arch/x86/kvm/msrs.c      |   2 +-
 arch/x86/kvm/x86.c       |   2 +-
 arch/x86/kvm/xen.c       |  24 ++++----
 include/linux/kvm_host.h |  18 +++++-
 virt/kvm/pfncache.c      | 119 +++++++++++++++++++++++++++++----------
 5 files changed, 117 insertions(+), 48 deletions(-)

diff --git a/arch/x86/kvm/msrs.c b/arch/x86/kvm/msrs.c
index dd3bb04878ca..4c0fbba6e7ae 100644
--- a/arch/x86/kvm/msrs.c
+++ b/arch/x86/kvm/msrs.c
@@ -1280,7 +1280,7 @@ static void kvm_write_system_time(struct kvm_vcpu *vcpu, gpa_t system_time,
 	/* we verify if the enable bit is set... */
 	if (system_time & 1)
 		kvm_gpc_activate(&vcpu->arch.pv_time, system_time & ~1ULL,
-				 sizeof(struct pvclock_vcpu_time_info));
+				 sizeof(struct pvclock_vcpu_time_info), false);
 	else
 		kvm_gpc_deactivate(&vcpu->arch.pv_time);
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 79468ddfe473..eadcaeee651a 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1727,7 +1727,7 @@ static void kvm_setup_guest_pvclock(struct pvclock_vcpu_time_info *ref_hv_clock,
 	while (!kvm_gpc_check(gpc, offset + sizeof(*guest_hv_clock))) {
 		read_unlock_irqrestore(&gpc->lock, flags);
 
-		if (kvm_gpc_refresh(gpc, offset + sizeof(*guest_hv_clock)))
+		if (kvm_gpc_refresh(gpc, offset + sizeof(*guest_hv_clock), false))
 			return;
 
 		read_lock_irqsave(&gpc->lock, flags);
diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index 6aa00f746cba..d64630a417a7 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -50,7 +50,7 @@ static int kvm_xen_shared_info_init(struct kvm *kvm)
 	while (!kvm_gpc_check(gpc, PAGE_SIZE)) {
 		read_unlock_irq(&gpc->lock);
 
-		ret = kvm_gpc_refresh(gpc, PAGE_SIZE);
+		ret = kvm_gpc_refresh(gpc, PAGE_SIZE, false);
 		if (ret)
 			goto out;
 
@@ -161,7 +161,7 @@ static int xen_get_guest_pvclock(struct kvm_vcpu *vcpu,
 	while (!kvm_gpc_check(gpc, offset + sizeof(*hv_clock))) {
 		read_unlock_irqrestore(&gpc->lock, flags);
 
-		r = kvm_gpc_refresh(gpc, offset + sizeof(*hv_clock));
+		r = kvm_gpc_refresh(gpc, offset + sizeof(*hv_clock), false);
 		if (r)
 			return r;
 
@@ -435,7 +435,7 @@ static void kvm_xen_update_runstate_guest(struct kvm_vcpu *v, bool atomic)
 		if (atomic)
 			return;
 
-		if (kvm_gpc_refresh(gpc1, user_len1))
+		if (kvm_gpc_refresh(gpc1, user_len1, false))
 			return;
 
 		read_lock_irqsave(&gpc1->lock, flags);
@@ -488,7 +488,7 @@ static void kvm_xen_update_runstate_guest(struct kvm_vcpu *v, bool atomic)
 			 * 64-bit mode, the second GPC won't have been set up.
 			 */
 			if (kvm_gpc_activate(gpc2, gpc1->gpa + user_len1,
-					     user_len2))
+					     user_len2, false))
 				return;
 
 			/*
@@ -669,7 +669,7 @@ void kvm_xen_inject_pending_events(struct kvm_vcpu *v)
 	while (!kvm_gpc_check(gpc, sizeof(struct vcpu_info))) {
 		read_unlock_irqrestore(&gpc->lock, flags);
 
-		if (kvm_gpc_refresh(gpc, sizeof(struct vcpu_info)))
+		if (kvm_gpc_refresh(gpc, sizeof(struct vcpu_info), false))
 			return;
 
 		read_lock_irqsave(&gpc->lock, flags);
@@ -757,7 +757,7 @@ int __kvm_xen_has_interrupt(struct kvm_vcpu *v)
 		if (in_atomic() || !task_is_running(current))
 			return 1;
 
-		if (kvm_gpc_refresh(gpc, sizeof(struct vcpu_info))) {
+		if (kvm_gpc_refresh(gpc, sizeof(struct vcpu_info), false)) {
 			/*
 			 * If this failed, userspace has screwed up the
 			 * vcpu_info mapping. No interrupts for you.
@@ -813,7 +813,7 @@ int kvm_xen_hvm_set_attr(struct kvm *kvm, struct kvm_xen_hvm_attr *data)
 				r = 0;
 			} else {
 				r = kvm_gpc_activate(&kvm->arch.xen.shinfo_cache,
-						     gfn_to_gpa(gfn), PAGE_SIZE);
+						     gfn_to_gpa(gfn), PAGE_SIZE, false);
 			}
 		} else {
 			void __user * hva = u64_to_user_ptr(data->u.shared_info.hva);
@@ -960,7 +960,7 @@ int kvm_xen_vcpu_set_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
 				break;
 
 			r = kvm_gpc_activate(&vcpu->arch.xen.vcpu_info_cache,
-					     data->u.gpa, sizeof(struct vcpu_info));
+					     data->u.gpa, sizeof(struct vcpu_info), false);
 		} else {
 			if (data->u.hva == 0) {
 				kvm_gpc_deactivate(&vcpu->arch.xen.vcpu_info_cache);
@@ -990,7 +990,7 @@ int kvm_xen_vcpu_set_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
 
 		r = kvm_gpc_activate(&vcpu->arch.xen.vcpu_time_info_cache,
 				     data->u.gpa,
-				     sizeof(struct pvclock_vcpu_time_info));
+				     sizeof(struct pvclock_vcpu_time_info), false);
 		if (!r)
 			kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu);
 		break;
@@ -1023,7 +1023,7 @@ int kvm_xen_vcpu_set_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
 		/* How much fits in the (first) page? */
 		sz1 = PAGE_SIZE - (data->u.gpa & ~PAGE_MASK);
 		r = kvm_gpc_activate(&vcpu->arch.xen.runstate_cache,
-				     data->u.gpa, sz1);
+				     data->u.gpa, sz1, false);
 		if (r)
 			goto deactivate_out;
 
@@ -1034,7 +1034,7 @@ int kvm_xen_vcpu_set_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
 			sz2 = sz - sz1;
 			BUG_ON((data->u.gpa + sz1) & ~PAGE_MASK);
 			r = kvm_gpc_activate(&vcpu->arch.xen.runstate2_cache,
-					     data->u.gpa + sz1, sz2);
+					     data->u.gpa + sz1, sz2, false);
 			if (r)
 				goto deactivate_out;
 		}
@@ -2011,7 +2011,7 @@ static int kvm_xen_set_evtchn(struct kvm_xen_evtchn *xe, struct kvm *kvm)
 			break;
 
 		idx = srcu_read_lock(&kvm->srcu);
-		rc = kvm_gpc_refresh(gpc, PAGE_SIZE);
+		rc = kvm_gpc_refresh(gpc, PAGE_SIZE, false);
 		srcu_read_unlock(&kvm->srcu, idx);
 	} while(!rc);
 
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 3dd04605f2e5..9031f4e3f4e1 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1474,16 +1474,24 @@ void kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct kvm *kvm);
  * @gpc:	   struct gfn_to_pfn_cache object.
  * @gpa:	   guest physical address to map.
  * @len:	   sanity check; the range being access must fit a single page.
+ * @wait:	   wait out a concurrent memslot update instead of returning
+ *		   -EAGAIN. May not be used while this task holds any kvm->srcu
+ *		   read section, and no lock held across the wait may ever be
+ *		   taken within one: the update's synchronize_srcu() would
+ *		   deadlock against the wait. Lockdep checks both.
  *
  * @return:	   0 for success.
  *		   -EINVAL for a mapping which would cross a page boundary.
  *		   -EFAULT for an untranslatable guest physical address.
+ *		   -EAGAIN if the memslot is being updated (only with
+ *		   wait == false, or on a fatal signal while waiting).
  *
  * This primes a gfn_to_pfn_cache and links it into the @gpc->kvm's list for
  * invalidations to be processed.  Callers are required to use kvm_gpc_check()
  * to ensure that the cache is valid before accessing the target page.
  */
-int kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long len);
+int kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long len,
+		     bool wait);
 
 /**
  * kvm_gpc_activate_hva - prepare a cached kernel mapping and HPA for a given HVA.
@@ -1499,7 +1507,8 @@ int kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long len)
  * The semantics of this function are the same as those of kvm_gpc_activate(). It
  * merely bypasses a layer of address translation.
  */
-int kvm_gpc_activate_hva(struct gfn_to_pfn_cache *gpc, unsigned long hva, unsigned long len);
+int kvm_gpc_activate_hva(struct gfn_to_pfn_cache *gpc, unsigned long hva,
+			 unsigned long len);
 
 /**
  * kvm_gpc_check - check validity of a gfn_to_pfn_cache.
@@ -1524,10 +1533,13 @@ bool kvm_gpc_check(struct gfn_to_pfn_cache *gpc, unsigned long len);
  *
  * @gpc:	   struct gfn_to_pfn_cache object.
  * @len:	   sanity check; the range being access must fit a single page.
+ * @wait:	   as kvm_gpc_activate().
  *
  * @return:	   0 for success.
  *		   -EINVAL for a mapping which would cross a page boundary.
  *		   -EFAULT for an untranslatable guest physical address.
+ *		   -EAGAIN if the memslot is being updated (only with
+ *		   wait == false, or on a fatal signal while waiting).
  *
  * 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
@@ -1535,7 +1547,7 @@ bool kvm_gpc_check(struct gfn_to_pfn_cache *gpc, unsigned long len);
  * still lock and check the cache status, as this function does not return
  * with the lock still held to permit access.
  */
-int kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, unsigned long len);
+int kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, unsigned long len, bool wait);
 
 /**
  * kvm_gpc_deactivate - deactivate and unlink a gfn_to_pfn_cache.
diff --git a/virt/kvm/pfncache.c b/virt/kvm/pfncache.c
index 17aaebd76db8..0e700c337f5a 100644
--- a/virt/kvm/pfncache.c
+++ b/virt/kvm/pfncache.c
@@ -378,23 +378,63 @@ static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned l
 	return ret;
 }
 
-int kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, unsigned long len)
+/*
+ * One refresh attempt, under the caller's refresh_lock. Takes a kvm->srcu
+ * read section for the attempt, as the memslot lookups consult the active
+ * memslots.
+ */
+static int gpc_refresh_locked(struct gfn_to_pfn_cache *gpc, gpa_t gpa,
+			      unsigned long uhva)
 {
-	unsigned long uhva;
+	int idx, ret;
 
-	guard(mutex)(&gpc->refresh_lock);
+	idx = srcu_read_lock(&gpc->kvm->srcu);
+	ret = __kvm_gpc_refresh(gpc, gpa, uhva);
+	srcu_read_unlock(&gpc->kvm->srcu, idx);
 
-	if (!kvm_gpc_is_valid_len(gpc->gpa, gpc->uhva, len))
-		return -EINVAL;
+	return ret;
+}
+
+int kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, unsigned long len, bool wait)
+{
+	unsigned long uhva;
+	int ret;
 
 	/*
-	 * If the GPA is valid then ignore the HVA, as a cache can be GPA-based
-	 * or HVA-based, not both.  For GPA-based caches, the HVA will be
-	 * recomputed during refresh if necessary.
+	 * The wait completes only after a memslot update's
+	 * synchronize_srcu(&kvm->srcu), so it functions as one for lockdep:
+	 * taking any lock held across it within a kvm->srcu read section
+	 * is reported as the deadlock it would be.
 	 */
-	uhva = kvm_is_error_gpa(gpc->gpa) ? gpc->uhva : KVM_HVA_ERR_BAD;
+	if (wait)
+		srcu_lock_sync(&gpc->kvm->srcu.dep_map);
+
+	for (;;) {
+		scoped_guard(mutex, &gpc->refresh_lock) {
+			if (!kvm_gpc_is_valid_len(gpc->gpa, gpc->uhva, len))
+				return -EINVAL;
+
+			/*
+			 * If the GPA is valid then ignore the HVA, as a cache
+			 * can be GPA-based or HVA-based, not both.  For
+			 * GPA-based caches, the HVA will be recomputed during
+			 * refresh if necessary.
+			 */
+			uhva = kvm_is_error_gpa(gpc->gpa) ? gpc->uhva : KVM_HVA_ERR_BAD;
+
+			ret = gpc_refresh_locked(gpc, gpc->gpa, uhva);
+		}
+
+		if (ret != -EAGAIN || !wait || fatal_signal_pending(current))
+			return ret;
 
-	return __kvm_gpc_refresh(gpc, gpc->gpa, uhva);
+		/*
+		 * Wait with refresh_lock dropped: a kvm->srcu reader blocked
+		 * on it must be able to proceed and quiesce, or the memslot
+		 * update being waited for could never complete.
+		 */
+		cond_resched();
+	}
 }
 
 void kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct kvm *kvm)
@@ -410,36 +450,51 @@ void kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct kvm *kvm)
 }
 
 static int __kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long uhva,
-			      unsigned long len)
+			      unsigned long len, bool wait)
 {
 	struct kvm *kvm = gpc->kvm;
 
 	if (!kvm_gpc_is_valid_len(gpa, uhva, len))
 		return -EINVAL;
 
-	guard(mutex)(&gpc->refresh_lock);
+	/* As in kvm_gpc_refresh(): the wait is a synchronize_srcu() to lockdep. */
+	if (wait)
+		srcu_lock_sync(&kvm->srcu.dep_map);
 
-	if (!gpc->active) {
-		if (KVM_BUG_ON(gpc->valid, kvm))
-			return -EIO;
+	for (;;) {
+		int ret;
 
-		spin_lock(&kvm->gpc_lock);
-		list_add(&gpc->list, &kvm->gpc_list);
-		spin_unlock(&kvm->gpc_lock);
+		scoped_guard(mutex, &gpc->refresh_lock) {
+			if (!gpc->active) {
+				if (KVM_BUG_ON(gpc->valid, kvm))
+					return -EIO;
 
-		/*
-		 * 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.
-		 */
-		write_lock_irq(&gpc->lock);
-		gpc->active = true;
-		write_unlock_irq(&gpc->lock);
+				spin_lock(&kvm->gpc_lock);
+				list_add(&gpc->list, &kvm->gpc_list);
+				spin_unlock(&kvm->gpc_lock);
+
+				/*
+				 * 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.
+				 */
+				write_lock_irq(&gpc->lock);
+				gpc->active = true;
+				write_unlock_irq(&gpc->lock);
+			}
+			ret = gpc_refresh_locked(gpc, gpa, uhva);
+		}
+
+		if (ret != -EAGAIN || !wait || fatal_signal_pending(current))
+			return ret;
+
+		cond_resched();
 	}
-	return __kvm_gpc_refresh(gpc, gpa, uhva);
 }
 
-int kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long len)
+int kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long len,
+		     bool wait)
 {
 	/*
 	 * Explicitly disallow INVALID_GPA so that the magic value can be used
@@ -448,15 +503,17 @@ int kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long len)
 	if (WARN_ON_ONCE(kvm_is_error_gpa(gpa)))
 		return -EINVAL;
 
-	return __kvm_gpc_activate(gpc, gpa, KVM_HVA_ERR_BAD, len);
+	return __kvm_gpc_activate(gpc, gpa, KVM_HVA_ERR_BAD, len, wait);
 }
 
-int kvm_gpc_activate_hva(struct gfn_to_pfn_cache *gpc, unsigned long uhva, unsigned long len)
+int kvm_gpc_activate_hva(struct gfn_to_pfn_cache *gpc, unsigned long uhva,
+			 unsigned long len)
 {
 	if (!access_ok((void __user *)uhva, len))
 		return -EINVAL;
 
-	return __kvm_gpc_activate(gpc, INVALID_GPA, uhva, len);
+	/* An HVA-based cache never consults the memslots, so never waits. */
+	return __kvm_gpc_activate(gpc, INVALID_GPA, uhva, len, false);
 }
 
 void kvm_gpc_deactivate(struct gfn_to_pfn_cache *gpc)
-- 
2.43.0

[PATCH v2 03/11] KVM: x86/xen: Don't block on xen_lock within the vCPU's kvm->srcu section

[David Woodhouse]

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

The next commit lets the Xen attribute ioctls wait out a memslot
update while holding xen_lock. A memslot update in turn waits for
all kvm->srcu readers, so a vCPU which blocks on xen_lock inside its
own read section would close a three-way cycle. The hypercall page
MSR write is the only such acquisition; step out of the read section
around it.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: LLM
---
 arch/x86/include/asm/kvm_host.h | 4 ++++
 arch/x86/kvm/xen.c              | 8 ++++++++
 2 files changed, 12 insertions(+)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 3af7395c2430..6bebc4a79a33 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1108,6 +1108,10 @@ struct kvm_hv {
 #ifdef CONFIG_KVM_XEN
 /* Xen emulation context */
 struct kvm_xen {
+	/*
+	 * Never taken inside a kvm->srcu read section: holders may wait
+	 * out memslot updates (synchronize_srcu).
+	 */
 	struct mutex xen_lock;
 	u32 xen_version;
 	bool long_mode;
diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index d64630a417a7..74dad6789e42 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -1320,7 +1320,15 @@ int kvm_xen_write_hypercall_page(struct kvm_vcpu *vcpu, u64 data)
 	bool lm = is_long_mode(vcpu);
 	int r = 0;
 
+	/*
+	 * The attribute ioctls wait out memslot updates while holding
+	 * xen_lock; a memslot update waits for this vCPU's kvm->srcu
+	 * read section to quiesce. Blocking on xen_lock inside that
+	 * read section would close the cycle, so step out of it first.
+	 */
+	kvm_vcpu_srcu_read_unlock(vcpu);
 	mutex_lock(&kvm->arch.xen.xen_lock);
+	kvm_vcpu_srcu_read_lock(vcpu);
 	if (kvm->arch.xen.long_mode != lm) {
 		kvm->arch.xen.long_mode = lm;
 
-- 
2.43.0

[PATCH v2 04/11] KVM: x86/xen: Wait out memslot updates in the Xen attribute ioctls

[David Woodhouse]

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

Activating a shared_info, vcpu_info, pvclock or runstate cache can
fail if it races a memslot update: previously with -EFAULT, now with
-EAGAIN. These ioctls may sleep, so pass wait and let the activation
ride the update out; userspace never sees the transient state.

kvm_xen_shared_info_init() gains the same flag for its refresh:
true from the attribute ioctls, false from the hypercall page MSR
write, which runs in vCPU context under kvm->srcu and keeps its
existing failure behaviour. That one could retry by restarting the
WRMSR, but that is painful to plumb for an error which only matters
to legacy userspace (the non-_HVA shared_info).

kvm_xen_shared_info_init() keeps a read section around each
kvm_gpc_check() and the write through the mapping — the check
consults the memslots — but drops it across the refresh.

The kvm->srcu read sections around the activation calls go away;
they existed to satisfy the old refresh contract, and holding one
across a waiting activation would deadlock against the memslot
update. The runstate updates in kvm_xen_vcpu_set_attr() still read
guest state through the caches, so they keep narrow read sections
of their own.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: LLM
---
 arch/x86/kvm/xen.c | 53 +++++++++++++++++++++++++---------------------
 1 file changed, 29 insertions(+), 24 deletions(-)

diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index 74dad6789e42..cd1063d659cc 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -36,7 +36,7 @@ static bool kvm_xen_hcall_evtchn_send(struct kvm_vcpu *vcpu, u64 param, u64 *r);
 
 DEFINE_STATIC_KEY_DEFERRED_FALSE(kvm_xen_enabled, HZ);
 
-static int kvm_xen_shared_info_init(struct kvm *kvm)
+static int kvm_xen_shared_info_init(struct kvm *kvm, bool wait)
 {
 	struct gfn_to_pfn_cache *gpc = &kvm->arch.xen.shinfo_cache;
 	struct pvclock_wall_clock *wc;
@@ -44,17 +44,24 @@ static int kvm_xen_shared_info_init(struct kvm *kvm)
 	u32 wc_version;
 	u64 wall_nsec;
 	int ret = 0;
-	int idx = srcu_read_lock(&kvm->srcu);
+	int idx;
 
-	read_lock_irq(&gpc->lock);
-	while (!kvm_gpc_check(gpc, PAGE_SIZE)) {
+	/*
+	 * kvm_gpc_check() consults the memslots, so take kvm->srcu around
+	 * each check and the write through the mapping — but not across
+	 * the refresh, which may wait for a memslot update to complete.
+	 */
+	for (;;) {
+		idx = srcu_read_lock(&kvm->srcu);
+		read_lock_irq(&gpc->lock);
+		if (kvm_gpc_check(gpc, PAGE_SIZE))
+			break;
 		read_unlock_irq(&gpc->lock);
+		srcu_read_unlock(&kvm->srcu, idx);
 
-		ret = kvm_gpc_refresh(gpc, PAGE_SIZE, false);
+		ret = kvm_gpc_refresh(gpc, PAGE_SIZE, wait);
 		if (ret)
 			goto out;
-
-		read_lock_irq(&gpc->lock);
 	}
 
 	/*
@@ -98,9 +105,9 @@ static int kvm_xen_shared_info_init(struct kvm *kvm)
 
 	wc->version = wc_version + 1;
 	read_unlock_irq(&gpc->lock);
+	srcu_read_unlock(&kvm->srcu, idx);
 
 out:
-	srcu_read_unlock(&kvm->srcu, idx);
 	return ret;
 }
 
@@ -792,19 +799,15 @@ int kvm_xen_hvm_set_attr(struct kvm *kvm, struct kvm_xen_hvm_attr *data)
 			 * to make the call anyway.
 			 */
 			r = kvm->arch.xen.shinfo_cache.active ?
-				kvm_xen_shared_info_init(kvm) : 0;
+				kvm_xen_shared_info_init(kvm, true) : 0;
 			mutex_unlock(&kvm->arch.xen.xen_lock);
 		}
 		break;
 
 	case KVM_XEN_ATTR_TYPE_SHARED_INFO:
 	case KVM_XEN_ATTR_TYPE_SHARED_INFO_HVA: {
-		int idx;
-
 		mutex_lock(&kvm->arch.xen.xen_lock);
 
-		idx = srcu_read_lock(&kvm->srcu);
-
 		if (data->type == KVM_XEN_ATTR_TYPE_SHARED_INFO) {
 			gfn_t gfn = data->u.shared_info.gfn;
 
@@ -813,7 +816,7 @@ int kvm_xen_hvm_set_attr(struct kvm *kvm, struct kvm_xen_hvm_attr *data)
 				r = 0;
 			} else {
 				r = kvm_gpc_activate(&kvm->arch.xen.shinfo_cache,
-						     gfn_to_gpa(gfn), PAGE_SIZE, false);
+						     gfn_to_gpa(gfn), PAGE_SIZE, true);
 			}
 		} else {
 			void __user * hva = u64_to_user_ptr(data->u.shared_info.hva);
@@ -829,10 +832,8 @@ int kvm_xen_hvm_set_attr(struct kvm *kvm, struct kvm_xen_hvm_attr *data)
 			}
 		}
 
-		srcu_read_unlock(&kvm->srcu, idx);
-
 		if (!r && kvm->arch.xen.shinfo_cache.active)
-			r = kvm_xen_shared_info_init(kvm);
+			r = kvm_xen_shared_info_init(kvm, true);
 
 		mutex_unlock(&kvm->arch.xen.xen_lock);
 		break;
@@ -937,7 +938,6 @@ 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);
-	idx = srcu_read_lock(&vcpu->kvm->srcu);
 
 	switch (data->type) {
 	case KVM_XEN_VCPU_ATTR_TYPE_VCPU_INFO:
@@ -960,7 +960,7 @@ int kvm_xen_vcpu_set_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
 				break;
 
 			r = kvm_gpc_activate(&vcpu->arch.xen.vcpu_info_cache,
-					     data->u.gpa, sizeof(struct vcpu_info), false);
+					     data->u.gpa, sizeof(struct vcpu_info), true);
 		} else {
 			if (data->u.hva == 0) {
 				kvm_gpc_deactivate(&vcpu->arch.xen.vcpu_info_cache);
@@ -990,7 +990,7 @@ int kvm_xen_vcpu_set_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
 
 		r = kvm_gpc_activate(&vcpu->arch.xen.vcpu_time_info_cache,
 				     data->u.gpa,
-				     sizeof(struct pvclock_vcpu_time_info), false);
+				     sizeof(struct pvclock_vcpu_time_info), true);
 		if (!r)
 			kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu);
 		break;
@@ -1023,7 +1023,7 @@ int kvm_xen_vcpu_set_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
 		/* How much fits in the (first) page? */
 		sz1 = PAGE_SIZE - (data->u.gpa & ~PAGE_MASK);
 		r = kvm_gpc_activate(&vcpu->arch.xen.runstate_cache,
-				     data->u.gpa, sz1, false);
+				     data->u.gpa, sz1, true);
 		if (r)
 			goto deactivate_out;
 
@@ -1034,12 +1034,14 @@ int kvm_xen_vcpu_set_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
 			sz2 = sz - sz1;
 			BUG_ON((data->u.gpa + sz1) & ~PAGE_MASK);
 			r = kvm_gpc_activate(&vcpu->arch.xen.runstate2_cache,
-					     data->u.gpa + sz1, sz2, false);
+					     data->u.gpa + sz1, sz2, true);
 			if (r)
 				goto deactivate_out;
 		}
 
+		idx = srcu_read_lock(&vcpu->kvm->srcu);
 		kvm_xen_update_runstate_guest(vcpu, false);
+		srcu_read_unlock(&vcpu->kvm->srcu, idx);
 		break;
 	}
 	case KVM_XEN_VCPU_ATTR_TYPE_RUNSTATE_CURRENT:
@@ -1052,7 +1054,9 @@ int kvm_xen_vcpu_set_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
 			break;
 		}
 
+		idx = srcu_read_lock(&vcpu->kvm->srcu);
 		kvm_xen_update_runstate(vcpu, data->u.runstate.state);
+		srcu_read_unlock(&vcpu->kvm->srcu, idx);
 		r = 0;
 		break;
 
@@ -1132,10 +1136,12 @@ int kvm_xen_vcpu_set_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
 		vcpu->arch.xen.runstate_times[RUNSTATE_offline] +=
 			data->u.runstate.time_offline;
 
+		idx = srcu_read_lock(&vcpu->kvm->srcu);
 		if (data->u.runstate.state <= RUNSTATE_offline)
 			kvm_xen_update_runstate(vcpu, data->u.runstate.state);
 		else if (vcpu->arch.xen.runstate_cache.active)
 			kvm_xen_update_runstate_guest(vcpu, false);
+		srcu_read_unlock(&vcpu->kvm->srcu, idx);
 		r = 0;
 		break;
 
@@ -1181,7 +1187,6 @@ int kvm_xen_vcpu_set_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
 		break;
 	}
 
-	srcu_read_unlock(&vcpu->kvm->srcu, idx);
 	mutex_unlock(&vcpu->kvm->arch.xen.xen_lock);
 	return r;
 }
@@ -1337,7 +1342,7 @@ int kvm_xen_write_hypercall_page(struct kvm_vcpu *vcpu, u64 data)
 		 * correct place.
 		 */
 		if (kvm->arch.xen.shinfo_cache.active &&
-		    kvm_xen_shared_info_init(kvm))
+		    kvm_xen_shared_info_init(kvm, false))
 			r = 1;
 	}
 	mutex_unlock(&kvm->arch.xen.xen_lock);
-- 
2.43.0

[PATCH v2 05/11] KVM: x86/xen: Don't lose the timer event when delivery fails transiently

[David Woodhouse]

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

kvm_xen_inject_timer_irqs() can drop events if kvm_xen_set_evtchn()
fails transiently, as it can when a memslot is in the process of
being updated. Defer the delivery in that case by leaving
->arch.xen.timer_pending set, so that it gets retried.

If the shared_info cache has been deactivated (-EINVAL), clear
timer_pending — a halted vCPU would otherwise spin forever, as
kvm_cpu_has_pending_timer() aborts every block while nothing can
deliver the event — but keep timer_expires, so the GET_ATTR/SET_ATTR
migration flow still carries the event and re-fires it on re-arm.
If delivery fails with -EFAULT, the shared_info gfn is genuinely
unbacked and can never succeed: drop the event and complete the
timer rather than spin on it.

True Xen does not have this problem: the shared_info page is owned
by Xen and always exists, even when not mapped into the guest. The
equivalent for a KVM/Xen VMM is KVM_XEN_ATTR_TYPE_SHARED_INFO_HVA,
which has no memslot dependency and is the recommended mode; this
fixes the legacy GFN mode, which xen_shinfo_test exercises.

Don't re-arm the hrtimer in the KVM_XEN_VCPU_GET_ATTR(TIMER) flush
while a delivery is still pending; a re-fired callback racing the
retry would deliver twice.

The drop is vanishingly rare upstream, but hangs roughly half of all
xen_shinfo_test runs once the following atomic SRCU conversion
widens the window; hence fixing it first.

Fixes: 536395260582 ("KVM: x86/xen: handle PV timers oneshot mode")
Assisted-by: LLM
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 arch/x86/kvm/xen.c                            | 51 ++++++++++++++++---
 .../selftests/kvm/x86/xen_shinfo_test.c       | 11 ++--
 2 files changed, 49 insertions(+), 13 deletions(-)

diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index cd1063d659cc..f1be9022bc6f 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -115,13 +115,35 @@ void kvm_xen_inject_timer_irqs(struct kvm_vcpu *vcpu)
 {
 	if (atomic_read(&vcpu->arch.xen.timer_pending) > 0) {
 		struct kvm_xen_evtchn e;
+		int rc;
 
 		e.vcpu_id = vcpu->vcpu_id;
 		e.vcpu_idx = vcpu->vcpu_idx;
 		e.port = vcpu->arch.xen.timer_virq;
 		e.priority = KVM_IRQ_ROUTING_XEN_EVTCHN_PRIO_2LEVEL;
 
-		kvm_xen_set_evtchn(&e, vcpu->kvm);
+		rc = kvm_xen_set_evtchn(&e, vcpu->kvm);
+
+		/*
+		 * Leave timer_pending set if the memslot is being updated
+		 * (-EAGAIN); delivery is retried on the next entry pass.
+		 */
+		if (rc == -EAGAIN)
+			return;
+
+		/*
+		 * If the event cannot currently be delivered (-EINVAL:
+		 * deactivated shared_info, or an out-of-range port), clear
+		 * timer_pending — a halted vCPU would otherwise spin on
+		 * kvm_cpu_has_pending_timer() — but keep timer_expires,
+		 * which still reports the event as owed through
+		 * KVM_XEN_VCPU_GET_ATTR; re-arming the timer with an expiry
+		 * in the past fires it again.
+		 */
+		if (rc == -EINVAL) {
+			atomic_set(&vcpu->arch.xen.timer_pending, 0);
+			return;
+		}
 
 		vcpu->arch.xen.timer_expires = 0;
 		atomic_set(&vcpu->arch.xen.timer_pending, 0);
@@ -1272,12 +1294,13 @@ int kvm_xen_vcpu_get_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
 
 	case KVM_XEN_VCPU_ATTR_TYPE_TIMER:
 		/*
-		 * Ensure a consistent snapshot of state is captured, with a
-		 * timer either being pending, or the event channel delivered
-		 * to the corresponding bit in the shared_info. Not still
-		 * lurking in the timer_pending flag for deferred delivery.
-		 * Purely as an optimisation, if the timer_expires field is
-		 * zero, that means the timer isn't active (or even in the
+		 * Ensure a consistent snapshot of state is captured: a timer
+		 * which already fired has either been delivered to the
+		 * corresponding bit in the shared_info, or is reported as
+		 * still owed (expires_ns != 0, if the shared_info page was
+		 * unavailable). Purely
+		 * as an optimisation, if the timer_expires field is zero,
+		 * that means the timer isn't active (or even in the
 		 * timer_pending flag) and there is no need to cancel it.
 		 */
 		if (vcpu->arch.xen.timer_expires) {
@@ -1296,8 +1319,20 @@ int kvm_xen_vcpu_get_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
 		 * That's fine, as the guest won't even have had a chance
 		 * to run and handle the interrupt. Asserting an already
 		 * pending event channel is idempotent.
+		 *
+		 * Don't re-arm if the flush above failed transiently and
+		 * left the timer pending: delivery is owned by the retry
+		 * on the guest entry path, and an immediate hrtimer fire
+		 * would race it — the callback's timer_pending suppression
+		 * check is not serialized against the entry path, so a
+		 * late callback could re-assert a port the entry path had
+		 * already delivered and the guest already consumed.
+		 *
+		 * An expiry already in the past fires immediately: the
+		 * event is still owed.
 		 */
-		if (vcpu->arch.xen.timer_expires)
+		if (vcpu->arch.xen.timer_expires &&
+		    !atomic_read(&vcpu->arch.xen.timer_pending))
 			hrtimer_start_expires(&vcpu->arch.xen.timer,
 					      HRTIMER_MODE_ABS_HARD);
 
diff --git a/tools/testing/selftests/kvm/x86/xen_shinfo_test.c b/tools/testing/selftests/kvm/x86/xen_shinfo_test.c
index aaa414c677ad..3b11d0ee253f 100644
--- a/tools/testing/selftests/kvm/x86/xen_shinfo_test.c
+++ b/tools/testing/selftests/kvm/x86/xen_shinfo_test.c
@@ -356,11 +356,12 @@ static void guest_code(void)
 wait_for_timer:
 	/*
 	 * Poll for a timer wake event while the worker thread is mucking with
-	 * the shared info.  KVM XEN drops timer IRQs if the shared info is
-	 * invalid when the timer expires.  Arbitrarily poll 100 times before
-	 * giving up and asking the VMM to re-arm the timer.  100 polls should
-	 * consume enough time to beat on KVM without taking too long if the
-	 * timer IRQ is dropped due to an invalid event channel.
+	 * the shared info.  KVM is now supposed to retry a timer IRQ whose
+	 * delivery hit an invalid shared info rather than drop it, but older
+	 * kernels drop it on the floor.  Arbitrarily poll 100 times before
+	 * giving up and asking the VMM to re-arm the timer, so the test still
+	 * completes (and documents the VMM-side recovery protocol) on
+	 * unfixed kernels.
 	 */
 	for (i = 0; i < 100 && !guest_saw_irq; i++)
 		__xen_hypercall(__HYPERVISOR_sched_op, SCHEDOP_poll, &p);
-- 
2.43.0

[PATCH v2 06/11] KVM: x86/xen: Retry event channel delivery which races a memslot update

[David Woodhouse]

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

Event channel delivery now returns -EAGAIN when it races a memslot
update. It always failed; now the failure is distinguishable, and
instead of dropping the event each caller reacts according to its
context:

 - kvm_xen_hvm_evtchn_send(): the KVM_XEN_HVM_EVTCHN_SEND ioctl.
   May sleep: passes wait == true, and kvm_gpc_refresh() rides the
   update out.

 - evtchn_set_fn(): irqfd routing, from the irqfd workqueue after
   the atomic fast path failed. May also wait.

 - kvm_xen_hcall_evtchn_send(): the guest's own EVTCHNOP_send, in
   vcpu_run's exit handling under the vCPU's kvm->srcu read section
   — the very reader the memslot update is waiting out, so waiting
   there deadlocks, and any guest can trigger it. Instead, leave the
   hypercall uncompleted, with RIP not advanced, and the guest
   re-executes it; each attempt cycles the srcu read section on the
   way back in, letting the update complete. This is how Xen itself
   restarts preempted hypercalls, and the pending delivery is pure
   guest state, so there is nothing extra to migrate.

 - kvm_xen_inject_timer_irqs(): the timer retry added by the
   previous commit — timer_pending *is* its pending state, so it
   keeps that and retries on the next entry pass. Neither caller may
   wait: the entry pass holds the vCPU's kvm->srcu, and the
   KVM_XEN_VCPU_GET_ATTR timer flush holds xen_lock, on which a vCPU
   writing the hypercall page MSR blocks inside its kvm->srcu
   section. The flush doesn't need to wait: an undelivered event is
   reported as still owed in the snapshot.

Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/all/20260920215341.5ECE91F000FF@smtp.kernel.org
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: LLM
---
 arch/x86/kvm/xen.c | 58 +++++++++++++++++++++++++++++++---------------
 1 file changed, 39 insertions(+), 19 deletions(-)

diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index f1be9022bc6f..a88f571e8682 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -30,9 +30,10 @@
 #include "cpuid.h"
 #include "trace.h"
 
-static int kvm_xen_set_evtchn(struct kvm_xen_evtchn *xe, struct kvm *kvm);
+static int kvm_xen_set_evtchn(struct kvm_xen_evtchn *xe, struct kvm *kvm,
+			      bool wait);
 static int kvm_xen_setattr_evtchn(struct kvm *kvm, struct kvm_xen_hvm_attr *data);
-static bool kvm_xen_hcall_evtchn_send(struct kvm_vcpu *vcpu, u64 param, u64 *r);
+static int kvm_xen_hcall_evtchn_send(struct kvm_vcpu *vcpu, u64 param, u64 *r);
 
 DEFINE_STATIC_KEY_DEFERRED_FALSE(kvm_xen_enabled, HZ);
 
@@ -122,7 +123,7 @@ void kvm_xen_inject_timer_irqs(struct kvm_vcpu *vcpu)
 		e.port = vcpu->arch.xen.timer_virq;
 		e.priority = KVM_IRQ_ROUTING_XEN_EVTCHN_PRIO_2LEVEL;
 
-		rc = kvm_xen_set_evtchn(&e, vcpu->kvm);
+		rc = kvm_xen_set_evtchn(&e, vcpu->kvm, false);
 
 		/*
 		 * Leave timer_pending set if the memslot is being updated
@@ -1823,8 +1824,21 @@ int kvm_xen_hypercall(struct kvm_vcpu *vcpu)
 		}
 		break;
 	case __HYPERVISOR_event_channel_op:
-		if (params[0] == EVTCHNOP_send)
-			handled = kvm_xen_hcall_evtchn_send(vcpu, params[1], &r);
+		if (params[0] == EVTCHNOP_send) {
+			int ret = kvm_xen_hcall_evtchn_send(vcpu, params[1], &r);
+
+			/*
+			 * Delivery raced a memslot update; waiting for it
+			 * here would deadlock under vcpu_run's kvm->srcu
+			 * read section. Leave the hypercall uncompleted —
+			 * RIP not advanced — so the guest re-executes it.
+			 * Each attempt cycles the srcu read section on the
+			 * way back in, letting the update complete.
+			 */
+			if (ret == -EAGAIN)
+				return 1;
+			handled = !ret;
+		}
 		break;
 	case __HYPERVISOR_sched_op:
 		handled = kvm_xen_hcall_sched_op(vcpu, is_64bit, params[0],
@@ -2015,7 +2029,8 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm)
 	return rc;
 }
 
-static int kvm_xen_set_evtchn(struct kvm_xen_evtchn *xe, struct kvm *kvm)
+static int kvm_xen_set_evtchn(struct kvm_xen_evtchn *xe, struct kvm *kvm,
+			      bool wait)
 {
 	bool mm_borrowed = false;
 	int rc;
@@ -2052,16 +2067,13 @@ static int kvm_xen_set_evtchn(struct kvm_xen_evtchn *xe, struct kvm *kvm)
 	 */
 	do {
 		struct gfn_to_pfn_cache *gpc = &kvm->arch.xen.shinfo_cache;
-		int idx;
 
 		rc = kvm_xen_set_evtchn_fast(xe, kvm);
 		if (rc != -EWOULDBLOCK)
 			break;
 
-		idx = srcu_read_lock(&kvm->srcu);
-		rc = kvm_gpc_refresh(gpc, PAGE_SIZE, false);
-		srcu_read_unlock(&kvm->srcu, idx);
-	} while(!rc);
+		rc = kvm_gpc_refresh(gpc, PAGE_SIZE, wait);
+	} while (!rc);
 
 	if (mm_borrowed)
 		kthread_unuse_mm(kvm->mm);
@@ -2076,7 +2088,7 @@ static int evtchn_set_fn(struct kvm_kernel_irq_routing_entry *e, struct kvm *kvm
 	if (!level)
 		return -EINVAL;
 
-	return kvm_xen_set_evtchn(&e->xen_evtchn, kvm);
+	return kvm_xen_set_evtchn(&e->xen_evtchn, kvm, true);
 }
 
 /*
@@ -2150,7 +2162,7 @@ int kvm_xen_hvm_evtchn_send(struct kvm *kvm, struct kvm_irq_routing_xen_evtchn *
 	e.vcpu_idx = -1;
 	e.priority = uxe->priority;
 
-	ret = kvm_xen_set_evtchn(&e, kvm);
+	ret = kvm_xen_set_evtchn(&e, kvm, true);
 
 	/*
 	 * None of that 'return 1 if it actually got delivered' nonsense.
@@ -2379,7 +2391,11 @@ static int kvm_xen_setattr_evtchn(struct kvm *kvm, struct kvm_xen_hvm_attr *data
 	return kvm_xen_eventfd_assign(kvm, data);
 }
 
-static bool kvm_xen_hcall_evtchn_send(struct kvm_vcpu *vcpu, u64 param, u64 *r)
+/*
+ * Returns 0 with *r set if handled, -EAGAIN if the hypercall should be
+ * re-executed by the guest, or -ENOENT to hand it to userspace.
+ */
+static int kvm_xen_hcall_evtchn_send(struct kvm_vcpu *vcpu, u64 param, u64 *r)
 {
 	struct evtchnfd *evtchnfd;
 	struct evtchn_send send;
@@ -2389,7 +2405,7 @@ static bool kvm_xen_hcall_evtchn_send(struct kvm_vcpu *vcpu, u64 param, u64 *r)
 	BUILD_BUG_ON(sizeof(send) != 4);
 	if (kvm_read_guest_virt(vcpu, param, &send, sizeof(send), &e)) {
 		*r = -EFAULT;
-		return true;
+		return 0;
 	}
 
 	/*
@@ -2398,18 +2414,22 @@ static bool kvm_xen_hcall_evtchn_send(struct kvm_vcpu *vcpu, u64 param, u64 *r)
 	 */
 	evtchnfd = xa_load(&vcpu->kvm->arch.xen.evtchn_ports, send.port);
 	if (!evtchnfd)
-		return false;
+		return -ENOENT;
 
 	if (evtchnfd->deliver.port.port) {
-		int ret = kvm_xen_set_evtchn(&evtchnfd->deliver.port, vcpu->kvm);
+		int ret = kvm_xen_set_evtchn(&evtchnfd->deliver.port,
+					     vcpu->kvm, false);
+
+		if (ret == -EAGAIN)
+			return ret;
 		if (ret < 0 && ret != -ENOTCONN)
-			return false;
+			return -ENOENT;
 	} else {
 		eventfd_signal(evtchnfd->deliver.eventfd.ctx);
 	}
 
 	*r = 0;
-	return true;
+	return 0;
 }
 
 void kvm_xen_init_vcpu(struct kvm_vcpu *vcpu)
-- 
2.43.0

[PATCH v2 07/11] KVM: x86/xen: Extract delivery of event to vCPU into a separate helper

[David Woodhouse]

From: Sean Christopherson <seanjc@google.com>

Move the marking of an event as pending in the target vCPU's vcpu_info
into a separate helper, __kvm_xen_set_evtchn_fast(), invoked after the
shared_info processing is complete and its RCU read-side critical
section has been exited. This makes both halves easier to read.

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
[dwmw2: rebased onto the -EAGAIN series. Sean's series first
 converted the GPC locks to read_trylock(), which this series does
 not carry, so the helper takes the plain read lock and the
 original's trylock failure path does not apply. The caller's
 kvm->srcu section extends across the helper call, since
 kvm_gpc_check() on the vcpu_info cache consults the memslot
 generation and the irqfd path enters holding only irq_srcu; in
 Sean's series that was covered by a guard(srcu) spanning the whole
 function.]
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: LLM
---
 arch/x86/kvm/xen.c | 158 ++++++++++++++++++++++++++++-----------------
 1 file changed, 97 insertions(+), 61 deletions(-)

diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index a88f571e8682..8491b4adaf51 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -1893,6 +1893,89 @@ static void kvm_xen_check_poller(struct kvm_vcpu *vcpu, int port)
 	}
 }
 
+static void __kvm_xen_set_evtchn_fast(struct kvm_vcpu *vcpu, int port_word_bit,
+				      bool has_64bit_shinfo)
+{
+	struct gfn_to_pfn_cache *gpc = &vcpu->arch.xen.vcpu_info_cache;
+	unsigned long vi_pending_sel_ofs;
+	bool inject_upcall = false;
+	bool kick_vcpu = false;
+	unsigned long flags;
+	bool old;
+
+	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);
+	if (!kvm_gpc_check(gpc, sizeof(struct vcpu_info))) {
+		/*
+		 * Could not access the vcpu_info. Set the bit in-kernel and
+		 * prod the vCPU to deliver it for itself. Note that an MSI is
+		 * *not* an acceptable substitute here even if the vCPU has an
+		 * upcall vector: the pending bit only exists in
+		 * evtchn_pending_sel so far, and the vCPU has to be forced
+		 * out of the guest so that kvm_xen_inject_pending_events()
+		 * can refresh the cache and propagate it to the guest.
+		 */
+		if (!test_and_set_bit(port_word_bit, &vcpu->arch.xen.evtchn_pending_sel))
+			kick_vcpu = true;
+		goto out_unlock;
+	}
+	inject_upcall = true;
+
+	/*
+	 * Explicitly use a 32-bit btsl instead of test_and_set_bit(), which
+	 * would use btsq on x86-64. The vcpu_info is guest-controlled and only
+	 * required to be 32-bit aligned, so a 64-bit access could generate a
+	 * split-lock #AC.
+	 *
+	 * Note, this does not apply to the test_and_set_bit() on pending_bits
+	 * in the caller: that is in the per-VM shared_info, which is page
+	 * aligned, so the access is guaranteed to be 64-bit aligned.
+	 */
+	old = GEN_BINARY_RMWcc(LOCK_PREFIX "btsl",
+			       *(u32 *)(gpc->khva + vi_pending_sel_ofs),
+			       c, "Ir", port_word_bit);
+	if (!old) {
+		struct vcpu_info *vi = gpc->khva;
+
+		/* No need for compat handling */
+		BUILD_BUG_ON(offsetof(struct vcpu_info, evtchn_upcall_pending) !=
+			     offsetof(struct compat_vcpu_info, evtchn_upcall_pending));
+
+		WRITE_ONCE(vi->evtchn_upcall_pending, 1);
+		kick_vcpu = true;
+	}
+
+out_unlock:
+	read_unlock_irqrestore(&gpc->lock, flags);
+
+	/*
+	 * 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.
+	 */
+	if (!kick_vcpu)
+		return;
+
+	/*
+	 * For the per-vCPU lapic vector, deliver it as MSI — but only if the
+	 * vcpu_info was actually updated above. If it wasn't, the vCPU must
+	 * be kicked instead (see above).
+	 */
+	if (inject_upcall && vcpu->arch.xen.upcall_vector) {
+		kvm_xen_inject_vcpu_vector(vcpu);
+		return;
+	}
+
+	kvm_make_request(KVM_REQ_UNBLOCK, vcpu);
+	kvm_vcpu_kick(vcpu);
+}
+
 /*
  * The return value from this function is propagated to kvm_set_irq() API,
  * so it returns:
@@ -1907,11 +1990,10 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm)
 {
 	struct gfn_to_pfn_cache *gpc = &kvm->arch.xen.shinfo_cache;
 	bool has_64bit_shinfo = kvm_xen_has_64bit_shinfo(kvm);
-	unsigned long *pending_bits, *mask_bits, vi_pending_sel_ofs;
+	unsigned long *pending_bits, *mask_bits;
 	struct kvm_vcpu *vcpu;
 	unsigned long flags;
 	int port_word_bit;
-	bool kick_vcpu = false;
 	int vcpu_idx, idx, rc;
 
 	vcpu_idx = READ_ONCE(xe->vcpu_idx);
@@ -1933,23 +2015,19 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm)
 
 	read_lock_irqsave(&gpc->lock, flags);
 	if (!kvm_gpc_check(gpc, PAGE_SIZE))
-		goto out_rcu;
+		goto out_unlock;
 
 	if (has_64bit_shinfo) {
 		struct shared_info *shinfo = gpc->khva;
 		pending_bits = (unsigned long *)&shinfo->evtchn_pending;
 		mask_bits = (unsigned long *)&shinfo->evtchn_mask;
 		port_word_bit = xe->port / 64;
-
-		vi_pending_sel_ofs = offsetof(struct vcpu_info, evtchn_pending_sel);
 	} else {
 		struct compat_shared_info *shinfo = gpc->khva;
 		pending_bits = (unsigned long *)&shinfo->evtchn_pending;
 		mask_bits = (unsigned long *)&shinfo->evtchn_mask;
 		port_word_bit = xe->port / 32;
 
-		vi_pending_sel_ofs = offsetof(struct compat_vcpu_info, evtchn_pending_sel);
-
 		/* test_and_set_bit() needs 64-bit alignment, but that's OK */
 		BUILD_BUG_ON(offsetof(struct compat_shared_info, evtchn_pending) & 7);
 	}
@@ -1965,66 +2043,24 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm)
 		rc = 0; /* It was already raised */
 	} else if (test_bit(xe->port, mask_bits)) {
 		rc = -ENOTCONN; /* Masked */
-		kvm_xen_check_poller(vcpu, xe->port);
 	} else {
-		bool old;
-
 		rc = 1; /* Delivered to the bitmap in shared_info. */
-		/* Now switch to the vCPU's vcpu_info to set the index and pending_sel */
-		read_unlock_irqrestore(&gpc->lock, flags);
-		gpc = &vcpu->arch.xen.vcpu_info_cache;
-
-		read_lock_irqsave(&gpc->lock, flags);
-		if (!kvm_gpc_check(gpc, sizeof(struct vcpu_info))) {
-			/*
-			 * Could not access the vcpu_info. Set the bit in-kernel
-			 * and prod the vCPU to deliver it for itself.
-			 */
-			if (!test_and_set_bit(port_word_bit, &vcpu->arch.xen.evtchn_pending_sel))
-				kick_vcpu = true;
-			goto out_rcu;
-		}
-
-		/*
-		 * Explicitly use a 32-bit btsl instead of test_and_set_bit(),
-		 * which would use btsq on x86-64. The vcpu_info is guest-
-		 * controlled and only required to be 32-bit aligned, so a
-		 * 64-bit access could generate a split-lock #AC.
-		 *
-		 * Note, this does not apply to the test_and_set_bit() on
-		 * pending_bits above: that is in the per-VM shared_info, which
-		 * is page aligned, so the access is guaranteed to be 64-bit
-		 * aligned.
-		 */
-		old = GEN_BINARY_RMWcc(LOCK_PREFIX "btsl",
-				       *(u32 *)(gpc->khva + vi_pending_sel_ofs),
-				       c, "Ir", port_word_bit);
-		if (!old) {
-			struct vcpu_info *vi = gpc->khva;
-
-			/* No need for compat handling */
-			BUILD_BUG_ON(offsetof(struct vcpu_info, evtchn_upcall_pending) !=
-				     offsetof(struct compat_vcpu_info, evtchn_upcall_pending));
-
-			WRITE_ONCE(vi->evtchn_upcall_pending, 1);
-			kick_vcpu = true;
-		}
-
-		/* For the per-vCPU lapic vector, deliver it as MSI. */
-		if (kick_vcpu && vcpu->arch.xen.upcall_vector) {
-			kvm_xen_inject_vcpu_vector(vcpu);
-			kick_vcpu = false;
-		}
 	}
 
- out_rcu:
+ out_unlock:
 	read_unlock_irqrestore(&gpc->lock, flags);
-	srcu_read_unlock(&kvm->srcu, idx);
 
-	if (kick_vcpu) {
-		kvm_make_request(KVM_REQ_UNBLOCK, vcpu);
-		kvm_vcpu_kick(vcpu);
-	}
+	/*
+	 * 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.
+	 */
+	if (rc == -ENOTCONN)
+		kvm_xen_check_poller(vcpu, xe->port);
+	else if (rc == 1)
+		__kvm_xen_set_evtchn_fast(vcpu, port_word_bit,
+					  has_64bit_shinfo);
+
+	srcu_read_unlock(&kvm->srcu, idx);
 
 	return rc;
 }
-- 
2.43.0

[PATCH v2 08/11] KVM: x86/xen: Explicitly tag "shared info" page as never being dirty tracked

[David Woodhouse]

From: Sean Christopherson <seanjc@google.com>

Explicitly mark the Xen shared info page as never being dirty tracked so
that higher-level gpc APIs can be added to automatically take care of
things like dirty tracking, without reintroducing the bug fixed by
commit 55749769fe60 ("KVM: x86: Fix wall clock writes in Xen shared_info
not to mark page dirty").  And because the code _looks_ buggy.

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: LLM
---
 arch/x86/kvm/xen.c        |  2 +-
 include/linux/kvm_host.h  | 10 ++++++++--
 include/linux/kvm_types.h |  1 +
 virt/kvm/pfncache.c       |  4 +++-
 4 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index 8491b4adaf51..df72ab95658b 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -2500,7 +2500,7 @@ void kvm_xen_init_vm(struct kvm *kvm)
 {
 	mutex_init(&kvm->arch.xen.xen_lock);
 	xa_init(&kvm->arch.xen.evtchn_ports);
-	kvm_gpc_init(&kvm->arch.xen.shinfo_cache, kvm);
+	__kvm_gpc_init(&kvm->arch.xen.shinfo_cache, kvm, true);
 }
 
 void kvm_xen_destroy_vm(struct kvm *kvm)
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 9031f4e3f4e1..debce84f4177 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1465,7 +1465,13 @@ int kvm_vcpu_write_guest(struct kvm_vcpu *vcpu, gpa_t gpa, const void *data,
  * immutable attributes.  Note, the cache must be zero-allocated (or zeroed by
  * the caller before init).
  */
-void kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct kvm *kvm);
+void __kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct kvm *kvm,
+		    bool never_dirty);
+
+static inline void kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct kvm *kvm)
+{
+	__kvm_gpc_init(gpc, kvm, false);
+}
 
 /**
  * kvm_gpc_activate - prepare a cached kernel mapping and HPA for a given guest
@@ -1988,7 +1994,7 @@ static inline void kvm_gpc_mark_dirty_in_slot(struct gfn_to_pfn_cache *gpc)
 {
 	lockdep_assert_held(&gpc->lock);
 
-	if (!gpc->memslot)
+	if (!gpc->memslot || gpc->never_dirty)
 		return;
 
 	mark_page_dirty_in_slot(gpc->kvm, gpc->memslot, gpa_to_gfn(gpc->gpa));
diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h
index a568d8e6f4e8..e850adc3f47e 100644
--- a/include/linux/kvm_types.h
+++ b/include/linux/kvm_types.h
@@ -94,6 +94,7 @@ struct gfn_to_pfn_cache {
 	kvm_pfn_t pfn;
 	bool active;
 	bool valid;
+	bool never_dirty;
 };
 
 #ifdef KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE
diff --git a/virt/kvm/pfncache.c b/virt/kvm/pfncache.c
index 0e700c337f5a..8498abb746ad 100644
--- a/virt/kvm/pfncache.c
+++ b/virt/kvm/pfncache.c
@@ -437,7 +437,8 @@ int kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, unsigned long len, bool wait)
 	}
 }
 
-void kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct kvm *kvm)
+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);
@@ -447,6 +448,7 @@ void kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct kvm *kvm)
 	gpc->gpa = INVALID_GPA;
 	gpc->uhva = KVM_HVA_ERR_BAD;
 	gpc->active = gpc->valid = false;
+	gpc->never_dirty = never_dirty;
 }
 
 static int __kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long uhva,
-- 
2.43.0

[PATCH v2 09/11] KVM: x86/xen: Don't dirty track "vCPU info" page

[David Woodhouse]

From: Sean Christopherson <seanjc@google.com>

Give the Xen per-vCPU info page the same treatment as the per-VM shared
info page, and never mark it dirty, as KVM clearly relies on userspace to
assume the page is always dirty.  While the page is marked dirty on writes
via kvm_xen_inject_pending_events(), it's not marked dirty when written by
__kvm_xen_set_evtchn_fast().

Furthermore, as was the case with the shared info page, writes in the event
channel fastpath may be done without an active vCPU, e.g. when called via
timer callback or irqfd injection.  I.e. attempting to fix the fastpath
would run afoul of the same issue that was fixed by
commit 55749769fe60 ("KVM: x86: Fix wall clock writes in Xen shared_info
not to mark page dirty").

This also covers the pvclock writes kvm_guest_time_update() makes
through the same cache; userspace assuming the page is always dirty
covers those equally.

Signed-off-by: Sean Christopherson <seanjc@google.com>
[dwmw2: also update the KVM_GET_DIRTY_LOG note in api.rst, which
 named only the shared_info page]
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: LLM
---
 Documentation/virt/kvm/api.rst | 5 +++--
 arch/x86/kvm/xen.c             | 3 +--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index e0430cc750c9..b24aac070b11 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -385,8 +385,9 @@ The bits in the dirty bitmap are cleared before the ioctl returns, unless
 KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2 is enabled.  For more information,
 see the description of the capability.
 
-Note that the Xen shared_info page, if configured, shall always be assumed
-to be dirty. KVM will not explicitly mark it such.
+Note that the Xen shared_info page and vcpu_info area, if configured,
+shall always be assumed to be dirty. KVM will not explicitly mark them
+such.
 
 
 4.10 KVM_RUN
diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index df72ab95658b..e72c0b3d2109 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -735,7 +735,6 @@ void kvm_xen_inject_pending_events(struct kvm_vcpu *v)
 		WRITE_ONCE(vi->evtchn_upcall_pending, 1);
 	}
 
-	kvm_gpc_mark_dirty_in_slot(gpc);
 	read_unlock_irqrestore(&gpc->lock, flags);
 
 	/* For the per-vCPU lapic vector, deliver it as MSI. */
@@ -2479,7 +2478,7 @@ void kvm_xen_init_vcpu(struct kvm_vcpu *vcpu)
 
 	kvm_gpc_init(&vcpu->arch.xen.runstate_cache, vcpu->kvm);
 	kvm_gpc_init(&vcpu->arch.xen.runstate2_cache, vcpu->kvm);
-	kvm_gpc_init(&vcpu->arch.xen.vcpu_info_cache, vcpu->kvm);
+	__kvm_gpc_init(&vcpu->arch.xen.vcpu_info_cache, vcpu->kvm, true);
 	kvm_gpc_init(&vcpu->arch.xen.vcpu_time_info_cache, vcpu->kvm);
 }
 
-- 
2.43.0

[PATCH v2 10/11] KVM: x86: Request the guest TLB flush from record_steal_time()

[David Woodhouse]

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

record_steal_time() performs a TLB flush on the guest's behalf when the
guest sets KVM_VCPU_FLUSH_TLB. With shadow paging,
kvm_vcpu_flush_tlb_guest() ends up in mmu_sync_children(), which can
reschedule via cond_resched_rwlock_write(). That is why the flush
currently sits between a user_access_end() and a fresh
user_access_begin(): it cannot be done while the guest page is mapped
for access.

Raise KVM_REQ_TLB_FLUSH_GUEST instead of flushing inline, and move the
processing of KVM_REQ_STEAL_UPDATE ahead of where the TLB flush requests
are serviced in vcpu_enter_guest() so that the flush still happens before
the vCPU enters the guest. There is no requirement for it to complete any
earlier than that.

kvm_make_request() on the vCPU which is running is nothing more than a
barrier and a set_bit(), so unlike the flush itself it can be done from
any context — which lets the surrounding access be simplified later.

Note that the xchg() must stay where it is: it atomically consumes any
KVM_VCPU_FLUSH_TLB which the guest has set, as required by commit
b043138246a4 ("x86/KVM: Make sure KVM_VCPU_FLUSH_TLB flag is not missed").

Reported-by: Sashiko AI review <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260530061932.7849D1F00893@smtp.kernel.org
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: LLM
---
 arch/x86/kvm/x86.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index eadcaeee651a..20119d07b477 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2106,8 +2106,22 @@ static void record_steal_time(struct kvm_vcpu *vcpu)
 
 		trace_kvm_pv_tlb_flush(vcpu->vcpu_id,
 				       st_preempted & KVM_VCPU_FLUSH_TLB);
+		/*
+		 * Request the flush instead of performing it inline. With
+		 * shadow paging kvm_vcpu_flush_tlb_guest() can reschedule (via
+		 * mmu_sync_children()), which is why the user access above has
+		 * to be closed before it. Requesting the flush is just a bit
+		 * set on the running vCPU, and the request is serviced before
+		 * the vCPU enters the guest; nothing requires the flush to
+		 * have completed any earlier than that.
+		 *
+		 * Note the xchg above must remain as-is: it atomically
+		 * consumes any KVM_VCPU_FLUSH_TLB the guest set, as required
+		 * by commit b043138246a4 ("x86/KVM: Make sure
+		 * KVM_VCPU_FLUSH_TLB flag is not missed").
+		 */
 		if (st_preempted & KVM_VCPU_FLUSH_TLB)
-			kvm_vcpu_flush_tlb_guest(vcpu);
+			kvm_make_request(KVM_REQ_TLB_FLUSH_GUEST, vcpu);
 
 		if (!user_access_begin(st, sizeof(*st)))
 			goto dirty;
@@ -8089,6 +8103,14 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
 			if (unlikely(r))
 				goto out;
 		}
+		/*
+		 * Process the steal time update before the TLB flush requests
+		 * are serviced below; record_steal_time() requests
+		 * KVM_REQ_TLB_FLUSH_GUEST on the guest's behalf rather than
+		 * performing the flush itself.
+		 */
+		if (kvm_check_request(KVM_REQ_STEAL_UPDATE, vcpu))
+			record_steal_time(vcpu);
 		if (kvm_check_request(KVM_REQ_MMU_SYNC, vcpu))
 			kvm_mmu_sync_roots(vcpu);
 		if (kvm_check_request(KVM_REQ_LOAD_MMU_PGD, vcpu))
@@ -8138,8 +8160,6 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
 			r = 1;
 			goto out;
 		}
-		if (kvm_check_request(KVM_REQ_STEAL_UPDATE, vcpu))
-			record_steal_time(vcpu);
 		if (kvm_check_request(KVM_REQ_PMU, vcpu))
 			kvm_pmu_handle_event(vcpu);
 		if (kvm_check_request(KVM_REQ_PMI, vcpu))
-- 
2.43.0

[PATCH v2 11/11] KVM: selftests: Add Xen evtchn vs memslot-update race test

[David Woodhouse]

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

Race a guest EVTCHNOP_send loop against a host thread which deletes
and recreates the shared_info memslot. Delivery which hits the update
window now returns -EAGAIN and the hypercall is re-executed; if KVM
instead waited in the vCPU's exit-handling context — under vcpu_run's
kvm->srcu read section — it would deadlock against
kvm_swap_active_memslots() waiting out that reader, and this test
livelocks within seconds.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: LLM
---
 tools/testing/selftests/kvm/Makefile.kvm      |   1 +
 .../kvm/x86/xen_evtchn_memslot_race_test.c    | 178 ++++++++++++++++++
 2 files changed, 179 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/x86/xen_evtchn_memslot_race_test.c

diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index 96bab7002d39..e7e17d19d6cb 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -144,6 +144,7 @@ TEST_GEN_PROGS_x86 += x86/xss_msr_test
 TEST_GEN_PROGS_x86 += x86/debug_regs
 TEST_GEN_PROGS_x86 += x86/tsc_msrs_test
 TEST_GEN_PROGS_x86 += x86/vmx_pmu_caps_test
+TEST_GEN_PROGS_x86 += x86/xen_evtchn_memslot_race_test
 TEST_GEN_PROGS_x86 += x86/xen_shinfo_test
 TEST_GEN_PROGS_x86 += x86/xen_vmcall_test
 TEST_GEN_PROGS_x86 += x86/sev_dbg_test
diff --git a/tools/testing/selftests/kvm/x86/xen_evtchn_memslot_race_test.c b/tools/testing/selftests/kvm/x86/xen_evtchn_memslot_race_test.c
new file mode 100644
index 000000000000..2b6974d3d464
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86/xen_evtchn_memslot_race_test.c
@@ -0,0 +1,178 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * xen_evtchn_memslot_race_test
+ *
+ * A guest EVTCHNOP_send whose delivery races a DELETE/re-CREATE of the
+ * memslot holding the shared_info page. The delivery path runs in the
+ * vCPU's exit-handling context, under vcpu_run's kvm->srcu read
+ * section; if it waits there for the memslot update to complete, it
+ * deadlocks against kvm_swap_active_memslots()'s
+ * synchronize_srcu_expedited() waiting out that very read section.
+ * Regression test for that deadlock: KVM must defer the delivery to
+ * the next guest entry rather than spin in exit context.
+ *
+ * The guest sends events in a tight loop; a host thread deletes and
+ * recreates the shared_info slot in a tight loop. On a broken host
+ * this livelocks within a second or two (the test then dies by
+ * timeout); on a fixed host every send eventually delivers and the
+ * test completes N iterations.
+ */
+#include <pthread.h>
+#include <stdatomic.h>
+
+#include "test_util.h"
+#include "kvm_util.h"
+#include "processor.h"
+
+#include <stdint.h>
+
+#define SHINFO_REGION_GPA	0xc0000000ULL
+#define SHINFO_REGION_SLOT	10
+
+#define EVTCHN_PORT		15
+#define TEST_ITERATIONS		4000
+
+#define EVTCHNOP_send		4
+#define __HYPERVISOR_event_channel_op	32
+
+#define XEN_HYPERCALL_MSR	0x40000000
+
+#define EVTCHNSTAT_ipi		5
+
+struct evtchn_send {
+	uint32_t port;
+};
+
+static void guest_code(void)
+{
+	struct evtchn_send s = { .port = EVTCHN_PORT };
+	int i;
+
+	for (i = 0; i < TEST_ITERATIONS; i++) {
+		/*
+		 * The delivery may legitimately fail while the shared_info
+		 * slot is mid-churn (between DELETE and re-CREATE there is
+		 * no slot at all, which is -EFAULT, not a transient). The
+		 * failure mode under test is the vCPU never coming back at
+		 * all (deadlock -> timeout), not the hypercall rc.
+		 */
+		__xen_hypercall(__HYPERVISOR_event_channel_op, EVTCHNOP_send, &s);
+		GUEST_SYNC(i);
+	}
+
+	GUEST_DONE();
+}
+
+static atomic_bool churn_stop;
+
+static void *memslot_churn(void *arg)
+{
+	struct kvm_vm *vm = arg;
+
+	while (!atomic_load(&churn_stop)) {
+		vm_mem_region_delete(vm, SHINFO_REGION_SLOT);
+		vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS,
+					    SHINFO_REGION_GPA,
+					    SHINFO_REGION_SLOT, 2, 0);
+	}
+	return NULL;
+}
+
+int main(int argc, char *argv[])
+{
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm;
+	pthread_t thread;
+	struct ucall uc;
+	int done = 0;
+	int r;
+
+	int xen_caps = kvm_check_cap(KVM_CAP_XEN_HVM);
+
+	TEST_REQUIRE(xen_caps & KVM_XEN_HVM_CONFIG_SHARED_INFO);
+	TEST_REQUIRE(xen_caps & KVM_XEN_HVM_CONFIG_EVTCHN_SEND);
+
+	vm = vm_create_with_one_vcpu(&vcpu, guest_code);
+
+	vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS,
+				    SHINFO_REGION_GPA, SHINFO_REGION_SLOT, 2, 0);
+
+	struct kvm_xen_hvm_config hvmc = {
+		.flags = KVM_XEN_HVM_CONFIG_INTERCEPT_HCALL |
+			 KVM_XEN_HVM_CONFIG_EVTCHN_SEND,
+		.msr = XEN_HYPERCALL_MSR,
+	};
+	vm_ioctl(vm, KVM_XEN_HVM_CONFIG, &hvmc);
+
+	struct kvm_xen_hvm_attr lm = {
+		.type = KVM_XEN_ATTR_TYPE_LONG_MODE,
+		.u.long_mode = 1,
+	};
+	vm_ioctl(vm, KVM_XEN_HVM_SET_ATTR, &lm);
+
+	struct kvm_xen_hvm_attr ha = {
+		.type = KVM_XEN_ATTR_TYPE_SHARED_INFO,
+		.u.shared_info.gfn = SHINFO_REGION_GPA / PAGE_SIZE,
+	};
+	vm_ioctl(vm, KVM_XEN_HVM_SET_ATTR, &ha);
+
+	struct kvm_xen_vcpu_attr vi = {
+		.type = KVM_XEN_VCPU_ATTR_TYPE_VCPU_INFO,
+		.u.gpa = SHINFO_REGION_GPA + 0x40,
+	};
+	vcpu_ioctl(vcpu, KVM_XEN_VCPU_SET_ATTR, &vi);
+
+	/* Loop the port back to this vCPU as an IPI. */
+	struct kvm_xen_hvm_attr evt = {
+		.type = KVM_XEN_ATTR_TYPE_EVTCHN,
+		.u.evtchn.send_port = EVTCHN_PORT,
+		.u.evtchn.type = EVTCHNSTAT_ipi,
+		.u.evtchn.flags = 0,
+		.u.evtchn.deliver.port.port = EVTCHN_PORT,
+		.u.evtchn.deliver.port.vcpu = vcpu->id,
+		.u.evtchn.deliver.port.priority = KVM_IRQ_ROUTING_XEN_EVTCHN_PRIO_2LEVEL,
+	};
+	vm_ioctl(vm, KVM_XEN_HVM_SET_ATTR, &evt);
+
+	r = pthread_create(&thread, NULL, memslot_churn, vm);
+	TEST_ASSERT(!r, "pthread_create() failed: %d", r);
+
+	while (!done) {
+		vcpu_run(vcpu);
+
+		/*
+		 * In the window where the shared_info slot is fully deleted
+		 * (between the churn thread's DELETE and re-CREATE), the
+		 * delivery fails with -EFAULT and the hypercall bounces to
+		 * userspace as KVM_EXIT_XEN. Complete it with an error
+		 * result and keep going; the failure mode under test is
+		 * the vCPU never coming back (deadlock -> timeout), not
+		 * the hypercall rc.
+		 */
+		if (vcpu->run->exit_reason == KVM_EXIT_XEN &&
+		    vcpu->run->xen.type == KVM_EXIT_XEN_HCALL) {
+			vcpu->run->xen.u.hcall.result = -EFAULT;
+			continue;
+		}
+
+		TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO);
+
+		switch (get_ucall(vcpu, &uc)) {
+		case UCALL_SYNC:
+			break;
+		case UCALL_DONE:
+			done = 1;
+			break;
+		case UCALL_ABORT:
+			REPORT_GUEST_ASSERT(uc);
+		default:
+			TEST_FAIL("Unexpected ucall %lu", uc.cmd);
+		}
+	}
+
+	atomic_store(&churn_stop, true);
+	pthread_join(thread, NULL);
+
+	kvm_vm_free(vm);
+	return 0;
+}
-- 
2.43.0

Re: [PATCH v2 05/11] KVM: x86/xen: Don't lose the timer event when delivery fails transiently

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] kvm: x86/xen: Stale hrtimer expiry used when re-arming past-due timer in GET_ATTR
--

commit 3d44826b4ceac0133a07269c9831163b8fa3607e
Author: David Woodhouse <dwmw@amazon.co.uk>

KVM: x86/xen: Don't lose the timer event when delivery fails transiently

This commit modifies the KVM Xen timer injection logic to retry deferred
timer events if the event channel fails to set due to transient issues like
memslot updates. It clears the pending flag on unrecoverable errors while
preserving the expiration to allow migration and retry. It also updates the
GET_ATTR timer path to avoid re-arming the hrtimer when delivery is pending.

> diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
> index cd1063d659cc5..f1be9022bc6fe 100644
> --- a/arch/x86/kvm/xen.c
> +++ b/arch/x86/kvm/xen.c

[ ... ]

> @@ -1296,8 +1319,20 @@ int kvm_xen_vcpu_get_attr(struct kvm_vcpu *vcpu, struct kvm_xen_vcpu_attr *data)
>  		 * That's fine, as the guest won't even have had a chance
>  		 * to run and handle the interrupt. Asserting an already
>  		 * pending event channel is idempotent.
> +		 *
> +		 * Don't re-arm if the flush above failed transiently and
> +		 * left the timer pending: delivery is owned by the retry
> +		 * on the guest entry path, and an immediate hrtimer fire
> +		 * would race it — the callback's timer_pending suppression
> +		 * check is not serialized against the entry path, so a
> +		 * late callback could re-assert a port the entry path had
> +		 * already delivered and the guest already consumed.
> +		 *
> +		 * An expiry already in the past fires immediately: the
> +		 * event is still owed.
>  		 */
> -		if (vcpu->arch.xen.timer_expires)
> +		if (vcpu->arch.xen.timer_expires &&
> +		    !atomic_read(&vcpu->arch.xen.timer_pending))
>  			hrtimer_start_expires(&vcpu->arch.xen.timer,
>  					      HRTIMER_MODE_ABS_HARD);

[Severity: High]
Does this re-arm the timer using a stale expiry time if the original timer was
set in the past?

In kvm_xen_start_timer(), if a guest sets a timer that is already in the past:

	if (delta <= 0)
		xen_timer_callback(&vcpu->arch.xen.timer);
	else
		hrtimer_start(&vcpu->arch.xen.timer,
			      ktime_add_ns(kernel_now, delta),
			      HRTIMER_MODE_ABS_HARD);

KVM directly calls xen_timer_callback() without calling hrtimer_start().
Bypassing hrtimer_start() means the internal expires field of the struct
hrtimer is not updated, retaining whatever previous timestamp it had.

If xen_timer_callback() fails to deliver the event, and then
kvm_xen_inject_timer_irqs() attempts delivery but fails with -EINVAL, this
patch clears timer_pending but keeps timer_expires > 0.

If userspace later calls KVM_XEN_VCPU_GET_ATTR, it sees timer_expires > 0 and
timer_pending == 0, and attempts to re-arm the timer here using
hrtimer_start_expires().

Because the internal hrtimer expiry was never updated, would
hrtimer_start_expires() arm the timer using the stale future expiry time,
effectively delaying the past-due timer and causing it to hang on the host?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014220508.518772-1-dwmw2@infradead.org?part=5

Re: [PATCH v2 06/11] KVM: x86/xen: Retry event channel delivery which races a memslot update

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] kvm: host DoS via userspace-reachable WARN_ON_ONCE in kvm_xen_set_evtchn
- [Medium] kvm: xen: guest #GP on hypercall page WRMSR racing with memslot update
--

commit 1bad660e92709c34e97084491648f1684e74aada
Author: David Woodhouse <dwmw@amazon.co.uk>

KVM: x86/xen: Retry event channel delivery which races a memslot update

Event channel delivery now returns -EAGAIN when it races a memslot
update. It always failed; now the failure is distinguishable, and
instead of dropping the event each caller reacts according to its
context.

Reported-by: sashiko-bot@kernel.org
Closes: https://lore.kernel.org/all/20260920215341.5ECE91F000FF@smtp.kernel.org
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: LLM

> Event channel delivery now returns -EAGAIN when it races a memslot
> update. It always failed; now the failure is distinguishable, and
> instead of dropping the event each caller reacts according to its
> context:

[Severity: Medium]
Will this newly distinguishable -EAGAIN error cause a guest #GP on hypercall
page WRMSR racing with a memslot update?

In arch/x86/kvm/xen.c:kvm_xen_write_hypercall_page(), the guest executes
WRMSR to set up the Xen hypercall page, which runs under the vCPU's
kvm->srcu read lock. If the guest's 32/64-bit mode changed, it attempts to
re-initialize the shared_info page:

	if (kvm->arch.xen.long_mode != lm) {
		kvm->arch.xen.long_mode = lm;

		/*
		 * Re-initialize shared_info to put the wallclock in the
		 * correct place.
		 */
		if (kvm->arch.xen.shinfo_cache.active &&
		    kvm_xen_shared_info_init(kvm, false))
			r = 1;
	}

Because wait=false is explicitly passed to avoid deadlocking srcu,
kvm_xen_shared_info_init returns -EAGAIN if it races with a concurrent
host memslot update.

kvm_xen_write_hypercall_page treats any non-zero return (including the
newly distinguishable -EAGAIN transient error) as a failure by setting
r = 1 and returning it. kvm_set_msr_common receives this 1 and incorrectly
injects a #GP into the guest, crashing the guest purely due to a transient
host-side memslot update.

> diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
> index f1be9022bc6fe..a88f571e8682b 100644
> --- a/arch/x86/kvm/xen.c
> +++ b/arch/x86/kvm/xen.c

[ ... ]

> @@ -2076,7 +2088,7 @@ static int evtchn_set_fn(struct kvm_kernel_irq_routing_entry *e, struct kvm *kvm
>  	if (!level)
>  		return -EINVAL;
>  
> -	return kvm_xen_set_evtchn(&e->xen_evtchn, kvm);
> +	return kvm_xen_set_evtchn(&e->xen_evtchn, kvm, true);
>  }

[Severity: High]
Does this expose a host denial of service to unprivileged userspace threads?

An unprivileged userspace process can create a KVM VM and set up an IRQ
routing entry for a Xen event channel using KVM_SET_GSI_ROUTING. The process
then forks, passing the VM file descriptor to the child process, which now
runs with a different mm (current->mm != kvm->mm).

The child process can issue a KVM_IRQ_LINE ioctl on the VM fd, which invokes
evtchn_set_fn() and subsequently kvm_xen_set_evtchn().

In kvm_xen_set_evtchn():

	if (current->mm != kvm->mm) {
		/*
		 * If not on a thread which already belongs to this KVM,
		 * we'd better be in the irqfd workqueue.
		 */
		if (WARN_ON_ONCE(current->mm))
			return -EINVAL;

Because current->mm != kvm->mm and the child process is a userspace thread
(current->mm != NULL), the WARN_ON_ONCE(current->mm) condition evaluates to
true. On systems configured with panic_on_warn, this allows an unprivileged
user to crash the host kernel.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014220508.518772-1-dwmw2@infradead.org?part=6

Re: [PATCH v2 04/11] KVM: x86/xen: Wait out memslot updates in the Xen attribute ioctls

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] kvm: unprotected kvm_memslots() dereference in kvm_xen_start_timer() due to missing srcu_read_lock()
--

commit 4fa2fbe5b2a708cc92e83f1efa33364d90021a44
Author: David Woodhouse <dwmw@amazon.co.uk>

KVM: x86/xen: Wait out memslot updates in the Xen attribute ioctls

Activating a shared_info, vcpu_info, pvclock or runstate cache can fail if it
races a memslot update: previously with -EFAULT, now with -EAGAIN. These
ioctls may sleep, so pass wait and let the activation ride the update out.
The overarching SRCU read sections around the activation calls are removed
to prevent deadlocks against memslot updates.

> diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
> index 74dad6789e428..cd1063d659cc5 100644
> --- a/arch/x86/kvm/xen.c
> +++ b/arch/x86/kvm/xen.c

[ ... ]

> @@ -937,7 +938,6 @@ 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);
> -	idx = srcu_read_lock(&vcpu->kvm->srcu);
>  
>  	switch (data->type) {
>  	case KVM_XEN_VCPU_ATTR_TYPE_VCPU_INFO:

[Severity: High]
With the overarching SRCU read lock removed here, are there paths in the
switch statement that still require the lock but were missed?

For example, in the KVM_XEN_VCPU_ATTR_TYPE_TIMER case:

arch/x86/kvm/xen.c:kvm_xen_vcpu_set_attr() {
    ...
    case KVM_XEN_VCPU_ATTR_TYPE_TIMER:
        ...
        if (data->u.timer.port && data->u.timer.expires_ns)
            kvm_xen_start_timer(vcpu, data->u.timer.expires_ns, false);
    ...
}

This calls down into xen_get_guest_pvclock() and kvm_gpc_check(), which
then accesses kvm_memslots(gpc->kvm):

virt/kvm/pfncache.c:kvm_gpc_check() {
    struct kvm_memslots *slots = kvm_memslots(gpc->kvm);
    ...
}

Accessing kvm_memslots() triggers a srcu_dereference_check() that fails
without the SRCU lock held, and could result in a use-after-free if
kvm->memslots is freed concurrently.

Will this path now result in a lockdep splat ('suspicious RCU usage') and
potential use-after-free when a userspace process calls the
KVM_XEN_VCPU_SET_ATTR ioctl with KVM_XEN_VCPU_ATTR_TYPE_TIMER?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261014220508.518772-1-dwmw2@infradead.org?part=4

Re: [PATCH v2 06/11] KVM: x86/xen: Retry event channel delivery which races a memslot update

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 1421 bytes --]

On Wed, 2026-10-14 at 22:20 +0000, sashiko-bot@kernel.org wrote:
> [Severity: Medium]
> Will this newly distinguishable -EAGAIN error cause a guest #GP on hypercall
> page WRMSR racing with a memslot update?

It always could. It was just -EFAULT before instead of -EAGAIN.

It's just never mattered because the guest isn't going to be doing
anything else that modifies memslots at the point in early boot where
it's still setting the hypercall page. And userspace really ought to be
using the _HVA shinfo which is immune anyway.

But now I've redone the ordering of xen_lock vs. kvm->srcu I think it
can just set the 'wait' flag. I'll check and do that in v3.

> [Severity: High]
> Does this expose a host denial of service to unprivileged userspace threads?
> 
> An unprivileged userspace process can create a KVM VM and set up an IRQ
> routing entry for a Xen event channel using KVM_SET_GSI_ROUTING. The process
> then forks, passing the VM file descriptor to the child process, which now
> runs with a different mm (current->mm != kvm->mm).

There's a gate on both kvm_vm_ioctl() and kvm_vcpu_ioctl() which will
return -EIO in that case, isn't there?

You could pass an *eventfd* to another process and trigger it there,
but that would only invoke kvm_xen_set_evtchn_fast() directly, and fall
back to the workqueue (with !current->mm) if needed.

I can't see how this one could happen.

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 6179 bytes --]