| Message ID | 20181107212528.15022-1-jmattson@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [kvm-unit-tests] x86: nVMX: Test of IA32_TSC on VM-exit MSR-store list | expand |
> 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>
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!
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),
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(+)