linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Yosry Ahmed <yosry.ahmed@linux.dev>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Jim Mattson <jmattson@google.com>,
	kvm@vger.kernel.org,  linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 11/13] KVM: nSVM: Simplify nested_svm_vmrun()
Date: Fri, 12 Dec 2025 15:23:27 -0800	[thread overview]
Message-ID: <aTyj73ORMhIX-4-c@google.com> (raw)
In-Reply-To: <rckoq7j5pbe7rkszw7d7kkcyjpjpmdwexyrlcw2hyf6cgzpohf@scxmalwv6buz>

On Wed, Dec 10, 2025, Yosry Ahmed wrote:
> On Tue, Dec 09, 2025 at 11:09:26AM -0800, Sean Christopherson wrote:
> > On Tue, Dec 09, 2025, 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:
> > > 
> > > Ack. Any objections to just dropping from_vmrun and moving
> > > kvm_make_request(KVM_REQ_GET_NESTED_STATE_PAGES) to svm_leave_smm()? I
> > > like the consistency of completely relying on from_vmrun or not at all
> > 
> > Zero objections.  When I was initially going through this, I actually thought you
> > were _adding_ the flag and was going to yell at you :-)
> 
> Ugh from_vmrun is also plumbed into nested_svm_load_cr3() as
> reload_pdptrs. Apparently we shouldn't do that in the call path from
> svm_leave_smm()? Anyway, seems like it'll be non-trivial to detangle (at
> least for me, I have 0 understanding of SMM), so I will leave it as-is.

Agreed, there's enough refactoring going on as it is, no need to turn the
snowball into an avalanche.

  reply	other threads:[~2025-12-12 23:23 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-10 22:29 [PATCH v2 00/13] Nested SVM fixes, cleanups, and hardening Yosry Ahmed
2025-11-10 22:29 ` [PATCH v2 01/13] KVM: SVM: Switch svm_copy_lbrs() to a macro Yosry Ahmed
2025-11-10 22:29 ` [PATCH v2 02/13] KVM: SVM: Add missing save/restore handling of LBR MSRs Yosry Ahmed
2025-11-10 22:29 ` [PATCH v2 03/13] KVM: selftests: Add a test for LBR save/restore (ft. nested) Yosry Ahmed
2025-11-10 22:29 ` [PATCH v2 04/13] KVM: nSVM: Fix consistency checks for NP_ENABLE Yosry Ahmed
2025-12-09 16:27   ` Sean Christopherson
2025-12-09 18:07     ` Yosry Ahmed
2025-12-09 18:26       ` Sean Christopherson
2025-12-09 18:35         ` Yosry Ahmed
2025-12-09 18:42           ` Sean Christopherson
2025-12-09 20:02             ` Yosry Ahmed
2025-12-12 18:32               ` Sean Christopherson
2025-12-12 18:38                 ` Yosry Ahmed
2025-12-13  1:07                   ` Sean Christopherson
2025-11-10 22:29 ` [PATCH v2 05/13] KVM: nSVM: Add missing consistency check for EFER, CR0, CR4, and CS Yosry Ahmed
2025-11-10 22:29 ` [PATCH v2 06/13] KVM: nSVM: Add missing consistency check for event_inj Yosry Ahmed
2025-11-10 22:29 ` [PATCH v2 07/13] KVM: SVM: Rename vmcb->nested_ctl to vmcb->misc_ctl Yosry Ahmed
2025-11-10 22:29 ` [PATCH v2 08/13] KVM: SVM: Rename vmcb->virt_ext to vmcb->misc_ctl2 Yosry Ahmed
2025-11-10 22:29 ` [PATCH v2 09/13] KVM: nSVM: Cache all used fields from VMCB12 Yosry Ahmed
2025-11-10 22:29 ` [PATCH v2 10/13] KVM: nSVM: Restrict mapping VMCB12 on nested VMRUN Yosry Ahmed
2025-12-09 16:03   ` Sean Christopherson
2025-12-09 18:24     ` Yosry Ahmed
2025-12-09 18:49       ` Sean Christopherson
2025-12-10 23:05     ` Yosry Ahmed
2025-12-11  0:55       ` Yosry Ahmed
2025-12-12 23:30         ` Sean Christopherson
2025-11-10 22:29 ` [PATCH v2 11/13] KVM: nSVM: Simplify nested_svm_vmrun() Yosry Ahmed
2025-12-09 16:11   ` Sean Christopherson
2025-12-09 18:30     ` Yosry Ahmed
2025-12-09 19:09       ` Sean Christopherson
2025-12-10 16:16         ` Yosry Ahmed
2025-12-12 23:23           ` Sean Christopherson [this message]
2025-12-11 19:25     ` Yosry Ahmed
2025-12-11 20:13       ` Yosry Ahmed
2025-12-13  0:01         ` Sean Christopherson
2025-11-10 22:29 ` [PATCH v2 12/13] KVM: nSVM: Sanitize control fields copied from VMCB12 Yosry Ahmed
2025-12-09 16:19   ` Sean Christopherson
2025-12-09 18:37     ` Yosry Ahmed
2025-11-10 22:29 ` [PATCH v2 13/13] KVM: nSVM: Only copy NP_ENABLE from VMCB01's misc_ctl Yosry Ahmed
2025-12-09 16:23   ` Sean Christopherson
2025-12-09 18:38     ` Yosry Ahmed

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=aTyj73ORMhIX-4-c@google.com \
    --to=seanjc@google.com \
    --cc=jmattson@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=yosry.ahmed@linux.dev \
    /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;
as well as URLs for NNTP newsgroup(s).