Re: [PATCH v2 6/6] KVM: nVMX: optimize prepare_vmcs02{,_full} for Enlightened VMCS case

[Vitaly Kuznetsov <vkuznets@redhat.com>]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 25/07/2018 10:37, Vitaly Kuznetsov wrote:
>>> Why is this needed?  If it weren't for it, you could pass hv_evmcs
>>> directly to evmcs_needs_write, which would simplify the code a bit in
>>> the caller.
>> This is an equivalent of prepare_vmcs02()/prepare_vmcs02_full() split
>> for eVMCS case: when we switch from L2 guest A to L2 guest B we need to
>> write the whole VMCS so evmcs_needs_write() needs to return true.
>
> Right, I missed the dirty_vmcs12 assignment in patch 5.
>
> But is L0 allowed to write to hv_clean_fields?  

It is kinda expected to: currently I reset it in vmx_vcpu_run() and (if
I remember correctly) L1 Hyper-V only clears bits in this mask when it
touches certain fields so if we don't set it to 'all clean' it stays
zeroed forever. So nothing stops us from doing 

       if (hv_evmcs && vmx->nested.dirty_vmcs12)
               hv_evmcs->hv_clean_fields &=
                       ~HV_VMX_ENLIGHTENED_CLEAN_FIELD_ALL;

in prepare_vmcs02() I guess.

> One possibility is to
> add a dirty_evmcs field to struct nested_vmx, and "OR" ~hv_clean_fields
> into it at the beginning of prepare_vmcs02.
>
> Something like
>
> 	if (vmx->nested.hv_evmcs) {
> 		vmx->nested.dirty_evmcs |=
> 			~vmx->nested.hv_evmcs->hv_clean_fields;
> 		prepare_vmcs02_full(vcpu, vmcs12,
> 				    vmx->nested.dirty_evmcs);
> 	} else if (vmx->nested.dirty_vmcs12) {
> 		prepare_vmcs02_full(vcpu, vmcs12, ~0);
> 	}
>
> 	...
> 	vmx->nested.dirty_evmcs = 0;
> 	vmx->nested.dirty_vmcs12 = false;
>
> ?
>

I think we can even get away with a local variable in prepare_vmcs02()
and pass it to prepare_vmcs02_full(), no need to have it in struct
nested_vmx. But I would slightly prefer to just reset
hv_evmcs->hv_clean_fields when vmcs12 is dirty.

Thanks,

-- 
  Vitaly