From: Naman Jain <[email protected]> Sent: Thursday, April 23, 2026 
5:42 AM
> 
> Move the vmbus_handler global variable and hv_setup_vmbus_handler()/
> hv_remove_vmbus_handler() from arch/x86 to drivers/hv/hv_common.c.
> 
> hv_setup_vmbus_handler() is called unconditionally in vmbus_bus_init()
> and works for both x86 (sysvec handler) and arm64 (vmbus_percpu_isr).
> 
> This eliminates the need for separate percpu vmbus handler setup
> functions and __weak stubs, that are needed for adding ARM64 support
> in MSHV_VTL driver where we need to set a custom per-cpu vmbus handler.
> 
> Signed-off-by: Naman Jain <[email protected]>
> ---
>  arch/x86/kernel/cpu/mshyperv.c | 12 ------------
>  drivers/hv/hv_common.c         |  9 +++++++--
>  drivers/hv/vmbus_drv.c         | 17 +++++++++--------
>  include/asm-generic/mshyperv.h |  1 +
>  4 files changed, 17 insertions(+), 22 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
> index 89a2eb8a0722..68706ff5880e 100644
> --- a/arch/x86/kernel/cpu/mshyperv.c
> +++ b/arch/x86/kernel/cpu/mshyperv.c
> @@ -145,7 +145,6 @@ void hv_set_msr(unsigned int reg, u64 value)
>  EXPORT_SYMBOL_GPL(hv_set_msr);
> 
>  static void (*mshv_handler)(void);
> -static void (*vmbus_handler)(void);
>  static void (*hv_stimer0_handler)(void);
>  static void (*hv_kexec_handler)(void);
>  static void (*hv_crash_handler)(struct pt_regs *regs);
> @@ -172,17 +171,6 @@ void hv_setup_mshv_handler(void (*handler)(void))
>       mshv_handler = handler;
>  }
> 
> -void hv_setup_vmbus_handler(void (*handler)(void))
> -{
> -     vmbus_handler = handler;
> -}
> -
> -void hv_remove_vmbus_handler(void)
> -{
> -     /* We have no way to deallocate the interrupt gate */
> -     vmbus_handler = NULL;
> -}
> -
>  /*
>   * Routines to do per-architecture handling of stimer0
>   * interrupts when in Direct Mode
> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> index e8633bc51d56..eb7b0028b45d 100644
> --- a/drivers/hv/hv_common.c
> +++ b/drivers/hv/hv_common.c
> @@ -758,13 +758,18 @@ bool __weak hv_isolation_type_tdx(void)
>  }
>  EXPORT_SYMBOL_GPL(hv_isolation_type_tdx);
> 
> -void __weak hv_setup_vmbus_handler(void (*handler)(void))
> +void (*vmbus_handler)(void);
> +EXPORT_SYMBOL_GPL(vmbus_handler);
> +
> +void hv_setup_vmbus_handler(void (*handler)(void))
>  {
> +     vmbus_handler = handler;
>  }
>  EXPORT_SYMBOL_GPL(hv_setup_vmbus_handler);
> 
> -void __weak hv_remove_vmbus_handler(void)
> +void hv_remove_vmbus_handler(void)
>  {
> +     vmbus_handler = NULL;
>  }
>  EXPORT_SYMBOL_GPL(hv_remove_vmbus_handler);

I'd suggest moving hv_setup_vmbus_handler() and
hv_remove_vmbus_handler() above or below the group
of __weak stubs in this source code file. There's a comment
describing the purpose of these __weak functions, and
intermixing these two functions that are no longer __weak
produces something of a jumble.

> 
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index bc4fc1951ae1..052ca8b11cee 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -1415,7 +1415,8 @@ EXPORT_SYMBOL_FOR_MODULES(vmbus_isr, "mshv_vtl");
> 
>  static irqreturn_t vmbus_percpu_isr(int irq, void *dev_id)
>  {
> -     vmbus_isr();
> +     if (vmbus_handler)
> +             vmbus_handler();

Is it necessary to test vmbus_handler first? From what I can
see, it is always set before the per-cpu interrupt is setup.

>       return IRQ_HANDLED;
>  }
> 
> @@ -1517,8 +1518,10 @@ static int vmbus_bus_init(void)
>               vmbus_irq_initialized = true;
>       }
> 
> +     hv_setup_vmbus_handler(vmbus_isr);
> +
>       if (vmbus_irq == -1) {
> -             hv_setup_vmbus_handler(vmbus_isr);
> +             /* x86: sysvec handler uses vmbus_handler directly */
>       } else {
>               ret = request_percpu_irq(vmbus_irq, vmbus_percpu_isr,
>                               "Hyper-V VMbus", &vmbus_evt);
> @@ -1553,9 +1556,8 @@ static int vmbus_bus_init(void)
>       return 0;
> 
>  err_connect:
> -     if (vmbus_irq == -1)
> -             hv_remove_vmbus_handler();
> -     else
> +     hv_remove_vmbus_handler();
> +     if (vmbus_irq != -1)
>               free_percpu_irq(vmbus_irq, &vmbus_evt);

These operations should be reordered so they are the inverse
of how they are setup.  I.e., free_percpu_irq() first, then remove
the VMBus handler. That's just good standard practice unless
there's a specific reason to do the cleanup ordering differently. In
fact, hv_remove_vmbus_handler() needs to be moved down
to the err_setup label so it's done if request_percpu_irq()
fails.

>  err_setup:
>       if (IS_ENABLED(CONFIG_PREEMPT_RT) && vmbus_irq_initialized) {
> @@ -3026,9 +3028,8 @@ static void __exit vmbus_exit(void)
>       vmbus_connection.conn_state = DISCONNECTED;
>       hv_stimer_global_cleanup();
>       vmbus_disconnect();
> -     if (vmbus_irq == -1)
> -             hv_remove_vmbus_handler();
> -     else
> +     hv_remove_vmbus_handler();
> +     if (vmbus_irq != -1)
>               free_percpu_irq(vmbus_irq, &vmbus_evt);

Ordering should be changed here as well so it is the inverse
of how things are set up.

>       if (IS_ENABLED(CONFIG_PREEMPT_RT) && vmbus_irq_initialized) {
>               smpboot_unregister_percpu_thread(&vmbus_irq_threads);
> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> index 2810aa05dc73..db183c8cfb95 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -179,6 +179,7 @@ static inline u64 hv_generate_guest_id(u64 kernel_version)
> 
>  int hv_get_hypervisor_version(union hv_hypervisor_version_info *info);
> 
> +extern void (*vmbus_handler)(void);
>  void hv_setup_vmbus_handler(void (*handler)(void));
>  void hv_remove_vmbus_handler(void);
>  void hv_setup_stimer0_handler(void (*handler)(void));
> --
> 2.43.0
> 


Reply via email to