From: Sean Christopherson <seanjc@google.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Kernel Mailing List, Linux" <linux-kernel@vger.kernel.org>,
kvm <kvm@vger.kernel.org>, Hillf Danton <hdanton@sina.com>,
Steven Rostedt <rostedt@goodmis.org>,
Harry Yoo <harry@kernel.org>,
syzbot <syzbot+f11512d1e5878ba5e60e@syzkaller.appspotmail.com>,
stable <stable@vger.kernel.org>
Subject: Re: [PATCH 1/2] KVM: register asynchronous page fault waits with interrupts disabled
Date: Tue, 3 Nov 2026 16:47:44 -0800 [thread overview]
Message-ID: <auqAsEk9o6buPNiR@google.com> (raw)
In-Reply-To: <CABgObfbvv-fQnC35EU+TxEtNMyYA0wgJ-nA_2B1eqKrZ_2qqkQ@mail.gmail.com>
On Tue, Nov 03, 2026, Paolo Bonzini wrote:
> Il mar 3 nov 2026, 15:43 Sean Christopherson <seanjc@google.com> ha scritto:
> >
> > On Tue, Nov 03, 2026, Paolo Bonzini wrote:
> > > KVM cannot use a node on the stack like kvm_async_pf_task_wait_schedule() does
> >
> > Oh! Yes, KVM can! If we fully handle the PV async #PF in vcpu_enter_guest(),
> > then the node can be put on the stack, right? Juggling preemption and guest
> > timing is heinous, but on the plus side it consolidates the PV async #PF code,
> > so I think it's a net positive? Especially when factoring in the node stuff.
> >
> > Fully handle PV async #PFs (when running as a KVM guest) before enabling
> > IRQs so that it's impossible for the current task, i.e. the vCPU task, to
> > be preempted before invoking kvm_async_pf_task_wait_schedule(). This will
> > allow the guest-side async #PF code to eliminate its dummy allocation,
> > which is needed only to handle the case where the page ready notification
> > is delivered before handling the not-present #PF.
> >
> > Add dedicated handling for PV async #PFs instead of wrapping/piggybacking
> > kvm_x86_ops.handle_exit_irqoff(), as the act of handling an async #PF may
> > enable IRQs and schedule out the current task, i.e. needs to be done just
> > before KVM itself explicitly enables IRQs, and needs to enable preemption
> > before calling into the kernel's PV async #PF handler.
> >
> > Opportunistically wrap the code with CONFIG_KVM_GUEST to help document the
> > context, and drop the CONFIG_KVM_GUEST=n stubs.
> >
> > Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
>
> More like Instigated-by. :)
>
> The main advantage would be that it bypasses the missing lock issue
> that Sashiko reported, but I don't know... That;s surely nice, but at
> the same time it is not great having to add a new callback,
Agreed, though IMO it's better than adding another vCPU field. E.g. we could
just as easily add apf.host_cr2 instead of host_apf_node.
> duplicate the guest_time_exit/preempt_enable code into kvm_handle_pv_async_pf().
Yeah, I was all excited about the idea, and that excitement definitely fizzled
when I realized preemption needed to be enabled.
> Not that the extra field in vcpu->arch is beautiful, I admit that, but
> it seems extra unwarranted complexity.
>
> Unless you insist I will send a v2 that takes the lock properly.
I don't insist, I agree the guest_time_exit() and preempt_enable() code is too
ugly. And the part that I actually care about cleaning up is not having a bunch
of apf.host_apf_flags checks sprinkled all over, especially in the nested code.
Which definitely doesn't require fully handling async #PF with IRQs disabled, we
just need to handle them before calling into vendor code (I think).
E.g. something like this? Though to land it on top of your patch to add
host_apf_node, we'd want to define pass a struct, but that should only make the
code prettier.
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 0378577d9d8b..09416d3588d0 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -959,7 +959,6 @@ struct kvm_vcpu_arch {
u64 msr_int_val; /* MSR_KVM_ASYNC_PF_INT */
u16 vec;
u32 id;
- u32 host_apf_flags;
bool pageready_pending;
} apf;
@@ -1688,7 +1687,8 @@ 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);
+ void (*handle_exit_irqoff)(struct kvm_vcpu *vcpu, u32 *async_pf_flags,
+ unsigned long *async_pf_cr2);
void (*update_cpu_dirty_logging)(struct kvm_vcpu *vcpu, bool enable);
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index f4b12d6aef55..3cc4bf506cb1 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -4954,9 +4954,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))
@@ -4977,21 +4974,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(fault_address);
- local_irq_enable();
- } 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 07554089ec05..d4951dc8b1f7 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 581bc3a23c95..5a92a22e8e6a 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -3811,7 +3811,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)
@@ -4758,11 +4757,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)
- vcpu->arch.apf.host_apf_flags =
- kvm_read_and_reset_apf_flags();
-
kvm_clear_available_registers(vcpu, SVM_REGS_LAZY_LOAD_SET);
trace_kvm_exit(vcpu, KVM_ISA_SVM);
@@ -5088,9 +5082,15 @@ static int svm_check_intercept(struct kvm_vcpu *vcpu,
return ret;
}
-static void svm_handle_exit_irqoff(struct kvm_vcpu *vcpu)
+static void svm_handle_exit_irqoff(struct kvm_vcpu *vcpu, u32 *async_pf_flags,
+ unsigned long *async_pf_cr2)
{
switch (to_svm(vcpu)->vmcb->control.exit_code) {
+ case SVM_EXIT_EXCP_BASE + PF_VECTOR:
+ *async_pf_flags = kvm_read_and_reset_apf_flags();
+ if (*async_pf_flags)
+ *async_pf_cr2 = to_svm(vcpu)->vmcb->control.exit_info_2;
+ break;
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 d75170a6199e..05e1d600dbd8 100644
--- a/arch/x86/kvm/vmx/main.c
+++ b/arch/x86/kvm/vmx/main.c
@@ -912,17 +912,21 @@ 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 void handle_exception_irqoff(struct kvm_vcpu *vcpu, u32 intr_info,
+ u32 *async_pf_flags,
+ unsigned long *async_pf_cr2)
{
- /* 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();
/* if exit due to NM, handle before interrupts are enabled */
- else if (is_nm_fault(intr_info))
+ if (is_page_fault(intr_info)) {
+ *async_pf_flags = kvm_read_and_reset_apf_flags();
+ if (*async_pf_flags)
+ *async_pf_cr2 = vt_get_exit_qual(vcpu);
+ } else 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))
+ } else if (is_machine_check(intr_info)) {
kvm_machine_check();
+ }
}
static void handle_external_interrupt_irqoff(struct kvm_vcpu *vcpu,
@@ -941,7 +945,8 @@ 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 void vt_handle_exit_irqoff(struct kvm_vcpu *vcpu, u32 *async_pf_flags,
+ unsigned long *async_pf_cr2)
{
if (to_vt(vcpu)->emulation_required)
return;
@@ -951,7 +956,8 @@ static void 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));
+ handle_exception_irqoff(vcpu, vt_get_intr_info(vcpu),
+ async_pf_flags, async_pf_cr2);
break;
case EXIT_REASON_MCE_DURING_VMENTRY:
kvm_machine_check();
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index f0d75f5a968c..087c12e11d74 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -6500,8 +6500,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 b341af4c6057..15f54474382b 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);
}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 26e7beaff8a0..57aff240cdb6 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8118,6 +8118,19 @@ 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(u32 flags, unsigned long cr2)
+{
+ if (WARN_ON_ONCE(!IS_ENABLED(CONFIG_KVM_GUEST)))
+ return;
+
+ if (WARN_ON_ONCE(!(flags & KVM_PV_REASON_PAGE_NOT_PRESENT)))
+ return;
+
+ local_irq_disable();
+ kvm_async_pf_task_wait_schedule(cr2);
+ local_irq_enable();
+}
+
/*
* Called within kvm->srcu read side.
* Returns 1 to let vcpu_run() continue the guest execution loop without
@@ -8130,9 +8143,10 @@ 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);
+ unsigned long async_pf_cr2;
fastpath_t exit_fastpath;
u64 run_flags, debug_ctl;
-
+ u32 async_pf_flags = 0;
bool req_immediate_exit = false;
if (kvm_request_pending(vcpu)) {
@@ -8506,7 +8520,7 @@ 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);
+ kvm_x86_call(handle_exit_irqoff)(vcpu, &async_pf_flags, &async_pf_cr2);
if (vcpu->arch.guest_fpu.xfd_err)
wrmsrq(MSR_IA32_XFD_ERR, 0);
@@ -8568,6 +8582,11 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
if (vcpu->arch.apic_attention)
kvm_lapic_sync_from_vapic(vcpu);
+ if (async_pf_flags && !WARN_ON_ONCE(exit_fastpath != EXIT_FASTPATH_NONE)) {
+ kvm_handle_pv_async_pf(async_pf_flags, async_pf_cr2);
+ exit_fastpath = EXIT_FASTPATH_EXIT_HANDLED;
+ }
+
r = kvm_x86_call(handle_exit)(vcpu, exit_fastpath);
return r;
next prev parent reply other threads:[~2026-11-04 0:47 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-03 12:49 [PATCH 0/2] KVM: remove the dummy allocation case from async page fault handling Paolo Bonzini
2026-11-03 12:49 ` [PATCH 1/2] KVM: register asynchronous page fault waits with interrupts disabled Paolo Bonzini
2026-11-03 14:43 ` Sean Christopherson
2026-11-03 20:55 ` Paolo Bonzini
2026-11-04 0:47 ` Sean Christopherson [this message]
2026-11-03 12:49 ` [PATCH 2/2] x86: KVM: remove the dummy allocation case from async page fault handling Paolo Bonzini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=auqAsEk9o6buPNiR@google.com \
--to=seanjc@google.com \
--cc=harry@kernel.org \
--cc=hdanton@sina.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=rostedt@goodmis.org \
--cc=stable@vger.kernel.org \
--cc=syzbot+f11512d1e5878ba5e60e@syzkaller.appspotmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®