[PATCH v2 0/5] KVM: remove the dummy allocation case from async page fault handling

[PATCH v2 0/5] KVM: remove the dummy allocation case from async page fault handling

[Paolo Bonzini]

Asynchronous page fault handling is supposedly able to detect page
ready notifications that are sent before the waiter is registered,
but this code is broken for PREEMPT_RT because it tries to allocate in
hardirq-disabled context.

This code however is almost dead, because page ready notification will
happen on the same vCPU as the waiter and not until after the waiter
enables interrupts.  Outside guest mode, interrupts are only enabled once
the waiter has registered itself, and this guarantees that an outstanding
(non-spurious) waiter will always be found.  This unfortunately is not
the case for asynchronous page faults that are delivered while in guest
mode, but that can be fixed and the dubious code removed.

On top of this I have rebased Sean's attempt at moving asynchronous
page fault handling to vcpu_enter_guest(), rebased on top of the new
code that splits registration for wait.

Paolo Bonzini (5):
  x86: KVM: take b->lock around wait
  KVM: x86: register asynchronous page fault waits with interrupts disabled
  x86: KVM: remove the dummy allocation case from async page fault handling
  KVM: x86: allow handle_exit_irqoff to return EXIT_FASTPATH_*
  KVM: x86: move asynchronous page fault handling to vcpu_enter_guest()

 arch/x86/include/asm/kvm_host.h |   5 +-
 arch/x86/include/asm/kvm_para.h |  20 ++++++-
 arch/x86/kernel/kvm.c           | 103 ++++++++++----------------------
 arch/x86/kvm/mmu.h              |  17 ++++++
 arch/x86/kvm/mmu/mmu.c          |  19 +-----
 arch/x86/kvm/svm/nested.c       |   4 --
 arch/x86/kvm/svm/svm.c          |  17 +++---
 arch/x86/kvm/vmx/main.c         |  24 ++++----
 arch/x86/kvm/vmx/nested.c       |   3 +-
 arch/x86/kvm/vmx/vmx.c          |   6 +-
 arch/x86/kvm/x86.c              |  18 +++++-
 11 files changed, 114 insertions(+), 122 deletions(-)

-- 
2.56.0

[PATCH 1/5] x86: KVM: take b->lock around wait

[Paolo Bonzini]

If a vCPU thread is migrated between an async page fault vmexit and the
call to kvm_async_pf_wait_schedule(), the wait will occur on a different
CPU than the page ready notification.  When this happens,
kvm_async_pf_wait_schedule() can exit and free the on-stack node
before swake_up_one() finishes.  Avoid this by protecting the
accesses to &n.link with the bucket spinlock.

Note that in principle this could use a single critical section that
also covers kvm_async_pf_queue_task(), but that call is going to be
moved out of kvm_async_pf_wait_schedule().

Cc: stable@vger.kernel.org
Cc: Sean Christopherson <seanjc@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kernel/kvm.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 9f0efbe94793..6c0a794ed89a 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -152,6 +152,8 @@ static bool kvm_async_pf_queue_task(u32 token, struct kvm_task_sleep_node *n)
  */
 void kvm_async_pf_task_wait_schedule(u32 token)
 {
+	u32 key = hash_32(token, KVM_TASK_SLEEP_HASHBITS);
+	struct kvm_task_sleep_head *b = &async_pf_sleepers[key];
 	struct kvm_task_sleep_node n;
 	DECLARE_SWAITQUEUE(wait);
 
@@ -160,16 +162,20 @@ void kvm_async_pf_task_wait_schedule(u32 token)
 	if (!kvm_async_pf_queue_task(token, &n))
 		return;
 
+	raw_spin_lock(&b->lock);
 	for (;;) {
 		prepare_to_swait_exclusive(&n.wq, &wait, TASK_UNINTERRUPTIBLE);
 		if (hlist_unhashed(&n.link))
 			break;
 
+		raw_spin_unlock(&b->lock);
 		local_irq_enable();
 		schedule();
 		local_irq_disable();
+		raw_spin_lock(&b->lock);
 	}
 	finish_swait(&n.wq, &wait);
+	raw_spin_unlock(&b->lock);
 }
 EXPORT_SYMBOL_FOR_KVM(kvm_async_pf_task_wait_schedule);
 
-- 
2.56.0

[PATCH 2/5] KVM: x86: register asynchronous page fault waits with interrupts disabled

[Paolo Bonzini]

Asynchronous page fault handling has some wonky code to detect page ready
notifications that are sent before the waiter is registered.  When page
ready notifications for asynchronous page faults were separated from
not-present notifications and turned into an interrupt, this code was
unwittingly made almost-redundant.  Page ready notification is now delayed
until after interrupts are enabled; and for an asynchronous page faults
that occur outside guest mode, Linux never enables interrupts between the
delivery of the #PF exception and the call to kvm_async_pf_queue_task().
Therefore, the dummy allocation path is fully dead on that common path.

For the nested virtualization case, however, vmx.c and svm.c do
record in the interrupt-disabled region whether a #PF vmexit is an
asynchronous page fault, but they defer registration of the waiter until
kvm_handle_page_fault().  Because of this, the vCPU thread can be preempted
before kvm_async_pf_task_wait_schedule() is called, and apf_task_wake_one()
may them not find the vCPU thread in the waiters list.  It then allocates
the struct kvm_task_sleep_node* on behalf of the task that received the
asynchronous page fault.  Besides keeping alive the wonky code, allocating
the node from an hardirq-disabled region is a problem for PREEMPT_RT kernels,
as duly reported by syzkaller.

Split registration from wait so that the caller decides where to
put the node and so that KVM can anticipate the registration before
interrupts are enabled.  KVM cannot use a node on the stack like
kvm_async_pf_task_wait_schedule() does, but it can place it in the vCPU
struct instead.

Reported-by: syzbot+f11512d1e5878ba5e60e@syzkaller.appspotmail.com
Cc: Sean Christopherson <seanjc@google.com>
Cc: stable@vger.kernel.org
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/include/asm/kvm_host.h |  1 +
 arch/x86/include/asm/kvm_para.h | 21 +++++++++++++++++++--
 arch/x86/kernel/kvm.c           | 33 +++++++++++++--------------------
 arch/x86/kvm/mmu.h              | 14 ++++++++++++++
 arch/x86/kvm/mmu/mmu.c          |  4 +++-
 arch/x86/kvm/svm/svm.c          |  3 +--
 arch/x86/kvm/vmx/main.c         |  2 +-
 7 files changed, 52 insertions(+), 26 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 1ee4b127004d..3f096a0f3387 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -951,6 +951,7 @@ struct kvm_vcpu_arch {
 		u16 vec;
 		u32 id;
 		u32 host_apf_flags;
+		struct kvm_task_sleep_node host_apf_node;
 		bool pageready_pending;
 	} apf;
 
diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h
index 4a47c16e2df8..1e28c89b1405 100644
--- a/arch/x86/include/asm/kvm_para.h
+++ b/arch/x86/include/asm/kvm_para.h
@@ -5,6 +5,7 @@
 #include <asm/processor.h>
 #include <asm/alternative.h>
 #include <linux/interrupt.h>
+#include <linux/swait.h>
 #include <uapi/asm/kvm_para.h>
 
 #include <asm/tdx.h>
@@ -117,13 +118,22 @@ static inline long kvm_sev_hypercall3(unsigned int nr, unsigned long p1,
 	return ret;
 }
 
+struct kvm_task_sleep_node {
+	struct hlist_node link;
+	struct swait_queue_head wq;
+	u32 token;
+	int cpu;
+	bool dummy;
+};
+
 #ifdef CONFIG_KVM_GUEST
 void kvmclock_init(void);
 void kvmclock_disable(void);
 bool kvm_para_available(void);
 unsigned int kvm_arch_para_features(void);
 unsigned int kvm_arch_para_hints(void);
-void kvm_async_pf_task_wait_schedule(u32 token);
+bool kvm_async_pf_queue_task(u32 token, struct kvm_task_sleep_node *n);
+void kvm_async_pf_task_wait_schedule(struct kvm_task_sleep_node *n);
 u32 kvm_read_and_reset_apf_flags(void);
 bool __kvm_handle_async_pf(struct pt_regs *regs, u32 token);
 
@@ -146,7 +156,14 @@ static inline void kvm_spinlock_init(void)
 #endif /* CONFIG_PARAVIRT_SPINLOCKS */
 
 #else /* CONFIG_KVM_GUEST */
-#define kvm_async_pf_task_wait_schedule(T) do {} while(0)
+static inline bool kvm_async_pf_queue_task(u32 token, struct kvm_task_sleep_node *n)
+{
+	return false;
+}
+
+static inline void kvm_async_pf_task_wait_schedule(struct kvm_task_sleep_node *n)
+{
+}
 
 static inline bool kvm_para_available(void)
 {
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index 6c0a794ed89a..d3a0e0d7bb84 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -80,14 +80,6 @@ static int has_guest_poll = 0;
 #define KVM_TASK_SLEEP_HASHBITS 8
 #define KVM_TASK_SLEEP_HASHSIZE (1<<KVM_TASK_SLEEP_HASHBITS)
 
-struct kvm_task_sleep_node {
-	struct hlist_node link;
-	struct swait_queue_head wq;
-	u32 token;
-	int cpu;
-	bool dummy;
-};
-
 static struct kvm_task_sleep_head {
 	raw_spinlock_t lock;
 	struct hlist_head list;
@@ -108,12 +100,14 @@ static struct kvm_task_sleep_node *_find_apf_task(struct kvm_task_sleep_head *b,
 	return NULL;
 }
 
-static bool kvm_async_pf_queue_task(u32 token, struct kvm_task_sleep_node *n)
+bool kvm_async_pf_queue_task(u32 token, struct kvm_task_sleep_node *n)
 {
 	u32 key = hash_32(token, KVM_TASK_SLEEP_HASHBITS);
 	struct kvm_task_sleep_head *b = &async_pf_sleepers[key];
 	struct kvm_task_sleep_node *e;
 
+	lockdep_assert_irqs_disabled();
+
 	raw_spin_lock(&b->lock);
 	e = _find_apf_task(b, token);
 	if (e) {
@@ -142,30 +136,27 @@ static bool kvm_async_pf_queue_task(u32 token, struct kvm_task_sleep_node *n)
 	raw_spin_unlock(&b->lock);
 	return true;
 }
+EXPORT_SYMBOL_FOR_KVM(kvm_async_pf_queue_task);
 
 /*
  * kvm_async_pf_task_wait_schedule - Wait for pagefault to be handled
- * @token:	Token to identify the sleep node entry
+ * @n:	Previously queued sleep node.
  *
  * Invoked from the async pagefault handling code or from the VM exit page
  * fault handler. In both cases RCU is watching.
  */
-void kvm_async_pf_task_wait_schedule(u32 token)
+void kvm_async_pf_task_wait_schedule(struct kvm_task_sleep_node *n)
 {
-	u32 key = hash_32(token, KVM_TASK_SLEEP_HASHBITS);
+	u32 key = hash_32(n->token, KVM_TASK_SLEEP_HASHBITS);
 	struct kvm_task_sleep_head *b = &async_pf_sleepers[key];
-	struct kvm_task_sleep_node n;
 	DECLARE_SWAITQUEUE(wait);
 
 	lockdep_assert_irqs_disabled();
 
-	if (!kvm_async_pf_queue_task(token, &n))
-		return;
-
 	raw_spin_lock(&b->lock);
 	for (;;) {
-		prepare_to_swait_exclusive(&n.wq, &wait, TASK_UNINTERRUPTIBLE);
-		if (hlist_unhashed(&n.link))
+		prepare_to_swait_exclusive(&n->wq, &wait, TASK_UNINTERRUPTIBLE);
+		if (hlist_unhashed(&n->link))
 			break;
 
 		raw_spin_unlock(&b->lock);
@@ -174,7 +165,7 @@ void kvm_async_pf_task_wait_schedule(u32 token)
 		local_irq_disable();
 		raw_spin_lock(&b->lock);
 	}
-	finish_swait(&n.wq, &wait);
+	finish_swait(&n->wq, &wait);
 	raw_spin_unlock(&b->lock);
 }
 EXPORT_SYMBOL_FOR_KVM(kvm_async_pf_task_wait_schedule);
@@ -279,6 +270,7 @@ static __always_inline bool kvm_apf_reason_pending(void)
 
 noinstr bool __kvm_handle_async_pf(struct pt_regs *regs, u32 token)
 {
+	struct kvm_task_sleep_node n;
 	irqentry_state_t state;
 	u32 flags;
 
@@ -320,7 +312,8 @@ noinstr bool __kvm_handle_async_pf(struct pt_regs *regs, u32 token)
 
 	if (flags & KVM_PV_REASON_PAGE_NOT_PRESENT) {
 		/* Page is swapped out by the host. */
-		kvm_async_pf_task_wait_schedule(token);
+		if (kvm_async_pf_queue_task(token, &n))
+			kvm_async_pf_task_wait_schedule(&n);
 	} else {
 		WARN_ONCE(1, "Unexpected async PF flags: %x\n", flags);
 	}
diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
index b2a295a9f084..3cb2616d88ac 100644
--- a/arch/x86/kvm/mmu.h
+++ b/arch/x86/kvm/mmu.h
@@ -162,6 +162,20 @@ void kvm_configure_mmu(bool enable_tdp, int tdp_forced_root_level,
 bool kvm_can_do_async_pf(struct kvm_vcpu *vcpu);
 int kvm_handle_page_fault(struct kvm_vcpu *vcpu, u64 error_code,
 				u64 fault_address, char *insn, int insn_len);
+
+static inline void kvm_handle_host_apf_irqoff(struct kvm_vcpu *vcpu, u32 token)
+{
+	vcpu->arch.apf.host_apf_flags = kvm_read_and_reset_apf_flags();
+	if ((vcpu->arch.apf.host_apf_flags & KVM_PV_REASON_PAGE_NOT_PRESENT) &&
+	    !kvm_async_pf_queue_task(token, &vcpu->arch.apf.host_apf_node)) {
+		/*
+		 * Nothing to do but regular page fault handling must still
+		 * be suppressed, so leave host_apf_flags nonzero.
+		 */
+		vcpu->arch.apf.host_apf_flags = KVM_PV_REASON_PAGE_READY;
+	}
+}
+
 void __kvm_mmu_refresh_passthrough_bits(struct kvm_vcpu *vcpu,
 					struct kvm_pagewalk *pw);
 
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index dc2aef72ffdd..1f2ae73275aa 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -4979,8 +4979,10 @@ int kvm_handle_page_fault(struct kvm_vcpu *vcpu, u64 error_code,
 	} else if (flags & KVM_PV_REASON_PAGE_NOT_PRESENT) {
 		vcpu->arch.apf.host_apf_flags = 0;
 		local_irq_disable();
-		kvm_async_pf_task_wait_schedule(fault_address);
+		kvm_async_pf_task_wait_schedule(&vcpu->arch.apf.host_apf_node);
 		local_irq_enable();
+	} else if (flags & KVM_PV_REASON_PAGE_READY) {
+		vcpu->arch.apf.host_apf_flags = 0;
 	} else {
 		WARN_ONCE(1, "Unexpected host async PF flags: %x\n", flags);
 	}
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 136069dfcd44..e0ff458761cf 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -4764,8 +4764,7 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu, u64 run_flags)
 
 	/* if exit due to PF check for async PF */
 	if (svm->vmcb->control.exit_code == SVM_EXIT_EXCP_BASE + PF_VECTOR)
-		vcpu->arch.apf.host_apf_flags =
-			kvm_read_and_reset_apf_flags();
+		kvm_handle_host_apf_irqoff(vcpu, svm->vmcb->control.exit_info_2);
 
 	kvm_clear_available_registers(vcpu, SVM_REGS_LAZY_LOAD_SET);
 
diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c
index d75170a6199e..b46f8f60ce70 100644
--- a/arch/x86/kvm/vmx/main.c
+++ b/arch/x86/kvm/vmx/main.c
@@ -916,7 +916,7 @@ static void handle_exception_irqoff(struct kvm_vcpu *vcpu, u32 intr_info)
 {
 	/* if exit due to PF check for async PF */
 	if (is_page_fault(intr_info))
-		vcpu->arch.apf.host_apf_flags = kvm_read_and_reset_apf_flags();
+		kvm_handle_host_apf_irqoff(vcpu, vt_get_exit_qual(vcpu));
 	/* if exit due to NM, handle before interrupts are enabled */
 	else if (is_nm_fault(intr_info))
 		handle_nm_fault_irqoff(vcpu);
-- 
2.56.0

[PATCH 3/5] x86: KVM: remove the dummy allocation case from async page fault handling

[Paolo Bonzini]

The page ready notification for asynchronous page faults is delivered as
an interrupt to the same vCPU that received the not-present exception.
As long as interrupts are not enabled before the page fault is registered
in the sleepers' hash table, the page ready notification must happen
after a prior registration.

This is now the case indeed, so the dummy allocation case is dead and
can be dropped.

Note however that a write of the apf-enable MSR that goes from enabled
to enabled might trigger *both* a broadcast notification interrupt
and, later, individual interrupts for pending asynchronous page faults.
While Linux will never do such a write, the host might do a KVM_SET_MSRS
call that would result in the broadcast notification, so do not complain
too loudly if the wait does not find a token and only do a pr_once().

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/include/asm/kvm_para.h |  1 -
 arch/x86/kernel/kvm.c           | 66 ++++++++-------------------------
 2 files changed, 15 insertions(+), 52 deletions(-)

diff --git a/arch/x86/include/asm/kvm_para.h b/arch/x86/include/asm/kvm_para.h
index 1e28c89b1405..b986edb1a225 100644
--- a/arch/x86/include/asm/kvm_para.h
+++ b/arch/x86/include/asm/kvm_para.h
@@ -123,7 +123,6 @@ struct kvm_task_sleep_node {
 	struct swait_queue_head wq;
 	u32 token;
 	int cpu;
-	bool dummy;
 };
 
 #ifdef CONFIG_KVM_GUEST
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index d3a0e0d7bb84..ad0b6a4ad097 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -111,26 +111,11 @@ bool kvm_async_pf_queue_task(u32 token, struct kvm_task_sleep_node *n)
 	raw_spin_lock(&b->lock);
 	e = _find_apf_task(b, token);
 	if (e) {
-		struct kvm_task_sleep_node *dummy = NULL;
-
-		/*
-		 * The entry can either be a 'dummy' entry (which is put on the
-		 * list when wake-up happens ahead of APF handling completion)
-		 * or a token from another task which should not be touched.
-		 */
-		if (e->dummy) {
-			hlist_del(&e->link);
-			dummy = e;
-		}
-
 		raw_spin_unlock(&b->lock);
-		kfree(dummy);
 		return false;
 	}
-
 	n->token = token;
 	n->cpu = smp_processor_id();
-	n->dummy = false;
 	init_swait_queue_head(&n->wq);
 	hlist_add_head(&n->link, &b->list);
 	raw_spin_unlock(&b->lock);
@@ -200,53 +185,32 @@ static void kvm_async_pf_task_wake(u32 token)
 {
 	u32 key = hash_32(token, KVM_TASK_SLEEP_HASHBITS);
 	struct kvm_task_sleep_head *b = &async_pf_sleepers[key];
-	struct kvm_task_sleep_node *n, *dummy = NULL;
+	struct kvm_task_sleep_node *n;
 
 	if (token == ~0) {
 		apf_task_wake_all();
 		return;
 	}
 
-again:
 	raw_spin_lock(&b->lock);
 	n = _find_apf_task(b, token);
-	if (!n) {
-		/*
-		 * Async #PF not yet handled, add a dummy entry for the token.
-		 * Allocating the token must be down outside of the raw lock
-		 * as the allocator is preemptible on PREEMPT_RT kernels.
-		 */
-		if (!dummy) {
-			raw_spin_unlock(&b->lock);
-			dummy = kzalloc_obj(*dummy, GFP_ATOMIC);
 
-			/*
-			 * Continue looping on allocation failure, eventually
-			 * the async #PF will be handled and allocating a new
-			 * node will be unnecessary.
-			 */
-			if (!dummy)
-				cpu_relax();
-
-			/*
-			 * Recheck for async #PF completion before enqueueing
-			 * the dummy token to avoid duplicate list entries.
-			 */
-			goto again;
-		}
-		dummy->token = token;
-		dummy->cpu = smp_processor_id();
-		dummy->dummy = true;
-		init_swait_queue_head(&dummy->wq);
-		hlist_add_head(&dummy->link, &b->list);
-		dummy = NULL;
-	} else {
+	/*
+	 * A broadcast wakeup, which can happen for example if the VMM calls
+	 * KVM_SET_MSRS, might still leave the individual wakeups queued.
+	 * In that case _find_apf_task() will not find the token when the
+	 * individual wakeup comes, so just ignore the case where n is NULL.
+	 * Nevertheless it *should* not happen, so print a message for the
+	 * user in case something bad actually happens later.
+	 */
+	if (n)
 		apf_task_wake_one(n);
-	}
-	raw_spin_unlock(&b->lock);
+	else
+		pr_info_once("Async page fault waiter not found. This is probably triggered "
+			     "by the host and harmless. However, if a userspace task hangs, "
+			     "please include this message in your bug report.\n");
 
-	/* A dummy token might be allocated and ultimately not used.  */
-	kfree(dummy);
+	raw_spin_unlock(&b->lock);
 }
 
 noinstr u32 kvm_read_and_reset_apf_flags(void)
-- 
2.56.0

[PATCH 4/5] KVM: x86: allow handle_exit_irqoff to return EXIT_FASTPATH_*

[Paolo Bonzini]

Let handle_exit_irqoff() pass what it did to vcpu_enter_guest() and/or
handle_exit().  This will be needed to trigger async page fault handling,
and to suppress reflection of APF #PF exceptions, once vcpu->arch
will no longer have a host_apf_flags field.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/include/asm/kvm_host.h | 2 +-
 arch/x86/kvm/svm/svm.c          | 4 +++-
 arch/x86/kvm/vmx/main.c         | 6 ++++--
 arch/x86/kvm/x86.c              | 3 ++-
 4 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 3f096a0f3387..93170c70a17c 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1681,7 +1681,7 @@ struct kvm_x86_ops {
 			       struct x86_instruction_info *info,
 			       enum x86_intercept_stage stage,
 			       struct x86_exception *exception);
-	void (*handle_exit_irqoff)(struct kvm_vcpu *vcpu);
+	fastpath_t (*handle_exit_irqoff)(struct kvm_vcpu *vcpu);
 
 	void (*update_cpu_dirty_logging)(struct kvm_vcpu *vcpu, bool enable);
 
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index e0ff458761cf..b720c1ff7134 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -5091,7 +5091,7 @@ static int svm_check_intercept(struct kvm_vcpu *vcpu,
 	return ret;
 }
 
-static void svm_handle_exit_irqoff(struct kvm_vcpu *vcpu)
+static fastpath_t svm_handle_exit_irqoff(struct kvm_vcpu *vcpu)
 {
 	switch (to_svm(vcpu)->vmcb->control.exit_code) {
 	case SVM_EXIT_EXCP_BASE + MC_VECTOR:
@@ -5103,6 +5103,8 @@ static void svm_handle_exit_irqoff(struct kvm_vcpu *vcpu)
 	default:
 		break;
 	}
+
+	return EXIT_FASTPATH_NONE;
 }
 
 static void svm_setup_mce(struct kvm_vcpu *vcpu)
diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c
index b46f8f60ce70..b742e2370dd6 100644
--- a/arch/x86/kvm/vmx/main.c
+++ b/arch/x86/kvm/vmx/main.c
@@ -941,10 +941,10 @@ static void handle_external_interrupt_irqoff(struct kvm_vcpu *vcpu,
 	vcpu->arch.at_instruction_boundary = true;
 }
 
-static void vt_handle_exit_irqoff(struct kvm_vcpu *vcpu)
+static fastpath_t vt_handle_exit_irqoff(struct kvm_vcpu *vcpu)
 {
 	if (to_vt(vcpu)->emulation_required)
-		return;
+		return EXIT_FASTPATH_NONE;
 
 	switch (vt_get_exit_reason(vcpu).basic) {
 	case EXIT_REASON_EXTERNAL_INTERRUPT:
@@ -959,6 +959,8 @@ static void vt_handle_exit_irqoff(struct kvm_vcpu *vcpu)
 	default:
 		break;
 	}
+
+	return EXIT_FASTPATH_NONE;
 }
 
 static int vt_handle_exit(struct kvm_vcpu *vcpu,
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 32563a91fba9..af8cdfd81180 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8504,7 +8504,8 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
 	if (vcpu->arch.xfd_no_write_intercept)
 		fpu_sync_guest_vmexit_xfd_state();
 
-	kvm_x86_call(handle_exit_irqoff)(vcpu);
+	if (exit_fastpath == EXIT_FASTPATH_NONE)
+		exit_fastpath = kvm_x86_call(handle_exit_irqoff)(vcpu);
 
 	if (vcpu->arch.guest_fpu.xfd_err)
 		wrmsrq(MSR_IA32_XFD_ERR, 0);
-- 
2.56.0

[PATCH 5/5] KVM: x86: move asynchronous page fault handling to vcpu_enter_guest()

[Paolo Bonzini]

Fully handle the PV async #PF in vcpu_enter_guest().  This has several
advantages; first of all nested async #PF code (which has more to do
with host page faults than nested MMU page faults) is moved away from
MMU page fault handling, second host_apf_flags is hidden (though only
partly so, because it is replaced by EXIT_FASTPATH_APF) and the node
put on the stack.

Reported-by: syzbot+f11512d1e5878ba5e60e@syzkaller.appspotmail.com
Link: https://patch.msgid.link/auqAsEk9o6buPNiR@google.com
Co-authored-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/include/asm/kvm_host.h |  6 +++---
 arch/x86/kvm/mmu.h              | 21 ++++++++++++---------
 arch/x86/kvm/mmu/mmu.c          | 21 ++-------------------
 arch/x86/kvm/svm/nested.c       |  4 ----
 arch/x86/kvm/svm/svm.c          | 14 +++++++-------
 arch/x86/kvm/vmx/main.c         | 20 +++++++++++---------
 arch/x86/kvm/vmx/nested.c       |  3 +--
 arch/x86/kvm/vmx/vmx.c          |  6 +-----
 arch/x86/kvm/x86.c              | 17 +++++++++++++++--
 9 files changed, 52 insertions(+), 60 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 93170c70a17c..f72028efe2ca 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -209,6 +209,7 @@ enum exit_fastpath_completion {
 	EXIT_FASTPATH_REENTER_GUEST,
 	EXIT_FASTPATH_EXIT_HANDLED,
 	EXIT_FASTPATH_EXIT_USERSPACE,
+	EXIT_FASTPATH_APF,
 };
 typedef enum exit_fastpath_completion fastpath_t;
 
@@ -950,8 +951,6 @@ struct kvm_vcpu_arch {
 		u64 msr_int_val; /* MSR_KVM_ASYNC_PF_INT */
 		u16 vec;
 		u32 id;
-		u32 host_apf_flags;
-		struct kvm_task_sleep_node host_apf_node;
 		bool pageready_pending;
 	} apf;
 
@@ -1681,7 +1680,8 @@ struct kvm_x86_ops {
 			       struct x86_instruction_info *info,
 			       enum x86_intercept_stage stage,
 			       struct x86_exception *exception);
-	fastpath_t (*handle_exit_irqoff)(struct kvm_vcpu *vcpu);
+	fastpath_t (*handle_exit_irqoff)(struct kvm_vcpu *vcpu,
+					 struct kvm_task_sleep_node *host_apf_node);
 
 	void (*update_cpu_dirty_logging)(struct kvm_vcpu *vcpu, bool enable);
 
diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
index 3cb2616d88ac..c01a1cf70d77 100644
--- a/arch/x86/kvm/mmu.h
+++ b/arch/x86/kvm/mmu.h
@@ -163,17 +163,20 @@ bool kvm_can_do_async_pf(struct kvm_vcpu *vcpu);
 int kvm_handle_page_fault(struct kvm_vcpu *vcpu, u64 error_code,
 				u64 fault_address, char *insn, int insn_len);
 
-static inline void kvm_handle_host_apf_irqoff(struct kvm_vcpu *vcpu, u32 token)
+static inline fastpath_t kvm_handle_host_apf_irqoff(struct kvm_task_sleep_node *host_apf_node,
+						    u32 token)
 {
-	vcpu->arch.apf.host_apf_flags = kvm_read_and_reset_apf_flags();
-	if ((vcpu->arch.apf.host_apf_flags & KVM_PV_REASON_PAGE_NOT_PRESENT) &&
-	    !kvm_async_pf_queue_task(token, &vcpu->arch.apf.host_apf_node)) {
-		/*
-		 * Nothing to do but regular page fault handling must still
-		 * be suppressed, so leave host_apf_flags nonzero.
-		 */
-		vcpu->arch.apf.host_apf_flags = KVM_PV_REASON_PAGE_READY;
+	u32 host_apf_flags = kvm_read_and_reset_apf_flags();
+
+	if (!host_apf_flags)
+		return EXIT_FASTPATH_NONE;
+
+	if (!WARN_ON_ONCE(!(host_apf_flags & KVM_PV_REASON_PAGE_NOT_PRESENT))) {
+		kvm_async_pf_queue_task(token, host_apf_node);
+		return EXIT_FASTPATH_APF;
 	}
+
+	return EXIT_FASTPATH_NONE;
 }
 
 void __kvm_mmu_refresh_passthrough_bits(struct kvm_vcpu *vcpu,
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 1f2ae73275aa..a8dfbfb83a6f 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -4948,9 +4948,6 @@ static int nonpaging_page_fault(struct kvm_vcpu *vcpu,
 int kvm_handle_page_fault(struct kvm_vcpu *vcpu, u64 error_code,
 				u64 fault_address, char *insn, int insn_len)
 {
-	int r = 1;
-	u32 flags = vcpu->arch.apf.host_apf_flags;
-
 #ifndef CONFIG_X86_64
 	/* A 64-bit CR2 should be impossible on 32-bit KVM. */
 	if (KVM_BUG_ON(fault_address >> 32, vcpu->kvm))
@@ -4971,23 +4968,9 @@ int kvm_handle_page_fault(struct kvm_vcpu *vcpu, u64 error_code,
 	BUILD_BUG_ON(lower_32_bits(PFERR_SYNTHETIC_MASK));
 
 	kvm_request_l1tf_flush_l1d();
-	if (!flags) {
-		trace_kvm_page_fault(vcpu, fault_address, error_code);
+	trace_kvm_page_fault(vcpu, fault_address, error_code);
 
-		r = kvm_mmu_page_fault(vcpu, fault_address, error_code, insn,
-				insn_len);
-	} else if (flags & KVM_PV_REASON_PAGE_NOT_PRESENT) {
-		vcpu->arch.apf.host_apf_flags = 0;
-		local_irq_disable();
-		kvm_async_pf_task_wait_schedule(&vcpu->arch.apf.host_apf_node);
-		local_irq_enable();
-	} else if (flags & KVM_PV_REASON_PAGE_READY) {
-		vcpu->arch.apf.host_apf_flags = 0;
-	} else {
-		WARN_ONCE(1, "Unexpected host async PF flags: %x\n", flags);
-	}
-
-	return r;
+	return kvm_mmu_page_fault(vcpu, fault_address, error_code, insn, insn_len);
 }
 EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_handle_page_fault);
 
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index af0ef678433d..36d9e39af34b 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -1818,10 +1818,6 @@ int nested_svm_exit_special(struct vcpu_svm *svm)
 		if (svm->vmcb01.ptr->control.intercepts[INTERCEPT_EXCEPTION] &
 		    excp_bits)
 			return NESTED_EXIT_HOST;
-		else if (exit_code == SVM_EXIT_EXCP_BASE + PF_VECTOR &&
-			 svm->vcpu.arch.apf.host_apf_flags)
-			/* Trap async PF even if not shadowing */
-			return NESTED_EXIT_HOST;
 		break;
 	}
 	case SVM_EXIT_VMMCALL:
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index b720c1ff7134..16d6947d981b 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -3815,7 +3815,6 @@ static void svm_get_entry_info(struct kvm_vcpu *vcpu, u32 *intr_info,
 		*error_code = control->event_inj_err;
 	else
 		*error_code = 0;
-
 }
 
 static int svm_handle_exit(struct kvm_vcpu *vcpu, fastpath_t exit_fastpath)
@@ -3834,7 +3833,7 @@ static int svm_handle_exit(struct kvm_vcpu *vcpu, fastpath_t exit_fastpath)
 	if (unlikely(exit_fastpath == EXIT_FASTPATH_EXIT_USERSPACE))
 		return 0;
 
-	if (is_guest_mode(vcpu)) {
+	if (is_guest_mode(vcpu) && exit_fastpath != EXIT_FASTPATH_APF) {
 		int vmexit;
 
 		trace_kvm_nested_vmexit(vcpu, KVM_ISA_SVM);
@@ -4762,10 +4761,6 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu, u64 run_flags)
 			       vcpu_to_pmu(vcpu)->global_ctrl);
 	}
 
-	/* if exit due to PF check for async PF */
-	if (svm->vmcb->control.exit_code == SVM_EXIT_EXCP_BASE + PF_VECTOR)
-		kvm_handle_host_apf_irqoff(vcpu, svm->vmcb->control.exit_info_2);
-
 	kvm_clear_available_registers(vcpu, SVM_REGS_LAZY_LOAD_SET);
 
 	trace_kvm_exit(vcpu, KVM_ISA_SVM);
@@ -5091,9 +5086,14 @@ static int svm_check_intercept(struct kvm_vcpu *vcpu,
 	return ret;
 }
 
-static fastpath_t svm_handle_exit_irqoff(struct kvm_vcpu *vcpu)
+static fastpath_t svm_handle_exit_irqoff(struct kvm_vcpu *vcpu,
+					 struct kvm_task_sleep_node *host_apf_node)
 {
+	struct vcpu_svm *svm = to_svm(vcpu);
+
 	switch (to_svm(vcpu)->vmcb->control.exit_code) {
+	case SVM_EXIT_EXCP_BASE + PF_VECTOR:
+		return kvm_handle_host_apf_irqoff(host_apf_node, svm->vmcb->control.exit_info_2);
 	case SVM_EXIT_EXCP_BASE + MC_VECTOR:
 		svm_handle_mce(vcpu);
 		break;
diff --git a/arch/x86/kvm/vmx/main.c b/arch/x86/kvm/vmx/main.c
index b742e2370dd6..6efa4e0051b5 100644
--- a/arch/x86/kvm/vmx/main.c
+++ b/arch/x86/kvm/vmx/main.c
@@ -912,17 +912,18 @@ static void handle_nm_fault_irqoff(struct kvm_vcpu *vcpu)
 		rdmsrq(MSR_IA32_XFD_ERR, vcpu->arch.guest_fpu.xfd_err);
 }
 
-static void handle_exception_irqoff(struct kvm_vcpu *vcpu, u32 intr_info)
+static fastpath_t handle_exception_irqoff(struct kvm_vcpu *vcpu, u32 intr_info,
+					  struct kvm_task_sleep_node *host_apf_node)
 {
-	/* if exit due to PF check for async PF */
 	if (is_page_fault(intr_info))
-		kvm_handle_host_apf_irqoff(vcpu, vt_get_exit_qual(vcpu));
-	/* if exit due to NM, handle before interrupts are enabled */
-	else if (is_nm_fault(intr_info))
+		return kvm_handle_host_apf_irqoff(host_apf_node, vt_get_exit_qual(vcpu));
+
+	if (is_nm_fault(intr_info))
 		handle_nm_fault_irqoff(vcpu);
-	/* Handle machine checks before interrupts are enabled */
 	else if (is_machine_check(intr_info))
 		kvm_machine_check();
+
+	return EXIT_FASTPATH_NONE;
 }
 
 static void handle_external_interrupt_irqoff(struct kvm_vcpu *vcpu,
@@ -941,7 +942,8 @@ static void handle_external_interrupt_irqoff(struct kvm_vcpu *vcpu,
 	vcpu->arch.at_instruction_boundary = true;
 }
 
-static fastpath_t vt_handle_exit_irqoff(struct kvm_vcpu *vcpu)
+static fastpath_t vt_handle_exit_irqoff(struct kvm_vcpu *vcpu,
+					struct kvm_task_sleep_node *host_apf_node)
 {
 	if (to_vt(vcpu)->emulation_required)
 		return EXIT_FASTPATH_NONE;
@@ -951,8 +953,8 @@ static fastpath_t vt_handle_exit_irqoff(struct kvm_vcpu *vcpu)
 		handle_external_interrupt_irqoff(vcpu, vt_get_intr_info(vcpu));
 		break;
 	case EXIT_REASON_EXCEPTION_NMI:
-		handle_exception_irqoff(vcpu, vt_get_intr_info(vcpu));
-		break;
+		return handle_exception_irqoff(vcpu, vt_get_intr_info(vcpu),
+					       host_apf_node);
 	case EXIT_REASON_MCE_DURING_VMENTRY:
 		kvm_machine_check();
 		break;
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 516ba03bf608..b7371c584f5a 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -6501,8 +6501,7 @@ static bool nested_vmx_l0_wants_exit(struct kvm_vcpu *vcpu,
 		if (is_nmi(intr_info))
 			return true;
 		else if (is_page_fault(intr_info))
-			return vcpu->arch.apf.host_apf_flags ||
-			       vmx_need_pf_intercept(vcpu);
+			return vmx_need_pf_intercept(vcpu);
 		else if (is_debug(intr_info) &&
 			 vcpu->guest_debug &
 			 (KVM_GUESTDBG_SINGLESTEP | KVM_GUESTDBG_USE_HW_BP))
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 82eb891f684c..3b6ad75ba7e7 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -5396,9 +5396,6 @@ static int vmx_handle_page_fault(struct kvm_vcpu *vcpu, u32 error_code)
 {
 	unsigned long cr2 = vt_get_exit_qual(vcpu);
 
-	if (vcpu->arch.apf.host_apf_flags)
-		goto handle_pf;
-
 	/* When using EPT, KVM intercepts #PF only to detect illegal GPAs. */
 	WARN_ON_ONCE(enable_ept && !allow_smaller_maxphyaddr);
 
@@ -5429,7 +5426,6 @@ static int vmx_handle_page_fault(struct kvm_vcpu *vcpu, u32 error_code)
 		return 1;
 	}
 
-handle_pf:
 	return kvm_handle_page_fault(vcpu, error_code, cr2, NULL, 0);
 }
 
@@ -6733,7 +6729,7 @@ int vmx_handle_exit(struct kvm_vcpu *vcpu, fastpath_t exit_fastpath)
 			return 1;
 		}
 
-		if (nested_vmx_reflect_vmexit(vcpu))
+		if (exit_fastpath != EXIT_FASTPATH_APF && nested_vmx_reflect_vmexit(vcpu))
 			return 1;
 	}
 
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index af8cdfd81180..37e721bd2372 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8116,6 +8116,16 @@ static void kvm_update_cpu_dirty_logging(struct kvm_vcpu *vcpu)
 	kvm_x86_call(update_cpu_dirty_logging)(vcpu, enable);
 }
 
+static void kvm_handle_pv_async_pf(struct kvm_task_sleep_node *n)
+{
+	if (WARN_ON_ONCE(!IS_ENABLED(CONFIG_KVM_GUEST)))
+		return;
+
+	local_irq_disable();
+	kvm_async_pf_task_wait_schedule(n);
+	local_irq_enable();
+}
+
 /*
  * Called within kvm->srcu read side.
  * Returns 1 to let vcpu_run() continue the guest execution loop without
@@ -8128,9 +8138,9 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
 	bool req_int_win =
 		dm_request_for_irq_injection(vcpu) &&
 		kvm_cpu_accept_dm_intr(vcpu);
+	struct kvm_task_sleep_node host_apf_node = {0};
 	fastpath_t exit_fastpath;
 	u64 run_flags, debug_ctl;
-
 	bool req_immediate_exit = false;
 
 	if (kvm_request_pending(vcpu)) {
@@ -8505,7 +8515,7 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
 		fpu_sync_guest_vmexit_xfd_state();
 
 	if (exit_fastpath == EXIT_FASTPATH_NONE)
-		exit_fastpath = kvm_x86_call(handle_exit_irqoff)(vcpu);
+		exit_fastpath = kvm_x86_call(handle_exit_irqoff)(vcpu, &host_apf_node);
 
 	if (vcpu->arch.guest_fpu.xfd_err)
 		wrmsrq(MSR_IA32_XFD_ERR, 0);
@@ -8567,6 +8577,9 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
 	if (vcpu->arch.apic_attention)
 		kvm_lapic_sync_from_vapic(vcpu);
 
+	if (exit_fastpath == EXIT_FASTPATH_APF && host_apf_node.token)
+		kvm_handle_pv_async_pf(&host_apf_node);
+
 	r = kvm_x86_call(handle_exit)(vcpu, exit_fastpath);
 	return r;
 
-- 
2.56.0

Re: [PATCH 5/5] KVM: x86: move asynchronous page fault handling to vcpu_enter_guest()

[Sean Christopherson]

On Thu, Nov 05, 2026, Paolo Bonzini wrote:
> Fully handle the PV async #PF in vcpu_enter_guest().  This has several
> advantages; first of all nested async #PF code (which has more to do
> with host page faults than nested MMU page faults) is moved away from
> MMU page fault handling, second host_apf_flags is hidden (though only
> partly so, because it is replaced by EXIT_FASTPATH_APF) and the node
> put on the stack.
> 
> Reported-by: syzbot+f11512d1e5878ba5e60e@syzkaller.appspotmail.com
> Link: https://patch.msgid.link/auqAsEk9o6buPNiR@google.com
> Co-authored-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/include/asm/kvm_host.h |  6 +++---
>  arch/x86/kvm/mmu.h              | 21 ++++++++++++---------
>  arch/x86/kvm/mmu/mmu.c          | 21 ++-------------------
>  arch/x86/kvm/svm/nested.c       |  4 ----
>  arch/x86/kvm/svm/svm.c          | 14 +++++++-------
>  arch/x86/kvm/vmx/main.c         | 20 +++++++++++---------
>  arch/x86/kvm/vmx/nested.c       |  3 +--
>  arch/x86/kvm/vmx/vmx.c          |  6 +-----
>  arch/x86/kvm/x86.c              | 17 +++++++++++++++--
>  9 files changed, 52 insertions(+), 60 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 93170c70a17c..f72028efe2ca 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -209,6 +209,7 @@ enum exit_fastpath_completion {
>  	EXIT_FASTPATH_REENTER_GUEST,
>  	EXIT_FASTPATH_EXIT_HANDLED,
>  	EXIT_FASTPATH_EXIT_USERSPACE,
> +	EXIT_FASTPATH_APF,

Ha!  I had coded up EXIT_FASTPATH_PF too, but never proposed it because it wasn't
useful since my approach used a dedicated kvm_x86_ops hook.

I'll take a close look at this series today or tomorrow.