[3/4,v3,nVMX] : Move the checks for Guest Control Registers and Guest MSRs to a separate function
diff mbox series

Message ID 20190403215905.2474-4-krish.sadhukhan@oracle.com
State New
Headers show
Series
  • [1/4,v3,nVMX] : Check "load IA32_PAT" VM-exit control on vmentry
Related show

Commit Message

Krish Sadhukhan April 3, 2019, 9:59 p.m. UTC
..in order to match the organization of the checks in Intel SDM vol 3C.

Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
Suggested-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
 arch/x86/kvm/vmx/nested.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

Comments

Sean Christopherson April 4, 2019, 4:08 p.m. UTC | #1
On Wed, Apr 03, 2019 at 05:59:04PM -0400, Krish Sadhukhan wrote:
>  ..in order to match the organization of the checks in Intel SDM vol 3C.
> 
> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
> Suggested-by: Sean Christopherson <sean.j.christopherson@intel.com>
> ---
>  arch/x86/kvm/vmx/nested.c | 25 +++++++++++++++++++------
>  1 file changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index b8bd449350b4..8347e9066e26 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -2674,6 +2674,24 @@ static int nested_vmx_check_vmcs_link_ptr(struct kvm_vcpu *vcpu,
>  	return r;
>  }
>  
> +/*
> + * Checks related to Control Registers, Debug Registers and MSRs in
> + * Guest State Area.
> + */
> +static int nested_check_guest_cregs_dregs_msrs(struct kvm_vcpu *vcpu,
> +					       struct vmcs12 *vmcs12)
> +{
> +	if (!nested_guest_cr0_valid(vcpu, vmcs12->guest_cr0) ||
> +	    !nested_guest_cr4_valid(vcpu, vmcs12->guest_cr4))
> +		return 1;
> +
> +	if ((vmcs12->vm_entry_controls & VM_ENTRY_LOAD_IA32_PAT) &&
> +	    !kvm_mtrr_valid(vcpu, MSR_IA32_CR_PAT, vmcs12->guest_ia32_pat))
> +		return 1;

It probably makes sense to change these to "return -EINVAL" to match
the other nested_check...() helpers.  And it'd be super nice if
nested_vmx_check_vmentry_postreqs() were modified to "return
VMX_EXIT_REASONS_FAILED_VMENTRY | EXIT_REASON_INVALID_STATE;" to
align with nested_vmx_check_vmentry_prereqs().  Even though the caller
only cares about pass vs. fail, returning the failure reason documents
the architectural effect of failing the checks.

The above can be done in follow-up patch if you spin v4 of this series.
I'd also be happy to submit a patch sometime in the future.

For this patch:

Reviewed-by: Sean Christopherson <sean.j.christopherson@intel.com>

> +
> +	return 0;
> +}
> +
>  static int nested_vmx_check_vmentry_postreqs(struct kvm_vcpu *vcpu,
>  					     struct vmcs12 *vmcs12,
>  					     u32 *exit_qual)
> @@ -2682,12 +2700,7 @@ static int nested_vmx_check_vmentry_postreqs(struct kvm_vcpu *vcpu,
>  
>  	*exit_qual = ENTRY_FAIL_DEFAULT;
>  
> -	if (!nested_guest_cr0_valid(vcpu, vmcs12->guest_cr0) ||
> -	    !nested_guest_cr4_valid(vcpu, vmcs12->guest_cr4))
> -		return 1;
> -
> -	if ((vmcs12->vm_entry_controls & VM_ENTRY_LOAD_IA32_PAT) &&
> -	    !kvm_mtrr_valid(vcpu, MSR_IA32_CR_PAT, vmcs12->guest_ia32_pat))
> +	if (nested_check_guest_cregs_dregs_msrs(vcpu, vmcs12))
>  		return 1;
>  
>  	if (nested_vmx_check_vmcs_link_ptr(vcpu, vmcs12)) {
> -- 
> 2.17.2
>

Patch
diff mbox series

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index b8bd449350b4..8347e9066e26 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -2674,6 +2674,24 @@  static int nested_vmx_check_vmcs_link_ptr(struct kvm_vcpu *vcpu,
 	return r;
 }
 
+/*
+ * Checks related to Control Registers, Debug Registers and MSRs in
+ * Guest State Area.
+ */
+static int nested_check_guest_cregs_dregs_msrs(struct kvm_vcpu *vcpu,
+					       struct vmcs12 *vmcs12)
+{
+	if (!nested_guest_cr0_valid(vcpu, vmcs12->guest_cr0) ||
+	    !nested_guest_cr4_valid(vcpu, vmcs12->guest_cr4))
+		return 1;
+
+	if ((vmcs12->vm_entry_controls & VM_ENTRY_LOAD_IA32_PAT) &&
+	    !kvm_mtrr_valid(vcpu, MSR_IA32_CR_PAT, vmcs12->guest_ia32_pat))
+		return 1;
+
+	return 0;
+}
+
 static int nested_vmx_check_vmentry_postreqs(struct kvm_vcpu *vcpu,
 					     struct vmcs12 *vmcs12,
 					     u32 *exit_qual)
@@ -2682,12 +2700,7 @@  static int nested_vmx_check_vmentry_postreqs(struct kvm_vcpu *vcpu,
 
 	*exit_qual = ENTRY_FAIL_DEFAULT;
 
-	if (!nested_guest_cr0_valid(vcpu, vmcs12->guest_cr0) ||
-	    !nested_guest_cr4_valid(vcpu, vmcs12->guest_cr4))
-		return 1;
-
-	if ((vmcs12->vm_entry_controls & VM_ENTRY_LOAD_IA32_PAT) &&
-	    !kvm_mtrr_valid(vcpu, MSR_IA32_CR_PAT, vmcs12->guest_ia32_pat))
+	if (nested_check_guest_cregs_dregs_msrs(vcpu, vmcs12))
 		return 1;
 
 	if (nested_vmx_check_vmcs_link_ptr(vcpu, vmcs12)) {