Re: [PATCH 06/15] KVM: nVMX: Don't "put" vCPU or host state when switching VMCS

Sean Christopherson
2019-06-07T17:08:35Z

On Fri, Jun 07, 2019 at 07:00:06PM +0200, Paolo Bonzini wrote:
> On 06/06/19 20:57, Sean Christopherson wrote:
> > What about taking the vmcs pointers, and using old/new instead of
> > prev/cur? Calling it prev is wonky since it's pulled from the current
> > value of loaded_cpu_state, especially since cur is the same type.
> > That oddity is also why I grabbed prev before setting loaded_vmcs,
> > it just felt wrong even though they really are two separate things.
> >
> > static void vmx_sync_vmcs_host_state(struct vcpu_vmx *vmx,
> > struct loaded_vmcs *old,
> > struct loaded_vmcs *new)
>
> I had it like that in the beginning actually. But the idea of this
> function is that because we're switching vmcs's, the host register
> fields have to be moved to the VMCS that will be used next. I don't see
> how it would be used with old and new being anything other than
> vmx->loaded_cpu_state and vmx->loaded_vmcs and, because we're switching
> VMCS, those are the "previously" active VMCS and the "currently" active
> VMCS.
>
> What would also make sense, is to change loaded_cpu_state to a bool (it
> must always be equal to loaded_vmcs anyway) and make the prototype
> something like this:
>
> static void vmx_sync_vmcs_host_state(struct vcpu_vmx *vmx,
> struct loaded_vmcs *prev)
>
>
> I'll send a patch.

Works for me. The only reason I made loaded_cpu_state was so that
vmx_prepare_switch_to_host() could WARN on it diverging from loaded_vmcs.
Seeing as how that WARN has never fired, I'm comfortable making it a bool.