[PATCH kvm-unit-tests 0/8]: x86: vmx: Test INIT processing in various CPU VMX states

[PATCH kvm-unit-tests 0/8]: x86: vmx: Test INIT processing in various CPU VMX states

[Liran Alon]

Hi,

This patch series aims to add a vmx test to verify the functionality
introduced by KVM commit:
4b9852f4f389 ("KVM: x86: Fix INIT signal handling in various CPU states")

The test verifies the following functionality:
1) An INIT signal received when CPU is in VMX operation
  is latched until it exits VMX operation.
2) If there is an INIT signal pending when CPU is in
  VMX non-root mode, it result in VMExit with (reason == 3).
3) Exit from VMX non-root mode on VMExit do not clear
  pending INIT signal in LAPIC.
4) When CPU exits VMX operation, pending INIT signal in
  LAPIC is processed.

In order to write such a complex test, the vmx tests framework was
enhanced to support using VMX in non BSP CPUs. This enhancement is
implemented in patches 1-7. The test itself is implemented at patch 8.
This enhancement to the vmx tests framework is a bit hackish, but
I believe it's OK because this functionality is rarely required by
other VMX tests.

Regards,
-Liran

[PATCH kvm-unit-tests 1/8] x86: vmx: Refactor init of VMX caps to separate function

[Liran Alon]

This is done as a preparation to future patches that will
introduce ability to run VMX on CPUs other than BSP.

Reviewed-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
Reviewed-by: Joao Martins <joao.m.martins@oracle.com>
Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
Signed-off-by: Liran Alon <liran.alon@oracle.com>
---
 x86/vmx.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index 6079420db33a..d74365049eeb 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -1247,19 +1247,8 @@ static int init_vmcs(struct vmcs **vmcs)
 	return 0;
 }
 
-static void init_vmx(void)
+static void init_vmx_caps(void)
 {
-	ulong fix_cr0_set, fix_cr0_clr;
-	ulong fix_cr4_set, fix_cr4_clr;
-
-	vmxon_region = alloc_page();
-
-	vmcs_root = alloc_page();
-
-	fix_cr0_set =  rdmsr(MSR_IA32_VMX_CR0_FIXED0);
-	fix_cr0_clr =  rdmsr(MSR_IA32_VMX_CR0_FIXED1);
-	fix_cr4_set =  rdmsr(MSR_IA32_VMX_CR4_FIXED0);
-	fix_cr4_clr = rdmsr(MSR_IA32_VMX_CR4_FIXED1);
 	basic.val = rdmsr(MSR_IA32_VMX_BASIC);
 	ctrl_pin_rev.val = rdmsr(basic.ctrl ? MSR_IA32_VMX_TRUE_PIN
 			: MSR_IA32_VMX_PINBASED_CTLS);
@@ -1277,6 +1266,23 @@ static void init_vmx(void)
 		ept_vpid.val = rdmsr(MSR_IA32_VMX_EPT_VPID_CAP);
 	else
 		ept_vpid.val = 0;
+}
+
+static void init_vmx(void)
+{
+	ulong fix_cr0_set, fix_cr0_clr;
+	ulong fix_cr4_set, fix_cr4_clr;
+
+	vmxon_region = alloc_page();
+
+	vmcs_root = alloc_page();
+
+	fix_cr0_set =  rdmsr(MSR_IA32_VMX_CR0_FIXED0);
+	fix_cr0_clr =  rdmsr(MSR_IA32_VMX_CR0_FIXED1);
+	fix_cr4_set =  rdmsr(MSR_IA32_VMX_CR4_FIXED0);
+	fix_cr4_clr = rdmsr(MSR_IA32_VMX_CR4_FIXED1);
+
+	init_vmx_caps();
 
 	write_cr0((read_cr0() & fix_cr0_clr) | fix_cr0_set);
 	write_cr4((read_cr4() & fix_cr4_clr) | fix_cr4_set | X86_CR4_VMXE);
-- 
2.20.1

[PATCH kvm-unit-tests 2/8] x86: vmx: Prepare init_vmx() for VMX support on AP CPUs

[Liran Alon]

This is done as a prepartion for future patches that will add
ability for running VMX in CPUs other than BSP.

Reviewed-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
Reviewed-by: Joao Martins <joao.m.martins@oracle.com>
Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
Signed-off-by: Liran Alon <liran.alon@oracle.com>
---
 x86/vmx.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index d74365049eeb..a8d485c3bd09 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -1268,29 +1268,35 @@ static void init_vmx_caps(void)
 		ept_vpid.val = 0;
 }
 
-static void init_vmx(void)
+static void init_vmx(u64 *vmxon_region)
 {
 	ulong fix_cr0_set, fix_cr0_clr;
 	ulong fix_cr4_set, fix_cr4_clr;
 
-	vmxon_region = alloc_page();
-
-	vmcs_root = alloc_page();
-
 	fix_cr0_set =  rdmsr(MSR_IA32_VMX_CR0_FIXED0);
 	fix_cr0_clr =  rdmsr(MSR_IA32_VMX_CR0_FIXED1);
 	fix_cr4_set =  rdmsr(MSR_IA32_VMX_CR4_FIXED0);
 	fix_cr4_clr = rdmsr(MSR_IA32_VMX_CR4_FIXED1);
 
-	init_vmx_caps();
-
 	write_cr0((read_cr0() & fix_cr0_clr) | fix_cr0_set);
 	write_cr4((read_cr4() & fix_cr4_clr) | fix_cr4_set | X86_CR4_VMXE);
 
 	*vmxon_region = basic.revision;
+}
 
+static void alloc_bsp_vmx_pages(void)
+{
+	vmxon_region = alloc_page();
 	guest_stack = alloc_page();
 	guest_syscall_stack = alloc_page();
+	vmcs_root = alloc_page();
+}
+
+static void init_bsp_vmx(void)
+{
+	init_vmx_caps();
+	alloc_bsp_vmx_pages();
+	init_vmx(vmxon_region);
 }
 
 static void do_vmxon_off(void *data)
@@ -1932,7 +1938,7 @@ int main(int argc, const char *argv[])
 		printf("WARNING: vmx not supported, add '-cpu host'\n");
 		goto exit;
 	}
-	init_vmx();
+	init_bsp_vmx();
 	if (test_wanted("test_vmx_feature_control", argv, argc)) {
 		/* Sets MSR_IA32_FEATURE_CONTROL to 0x5 */
 		if (test_vmx_feature_control() != 0)
-- 
2.20.1

[PATCH kvm-unit-tests 3/8] x86: vmx: Expose vmx_init() to be used on AP CPUs

[Liran Alon]

Reviewed-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
Reviewed-by: Joao Martins <joao.m.martins@oracle.com>
Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
Signed-off-by: Liran Alon <liran.alon@oracle.com>
---
 x86/vmx.c | 2 +-
 x86/vmx.h | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index a8d485c3bd09..10b0a423dd23 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -1268,7 +1268,7 @@ static void init_vmx_caps(void)
 		ept_vpid.val = 0;
 }
 
-static void init_vmx(u64 *vmxon_region)
+void init_vmx(u64 *vmxon_region)
 {
 	ulong fix_cr0_set, fix_cr0_clr;
 	ulong fix_cr4_set, fix_cr4_clr;
diff --git a/x86/vmx.h b/x86/vmx.h
index 75abf9a489dd..6127db3cfdd5 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -785,6 +785,8 @@ static inline bool invvpid(unsigned long type, u64 vpid, u64 gla)
 	return ret;
 }
 
+void init_vmx(u64 *vmxon_region);
+
 const char *exit_reason_description(u64 reason);
 void print_vmexit_info(void);
 void print_vmentry_failure_info(struct vmentry_failure *failure);
-- 
2.20.1

[PATCH kvm-unit-tests 4/8] x86: vmx: Support VMXON on AP CPUs VMX region

[Liran Alon]

Reviewed-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
Reviewed-by: Joao Martins <joao.m.martins@oracle.com>
Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
Signed-off-by: Liran Alon <liran.alon@oracle.com>
---
 x86/vmx.c | 25 ++++++++++++-------------
 x86/vmx.h |  9 +++++++--
 2 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index 10b0a423dd23..4b839ea8cc66 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -38,7 +38,7 @@
 #include "smp.h"
 #include "apic.h"
 
-u64 *vmxon_region;
+u64 *bsp_vmxon_region;
 struct vmcs *vmcs_root;
 u32 vpid_cnt;
 void *guest_stack, *guest_syscall_stack;
@@ -693,7 +693,7 @@ static void test_vmclear(void)
 	       vmcs_clear(tmp_root) == 1);
 
 	/* Pass VMXON region */
-	tmp_root = (struct vmcs *)vmxon_region;
+	tmp_root = (struct vmcs *)bsp_vmxon_region;
 	report("test vmclear with vmxon region",
 	       vmcs_clear(tmp_root) == 1);
 
@@ -1286,7 +1286,7 @@ void init_vmx(u64 *vmxon_region)
 
 static void alloc_bsp_vmx_pages(void)
 {
-	vmxon_region = alloc_page();
+	bsp_vmxon_region = alloc_page();
 	guest_stack = alloc_page();
 	guest_syscall_stack = alloc_page();
 	vmcs_root = alloc_page();
@@ -1296,7 +1296,7 @@ static void init_bsp_vmx(void)
 {
 	init_vmx_caps();
 	alloc_bsp_vmx_pages();
-	init_vmx(vmxon_region);
+	init_vmx(bsp_vmxon_region);
 }
 
 static void do_vmxon_off(void *data)
@@ -1346,12 +1346,12 @@ static int test_vmx_feature_control(void)
 static int test_vmxon(void)
 {
 	int ret, ret1;
-	u64 *tmp_region = vmxon_region;
+	u64 *vmxon_region;
 	int width = cpuid_maxphyaddr();
 
 	/* Unaligned page access */
-	vmxon_region = (u64 *)((intptr_t)vmxon_region + 1);
-	ret1 = vmx_on();
+	vmxon_region = (u64 *)((intptr_t)bsp_vmxon_region + 1);
+	ret1 = _vmx_on(vmxon_region);
 	report("test vmxon with unaligned vmxon region", ret1);
 	if (!ret1) {
 		ret = 1;
@@ -1359,8 +1359,8 @@ static int test_vmxon(void)
 	}
 
 	/* gpa bits beyond physical address width are set*/
-	vmxon_region = (u64 *)((intptr_t)tmp_region | ((u64)1 << (width+1)));
-	ret1 = vmx_on();
+	vmxon_region = (u64 *)((intptr_t)bsp_vmxon_region | ((u64)1 << (width+1)));
+	ret1 = _vmx_on(vmxon_region);
 	report("test vmxon with bits set beyond physical address width", ret1);
 	if (!ret1) {
 		ret = 1;
@@ -1368,8 +1368,7 @@ static int test_vmxon(void)
 	}
 
 	/* invalid revision indentifier */
-	vmxon_region = tmp_region;
-	*vmxon_region = 0xba9da9;
+	*bsp_vmxon_region = 0xba9da9;
 	ret1 = vmx_on();
 	report("test vmxon with invalid revision identifier", ret1);
 	if (!ret1) {
@@ -1378,7 +1377,7 @@ static int test_vmxon(void)
 	}
 
 	/* and finally a valid region */
-	*vmxon_region = basic.revision;
+	*bsp_vmxon_region = basic.revision;
 	ret = vmx_on();
 	report("test vmxon with valid vmxon region", !ret);
 
@@ -1408,7 +1407,7 @@ static void test_vmptrld(void)
 	/* Pass VMXON region */
 	assert(!vmcs_clear(vmcs));
 	assert(!make_vmcs_current(vmcs));
-	tmp_root = (struct vmcs *)vmxon_region;
+	tmp_root = (struct vmcs *)bsp_vmxon_region;
 	report("test vmptrld with vmxon region",
 	       make_vmcs_current(tmp_root) == 1);
 	report("test vmptrld with vmxon region vm-instruction error",
diff --git a/x86/vmx.h b/x86/vmx.h
index 6127db3cfdd5..fdc6f7171826 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -660,13 +660,13 @@ extern union vmx_ctrl_msr ctrl_exit_rev;
 extern union vmx_ctrl_msr ctrl_enter_rev;
 extern union vmx_ept_vpid  ept_vpid;
 
-extern u64 *vmxon_region;
+extern u64 *bsp_vmxon_region;
 
 void vmx_set_test_stage(u32 s);
 u32 vmx_get_test_stage(void);
 void vmx_inc_test_stage(void);
 
-static int vmx_on(void)
+static int _vmx_on(u64 *vmxon_region)
 {
 	bool ret;
 	u64 rflags = read_rflags() | X86_EFLAGS_CF | X86_EFLAGS_ZF;
@@ -675,6 +675,11 @@ static int vmx_on(void)
 	return ret;
 }
 
+static int vmx_on(void)
+{
+	return _vmx_on(bsp_vmxon_region);
+}
+
 static int vmx_off(void)
 {
 	bool ret;
-- 
2.20.1

[PATCH kvm-unit-tests 5/8] x86: vmx: Use MSR_IA32_FEATURE_CONTROL bits names

[Liran Alon]

Avoid using hard-coded numbers to improve code readability.

Reviewed-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
Reviewed-by: Joao Martins <joao.m.martins@oracle.com>
Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
Signed-off-by: Liran Alon <liran.alon@oracle.com>
---
 x86/vmx.c | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index 4b839ea8cc66..146734d334a1 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -1314,13 +1314,18 @@ static int test_vmx_feature_control(void)
 {
 	u64 ia32_feature_control;
 	bool vmx_enabled;
+	bool feature_control_locked;
 
 	ia32_feature_control = rdmsr(MSR_IA32_FEATURE_CONTROL);
-	vmx_enabled = ((ia32_feature_control & 0x5) == 0x5);
-	if ((ia32_feature_control & 0x5) == 0x5) {
+	vmx_enabled =
+		ia32_feature_control & FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX;
+	feature_control_locked =
+		ia32_feature_control & FEATURE_CONTROL_LOCKED;
+
+	if (vmx_enabled && feature_control_locked) {
 		printf("VMX enabled and locked by BIOS\n");
 		return 0;
-	} else if (ia32_feature_control & 0x1) {
+	} else if (feature_control_locked) {
 		printf("ERROR: VMX locked out by BIOS!?\n");
 		return 1;
 	}
@@ -1329,12 +1334,17 @@ static int test_vmx_feature_control(void)
 	report("test vmxon with FEATURE_CONTROL cleared",
 	       test_for_exception(GP_VECTOR, &do_vmxon_off, NULL));
 
-	wrmsr(MSR_IA32_FEATURE_CONTROL, 0x4);
+	wrmsr(MSR_IA32_FEATURE_CONTROL, FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX);
 	report("test vmxon without FEATURE_CONTROL lock",
 	       test_for_exception(GP_VECTOR, &do_vmxon_off, NULL));
 
-	wrmsr(MSR_IA32_FEATURE_CONTROL, 0x5);
-	vmx_enabled = ((rdmsr(MSR_IA32_FEATURE_CONTROL) & 0x5) == 0x5);
+	wrmsr(MSR_IA32_FEATURE_CONTROL,
+		  FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX |
+		  FEATURE_CONTROL_LOCKED);
+
+	ia32_feature_control = rdmsr(MSR_IA32_FEATURE_CONTROL);
+	vmx_enabled =
+		ia32_feature_control & FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX;
 	report("test enable VMX in FEATURE_CONTROL", vmx_enabled);
 
 	report("test FEATURE_CONTROL lock bit",
@@ -1922,6 +1932,7 @@ test_wanted(const char *name, const char *filters[], int filter_count)
 int main(int argc, const char *argv[])
 {
 	int i = 0;
+	bool vmx_enabled;
 
 	setup_vm();
 	smp_init();
@@ -1943,8 +1954,13 @@ int main(int argc, const char *argv[])
 		if (test_vmx_feature_control() != 0)
 			goto exit;
 	} else {
-		if ((rdmsr(MSR_IA32_FEATURE_CONTROL) & 0x5) != 0x5)
-			wrmsr(MSR_IA32_FEATURE_CONTROL, 0x5);
+		vmx_enabled = rdmsr(MSR_IA32_FEATURE_CONTROL) &
+			FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX;
+		if (!vmx_enabled) {
+			wrmsr(MSR_IA32_FEATURE_CONTROL,
+				  FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX |
+				  FEATURE_CONTROL_LOCKED);
+		}
 	}
 
 	if (test_wanted("test_vmxon", argv, argc)) {
-- 
2.20.1

[PATCH kvm-unit-tests 6/8] x86: vmx: Expose util to enable VMX in MSR_IA32_FEATURE_CONTROL

[Liran Alon]

Reviewed-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
Reviewed-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Liran Alon <liran.alon@oracle.com>
---
 x86/vmx.c | 22 ++++++++++++++--------
 x86/vmx.h |  1 +
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index 146734d334a1..b9328d9ca75c 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -1247,6 +1247,19 @@ static int init_vmcs(struct vmcs **vmcs)
 	return 0;
 }
 
+void enable_vmx(void)
+{
+	bool vmx_enabled =
+		rdmsr(MSR_IA32_FEATURE_CONTROL) &
+		FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX;
+
+	if (!vmx_enabled) {
+		wrmsr(MSR_IA32_FEATURE_CONTROL,
+				FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX |
+				FEATURE_CONTROL_LOCKED);
+	}
+}
+
 static void init_vmx_caps(void)
 {
 	basic.val = rdmsr(MSR_IA32_VMX_BASIC);
@@ -1932,7 +1945,6 @@ test_wanted(const char *name, const char *filters[], int filter_count)
 int main(int argc, const char *argv[])
 {
 	int i = 0;
-	bool vmx_enabled;
 
 	setup_vm();
 	smp_init();
@@ -1954,13 +1966,7 @@ int main(int argc, const char *argv[])
 		if (test_vmx_feature_control() != 0)
 			goto exit;
 	} else {
-		vmx_enabled = rdmsr(MSR_IA32_FEATURE_CONTROL) &
-			FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX;
-		if (!vmx_enabled) {
-			wrmsr(MSR_IA32_FEATURE_CONTROL,
-				  FEATURE_CONTROL_VMXON_ENABLED_OUTSIDE_SMX |
-				  FEATURE_CONTROL_LOCKED);
-		}
+		enable_vmx();
 	}
 
 	if (test_wanted("test_vmxon", argv, argc)) {
diff --git a/x86/vmx.h b/x86/vmx.h
index fdc6f7171826..e47134e29e83 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -790,6 +790,7 @@ static inline bool invvpid(unsigned long type, u64 vpid, u64 gla)
 	return ret;
 }
 
+void enable_vmx(void);
 void init_vmx(u64 *vmxon_region);
 
 const char *exit_reason_description(u64 reason);
-- 
2.20.1

[PATCH kvm-unit-tests 7/8] x86: vmx: Allow tests to hand-over test-vmcs between CPUs

[Liran Alon]

By exposing launched, test-vmcs can be hand-over between CPUs
by VMCLEAR test-vmcs and resetting launched to false.

Reviewed-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
Reviewed-by: Joao Martins <joao.m.martins@oracle.com>
Signed-off-by: Liran Alon <liran.alon@oracle.com>
---
 x86/vmx.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/x86/vmx.h b/x86/vmx.h
index e47134e29e83..a8bc8472ed61 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -661,6 +661,7 @@ extern union vmx_ctrl_msr ctrl_enter_rev;
 extern union vmx_ept_vpid  ept_vpid;
 
 extern u64 *bsp_vmxon_region;
+extern bool launched;
 
 void vmx_set_test_stage(u32 s);
 u32 vmx_get_test_stage(void);
-- 
2.20.1

[PATCH kvm-unit-tests 8/8] x86: vmx: Test INIT processing during various CPU VMX states

[Liran Alon]

Add vmx test to verify the functionality introduced by KVM commit:
4b9852f4f389 ("KVM: x86: Fix INIT signal handling in various CPU states").

The test verifies the following functionality:
1) An INIT signal received when CPU is in VMX operation
   is blocked until it exits VMX operation.
2) If there is an INIT signal pending when CPU is in
   VMX non-root mode, it result in VMExit with (reason == 3).
3) Exit from VMX non-root mode on VMExit do not clear
   pending INIT signal in LAPIC.
4) When CPU exits VMX operation, pending INIT signal in
   LAPIC is processed.

Note: The test is excluded from the "vmx" test-suite as
it ruins the execution environment of CPU1 because
during the test it process an INIT signal.

Reviewed-by: Nikita Leshenko <nikita.leshchenko@oracle.com>
Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
Signed-off-by: Liran Alon <liran.alon@oracle.com>
---
 x86/unittests.cfg |  10 +++-
 x86/vmx_tests.c   | 145 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 154 insertions(+), 1 deletion(-)

diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index 694ee3d42f3a..8156256146c3 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -227,7 +227,7 @@ extra_params = -cpu qemu64,+umip
 
 [vmx]
 file = vmx.flat
-extra_params = -cpu host,+vmx -append "-exit_monitor_from_l2_test -ept_access* -vmx_smp* -vmx_vmcs_shadow_test"
+extra_params = -cpu host,+vmx -append "-exit_monitor_from_l2_test -ept_access* -vmx_smp* -vmx_vmcs_shadow_test -vmx_init_signal_test"
 arch = x86_64
 groups = vmx
 
@@ -265,6 +265,14 @@ extra_params = -cpu host,+vmx -m 2048 -append vmx_apic_passthrough_thread_test
 arch = x86_64
 groups = vmx
 
+[vmx_init_signal_test]
+file = vmx.flat
+smp = 2
+extra_params = -cpu host,+vmx -m 2048 -append vmx_init_signal_test
+arch = x86_64
+groups = vmx
+timeout = 10
+
 [vmx_vmcs_shadow_test]
 file = vmx.flat
 extra_params = -cpu host,+vmx -append vmx_vmcs_shadow_test
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 096640a56891..962990e30d33 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -8219,6 +8219,150 @@ static void vmx_apic_passthrough_thread_test(void)
 	vmx_apic_passthrough(true);
 }
 
+static u64 init_signal_test_exit_reason;
+static bool init_signal_test_thread_continued;
+
+static void init_signal_test_thread(void *data)
+{
+	struct vmcs *test_vmcs = data;
+
+	/* Enter VMX operation (i.e. exec VMXON) */
+	u64 *ap_vmxon_region = alloc_page();
+	enable_vmx();
+	init_vmx(ap_vmxon_region);
+	_vmx_on(ap_vmxon_region);
+
+	/* Signal CPU have entered VMX operation */
+	vmx_set_test_stage(1);
+
+	/* Wait for BSP CPU to send INIT signal */
+	while (vmx_get_test_stage() != 2)
+		;
+
+	/*
+	 * Signal that we continue as usual as INIT signal
+	 * should be blocked while CPU is in VMX operation
+	 */
+	vmx_set_test_stage(3);
+
+	/* Wait for signal to enter VMX non-root mode */
+	while (vmx_get_test_stage() != 4)
+		;
+
+	/* Enter VMX non-root mode */
+	test_set_guest(v2_null_test_guest);
+	make_vmcs_current(test_vmcs);
+	enter_guest();
+	/* Save exit reason for BSP CPU to compare to expected result */
+	init_signal_test_exit_reason = vmcs_read(EXI_REASON);
+	/* VMCLEAR test-vmcs so it could be loaded by BSP CPU */
+	vmcs_clear(test_vmcs);
+	launched = false;
+	/* Signal that CPU exited to VMX root mode */
+	vmx_set_test_stage(5);
+
+	/* Wait for signal to exit VMX operation */
+	while (vmx_get_test_stage() != 6)
+		;
+
+	/* Exit VMX operation (i.e. exec VMXOFF) */
+	vmx_off();
+
+	/*
+	 * Exiting VMX operation should result in latched
+	 * INIT signal being processed. Therefore, we should
+	 * never reach the below code. Thus, signal to BSP
+	 * CPU if we have reached here so it is able to
+	 * report an issue if it happens.
+	 */
+	init_signal_test_thread_continued = true;
+}
+
+#define INIT_SIGNAL_TEST_DELAY	100000000ULL
+
+static void vmx_init_signal_test(void)
+{
+	struct vmcs *test_vmcs;
+
+	if (cpu_count() < 2) {
+		report_skip(__func__);
+		return;
+	}
+
+	/* VMCLEAR test-vmcs so it could be loaded by other CPU */
+	vmcs_save(&test_vmcs);
+	vmcs_clear(test_vmcs);
+
+	vmx_set_test_stage(0);
+	on_cpu_async(1, init_signal_test_thread, test_vmcs);
+
+	/* Wait for other CPU to enter VMX operation */
+	while (vmx_get_test_stage() != 1)
+		;
+
+	/* Send INIT signal to other CPU */
+	apic_icr_write(APIC_DEST_PHYSICAL | APIC_DM_INIT | APIC_INT_ASSERT,
+				   id_map[1]);
+	/* Signal other CPU we have sent INIT signal */
+	vmx_set_test_stage(2);
+
+	/*
+	 * Wait reasonable amount of time for INIT signal to
+	 * be received on other CPU and verify that other CPU
+	 * have proceed as usual to next test stage as INIT
+	 * signal should be blocked while other CPU in
+	 * VMX operation
+	 */
+	delay(INIT_SIGNAL_TEST_DELAY);
+	report("INIT signal blocked when CPU in VMX operation",
+		   vmx_get_test_stage() == 3);
+	/* No point to continue if we failed at this point */
+	if (vmx_get_test_stage() != 3)
+		return;
+
+	/* Signal other CPU to enter VMX non-root mode */
+	init_signal_test_exit_reason = -1ull;
+	vmx_set_test_stage(4);
+	/*
+	 * Wait reasonable amont of time for other CPU
+	 * to exit to VMX root mode
+	 */
+	delay(INIT_SIGNAL_TEST_DELAY);
+	if (vmx_get_test_stage() != 5) {
+		report("Pending INIT signal haven't resulted in VMX exit", false);
+		return;
+	}
+	report("INIT signal during VMX non-root mode result in exit-reason %s (%lu)",
+			init_signal_test_exit_reason == VMX_INIT,
+			exit_reason_description(init_signal_test_exit_reason),
+			init_signal_test_exit_reason);
+
+	/* Run guest to completion */
+	make_vmcs_current(test_vmcs);
+	enter_guest();
+
+	/* Signal other CPU to exit VMX operation */
+	init_signal_test_thread_continued = false;
+	vmx_set_test_stage(6);
+
+	/*
+	 * Wait reasonable amount of time for other CPU
+	 * to run after INIT signal was processed
+	 */
+	delay(INIT_SIGNAL_TEST_DELAY);
+	report("INIT signal processed after exit VMX operation",
+		   !init_signal_test_thread_continued);
+
+	/*
+	 * TODO: Send SIPI to other CPU to sipi_entry (See x86/cstart64.S)
+	 * to re-init it to kvm-unit-tests standard environment.
+	 * Somehow (?) verify that SIPI was indeed received.
+	 */
+
+	/* Report success */
+	report(__func__, 1);
+}
+
 enum vmcs_access {
 	ACCESS_VMREAD,
 	ACCESS_VMWRITE,
@@ -8629,6 +8773,7 @@ struct vmx_test vmx_tests[] = {
 	/* APIC pass-through tests */
 	TEST(vmx_apic_passthrough_test),
 	TEST(vmx_apic_passthrough_thread_test),
+	TEST(vmx_init_signal_test),
 	/* VMCS Shadowing tests */
 	TEST(vmx_vmcs_shadow_test),
 	/* Regression tests */
-- 
2.20.1

Re: [PATCH kvm-unit-tests 0/8]: x86: vmx: Test INIT processing in various CPU VMX states

[Vitaly Kuznetsov]

Liran Alon <liran.alon@oracle.com> writes:

> Hi,
>
> This patch series aims to add a vmx test to verify the functionality
> introduced by KVM commit:
> 4b9852f4f389 ("KVM: x86: Fix INIT signal handling in various CPU states")
>
> The test verifies the following functionality:
> 1) An INIT signal received when CPU is in VMX operation
>   is latched until it exits VMX operation.
> 2) If there is an INIT signal pending when CPU is in
>   VMX non-root mode, it result in VMExit with (reason == 3).
> 3) Exit from VMX non-root mode on VMExit do not clear
>   pending INIT signal in LAPIC.
> 4) When CPU exits VMX operation, pending INIT signal in
>   LAPIC is processed.
>
> In order to write such a complex test, the vmx tests framework was
> enhanced to support using VMX in non BSP CPUs. This enhancement is
> implemented in patches 1-7. The test itself is implemented at patch 8.
> This enhancement to the vmx tests framework is a bit hackish, but
> I believe it's OK because this functionality is rarely required by
> other VMX tests.
>

Tested-by: Vitaly Kuznetsov <vkuznets@redhat.com>

Thanks!

-- 
Vitaly

Re: [PATCH kvm-unit-tests 0/8]: x86: vmx: Test INIT processing in various CPU VMX states

[Liran Alon]

Gentle ping.

> On 19 Sep 2019, at 17:08, Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
> 
> Liran Alon <liran.alon@oracle.com> writes:
> 
>> Hi,
>> 
>> This patch series aims to add a vmx test to verify the functionality
>> introduced by KVM commit:
>> 4b9852f4f389 ("KVM: x86: Fix INIT signal handling in various CPU states")
>> 
>> The test verifies the following functionality:
>> 1) An INIT signal received when CPU is in VMX operation
>>  is latched until it exits VMX operation.
>> 2) If there is an INIT signal pending when CPU is in
>>  VMX non-root mode, it result in VMExit with (reason == 3).
>> 3) Exit from VMX non-root mode on VMExit do not clear
>>  pending INIT signal in LAPIC.
>> 4) When CPU exits VMX operation, pending INIT signal in
>>  LAPIC is processed.
>> 
>> In order to write such a complex test, the vmx tests framework was
>> enhanced to support using VMX in non BSP CPUs. This enhancement is
>> implemented in patches 1-7. The test itself is implemented at patch 8.
>> This enhancement to the vmx tests framework is a bit hackish, but
>> I believe it's OK because this functionality is rarely required by
>> other VMX tests.
>> 
> 
> Tested-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> 
> Thanks!
> 
> -- 
> Vitaly

Re: [PATCH kvm-unit-tests 0/8]: x86: vmx: Test INIT processing in various CPU VMX states

[Paolo Bonzini]

On 24/09/19 17:34, Liran Alon wrote:
> Gentle ping.

I'm going to send another pull request to Linus this week and then will
get back to this patch (and also Krish's performance counter series).

Paolo

>> On 19 Sep 2019, at 17:08, Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>>
>> Liran Alon <liran.alon@oracle.com> writes:
>>
>>> Hi,
>>>
>>> This patch series aims to add a vmx test to verify the functionality
>>> introduced by KVM commit:
>>> 4b9852f4f389 ("KVM: x86: Fix INIT signal handling in various CPU states")
>>>
>>> The test verifies the following functionality:
>>> 1) An INIT signal received when CPU is in VMX operation
>>>  is latched until it exits VMX operation.
>>> 2) If there is an INIT signal pending when CPU is in
>>>  VMX non-root mode, it result in VMExit with (reason == 3).
>>> 3) Exit from VMX non-root mode on VMExit do not clear
>>>  pending INIT signal in LAPIC.
>>> 4) When CPU exits VMX operation, pending INIT signal in
>>>  LAPIC is processed.
>>>
>>> In order to write such a complex test, the vmx tests framework was
>>> enhanced to support using VMX in non BSP CPUs. This enhancement is
>>> implemented in patches 1-7. The test itself is implemented at patch 8.
>>> This enhancement to the vmx tests framework is a bit hackish, but
>>> I believe it's OK because this functionality is rarely required by
>>> other VMX tests.
>>>
>>
>> Tested-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>>
>> Thanks!
>>
>> -- 
>> Vitaly
> 

Re: [PATCH kvm-unit-tests 0/8]: x86: vmx: Test INIT processing in various CPU VMX states

[Liran Alon]

Paolo, I have noticed that all the patches of these series were merged to origin/master
besides the last one which adds the patch itself.

Have you missed the last patch by mistake?

-Liran

> On 24 Sep 2019, at 18:42, Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> On 24/09/19 17:34, Liran Alon wrote:
>> Gentle ping.
> 
> I'm going to send another pull request to Linus this week and then will
> get back to this patch (and also Krish's performance counter series).
> 
> Paolo
> 
>>> On 19 Sep 2019, at 17:08, Vitaly Kuznetsov <vkuznets@redhat.com> wrote:
>>> 
>>> Liran Alon <liran.alon@oracle.com> writes:
>>> 
>>>> Hi,
>>>> 
>>>> This patch series aims to add a vmx test to verify the functionality
>>>> introduced by KVM commit:
>>>> 4b9852f4f389 ("KVM: x86: Fix INIT signal handling in various CPU states")
>>>> 
>>>> The test verifies the following functionality:
>>>> 1) An INIT signal received when CPU is in VMX operation
>>>> is latched until it exits VMX operation.
>>>> 2) If there is an INIT signal pending when CPU is in
>>>> VMX non-root mode, it result in VMExit with (reason == 3).
>>>> 3) Exit from VMX non-root mode on VMExit do not clear
>>>> pending INIT signal in LAPIC.
>>>> 4) When CPU exits VMX operation, pending INIT signal in
>>>> LAPIC is processed.
>>>> 
>>>> In order to write such a complex test, the vmx tests framework was
>>>> enhanced to support using VMX in non BSP CPUs. This enhancement is
>>>> implemented in patches 1-7. The test itself is implemented at patch 8.
>>>> This enhancement to the vmx tests framework is a bit hackish, but
>>>> I believe it's OK because this functionality is rarely required by
>>>> other VMX tests.
>>>> 
>>> 
>>> Tested-by: Vitaly Kuznetsov <vkuznets@redhat.com>
>>> 
>>> Thanks!
>>> 
>>> -- 
>>> Vitaly
>> 
> 

Re: [PATCH kvm-unit-tests 0/8]: x86: vmx: Test INIT processing in various CPU VMX states

[Paolo Bonzini]

On 26/09/19 01:57, Liran Alon wrote:
> Paolo, I have noticed that all the patches of these series were merged to origin/master
> besides the last one which adds the patch itself.
> 
> Have you missed the last patch by mistake?

No, I was sidetracked by the HOST_EFER failure; I immediately checked if
(for whatever reason) it was caused by the refactoring part of this
series, and pushed those 7 patches because it wasn't.

I will push patch 8 shortly, since I have the HOST_EFER failure covered
and have now tested the new INIT testcases properly.

Paolo

Re: [PATCH kvm-unit-tests 0/8]: x86: vmx: Test INIT processing in various CPU VMX states

[Nadav Amit]

> On Sep 19, 2019, at 5:52 AM, Liran Alon <liran.alon@oracle.com> wrote:
> 
> Hi,
> 
> This patch series aims to add a vmx test to verify the functionality
> introduced by KVM commit:
> 4b9852f4f389 ("KVM: x86: Fix INIT signal handling in various CPU states")
> 
> The test verifies the following functionality:
> 1) An INIT signal received when CPU is in VMX operation
>  is latched until it exits VMX operation.
> 2) If there is an INIT signal pending when CPU is in
>  VMX non-root mode, it result in VMExit with (reason == 3).
> 3) Exit from VMX non-root mode on VMExit do not clear
>  pending INIT signal in LAPIC.
> 4) When CPU exits VMX operation, pending INIT signal in
>  LAPIC is processed.
> 
> In order to write such a complex test, the vmx tests framework was
> enhanced to support using VMX in non BSP CPUs. This enhancement is
> implemented in patches 1-7. The test itself is implemented at patch 8.
> This enhancement to the vmx tests framework is a bit hackish, but
> I believe it's OK because this functionality is rarely required by
> other VMX tests.
> 
> Regards,
> -Liran

Hi Liran,

I ran this test on bare-metal and it fails:

 Test suite: vmx_init_signal_test
 PASS: INIT signal blocked when CPU in VMX operation
 PASS: INIT signal during VMX non-root mode result in exit-reason VMX_INIT (3)
 FAIL: INIT signal processed after exit VMX operation
 SUMMARY: 8 tests, 1 unexpected failures

I don’t have time to debug this issue, but let me know if you want some
print-outs.

Nadav

Re: [PATCH kvm-unit-tests 0/8]: x86: vmx: Test INIT processing in various CPU VMX states

[Liran Alon]

> On 1 Oct 2019, at 2:02, Nadav Amit <nadav.amit@gmail.com> wrote:
> 
>> On Sep 19, 2019, at 5:52 AM, Liran Alon <liran.alon@oracle.com> wrote:
>> 
>> Hi,
>> 
>> This patch series aims to add a vmx test to verify the functionality
>> introduced by KVM commit:
>> 4b9852f4f389 ("KVM: x86: Fix INIT signal handling in various CPU states")
>> 
>> The test verifies the following functionality:
>> 1) An INIT signal received when CPU is in VMX operation
>> is latched until it exits VMX operation.
>> 2) If there is an INIT signal pending when CPU is in
>> VMX non-root mode, it result in VMExit with (reason == 3).
>> 3) Exit from VMX non-root mode on VMExit do not clear
>> pending INIT signal in LAPIC.
>> 4) When CPU exits VMX operation, pending INIT signal in
>> LAPIC is processed.
>> 
>> In order to write such a complex test, the vmx tests framework was
>> enhanced to support using VMX in non BSP CPUs. This enhancement is
>> implemented in patches 1-7. The test itself is implemented at patch 8.
>> This enhancement to the vmx tests framework is a bit hackish, but
>> I believe it's OK because this functionality is rarely required by
>> other VMX tests.
>> 
>> Regards,
>> -Liran
> 
> Hi Liran,
> 
> I ran this test on bare-metal and it fails:
> 
> Test suite: vmx_init_signal_test
> PASS: INIT signal blocked when CPU in VMX operation
> PASS: INIT signal during VMX non-root mode result in exit-reason VMX_INIT (3)
> FAIL: INIT signal processed after exit VMX operation
> SUMMARY: 8 tests, 1 unexpected failures
> 
> I don’t have time to debug this issue, but let me know if you want some
> print-outs.
> 
> Nadav
> 

Thanks Nadav for running this on bare-metal. This is very useful!

It seems that when CPU exited on exit-reason VMX_INIT (3), the LAPIC INIT pending event
was consumed instead of still being latched until CPU exits VMX operation.

In my commit which this unit-test verifies 4b9852f4f389 ("KVM: x86: Fix INIT signal handling in various CPU states”),
I have assumed that such exit-reason don’t consume the LAPIC INIT pending event.
My assumption was based on the phrasing of Intel SDM section 25.2 OTHER CAUSES OF VM EXITS regarding INIT signals:
"Such exits do not modify register state or clear pending events as they would outside of VMX operation."
I thought Intel logic behind this is that if an INIT signal is sent to a CPU in VMX non-root mode, it would exit
on exit-reason 3 which would allow hypervisor to decide to exit VMX operation in order to consume INIT signal.

Nadav, can you attempt to just add a delay in init_signal_test_thread() between calling vmx_off() & setting init_signal_test_thread_continued to true?
It may be that real hardware delays a bit when the INIT signal is released from LAPIC after exit VMX operation.

Thanks,
-Liran

Re: [PATCH kvm-unit-tests 0/8]: x86: vmx: Test INIT processing in various CPU VMX states

[Nadav Amit]

> On Sep 30, 2019, at 5:48 PM, Liran Alon <liran.alon@oracle.com> wrote:
> 
> 
> 
>> On 1 Oct 2019, at 2:02, Nadav Amit <nadav.amit@gmail.com> wrote:
>> 
>>> On Sep 19, 2019, at 5:52 AM, Liran Alon <liran.alon@oracle.com> wrote:
>>> 
>>> Hi,
>>> 
>>> This patch series aims to add a vmx test to verify the functionality
>>> introduced by KVM commit:
>>> 4b9852f4f389 ("KVM: x86: Fix INIT signal handling in various CPU states")
>>> 
>>> The test verifies the following functionality:
>>> 1) An INIT signal received when CPU is in VMX operation
>>> is latched until it exits VMX operation.
>>> 2) If there is an INIT signal pending when CPU is in
>>> VMX non-root mode, it result in VMExit with (reason == 3).
>>> 3) Exit from VMX non-root mode on VMExit do not clear
>>> pending INIT signal in LAPIC.
>>> 4) When CPU exits VMX operation, pending INIT signal in
>>> LAPIC is processed.
>>> 
>>> In order to write such a complex test, the vmx tests framework was
>>> enhanced to support using VMX in non BSP CPUs. This enhancement is
>>> implemented in patches 1-7. The test itself is implemented at patch 8.
>>> This enhancement to the vmx tests framework is a bit hackish, but
>>> I believe it's OK because this functionality is rarely required by
>>> other VMX tests.
>>> 
>>> Regards,
>>> -Liran
>> 
>> Hi Liran,
>> 
>> I ran this test on bare-metal and it fails:
>> 
>> Test suite: vmx_init_signal_test
>> PASS: INIT signal blocked when CPU in VMX operation
>> PASS: INIT signal during VMX non-root mode result in exit-reason VMX_INIT (3)
>> FAIL: INIT signal processed after exit VMX operation
>> SUMMARY: 8 tests, 1 unexpected failures
>> 
>> I don’t have time to debug this issue, but let me know if you want some
>> print-outs.
>> 
>> Nadav
> 
> Thanks Nadav for running this on bare-metal. This is very useful!
> 
> It seems that when CPU exited on exit-reason VMX_INIT (3), the LAPIC INIT pending event
> was consumed instead of still being latched until CPU exits VMX operation.
> 
> In my commit which this unit-test verifies 4b9852f4f389 ("KVM: x86: Fix INIT signal handling in various CPU states”),
> I have assumed that such exit-reason don’t consume the LAPIC INIT pending event.
> My assumption was based on the phrasing of Intel SDM section 25.2 OTHER CAUSES OF VM EXITS regarding INIT signals:
> "Such exits do not modify register state or clear pending events as they would outside of VMX operation."
> I thought Intel logic behind this is that if an INIT signal is sent to a CPU in VMX non-root mode, it would exit
> on exit-reason 3 which would allow hypervisor to decide to exit VMX operation in order to consume INIT signal.

I think this sentence can be read differently. It also reasonable not to
bound the host to get an INIT signal the moment it disabled vmx.

> Nadav, can you attempt to just add a delay in init_signal_test_thread() between calling vmx_off() & setting init_signal_test_thread_continued to true?
> It may be that real hardware delays a bit when the INIT signal is released from LAPIC after exit VMX operation.

I added “delay(100000)” between them, but got the same result.

Re: [PATCH kvm-unit-tests 0/8]: x86: vmx: Test INIT processing in various CPU VMX states

[Liran Alon]

> On 1 Oct 2019, at 4:14, Nadav Amit <nadav.amit@gmail.com> wrote:
> 
>> On Sep 30, 2019, at 5:48 PM, Liran Alon <liran.alon@oracle.com> wrote:
>> 
>> 
>> 
>>> On 1 Oct 2019, at 2:02, Nadav Amit <nadav.amit@gmail.com> wrote:
>>> 
>>>> On Sep 19, 2019, at 5:52 AM, Liran Alon <liran.alon@oracle.com> wrote:
>>>> 
>>>> Hi,
>>>> 
>>>> This patch series aims to add a vmx test to verify the functionality
>>>> introduced by KVM commit:
>>>> 4b9852f4f389 ("KVM: x86: Fix INIT signal handling in various CPU states")
>>>> 
>>>> The test verifies the following functionality:
>>>> 1) An INIT signal received when CPU is in VMX operation
>>>> is latched until it exits VMX operation.
>>>> 2) If there is an INIT signal pending when CPU is in
>>>> VMX non-root mode, it result in VMExit with (reason == 3).
>>>> 3) Exit from VMX non-root mode on VMExit do not clear
>>>> pending INIT signal in LAPIC.
>>>> 4) When CPU exits VMX operation, pending INIT signal in
>>>> LAPIC is processed.
>>>> 
>>>> In order to write such a complex test, the vmx tests framework was
>>>> enhanced to support using VMX in non BSP CPUs. This enhancement is
>>>> implemented in patches 1-7. The test itself is implemented at patch 8.
>>>> This enhancement to the vmx tests framework is a bit hackish, but
>>>> I believe it's OK because this functionality is rarely required by
>>>> other VMX tests.
>>>> 
>>>> Regards,
>>>> -Liran
>>> 
>>> Hi Liran,
>>> 
>>> I ran this test on bare-metal and it fails:
>>> 
>>> Test suite: vmx_init_signal_test
>>> PASS: INIT signal blocked when CPU in VMX operation
>>> PASS: INIT signal during VMX non-root mode result in exit-reason VMX_INIT (3)
>>> FAIL: INIT signal processed after exit VMX operation
>>> SUMMARY: 8 tests, 1 unexpected failures
>>> 
>>> I don’t have time to debug this issue, but let me know if you want some
>>> print-outs.
>>> 
>>> Nadav
>> 
>> Thanks Nadav for running this on bare-metal. This is very useful!
>> 
>> It seems that when CPU exited on exit-reason VMX_INIT (3), the LAPIC INIT pending event
>> was consumed instead of still being latched until CPU exits VMX operation.
>> 
>> In my commit which this unit-test verifies 4b9852f4f389 ("KVM: x86: Fix INIT signal handling in various CPU states”),
>> I have assumed that such exit-reason don’t consume the LAPIC INIT pending event.
>> My assumption was based on the phrasing of Intel SDM section 25.2 OTHER CAUSES OF VM EXITS regarding INIT signals:
>> "Such exits do not modify register state or clear pending events as they would outside of VMX operation."
>> I thought Intel logic behind this is that if an INIT signal is sent to a CPU in VMX non-root mode, it would exit
>> on exit-reason 3 which would allow hypervisor to decide to exit VMX operation in order to consume INIT signal.
> 
> I think this sentence can be read differently. It also reasonable not to
> bound the host to get an INIT signal the moment it disabled vmx.

If INIT signal won’t be kept pending until exiting VMX operation, target CPU which was sent with INIT signal
when it was running guest, basically lost INIT signal forever and just received an exit-reason it cannot do much with.
That’s why I thought Intel designed this mechanism like I specified above.

I also remembered to verify this behaviour against some discussions made online:
1) https://software.intel.com/en-us/forums/virtualization-software-development/topic/355484
* "When the 16-bit guest issues an INIT IPI to itself using the APIC, I run into an infinite VMExit situation that my hypervisor cannot seem to recover from.”
* "In response to the VMExit with a reason of 3 (which is expected), the hypervisor resets the 16-bit guest's registers, limits, access rights, etc. to simulate starting execution from a known initialization point.  However, it seems that as soon as the hypervisor resumes guest execution, the VMExit occurs again, repeatedly.”
2) https://patchwork.kernel.org/patch/2244311/
"I actually find it very useful. On INIT vmexit hypervisor may call vmxoff and do proper reset."

Anyway, Sean, can you assist verifying inside Intel what should be the expected behaviour?

> 
>> Nadav, can you attempt to just add a delay in init_signal_test_thread() between calling vmx_off() & setting init_signal_test_thread_continued to true?
>> It may be that real hardware delays a bit when the INIT signal is released from LAPIC after exit VMX operation.
> 
> I added “delay(100000)” between them, but got the same result.
> 

Weird. Then I have no idea why this happening…

Thanks,
-Liran

Re: [PATCH kvm-unit-tests 0/8]: x86: vmx: Test INIT processing in various CPU VMX states

[Nadav Amit]

> On Sep 30, 2019, at 6:23 PM, Liran Alon <liran.alon@oracle.com> wrote:
> 
> 
> 
>> On 1 Oct 2019, at 4:14, Nadav Amit <nadav.amit@gmail.com> wrote:
>> 
>>> On Sep 30, 2019, at 5:48 PM, Liran Alon <liran.alon@oracle.com> wrote:
>>> 
>>> 
>>> 
>>>> On 1 Oct 2019, at 2:02, Nadav Amit <nadav.amit@gmail.com> wrote:
>>>> 
>>>>> On Sep 19, 2019, at 5:52 AM, Liran Alon <liran.alon@oracle.com> wrote:
>>>>> 
>>>>> Hi,
>>>>> 
>>>>> This patch series aims to add a vmx test to verify the functionality
>>>>> introduced by KVM commit:
>>>>> 4b9852f4f389 ("KVM: x86: Fix INIT signal handling in various CPU states")
>>>>> 
>>>>> The test verifies the following functionality:
>>>>> 1) An INIT signal received when CPU is in VMX operation
>>>>> is latched until it exits VMX operation.
>>>>> 2) If there is an INIT signal pending when CPU is in
>>>>> VMX non-root mode, it result in VMExit with (reason == 3).
>>>>> 3) Exit from VMX non-root mode on VMExit do not clear
>>>>> pending INIT signal in LAPIC.
>>>>> 4) When CPU exits VMX operation, pending INIT signal in
>>>>> LAPIC is processed.
>>>>> 
>>>>> In order to write such a complex test, the vmx tests framework was
>>>>> enhanced to support using VMX in non BSP CPUs. This enhancement is
>>>>> implemented in patches 1-7. The test itself is implemented at patch 8.
>>>>> This enhancement to the vmx tests framework is a bit hackish, but
>>>>> I believe it's OK because this functionality is rarely required by
>>>>> other VMX tests.
>>>>> 
>>>>> Regards,
>>>>> -Liran
>>>> 
>>>> Hi Liran,
>>>> 
>>>> I ran this test on bare-metal and it fails:
>>>> 
>>>> Test suite: vmx_init_signal_test
>>>> PASS: INIT signal blocked when CPU in VMX operation
>>>> PASS: INIT signal during VMX non-root mode result in exit-reason VMX_INIT (3)
>>>> FAIL: INIT signal processed after exit VMX operation
>>>> SUMMARY: 8 tests, 1 unexpected failures
>>>> 
>>>> I don’t have time to debug this issue, but let me know if you want some
>>>> print-outs.
>>>> 
>>>> Nadav
>>> 
>>> Thanks Nadav for running this on bare-metal. This is very useful!
>>> 
>>> It seems that when CPU exited on exit-reason VMX_INIT (3), the LAPIC INIT pending event
>>> was consumed instead of still being latched until CPU exits VMX operation.
>>> 
>>> In my commit which this unit-test verifies 4b9852f4f389 ("KVM: x86: Fix INIT signal handling in various CPU states”),
>>> I have assumed that such exit-reason don’t consume the LAPIC INIT pending event.
>>> My assumption was based on the phrasing of Intel SDM section 25.2 OTHER CAUSES OF VM EXITS regarding INIT signals:
>>> "Such exits do not modify register state or clear pending events as they would outside of VMX operation."
>>> I thought Intel logic behind this is that if an INIT signal is sent to a CPU in VMX non-root mode, it would exit
>>> on exit-reason 3 which would allow hypervisor to decide to exit VMX operation in order to consume INIT signal.
>> 
>> I think this sentence can be read differently. It also reasonable not to
>> bound the host to get an INIT signal the moment it disabled vmx.
> 
> If INIT signal won’t be kept pending until exiting VMX operation, target CPU which was sent with INIT signal
> when it was running guest, basically lost INIT signal forever and just received an exit-reason it cannot do much with.
> That’s why I thought Intel designed this mechanism like I specified above.

Well, the host can always issue an INIT using an IPI.

> 
> I also remembered to verify this behaviour against some discussions made online:
> 1) https://software.intel.com/en-us/forums/virtualization-software-development/topic/355484
> * "When the 16-bit guest issues an INIT IPI to itself using the APIC, I run into an infinite VMExit situation that my hypervisor cannot seem to recover from.”
> * "In response to the VMExit with a reason of 3 (which is expected), the hypervisor resets the 16-bit guest's registers, limits, access rights, etc. to simulate starting execution from a known initialization point.  However, it seems that as soon as the hypervisor resumes guest execution, the VMExit occurs again, repeatedly.”
> 2) https://patchwork.kernel.org/patch/2244311/
> "I actually find it very useful. On INIT vmexit hypervisor may call vmxoff and do proper reset."
> 
> Anyway, Sean, can you assist verifying inside Intel what should be the expected behaviour?

It might always be (yet) another kvm-unit-tests bug that is only apparent on
bare-metal. But if Sean can confirm what the expected behavior is, it would
save time.

I do not have an ITP, so debugging on bare-metal is not fun at all...

Re: [PATCH kvm-unit-tests 0/8]: x86: vmx: Test INIT processing in various CPU VMX states

[Sean Christopherson]

On Mon, Sep 30, 2019 at 06:29:52PM -0700, Nadav Amit wrote:
> > On Sep 30, 2019, at 6:23 PM, Liran Alon <liran.alon@oracle.com> wrote:
> > 
> > If INIT signal won’t be kept pending until exiting VMX operation, target
> > CPU which was sent with INIT signal when it was running guest, basically
> > lost INIT signal forever and just received an exit-reason it cannot do much
> > with.  That’s why I thought Intel designed this mechanism like I specified
> > above.
> 
> Well, the host can always issue an INIT using an IPI.

And conversely, if the INIT persisted then the host would be forced to do
VMXOFF, otherwise it effectively couldn't do VM-Enter on that logical CPU.

> > I also remembered to verify this behaviour against some discussions made online:
> > 1) https://software.intel.com/en-us/forums/virtualization-software-development/topic/355484
> > * "When the 16-bit guest issues an INIT IPI to itself using the APIC, I run into an infinite VMExit situation that my hypervisor cannot seem to recover from.”
> > * "In response to the VMExit with a reason of 3 (which is expected), the hypervisor resets the 16-bit guest's registers, limits, access rights, etc. to simulate starting execution from a known initialization point.  However, it seems that as soon as the hypervisor resumes guest execution, the VMExit occurs again, repeatedly.”
> > 2) https://patchwork.kernel.org/patch/2244311/
> > "I actually find it very useful. On INIT vmexit hypervisor may call vmxoff and do proper reset."
> > 
> > Anyway, Sean, can you assist verifying inside Intel what should be the expected behaviour?
> 
> It might always be (yet) another kvm-unit-tests bug that is only apparent on
> bare-metal. But if Sean can confirm what the expected behavior is, it would
> save time.
> 
> I do not have an ITP, so debugging on bare-metal is not fun at all...

My understanding of the architecture is that the INIT should be consumed
on VM-Exit.  The only scenario where an event is not consumed/acknowledge
is when a vanilla interrupt occurs without VM_EXIT_ACK_INTR_ON_EXIT set,
in which case the VM-Exit is technically considered a "pending" interrupt.
For all other cases (NMI, SMI, INIT, and INTR w/ ACK-ON-EXIT), the VM-Exit
is the end result of delivering the event.

INITs are indeed blocked and not dropped in VMX root mode.  But entering
non-root (guest) mode should unblock INITs and cause a VM-Exit, and thus
clear the INIT that was pended while in VMX root mode.  This behavior does
not conflict with the whitepaper[*] referenced by link (2) above, and in
fact the whitepaper explicitly covers guest mode behavior in a footnote:

  When the processor is in VMX guest mode, delivery of INIT causes a
  normal VMEXIT, of course.

The INIT attack described uses "VMX mode" to refer to VMX root mode, and
other than the footnote, doesn't mention VMX guest mode.  My reading of it
is that they're showing a proof of concept of based on getting the OS into
VMX root mode but not actually running a guest, e.g. this can be done
in KVM by creating a VM (KVM_CREATE_VM) but not running it (KVM_RUN).

Anyways, I'll double check that the INIT should indeed be consumed as part
of the VM-Exit.

[*] https://invisiblethingslab.com/resources/2011/Software%20Attacks%20on%20Intel%20VT-d.pdf

Re: [PATCH kvm-unit-tests 0/8]: x86: vmx: Test INIT processing in various CPU VMX states

[Sean Christopherson]

On Tue, Oct 01, 2019 at 11:40:34AM -0700, Sean Christopherson wrote:
> On Mon, Sep 30, 2019 at 06:29:52PM -0700, Nadav Amit wrote:
> > > On Sep 30, 2019, at 6:23 PM, Liran Alon <liran.alon@oracle.com> wrote:

...

> > > I also remembered to verify this behaviour against some discussions made online:
> > > 1) https://software.intel.com/en-us/forums/virtualization-software-development/topic/355484
> > > * "When the 16-bit guest issues an INIT IPI to itself using the APIC, I run into an infinite VMExit situation that my hypervisor cannot seem to recover from.”
> > > * "In response to the VMExit with a reason of 3 (which is expected), the hypervisor resets the 16-bit guest's registers, limits, access rights, etc. to simulate starting execution from a known initialization point.  However, it seems that as soon as the hypervisor resumes guest execution, the VMExit occurs again, repeatedly.”
> > > 2) https://patchwork.kernel.org/patch/2244311/
> > > "I actually find it very useful. On INIT vmexit hypervisor may call vmxoff and do proper reset."
> > > 
> > > Anyway, Sean, can you assist verifying inside Intel what should be the expected behaviour?
> > 
> > It might always be (yet) another kvm-unit-tests bug that is only apparent on
> > bare-metal. But if Sean can confirm what the expected behavior is, it would
> > save time.
> > 
> > I do not have an ITP, so debugging on bare-metal is not fun at all...
> 
> My understanding of the architecture is that the INIT should be consumed
> on VM-Exit.  The only scenario where an event is not consumed/acknowledge
> is when a vanilla interrupt occurs without VM_EXIT_ACK_INTR_ON_EXIT set,
> in which case the VM-Exit is technically considered a "pending" interrupt.
> For all other cases (NMI, SMI, INIT, and INTR w/ ACK-ON-EXIT), the VM-Exit
> is the end result of delivering the event.
> 
> INITs are indeed blocked and not dropped in VMX root mode.  But entering
> non-root (guest) mode should unblock INITs and cause a VM-Exit, and thus
> clear the INIT that was pended while in VMX root mode.  This behavior does
> not conflict with the whitepaper[*] referenced by link (2) above, and in
> fact the whitepaper explicitly covers guest mode behavior in a footnote:
> 
>   When the processor is in VMX guest mode, delivery of INIT causes a
>   normal VMEXIT, of course.
> 
> The INIT attack described uses "VMX mode" to refer to VMX root mode, and
> other than the footnote, doesn't mention VMX guest mode.  My reading of it
> is that they're showing a proof of concept of based on getting the OS into
> VMX root mode but not actually running a guest, e.g. this can be done
> in KVM by creating a VM (KVM_CREATE_VM) but not running it (KVM_RUN).
> 
> Anyways, I'll double check that the INIT should indeed be consumed as part
> of the VM-Exit.

I couldn't help but run a few tests before reaching out to the architecture
folks...

I modified KVM to have the CPU send an INIT IPI to itself in vmx_vcpu_run(),
with a bit of delay to ensure the INIT is pending prior to VM-Enter.  On an
INIT VM-Exit, KVM immediately resumes the guest.  On Haswell client system,
the INIT does indeed appear to be consumed when it's handled by VM-Exit,
i.e. KVM doesn't get stuck in an infinite INIT VM-Exits loop.

One possible explanation for the infinite loop observed in (1) above, is
that the developer didn't properly reconfigure guest state when emulating
INIT and hit a VM-Fail.  Because vmcs.EXIT_REASON isn't written on VM-Fail,
if the VMM isn't checking for VM-Fail it will think it's getting endless
INIT VM-Exits.  I did exactly this when tweaking KVM to handle INIT (forgot
to mark the VMCS as launched redoing VM-Enter), so I even inadvertantly
confirmed that it's plausible :-)

Re: [PATCH kvm-unit-tests 0/8]: x86: vmx: Test INIT processing in various CPU VMX states

[Sean Christopherson]

On Tue, Oct 01, 2019 at 11:40:34AM -0700, Sean Christopherson wrote:
> Anyways, I'll double check that the INIT should indeed be consumed as part
> of the VM-Exit.

Confirmed that the INIT is cleared prior to delivering VM-Exit.

Re: [PATCH kvm-unit-tests 0/8]: x86: vmx: Test INIT processing in various CPU VMX states

[Nadav Amit]

> On Oct 1, 2019, at 4:34 PM, Sean Christopherson <sean.j.christopherson@intel.com> wrote:
> 
> On Tue, Oct 01, 2019 at 11:40:34AM -0700, Sean Christopherson wrote:
>> Anyways, I'll double check that the INIT should indeed be consumed as part
>> of the VM-Exit.
> 
> Confirmed that the INIT is cleared prior to delivering VM-Exit.

Thanks for checking. I guess Liran will take it from here - I just wanted to
ensure kvm-unit-tests on bare-metal is not broken.

Re: [PATCH kvm-unit-tests 0/8]: x86: vmx: Test INIT processing in various CPU VMX states

[Liran Alon]

> On 2 Oct 2019, at 2:37, Nadav Amit <nadav.amit@gmail.com> wrote:
> 
>> On Oct 1, 2019, at 4:34 PM, Sean Christopherson <sean.j.christopherson@intel.com> wrote:
>> 
>> On Tue, Oct 01, 2019 at 11:40:34AM -0700, Sean Christopherson wrote:
>>> Anyways, I'll double check that the INIT should indeed be consumed as part
>>> of the VM-Exit.
>> 
>> Confirmed that the INIT is cleared prior to delivering VM-Exit.
> 
> Thanks for checking. I guess Liran will take it from here - I just wanted to
> ensure kvm-unit-tests on bare-metal is not broken.
> 

Yes, thanks everyone. I will submit a patch for both KVM and kvm-unit-tests for this.

-Liran

Re: [PATCH kvm-unit-tests 0/8]: x86: vmx: Test INIT processing in various CPU VMX states

[Liran Alon]

> On 1 Oct 2019, at 21:40, Sean Christopherson <sean.j.christopherson@intel.com> wrote:
> 
> On Mon, Sep 30, 2019 at 06:29:52PM -0700, Nadav Amit wrote:
>>> On Sep 30, 2019, at 6:23 PM, Liran Alon <liran.alon@oracle.com> wrote:
>>> 
>>> If INIT signal won’t be kept pending until exiting VMX operation, target
>>> CPU which was sent with INIT signal when it was running guest, basically
>>> lost INIT signal forever and just received an exit-reason it cannot do much
>>> with.  That’s why I thought Intel designed this mechanism like I specified
>>> above.
>> 
>> Well, the host can always issue an INIT using an IPI.
> 
> And conversely, if the INIT persisted then the host would be forced to do
> VMXOFF, otherwise it effectively couldn't do VM-Enter on that logical CPU.

The way I grasped it previously is that hypervisor have 2 different options to respond to an INIT-signal exit-reason:
1) Interpret INIT signal as suppose to be delivered to host (e.g. KVM use-case). i.e. Allow other CPU which send INIT signal to reset it. By just exiting VMX operation with VMXOFF.
2) Interpet INIT signal as suppose to be delivered to guest (e.g. A “passthrough” security hypervisor loaded during boot-chain). In this case, hypervisor would reset vCPU context and then enter guest with wait-for-SIPI activity-state. That blocks pending INIT signal from being delivered and exiting from non-root mode. Then next physical SIPI delivered to CPU will be consumed properly.

Anyway, just wanted to layout my previous thoughts on the matter.
I think Intel SDM phrasing on this regard is very confusing…

-Liran