All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <sean.j.christopherson@intel.com>
To: "Paolo Bonzini" <pbonzini@redhat.com>,
	"Radim Krčmář" <rkrcmar@redhat.com>
Cc: kvm@vger.kernel.org, Krish Sadhukhan <krish.sadhukhan@oracle.com>,
	Karl Heubaum <karl.heubaum@oracle.com>,
	Sean Christopherson <sean.j.christopherson@intel.com>,
	Jim Mattson <jmattson@google.com>
Subject: [PATCH v6 5/7] KVM: nVMX: Set VM-{Fail,Exit} failure info via params, not return val
Date: Thu, 11 Apr 2019 12:18:07 -0700	[thread overview]
Message-ID: <20190411191809.8131-6-sean.j.christopherson@intel.com> (raw)
In-Reply-To: <20190411191809.8131-1-sean.j.christopherson@intel.com>

Convert all top-level nested VM-Enter consistency check functions to
use explicit parameters to pass failure information to the caller.
Using an explicit parameter achieves several goals:

  - Provides consistent prototypes for all functions.
  - Self-documents the net effect of failure, e.g. without the explicit
    parameter it may not be obvious that nested_vmx_check_guest_state()
    leads to a VM-Exit.
  - Does not give the false impression that failure information is
    always consumed and/or relevant, e.g. vmx_set_nested_state() only
    cares whether or not the checks were successful.

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
 arch/x86/kvm/vmx/nested.c | 61 ++++++++++++++++++++++-----------------
 1 file changed, 35 insertions(+), 26 deletions(-)

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index b22605d5ee9e..16cff40456ee 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -817,7 +817,8 @@ static int nested_vmx_store_msr_check(struct kvm_vcpu *vcpu,
  * Load guest's/host's msr at nested entry/exit.
  * return 0 for success, entry index for failure.
  */
-static u32 nested_vmx_load_msr(struct kvm_vcpu *vcpu, u64 gpa, u32 count)
+static int nested_vmx_load_msr(struct kvm_vcpu *vcpu, u64 gpa, u32 count,
+			       u32 *exit_reason, u32 *exit_qual)
 {
 	u32 i;
 	struct vmx_msr_entry e;
@@ -849,7 +850,9 @@ static u32 nested_vmx_load_msr(struct kvm_vcpu *vcpu, u64 gpa, u32 count)
 	}
 	return 0;
 fail:
-	return i + 1;
+	*exit_reason = EXIT_REASON_MSR_LOAD_FAIL;
+	*exit_qual = i + 1;
+	return -EINVAL;
 }
 
 static int nested_vmx_store_msr(struct kvm_vcpu *vcpu, u64 gpa, u32 count)
@@ -2574,12 +2577,15 @@ static int nested_check_vm_entry_controls(struct kvm_vcpu *vcpu,
 }
 
 static int nested_vmx_check_controls(struct kvm_vcpu *vcpu,
-				     struct vmcs12 *vmcs12)
+				     struct vmcs12 *vmcs12,
+				     u32 *vm_instruction_error)
 {
+	*vm_instruction_error = VMXERR_ENTRY_INVALID_CONTROL_FIELD;
+
 	if (nested_check_vm_execution_controls(vcpu, vmcs12) ||
 	    nested_check_vm_exit_controls(vcpu, vmcs12) ||
 	    nested_check_vm_entry_controls(vcpu, vmcs12))
-		return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
+		return -EINVAL;
 
 	return 0;
 }
@@ -2624,10 +2630,13 @@ static int nested_check_host_control_regs(struct kvm_vcpu *vcpu,
 }
 
 static int nested_vmx_check_host_state(struct kvm_vcpu *vcpu,
-				       struct vmcs12 *vmcs12)
+				       struct vmcs12 *vmcs12,
+				       u32 *vm_instruction_error)
 {
+	*vm_instruction_error = VMXERR_ENTRY_INVALID_HOST_STATE_FIELD;
+
 	if (nested_check_host_control_regs(vcpu, vmcs12))
-		return VMXERR_ENTRY_INVALID_HOST_STATE_FIELD;
+		return -EINVAL;
 
 	return 0;
 }
@@ -2673,10 +2682,12 @@ static int nested_check_guest_non_reg_state(struct vmcs12 *vmcs12)
 
 static int nested_vmx_check_guest_state(struct kvm_vcpu *vcpu,
 					struct vmcs12 *vmcs12,
+					u32 *exit_reason,
 					u32 *exit_qual)
 {
 	bool ia32e;
 
+	*exit_reason = EXIT_REASON_INVALID_STATE;
 	*exit_qual = ENTRY_FAIL_DEFAULT;
 
 	if (!nested_guest_cr0_valid(vcpu, vmcs12->guest_cr0) ||
@@ -2965,7 +2976,7 @@ int nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu, bool from_vmentry)
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
 	struct vmcs12 *vmcs12 = get_vmcs12(vcpu);
 	bool evaluate_pending_interrupts;
-	u32 exit_reason = EXIT_REASON_INVALID_STATE;
+	u32 exit_reason;
 	u32 exit_qual;
 
 	evaluate_pending_interrupts = vmcs_read32(CPU_BASED_VM_EXEC_CONTROL) &
@@ -2991,7 +3002,8 @@ int nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu, bool from_vmentry)
 			return -1;
 		}
 
-		if (nested_vmx_check_guest_state(vcpu, vmcs12, &exit_qual))
+		if (nested_vmx_check_guest_state(vcpu, vmcs12,
+						 &exit_reason, &exit_qual))
 			goto vmentry_fail_vmexit;
 	}
 
@@ -3003,11 +3015,9 @@ int nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu, bool from_vmentry)
 		goto vmentry_fail_vmexit_guest_mode;
 
 	if (from_vmentry) {
-		exit_reason = EXIT_REASON_MSR_LOAD_FAIL;
-		exit_qual = nested_vmx_load_msr(vcpu,
-						vmcs12->vm_entry_msr_load_addr,
-						vmcs12->vm_entry_msr_load_count);
-		if (exit_qual)
+		if (nested_vmx_load_msr(vcpu, vmcs12->vm_entry_msr_load_addr,
+					vmcs12->vm_entry_msr_load_count,
+					&exit_reason, &exit_qual))
 			goto vmentry_fail_vmexit_guest_mode;
 	} else {
 		/*
@@ -3087,6 +3097,7 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch)
 	struct vmcs12 *vmcs12;
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
 	u32 interrupt_shadow = vmx_get_interrupt_shadow(vcpu);
+	u32 vm_instruction_error;
 	int ret;
 
 	if (!nested_vmx_check_permission(vcpu))
@@ -3136,13 +3147,11 @@ static int nested_vmx_run(struct kvm_vcpu *vcpu, bool launch)
 			launch ? VMXERR_VMLAUNCH_NONCLEAR_VMCS
 			       : VMXERR_VMRESUME_NONLAUNCHED_VMCS);
 
-	ret = nested_vmx_check_controls(vcpu, vmcs12);
-	if (ret)
-		return nested_vmx_failValid(vcpu, ret);
+	if (nested_vmx_check_controls(vcpu, vmcs12, &vm_instruction_error))
+		return nested_vmx_failValid(vcpu, vm_instruction_error);
 
-	ret = nested_vmx_check_host_state(vcpu, vmcs12);
-	if (ret)
-		return nested_vmx_failValid(vcpu, ret);
+	if (nested_vmx_check_host_state(vcpu, vmcs12, &vm_instruction_error))
+		return nested_vmx_failValid(vcpu, vm_instruction_error);
 
 	/*
 	 * We're finally done with prerequisite checking, and can start with
@@ -3594,7 +3603,7 @@ static void load_vmcs12_host_state(struct kvm_vcpu *vcpu,
 				   struct vmcs12 *vmcs12)
 {
 	struct kvm_segment seg;
-	u32 entry_failure_code;
+	u32 ign;
 
 	if (vmcs12->vm_exit_controls & VM_EXIT_LOAD_IA32_EFER)
 		vcpu->arch.efer = vmcs12->host_ia32_efer;
@@ -3629,7 +3638,7 @@ static void load_vmcs12_host_state(struct kvm_vcpu *vcpu,
 	 * Only PDPTE load can fail as the value of cr3 was checked on entry and
 	 * couldn't have changed.
 	 */
-	if (nested_vmx_load_cr3(vcpu, vmcs12->host_cr3, false, &entry_failure_code))
+	if (nested_vmx_load_cr3(vcpu, vmcs12->host_cr3, false, &ign))
 		nested_vmx_abort(vcpu, VMX_ABORT_LOAD_HOST_PDPTE_FAIL);
 
 	if (!enable_ept)
@@ -3727,7 +3736,7 @@ static void load_vmcs12_host_state(struct kvm_vcpu *vcpu,
 		vmx_update_msr_bitmap(vcpu);
 
 	if (nested_vmx_load_msr(vcpu, vmcs12->vm_exit_msr_load_addr,
-				vmcs12->vm_exit_msr_load_count))
+				vmcs12->vm_exit_msr_load_count, &ign, &ign))
 		nested_vmx_abort(vcpu, VMX_ABORT_LOAD_HOST_MSR_FAIL);
 }
 
@@ -5352,7 +5361,7 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu,
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
 	struct vmcs12 *vmcs12;
-	u32 exit_qual;
+	u32 ign;
 	int ret;
 
 	if (kvm_state->format != 0)
@@ -5465,9 +5474,9 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu,
 			return -EINVAL;
 	}
 
-	if (nested_vmx_check_controls(vcpu, vmcs12) ||
-	    nested_vmx_check_host_state(vcpu, vmcs12) ||
-	    nested_vmx_check_guest_state(vcpu, vmcs12, &exit_qual))
+	if (nested_vmx_check_controls(vcpu, vmcs12, &ign) ||
+	    nested_vmx_check_host_state(vcpu, vmcs12, &ign) ||
+	    nested_vmx_check_guest_state(vcpu, vmcs12, &ign, &ign))
 		return -EINVAL;
 
 	vmx->nested.dirty_vmcs12 = true;
-- 
2.21.0


  parent reply	other threads:[~2019-04-11 19:18 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-11 19:18 [PATCH v6 0/7] KVM: nVMX Add IA32_PAT consistency checks Sean Christopherson
2019-04-11 19:18 ` [PATCH v6 1/7] Check "load IA32_PAT" VM-exit control on vmentry Sean Christopherson
2019-04-11 19:18 ` [PATCH v6 2/7] Check "load IA32_PAT" VM-entry " Sean Christopherson
2019-04-11 19:18 ` [PATCH v6 3/7] KVM: nVMX: Move guest non-reg state checks to VM-Exit path Sean Christopherson
2019-04-11 21:00   ` Krish Sadhukhan
2019-04-11 19:18 ` [PATCH v6 4/7] KVM: nVMX: Rename and split top-level consistency checks to match SDM Sean Christopherson
2019-04-11 21:23   ` Krish Sadhukhan
2019-04-11 19:18 ` Sean Christopherson [this message]
2019-04-11 21:56   ` [PATCH v6 5/7] KVM: nVMX: Set VM-{Fail,Exit} failure info via params, not return val Krish Sadhukhan
2019-04-12  8:30   ` Paolo Bonzini
2019-04-12 19:12     ` Sean Christopherson
2019-04-11 19:18 ` [PATCH v6 6/7] KVM: nVMX: Collapse nested_check_host_control_regs() into its caller Sean Christopherson
2019-04-11 22:02   ` Krish Sadhukhan
2019-04-11 19:18 ` [PATCH v6 7/7] KVM: nVMX: Return -EINVAL when signaling failure in VM-Entry helpers Sean Christopherson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190411191809.8131-6-sean.j.christopherson@intel.com \
    --to=sean.j.christopherson@intel.com \
    --cc=jmattson@google.com \
    --cc=karl.heubaum@oracle.com \
    --cc=krish.sadhukhan@oracle.com \
    --cc=kvm@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=rkrcmar@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.