Re: [PATCH 1/4] KVM: nSVM: Sync next_rip to cached vmcb12 after VMRUN of L2
From: Sean Christopherson
Date: Mon Feb 23 2026 - 12:01:57 EST
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...