> On 7 Nov 2018, at 23:25, Jim Mattson <jmattson@xxxxxxxxxx> 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@xxxxxxxxxx>
> Reviewed-by: Marc Orr <marcorr@xxxxxxxxxx>
> Reviewed-by: Peter Shier <pshier@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx>
