Re: [PATCH] KVM: nVMX: Only update last_vpid on a successful nested VM-Enter

[Yosry Ahmed]

> > The reason why I moved the call to nested_vmx_transition_tlb_flush()
> > is that it only makes sense (semantically) to update last_vpid when we
> > will actually use the VPID. Otherwise, if a VM entry fails, the CPU
> > couldn't have cached any translations associated with the new VPID,
> > and a flush is not needed if the VPID is changed again.
> >
> > IOW, the choice was purely based on semantics and code readability.
> >
> > > Because the other way the TLB flushes
> > > can be queued during VM-Enter is via the MSR load lists:
> > >
> > >   If any MSR is being loaded in such a way that would architecturally require
> > >   a TLB flush, the TLBs are updated so that, after VM entry, the logical
> > >   processor will not use any translations that were cached before the transition.
> > >
> > > E.g. if L1 successfully loads one or more MTRRs on VM-Enter to L2[*], then fails
> > > on a subsequent MSR, architecturally I believe L2 TLB entries are guaranteed to
> > > be flushed.
> >
> > Hmm that is an interesting case. I guess the right thing to do here
> > depends on hardware, but yeah I think it makes sense in this case to
> > service local flushes in the failure path. It still annoys me that we
> > would update last_vpid even on failed nested VM entries, so part of me
> > still wants to move the call to nested_vmx_transition_tlb_flush() just
> > for that, but that may not make sense for the VPID disabled case.
>
> Yeah, but I'm not convinced that KVM is actually diverging from hardware.  At
> some point during VM-Enter, hardware needs to "officially" switch to VMX Non-Root
> and start tagging TLB entries with the new VPID.  I can see that being at the
> bitter end, when success is confirmed, but I could also see it happening before
> ucode starts loading guest state into hardware.
>
> Heh, I wonder if we could abuse the MSR load list to deduce when hardware switches
> its TLB tagging to VMX Non-Root and starts using the new VPID.  E.g. maybe put
> DS_AREA and PEBS_ENABLE in the load list, followed by a ton of MSRs to chew up
> CPU cycles, and then reverse engineering how translations to service PEBS buffer
> writes are resolved :-)

Sounds like a very interesting exercise that I do *not* want to do :P