Skip to contentSkip to conversation

x86/kvm/hyperv: stable clocksorce for L2 guests when running nested KVM on Hyper-V

PATCH Superseded Vitaly Kuznetsov 3 participants 17 messages Dec 13, 2017 – Jan 15, 2018

Subsystems (5)

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

v2 cover x86/kvm/hyperv: stable clocksorce for L2 guests when running nested KVM on Hyper-V

Changes since v1:
- PATCH5 "x86/irq: Count Hyper-V reenlightenment interrupts" added [Michael Kelley]
- PATCH3 "x86/hyper-v: reenlightenment notifications support" changes:
- rename register_hv_tsc_update/unregister_hv_tsc_update() to set_hv_tscchange_cb()/clear_hv_tscchange_cb() to avoid the confusion with notification chains. [Roman Kagan]
- pr_warn() on callback registration when reenlightenment feature is not available [Roman Kagan]
- don't issue the callback when TSC accesses are not emulated [Roman Kagan]
- check for TSC frequency MSRs availability [Roman Kagan]
- allocate HYPERV_REENLIGHTENMENT_VECTOR dynamically from ms_hyperv_init_platform() to support future conditional output in /proc/interrupts

Original description:

Currently, KVM passes PVCLOCK_TSC_STABLE_BIT to its guests when running in so called 'masterclock' mode and this is only possible when the clocksource on the host is TSC. When running nested on Hyper-V we're using a different clocksource in L1 (Hyper-V TSC Page) which can actually be used for masterclock. This series brings the required support.

Making KVM work with TSC page clocksource is relatively easy, it is done in PATCH 5 of the series. All the rest is required to support L1 migration when TSC frequency changes, we use a special feature from Hyper-V to do the job.

Vitaly Kuznetsov (7):
x86/hyper-v: check for required priviliges in hyperv_init() x86/hyper-v: add a function to read both TSC and TSC page value
simulateneously
x86/hyper-v: reenlightenment notifications support x86/hyper-v: redirect reenlightment notifications on CPU offlining x86/irq: Count Hyper-V reenlightenment interrupts x86/kvm: pass stable clocksource to guests when running nested on
Hyper-V
x86/kvm: support Hyper-V reenlightenment

 arch/x86/entry/entry_32.S          |   3 +
 arch/x86/entry/entry_64.S          |   3 +
 arch/x86/hyperv/hv_init.c          | 133 ++++++++++++++++++++++++++++++++++-
 arch/x86/include/asm/hardirq.h     |   3 +
 arch/x86/include/asm/irq_vectors.h |   7 +-
 arch/x86/include/asm/mshyperv.h    |  32 +++++++--
 arch/x86/include/uapi/asm/hyperv.h |  27 ++++++++
 arch/x86/kernel/cpu/mshyperv.c     |   6 ++
 arch/x86/kernel/irq.c              |   9 +++
 arch/x86/kvm/x86.c                 | 138 ++++++++++++++++++++++++++++++-------
 10 files changed, 329 insertions(+), 32 deletions(-)
Signature · 1 line
2.14.3
#1 lore raw

PATCH v2 1/7 x86/hyper-v: check for required priviliges in hyperv_init()

In hyperv_init() we presume we always have access to VP index and hypercall MSRs while according to the specification we should check if we're allowed to access the corresponding MSRs before accessing them.

---
 arch/x86/hyperv/hv_init.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
arch/x86/hyperv/hv_init.c +8 -1
index 189a398290db..21f9d53d9f00 100644
@@ -110,12 +110,19 @@ static int hv_cpu_init(unsigned int cpu)
  */
 void hyperv_init(void)
 {
-	u64 guest_id;
+	u64 guest_id, required_msrs;
 	union hv_x64_msr_hypercall_contents hypercall_msr;
 
 	if (x86_hyper_type != X86_HYPER_MS_HYPERV)
 		return;
 
+	/* Absolutely required MSRs */
+	required_msrs = HV_X64_MSR_HYPERCALL_AVAILABLE |
+		HV_X64_MSR_VP_INDEX_AVAILABLE;
+
+	if ((ms_hyperv.features & required_msrs) != required_msrs)
+		return;
+
 	/* Allocate percpu VP index */
 	hv_vp_index = kmalloc_array(num_possible_cpus(), sizeof(*hv_vp_index),
 				    GFP_KERNEL);
Signature · 1 line
2.14.3
Reviewed-by on v2 1/7
#2 lore raw

Re: [PATCH v2 1/7] x86/hyper-v: check for required priviliges in hyperv_init()

Quoted 5 lines · Vitaly Kuznetsov

In hyperv_init() we presume we always have access to VP index and hypercall MSRs while according to the specification we should check if we're allowed to access the corresponding MSRs before accessing them.

Quoted 33 lines
---
 arch/x86/hyperv/hv_init.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
arch/x86/hyperv/hv_init.c +8 -1
index 189a398290db..21f9d53d9f00 100644
@@ -110,12 +110,19 @@ static int hv_cpu_init(unsigned int cpu)
  */
 void hyperv_init(void)
 {
-	u64 guest_id;
+	u64 guest_id, required_msrs;
 	union hv_x64_msr_hypercall_contents hypercall_msr;
 
 	if (x86_hyper_type != X86_HYPER_MS_HYPERV)
 		return;
 
+	/* Absolutely required MSRs */
+	required_msrs = HV_X64_MSR_HYPERCALL_AVAILABLE |
+		HV_X64_MSR_VP_INDEX_AVAILABLE;
+
+	if ((ms_hyperv.features & required_msrs) != required_msrs)
+		return;
+
 	/* Allocate percpu VP index */
 	hv_vp_index = kmalloc_array(num_possible_cpus(), sizeof(*hv_vp_index),
 				    GFP_KERNEL);
Signature · 1 line
2.14.3
#3 lore raw

PATCH v2 2/7 x86/hyper-v: add a function to read both TSC and TSC page value simulateneously

In reply to Vitaly Kuznetsov

This is going to be used from KVM code where we need to get both TSC and TSC page value.

When Hyper-V code is compiled out just return rdtsc(), this will allow us to avoid ugly ifdefs in non-Hyper-V code.

---
 arch/x86/hyperv/hv_init.c       |  1 +
 arch/x86/include/asm/mshyperv.h | 23 +++++++++++++++++++----
 2 files changed, 20 insertions(+), 4 deletions(-)
arch/x86/hyperv/hv_init.c +1 -0
index 21f9d53d9f00..1a6c63f721bc 100644
@@ -37,6 +37,7 @@ struct ms_hyperv_tsc_page *hv_get_tsc_page(void)
 {
 	return tsc_pg;
 }
+EXPORT_SYMBOL_GPL(hv_get_tsc_page);
 
 static u64 read_hv_clock_tsc(struct clocksource *arg)
 {
arch/x86/include/asm/mshyperv.h +19 -4
index 5400add2885b..a0b34994f453 100644
@@ -323,9 +323,10 @@ static inline void hyperv_setup_mmu_ops(void) {}
 
 #ifdef CONFIG_HYPERV_TSCPAGE
 struct ms_hyperv_tsc_page *hv_get_tsc_page(void);
-static inline u64 hv_read_tsc_page(const struct ms_hyperv_tsc_page *tsc_pg)
+static inline u64 hv_read_tsc_page_tsc(const struct ms_hyperv_tsc_page *tsc_pg,
+				       u64 *cur_tsc)
 {
-	u64 scale, offset, cur_tsc;
+	u64 scale, offset;
 	u32 sequence;
 
 	/*
@@ -356,7 +357,7 @@ static inline u64 hv_read_tsc_page(const struct ms_hyperv_tsc_page *tsc_pg)
 
 		scale = READ_ONCE(tsc_pg->tsc_scale);
 		offset = READ_ONCE(tsc_pg->tsc_offset);
-		cur_tsc = rdtsc_ordered();
+		*cur_tsc = rdtsc_ordered();
 
 		/*
 		 * Make sure we read sequence after we read all other values
@@ -366,7 +367,14 @@ static inline u64 hv_read_tsc_page(const struct ms_hyperv_tsc_page *tsc_pg)
 
 	} while (READ_ONCE(tsc_pg->tsc_sequence) != sequence);
 
-	return mul_u64_u64_shr(cur_tsc, scale, 64) + offset;
+	return mul_u64_u64_shr(*cur_tsc, scale, 64) + offset;
+}
+
+static inline u64 hv_read_tsc_page(const struct ms_hyperv_tsc_page *tsc_pg)
+{
+	u64 cur_tsc;
+
+	return hv_read_tsc_page_tsc(tsc_pg, &cur_tsc);
 }
 
 #else
@@ -374,5 +382,12 @@ static inline struct ms_hyperv_tsc_page *hv_get_tsc_page(void)
 {
 	return NULL;
 }
+
+static inline u64 hv_read_tsc_page_tsc(const struct ms_hyperv_tsc_page *tsc_pg,
+				       u64 *cur_tsc)
+{
+	BUG();
+	return U64_MAX;
+}
 #endif
 #endif
Signature · 1 line
2.14.3
#4 lore raw

Re: [PATCH v2 2/7] x86/hyper-v: add a function to read both TSC and TSC page value simulateneously

Quoted 5 lines · Vitaly Kuznetsov

This is going to be used from KVM code where we need to get both TSC and TSC page value.

When Hyper-V code is compiled out just return rdtsc(), this will allow us to avoid ugly ifdefs in non-Hyper-V code.

That's not what the patch implements....

arch/x86/include/asm/mshyperv.h static inline struct ms_hyperv_tsc_page *hv_get_tsc_page(void) · line 391

Quoted 6 lines
arch/x86/include/asm/mshyperv.h +6 -0
+static inline u64 hv_read_tsc_page_tsc(const struct ms_hyperv_tsc_page *tsc_pg,
+				       u64 *cur_tsc)
+{
+	BUG();
+	return U64_MAX;
+}

Thanks,

tglx

#5 lore raw

Re: [PATCH v2 2/7] x86/hyper-v: add a function to read both TSC and TSC page value simulateneously

Quoted 10 lines · Thomas Gleixner
Quoted 5 lines · Vitaly Kuznetsov

This is going to be used from KVM code where we need to get both TSC and TSC page value.

When Hyper-V code is compiled out just return rdtsc(), this will allow us to avoid ugly ifdefs in non-Hyper-V code.

That's not what the patch implements....

Not anymore, thanks, will fix the description.

Quoted 6 lines
Quoted 6 lines
arch/x86/include/asm/mshyperv.h +6 -0
+static inline u64 hv_read_tsc_page_tsc(const struct ms_hyperv_tsc_page *tsc_pg,
+				       u64 *cur_tsc)
+{
+	BUG();
+	return U64_MAX;
+}
Signature · 1 line
  Vitaly
#6 lore raw

PATCH v2 3/7 x86/hyper-v: reenlightenment notifications support

In reply to Vitaly Kuznetsov

Hyper-V supports Live Migration notification. This is supposed to be used in conjunction with TSC emulation: when we are migrated to a host with different TSC frequency for some short period host emulates our accesses to TSC and sends us an interrupt to notify about the event. When we're done updating everything we can disable TSC emulation and everything will start working fast again.

We didn't need these notifications before as Hyper-V guests are not supposed to use TSC as a clocksource: in Linux we even mark it as unstable on boot. Guests normally use 'tsc page' clocksouce and host updates its values on migrations automatically.

Things change when we want to run nested virtualization: even when we pass through PV clocksources (kvm-clock or tsc page) to our guests we need to know TSC frequency and when it changes.

Hyper-V Top Level Functional Specification (as of v5.0b) wrongly specifies EAX:BIT(12) of CPUID:0x40000009 as the feature identification bit. The right one to check is EAX:BIT(13) of CPUID:0x40000003. I was assured that the fix in on the way.

---
v1 -> v2:
- rename register_hv_tsc_update/unregister_hv_tsc_update() to set_hv_tscchange_cb()/clear_hv_tscchange_cb() to avoid the confusion with notification chains. [Roman Kagan]
- pr_warn() on callback registration when reenlightenment feature is not available [Roman Kagan]
- don't issue the callback when TSC accesses are not emulated [Roman Kagan]
- check for TSC frequency MSRs availability [Roman Kagan]
- allocate HYPERV_REENLIGHTENMENT_VECTOR dynamically from ms_hyperv_init_platform() to support future conditional output in /proc/interrupts

---
 arch/x86/entry/entry_32.S          |  3 ++
 arch/x86/entry/entry_64.S          |  3 ++
 arch/x86/hyperv/hv_init.c          | 89 ++++++++++++++++++++++++++++++++++++++
 arch/x86/include/asm/irq_vectors.h |  7 ++-
 arch/x86/include/asm/mshyperv.h    |  9 ++++
 arch/x86/include/uapi/asm/hyperv.h | 27 ++++++++++++
 arch/x86/kernel/cpu/mshyperv.c     |  6 +++
 7 files changed, 143 insertions(+), 1 deletion(-)
arch/x86/entry/entry_32.S +3 -0
index 4838037f97f6..7351879c6d2c 100644
@@ -882,6 +882,9 @@ BUILD_INTERRUPT3(xen_hvm_callback_vector, HYPERVISOR_CALLBACK_VECTOR,
 BUILD_INTERRUPT3(hyperv_callback_vector, HYPERVISOR_CALLBACK_VECTOR,
 		 hyperv_vector_handler)
 
+BUILD_INTERRUPT3(hyperv_reenlightenment_vector, HYPERV_REENLIGHTENMENT_VECTOR,
+		 hyperv_reenlightenment_intr)
+
 #endif /* CONFIG_HYPERV */
 
 ENTRY(page_fault)
arch/x86/entry/entry_64.S +3 -0
index f81d50d7ceac..d8423efe7f50 100644
@@ -1078,6 +1078,9 @@ apicinterrupt3 HYPERVISOR_CALLBACK_VECTOR \
 #if IS_ENABLED(CONFIG_HYPERV)
 apicinterrupt3 HYPERVISOR_CALLBACK_VECTOR \
 	hyperv_callback_vector hyperv_vector_handler
+
+apicinterrupt3 HYPERV_REENLIGHTENMENT_VECTOR \
+	hyperv_reenlightenment_vector hyperv_reenlightenment_intr
 #endif /* CONFIG_HYPERV */
 
 idtentry debug			do_debug		has_error_code=0	paranoid=1 shift_ist=DEBUG_STACK
arch/x86/hyperv/hv_init.c +89 -0

101 lines not on this page · raw message

arch/x86/include/asm/irq_vectors.h +6 -1
index 67421f649cfa..e71c1120426b 100644
@@ -103,7 +103,12 @@
 #endif
 
 #define MANAGED_IRQ_SHUTDOWN_VECTOR	0xef
-#define LOCAL_TIMER_VECTOR		0xee
+
+#if IS_ENABLED(CONFIG_HYPERV)
+#define HYPERV_REENLIGHTENMENT_VECTOR	0xee
+#endif
+
+#define LOCAL_TIMER_VECTOR		0xed
 
 #define NR_VECTORS			 256
 
arch/x86/include/asm/mshyperv.h +9 -0
index a0b34994f453..f59e21417f51 100644
@@ -159,6 +159,7 @@ static inline void vmbus_signal_eom(struct hv_message *msg, u32 old_msg_type)
 #define hv_set_synint_state(int_num, val) wrmsrl(int_num, val)
 
 void hyperv_callback_vector(void);
+void hyperv_reenlightenment_vector(void);
 #ifdef CONFIG_TRACING
 #define trace_hyperv_callback_vector hyperv_callback_vector
 #endif
@@ -314,11 +315,19 @@ void hyper_alloc_mmu(void);
 void hyperv_report_panic(struct pt_regs *regs, long err);
 bool hv_is_hypercall_page_setup(void);
 void hyperv_cleanup(void);
+
+void hyperv_reenlightenment_intr(struct pt_regs *regs);
+void set_hv_tscchange_cb(void (*cb)(void));
+void clear_hv_tscchange_cb(void);
+void hyperv_stop_tsc_emulation(void);
 #else /* CONFIG_HYPERV */
 static inline void hyperv_init(void) {}
 static inline bool hv_is_hypercall_page_setup(void) { return false; }
 static inline void hyperv_cleanup(void) {}
 static inline void hyperv_setup_mmu_ops(void) {}
+static inline void set_hv_tscchange_cb(void (*cb)(void)) {}
+static inline void clear_hv_tscchange_cb(void) {}
+static inline void hyperv_stop_tsc_emulation(void) {};
 #endif /* CONFIG_HYPERV */
 
 #ifdef CONFIG_HYPERV_TSCPAGE
arch/x86/include/uapi/asm/hyperv.h +27 -0
index 1a5bfead93b4..197c2e6c7376 100644
@@ -40,6 +40,9 @@
  */
 #define HV_X64_ACCESS_FREQUENCY_MSRS		(1 << 11)
 
+/* AccessReenlightenmentControls privilege */
+#define HV_X64_ACCESS_REENLIGHTENMENT		BIT(13)
+
 /*
  * Basic SynIC MSRs (HV_X64_MSR_SCONTROL through HV_X64_MSR_EOM
  * and HV_X64_MSR_SINT0 through HV_X64_MSR_SINT15) available
@@ -234,6 +237,30 @@
 #define HV_X64_MSR_CRASH_PARAMS		\
 		(1 + (HV_X64_MSR_CRASH_P4 - HV_X64_MSR_CRASH_P0))
 
+/* TSC emulation after migration */
+#define HV_X64_MSR_REENLIGHTENMENT_CONTROL	0x40000106
+
+struct hv_reenlightenment_control {
+	u64 vector:8;
+	u64 reserved1:8;
+	u64 enabled:1;
+	u64 reserved2:15;
+	u64 target_vp:32;
+};
+
+#define HV_X64_MSR_TSC_EMULATION_CONTROL	0x40000107
+#define HV_X64_MSR_TSC_EMULATION_STATUS		0x40000108
+
+struct hv_tsc_emulation_control {
+	u64 enabled:1;
+	u64 reserved:63;
+};
+
+struct hv_tsc_emulation_status {
+	u64 inprogress:1;
+	u64 reserved:63;
+};
+
 #define HV_X64_MSR_HYPERCALL_ENABLE		0x00000001
 #define HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_SHIFT	12
 #define HV_X64_MSR_HYPERCALL_PAGE_ADDRESS_MASK	\
arch/x86/kernel/cpu/mshyperv.c +6 -0
index 85eb5fc180c8..9340f41ce8d3 100644
@@ -251,6 +251,12 @@ static void __init ms_hyperv_init_platform(void)
 	hyperv_setup_mmu_ops();
 	/* Setup the IDT for hypervisor callback */
 	alloc_intr_gate(HYPERVISOR_CALLBACK_VECTOR, hyperv_callback_vector);
+
+	/* Setup the IDT for reenlightenment notifications */
+	if (ms_hyperv.features & HV_X64_ACCESS_REENLIGHTENMENT)
+		alloc_intr_gate(HYPERV_REENLIGHTENMENT_VECTOR,
+				hyperv_reenlightenment_vector);
+
 #endif
 }
 
Signature · 1 line
2.14.3
#7 lore raw

Re: [PATCH v2 3/7] x86/hyper-v: reenlightenment notifications support

Quoted 1 line · Vitaly Kuznetsov

+void hyperv_reenlightenment_intr(struct pt_regs *regs)

Lacks __visible and __irq_entry annotations.

Thanks,

tglx

#8 lore raw

PATCH v2 4/7 x86/hyper-v: redirect reenlightment notifications on CPU offlining

In reply to Vitaly Kuznetsov

It is very unlikely for CPUs to get offlined when we run on Hyper-V as we have a protection in vmbus module which prevents it when we have any VMBus devices assigned. This, however, may change in future if an option to reassign an already active channel will be added. It is also possible to run without any Hyper-V devices of have a CPU with no assigned channels.

Reassign reenlightenment notifications to some other active CPU when the CPU which is assigned to get them goes offline.

---
 arch/x86/hyperv/hv_init.c | 32 +++++++++++++++++++++++++++++++-
 1 file changed, 31 insertions(+), 1 deletion(-)
arch/x86/hyperv/hv_init.c +31 -1
index 347be4de3edd..38118ca38d97 100644
@@ -191,6 +191,36 @@ void clear_hv_tscchange_cb(void)
 }
 EXPORT_SYMBOL_GPL(clear_hv_tscchange_cb);
 
+static int hv_cpu_die(unsigned int cpu)
+{
+	struct hv_reenlightenment_control re_ctrl;
+	int i;
+	static DEFINE_SPINLOCK(lock);
+
+	if (hv_reenlightenment_cb == NULL)
+		return 0;
+
+	/* Make sure the CPU we migrate to is not going away too */
+	spin_lock(&lock);
+	rdmsrl(HV_X64_MSR_REENLIGHTENMENT_CONTROL, *((u64 *)&re_ctrl));
+	if (re_ctrl.target_vp == hv_vp_index[cpu]) {
+		/* Find some other online CPU */
+		for_each_online_cpu(i) {
+			if (i == cpu)
+				continue;
+
+			re_ctrl.target_vp = hv_vp_index[i];
+			wrmsrl(HV_X64_MSR_REENLIGHTENMENT_CONTROL,
+			       *((u64 *)&re_ctrl));
+
+			break;
+		}
+	}
+	spin_unlock(&lock);
+
+	return 0;
+}
+
 /*
  * This function is to be invoked early in the boot sequence after the
  * hypervisor has been detected.
@@ -220,7 +250,7 @@ void hyperv_init(void)
 		return;
 
 	if (cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "x86/hyperv_init:online",
-			      hv_cpu_init, NULL) < 0)
+			      hv_cpu_init, hv_cpu_die) < 0)
 		goto free_vp_index;
 
 	/*
Signature · 1 line
2.14.3
#9 lore raw

Re: [PATCH v2 4/7] x86/hyper-v: redirect reenlightment notifications on CPU offlining

arch/x86/hyperv/hv_init.c void clear_hv_tscchange_cb(void) · line 204

Quoted 11 lines · Vitaly Kuznetsov
arch/x86/hyperv/hv_init.c +11 -0
+static int hv_cpu_die(unsigned int cpu)
+{
+	struct hv_reenlightenment_control re_ctrl;
+	int i;
+	static DEFINE_SPINLOCK(lock);
+
+	if (hv_reenlightenment_cb == NULL)
+		return 0;
+
+	/* Make sure the CPU we migrate to is not going away too */
+	spin_lock(&lock);

What kind of voodoo is this? CPU hotplug is serialized already...

arch/x86/hyperv/hv_init.c void clear_hv_tscchange_cb(void) · line 208

Quoted 4 lines
arch/x86/hyperv/hv_init.c +4 -0
+	rdmsrl(HV_X64_MSR_REENLIGHTENMENT_CONTROL, *((u64 *)&re_ctrl));
+	if (re_ctrl.target_vp == hv_vp_index[cpu]) {
+		/* Find some other online CPU */
+		for_each_online_cpu(i) {
  		cpu = cpumask_any_but(cpu_online_mask);

Hmm?

Thanks,

tglx

#10 lore raw

Re: [PATCH v2 4/7] x86/hyper-v: redirect reenlightment notifications on CPU offlining

Quoted 15 lines · Thomas Gleixner

arch/x86/hyperv/hv_init.c void clear_hv_tscchange_cb(void) · line 204

Quoted 11 lines · Vitaly Kuznetsov
arch/x86/hyperv/hv_init.c +11 -0
+static int hv_cpu_die(unsigned int cpu)
+{
+	struct hv_reenlightenment_control re_ctrl;
+	int i;
+	static DEFINE_SPINLOCK(lock);
+
+	if (hv_reenlightenment_cb == NULL)
+		return 0;
+
+	/* Make sure the CPU we migrate to is not going away too */
+	spin_lock(&lock);

What kind of voodoo is this? CPU hotplug is serialized already...

Yes, someone around made a comment 'what happens if some day we'll have parallel cpu hot[un]plug' and I added this. Not really needed, will drop in v3.

Quoted 9 lines

arch/x86/hyperv/hv_init.c void clear_hv_tscchange_cb(void) · line 208

Quoted 4 lines
arch/x86/hyperv/hv_init.c +4 -0
+	rdmsrl(HV_X64_MSR_REENLIGHTENMENT_CONTROL, *((u64 *)&re_ctrl));
+	if (re_ctrl.target_vp == hv_vp_index[cpu]) {
+		/* Find some other online CPU */
+		for_each_online_cpu(i) {
  		cpu = cpumask_any_but(cpu_online_mask);

Hmm?

Cool, thanks)

Signature · 1 line
  Vitaly
#11 lore raw

PATCH v2 5/7 x86/irq: Count Hyper-V reenlightenment interrupts

In reply to Vitaly Kuznetsov

Hyper-V reenlightenment interrupts arrive when the VM is migrated, we're not supposed to see many of them. However, it may be important to know that the event has happened in case we have L2 nested guests.

---
 arch/x86/hyperv/hv_init.c      | 2 ++
 arch/x86/include/asm/hardirq.h | 3 +++
 arch/x86/kernel/irq.c          | 9 +++++++++
 3 files changed, 14 insertions(+)
arch/x86/hyperv/hv_init.c +2 -0
index 38118ca38d97..6b59386746f5 100644
@@ -147,6 +147,8 @@ void hyperv_reenlightenment_intr(struct pt_regs *regs)
 {
 	entering_ack_irq();
 
+	inc_irq_stat(irq_hv_reenlightenment_count);
+
 	schedule_delayed_work(&hv_reenlightenment_work, HZ/10);
 
 	exiting_irq();
arch/x86/include/asm/hardirq.h +3 -0
index 51cc979dd364..7c341a74ec8c 100644
@@ -38,6 +38,9 @@ typedef struct {
 #if IS_ENABLED(CONFIG_HYPERV) || defined(CONFIG_XEN)
 	unsigned int irq_hv_callback_count;
 #endif
+#if IS_ENABLED(CONFIG_HYPERV)
+	unsigned int irq_hv_reenlightenment_count;
+#endif
 } ____cacheline_aligned irq_cpustat_t;
 
 DECLARE_PER_CPU_SHARED_ALIGNED(irq_cpustat_t, irq_stat);
arch/x86/kernel/irq.c +9 -0
index 49cfd9fe7589..8aac86b5c4d8 100644
@@ -141,6 +141,15 @@ int arch_show_interrupts(struct seq_file *p, int prec)
 				   irq_stats(j)->irq_hv_callback_count);
 		seq_puts(p, "  Hypervisor callback interrupts\n");
 	}
+#endif
+#if IS_ENABLED(CONFIG_HYPERV)
+	if (test_bit(HYPERV_REENLIGHTENMENT_VECTOR, system_vectors)) {
+		seq_printf(p, "%*s: ", prec, "HRE");
+		for_each_online_cpu(j)
+			seq_printf(p, "%10u ",
+				   irq_stats(j)->irq_hv_reenlightenment_count);
+		seq_puts(p, "  Hyper-V reenlightenment interrupts\n");
+	}
 #endif
 	seq_printf(p, "%*s: %10u\n", prec, "ERR", atomic_read(&irq_err_count));
 #if defined(CONFIG_X86_IO_APIC)
Signature · 1 line
2.14.3
#12 lore raw

PATCH v2 6/7 x86/kvm: pass stable clocksource to guests when running nested on Hyper-V

In reply to Vitaly Kuznetsov

Currently, KVM is able to work in 'masterclock' mode passing PVCLOCK_TSC_STABLE_BIT to guests when the clocksource we use on the host is TSC. When running nested on Hyper-V we normally use a different one:
TSC page which is resistant to TSC frequency changes on event like L1 migration. Add support for it in KVM.

The only non-trivial change in the patch is in vgettsc(): when updating our gtod copy we now need to get both the clockread and tsc value.

---
 arch/x86/kvm/x86.c | 93 +++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 68 insertions(+), 25 deletions(-)
arch/x86/kvm/x86.c +68 -25
index faf843c9b916..843a8596f57e 100644
@@ -67,6 +67,7 @@
 #include <asm/pvclock.h>
 #include <asm/div64.h>
 #include <asm/irq_remapping.h>
+#include <asm/mshyperv.h>
 
 #define CREATE_TRACE_POINTS
 #include "trace.h"
@@ -1377,6 +1378,11 @@ static u64 compute_guest_tsc(struct kvm_vcpu *vcpu, s64 kernel_ns)
 	return tsc;
 }
 
+static inline int gtod_is_based_on_tsc(int mode)
+{
+	return mode == VCLOCK_TSC || mode == VCLOCK_HVCLOCK;
+}
+
 static void kvm_track_tsc_matching(struct kvm_vcpu *vcpu)
 {
 #ifdef CONFIG_X86_64
@@ -1396,7 +1402,7 @@ static void kvm_track_tsc_matching(struct kvm_vcpu *vcpu)
 	 * perform request to enable masterclock.
 	 */
 	if (ka->use_master_clock ||
-	    (gtod->clock.vclock_mode == VCLOCK_TSC && vcpus_matched))
+	    (gtod_is_based_on_tsc(gtod->clock.vclock_mode) && vcpus_matched))
 		kvm_make_request(KVM_REQ_MASTERCLOCK_UPDATE, vcpu);
 
 	trace_kvm_track_tsc(vcpu->vcpu_id, ka->nr_vcpus_matched_tsc,
@@ -1459,6 +1465,19 @@ static void kvm_vcpu_write_tsc_offset(struct kvm_vcpu *vcpu, u64 offset)
 	vcpu->arch.tsc_offset = offset;
 }
 
+static inline bool kvm_check_tsc_unstable(void)
+{
+#ifdef CONFIG_X86_64
+	/*
+	 * TSC is marked unstable when we're running on Hyper-V,
+	 * 'TSC page' clocksource is good.
+	 */
+	if (pvclock_gtod_data.clock.vclock_mode == VCLOCK_HVCLOCK)
+		return false;
+#endif
+	return check_tsc_unstable();
+}
+
 void kvm_write_tsc(struct kvm_vcpu *vcpu, struct msr_data *msr)
 {
 	struct kvm *kvm = vcpu->kvm;
@@ -1504,7 +1523,7 @@ void kvm_write_tsc(struct kvm_vcpu *vcpu, struct msr_data *msr)
          */
 	if (synchronizing &&
 	    vcpu->arch.virtual_tsc_khz == kvm->arch.last_tsc_khz) {
-		if (!check_tsc_unstable()) {
+		if (!kvm_check_tsc_unstable()) {
 			offset = kvm->arch.cur_tsc_offset;
 			pr_debug("kvm: matched tsc offset for %llu\n", data);
 		} else {
@@ -1604,18 +1623,43 @@ static u64 read_tsc(void)
 	return last;
 }
 
-static inline u64 vgettsc(u64 *cycle_now)
+static inline u64 vgettsc(u64 *tsc_timestamp, int *mode)
 {
 	long v;
 	struct pvclock_gtod_data *gtod = &pvclock_gtod_data;
+	u64 tsc_pg_val;
+
+	switch (gtod->clock.vclock_mode) {
+	case VCLOCK_HVCLOCK:
+		tsc_pg_val = hv_read_tsc_page_tsc(hv_get_tsc_page(),
+						  tsc_timestamp);
+		if (tsc_pg_val != U64_MAX) {
+			/* TSC page valid */
+			*mode = VCLOCK_HVCLOCK;
+			v = (tsc_pg_val - gtod->clock.cycle_last) &
+				gtod->clock.mask;
+		} else {
+			/* TSC page invalid */
+			*mode = VCLOCK_NONE;
+		}
+		break;
+	case VCLOCK_TSC:
+		*mode = VCLOCK_TSC;
+		*tsc_timestamp = read_tsc();
+		v = (*tsc_timestamp - gtod->clock.cycle_last) &
+			gtod->clock.mask;
+		break;
+	default:
+		*mode = VCLOCK_NONE;
+	}
 
-	*cycle_now = read_tsc();
+	if (*mode == VCLOCK_NONE)
+		*tsc_timestamp = v = 0;
 
-	v = (*cycle_now - gtod->clock.cycle_last) & gtod->clock.mask;
 	return v * gtod->clock.mult;
 }
 
-static int do_monotonic_boot(s64 *t, u64 *cycle_now)
+static int do_monotonic_boot(s64 *t, u64 *tsc_timestamp)
 {
 	struct pvclock_gtod_data *gtod = &pvclock_gtod_data;
 	unsigned long seq;
@@ -1624,9 +1668,8 @@ static int do_monotonic_boot(s64 *t, u64 *cycle_now)
 
 	do {
 		seq = read_seqcount_begin(&gtod->seq);
-		mode = gtod->clock.vclock_mode;
 		ns = gtod->nsec_base;
-		ns += vgettsc(cycle_now);
+		ns += vgettsc(tsc_timestamp, &mode);
 		ns >>= gtod->clock.shift;
 		ns += gtod->boot_ns;
 	} while (unlikely(read_seqcount_retry(&gtod->seq, seq)));
@@ -1635,7 +1678,7 @@ static int do_monotonic_boot(s64 *t, u64 *cycle_now)
 	return mode;
 }
 
-static int do_realtime(struct timespec *ts, u64 *cycle_now)
+static int do_realtime(struct timespec *ts, u64 *tsc_timestamp)
 {
 	struct pvclock_gtod_data *gtod = &pvclock_gtod_data;
 	unsigned long seq;
@@ -1644,10 +1687,9 @@ static int do_realtime(struct timespec *ts, u64 *cycle_now)
 
 	do {
 		seq = read_seqcount_begin(&gtod->seq);
-		mode = gtod->clock.vclock_mode;
 		ts->tv_sec = gtod->wall_time_sec;
 		ns = gtod->nsec_base;
-		ns += vgettsc(cycle_now);
+		ns += vgettsc(tsc_timestamp, &mode);
 		ns >>= gtod->clock.shift;
 	} while (unlikely(read_seqcount_retry(&gtod->seq, seq)));
 
@@ -1657,25 +1699,26 @@ static int do_realtime(struct timespec *ts, u64 *cycle_now)
 	return mode;
 }
 
-/* returns true if host is using tsc clocksource */
-static bool kvm_get_time_and_clockread(s64 *kernel_ns, u64 *cycle_now)
+/* returns true if host is using TSC based clocksource */
+static bool kvm_get_time_and_clockread(s64 *kernel_ns, u64 *tsc_timestamp)
 {
 	/* checked again under seqlock below */
-	if (pvclock_gtod_data.clock.vclock_mode != VCLOCK_TSC)
+	if (!gtod_is_based_on_tsc(pvclock_gtod_data.clock.vclock_mode))
 		return false;
 
-	return do_monotonic_boot(kernel_ns, cycle_now) == VCLOCK_TSC;
+	return gtod_is_based_on_tsc(do_monotonic_boot(kernel_ns,
+						      tsc_timestamp));
 }
 
-/* returns true if host is using tsc clocksource */
+/* returns true if host is using TSC based clocksource */
 static bool kvm_get_walltime_and_clockread(struct timespec *ts,
-					   u64 *cycle_now)
+					   u64 *tsc_timestamp)
 {
 	/* checked again under seqlock below */
-	if (pvclock_gtod_data.clock.vclock_mode != VCLOCK_TSC)
+	if (!gtod_is_based_on_tsc(pvclock_gtod_data.clock.vclock_mode))
 		return false;
 
-	return do_realtime(ts, cycle_now) == VCLOCK_TSC;
+	return gtod_is_based_on_tsc(do_realtime(ts, tsc_timestamp));
 }
 #endif
 
@@ -2869,13 +2912,13 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 		kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu);
 	}
 
-	if (unlikely(vcpu->cpu != cpu) || check_tsc_unstable()) {
+	if (unlikely(vcpu->cpu != cpu) || kvm_check_tsc_unstable()) {
 		s64 tsc_delta = !vcpu->arch.last_host_tsc ? 0 :
 				rdtsc() - vcpu->arch.last_host_tsc;
 		if (tsc_delta < 0)
 			mark_tsc_unstable("KVM discovered backwards TSC");
 
-		if (check_tsc_unstable()) {
+		if (kvm_check_tsc_unstable()) {
 			u64 offset = kvm_compute_tsc_offset(vcpu,
 						vcpu->arch.last_guest_tsc);
 			kvm_vcpu_write_tsc_offset(vcpu, offset);
@@ -6110,9 +6153,9 @@ static int pvclock_gtod_notify(struct notifier_block *nb, unsigned long unused,
 	update_pvclock_gtod(tk);
 
 	/* disable master clock if host does not trust, or does not
-	 * use, TSC clocksource
+	 * use, TSC based clocksource.
 	 */
-	if (gtod->clock.vclock_mode != VCLOCK_TSC &&
+	if (!gtod_is_based_on_tsc(gtod->clock.vclock_mode) &&
 	    atomic_read(&kvm_guest_has_master_clock) != 0)
 		queue_work(system_long_wq, &pvclock_gtod_work);
 
@@ -7745,7 +7788,7 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm,
 {
 	struct kvm_vcpu *vcpu;
 
-	if (check_tsc_unstable() && atomic_read(&kvm->online_vcpus) != 0)
+	if (kvm_check_tsc_unstable() && atomic_read(&kvm->online_vcpus) != 0)
 		printk_once(KERN_WARNING
 		"kvm: SMP vm created on host with unstable TSC; "
 		"guest TSC will not be reliable\n");
@@ -7902,7 +7945,7 @@ int kvm_arch_hardware_enable(void)
 		return ret;
 
 	local_tsc = rdtsc();
-	stable = !check_tsc_unstable();
+	stable = !kvm_check_tsc_unstable();
 	list_for_each_entry(kvm, &vm_list, vm_list) {
 		kvm_for_each_vcpu(i, vcpu, kvm) {
 			if (!stable && vcpu->cpu == smp_processor_id())
Signature · 1 line
2.14.3
#13 lore raw

PATCH v2 7/7 x86/kvm: support Hyper-V reenlightenment

In reply to Vitaly Kuznetsov

When we run nested KVM on Hyper-V guests we need to update masterclocks for all guests when L1 migrates to a host with different TSC frequency. Implement the procedure in the following way:
- Pause all guests.
- Tell our host (Hyper-V) to stop emulating TSC accesses.
- Update our gtod copy, recompute clocks.
- Unpause all guests.

This is somewhat similar to cpufreq but we have two important differences:
we can only disable TSC emulation globally (on all CPUs) and we don't know the new TSC frequency until we turn the emulation off so we can't 'prepare' ourselves to the event.

---
 arch/x86/kvm/x86.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)
arch/x86/kvm/x86.c +45 -0
index 843a8596f57e..d98e5165c1ad 100644
@@ -68,6 +68,7 @@
 #include <asm/div64.h>
 #include <asm/irq_remapping.h>
 #include <asm/mshyperv.h>
+#include <asm/hypervisor.h>
 
 #define CREATE_TRACE_POINTS
 #include "trace.h"
@@ -5932,6 +5933,43 @@ static void tsc_khz_changed(void *data)
 	__this_cpu_write(cpu_tsc_khz, khz);
 }
 
+void kvm_hyperv_tsc_notifier(void)
+{
+#ifdef CONFIG_X86_64
+	struct kvm *kvm;
+	struct kvm_vcpu *vcpu;
+	int cpu;
+
+	spin_lock(&kvm_lock);
+	list_for_each_entry(kvm, &vm_list, vm_list)
+		kvm_make_mclock_inprogress_request(kvm);
+
+	hyperv_stop_tsc_emulation();
+
+	/* TSC frequency always matches when on Hyper-V */
+	for_each_present_cpu(cpu)
+		per_cpu(cpu_tsc_khz, cpu) = tsc_khz;
+	kvm_max_guest_tsc_khz = tsc_khz;
+
+	list_for_each_entry(kvm, &vm_list, vm_list) {
+		struct kvm_arch *ka = &kvm->arch;
+
+		spin_lock(&ka->pvclock_gtod_sync_lock);
+
+		pvclock_update_vm_gtod_copy(kvm);
+
+		kvm_for_each_vcpu(cpu, vcpu, kvm)
+			kvm_make_request(KVM_REQ_CLOCK_UPDATE, vcpu);
+
+		kvm_for_each_vcpu(cpu, vcpu, kvm)
+			kvm_clear_request(KVM_REQ_MCLOCK_INPROGRESS, vcpu);
+
+		spin_unlock(&ka->pvclock_gtod_sync_lock);
+	}
+	spin_unlock(&kvm_lock);
+#endif
+}
+
 static int kvmclock_cpufreq_notifier(struct notifier_block *nb, unsigned long val,
 				     void *data)
 {
@@ -6217,6 +6255,9 @@ int kvm_arch_init(void *opaque)
 	kvm_lapic_init();
 #ifdef CONFIG_X86_64
 	pvclock_gtod_register_notifier(&pvclock_gtod_notifier);
+
+	if (x86_hyper_type == X86_HYPER_MS_HYPERV)
+		set_hv_tscchange_cb(kvm_hyperv_tsc_notifier);
 #endif
 
 	return 0;
@@ -6229,6 +6270,10 @@ int kvm_arch_init(void *opaque)
 
 void kvm_arch_exit(void)
 {
+#ifdef CONFIG_X86_64
+	if (x86_hyper_type == X86_HYPER_MS_HYPERV)
+		clear_hv_tscchange_cb();
+#endif
 	kvm_lapic_exit();
 	perf_unregister_guest_info_callbacks(&kvm_guest_cbs);
 
Signature · 1 line
2.14.3
maintainer Acked-by on the v2 series
#14 lore raw
In reply to Vitaly Kuznetsov
Quoted 8 lines · Vitaly Kuznetsov

Currently, KVM passes PVCLOCK_TSC_STABLE_BIT to its guests when running in so called 'masterclock' mode and this is only possible when the clocksource on the host is TSC. When running nested on Hyper-V we're using a different clocksource in L1 (Hyper-V TSC Page) which can actually be used for masterclock. This series brings the required support.

Making KVM work with TSC page clocksource is relatively easy, it is done in PATCH 5 of the series. All the rest is required to support L1 migration

^^^^^^^

Patch 6. :)

Quoted 2 lines

when TSC frequency changes, we use a special feature from Hyper-V to do the job.

Patches 5-7 are

I would appreciate if the Hyper-V folks can provide a topic branch to be merged in both HV and KVM trees.

Thanks,

Paolo

Quoted 23 lines

Vitaly Kuznetsov (7):
x86/hyper-v: check for required priviliges in hyperv_init() x86/hyper-v: add a function to read both TSC and TSC page value
simulateneously
x86/hyper-v: reenlightenment notifications support x86/hyper-v: redirect reenlightment notifications on CPU offlining x86/irq: Count Hyper-V reenlightenment interrupts x86/kvm: pass stable clocksource to guests when running nested on
Hyper-V
x86/kvm: support Hyper-V reenlightenment

 arch/x86/entry/entry_32.S          |   3 +
 arch/x86/entry/entry_64.S          |   3 +
 arch/x86/hyperv/hv_init.c          | 133 ++++++++++++++++++++++++++++++++++-
 arch/x86/include/asm/hardirq.h     |   3 +
 arch/x86/include/asm/irq_vectors.h |   7 +-
 arch/x86/include/asm/mshyperv.h    |  32 +++++++--
 arch/x86/include/uapi/asm/hyperv.h |  27 ++++++++
 arch/x86/kernel/cpu/mshyperv.c     |   6 ++
 arch/x86/kernel/irq.c              |   9 +++
 arch/x86/kvm/x86.c                 | 138 ++++++++++++++++++++++++++++++-------
 10 files changed, 329 insertions(+), 32 deletions(-)
#15 lore raw
Quoted 7 lines · Paolo Bonzini
Quoted 6 lines · Vitaly Kuznetsov

Currently, KVM passes PVCLOCK_TSC_STABLE_BIT to its guests when running in so called 'masterclock' mode and this is only possible when the clocksource on the host is TSC. When running nested on Hyper-V we're using a different clocksource in L1 (Hyper-V TSC Page) which can actually be used for masterclock. This series brings the required support.

Quoted 6 lines
Quoted 2 lines

Making KVM work with TSC page clocksource is relatively easy, it is done in PATCH 5 of the series. All the rest is required to support L1 migration

^^^^^^^

Patch 6. :)

Off-by-one :-)

Quoted 7 lines
Quoted 2 lines

when TSC frequency changes, we use a special feature from Hyper-V to do the job.

Patches 5-7 are

Thanks!

Quoted 3 lines

I would appreciate if the Hyper-V folks can provide a topic branch to be merged in both HV and KVM trees.

There's no such thing as Hyper-V tree, patches are usually getting merged through 'tip' tree when the majority of changes go to arch/x86 or Greg's char-misc tree when changes are drivers/hv heavy (+ net, scsi, pci, hid, ... trees for individual drivers).

In this particular case the series is x86-heavy and I believe it should go through x86 'tip' tree.

Thomas, Ingo, Peter, could you please take a look? Thanks!

Signature · 1 line
  Vitaly
#16 lore raw
Quoted 15 lines · Vitaly Kuznetsov
Quoted 8 lines · Paolo Bonzini

Patches 5-7 are

I would appreciate if the Hyper-V folks can provide a topic branch to be merged in both HV and KVM trees.

There's no such thing as Hyper-V tree, patches are usually getting merged through 'tip' tree when the majority of changes go to arch/x86 or Greg's char-misc tree when changes are drivers/hv heavy (+ net, scsi, pci, hid, ... trees for individual drivers).

Quoted 4 lines

In this particular case the series is x86-heavy and I believe it should go through x86 'tip' tree.

Thomas, Ingo, Peter, could you please take a look? Thanks!

Gentle ping after the holidays season :-)

Signature · 1 line
  Vitaly