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.
