[PATCH v2 char-misc 1/1] x86/hyperv: Fix PIT shutdown quirk

[PATCH v2 char-misc 1/1] x86/hyperv: Fix PIT shutdown quirk

[Michael Kelley]

pit_shutdown() doesn't work on Hyper-V because of a quirk in the
PIT emulation. After shutdown the emulated PIT continues to interrupt
@18.2 HZ. This problem exists in all versions of Hyper-V and just
had not previously been noticed. So replace the normal PIT shutdown
function with an alternate version that works on Hyper-V.

The alternate shutdown function could go in the i8253.c module,
but it seems better to keep the hack with the Hyper-V code rather
than clutter up i8253.c with #ifdef CONFIG_HYPERV.

Signed-off-by: Michael Kelley <mikelley@microsoft.com>
---
Changes in v2:
* Fixed commit message and comment in the code to indicate
  that the PIT continues to interrupt @18.2 HZ, not every
  18.2 seconds. [Dexuan Cui]

---
 arch/x86/hyperv/hv_init.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 7abb09e..514fcbe 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -32,6 +32,9 @@
 #include <linux/slab.h>
 #include <linux/cpuhotplug.h>
 
+#include <linux/io.h>
+#include <linux/i8253.h>
+
 #ifdef CONFIG_HYPERV_TSCPAGE
 
 static struct ms_hyperv_tsc_page *tsc_pg;
@@ -97,6 +100,26 @@ static u64 read_hv_clock_msr(struct clocksource *arg)
 
 u32 hv_max_vp_index;
 
+#if defined(CONFIG_CLKEVT_I8253)
+/*
+ * Hyper-V emulation of the PIT has a quirk such that the
+ * normal i8253 pit_shutdown() routine doesn't stop the PIT.
+ * It keeps interrupting @18.2 HZ.  This alternate
+ * shutdown routine mirrors the normal one, but does not
+ * set the counter value to zero.
+ */
+static int hyperv_pit_shutdown(struct clock_event_device *evt)
+{
+	if (!clockevent_state_oneshot(evt) && !clockevent_state_periodic(evt))
+		return 0;
+
+	raw_spin_lock(&i8253_lock);
+	outb_p(0x30, PIT_MODE);
+	raw_spin_unlock(&i8253_lock);
+	return 0;
+}
+#endif
+
 static int hv_cpu_init(unsigned int cpu)
 {
 	u64 msr_vp_index;
@@ -325,6 +348,14 @@ void __init hyperv_init(void)
 	if (cpuhp < 0)
 		goto free_vp_assist_page;
 
+#if defined(CONFIG_CLKEVT_I8253)
+	/*
+	 * Override the PIT shutdown routine due to a quirk in
+	 * Hyper-V emulation of the PIT.
+	 */
+	i8253_clockevent.set_state_shutdown = hyperv_pit_shutdown;
+#endif
+
 	/*
 	 * Setup the hypercall page and enable hypercalls.
 	 * 1. Register the guest ID
-- 
1.8.3.1

Re: [PATCH v2 char-misc 1/1] x86/hyperv: Fix PIT shutdown quirk

[Thomas Gleixner]

Michael,

On Mon, 22 Oct 2018, Michael Kelley wrote:

> pit_shutdown() doesn't work on Hyper-V because of a quirk in the
> PIT emulation. After shutdown the emulated PIT continues to interrupt
> @18.2 HZ. This problem exists in all versions of Hyper-V and just
> had not previously been noticed. So replace the normal PIT shutdown
> function with an alternate version that works on Hyper-V.
> 
> The alternate shutdown function could go in the i8253.c module,
> but it seems better to keep the hack with the Hyper-V code rather
> than clutter up i8253.c with #ifdef CONFIG_HYPERV.

You can avoid the ideffery completely. Something like the uncompiled patch
(lacks comments) below keeps everything in i8253 and should just work.

Thanks,

	tglx

8<-------------------

diff --git a/drivers/clocksource/i8253.c b/drivers/clocksource/i8253.c
index 9c38895542f4..9b5d2259c2e5 100644
--- a/drivers/clocksource/i8253.c
+++ b/drivers/clocksource/i8253.c
@@ -10,6 +10,7 @@
 #include <linux/module.h>
 #include <linux/i8253.h>
 #include <linux/smp.h>
+#include <linux/hypervisor.h>
 
 /*
  * Protects access to I/O ports
@@ -109,9 +110,10 @@ static int pit_shutdown(struct clock_event_device *evt)
 	raw_spin_lock(&i8253_lock);
 
 	outb_p(0x30, PIT_MODE);
-	outb_p(0, PIT_CH0);
-	outb_p(0, PIT_CH0);
-
+	if (!hypervisor_is_mshyperv()) {
+		outb_p(0, PIT_CH0);
+		outb_p(0, PIT_CH0);
+	}
 	raw_spin_unlock(&i8253_lock);
 	return 0;
 }
diff --git a/include/linux/hypervisor.h b/include/linux/hypervisor.h
index fc08b433c856..2ab7303675f3 100644
--- a/include/linux/hypervisor.h
+++ b/include/linux/hypervisor.h
@@ -10,6 +10,7 @@
 #ifdef CONFIG_X86
 
 #include <asm/jailhouse_para.h>
+#include <asm/hypervisor.h>
 #include <asm/x86_init.h>
 
 static inline void hypervisor_pin_vcpu(int cpu)
@@ -17,6 +18,11 @@ static inline void hypervisor_pin_vcpu(int cpu)
 	x86_platform.hyper.pin_vcpu(cpu);
 }
 
+static inline bool hypervisor_is_mshyperv(void)
+{
+	return hypervisor_is_type(X86_HYPER_MS_HYPERV);
+}
+
 #else /* !CONFIG_X86 */
 
 #include <linux/of.h>
@@ -30,6 +36,11 @@ static inline bool jailhouse_paravirt(void)
 	return of_find_compatible_node(NULL, NULL, "jailhouse,cell");
 }
 
+static inline bool hypervisor_is_mshyperv(void)
+{
+	return false;
+}
+
 #endif /* !CONFIG_X86 */
 
 #endif /* __LINUX_HYPEVISOR_H */

RE: [PATCH v2 char-misc 1/1] x86/hyperv: Fix PIT shutdown quirk

[Michael Kelley]

From: Thomas Gleixner <tglx@linutronix.de> Sent: Tuesday, October 30, 2018 1:36 AM
> 
> You can avoid the ideffery completely. Something like the uncompiled patch
> (lacks comments) below keeps everything in i8253 and should just work.
> 

Thanks.  I'll spin a new version with that approach.  Having to clutter the i8253
code with a Hyper-V quirk is unfortunate, but it's a lot fewer lines of code overall.

Michael