RE: [PATCH 03/11] Drivers: hv: Add support to setup percpu vmbus handler

[Michael Kelley]

From: Naman Jain <namjain@xxxxxxxxxxxxxxxxxxx> Sent: Monday, March 16, 2026 5:13 AM
> 
> Add a wrapper function - hv_setup_percpu_vmbus_handler(), similar to
> hv_setup_vmbus_handler() to allow setting up custom per-cpu VMBus
> interrupt handler. This is required for arm64 support, to be added
> in MSHV_VTL driver, where per-cpu VMBus interrupt handler will be
> set to mshv_vtl_vmbus_isr() for VTL2 (Virtual Trust Level 2).

Needing both hv_setup_vmbus_handler() and
hv_setup_percpu_vmbus_handler() seems unfortunate. Here's an
alternate approach to consider:

1. I think the x86 VMBus sysvec handler and the vmbus_percpu_isr()
functions could both use the same vmbus_handler global variable.
Looking at your changes in this patch set, hv_setup_vmbus_handler()
and hv_setup_percpu_vmbus_handler() are used together and always
set the same value.

2. So move the global variable vmbus_handler out from arch/x86
and into hv_common.c, and export it. The x86 sysvec handler can
still reference it, and vmbus_percpu_isr() in vmbus_drv.c can
also reference it.  No need to have vmbus_percpu_isr() under
arch/arm64 or have a stub in hv_common.c.

3. hv_setup_vmbus_handler() and hv_remove_vmbus_handler()
also move to hv_common.c.  The __weak stubs go away.

With these changes, only hv_setup_vmbus_handler() needs to
be called, and it works for both x86 with the sysvec handler and
for arm64 with vmbus_percpu_isr().

I haven't coded this up, so maybe there's some problematic detail,
but the idea seems like it would work. If it does work, some of my
comments below are no longer applicable.

> 
> Signed-off-by: Saurabh Sengar <ssengar@xxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Naman Jain <namjain@xxxxxxxxxxxxxxxxxxx>
> ---
>  arch/arm64/hyperv/mshyperv.c   | 13 +++++++++++++
>  drivers/hv/hv_common.c         | 11 +++++++++++
>  drivers/hv/vmbus_drv.c         |  7 +------
>  include/asm-generic/mshyperv.h |  3 +++
>  4 files changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/arm64/hyperv/mshyperv.c b/arch/arm64/hyperv/mshyperv.c
> index 4fdc26ade1d7..d4494ceeaad0 100644
> --- a/arch/arm64/hyperv/mshyperv.c
> +++ b/arch/arm64/hyperv/mshyperv.c
> @@ -134,3 +134,16 @@ bool hv_is_hyperv_initialized(void)
>  	return hyperv_initialized;
>  }
>  EXPORT_SYMBOL_GPL(hv_is_hyperv_initialized);
> +
> +static void (*vmbus_percpu_handler)(void);
> +void hv_setup_percpu_vmbus_handler(void (*handler)(void))
> +{
> +	vmbus_percpu_handler = handler;
> +}
> +
> +irqreturn_t vmbus_percpu_isr(int irq, void *dev_id)
> +{
> +	if (vmbus_percpu_handler)
> +		vmbus_percpu_handler();
> +	return IRQ_HANDLED;
> +}
> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> index d1ebc0ebd08f..a5064f558bf6 100644
> --- a/drivers/hv/hv_common.c
> +++ b/drivers/hv/hv_common.c
> @@ -759,6 +759,17 @@ void __weak hv_setup_vmbus_handler(void (*handler)(void))
>  }
>  EXPORT_SYMBOL_GPL(hv_setup_vmbus_handler);
> 
> +irqreturn_t __weak vmbus_percpu_isr(int irq, void *dev_id)
> +{
> +	return IRQ_HANDLED;
> +}
> +EXPORT_SYMBOL_GPL(vmbus_percpu_isr);
> +
> +void __weak hv_setup_percpu_vmbus_handler(void (*handler)(void))
> +{
> +}
> +EXPORT_SYMBOL_GPL(hv_setup_percpu_vmbus_handler);

You've implemented hv_setup_percpu_vmbus_handler() following
the pattern of hv_setup_vmbus_handler(), which is reasonable.
But that turns out to be unnecessarily complicated. The existing
hv_setup_vmbus_handler() has a portion in
arch/x86/kernel/cpu/mshyperv.c as a special case because it uses a
hard-coded interrupt vector on x86/x64, and has its own custom
sysvec code. And there's a need for a __weak stub in hv_common.c
so that vmbus_drv.c will compile on arm64.

But hv_setup_percpu_vmbus_handler() does not have the same
requirements. It could be implemented entirely in vmbus_drv.c,
with no code under arch/x86 or arch/arm64, and no __weak stub
in hv_common.c.  vmbus_drv.c would just need to
EXPORT_SYMBOL_FOR_MODULES, like it already does with vmbus_isr.
I didn't code it up, but I think that approach would be simpler with
fewer piece-parts scattered all over. If so, it would be worth
breaking the symmetry with hv_setup_vmbus_handler().

> +
>  void __weak hv_remove_vmbus_handler(void)
>  {
>  }
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index bc4fc1951ae1..f99d4f2d3862 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -1413,12 +1413,6 @@ void vmbus_isr(void)
>  }
>  EXPORT_SYMBOL_FOR_MODULES(vmbus_isr, "mshv_vtl");
> 
> -static irqreturn_t vmbus_percpu_isr(int irq, void *dev_id)
> -{
> -	vmbus_isr();
> -	return IRQ_HANDLED;
> -}
> -
>  static void vmbus_percpu_work(struct work_struct *work)
>  {
>  	unsigned int cpu = smp_processor_id();
> @@ -1520,6 +1514,7 @@ static int vmbus_bus_init(void)
>  	if (vmbus_irq == -1) {
>  		hv_setup_vmbus_handler(vmbus_isr);
>  	} else {
> +		hv_setup_percpu_vmbus_handler(vmbus_isr);
>  		ret = request_percpu_irq(vmbus_irq, vmbus_percpu_isr,
>  				"Hyper-V VMbus", &vmbus_evt);
>  		if (ret) {
> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> index 108f135d4fd9..b147a12085e4 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -22,6 +22,7 @@
>  #include <linux/bitops.h>
>  #include <acpi/acpi_numa.h>
>  #include <linux/cpumask.h>
> +#include <linux/interrupt.h>
>  #include <linux/nmi.h>
>  #include <asm/ptrace.h>
>  #include <hyperv/hvhdk.h>
> @@ -179,6 +180,8 @@ static inline u64 hv_generate_guest_id(u64 kernel_version)
> 
>  int hv_get_hypervisor_version(union hv_hypervisor_version_info *info);
> 
> +irqreturn_t vmbus_percpu_isr(int irq, void *dev_id);
> +void hv_setup_percpu_vmbus_handler(void (*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
>