Patchwork [3/4,v2,kvm-unit-test,nVMX] : Change 'test_vmx_controls' to 'test_vmlaunch' and add a parameter for expected error

login
register
mail settings
Submitter Christopherson, Sean J
Date Feb. 5, 2019, 11:30 p.m.
Message ID <20190205233040.GD25434@linux.intel.com>
Download mbox | patch
Permalink /patch/719123/
State New
Headers show

Comments

Christopherson, Sean J - Feb. 5, 2019, 11:30 p.m.
On Tue, Feb 05, 2019 at 05:34:26PM -0500, Krish Sadhukhan wrote:
> The function 'test_vmx_controls' has thus far been used to test the VMX
> controls and has the expected error code (returned by KVM) hard-coded in it.
> When vmlaunch fails due to a bad Host State Area, the error code returned by
> KVM is different. So rename this function and add a new parameter so that it
> can also be used for testing the Host State Area during vmlaunch of L2 guests.
> 
> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
> ---
>  x86/vmx_tests.c | 164 +++++++++++++++++++++++++-----------------------
>  1 file changed, 86 insertions(+), 78 deletions(-)
> 
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index 9a3cdee..b69a7d9 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -3285,7 +3285,7 @@ success:
>  /*
>   * Try to launch the current VMCS.
>   */
> -static void test_vmx_controls(bool controls_valid, bool xfail)
> +static void test_vmlaunch(bool controls_valid, bool xfail, u32 error_expected)
>  {
>  	bool success = vmlaunch_succeeds();
>  	u32 vmx_inst_err;
> @@ -3295,8 +3295,8 @@ static void test_vmx_controls(bool controls_valid, bool xfail)
>  	if (!success) {
>  		vmx_inst_err = vmcs_read(VMX_INST_ERROR);
>  		report("VMX inst error is %d (actual %d)",
> -		       vmx_inst_err == VMXERR_ENTRY_INVALID_CONTROL_FIELD,
> -		       VMXERR_ENTRY_INVALID_CONTROL_FIELD, vmx_inst_err);
> +		       vmx_inst_err == error_expected, error_expected,
> +		       vmx_inst_err);
>  	}

That's a lot of call sites to change just to reuse a few lines of code.
And the xfail stuff isn't needed for the new host state checks.  What
about moving the VMX_INST_ERROR logic to a helper and reusing that?

From 01c9fe2e0d4e0b4593334396a0a880f5adfd3fb5 Mon Sep 17 00:00:00 2001
From: Sean Christopherson <sean.j.christopherson@intel.com>
Date: Tue, 5 Feb 2019 15:25:24 -0800
Subject: [PATCH] KVM: nVMX: Add helper to check VMX_INST_ERROR after failed
 vmlaunch

...so that the code can be reused for failure due to reasons other than
invalid controls, e.g. invalid host state.

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
 x86/vmx_tests.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

Patch

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 28bab81..97c2163 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -3285,22 +3285,26 @@  success:
 	return true;
 }
 
+static void check_vmx_instr_err(u32 expected)
+{
+	u32 vmx_inst_err;
+
+	vmx_inst_err = vmcs_read(VMX_INST_ERROR);
+	report("VMX inst error is %d (actual %d)",
+	       vmx_inst_err == expected, expected, vmx_inst_err);
+}
+
 /*
  * Try to launch the current VMCS.
  */
 static void test_vmx_controls(bool controls_valid, bool xfail)
 {
 	bool success = vmlaunch_succeeds();
-	u32 vmx_inst_err;
 
 	report_xfail("vmlaunch %s", xfail, success == controls_valid,
 		     controls_valid ? "succeeds" : "fails");
-	if (!success) {
-		vmx_inst_err = vmcs_read(VMX_INST_ERROR);
-		report("VMX inst error is %d (actual %d)",
-		       vmx_inst_err == VMXERR_ENTRY_INVALID_CONTROL_FIELD,
-		       VMXERR_ENTRY_INVALID_CONTROL_FIELD, vmx_inst_err);
-	}
+	if (!success)
+		check_vmx_instr_err(VMXERR_ENTRY_INVALID_CONTROL_FIELD);
 }
 
 /*