[kvm-unit-tests] x86: nVMX: Test of IA32_TSC on VM-exit MSR-store list
diff mbox series

Message ID 20181107212528.15022-1-jmattson@google.com
State New
Headers show
Series
  • [kvm-unit-tests] x86: nVMX: Test of IA32_TSC on VM-exit MSR-store list
Related show

Commit Message

Jim Mattson Nov. 7, 2018, 9:25 p.m. UTC
When IA32_TSC is on the VM-exit MSR-store list, the value saved is not
subject to the "use TSC offsetting" VM-execution control for the
current VMCS.

Prior to commit e79f245ddec17 ("X86/KVM: Properly update 'tsc_offset'
to represent the running guest"), kvm did not handle this situation
properly.

Signed-off-by: Jim Mattson <jmattson@google.com>
Reviewed-by: Marc Orr <marcorr@google.com>
Reviewed-by: Peter Shier <pshier@google.com>
---
 x86/unittests.cfg |  6 ++++++
 x86/vmx.h         |  1 +
 x86/vmx_tests.c   | 45 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 52 insertions(+)

Comments

Liran Alon Nov. 7, 2018, 9:38 p.m. UTC | #1
> On 7 Nov 2018, at 23:25, Jim Mattson <jmattson@google.com> wrote:
> 
> When IA32_TSC is on the VM-exit MSR-store list, the value saved is not
> subject to the "use TSC offsetting" VM-execution control for the
> current VMCS.
> 
> Prior to commit e79f245ddec17 ("X86/KVM: Properly update 'tsc_offset'
> to represent the running guest"), kvm did not handle this situation
> properly.
> 
> Signed-off-by: Jim Mattson <jmattson@google.com>
> Reviewed-by: Marc Orr <marcorr@google.com>
> Reviewed-by: Peter Shier <pshier@google.com>
> ---
> x86/unittests.cfg |  6 ++++++
> x86/vmx.h         |  1 +
> x86/vmx_tests.c   | 45 +++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 52 insertions(+)
> 
> diff --git a/x86/unittests.cfg b/x86/unittests.cfg
> index 3b21a85..9448409 100644
> --- a/x86/unittests.cfg
> +++ b/x86/unittests.cfg
> @@ -572,6 +572,12 @@ extra_params = -cpu host,+vmx -m 2560 -append vmx_pending_event_hlt_test
> arch = x86_64
> groups = vmx
> 
> +[vmx_store_tsc_test]
> +file = vmx.flat
> +extra_params = -cpu host,+vmx -m 2560 -append vmx_store_tsc_test
> +arch = x86_64
> +groups = vmx
> +
> [vmx_db_test]
> file = vmx.flat
> extra_params = -cpu host,+vmx -m 2560 -append vmx_db_test
> diff --git a/x86/vmx.h b/x86/vmx.h
> index ba47455..8a00f73 100644
> --- a/x86/vmx.h
> +++ b/x86/vmx.h
> @@ -384,6 +384,7 @@ enum Ctrl_pin {
> 
> enum Ctrl0 {
> 	CPU_INTR_WINDOW		= 1ul << 2,
> +	CPU_USE_TSC_OFFSET	= 1ul << 3,
> 	CPU_HLT			= 1ul << 7,
> 	CPU_INVLPG		= 1ul << 9,
> 	CPU_MWAIT		= 1ul << 10,
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index b105b23..d223dea 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -5031,6 +5031,50 @@ static void vmx_pending_event_hlt_test(void)
> 	vmx_pending_event_test_core(true);
> }
> 
> +#define GUEST_TSC_OFFSET (1u << 30)
> +
> +static u64 guest_tsc;
> +
> +static void vmx_store_tsc_test_guest(void)
> +{
> +	guest_tsc = rdtsc();
> +}
> +
> +/*
> + * This test ensures that when IA32_TSC is in the VM-exit MSR-store
> + * list, the value saved is not subject to the TSC offset that is
> + * applied to RDTSC/RDTSCP/RDMSR(IA32_TSC) in guest execution.
> + */
> +static void vmx_store_tsc_test(void)
> +{
> +	struct vmx_msr_entry msr_entry = { .index = MSR_IA32_TSC };
> +	u64 low, high;
> +
> +	if (!(ctrl_cpu_rev[0].clr & CPU_USE_TSC_OFFSET)) {
> +		printf("\t'Use TSC offsetting' not supported.\n”);

Should you use report_skip() instead of printf()?

> +		return;
> +	}
> +
> +	test_set_guest(vmx_store_tsc_test_guest);
> +
> +	vmcs_set_bits(CPU_EXEC_CTRL0, CPU_USE_TSC_OFFSET);
> +	vmcs_write(EXI_MSR_ST_CNT, 1);
> +	vmcs_write(EXIT_MSR_ST_ADDR, virt_to_phys(&msr_entry));
> +	vmcs_write(TSC_OFFSET, GUEST_TSC_OFFSET);
> +
> +	low = rdtsc();
> +	enter_guest();
> +	high = rdtsc();
> +
> +	report("RDTSC value in the guest (%lu) is in range [%lu, %lu]",
> +	       low + GUEST_TSC_OFFSET <= guest_tsc &&
> +	       guest_tsc <= high + GUEST_TSC_OFFSET,
> +	       guest_tsc, low + GUEST_TSC_OFFSET, high + GUEST_TSC_OFFSET);
> +	report("IA32_TSC value saved in the VM-exit MSR-store list (%lu) is in range [%lu, %lu]",
> +	       low <= msr_entry.value && msr_entry.value <= high,
> +	       msr_entry.value, low, high);
> +}
> +
> static void vmx_db_test_guest(void)
> {
> 	/*
> @@ -5835,6 +5879,7 @@ struct vmx_test vmx_tests[] = {
> 	TEST(vmx_db_test),
> 	TEST(vmx_pending_event_test),
> 	TEST(vmx_pending_event_hlt_test),
> +	TEST(vmx_store_tsc_test),
> 	/* EPT access tests. */
> 	TEST(ept_access_test_not_present),
> 	TEST(ept_access_test_read_only),
> -- 
> 2.19.1.930.g4563a0d9d0-goog
> 

Nicely done.
Reviewed-by: Liran Alon <liran.alon@oracle.com>
Jim Mattson Nov. 7, 2018, 10:26 p.m. UTC | #2
On Wed, Nov 7, 2018 at 1:38 PM, Liran Alon <liran.alon@oracle.com> wrote:

> Should you use report_skip() instead of printf()?

Indeed. I didn't know about (or had forgotten about) report_skip. I'm
out all day tomorrow, but I'll send a V2 on Friday.

Thanks!

Patch
diff mbox series

diff --git a/x86/unittests.cfg b/x86/unittests.cfg
index 3b21a85..9448409 100644
--- a/x86/unittests.cfg
+++ b/x86/unittests.cfg
@@ -572,6 +572,12 @@  extra_params = -cpu host,+vmx -m 2560 -append vmx_pending_event_hlt_test
 arch = x86_64
 groups = vmx
 
+[vmx_store_tsc_test]
+file = vmx.flat
+extra_params = -cpu host,+vmx -m 2560 -append vmx_store_tsc_test
+arch = x86_64
+groups = vmx
+
 [vmx_db_test]
 file = vmx.flat
 extra_params = -cpu host,+vmx -m 2560 -append vmx_db_test
diff --git a/x86/vmx.h b/x86/vmx.h
index ba47455..8a00f73 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -384,6 +384,7 @@  enum Ctrl_pin {
 
 enum Ctrl0 {
 	CPU_INTR_WINDOW		= 1ul << 2,
+	CPU_USE_TSC_OFFSET	= 1ul << 3,
 	CPU_HLT			= 1ul << 7,
 	CPU_INVLPG		= 1ul << 9,
 	CPU_MWAIT		= 1ul << 10,
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index b105b23..d223dea 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -5031,6 +5031,50 @@  static void vmx_pending_event_hlt_test(void)
 	vmx_pending_event_test_core(true);
 }
 
+#define GUEST_TSC_OFFSET (1u << 30)
+
+static u64 guest_tsc;
+
+static void vmx_store_tsc_test_guest(void)
+{
+	guest_tsc = rdtsc();
+}
+
+/*
+ * This test ensures that when IA32_TSC is in the VM-exit MSR-store
+ * list, the value saved is not subject to the TSC offset that is
+ * applied to RDTSC/RDTSCP/RDMSR(IA32_TSC) in guest execution.
+ */
+static void vmx_store_tsc_test(void)
+{
+	struct vmx_msr_entry msr_entry = { .index = MSR_IA32_TSC };
+	u64 low, high;
+
+	if (!(ctrl_cpu_rev[0].clr & CPU_USE_TSC_OFFSET)) {
+		printf("\t'Use TSC offsetting' not supported.\n");
+		return;
+	}
+
+	test_set_guest(vmx_store_tsc_test_guest);
+
+	vmcs_set_bits(CPU_EXEC_CTRL0, CPU_USE_TSC_OFFSET);
+	vmcs_write(EXI_MSR_ST_CNT, 1);
+	vmcs_write(EXIT_MSR_ST_ADDR, virt_to_phys(&msr_entry));
+	vmcs_write(TSC_OFFSET, GUEST_TSC_OFFSET);
+
+	low = rdtsc();
+	enter_guest();
+	high = rdtsc();
+
+	report("RDTSC value in the guest (%lu) is in range [%lu, %lu]",
+	       low + GUEST_TSC_OFFSET <= guest_tsc &&
+	       guest_tsc <= high + GUEST_TSC_OFFSET,
+	       guest_tsc, low + GUEST_TSC_OFFSET, high + GUEST_TSC_OFFSET);
+	report("IA32_TSC value saved in the VM-exit MSR-store list (%lu) is in range [%lu, %lu]",
+	       low <= msr_entry.value && msr_entry.value <= high,
+	       msr_entry.value, low, high);
+}
+
 static void vmx_db_test_guest(void)
 {
 	/*
@@ -5835,6 +5879,7 @@  struct vmx_test vmx_tests[] = {
 	TEST(vmx_db_test),
 	TEST(vmx_pending_event_test),
 	TEST(vmx_pending_event_hlt_test),
+	TEST(vmx_store_tsc_test),
 	/* EPT access tests. */
 	TEST(ept_access_test_not_present),
 	TEST(ept_access_test_read_only),