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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 10 Jul 2017 11:20:45 +0200
From:   David Hildenbrand <david@...hat.com>
To:     Paolo Bonzini <pbonzini@...hat.com>, Bandan Das <bsd@...hat.com>,
        kvm@...r.kernel.org
Cc:     linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/3 v2] KVM: vmx: Enable VMFUNCs

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@...hat.com>

> 
> Paolo


-- 

Thanks,

David