kvm: x86: add KVM_{GET,SET}_TSC_OFFSET ioctls

PATCH Superseded Oliver Upton 3 participants 9 messages Jul 10, 2020 – Jul 13, 2020

Subsystems (5)

Documentation x86 KVM Kselftest +1 more subsystems

lore

Share & tools

lore

Oliver Upton

author

2020-07-10 20:07 UTC #0 lore raw

PATCH 1/4 kvm: x86: add KVM_{GET,SET}_TSC_OFFSET ioctls

From: Peter Hornyack <[email protected]>

The KVM_SET_MSR vcpu ioctl has some temporal and value-based heuristics for determining when userspace is attempting to synchronize TSCs. Instead of guessing at userspace's intentions in the kernel, directly expose control of the TSC offset field to userspace such that userspace may deliberately synchronize the guest TSCs.

Note that TSC offset support is mandatory for KVM on both SVM and VMX.

Reviewed-by Jim Mattson <[email protected]>

Signed-off-by Peter Hornyack <[email protected]>

Signed-off-by Oliver Upton <[email protected]>

---
 Documentation/virt/kvm/api.rst | 27 +++++++++++++++++++++++++++
 arch/x86/kvm/x86.c             | 28 ++++++++++++++++++++++++++++
 include/uapi/linux/kvm.h       |  5 +++++
 3 files changed, 60 insertions(+)

Documentation/virt/kvm/api.rst +27 -0

index 644e5326aa50..ef1fc109b901 100644
@@ -4701,6 +4701,33 @@ KVM_PV_VM_VERIFY
   KVM is allowed to start protected VCPUs.
 
 
+4.126 KVM_GET_TSC_OFFSET
+------------------------
+
+:Capability: KVM_CAP_TSC_OFFSET
+:Architectures: x86
+:Type: vcpu ioctl
+:Parameters: __u64 (out)
+:Returns: 0 on success, < 0 on error
+
+This ioctl gets the TSC offset field. The offset is returned as an
+unsigned value, though it is interpreted as a signed value by hardware.
+
+
+4.127 KVM_SET_TSC_OFFSET
+------------------------
+
+:Capability: KVM_CAP_TSC_OFFSET
+:Architectures: x86
+:Type: vcpu ioctl
+:Parameters: __u64 (in)
+:Returns: 0 on success, < 0 on error
+
+This ioctl sets the TSC offset field. The offset is represented as an
+unsigned value, though it is interpreted as a signed value by hardware.
+The guest's TSC value will change based on the written offset.
+
+
 5. The kvm_run structure
 ========================

arch/x86/kvm/x86.c +28 -0

index e27d3db7e43f..563256ce5ade 100644
@@ -2026,6 +2026,11 @@ u64 kvm_read_l1_tsc(struct kvm_vcpu *vcpu, u64 host_tsc)
 }
 EXPORT_SYMBOL_GPL(kvm_read_l1_tsc);
 
+static u64 kvm_vcpu_read_tsc_offset(struct kvm_vcpu *vcpu)
+{
+	return vcpu->arch.l1_tsc_offset;
+}
+
 static void kvm_vcpu_write_tsc_offset(struct kvm_vcpu *vcpu, u64 offset)
 {
 	vcpu->arch.l1_tsc_offset = offset;

@@ -3482,6 +3487,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 	case KVM_CAP_HYPERV_TIME:
 	case KVM_CAP_IOAPIC_POLARITY_IGNORED:
 	case KVM_CAP_TSC_DEADLINE_TIMER:
+	case KVM_CAP_TSC_OFFSET:
 	case KVM_CAP_DISABLE_QUIRKS:
 	case KVM_CAP_SET_BOOT_CPU_ID:
  	case KVM_CAP_SPLIT_IRQCHIP:

@@ -4734,6 +4740,28 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 		r = 0;
 		break;
 	}
+	case KVM_GET_TSC_OFFSET: {
+		u64 tsc_offset;
+
+		r = -EFAULT;
+		tsc_offset = kvm_vcpu_read_tsc_offset(vcpu);
+		if (copy_to_user(argp, &tsc_offset, sizeof(tsc_offset)))
+			goto out;
+		r = 0;
+		break;
+	}
+	case KVM_SET_TSC_OFFSET: {
+		u64 __user *tsc_offset_arg = argp;
+		u64 tsc_offset;
+
+		r = -EFAULT;
+		if (copy_from_user(&tsc_offset, tsc_offset_arg,
+				   sizeof(tsc_offset)))
+			goto out;
+		kvm_vcpu_write_tsc_offset(vcpu, tsc_offset);
+		r = 0;
+		break;
+	}
 	default:
 		r = -EINVAL;
 	}

include/uapi/linux/kvm.h +5 -0

index ff9b335620d0..41f387ffcd11 100644
@@ -1033,6 +1033,7 @@ struct kvm_ppc_resize_hpt {
 #define KVM_CAP_HALT_POLL 182
 #define KVM_CAP_ASYNC_PF_INT 183
 #define KVM_CAP_LAST_CPU 184
+#define KVM_CAP_TSC_OFFSET 185
 
 #ifdef KVM_CAP_IRQ_ROUTING

@@ -1501,6 +1502,10 @@ struct kvm_enc_region {
 #define KVM_S390_NORMAL_RESET	_IO(KVMIO,   0xc3)
 #define KVM_S390_CLEAR_RESET	_IO(KVMIO,   0xc4)
 
+/* Available with KVM_CAP_TSC_OFFSET */
+#define KVM_GET_TSC_OFFSET	_IOR(KVMIO, 0xc5, __u64)
+#define KVM_SET_TSC_OFFSET	_IOW(KVMIO, 0xc6, __u64)
+
 struct kvm_s390_pv_sec_parm {
 	__u64 origin;
 	__u64 length;

Signature · 1 line

2.27.0.383.g050319c2ae-goog

Oliver Upton

author

2020-07-10 20:07 UTC #1 lore raw

PATCH 2/4 kvm: vmx: check tsc offsetting with nested_cpu_has()

No functional change intended.

Reviewed-by Jim Mattson <[email protected]>

Signed-off-by Oliver Upton <[email protected]>

---
 arch/x86/kvm/vmx/vmx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/x86/kvm/vmx/vmx.c +1 -1

index 2b41d987b101..fa201010dbe0 100644
@@ -1775,7 +1775,7 @@ static u64 vmx_write_l1_tsc_offset(struct kvm_vcpu *vcpu, u64 offset)
 	 * to the newly set TSC to get L2's TSC.
 	 */
 	if (is_guest_mode(vcpu) &&
-	    (vmcs12->cpu_based_vm_exec_control & CPU_BASED_USE_TSC_OFFSETTING))
+	    nested_cpu_has(vmcs12, CPU_BASED_USE_TSC_OFFSETTING))
 		g_tsc_offset = vmcs12->tsc_offset;
 
 	trace_kvm_write_tsc_offset(vcpu->vcpu_id,

Signature · 1 line

2.27.0.383.g050319c2ae-goog

Oliver Upton

author

2020-07-10 20:07 UTC #2 lore raw

PATCH 3/4 selftests: kvm: use a helper function for reading cpuid

In reply to Oliver Upton

Derived from kvm-unit-tests.

Reviewed-by Jim Mattson <[email protected]>

Signed-off-by Oliver Upton <[email protected]>

---
 .../selftests/kvm/include/x86_64/processor.h      | 15 +++++++++++++++
 .../selftests/kvm/include/x86_64/svm_util.h       |  7 ++-----
 2 files changed, 17 insertions(+), 5 deletions(-)

tools/testing/selftests/kvm/include/x86_64/processor.h +15 -0

index 82b7fe16a824..316fbd4700ef 100644
@@ -338,6 +338,21 @@ uint32_t kvm_get_cpuid_max_basic(void);
 uint32_t kvm_get_cpuid_max_extended(void);
 void kvm_get_cpu_address_width(unsigned int *pa_bits, unsigned int *va_bits);
 
+struct cpuid {
+	u32 a, b, c, d;
+};
+
+static inline struct cpuid raw_cpuid(u32 function, u32 index)
+{
+	struct cpuid r;
+
+	asm volatile("cpuid"
+		     : "=a"(r.a), "=b"(r.b), "=c"(r.c), "=d"(r.d)
+		     : "0"(function), "2"(index));
+
+	return r;
+}
+
 /*
  * Basic CPU control in CR0
  */

tools/testing/selftests/kvm/include/x86_64/svm_util.h +2 -5

index b7531c83b8ae..47a13aaee460 100644
@@ -38,12 +38,9 @@ void nested_svm_check_supported(void);
 
 static inline bool cpu_has_svm(void)
 {
-	u32 eax = 0x80000001, ecx;
+	struct cpuid r = raw_cpuid(0x80000001, 0);
 
-	asm("cpuid" :
-	    "=a" (eax), "=c" (ecx) : "0" (eax) : "ebx", "edx");
-
-	return ecx & CPUID_SVM;
+	return r.c & CPUID_SVM;
 }
 
 #endif /* SELFTEST_KVM_SVM_UTILS_H */

Signature · 1 line

2.27.0.383.g050319c2ae-goog

Oliver Upton

author

2020-07-10 20:07 UTC #3 lore raw

PATCH 4/4 selftests: kvm: introduce tsc_offset_test

In reply to Oliver Upton

Test the KVM_{GET,SET}_TSC_OFFSET ioctls. Ensure the following:
- KVM_GET_TSC_OFFSET returns the value written by KVM_SET_TSC_OFFSET
- The L1 TSC is appropriately adjusted based on the written value
- Guest manipulation of the TSC results in a changed offset in KVM_GET_TSC_OFFSET
- Modifying the TSC offset while in guest mode affects L2 guest
- Modification in guest mode is also reflected in L1

Reviewed-by Jim Mattson <[email protected]>

Signed-off-by Oliver Upton <[email protected]>

---
 tools/testing/selftests/kvm/.gitignore        |   1 +
 tools/testing/selftests/kvm/Makefile          |   1 +
 .../selftests/kvm/include/x86_64/svm_util.h   |   5 +
 .../selftests/kvm/include/x86_64/vmx.h        |   9 +
 tools/testing/selftests/kvm/lib/kvm_util.c    |   1 +
 tools/testing/selftests/kvm/lib/x86_64/vmx.c  |  11 +
 .../selftests/kvm/x86_64/tsc_offset_test.c    | 344 ++++++++++++++++++
 7 files changed, 372 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/x86_64/tsc_offset_test.c

tools/testing/selftests/kvm/.gitignore +1 -0

index 452787152748..f5608692b43a 100644
@@ -11,6 +11,7 @@
 /x86_64/set_sregs_test
 /x86_64/smm_test
 /x86_64/state_test
+/x86_64/tsc_offset_test
 /x86_64/vmx_preemption_timer_test
 /x86_64/svm_vmcall_test
 /x86_64/sync_regs_test

tools/testing/selftests/kvm/Makefile +1 -0

index 4a166588d99f..03dfaa49c3c4 100644
@@ -55,6 +55,7 @@ TEST_GEN_PROGS_x86_64 += x86_64/vmx_set_nested_state_test
 TEST_GEN_PROGS_x86_64 += x86_64/vmx_tsc_adjust_test
 TEST_GEN_PROGS_x86_64 += x86_64/xss_msr_test
 TEST_GEN_PROGS_x86_64 += x86_64/debug_regs
+TEST_GEN_PROGS_x86_64 += x86_64/tsc_offset_test
 TEST_GEN_PROGS_x86_64 += clear_dirty_log_test
 TEST_GEN_PROGS_x86_64 += demand_paging_test
 TEST_GEN_PROGS_x86_64 += dirty_log_test

tools/testing/selftests/kvm/include/x86_64/svm_util.h +5 -0

index 47a13aaee460..ef2791be3f6b 100644
@@ -43,4 +43,9 @@ static inline bool cpu_has_svm(void)
 	return r.c & CPUID_SVM;
 }
 
+static inline void vmmcall(void)
+{
+	__asm__ __volatile__("vmmcall");
+}
+
 #endif /* SELFTEST_KVM_SVM_UTILS_H */

tools/testing/selftests/kvm/include/x86_64/vmx.h +9 -0

index 16fa21ebb99c..d70bf4f350f8 100644
@@ -602,6 +602,8 @@ struct vmx_pages *vcpu_alloc_vmx(struct kvm_vm *vm, vm_vaddr_t *p_vmx_gva);
 bool prepare_for_vmx_operation(struct vmx_pages *vmx);
 void prepare_vmcs(struct vmx_pages *vmx, void *guest_rip, void *guest_rsp);
 bool load_vmcs(struct vmx_pages *vmx);
+void generic_vmx_setup(struct vmx_pages *vmx, void *guest_rip,
+		       void *guest_rsp);
 
 bool nested_vmx_supported(void);
 void nested_vmx_check_supported(void);

@@ -616,4 +618,11 @@ void nested_map_memslot(struct vmx_pages *vmx, struct kvm_vm *vm,
 void prepare_eptp(struct vmx_pages *vmx, struct kvm_vm *vm,
 		  uint32_t eptp_memslot);
 
+static inline bool cpu_has_vmx(void)
+{
+	struct cpuid r = raw_cpuid(1, 0);
+
+	return r.c & CPUID_VMX;
+}
+
 #endif /* SELFTEST_KVM_VMX_H */

tools/testing/selftests/kvm/lib/kvm_util.c +1 -0

index 74776ee228f2..1b9bd2ed58c8 100644
@@ -23,6 +23,7 @@
 static void *align(void *x, size_t size)
 {
 	size_t mask = size - 1;
+
 	TEST_ASSERT(size != 0 && !(size & (size - 1)),
 		    "size not a power of 2: %lu", size);
 	return (void *) (((size_t) x + mask) & ~mask);

tools/testing/selftests/kvm/lib/x86_64/vmx.c +11 -0

index f1e00d43eea2..a5fb3bca0219 100644
@@ -542,3 +542,14 @@ void prepare_eptp(struct vmx_pages *vmx, struct kvm_vm *vm,
 	vmx->eptp_hva = addr_gva2hva(vm, (uintptr_t)vmx->eptp);
 	vmx->eptp_gpa = addr_gva2gpa(vm, (uintptr_t)vmx->eptp);
 }
+
+void generic_vmx_setup(struct vmx_pages *vmx_pages, void *guest_rip,
+		       void *guest_rsp)
+{
+	GUEST_ASSERT(vmx_pages && vmx_pages->vmcs_gpa);
+	GUEST_ASSERT(prepare_for_vmx_operation(vmx_pages));
+	GUEST_ASSERT(load_vmcs(vmx_pages));
+	GUEST_ASSERT(vmptrstz() == vmx_pages->vmcs_gpa);
+	prepare_vmcs(vmx_pages, guest_rip, guest_rsp);
+}
+

tools/testing/selftests/kvm/x86_64/tsc_offset_test.c new file +344 -0

344 lines not on this page · raw message

Signature · 1 line

2.27.0.383.g050319c2ae-goog

Paolo Bonzini

maintainer

2020-07-10 20:38 UTC #4 lore raw

In reply to Oliver Upton

Quoted 10 lines · Oliver Upton

From: Peter Hornyack <[email protected]>
The KVM_SET_MSR vcpu ioctl has some temporal and value-based heuristics for determining when userspace is attempting to synchronize TSCs. Instead of guessing at userspace's intentions in the kernel, directly expose control of the TSC offset field to userspace such that userspace may deliberately synchronize the guest TSCs.
Note that TSC offset support is mandatory for KVM on both SVM and VMX.

Quoted 8 lines

Reviewed-by Jim Mattson <[email protected]>
Signed-off-by Peter Hornyack <[email protected]>
Signed-off-by Oliver Upton <[email protected]>
---
 Documentation/virt/kvm/api.rst | 27 +++++++++++++++++++++++++++
 arch/x86/kvm/x86.c             | 28 ++++++++++++++++++++++++++++
 include/uapi/linux/kvm.h       |  5 +++++
 3 files changed, 60 insertions(+)

Needless to say, a patch that comes with tests starts on the fast lane.
But I have a fundamental question that isn't answered by either the
test or the documentation: how should KVM_SET_TSC_OFFSET be used _in practice_ by a VMM?

Thanks,

Paolo

Jim Mattson

2020-07-10 22:09 UTC #5 lore raw

Quoted 21 lines · Paolo Bonzini

Quoted 10 lines · Oliver Upton
From: Peter Hornyack <[email protected]>
The KVM_SET_MSR vcpu ioctl has some temporal and value-based heuristics for determining when userspace is attempting to synchronize TSCs. Instead of guessing at userspace's intentions in the kernel, directly expose control of the TSC offset field to userspace such that userspace may deliberately synchronize the guest TSCs.
Note that TSC offset support is mandatory for KVM on both SVM and VMX.
Quoted 8 lines
Reviewed-by Jim Mattson <[email protected]>
Signed-off-by Peter Hornyack <[email protected]>
Signed-off-by Oliver Upton <[email protected]>
---
 Documentation/virt/kvm/api.rst | 27 +++++++++++++++++++++++++++
 arch/x86/kvm/x86.c             | 28 ++++++++++++++++++++++++++++
 include/uapi/linux/kvm.h       |  5 +++++
 3 files changed, 60 insertions(+)

Quoted 4 lines

Needless to say, a patch that comes with tests starts on the fast lane.
But I have a fundamental question that isn't answered by either the
test or the documentation: how should KVM_SET_TSC_OFFSET be used _in practice_ by a VMM?

One could either omit IA32_TIME_STAMP_COUNTER from KVM_SET_MSRS, or one could call KVM_SET_TSC_OFFSET after KVM_SET_MSRS. We do the former.

This isn't the only undocumented dependency among the various KVM_SET_* calls, but I agree that it would be helpful to document it.

Oliver Upton

author

2020-07-10 22:37 UTC #6 lore raw

Quoted 28 lines · Jim Mattson

Quoted 21 lines · Paolo Bonzini
Quoted 10 lines · Oliver Upton
From: Peter Hornyack <[email protected]>
The KVM_SET_MSR vcpu ioctl has some temporal and value-based heuristics for determining when userspace is attempting to synchronize TSCs. Instead of guessing at userspace's intentions in the kernel, directly expose control of the TSC offset field to userspace such that userspace may deliberately synchronize the guest TSCs.
Note that TSC offset support is mandatory for KVM on both SVM and VMX.
Quoted 8 lines
Reviewed-by Jim Mattson <[email protected]>
Signed-off-by Peter Hornyack <[email protected]>
Signed-off-by Oliver Upton <[email protected]>
---
 Documentation/virt/kvm/api.rst | 27 +++++++++++++++++++++++++++
 arch/x86/kvm/x86.c             | 28 ++++++++++++++++++++++++++++
 include/uapi/linux/kvm.h       |  5 +++++
 3 files changed, 60 insertions(+)
Quoted 4 lines
Needless to say, a patch that comes with tests starts on the fast lane.
But I have a fundamental question that isn't answered by either the
test or the documentation: how should KVM_SET_TSC_OFFSET be used _in practice_ by a VMM?

Quoted 6 lines

One could either omit IA32_TIME_STAMP_COUNTER from KVM_SET_MSRS, or one could call KVM_SET_TSC_OFFSET after KVM_SET_MSRS. We do the former.
This isn't the only undocumented dependency among the various KVM_SET_* calls, but I agree that it would be helpful to document it.

Sorry, I was AFK for a bit but it seems Jim beat me to the punch :-)

I'd be glad to add a bit of color on this to the documentation.

Paolo Bonzini

maintainer

2020-07-10 22:40 UTC #7 lore raw

In reply to Jim Mattson

Quoted 7 lines · Jim Mattson

Quoted 3 lines
But I have a fundamental question that isn't answered by either the
test or the documentation: how should KVM_SET_TSC_OFFSET be used _in practice_ by a VMM?
One could either omit IA32_TIME_STAMP_COUNTER from KVM_SET_MSRS, or one could call KVM_SET_TSC_OFFSET after KVM_SET_MSRS. We do the former.

Oliver Upton

author

2020-07-13 19:55 UTC #8 lore raw

Quoted 6 lines · Paolo Bonzini

Quoted 4 lines · Jim Mattson
Quoted 3 lines
But I have a fundamental question that isn't answered by either the
test or the documentation: how should KVM_SET_TSC_OFFSET be used _in practice_ by a VMM?

Quoted 8 lines

Quoted 3 lines
One could either omit IA32_TIME_STAMP_COUNTER from KVM_SET_MSRS, or one could call KVM_SET_TSC_OFFSET after KVM_SET_MSRS. We do the former.
Other questions:
1) how do you handle non-synchronized TSC between source and destination?

On the source side we record a host TSC reading establishing the start of migration, along with the TSC offsets. We recalculate new offsets that compensate for the elapsed time + TSC difference between source/target when restoring the VM.

Quoted 2 lines

2) how is KVM_REQ_MASTERCLOCK_UPDATE triggered, since that's the main function of the various heuristics?

We initialize all TSCs to 0 at VM creation, then restore offsets (if any). Should the offsets be out of phase, we write a garbage value to the TSC MSR to break the vCPU matching, then set the offset. Not the most elegant solution, it'd be better if the {GET,SET}_TSC_OFFSET ioctls instrumented this directly.

Quoted 3 lines

Paolo