[2/5] KVM: nVMX: Skip VM-Exit Control vmentry checks that are necessary only if VMCS12 is dirty
diff mbox series

Message ID 20190707071147.11651-3-krish.sadhukhan@oracle.com
State New
Headers show
Series
  • KVM: nVMX: Skip vmentry checks that are necessary only if VMCS12 is dirty
Related show

Commit Message

Krish Sadhukhan July 7, 2019, 7:11 a.m. UTC
..so that every nested vmentry is not slowed down by those checks.

Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
---
 arch/x86/kvm/vmx/nested.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

Comments

Paolo Bonzini July 10, 2019, 2:28 p.m. UTC | #1
On 07/07/19 09:11, Krish Sadhukhan wrote:
>  
>  	if (!vmx_control_verify(vmcs12->vm_exit_controls,
>  				vmx->nested.msrs.exit_ctls_low,
> -				vmx->nested.msrs.exit_ctls_high) ||
> -	    nested_vmx_check_exit_msr_switch_controls(vcpu, vmcs12))
> +				vmx->nested.msrs.exit_ctls_high))
> +		return -EINVAL;
> +

Exit controls are not shadowed, are they?

Paolo
Krish Sadhukhan July 10, 2019, 7:18 p.m. UTC | #2
On 7/10/19 7:28 AM, Paolo Bonzini wrote:
> On 07/07/19 09:11, Krish Sadhukhan wrote:
>>   
>>   	if (!vmx_control_verify(vmcs12->vm_exit_controls,
>>   				vmx->nested.msrs.exit_ctls_low,
>> -				vmx->nested.msrs.exit_ctls_high) ||
>> -	    nested_vmx_check_exit_msr_switch_controls(vcpu, vmcs12))
>> +				vmx->nested.msrs.exit_ctls_high))
>> +		return -EINVAL;
>> +
> Exit controls are not shadowed, are they?

No, they aren't.   However, I see that prepare_vmcs02_constant_state() 
which is called in the path of prepare_vmcs02_early_full() writes those 
Exit Control fields:

        vmcs_write32(VM_EXIT_MSR_STORE_COUNT, 0);
         vmcs_write64(VM_EXIT_MSR_LOAD_ADDR, 
__pa(vmx->msr_autoload.host.val));
         vmcs_write64(VM_ENTRY_MSR_LOAD_ADDR, 
__pa(vmx->msr_autoload.guest.val));


Should we add these fields to the shadow list then ?

>
> Paolo
Sean Christopherson July 10, 2019, 8:11 p.m. UTC | #3
On Wed, Jul 10, 2019 at 12:18:55PM -0700, Krish Sadhukhan wrote:
> 
> On 7/10/19 7:28 AM, Paolo Bonzini wrote:
> >On 07/07/19 09:11, Krish Sadhukhan wrote:
> >>  	if (!vmx_control_verify(vmcs12->vm_exit_controls,
> >>  				vmx->nested.msrs.exit_ctls_low,
> >>-				vmx->nested.msrs.exit_ctls_high) ||
> >>-	    nested_vmx_check_exit_msr_switch_controls(vcpu, vmcs12))
> >>+				vmx->nested.msrs.exit_ctls_high))
> >>+		return -EINVAL;
> >>+
> >Exit controls are not shadowed, are they?
> 
> No, they aren't.   However, I see that prepare_vmcs02_constant_state() which
> is called in the path of prepare_vmcs02_early_full() writes those Exit
> Control fields:
> 
>        vmcs_write32(VM_EXIT_MSR_STORE_COUNT, 0);
>         vmcs_write64(VM_EXIT_MSR_LOAD_ADDR,
> __pa(vmx->msr_autoload.host.val));
>         vmcs_write64(VM_ENTRY_MSR_LOAD_ADDR,
> __pa(vmx->msr_autoload.guest.val));

That's writing L0's values into vmcs02 when vmcs02 is first used.  L1's
MSR load lists are processed purely in software, e.g. nested_vmx_load_msr().
The vmcs12 entries are consumed by KVM on every nested transition to
emulate the load/store functionality, but the validity of the *controls*
only needs to be checked when vmcs12 is dirty.

> 
> 
> Should we add these fields to the shadow list then ?
> 
> >
> >Paolo

Patch
diff mbox series

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index b0b59c78b3e8..056eba497730 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -2505,6 +2505,15 @@  static int nested_check_vm_execution_controls(struct kvm_vcpu *vcpu,
 	return 0;
 }
 
+static int nested_check_vm_exit_controls_full(struct kvm_vcpu *vcpu,
+					      struct vmcs12 *vmcs12)
+{
+	if (nested_vmx_check_exit_msr_switch_controls(vcpu, vmcs12))
+		return -EINVAL;
+
+	return 0;
+}
+
 /*
  * Checks related to VM-Exit Control Fields
  */
@@ -2515,8 +2524,11 @@  static int nested_check_vm_exit_controls(struct kvm_vcpu *vcpu,
 
 	if (!vmx_control_verify(vmcs12->vm_exit_controls,
 				vmx->nested.msrs.exit_ctls_low,
-				vmx->nested.msrs.exit_ctls_high) ||
-	    nested_vmx_check_exit_msr_switch_controls(vcpu, vmcs12))
+				vmx->nested.msrs.exit_ctls_high))
+		return -EINVAL;
+
+	if ((vmx->nested.dirty_vmcs12) &&
+	    nested_check_vm_exit_controls_full(vcpu, vmcs12))
 		return -EINVAL;
 
 	return 0;