Re: [PATCH v4 3/5] KVM: SVM: Fix nested NPF injection of PFERR_GUEST_{PAGE,FINAL}_MASK bits

[Sean Christopherson <seanjc@google.com>]

On Tue, May 26, 2026, Yosry Ahmed wrote:
> On Fri, May 22, 2026 at 4:27 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > From: Kevin Cheng <chengkev@google.com>
> >
> > Fix KVM's generation of PFERR_GUEST_{PAGE,FINAL}_MASK bits when injecting a
> > Nested Page Fault into L1.  Currently, KVM blindly stuffs GUEST_FINAL into
> > L1, which is blatantly wrong given that KVM obviously generates NPFs for
> > page table accesses.
> >
> > There are two paths that trigger NPF injection: hardware NPF exits (from
> > L2) and emulation-triggered faults, i.e. when KVM detects a NPF as part of
> > emulating an L2 GVA access.  For the hardware case, use the bits verbatim
> > from the VMCB, as KVM is simply forwarding a NPF to L1.  For the emulation
> > case, propagate the GUEST_{PAGE,FINAL} bits from the access field (which
> > were recently added for MBEC+GMET support).
> >
> > To differentiate between the two cases, add "hardware_nested_page_fault"
> > to "struct x86_exception", and set it when injecting a NPF in response to
> > an NPF exit from L2.
> 
> hardware_nested_page_fault is no more.

Hrm, I suspect I unintentionally discarded a changelog update, I distinctly
remember rewriting this.  *sigh*

> > To help guard against future goofs, assert that exactly one of GUEST_PAGE
> > or GUEST_FINAL is set when injecting a NPF.  Unlike VMX, there are no
> > (known) cases where hardware doesn't set either bit, and KVM should always
> > set one or the other when emulating a GVA access.
> >
> > Signed-off-by: Kevin Cheng <chengkev@google.com>
> > [sean: use plumbed in @access bits, massage changelog]
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> [..]
> > @@ -39,19 +39,32 @@ static void nested_svm_inject_npf_exit(struct kvm_vcpu *vcpu,
> >  {
> >         struct vcpu_svm *svm = to_svm(vcpu);
> >         struct vmcb *vmcb = svm->vmcb;
> > +       u64 fault_stage;
> >
> > -       if (vmcb->control.exit_code != SVM_EXIT_NPF) {
> > -               /*
> > -                * TODO: track the cause of the nested page fault, and
> > -                * correctly fill in the high bits of exit_info_1.
> > -                */
> > -               vmcb->control.exit_code = SVM_EXIT_NPF;
> > -               vmcb->control.exit_info_1 = (1ULL << 32);
> > -               vmcb->control.exit_info_2 = fault->address;
> > -       }
> > +       /*
> > +        * For hardware NPF exits, the GUEST_FAULT_STAGE bits are only
> > +        * available in the hardware exit_info_1, since the guest_mmu
> > +        * walker doesn't know whether the faulting GPA was a page table
> > +        * page or final page from L2's perspective.
> > +        */
> > +       if (from_hardware)
> > +               fault_stage = vmcb->control.exit_info_1 &
> > +                             PFERR_GUEST_FAULT_STAGE_MASK;
> > +       else
> > +               fault_stage = fault->error_code & PFERR_GUEST_FAULT_STAGE_MASK;
> >
> > -       vmcb->control.exit_info_1 &= ~0xffffffffULL;
> > -       vmcb->control.exit_info_1 |= fault->error_code;
> > +       /*
> > +        * All nested page faults should be annotated as occurring on the
> > +        * final translation *or* the page walk. Arbitrarily choose "final"
> > +        * if KVM is buggy and enumerated both or neither.
> > +        */
> > +       if (WARN_ON_ONCE(hweight64(fault_stage) != 1))
> > +               fault_stage = PFERR_GUEST_FINAL_MASK;
> > +
> > +       vmcb->control.exit_code = SVM_EXIT_NPF;
> > +       vmcb->control.exit_info_1 = fault_stage |
> > +                                   (fault->error_code & ~PFERR_GUEST_FAULT_STAGE_MASK);
> 
> Do we need to do this in the common path?

What do you mean by "this"?  Pulling flags from fault->error_code?

> If from_hardware=true, can the fault injected by KVM have different flags
> from the one produced by hardware? 

Flags, yes.  fault_stage, no.

> I guess the answer is yes, (e.g. if KVM is doing write-protection?). Might be
> worth a comment.

Or if L1 has modified its TDP PTEs in memory, but hasn't yet flushed TLBs.  In
that case, KVM's software walker can see the updated PTEs, while hardware may
have seen something else.