Re: [PATCH v5 06/26] KVM: nSVM: Triple fault if mapping VMCB12 fails on nested #VMEXIT

[Sean Christopherson]

On Mon, Feb 23, 2026, Yosry Ahmed wrote:
> > > @@ -1146,8 +1136,16 @@ int nested_svm_vmexit(struct vcpu_svm *svm)
> > >       /* in case we halted in L2 */
> > >       kvm_set_mp_state(vcpu, KVM_MP_STATE_RUNNABLE);
> > >
> > > +     svm->nested.vmcb12_gpa = 0;
> > > +
> > > +     if (kvm_vcpu_map(vcpu, gpa_to_gfn(vmcb12_gpa), &map)) {
> > > +             kvm_make_request(KVM_REQ_TRIPLE_FAULT, vcpu);
> > > +             return 1;
> >
> > Returning early isn't entirely correct.  In fact, I think it's worse than the
> > current behavior in many aspects.
> >
> > By doing leave_guest_mode() and not switching back to vmcb01 and not putting
> > vcpu->arch.mmu back to root_mmu, the vCPU will be in L1 but with vmcb02 and L2's
> > MMU active.
> 
> Hmm yeah, the same problem also exists in
> nested_svm_vmrun_error_vmexit() after "KVM: nSVM: Restrict mapping
> VMCB12 on nested VMRUN". In that path, we only need to map vmcb12 to
> zero event_inj in __nested_svm_vmexit(). We can probably move them to
> the callers (nested_svm_vmrun_error_vmexit() and nested_svm_vmexit())
> to make it easier to skip if mapping fails.

Agreed, I don't see a better option.

> > The idea I can come up with is to isolate the vmcb12 writes (which is suprisingly
> > straightforward), and then simply skip the vmcb12 updates.  E.g.
> >
> > ---
> [..]
> > @@ -1184,14 +1168,53 @@ int nested_svm_vmexit(struct vcpu_svm *svm)
> >         if (guest_cpu_cap_has(vcpu, X86_FEATURE_NRIPS))
> >                 vmcb12->control.next_rip  = vmcb02->control.next_rip;
> >
> > +       if (nested_vmcb12_has_lbrv(vcpu))
> > +               svm_copy_lbrs(&vmcb12->save, &vmcb02->save);
> > +
> >         vmcb12->control.int_ctl           = svm->nested.ctl.int_ctl;
> >         vmcb12->control.event_inj         = svm->nested.ctl.event_inj;
> >         vmcb12->control.event_inj_err     = svm->nested.ctl.event_inj_err;
> >
> > +       trace_kvm_nested_vmexit_inject(vmcb12->control.exit_code,
> > +                                      vmcb12->control.exit_info_1,
> > +                                      vmcb12->control.exit_info_2,
> > +                                      vmcb12->control.exit_int_info,
> > +                                      vmcb12->control.exit_int_info_err,
> > +                                      KVM_ISA_SVM);
> > +}
> > +
> > +int nested_svm_vmexit(struct vcpu_svm *svm)
> > +{
> > +       struct kvm_vcpu *vcpu = &svm->vcpu;
> > +       struct vmcb *vmcb01 = svm->vmcb01.ptr;
> > +       struct vmcb *vmcb02 = svm->nested.vmcb02.ptr;
> > +       struct vmcb *vmcb12;
> > +       struct kvm_host_map map;
> > +       int rc;
> > +
> > +       if (!kvm_vcpu_map(vcpu, gpa_to_gfn(svm->nested.vmcb12_gpa), &map)) {
> > +               vmcb12 = map.hva;
> 
> Maybe also kvm_vcpu_map() mapping call to
> nested_svm_vmexit_update_vmcb12() and inject a tripe fault if it
> fails? Probably plays nicer with "KVM: nSVM: Restrict mapping VMCB12
> on nested VMRUN".

Oh, yeah, good call!  That would be way cleaner (I initially didn't move all
vmcb12 reference, but that's a *really* good argument for doing so).

> Otherwise it looks good to me.
> 
> Should I send a new version to add all the changes?

Yes please.  Thanks!