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

[Paolo Bonzini <pbonzini@redhat.com>]

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.

Paolo

>>  	default:
>>  		return true;
>>  	}
>>
> 
> 
> Looks good to me.
>