public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Yosry Ahmed <yosry@kernel.org>
Cc: Yosry Ahmed <yosry.ahmed@linux.dev>,
	Paolo Bonzini <pbonzini@redhat.com>,
	kvm@vger.kernel.org,  linux-kernel@vger.kernel.org,
	stable@vger.kernel.org
Subject: Re: [PATCH 1/4] KVM: nSVM: Sync next_rip to cached vmcb12 after VMRUN of L2
Date: Mon, 23 Feb 2026 08:59:36 -0800	[thread overview]
Message-ID: <aZyHeKp2Dzzrjb5C@google.com> (raw)
In-Reply-To: <ftjb625b4wsz5vdty3fcxqanuxriiqcewqkzp2ml2hc4eojuoc@ewhboiiqmcd4>

On Sat, Feb 21, 2026, Yosry Ahmed wrote:
> [..]
> > > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > > index de90b104a0dd..0a73dd8f9163 100644
> > > --- a/arch/x86/kvm/svm/nested.c
> > > +++ b/arch/x86/kvm/svm/nested.c
> > > @@ -1343,10 +1343,17 @@ static void nested_svm_triple_fault(struct kvm_vcpu *vcpu)
> > >         nested_svm_simple_vmexit(to_svm(vcpu), SVM_EXIT_SHUTDOWN);
> > >  }
> > > 
> > > +static const struct vmcb_ctrl_area_cached *svm_cached_vmcb12_control(struct vcpu_svm *svm) {
> > > +       return &svm->nested.ctl;
> > > +}
> > 
> > ...
> > 
> > > Is this sufficient?
> > 
> > It's certainly better, but unless a sea of helpers is orders of magnitude worse,
> > I would prefer to make it even harder to put hole in our foot.
> > 
> > E.g. unless we're hyper diligent about constifying everything, it's not _that_
> > hard to imagine a chain of events where we end up with a "live" pointer to the
> > cache.
> > 
> >   1. A helper like __nested_vmcb_check_controls() isn't const, so we cast to strip
> >      the const.
> 
> I would argue that casting to strip the const is a red flag and this
> scenario should have stopped here :P

Key word "should" :-)

> > > > Oh, good point.  In that case, I think it makes sense to add the flag asap, so
> > > > that _if_ it turns out that KVM needs to consume a field that isn't currently
> > > > saved/restored, we'll at least have a better story for KVM's that save/restore
> > > > everything.
> > > 
> > > Not sure I follow. Do you mean start serializing everything and setting
> > > the flag ASAP (which IIUC would be after the rework we discussed), 
> > 
> > Yep.
> 
> I don't think it matters that much when we start doing this. In all
> cases:
> 
> 1. KVM will need to be backward-compatible.
> 
> 2. Any new features that depend on save+restore of those fields will be
> a in a new KVM that does the 'full' save+restore (assuming we don't let
> people add per-field flags).
> 
> The only scenario that I can think of is if a feature can be enabled at
> runtime, and we want to be able to enable it for a running VM after
> migrating from an old KVM to a new KVM. Not sure how likely this is.

The scenario I'm thinking of is where we belatedly realize we should have been
saving+restoring a field for a feature that is already supported, e.g. gpat.  If
KVM saves+restores everything, then we don't have to come up with a hacky solution
for older KVM, because it already provides the desired behavior for the "save",
only the "restore" for the older KVM is broken.

Does that make sense?  It makes sense in my head, but I'm not sure I communicated
the idea very well...

  reply	other threads:[~2026-02-23 16:59 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-10  0:54 [PATCH 0/4] KVM: nSVM: Fix save/restore of next_rip & int_state Yosry Ahmed
2026-02-10  0:54 ` [PATCH 1/4] KVM: nSVM: Sync next_rip to cached vmcb12 after VMRUN of L2 Yosry Ahmed
2026-02-10  1:49   ` Sean Christopherson
2026-02-10 16:25     ` Yosry Ahmed
2026-02-10 19:20       ` Sean Christopherson
2026-02-10 22:19         ` Yosry Ahmed
2026-02-11  0:09           ` Sean Christopherson
2026-02-11  0:27             ` Yosry Ahmed
2026-02-11  0:39               ` Sean Christopherson
2026-02-11  1:02                 ` Yosry Ahmed
2026-02-21  0:03                   ` Sean Christopherson
2026-02-21  9:11                     ` Yosry Ahmed
2026-02-23 16:59                       ` Sean Christopherson [this message]
2026-02-23 17:21                         ` Yosry Ahmed
2026-02-23 20:23                           ` Sean Christopherson
2026-02-10  0:54 ` [PATCH 2/4] KVM: nSVM: Sync int_state " Yosry Ahmed
2026-02-10  0:54 ` [PATCH 3/4] KVM: selftests: Extend state_test to check vGIF Yosry Ahmed
2026-02-10  0:54 ` [PATCH 4/4] KVM: selftests: Extend state_test to check next_rip 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=aZyHeKp2Dzzrjb5C@google.com \
    --to=seanjc@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=stable@vger.kernel.org \
    --cc=yosry.ahmed@linux.dev \
    --cc=yosry@kernel.org \
    /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