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@xxxxxxxxx>
> Reviewed-by: Mihai Carabas <mihai.carabas@xxxxxxxxxx>
> Signed-off-by: Liran Alon <liran.alon@xxxxxxxxxx>
> ---
> 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.
