Re: [PATCH 1/3 v2] KVM: vmx: Enable VMFUNCs

[David Hildenbrand <david@redhat.com>]

On 10.07.2017 11:17, Paolo Bonzini wrote:
> 
> 
> On 10/07/2017 10:54, David Hildenbrand wrote:
>>
>>>  /*
>>>   * The exit handlers return 1 if the exit was handled fully and guest execution
>>>   * may resume.  Otherwise they set the kvm_run parameter to indicate what needs
>>> @@ -7790,6 +7806,7 @@ static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = {
>>>  	[EXIT_REASON_XSAVES]                  = handle_xsaves,
>>>  	[EXIT_REASON_XRSTORS]                 = handle_xrstors,
>>>  	[EXIT_REASON_PML_FULL]		      = handle_pml_full,
>>> +	[EXIT_REASON_VMFUNC]                  = handle_vmfunc,
>>>  	[EXIT_REASON_PREEMPTION_TIMER]	      = handle_preemption_timer,
>>>  };
>>>  
>>> @@ -8111,6 +8128,9 @@ static bool nested_vmx_exit_handled(struct kvm_vcpu *vcpu)
>>>  	case EXIT_REASON_PML_FULL:
>>>  		/* We emulate PML support to L1. */
>>>  		return false;
>>> +	case EXIT_REASON_VMFUNC:
>>> +		/* VM functions are emulated through L2->L0 vmexits. */
>>> +		return false;
>>
>> This would fit better into the second patch.
> 
> It depends on how you reason about it.  I put it here because:
> 
> - until this patch, EXIT_REASON_VMFUNC should never be generated.  We
> don't even know that it exists.
> 
> - after this patch, it should still never be generated in nested
> scenarios.  However, if it did because of a bug, we're in a better place
> to handle it than L1 (because as far as L1 knows, it should never be
> generated).
> 
> Perhaps this is an argument in favor of changing the default case of
> nested_vmx_exit_handled from true to false.

I remember having the same discussion before :) I still think the
default should be changed (then we don't need nVMX hunks in VMX patches
;) ).

Anyhow

Reviewed-by: David Hildenbrand <david@redhat.com>

> 
> Paolo


-- 

Thanks,

David