Re: [PATCH 1/4] KVM: nSVM: Sync next_rip to cached vmcb12 after VMRUN of L2

[Yosry Ahmed]

> > > > > 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...

Kinda? What I am getting at is that we'll always have an old KVM that
doesn't save everything that we'll need to handle. I think the
scenario you have in mind is where we introduce a feature *after* we
start saving everything, and at a later point realize we didn't add
proper "restore" support, but the "save" support must have always been
there.

gPAT is not a good example because it's in the "save" area :P

But yeah, I see your point. It's not very straightforward now because
what we save comes from the cache, and we only cache what we need for
the current set of features. So this will need to be done on top of
the rework we've been discussing, where vmcb02 starts being the source
of truth instead of the cache, then we just (mostly) save vmcb02's
control area as-is.