Re: [PATCH v2 11/13] KVM: nSVM: Simplify nested_svm_vmrun()

[Sean Christopherson <seanjc@google.com>]

On Thu, Dec 11, 2025, Yosry Ahmed wrote:
> On Thu, Dec 11, 2025 at 07:25:21PM +0000, Yosry Ahmed wrote:
> > On Tue, Dec 09, 2025 at 08:11:41AM -0800, Sean Christopherson wrote:
> > > On Mon, Nov 10, 2025, Yosry Ahmed wrote:
> > > > Call nested_svm_merge_msrpm() from enter_svm_guest_mode() if called from
> > > > the VMRUN path, instead of making the call in nested_svm_vmrun(). This
> > > > simplifies the flow of nested_svm_vmrun() and removes all jumps to
> > > > cleanup labels.
> > > > 
> > > > Signed-off-by: Yosry Ahmed <yosry.ahmed@linux.dev>
> > > > ---
> > > >  arch/x86/kvm/svm/nested.c | 28 +++++++++++++---------------
> > > >  1 file changed, 13 insertions(+), 15 deletions(-)
> > > > 
> > > > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > > > index a48668c36a191..89830380cebc5 100644
> > > > --- a/arch/x86/kvm/svm/nested.c
> > > > +++ b/arch/x86/kvm/svm/nested.c
> > > > @@ -1020,6 +1020,9 @@ int enter_svm_guest_mode(struct kvm_vcpu *vcpu, u64 vmcb12_gpa, bool from_vmrun)
> > > >  
> > > >  	nested_svm_hv_update_vm_vp_ids(vcpu);
> > > >  
> > > > +	if (from_vmrun && !nested_svm_merge_msrpm(vcpu))
> > > 
> > > This is silly, just do:
> > > 
> > > 	if (enter_svm_guest_mode(vcpu, vmcb12_gpa, true) ||
> > > 	    nested_svm_merge_msrpm(vcpu)) {
> > > 		svm->nested.nested_run_pending = 0;
> > > 		svm->nmi_l1_to_l2 = false;
> > > 		svm->soft_int_injected = false;
> > > 
> > > 		svm->vmcb->control.exit_code    = SVM_EXIT_ERR;
> > > 		svm->vmcb->control.exit_code_hi = -1u;
> > > 		svm->vmcb->control.exit_info_1  = 0;
> > > 		svm->vmcb->control.exit_info_2  = 0;
> > > 
> > > 		nested_svm_vmexit(svm);
> > > 	}
> > 
> > Actually, if we go with the approach of making all VMRUN failures
> > happen before preparing the VMCB02 (as discussed in the other thread),
> > then we will want to call nested_svm_merge_msrpm() from within
> > enter_svm_guest_mode().
> 
> We can also just call nested_svm_merge_msrpm() before
> enter_svm_guest_mode(), which seems to work. 

That's likely unsafe, nested_vmcb_check_controls() checks fields that are consumed
by nested_svm_merge_msrpm().

> > Otherwise, we either have a separate failure path for
> > nested_svm_merge_msrpm(), or we make all VMRUN failures happen after
> > preparing the VMCB02 and handled by nested_svm_vmexit().
> > 
> > I like having a separate exit path for VMRUN failures, and it makes more
> > sense to do the consistency checks on VMCB12 before preparing VMCB02.
> > But I understand if you prefer to keep things simple and move all
> > failures after VMCB02.
> > 
> > I already have it implemented with the separate VMRUN failure path, but
> > I don't wanna spam you with another series if you prefer it the other
> > way.

Spam away.