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

[Paolo Bonzini <pbonzini@redhat.com>]

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
Link: https://lore.kernel.org/kvm/6adf2939.67317f00.38c4bc.00ae.GAE@google.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           | 31 ++++++++++++-------------------
 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, 51 insertions(+), 25 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 9f0efbe94793..ca5dcdc7ffde 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,34 +136,31 @@ 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)
 {
-	struct kvm_task_sleep_node n;
 	DECLARE_SWAITQUEUE(wait);
 
 	lockdep_assert_irqs_disabled();
 
-	if (!kvm_async_pf_queue_task(token, &n))
-		return;
-
 	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;
 
 		local_irq_enable();
 		schedule();
 		local_irq_disable();
 	}
-	finish_swait(&n.wq, &wait);
+	finish_swait(&n->wq, &wait);
 }
 EXPORT_SYMBOL_FOR_KVM(kvm_async_pf_task_wait_schedule);
 
@@ -273,6 +264,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;
 
@@ -314,7 +306,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