From: Boqun Feng <boqun.feng@gmail.com>
To: Michael Kelley <mikelley@microsoft.com>
Cc: sthemmin@microsoft.com, kys@microsoft.com, wei.liu@kernel.org,
tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
hpa@zytor.com, daniel.lezcano@linaro.org, arnd@arndb.de,
linux-hyperv@vger.kernel.org, linux-kernel@vger.kernel.org,
x86@kernel.org, linux-arch@vger.kernel.org
Subject: Re: [PATCH 06/10] Drivers: hv: vmbus: Move handling of VMbus interrupts
Date: Mon, 22 Feb 2021 11:54:19 +0800 [thread overview]
Message-ID: <YDMq67DpGaMC2yfI@boqun-archlinux> (raw)
In-Reply-To: <1611779025-21503-7-git-send-email-mikelley@microsoft.com>
On Wed, Jan 27, 2021 at 12:23:41PM -0800, Michael Kelley wrote:
> VMbus interrupts are most naturally modelled as per-cpu IRQs. But
> because x86/x64 doesn't have per-cpu IRQs, the core VMbus interrupt
> handling machinery is done in code under arch/x86 and Linux IRQs are
> not used. Adding support for ARM64 means adding equivalent code
> using per-cpu IRQs under arch/arm64.
>
> A better model is to treat per-cpu IRQs as the normal path (which it is
> for modern architectures), and the x86/x64 path as the exception. Do this
> by incorporating standard Linux per-cpu IRQ allocation into the main VMbus
> driver, and bypassing it in the x86/x64 exception case. For x86/x64,
> special case code is retained under arch/x86, but no VMbus interrupt
> handling code is needed under arch/arm64.
>
> No functional change.
>
> Signed-off-by: Michael Kelley <mikelley@microsoft.com>
Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
Regards,
Boqun
> ---
> arch/x86/include/asm/mshyperv.h | 1 -
> arch/x86/kernel/cpu/mshyperv.c | 13 +++------
> drivers/hv/hv.c | 8 +++++-
> drivers/hv/vmbus_drv.c | 63 ++++++++++++++++++++++++++++++++++++-----
> include/asm-generic/mshyperv.h | 7 ++---
> 5 files changed, 70 insertions(+), 22 deletions(-)
>
> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
> index d12a188..4d3e0c5 100644
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> @@ -32,7 +32,6 @@ static inline u64 hv_get_register(unsigned int reg)
> #define hv_enable_vdso_clocksource() \
> vclocks_set_used(VDSO_CLOCKMODE_HVCLOCK);
> #define hv_get_raw_timer() rdtsc_ordered()
> -#define hv_get_vector() HYPERVISOR_CALLBACK_VECTOR
>
> /*
> * Reference to pv_ops must be inline so objtool
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index f628e3dc..5679100a1 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -55,23 +55,18 @@
> set_irq_regs(old_regs);
> }
>
> -int hv_setup_vmbus_irq(int irq, void (*handler)(void))
> +void hv_setup_vmbus_handler(void (*handler)(void))
> {
> - /*
> - * The 'irq' argument is ignored on x86/x64 because a hard-coded
> - * interrupt vector is used for Hyper-V interrupts.
> - */
> vmbus_handler = handler;
> - return 0;
> }
> +EXPORT_SYMBOL_GPL(hv_setup_vmbus_handler);
>
> -void hv_remove_vmbus_irq(void)
> +void hv_remove_vmbus_handler(void)
> {
> /* We have no way to deallocate the interrupt gate */
> vmbus_handler = NULL;
> }
> -EXPORT_SYMBOL_GPL(hv_setup_vmbus_irq);
> -EXPORT_SYMBOL_GPL(hv_remove_vmbus_irq);
> +EXPORT_SYMBOL_GPL(hv_remove_vmbus_handler);
>
> /*
> * Routines to do per-architecture handling of stimer0
> diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> index afe7a62..917b29e 100644
> --- a/drivers/hv/hv.c
> +++ b/drivers/hv/hv.c
> @@ -16,6 +16,7 @@
> #include <linux/version.h>
> #include <linux/random.h>
> #include <linux/clockchips.h>
> +#include <linux/interrupt.h>
> #include <clocksource/hyperv_timer.h>
> #include <asm/mshyperv.h>
> #include "hyperv_vmbus.h"
> @@ -214,10 +215,12 @@ void hv_synic_enable_regs(unsigned int cpu)
> hv_set_register(HV_REGISTER_SIEFP, siefp.as_uint64);
>
> /* Setup the shared SINT. */
> + if (vmbus_irq != -1)
> + enable_percpu_irq(vmbus_irq, 0);
> shared_sint.as_uint64 = hv_get_register(HV_REGISTER_SINT0 +
> VMBUS_MESSAGE_SINT);
>
> - shared_sint.vector = hv_get_vector();
> + shared_sint.vector = vmbus_interrupt;
> shared_sint.masked = false;
>
> /*
> @@ -285,6 +288,9 @@ void hv_synic_disable_regs(unsigned int cpu)
> sctrl.as_uint64 = hv_get_register(HV_REGISTER_SCONTROL);
> sctrl.enable = 0;
> hv_set_register(HV_REGISTER_SCONTROL, sctrl.as_uint64);
> +
> + if (vmbus_irq != -1)
> + disable_percpu_irq(vmbus_irq);
> }
>
>
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index 8affe68..62721a7 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -48,8 +48,10 @@ struct vmbus_dynid {
>
> static void *hv_panic_page;
>
> +static long __percpu *vmbus_evt;
> +
> /* Values parsed from ACPI DSDT */
> -static int vmbus_irq;
> +int vmbus_irq;
> int vmbus_interrupt;
>
> /*
> @@ -1354,7 +1356,13 @@ static void vmbus_isr(void)
> tasklet_schedule(&hv_cpu->msg_dpc);
> }
>
> - add_interrupt_randomness(hv_get_vector(), 0);
> + add_interrupt_randomness(vmbus_interrupt, 0);
> +}
> +
> +static irqreturn_t vmbus_percpu_isr(int irq, void *dev_id)
> +{
> + vmbus_isr();
> + return IRQ_HANDLED;
> }
>
> /*
> @@ -1469,9 +1477,28 @@ static int vmbus_bus_init(void)
> if (ret)
> return ret;
>
> - ret = hv_setup_vmbus_irq(vmbus_irq, vmbus_isr);
> - if (ret)
> - goto err_setup;
> + /*
> + * VMbus interrupts are best modeled as per-cpu interrupts. If
> + * on an architecture with support for per-cpu IRQs (e.g. ARM64),
> + * allocate a per-cpu IRQ using standard Linux kernel functionality.
> + * If not on such an architecture (e.g., x86/x64), then rely on
> + * code in the arch-specific portion of the code tree to connect
> + * the VMbus interrupt handler.
> + */
> +
> + if (vmbus_irq == -1) {
> + hv_setup_vmbus_handler(vmbus_isr);
> + } else {
> + vmbus_evt = alloc_percpu(long);
> + ret = request_percpu_irq(vmbus_irq, vmbus_percpu_isr,
> + "Hyper-V VMbus", vmbus_evt);
> + if (ret) {
> + pr_err("Can't request Hyper-V VMbus IRQ %d, Err %d",
> + vmbus_irq, ret);
> + free_percpu(vmbus_evt);
> + goto err_setup;
> + }
> + }
>
> ret = hv_synic_alloc();
> if (ret)
> @@ -1532,7 +1559,12 @@ static int vmbus_bus_init(void)
> err_cpuhp:
> hv_synic_free();
> err_alloc:
> - hv_remove_vmbus_irq();
> + if (vmbus_irq == -1) {
> + hv_remove_vmbus_handler();
> + } else {
> + free_percpu_irq(vmbus_irq, vmbus_evt);
> + free_percpu(vmbus_evt);
> + }
> err_setup:
> bus_unregister(&hv_bus);
> unregister_sysctl_table(hv_ctl_table_hdr);
> @@ -2649,6 +2681,18 @@ static int __init hv_acpi_init(void)
> ret = -ETIMEDOUT;
> goto cleanup;
> }
> +
> + /*
> + * If we're on an architecture with a hardcoded hypervisor
> + * vector (i.e. x86/x64), override the VMbus interrupt found
> + * in the ACPI tables. Ensure vmbus_irq is not set since the
> + * normal Linux IRQ mechanism is not used in this case.
> + */
> +#ifdef HYPERVISOR_CALLBACK_VECTOR
> + vmbus_interrupt = HYPERVISOR_CALLBACK_VECTOR;
> + vmbus_irq = -1;
> +#endif
> +
> hv_debug_init();
>
> ret = vmbus_bus_init();
> @@ -2679,7 +2723,12 @@ static void __exit vmbus_exit(void)
> vmbus_connection.conn_state = DISCONNECTED;
> hv_stimer_global_cleanup();
> vmbus_disconnect();
> - hv_remove_vmbus_irq();
> + if (vmbus_irq == -1) {
> + hv_remove_vmbus_handler();
> + } else {
> + free_percpu_irq(vmbus_irq, vmbus_evt);
> + free_percpu(vmbus_evt);
> + }
> for_each_online_cpu(cpu) {
> struct hv_per_cpu_context *hv_cpu
> = per_cpu_ptr(hv_context.cpu_context, cpu);
> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> index 6a8072f..9f4089b 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -89,10 +89,8 @@ static inline void vmbus_signal_eom(struct hv_message *msg, u32 old_msg_type)
> }
> }
>
> -int hv_setup_vmbus_irq(int irq, void (*handler)(void));
> -void hv_remove_vmbus_irq(void);
> -void hv_enable_vmbus_irq(void);
> -void hv_disable_vmbus_irq(void);
> +void hv_setup_vmbus_handler(void (*handler)(void));
> +void hv_remove_vmbus_handler(void);
>
> void hv_setup_kexec_handler(void (*handler)(void));
> void hv_remove_kexec_handler(void);
> @@ -100,6 +98,7 @@ static inline void vmbus_signal_eom(struct hv_message *msg, u32 old_msg_type)
> void hv_remove_crash_handler(void);
>
> extern int vmbus_interrupt;
> +extern int vmbus_irq;
>
> #if IS_ENABLED(CONFIG_HYPERV)
> /*
> --
> 1.8.3.1
>
next prev parent reply other threads:[~2021-02-22 3:55 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-27 20:23 [PATCH 00/10] Refactor arch specific Hyper-V code Michael Kelley
2021-01-27 20:23 ` [PATCH 01/10] Drivers: hv: vmbus: Move Hyper-V page allocator to arch neutral code Michael Kelley
2021-02-22 3:04 ` Boqun Feng
2021-01-27 20:23 ` [PATCH 02/10] x86/hyper-v: Move hv_message_type to architecture neutral module Michael Kelley
2021-02-22 3:19 ` Boqun Feng
2021-01-27 20:23 ` [PATCH 03/10] Drivers: hv: Redo Hyper-V synthetic MSR get/set functions Michael Kelley
2021-02-22 3:25 ` Boqun Feng
2021-01-27 20:23 ` [PATCH 04/10] Drivers: hv: vmbus: Move hyperv_report_panic_msg to arch neutral code Michael Kelley
2021-02-22 3:27 ` Boqun Feng
2021-01-27 20:23 ` [PATCH 05/10] Drivers: hv: vmbus: Handle auto EOI quirk inline Michael Kelley
2021-02-22 3:30 ` Boqun Feng
2021-01-27 20:23 ` [PATCH 06/10] Drivers: hv: vmbus: Move handling of VMbus interrupts Michael Kelley
2021-02-22 3:54 ` Boqun Feng [this message]
2021-01-27 20:23 ` [PATCH 07/10] clocksource/drivers/hyper-v: Handle vDSO differences inline Michael Kelley
2021-02-22 4:07 ` Boqun Feng
2021-01-27 20:23 ` [PATCH 08/10] clocksource/drivers/hyper-v: Handle sched_clock " Michael Kelley
2021-02-01 18:55 ` Wei Liu
2021-02-04 16:28 ` Michael Kelley
2021-02-04 16:31 ` Wei Liu
2021-02-22 15:17 ` Boqun Feng
2021-01-27 20:23 ` [PATCH 09/10] clocksource/drivers/hyper-v: Set clocksource rating based on Hyper-V feature Michael Kelley
2021-02-22 16:01 ` Boqun Feng
2021-02-22 22:48 ` Michael Kelley
2021-01-27 20:23 ` [PATCH 10/10] clocksource/drivers/hyper-v: Move handling of STIMER0 interrupts Michael Kelley
2021-02-01 19:53 ` Wei Liu
2021-02-04 16:30 ` Michael Kelley
2021-02-23 6:47 ` Boqun Feng
2021-02-25 18:56 ` Michael Kelley
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YDMq67DpGaMC2yfI@boqun-archlinux \
--to=boqun.feng@gmail.com \
--cc=arnd@arndb.de \
--cc=bp@alien8.de \
--cc=daniel.lezcano@linaro.org \
--cc=hpa@zytor.com \
--cc=kys@microsoft.com \
--cc=linux-arch@vger.kernel.org \
--cc=linux-hyperv@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mikelley@microsoft.com \
--cc=mingo@redhat.com \
--cc=sthemmin@microsoft.com \
--cc=tglx@linutronix.de \
--cc=wei.liu@kernel.org \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).