[PATCH] x86: vmx: Verify pending LAPIC INIT event consume when exit on VMX_INIT

[PATCH] x86: vmx: Verify pending LAPIC INIT event consume when exit on VMX_INIT

[Liran Alon]

Intel SDM section 25.2 OTHER CAUSES OF VM EXITS specifies the following
on INIT signals: "Such exits do not modify register state or clear pending
events as they would outside of VMX operation."

When commit 48adfb0f2e8e ("x86: vmx: Test INIT processing during various CPU VMX states")
was applied, I interepted above Intel SDM statement such that
VMX_INIT exit don’t consume the pending LAPIC INIT event.

However, when Nadav Amit run the unit-test on a bare-metal
machine, it turned out my interpetation was wrong. i.e. VMX_INIT
exit does consume the pending LAPIC INIT event.
(See: https://www.spinics.net/lists/kvm/msg196757.html)

Therefore, fix unit-test code to behave as observed on bare-metal.
i.e. End unit-test with the following steps:
1) Exit VMX operation and verify it still continues to run properly
as pending LAPIC INIT event should have been already consumed by
VMX_INIT exit.
2) Re-enter VMX operation and send another INIT signal to keep it
blocked until exit from VMX operation.
3) Exit VMX operation and verify that pending LAPIC INIT signal
is processed when exiting VMX operation.

Fixes: 48adfb0f2e8e ("x86: vmx: Test INIT processing during various CPU VMX states")
Reported-by: Nadav Amit <nadav.amit@gmail.com>
Reviewed-by: Mihai Carabas <mihai.carabas@oracle.com>
Signed-off-by: Liran Alon <liran.alon@oracle.com>
---
 x86/vmx_tests.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 43 insertions(+), 2 deletions(-)

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index b137fc5456b8..a63dc2fafb49 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -8427,13 +8427,34 @@ static void init_signal_test_thread(void *data)
 	/* Signal that CPU exited to VMX root mode */
 	vmx_set_test_stage(5);
 
-	/* Wait for signal to exit VMX operation */
+	/* Wait for BSP CPU to signal to exit VMX operation */
 	while (vmx_get_test_stage() != 6)
 		;
 
 	/* Exit VMX operation (i.e. exec VMXOFF) */
 	vmx_off();
 
+	/*
+	 * Signal to BSP CPU that we continue as usual as INIT signal
+	 * should have been consumed by VMX_INIT exit from guest
+	 */
+	vmx_set_test_stage(7);
+
+	/* Wait for BSP CPU to signal to enter VMX operation */
+	while (vmx_get_test_stage() != 8)
+		;
+	/* Enter VMX operation (i.e. exec VMXON) */
+	_vmx_on(ap_vmxon_region);
+	/* Signal to BSP we are in VMX operation */
+	vmx_set_test_stage(9);
+
+	/* Wait for BSP CPU to send INIT signal */
+	while (vmx_get_test_stage() != 10)
+		;
+
+	/* Exit VMX operation (i.e. exec VMXOFF) */
+	vmx_off();
+
 	/*
 	 * Exiting VMX operation should result in latched
 	 * INIT signal being processed. Therefore, we should
@@ -8511,9 +8532,29 @@ static void vmx_init_signal_test(void)
 	init_signal_test_thread_continued = false;
 	vmx_set_test_stage(6);
 
+	/* Wait reasonable amount of time for other CPU to exit VMX operation */
+	delay(INIT_SIGNAL_TEST_DELAY);
+	report("INIT signal consumed on VMX_INIT exit",
+		   vmx_get_test_stage() == 7);
+	/* No point to continue if we failed at this point */
+	if (vmx_get_test_stage() != 7)
+		return;
+
+	/* Signal other CPU to enter VMX operation */
+	vmx_set_test_stage(8);
+	/* Wait for other CPU to enter VMX operation */
+	while (vmx_get_test_stage() != 9)
+		;
+
+	/* 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(10);
+
 	/*
 	 * Wait reasonable amount of time for other CPU
-	 * to run after INIT signal was processed
+	 * to exit VMX operation and process INIT signal
 	 */
 	delay(INIT_SIGNAL_TEST_DELAY);
 	report("INIT signal processed after exit VMX operation",
-- 
2.20.1

Re: [PATCH] x86: vmx: Verify pending LAPIC INIT event consume when exit on VMX_INIT

[Paolo Bonzini]

On 11/11/19 13:40, Liran Alon wrote:
> Intel SDM section 25.2 OTHER CAUSES OF VM EXITS specifies the following
> on INIT signals: "Such exits do not modify register state or clear pending
> events as they would outside of VMX operation."
> 
> When commit 48adfb0f2e8e ("x86: vmx: Test INIT processing during various CPU VMX states")
> was applied, I interepted above Intel SDM statement such that
> VMX_INIT exit don’t consume the pending LAPIC INIT event.
> 
> However, when Nadav Amit run the unit-test on a bare-metal
> machine, it turned out my interpetation was wrong. i.e. VMX_INIT
> exit does consume the pending LAPIC INIT event.
> (See: https://www.spinics.net/lists/kvm/msg196757.html)
> 
> Therefore, fix unit-test code to behave as observed on bare-metal.
> i.e. End unit-test with the following steps:
> 1) Exit VMX operation and verify it still continues to run properly
> as pending LAPIC INIT event should have been already consumed by
> VMX_INIT exit.
> 2) Re-enter VMX operation and send another INIT signal to keep it
> blocked until exit from VMX operation.
> 3) Exit VMX operation and verify that pending LAPIC INIT signal
> is processed when exiting VMX operation.
> 
> Fixes: 48adfb0f2e8e ("x86: vmx: Test INIT processing during various CPU VMX states")
> Reported-by: Nadav Amit <nadav.amit@gmail.com>
> Reviewed-by: Mihai Carabas <mihai.carabas@oracle.com>
> Signed-off-by: Liran Alon <liran.alon@oracle.com>
> ---
>  x86/vmx_tests.c | 45 +++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 43 insertions(+), 2 deletions(-)
> 
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index b137fc5456b8..a63dc2fafb49 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -8427,13 +8427,34 @@ static void init_signal_test_thread(void *data)
>  	/* Signal that CPU exited to VMX root mode */
>  	vmx_set_test_stage(5);
>  
> -	/* Wait for signal to exit VMX operation */
> +	/* Wait for BSP CPU to signal to exit VMX operation */
>  	while (vmx_get_test_stage() != 6)
>  		;
>  
>  	/* Exit VMX operation (i.e. exec VMXOFF) */
>  	vmx_off();
>  
> +	/*
> +	 * Signal to BSP CPU that we continue as usual as INIT signal
> +	 * should have been consumed by VMX_INIT exit from guest
> +	 */
> +	vmx_set_test_stage(7);
> +
> +	/* Wait for BSP CPU to signal to enter VMX operation */
> +	while (vmx_get_test_stage() != 8)
> +		;
> +	/* Enter VMX operation (i.e. exec VMXON) */
> +	_vmx_on(ap_vmxon_region);
> +	/* Signal to BSP we are in VMX operation */
> +	vmx_set_test_stage(9);
> +
> +	/* Wait for BSP CPU to send INIT signal */
> +	while (vmx_get_test_stage() != 10)
> +		;
> +
> +	/* Exit VMX operation (i.e. exec VMXOFF) */
> +	vmx_off();
> +
>  	/*
>  	 * Exiting VMX operation should result in latched
>  	 * INIT signal being processed. Therefore, we should
> @@ -8511,9 +8532,29 @@ static void vmx_init_signal_test(void)
>  	init_signal_test_thread_continued = false;
>  	vmx_set_test_stage(6);
>  
> +	/* Wait reasonable amount of time for other CPU to exit VMX operation */
> +	delay(INIT_SIGNAL_TEST_DELAY);
> +	report("INIT signal consumed on VMX_INIT exit",
> +		   vmx_get_test_stage() == 7);
> +	/* No point to continue if we failed at this point */
> +	if (vmx_get_test_stage() != 7)
> +		return;
> +
> +	/* Signal other CPU to enter VMX operation */
> +	vmx_set_test_stage(8);
> +	/* Wait for other CPU to enter VMX operation */
> +	while (vmx_get_test_stage() != 9)
> +		;
> +
> +	/* 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(10);
> +
>  	/*
>  	 * Wait reasonable amount of time for other CPU
> -	 * to run after INIT signal was processed
> +	 * to exit VMX operation and process INIT signal
>  	 */
>  	delay(INIT_SIGNAL_TEST_DELAY);
>  	report("INIT signal processed after exit VMX operation",
> 

Queued, thanks.