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@xxxxxxxxxxxxx>
Reviewed-by: Boqun Feng <boqun.feng@xxxxxxxxx>
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
>
