diff mbox series

[v5,2/7] KVM: nVMX: Move the checks for VM-Execution Control Fields to a separate helper function

Message ID 20181212183012.32356-3-krish.sadhukhan@oracle.com (mailing list archive)
State New, archived
Headers show
Series [v5,1/7] KVM: nVMX: Prepend "nested_vmx_" to check_vmentry_{pre,post}reqs() | expand

Commit Message

Krish Sadhukhan Dec. 12, 2018, 6:30 p.m. UTC 
.. to improve readability and maintainability, and to align the code as per
the layout of the checks in chapter "VM Entries" in Intel SDM vol 3C.

Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
Reviewed-by: Mihai Carabas <mihai.carabas@oracle.com>
Reviewed-by: Mark Kanda <mark.kanda@oracle.com>
---
 arch/x86/kvm/vmx.c | 118 +++++++++++++++++++++++++++--------------------------
 1 file changed, 61 insertions(+), 57 deletions(-)

Comments

kernel test robot Dec. 13, 2018, 1:01 a.m. UTC | #1 
Hi Krish,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on kvm/linux-next]
[also build test ERROR on v4.20-rc6 next-20181212]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Krish-Sadhukhan/KVM-nVMX-Prepend-nested_vmx_-to-check_vmentry_-pre-post-reqs/20181213-081733
base:   https://git.kernel.org/pub/scm/virt/kvm/kvm.git linux-next
config: x86_64-randconfig-x010-201849 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

Note: the linux-review/Krish-Sadhukhan/KVM-nVMX-Prepend-nested_vmx_-to-check_vmentry_-pre-post-reqs/20181213-081733 HEAD f11b35a2245891c27876793e426ba73f5323e80f builds fine.
      It only hurts bisectibility.

All errors (new ones prefixed by >>):

   arch/x86/kvm/vmx.c: In function 'nested_vmx_check_vmentry_prereqs':
>> arch/x86/kvm/vmx.c:13048:5: error: 'vmx' undeclared (first use in this function); did you mean 'vmcs'?
        vmx->nested.msrs.exit_ctls_low,
        ^~~
        vmcs
   arch/x86/kvm/vmx.c:13048:5: note: each undeclared identifier is reported only once for each function it appears in

vim +13048 arch/x86/kvm/vmx.c

 13026	
 13027	static int nested_vmx_check_vmentry_prereqs(struct kvm_vcpu *vcpu,
 13028						    struct vmcs12 *vmcs12)
 13029	{
 13030		bool ia32e;
 13031	
 13032		if (vmcs12->guest_activity_state != GUEST_ACTIVITY_ACTIVE &&
 13033		    vmcs12->guest_activity_state != GUEST_ACTIVITY_HLT)
 13034			return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
 13035	
 13036		if (nested_check_vm_execution_controls(vcpu, vmcs12))
 13037			return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
 13038	
 13039		if (nested_vmx_check_msr_switch_controls(vcpu, vmcs12))
 13040			return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
 13041	
 13042		if (!nested_host_cr0_valid(vcpu, vmcs12->host_cr0) ||
 13043		    !nested_host_cr4_valid(vcpu, vmcs12->host_cr4) ||
 13044		    !nested_cr3_valid(vcpu, vmcs12->host_cr3))
 13045			return VMXERR_ENTRY_INVALID_HOST_STATE_FIELD;
 13046	
 13047		if (!vmx_control_verify(vmcs12->vm_exit_controls,
 13048					vmx->nested.msrs.exit_ctls_low,
 13049					vmx->nested.msrs.exit_ctls_high) ||
 13050		    !vmx_control_verify(vmcs12->vm_entry_controls,
 13051					vmx->nested.msrs.entry_ctls_low,
 13052					vmx->nested.msrs.entry_ctls_high))
 13053			return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
 13054	
 13055		/*
 13056		 * If the load IA32_EFER VM-exit control is 1, bits reserved in the
 13057		 * IA32_EFER MSR must be 0 in the field for that register. In addition,
 13058		 * the values of the LMA and LME bits in the field must each be that of
 13059		 * the host address-space size VM-exit control.
 13060		 */
 13061		if (vmcs12->vm_exit_controls & VM_EXIT_LOAD_IA32_EFER) {
 13062			ia32e = (vmcs12->vm_exit_controls &
 13063				 VM_EXIT_HOST_ADDR_SPACE_SIZE) != 0;
 13064			if (!kvm_valid_efer(vcpu, vmcs12->host_ia32_efer) ||
 13065			    ia32e != !!(vmcs12->host_ia32_efer & EFER_LMA) ||
 13066			    ia32e != !!(vmcs12->host_ia32_efer & EFER_LME))
 13067				return VMXERR_ENTRY_INVALID_HOST_STATE_FIELD;
 13068		}
 13069	
 13070		/*
 13071		 * From the Intel SDM, volume 3:
 13072		 * Fields relevant to VM-entry event injection must be set properly.
 13073		 * These fields are the VM-entry interruption-information field, the
 13074		 * VM-entry exception error code, and the VM-entry instruction length.
 13075		 */
 13076		if (vmcs12->vm_entry_intr_info_field & INTR_INFO_VALID_MASK) {
 13077			u32 intr_info = vmcs12->vm_entry_intr_info_field;
 13078			u8 vector = intr_info & INTR_INFO_VECTOR_MASK;
 13079			u32 intr_type = intr_info & INTR_INFO_INTR_TYPE_MASK;
 13080			bool has_error_code = intr_info & INTR_INFO_DELIVER_CODE_MASK;
 13081			bool should_have_error_code;
 13082			bool urg = nested_cpu_has2(vmcs12,
 13083						   SECONDARY_EXEC_UNRESTRICTED_GUEST);
 13084			bool prot_mode = !urg || vmcs12->guest_cr0 & X86_CR0_PE;
 13085	
 13086			/* VM-entry interruption-info field: interruption type */
 13087			if (intr_type == INTR_TYPE_RESERVED ||
 13088			    (intr_type == INTR_TYPE_OTHER_EVENT &&
 13089			     !nested_cpu_supports_monitor_trap_flag(vcpu)))
 13090				return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
 13091	
 13092			/* VM-entry interruption-info field: vector */
 13093			if ((intr_type == INTR_TYPE_NMI_INTR && vector != NMI_VECTOR) ||
 13094			    (intr_type == INTR_TYPE_HARD_EXCEPTION && vector > 31) ||
 13095			    (intr_type == INTR_TYPE_OTHER_EVENT && vector != 0))
 13096				return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
 13097	
 13098			/* VM-entry interruption-info field: deliver error code */
 13099			should_have_error_code =
 13100				intr_type == INTR_TYPE_HARD_EXCEPTION && prot_mode &&
 13101				x86_exception_has_error_code(vector);
 13102			if (has_error_code != should_have_error_code)
 13103				return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
 13104	
 13105			/* VM-entry exception error code */
 13106			if (has_error_code &&
 13107			    vmcs12->vm_entry_exception_error_code & GENMASK(31, 15))
 13108				return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
 13109	
 13110			/* VM-entry interruption-info field: reserved bits */
 13111			if (intr_info & INTR_INFO_RESVD_BITS_MASK)
 13112				return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
 13113	
 13114			/* VM-entry instruction length */
 13115			switch (intr_type) {
 13116			case INTR_TYPE_SOFT_EXCEPTION:
 13117			case INTR_TYPE_SOFT_INTR:
 13118			case INTR_TYPE_PRIV_SW_EXCEPTION:
 13119				if ((vmcs12->vm_entry_instruction_len > 15) ||
 13120				    (vmcs12->vm_entry_instruction_len == 0 &&
 13121				     !nested_cpu_has_zero_length_injection(vcpu)))
 13122					return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
 13123			}
 13124		}
 13125	
 13126		return 0;
 13127	}
 13128	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox series

Patch

diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index 6ae408e..aedb5f6 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -13003,77 +13003,76 @@  static int nested_vmx_check_nmi_controls(struct vmcs12 *vmcs12)
 	return 0;
 }
 
-static int nested_vmx_check_vmentry_prereqs(struct kvm_vcpu *vcpu,
-					struct vmcs12 *vmcs12)
+/*
+ * Checks related to VM-Execution Control Fields
+ */
+static int nested_check_vm_execution_controls(struct kvm_vcpu *vcpu,
+                                              struct vmcs12 *vmcs12)
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
-	bool ia32e;
-
-	if (vmcs12->guest_activity_state != GUEST_ACTIVITY_ACTIVE &&
-	    vmcs12->guest_activity_state != GUEST_ACTIVITY_HLT)
-		return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
-
-	if (nested_cpu_has_vpid(vmcs12) && !vmcs12->virtual_processor_id)
-		return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
 
-	if (nested_vmx_check_io_bitmap_controls(vcpu, vmcs12))
-		return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
-
-	if (nested_vmx_check_msr_bitmap_controls(vcpu, vmcs12))
-		return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
-
-	if (nested_vmx_check_apic_access_controls(vcpu, vmcs12))
-		return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
+	if (!vmx_control_verify(vmcs12->pin_based_vm_exec_control,
+				vmx->nested.msrs.pinbased_ctls_low,
+				vmx->nested.msrs.pinbased_ctls_high) ||
+	    !vmx_control_verify(vmcs12->cpu_based_vm_exec_control,
+				vmx->nested.msrs.procbased_ctls_low,
+				vmx->nested.msrs.procbased_ctls_high))
+		return -EINVAL;
 
-	if (nested_vmx_check_tpr_shadow_controls(vcpu, vmcs12))
-		return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
+	if (nested_cpu_has(vmcs12, CPU_BASED_ACTIVATE_SECONDARY_CONTROLS) &&
+	    !vmx_control_verify(vmcs12->secondary_vm_exec_control,
+				 vmx->nested.msrs.secondary_ctls_low,
+				 vmx->nested.msrs.secondary_ctls_high))
+		return -EINVAL;
 
-	if (nested_vmx_check_apicv_controls(vcpu, vmcs12))
-		return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
+	if (vmcs12->cr3_target_count > nested_cpu_vmx_misc_cr3_count(vcpu) ||
+	    nested_vmx_check_io_bitmap_controls(vcpu, vmcs12) ||
+	    nested_vmx_check_msr_bitmap_controls(vcpu, vmcs12) ||
+	    nested_vmx_check_tpr_shadow_controls(vcpu, vmcs12) ||
+	    nested_vmx_check_apic_access_controls(vcpu, vmcs12) ||
+	    nested_vmx_check_apicv_controls(vcpu, vmcs12) ||
+	    nested_vmx_check_nmi_controls(vmcs12) ||
+	    (nested_cpu_has_vpid(vmcs12) && !vmcs12->virtual_processor_id))
+		return -EINVAL;
 
-	if (nested_vmx_check_msr_switch_controls(vcpu, vmcs12))
-		return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
+	if (nested_cpu_has_ept(vmcs12) &&
+	    !valid_ept_address(vcpu, vmcs12->ept_pointer))
+		return -EINVAL;
 
 	if (nested_vmx_check_pml_controls(vcpu, vmcs12))
-		return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
-
-	if (nested_vmx_check_shadow_vmcs_controls(vcpu, vmcs12))
-		return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
-
-	if (!vmx_control_verify(vmcs12->cpu_based_vm_exec_control,
-				vmx->nested.msrs.procbased_ctls_low,
-				vmx->nested.msrs.procbased_ctls_high) ||
-	    (nested_cpu_has(vmcs12, CPU_BASED_ACTIVATE_SECONDARY_CONTROLS) &&
-	     !vmx_control_verify(vmcs12->secondary_vm_exec_control,
-				 vmx->nested.msrs.secondary_ctls_low,
-				 vmx->nested.msrs.secondary_ctls_high)) ||
-	    !vmx_control_verify(vmcs12->pin_based_vm_exec_control,
-				vmx->nested.msrs.pinbased_ctls_low,
-				vmx->nested.msrs.pinbased_ctls_high) ||
-	    !vmx_control_verify(vmcs12->vm_exit_controls,
-				vmx->nested.msrs.exit_ctls_low,
-				vmx->nested.msrs.exit_ctls_high) ||
-	    !vmx_control_verify(vmcs12->vm_entry_controls,
-				vmx->nested.msrs.entry_ctls_low,
-				vmx->nested.msrs.entry_ctls_high))
-		return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
-
-	if (nested_vmx_check_nmi_controls(vmcs12))
-		return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
+		return -EINVAL;
 
 	if (nested_cpu_has_vmfunc(vmcs12)) {
 		if (vmcs12->vm_function_control &
 		    ~vmx->nested.msrs.vmfunc_controls)
-			return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
+			return -EINVAL;
 
 		if (nested_cpu_has_eptp_switching(vmcs12)) {
 			if (!nested_cpu_has_ept(vmcs12) ||
 			    !page_address_valid(vcpu, vmcs12->eptp_list_address))
-				return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
+				return -EINVAL;
 		}
 	}
 
-	if (vmcs12->cr3_target_count > nested_cpu_vmx_misc_cr3_count(vcpu))
+	if (nested_vmx_check_shadow_vmcs_controls(vcpu, vmcs12))
+		return -EINVAL;
+
+	return 0;
+}
+
+static int nested_vmx_check_vmentry_prereqs(struct kvm_vcpu *vcpu,
+					    struct vmcs12 *vmcs12)
+{
+	bool ia32e;
+
+	if (vmcs12->guest_activity_state != GUEST_ACTIVITY_ACTIVE &&
+	    vmcs12->guest_activity_state != GUEST_ACTIVITY_HLT)
+		return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
+
+	if (nested_check_vm_execution_controls(vcpu, vmcs12))
+		return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
+
+	if (nested_vmx_check_msr_switch_controls(vcpu, vmcs12))
 		return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
 
 	if (!nested_host_cr0_valid(vcpu, vmcs12->host_cr0) ||
@@ -13081,6 +13080,14 @@  static int nested_vmx_check_vmentry_prereqs(struct kvm_vcpu *vcpu,
 	    !nested_cr3_valid(vcpu, vmcs12->host_cr3))
 		return VMXERR_ENTRY_INVALID_HOST_STATE_FIELD;
 
+	if (!vmx_control_verify(vmcs12->vm_exit_controls,
+				vmx->nested.msrs.exit_ctls_low,
+				vmx->nested.msrs.exit_ctls_high) ||
+	    !vmx_control_verify(vmcs12->vm_entry_controls,
+				vmx->nested.msrs.entry_ctls_low,
+				vmx->nested.msrs.entry_ctls_high))
+		return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
+
 	/*
 	 * If the load IA32_EFER VM-exit control is 1, bits reserved in the
 	 * IA32_EFER MSR must be 0 in the field for that register. In addition,
@@ -13152,10 +13159,6 @@  static int nested_vmx_check_vmentry_prereqs(struct kvm_vcpu *vcpu,
 		}
 	}
 
-	if (nested_cpu_has_ept(vmcs12) &&
-	    !valid_ept_address(vcpu, vmcs12->ept_pointer))
-		return VMXERR_ENTRY_INVALID_CONTROL_FIELD;
-
 	return 0;
 }
 
@@ -13187,7 +13190,8 @@  static int nested_vmx_check_vmcs_link_ptr(struct kvm_vcpu *vcpu,
 }
 
 static int nested_vmx_check_vmentry_postreqs(struct kvm_vcpu *vcpu,
-					 struct vmcs12 *vmcs12, u32 *exit_qual)
+					     struct vmcs12 *vmcs12,
+					     u32 *exit_qual)
 {
 	bool ia32e;