diff mbox series

[v7,05/10] arm64: hyperv: Add interrupt handlers for VMbus and stimer

Message ID 1598287583-71762-6-git-send-email-mikelley@microsoft.com (mailing list archive)
State New, archived
Headers show
Series Enable Linux guests on Hyper-V on ARM64 | expand

Commit Message

Michael Kelley (LINUX) Aug. 24, 2020, 4:46 p.m. UTC
Add ARM64-specific code to set up and handle the interrupts
generated by Hyper-V for VMbus messages and for stimer expiration.

This code is architecture dependent and is mostly driven by
architecture independent code in the VMbus driver and the
Hyper-V timer clocksource driver.

This code is built only when CONFIG_HYPERV is enabled.

Signed-off-by: Michael Kelley <mikelley@microsoft.com>
---
 arch/arm64/hyperv/Makefile        |   2 +-
 arch/arm64/hyperv/mshyperv.c      | 133 ++++++++++++++++++++++++++++++++++++++
 arch/arm64/include/asm/mshyperv.h |  70 ++++++++++++++++++++
 3 files changed, 204 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm64/hyperv/mshyperv.c

Comments

Arnd Bergmann Aug. 24, 2020, 6:54 p.m. UTC | #1
On Mon, Aug 24, 2020 at 6:46 PM Michael Kelley <mikelley@microsoft.com> wrote:
>
> Add ARM64-specific code to set up and handle the interrupts
> generated by Hyper-V for VMbus messages and for stimer expiration.
>
> This code is architecture dependent and is mostly driven by
> architecture independent code in the VMbus driver and the
> Hyper-V timer clocksource driver.
>
> This code is built only when CONFIG_HYPERV is enabled.
>
> Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> ---
>  arch/arm64/hyperv/Makefile        |   2 +-
>  arch/arm64/hyperv/mshyperv.c      | 133 ++++++++++++++++++++++++++++++++++++++
>  arch/arm64/include/asm/mshyperv.h |  70 ++++++++++++++++++++

I still have the feeling that most of the code in arch/arm64/hyperv/ is
misplaced: the only callers are loadable modules in drivers/hv/, and the
code is not really part of the architecture but part of the platform.

For the arm64 architecture, we have a rule that platform specific
code belongs into device drivers rather than into the architecture
code as we used to do in the linux-2.6 days for arch/arm/.

I don't see hyperv being virtual rather than an SoC as a differentiator
either; it's still just one of many platforms. If you look at
arch/arm64/xen/, you can see that they have managed to get
to a much simpler implementation in comparison.

I'm not sure what the correct solution should be, but what I'd try to
do here is to move every function that just considers the platform
rather than the architecture somewhere into drivers/hv where it
can be linked into the same modules as the existing files when
building for arm64, while trying to keep architecture specific code
in the header file where it can be included from those modules.

      Arnd
Michael Kelley (LINUX) Aug. 25, 2020, 10:04 p.m. UTC | #2
From: Arnd Bergmann <arnd@arndb.de> Sent: Monday, August 24, 2020 11:54 AM
> 
> On Mon, Aug 24, 2020 at 6:46 PM Michael Kelley <mikelley@microsoft.com> wrote:
> >
> > Add ARM64-specific code to set up and handle the interrupts
> > generated by Hyper-V for VMbus messages and for stimer expiration.
> >
> > This code is architecture dependent and is mostly driven by
> > architecture independent code in the VMbus driver and the
> > Hyper-V timer clocksource driver.
> >
> > This code is built only when CONFIG_HYPERV is enabled.
> >
> > Signed-off-by: Michael Kelley <mikelley@microsoft.com>
> > ---
> >  arch/arm64/hyperv/Makefile        |   2 +-
> >  arch/arm64/hyperv/mshyperv.c      | 133
> ++++++++++++++++++++++++++++++++++++++
> >  arch/arm64/include/asm/mshyperv.h |  70 ++++++++++++++++++++
> 
> I still have the feeling that most of the code in arch/arm64/hyperv/ is
> misplaced: the only callers are loadable modules in drivers/hv/, and the
> code is not really part of the architecture but part of the platform.
> 
> For the arm64 architecture, we have a rule that platform specific
> code belongs into device drivers rather than into the architecture
> code as we used to do in the linux-2.6 days for arch/arm/.
> 
> I don't see hyperv being virtual rather than an SoC as a differentiator
> either; it's still just one of many platforms. If you look at
> arch/arm64/xen/, you can see that they have managed to get
> to a much simpler implementation in comparison.
> 
> I'm not sure what the correct solution should be, but what I'd try to
> do here is to move every function that just considers the platform
> rather than the architecture somewhere into drivers/hv where it
> can be linked into the same modules as the existing files when
> building for arm64, while trying to keep architecture specific code
> in the header file where it can be included from those modules.

OK.  The concept of separating platform from architecture makes
sense to me.  The original separation of the Hyper-V code into
architecture independent portions and x86-specific portions could
use some tweaking now that we're dealing with n=2 architectures.  With
that tweaking, I can reduce the amount of Hyper-V code under arch/x86
and under arch/arm64.

On the flip side, the Hyper-V implementation on x86 and ARM64 has
differences that are semi-related to the architecture.  For example, on
x86 Hyper-V uses synthetic MSRs for a lot of guest-hypervisor setup, while
hypercalls are required on ARM64.  So I'm assuming those differences
will end up in code under arch/x86 and arch/arm64.  Arguably, I could
introduce a level of indirection (such as CONFIG_HYPERV_USE_MSRS vs.
CONFIG_HYPERV_USE_HYPERCALLS) to distinguish the two behaviors.
The selection would be tied to the architecture, and then code in
drivers/hv can #ifdef the two cases.  But I wonder if getting code out of
arch/x86 and arch/arm64 is worth that additional messiness.

Looking at the Xen code in drivers/xen, it looks like a lot of the Xen functionality
is implemented in hypercalls that can be consistent across architectures,
though I was a bit surprised to see a dozen or so instances of #ifdef CONFIG_X86.
Xen also #ifdefs on PV vs. PVHVM, which may handle some architecture
differences implicitly.  But I'm assuming that doing #ifdef <architecture>
in the Hyper-V code in order to reduce code under arch/x86 or arch/arm64
is not the right way to go.

Michael

> 
>       Arnd
Arnd Bergmann Aug. 26, 2020, 7:14 a.m. UTC | #3
On Wed, Aug 26, 2020 at 12:04 AM Michael Kelley <mikelley@microsoft.com> wrote:
> From: Arnd Bergmann <arnd@arndb.de> Sent: Monday, August 24, 2020 11:54 AM

> >
> > I'm not sure what the correct solution should be, but what I'd try to
> > do here is to move every function that just considers the platform
> > rather than the architecture somewhere into drivers/hv where it
> > can be linked into the same modules as the existing files when
> > building for arm64, while trying to keep architecture specific code
> > in the header file where it can be included from those modules.
>
> OK.  The concept of separating platform from architecture makes
> sense to me.  The original separation of the Hyper-V code into
> architecture independent portions and x86-specific portions could
> use some tweaking now that we're dealing with n=2 architectures.  With
> that tweaking, I can reduce the amount of Hyper-V code under arch/x86
> and under arch/arm64.
>
> On the flip side, the Hyper-V implementation on x86 and ARM64 has
> differences that are semi-related to the architecture.  For example, on
> x86 Hyper-V uses synthetic MSRs for a lot of guest-hypervisor setup, while
> hypercalls are required on ARM64.  So I'm assuming those differences
> will end up in code under arch/x86 and arch/arm64.

Yes, that absolutely makes sense.

> Arguably, I could introduce a level of indirection (such as
> CONFIG_HYPERV_USE_MSRS vs.
> CONFIG_HYPERV_USE_HYPERCALLS) to distinguish the two behaviors.
> The selection would be tied to the architecture, and then code in
> drivers/hv can #ifdef the two cases.  But I wonder if getting code out of
> arch/x86 and arch/arm64 is worth that additional messiness.

No, I think that would take it a little too far, and conflicts with the
generic rule that code under drivers/* should be written to be portable
even if can only run on a particular target platform.

> Looking at the Xen code in drivers/xen, it looks like a lot of the Xen functionality
> is implemented in hypercalls that can be consistent across architectures,
> though I was a bit surprised to see a dozen or so instances of #ifdef CONFIG_X86.
> Xen also #ifdefs on PV vs. PVHVM, which may handle some architecture
> differences implicitly.  But I'm assuming that doing #ifdef <architecture>
> in the Hyper-V code in order to reduce code under arch/x86 or arch/arm64
> is not the right way to go.

In general that is true, adding a lot of #ifdefs makes code less readable and
harder to test. OTOH there are cases where a single #ifdef can be useful when
it avoids adding a larger amount of complexity elsewhere. Many subsystems
try to restrict the #ifdef checks to header files while keeping the
drivers/* code
free of them.

       Arnd
diff mbox series

Patch

diff --git a/arch/arm64/hyperv/Makefile b/arch/arm64/hyperv/Makefile
index 1697d30..87c31c0 100644
--- a/arch/arm64/hyperv/Makefile
+++ b/arch/arm64/hyperv/Makefile
@@ -1,2 +1,2 @@ 
 # SPDX-License-Identifier: GPL-2.0
-obj-y		:= hv_core.o
+obj-y		:= hv_core.o mshyperv.o
diff --git a/arch/arm64/hyperv/mshyperv.c b/arch/arm64/hyperv/mshyperv.c
new file mode 100644
index 0000000..be2cd2f
--- /dev/null
+++ b/arch/arm64/hyperv/mshyperv.c
@@ -0,0 +1,133 @@ 
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Core routines for interacting with Microsoft's Hyper-V hypervisor,
+ * including setting up VMbus and STIMER interrupts, and handling
+ * crashes and kexecs. These interactions are through a set of
+ * static "handler" variables set by the architecture independent
+ * VMbus and STIMER drivers.
+ *
+ * Copyright (C) 2019, Microsoft, Inc.
+ *
+ * Author : Michael Kelley <mikelley@microsoft.com>
+ */
+
+#include <linux/types.h>
+#include <linux/export.h>
+#include <linux/interrupt.h>
+#include <linux/kexec.h>
+#include <linux/acpi.h>
+#include <linux/ptrace.h>
+#include <asm/hyperv-tlfs.h>
+#include <asm/mshyperv.h>
+
+static void (*vmbus_handler)(void);
+static void (*hv_stimer0_handler)(void);
+
+static int vmbus_irq;
+static long __percpu *vmbus_evt;
+static long __percpu *stimer0_evt;
+
+irqreturn_t hyperv_vector_handler(int irq, void *dev_id)
+{
+	vmbus_handler();
+	return IRQ_HANDLED;
+}
+
+/* Must be done just once */
+int hv_setup_vmbus_irq(int irq, void (*handler)(void))
+{
+	int result;
+
+	vmbus_handler = handler;
+
+	vmbus_evt = alloc_percpu(long);
+	result = request_percpu_irq(irq, hyperv_vector_handler,
+			"Hyper-V VMbus", vmbus_evt);
+	if (result) {
+		pr_err("Can't request Hyper-V VMBus IRQ %d. Error %d",
+			irq, result);
+		free_percpu(vmbus_evt);
+		return result;
+	}
+
+	vmbus_irq = irq;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(hv_setup_vmbus_irq);
+
+/* Must be done just once */
+void hv_remove_vmbus_irq(void)
+{
+	if (vmbus_irq) {
+		free_percpu_irq(vmbus_irq, vmbus_evt);
+		free_percpu(vmbus_evt);
+	}
+}
+EXPORT_SYMBOL_GPL(hv_remove_vmbus_irq);
+
+/* Must be done by each CPU */
+void hv_enable_vmbus_irq(void)
+{
+	enable_percpu_irq(vmbus_irq, 0);
+}
+EXPORT_SYMBOL_GPL(hv_enable_vmbus_irq);
+
+/* Must be done by each CPU */
+void hv_disable_vmbus_irq(void)
+{
+	disable_percpu_irq(vmbus_irq);
+}
+EXPORT_SYMBOL_GPL(hv_disable_vmbus_irq);
+
+/* Routines to do per-architecture handling of STIMER0 when in Direct Mode */
+
+static irqreturn_t hv_stimer0_vector_handler(int irq, void *dev_id)
+{
+	if (hv_stimer0_handler)
+		hv_stimer0_handler();
+	return IRQ_HANDLED;
+}
+
+int hv_setup_stimer0_irq(int *irq, int *vector, void (*handler)(void))
+{
+	int localirq;
+	int result;
+
+	localirq = acpi_register_gsi(NULL, HV_STIMER0_INTID,
+			ACPI_EDGE_SENSITIVE, ACPI_ACTIVE_HIGH);
+	if (localirq <= 0) {
+		pr_err("Can't register Hyper-V stimer0 GSI. Error %d",
+			localirq);
+		*irq = 0;
+		return -1;
+	}
+	stimer0_evt = alloc_percpu(long);
+	result = request_percpu_irq(localirq, hv_stimer0_vector_handler,
+					 "Hyper-V stimer0", stimer0_evt);
+	if (result) {
+		pr_err("Can't request Hyper-V stimer0 IRQ %d. Error %d",
+			localirq, result);
+		free_percpu(stimer0_evt);
+		acpi_unregister_gsi(localirq);
+		*irq = 0;
+		return result;
+	}
+
+	hv_stimer0_handler = handler;
+	*vector = HV_STIMER0_INTID;
+	*irq = localirq;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(hv_setup_stimer0_irq);
+
+void hv_remove_stimer0_irq(int irq)
+{
+	hv_stimer0_handler = NULL;
+	if (irq) {
+		free_percpu_irq(irq, stimer0_evt);
+		free_percpu(stimer0_evt);
+		acpi_unregister_gsi(irq);
+	}
+}
+EXPORT_SYMBOL_GPL(hv_remove_stimer0_irq);
diff --git a/arch/arm64/include/asm/mshyperv.h b/arch/arm64/include/asm/mshyperv.h
index b17d4a1..2ea64e54 100644
--- a/arch/arm64/include/asm/mshyperv.h
+++ b/arch/arm64/include/asm/mshyperv.h
@@ -23,6 +23,7 @@ 
 #include <linux/clocksource.h>
 #include <linux/irq.h>
 #include <linux/irqdesc.h>
+#include <linux/sched_clock.h>
 #include <linux/arm-smccc.h>
 #include <asm/hyperv-tlfs.h>
 
@@ -80,6 +81,75 @@  static inline void hv_set_synint_state(u32 sint_num, u64 val)
 		(val = hv_get_vpreg(HV_REGISTER_SINT0 + sint_num))
 
 
+/*
+ * Define the INTID used by STIMER0 Direct Mode interrupts.  This
+ * value can't come from ACPI tables because it is needed before
+ * the Linux ACPI subsystem is initialized.
+ */
+#define	HV_STIMER0_INTID	31
+
+/*
+ * Use the Hyper-V provided stimer0 as the timer that is made
+ * available to the architecture independent Hyper-V drivers.
+ */
+static inline void hv_init_timer(u32 timer, u64 tick)
+{
+	hv_set_vpreg(HV_REGISTER_STIMER0_COUNT + (2*timer), tick);
+}
+
+static inline void hv_init_timer_config(u32 timer, u64 val)
+{
+	hv_set_vpreg(HV_REGISTER_STIMER0_CONFIG + (2*timer), val);
+}
+
+#define hv_get_time_ref_count(val) \
+		(val = hv_get_vpreg(HV_REGISTER_TIME_REFCOUNT))
+#define hv_get_reference_tsc(val) \
+		(val = hv_get_vpreg(HV_REGISTER_REFERENCE_TSC))
+
+static inline void hv_set_reference_tsc(u64 val)
+{
+	hv_set_vpreg(HV_REGISTER_REFERENCE_TSC, val);
+}
+
+#define hv_set_clocksource_vdso(val) \
+		((val).vdso_clock_mode = VDSO_CLOCKMODE_NONE)
+
+static inline void hv_enable_vdso_clocksource(void) {}
+
+static inline void hv_enable_stimer0_percpu_irq(int irq)
+{
+	enable_percpu_irq(irq, 0);
+}
+
+static inline void hv_disable_stimer0_percpu_irq(int irq)
+{
+	disable_percpu_irq(irq);
+}
+
+static inline u64 hv_get_raw_timer(void)
+{
+	return arch_timer_read_counter();
+}
+
+static inline void hv_setup_sched_clock(void *sched_clock)
+{
+	/*
+	 * The Hyper-V sched clock read function returns nanoseconds,
+	 * not the normal 100ns units of the Hyper-V synthetic clock,
+	 * so specify 1 GHz here as the rate.
+	 */
+	sched_clock_register(sched_clock, 64, NSEC_PER_SEC);
+}
+
+extern int vmbus_interrupt;
+
+static inline int hv_get_vector(void)
+{
+	return vmbus_interrupt;
+}
+
+
 /* SMCCC hypercall parameters */
 #define HV_SMCCC_FUNC_NUMBER	1
 #define HV_FUNC_ID	ARM_SMCCC_CALL_VAL(			\