* RE: [PATCH 06/11] Drivers: hv: Make sint vector architecture neutral in MSHV_VTL
From: Michael Kelley @ 2026-04-01 16:57 UTC (permalink / raw)
To: Naman Jain, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Catalin Marinas, Will Deacon,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
x86@kernel.org, H . Peter Anvin, Arnd Bergmann, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti
Cc: Marc Zyngier, Timothy Hayes, Lorenzo Pieralisi, mrigendrachaubey,
ssengar@linux.microsoft.com, Michael Kelley,
linux-hyperv@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
linux-riscv@lists.infradead.org
In-Reply-To: <20260316121241.910764-7-namjain@linux.microsoft.com>
From: Naman Jain <namjain@linux.microsoft.com> Sent: Monday, March 16, 2026 5:13 AM
>
> Generalize Synthetic interrupt source vector (sint) to use
> vmbus_interrupt variable instead, which automatically takes care of
> architectures where HYPERVISOR_CALLBACK_VECTOR is not present (arm64).
Sashiko AI raised an interesting question about the startup timing --
whether the vmbus_platform_driver_probe() is guaranteed to have
set vmbus_interrupt before the VTL functions below run and use it.
What causes the mshv_vtl.ko module to be loaded, and hence run
mshv_vtl_init()?
>
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
> ---
> drivers/hv/mshv_vtl_main.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hv/mshv_vtl_main.c b/drivers/hv/mshv_vtl_main.c
> index b607b6e7e121..91517b45d526 100644
> --- a/drivers/hv/mshv_vtl_main.c
> +++ b/drivers/hv/mshv_vtl_main.c
> @@ -234,7 +234,7 @@ static void mshv_vtl_synic_enable_regs(unsigned int cpu)
> union hv_synic_sint sint;
>
> sint.as_uint64 = 0;
> - sint.vector = HYPERVISOR_CALLBACK_VECTOR;
> + sint.vector = vmbus_interrupt;
> sint.masked = false;
> sint.auto_eoi = hv_recommend_using_aeoi();
>
> @@ -753,7 +753,7 @@ static void mshv_vtl_synic_mask_vmbus_sint(void *info)
> const u8 *mask = info;
>
> sint.as_uint64 = 0;
> - sint.vector = HYPERVISOR_CALLBACK_VECTOR;
> + sint.vector = vmbus_interrupt;
> sint.masked = (*mask != 0);
> sint.auto_eoi = hv_recommend_using_aeoi();
>
> --
> 2.43.0
>
Assuming there's no timing problem vs. the VMBus driver,
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
^ permalink raw reply
* RE: [PATCH 05/11] drivers: hv: Export vmbus_interrupt for mshv_vtl module
From: Michael Kelley @ 2026-04-01 16:56 UTC (permalink / raw)
To: Naman Jain, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Catalin Marinas, Will Deacon,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
x86@kernel.org, H . Peter Anvin, Arnd Bergmann, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti
Cc: Marc Zyngier, Timothy Hayes, Lorenzo Pieralisi, mrigendrachaubey,
ssengar@linux.microsoft.com, Michael Kelley,
linux-hyperv@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
linux-riscv@lists.infradead.org
In-Reply-To: <20260316121241.910764-6-namjain@linux.microsoft.com>
From: Naman Jain <namjain@linux.microsoft.com> Sent: Monday, March 16, 2026 5:13 AM
>
Nit: For the patch Subject, capitalize "Drivers:" in the prefix.
> vmbus_interrupt is used in mshv_vtl_main.c to set the SINT vector.
> When CONFIG_MSHV_VTL=m and CONFIG_HYPERV_VMBUS=y (built-in), the module
> cannot access vmbus_interrupt at load time since it is not exported.
>
> Export it using EXPORT_SYMBOL_FOR_MODULES consistent with the existing
> pattern used for vmbus_isr.
>
> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
> ---
> drivers/hv/vmbus_drv.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index f99d4f2d3862..de191799a8f6 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -57,6 +57,7 @@ static DEFINE_PER_CPU(long, vmbus_evt);
> /* Values parsed from ACPI DSDT */
> int vmbus_irq;
> int vmbus_interrupt;
> +EXPORT_SYMBOL_FOR_MODULES(vmbus_interrupt, "mshv_vtl");
>
> /*
> * If the Confidential VMBus is used, the data on the "wire" is not
> --
> 2.43.0
>
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
^ permalink raw reply
* RE: [PATCH 04/11] Drivers: hv: Refactor mshv_vtl for ARM64 support to be added
From: Michael Kelley @ 2026-04-01 16:56 UTC (permalink / raw)
To: Naman Jain, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Catalin Marinas, Will Deacon,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
x86@kernel.org, H . Peter Anvin, Arnd Bergmann, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti
Cc: Marc Zyngier, Timothy Hayes, Lorenzo Pieralisi, mrigendrachaubey,
ssengar@linux.microsoft.com, Michael Kelley,
linux-hyperv@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
linux-riscv@lists.infradead.org
In-Reply-To: <20260316121241.910764-5-namjain@linux.microsoft.com>
From: Naman Jain <namjain@linux.microsoft.com> Sent: Monday, March 16, 2026 5:13 AM
>
> Refactor MSHV_VTL driver to move some of the x86 specific code to arch
> specific files, and add corresponding functions for arm64.
>
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
> ---
> arch/arm64/include/asm/mshyperv.h | 10 +++
> arch/x86/hyperv/hv_vtl.c | 98 ++++++++++++++++++++++++++++
> arch/x86/include/asm/mshyperv.h | 1 +
> drivers/hv/mshv_vtl_main.c | 102 +-----------------------------
> 4 files changed, 111 insertions(+), 100 deletions(-)
>
> diff --git a/arch/arm64/include/asm/mshyperv.h
> b/arch/arm64/include/asm/mshyperv.h
> index b721d3134ab6..804068e0941b 100644
> --- a/arch/arm64/include/asm/mshyperv.h
> +++ b/arch/arm64/include/asm/mshyperv.h
> @@ -60,6 +60,16 @@ static inline u64 hv_get_non_nested_msr(unsigned int reg)
> ARM_SMCCC_SMC_64, \
> ARM_SMCCC_OWNER_VENDOR_HYP, \
> HV_SMCCC_FUNC_NUMBER)
> +#ifdef CONFIG_HYPERV_VTL_MODE
> +/*
> + * Get/Set the register. If the function returns `1`, that must be done via
> + * a hypercall. Returning `0` means success.
> + */
> +static inline int hv_vtl_get_set_reg(struct hv_register_assoc *regs, bool set, u64 shared)
> +{
> + return 1;
> +}
> +#endif
>
> #include <asm-generic/mshyperv.h>
>
> diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
> index 9b6a9bc4ab76..72a0bb4ae0c7 100644
> --- a/arch/x86/hyperv/hv_vtl.c
> +++ b/arch/x86/hyperv/hv_vtl.c
> @@ -17,6 +17,8 @@
> #include <asm/realmode.h>
> #include <asm/reboot.h>
> #include <asm/smap.h>
> +#include <uapi/asm/mtrr.h>
> +#include <asm/debugreg.h>
> #include <linux/export.h>
> #include <../kernel/smpboot.h>
> #include "../../kernel/fpu/legacy.h"
> @@ -281,3 +283,99 @@ void mshv_vtl_return_call(struct mshv_vtl_cpu_context *vtl0)
> kernel_fpu_end();
> }
> EXPORT_SYMBOL(mshv_vtl_return_call);
> +
> +/* Static table mapping register names to their corresponding actions */
> +static const struct {
> + enum hv_register_name reg_name;
> + int debug_reg_num; /* -1 if not a debug register */
> + u32 msr_addr; /* 0 if not an MSR */
> +} reg_table[] = {
> + /* Debug registers */
> + {HV_X64_REGISTER_DR0, 0, 0},
> + {HV_X64_REGISTER_DR1, 1, 0},
> + {HV_X64_REGISTER_DR2, 2, 0},
> + {HV_X64_REGISTER_DR3, 3, 0},
> + {HV_X64_REGISTER_DR6, 6, 0},
> + /* MTRR MSRs */
> + {HV_X64_REGISTER_MSR_MTRR_CAP, -1, MSR_MTRRcap},
> + {HV_X64_REGISTER_MSR_MTRR_DEF_TYPE, -1, MSR_MTRRdefType},
> + {HV_X64_REGISTER_MSR_MTRR_PHYS_BASE0, -1, MTRRphysBase_MSR(0)},
> + {HV_X64_REGISTER_MSR_MTRR_PHYS_BASE1, -1, MTRRphysBase_MSR(1)},
> + {HV_X64_REGISTER_MSR_MTRR_PHYS_BASE2, -1, MTRRphysBase_MSR(2)},
> + {HV_X64_REGISTER_MSR_MTRR_PHYS_BASE3, -1, MTRRphysBase_MSR(3)},
> + {HV_X64_REGISTER_MSR_MTRR_PHYS_BASE4, -1, MTRRphysBase_MSR(4)},
> + {HV_X64_REGISTER_MSR_MTRR_PHYS_BASE5, -1, MTRRphysBase_MSR(5)},
> + {HV_X64_REGISTER_MSR_MTRR_PHYS_BASE6, -1, MTRRphysBase_MSR(6)},
> + {HV_X64_REGISTER_MSR_MTRR_PHYS_BASE7, -1, MTRRphysBase_MSR(7)},
> + {HV_X64_REGISTER_MSR_MTRR_PHYS_BASE8, -1, MTRRphysBase_MSR(8)},
> + {HV_X64_REGISTER_MSR_MTRR_PHYS_BASE9, -1, MTRRphysBase_MSR(9)},
> + {HV_X64_REGISTER_MSR_MTRR_PHYS_BASEA, -1, MTRRphysBase_MSR(0xa)},
> + {HV_X64_REGISTER_MSR_MTRR_PHYS_BASEB, -1, MTRRphysBase_MSR(0xb)},
> + {HV_X64_REGISTER_MSR_MTRR_PHYS_BASEC, -1, MTRRphysBase_MSR(0xc)},
> + {HV_X64_REGISTER_MSR_MTRR_PHYS_BASED, -1, MTRRphysBase_MSR(0xd)},
> + {HV_X64_REGISTER_MSR_MTRR_PHYS_BASEE, -1, MTRRphysBase_MSR(0xe)},
> + {HV_X64_REGISTER_MSR_MTRR_PHYS_BASEF, -1, MTRRphysBase_MSR(0xf)},
> + {HV_X64_REGISTER_MSR_MTRR_PHYS_MASK0, -1, MTRRphysMask_MSR(0)},
> + {HV_X64_REGISTER_MSR_MTRR_PHYS_MASK1, -1, MTRRphysMask_MSR(1)},
> + {HV_X64_REGISTER_MSR_MTRR_PHYS_MASK2, -1, MTRRphysMask_MSR(2)},
> + {HV_X64_REGISTER_MSR_MTRR_PHYS_MASK3, -1, MTRRphysMask_MSR(3)},
> + {HV_X64_REGISTER_MSR_MTRR_PHYS_MASK4, -1, MTRRphysMask_MSR(4)},
> + {HV_X64_REGISTER_MSR_MTRR_PHYS_MASK5, -1, MTRRphysMask_MSR(5)},
> + {HV_X64_REGISTER_MSR_MTRR_PHYS_MASK6, -1, MTRRphysMask_MSR(6)},
> + {HV_X64_REGISTER_MSR_MTRR_PHYS_MASK7, -1, MTRRphysMask_MSR(7)},
> + {HV_X64_REGISTER_MSR_MTRR_PHYS_MASK8, -1, MTRRphysMask_MSR(8)},
> + {HV_X64_REGISTER_MSR_MTRR_PHYS_MASK9, -1, MTRRphysMask_MSR(9)},
> + {HV_X64_REGISTER_MSR_MTRR_PHYS_MASKA, -1, MTRRphysMask_MSR(0xa)},
> + {HV_X64_REGISTER_MSR_MTRR_PHYS_MASKB, -1, MTRRphysMask_MSR(0xb)},
> + {HV_X64_REGISTER_MSR_MTRR_PHYS_MASKC, -1, MTRRphysMask_MSR(0xc)},
> + {HV_X64_REGISTER_MSR_MTRR_PHYS_MASKD, -1, MTRRphysMask_MSR(0xd)},
> + {HV_X64_REGISTER_MSR_MTRR_PHYS_MASKE, -1, MTRRphysMask_MSR(0xe)},
> + {HV_X64_REGISTER_MSR_MTRR_PHYS_MASKF, -1, MTRRphysMask_MSR(0xf)},
> + {HV_X64_REGISTER_MSR_MTRR_FIX64K00000, -1, MSR_MTRRfix64K_00000},
> + {HV_X64_REGISTER_MSR_MTRR_FIX16K80000, -1, MSR_MTRRfix16K_80000},
> + {HV_X64_REGISTER_MSR_MTRR_FIX16KA0000, -1, MSR_MTRRfix16K_A0000},
> + {HV_X64_REGISTER_MSR_MTRR_FIX4KC0000, -1, MSR_MTRRfix4K_C0000},
> + {HV_X64_REGISTER_MSR_MTRR_FIX4KC8000, -1, MSR_MTRRfix4K_C8000},
> + {HV_X64_REGISTER_MSR_MTRR_FIX4KD0000, -1, MSR_MTRRfix4K_D0000},
> + {HV_X64_REGISTER_MSR_MTRR_FIX4KD8000, -1, MSR_MTRRfix4K_D8000},
> + {HV_X64_REGISTER_MSR_MTRR_FIX4KE0000, -1, MSR_MTRRfix4K_E0000},
> + {HV_X64_REGISTER_MSR_MTRR_FIX4KE8000, -1, MSR_MTRRfix4K_E8000},
> + {HV_X64_REGISTER_MSR_MTRR_FIX4KF0000, -1, MSR_MTRRfix4K_F0000},
> + {HV_X64_REGISTER_MSR_MTRR_FIX4KF8000, -1, MSR_MTRRfix4K_F8000},
> +};
> +
> +int hv_vtl_get_set_reg(struct hv_register_assoc *regs, bool set, u64 shared)
> +{
> + u64 *reg64;
> + enum hv_register_name gpr_name;
> + int i;
> +
> + gpr_name = regs->name;
> + reg64 = ®s->value.reg64;
> +
> + /* Search for the register in the table */
> + for (i = 0; i < ARRAY_SIZE(reg_table); i++) {
> + if (reg_table[i].reg_name != gpr_name)
> + continue;
> + if (reg_table[i].debug_reg_num != -1) {
> + /* Handle debug registers */
> + if (gpr_name == HV_X64_REGISTER_DR6 && !shared)
> + goto hypercall;
> + if (set)
> + native_set_debugreg(reg_table[i].debug_reg_num, *reg64);
> + else
> + *reg64 = native_get_debugreg(reg_table[i].debug_reg_num);
> + } else {
> + /* Handle MSRs */
> + if (set)
> + wrmsrl(reg_table[i].msr_addr, *reg64);
> + else
> + rdmsrl(reg_table[i].msr_addr, *reg64);
> + }
> + return 0;
> + }
> +
> +hypercall:
> + return 1;
> +}
> +EXPORT_SYMBOL(hv_vtl_get_set_reg);
> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
> index f64393e853ee..d5355a5b7517 100644
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> @@ -304,6 +304,7 @@ void mshv_vtl_return_call(struct mshv_vtl_cpu_context *vtl0);
> void mshv_vtl_return_call_init(u64 vtl_return_offset);
> void mshv_vtl_return_hypercall(void);
> void __mshv_vtl_return_call(struct mshv_vtl_cpu_context *vtl0);
> +int hv_vtl_get_set_reg(struct hv_register_assoc *regs, bool set, u64 shared);
Can this move to asm-generic/mshyperv.h? The function is no longer specific
to x86/x64, so one would want to not declare it in the arch/x86 version
of mshyperv.h. But maybe moving it to asm-generic/mshyperv.h breaks
compilation on arm64 because there's also the static inline stub there.
> #else
> static inline void __init hv_vtl_init_platform(void) {}
> static inline int __init hv_vtl_early_init(void) { return 0; }
> diff --git a/drivers/hv/mshv_vtl_main.c b/drivers/hv/mshv_vtl_main.c
> index 5856975f32e1..b607b6e7e121 100644
> --- a/drivers/hv/mshv_vtl_main.c
> +++ b/drivers/hv/mshv_vtl_main.c
> @@ -19,10 +19,8 @@
> #include <linux/poll.h>
> #include <linux/file.h>
> #include <linux/vmalloc.h>
> -#include <asm/debugreg.h>
> #include <asm/mshyperv.h>
> #include <trace/events/ipi.h>
> -#include <uapi/asm/mtrr.h>
> #include <uapi/linux/mshv.h>
> #include <hyperv/hvhdk.h>
>
> @@ -505,102 +503,6 @@ static int mshv_vtl_ioctl_set_poll_file(struct mshv_vtl_set_poll_file __user *us
> return 0;
> }
>
> -/* Static table mapping register names to their corresponding actions */
> -static const struct {
> - enum hv_register_name reg_name;
> - int debug_reg_num; /* -1 if not a debug register */
> - u32 msr_addr; /* 0 if not an MSR */
> -} reg_table[] = {
> - /* Debug registers */
> - {HV_X64_REGISTER_DR0, 0, 0},
> - {HV_X64_REGISTER_DR1, 1, 0},
> - {HV_X64_REGISTER_DR2, 2, 0},
> - {HV_X64_REGISTER_DR3, 3, 0},
> - {HV_X64_REGISTER_DR6, 6, 0},
> - /* MTRR MSRs */
> - {HV_X64_REGISTER_MSR_MTRR_CAP, -1, MSR_MTRRcap},
> - {HV_X64_REGISTER_MSR_MTRR_DEF_TYPE, -1, MSR_MTRRdefType},
> - {HV_X64_REGISTER_MSR_MTRR_PHYS_BASE0, -1, MTRRphysBase_MSR(0)},
> - {HV_X64_REGISTER_MSR_MTRR_PHYS_BASE1, -1, MTRRphysBase_MSR(1)},
> - {HV_X64_REGISTER_MSR_MTRR_PHYS_BASE2, -1, MTRRphysBase_MSR(2)},
> - {HV_X64_REGISTER_MSR_MTRR_PHYS_BASE3, -1, MTRRphysBase_MSR(3)},
> - {HV_X64_REGISTER_MSR_MTRR_PHYS_BASE4, -1, MTRRphysBase_MSR(4)},
> - {HV_X64_REGISTER_MSR_MTRR_PHYS_BASE5, -1, MTRRphysBase_MSR(5)},
> - {HV_X64_REGISTER_MSR_MTRR_PHYS_BASE6, -1, MTRRphysBase_MSR(6)},
> - {HV_X64_REGISTER_MSR_MTRR_PHYS_BASE7, -1, MTRRphysBase_MSR(7)},
> - {HV_X64_REGISTER_MSR_MTRR_PHYS_BASE8, -1, MTRRphysBase_MSR(8)},
> - {HV_X64_REGISTER_MSR_MTRR_PHYS_BASE9, -1, MTRRphysBase_MSR(9)},
> - {HV_X64_REGISTER_MSR_MTRR_PHYS_BASEA, -1, MTRRphysBase_MSR(0xa)},
> - {HV_X64_REGISTER_MSR_MTRR_PHYS_BASEB, -1, MTRRphysBase_MSR(0xb)},
> - {HV_X64_REGISTER_MSR_MTRR_PHYS_BASEC, -1, MTRRphysBase_MSR(0xc)},
> - {HV_X64_REGISTER_MSR_MTRR_PHYS_BASED, -1, MTRRphysBase_MSR(0xd)},
> - {HV_X64_REGISTER_MSR_MTRR_PHYS_BASEE, -1, MTRRphysBase_MSR(0xe)},
> - {HV_X64_REGISTER_MSR_MTRR_PHYS_BASEF, -1, MTRRphysBase_MSR(0xf)},
> - {HV_X64_REGISTER_MSR_MTRR_PHYS_MASK0, -1, MTRRphysMask_MSR(0)},
> - {HV_X64_REGISTER_MSR_MTRR_PHYS_MASK1, -1, MTRRphysMask_MSR(1)},
> - {HV_X64_REGISTER_MSR_MTRR_PHYS_MASK2, -1, MTRRphysMask_MSR(2)},
> - {HV_X64_REGISTER_MSR_MTRR_PHYS_MASK3, -1, MTRRphysMask_MSR(3)},
> - {HV_X64_REGISTER_MSR_MTRR_PHYS_MASK4, -1, MTRRphysMask_MSR(4)},
> - {HV_X64_REGISTER_MSR_MTRR_PHYS_MASK5, -1, MTRRphysMask_MSR(5)},
> - {HV_X64_REGISTER_MSR_MTRR_PHYS_MASK6, -1, MTRRphysMask_MSR(6)},
> - {HV_X64_REGISTER_MSR_MTRR_PHYS_MASK7, -1, MTRRphysMask_MSR(7)},
> - {HV_X64_REGISTER_MSR_MTRR_PHYS_MASK8, -1, MTRRphysMask_MSR(8)},
> - {HV_X64_REGISTER_MSR_MTRR_PHYS_MASK9, -1, MTRRphysMask_MSR(9)},
> - {HV_X64_REGISTER_MSR_MTRR_PHYS_MASKA, -1, MTRRphysMask_MSR(0xa)},
> - {HV_X64_REGISTER_MSR_MTRR_PHYS_MASKB, -1, MTRRphysMask_MSR(0xb)},
> - {HV_X64_REGISTER_MSR_MTRR_PHYS_MASKC, -1, MTRRphysMask_MSR(0xc)},
> - {HV_X64_REGISTER_MSR_MTRR_PHYS_MASKD, -1, MTRRphysMask_MSR(0xd)},
> - {HV_X64_REGISTER_MSR_MTRR_PHYS_MASKE, -1, MTRRphysMask_MSR(0xe)},
> - {HV_X64_REGISTER_MSR_MTRR_PHYS_MASKF, -1, MTRRphysMask_MSR(0xf)},
> - {HV_X64_REGISTER_MSR_MTRR_FIX64K00000, -1, MSR_MTRRfix64K_00000},
> - {HV_X64_REGISTER_MSR_MTRR_FIX16K80000, -1, MSR_MTRRfix16K_80000},
> - {HV_X64_REGISTER_MSR_MTRR_FIX16KA0000, -1, MSR_MTRRfix16K_A0000},
> - {HV_X64_REGISTER_MSR_MTRR_FIX4KC0000, -1, MSR_MTRRfix4K_C0000},
> - {HV_X64_REGISTER_MSR_MTRR_FIX4KC8000, -1, MSR_MTRRfix4K_C8000},
> - {HV_X64_REGISTER_MSR_MTRR_FIX4KD0000, -1, MSR_MTRRfix4K_D0000},
> - {HV_X64_REGISTER_MSR_MTRR_FIX4KD8000, -1, MSR_MTRRfix4K_D8000},
> - {HV_X64_REGISTER_MSR_MTRR_FIX4KE0000, -1, MSR_MTRRfix4K_E0000},
> - {HV_X64_REGISTER_MSR_MTRR_FIX4KE8000, -1, MSR_MTRRfix4K_E8000},
> - {HV_X64_REGISTER_MSR_MTRR_FIX4KF0000, -1, MSR_MTRRfix4K_F0000},
> - {HV_X64_REGISTER_MSR_MTRR_FIX4KF8000, -1, MSR_MTRRfix4K_F8000},
> -};
> -
> -static int mshv_vtl_get_set_reg(struct hv_register_assoc *regs, bool set)
> -{
> - u64 *reg64;
> - enum hv_register_name gpr_name;
> - int i;
> -
> - gpr_name = regs->name;
> - reg64 = ®s->value.reg64;
> -
> - /* Search for the register in the table */
> - for (i = 0; i < ARRAY_SIZE(reg_table); i++) {
> - if (reg_table[i].reg_name != gpr_name)
> - continue;
> - if (reg_table[i].debug_reg_num != -1) {
> - /* Handle debug registers */
> - if (gpr_name == HV_X64_REGISTER_DR6 &&
> - !mshv_vsm_capabilities.dr6_shared)
> - goto hypercall;
> - if (set)
> - native_set_debugreg(reg_table[i].debug_reg_num, *reg64);
> - else
> - *reg64 = native_get_debugreg(reg_table[i].debug_reg_num);
> - } else {
> - /* Handle MSRs */
> - if (set)
> - wrmsrl(reg_table[i].msr_addr, *reg64);
> - else
> - rdmsrl(reg_table[i].msr_addr, *reg64);
> - }
> - return 0;
> - }
> -
> -hypercall:
> - return 1;
> -}
> -
> static void mshv_vtl_return(struct mshv_vtl_cpu_context *vtl0)
> {
> struct hv_vp_assist_page *hvp;
> @@ -720,7 +622,7 @@ mshv_vtl_ioctl_get_regs(void __user *user_args)
> sizeof(reg)))
> return -EFAULT;
>
> - ret = mshv_vtl_get_set_reg(®, false);
> + ret = hv_vtl_get_set_reg(®, false, mshv_vsm_capabilities.dr6_shared);
> if (!ret)
> goto copy_args; /* No need of hypercall */
> ret = vtl_get_vp_register(®);
> @@ -751,7 +653,7 @@ mshv_vtl_ioctl_set_regs(void __user *user_args)
> if (copy_from_user(®, (void __user *)args.regs_ptr, sizeof(reg)))
> return -EFAULT;
>
> - ret = mshv_vtl_get_set_reg(®, true);
> + ret = hv_vtl_get_set_reg(®, true, mshv_vsm_capabilities.dr6_shared);
> if (!ret)
> return ret; /* No need of hypercall */
> ret = vtl_set_vp_register(®);
> --
> 2.43.0
>
^ permalink raw reply
* RE: [PATCH 03/11] Drivers: hv: Add support to setup percpu vmbus handler
From: Michael Kelley @ 2026-04-01 16:55 UTC (permalink / raw)
To: Naman Jain, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Catalin Marinas, Will Deacon,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
x86@kernel.org, H . Peter Anvin, Arnd Bergmann, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti
Cc: Marc Zyngier, Timothy Hayes, Lorenzo Pieralisi, mrigendrachaubey,
ssengar@linux.microsoft.com, Michael Kelley,
linux-hyperv@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
linux-riscv@lists.infradead.org
In-Reply-To: <20260316121241.910764-4-namjain@linux.microsoft.com>
From: Naman Jain <namjain@linux.microsoft.com> 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@linux.microsoft.com>
> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
> ---
> 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
>
^ permalink raw reply
* RE: [PATCH 02/11] Drivers: hv: Move hv_vp_assist_page to common files
From: Michael Kelley @ 2026-04-01 16:55 UTC (permalink / raw)
To: Naman Jain, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Catalin Marinas, Will Deacon,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
x86@kernel.org, H . Peter Anvin, Arnd Bergmann, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti
Cc: Marc Zyngier, Timothy Hayes, Lorenzo Pieralisi, mrigendrachaubey,
ssengar@linux.microsoft.com, Michael Kelley,
linux-hyperv@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
linux-riscv@lists.infradead.org
In-Reply-To: <20260316121241.910764-3-namjain@linux.microsoft.com>
From: Naman Jain <namjain@linux.microsoft.com> Sent: Monday, March 16, 2026 5:13 AM
>
> Move the logic to initialize and export hv_vp_assist_page from x86
> architecture code to Hyper-V common code to allow it to be used for
> upcoming arm64 support in MSHV_VTL driver.
> Note: This change also improves error handling - if VP assist page
> allocation fails, hyperv_init() now returns early instead of
> continuing with partial initialization.
>
> Signed-off-by: Roman Kisel <romank@linux.microsoft.com>
> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
> ---
> arch/x86/hyperv/hv_init.c | 88 +---------------------------------
> drivers/hv/hv_common.c | 88 ++++++++++++++++++++++++++++++++++
> include/asm-generic/mshyperv.h | 4 ++
> include/hyperv/hvgdk_mini.h | 2 +
> 4 files changed, 95 insertions(+), 87 deletions(-)
>
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index 323adc93f2dc..75a98b5e451b 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -81,9 +81,6 @@ union hv_ghcb * __percpu *hv_ghcb_pg;
> /* Storage to save the hypercall page temporarily for hibernation */
> static void *hv_hypercall_pg_saved;
>
> -struct hv_vp_assist_page **hv_vp_assist_page;
> -EXPORT_SYMBOL_GPL(hv_vp_assist_page);
> -
> static int hyperv_init_ghcb(void)
> {
> u64 ghcb_gpa;
> @@ -117,59 +114,12 @@ static int hyperv_init_ghcb(void)
>
> static int hv_cpu_init(unsigned int cpu)
> {
> - union hv_vp_assist_msr_contents msr = { 0 };
> - struct hv_vp_assist_page **hvp;
> int ret;
>
> ret = hv_common_cpu_init(cpu);
> if (ret)
> return ret;
>
> - if (!hv_vp_assist_page)
> - return 0;
> -
> - hvp = &hv_vp_assist_page[cpu];
> - if (hv_root_partition()) {
> - /*
> - * For root partition we get the hypervisor provided VP assist
> - * page, instead of allocating a new page.
> - */
> - rdmsrq(HV_X64_MSR_VP_ASSIST_PAGE, msr.as_uint64);
> - *hvp = memremap(msr.pfn << HV_X64_MSR_VP_ASSIST_PAGE_ADDRESS_SHIFT,
> - PAGE_SIZE, MEMREMAP_WB);
> - } else {
> - /*
> - * The VP assist page is an "overlay" page (see Hyper-V TLFS's
> - * Section 5.2.1 "GPA Overlay Pages"). Here it must be zeroed
> - * out to make sure we always write the EOI MSR in
> - * hv_apic_eoi_write() *after* the EOI optimization is disabled
> - * in hv_cpu_die(), otherwise a CPU may not be stopped in the
> - * case of CPU offlining and the VM will hang.
> - */
> - if (!*hvp) {
> - *hvp = __vmalloc(PAGE_SIZE, GFP_KERNEL | __GFP_ZERO);
> -
> - /*
> - * Hyper-V should never specify a VM that is a Confidential
> - * VM and also running in the root partition. Root partition
> - * is blocked to run in Confidential VM. So only decrypt assist
> - * page in non-root partition here.
> - */
> - if (*hvp && !ms_hyperv.paravisor_present && hv_isolation_type_snp()) {
> - WARN_ON_ONCE(set_memory_decrypted((unsigned long)(*hvp), 1));
> - memset(*hvp, 0, PAGE_SIZE);
> - }
> - }
> -
> - if (*hvp)
> - msr.pfn = vmalloc_to_pfn(*hvp);
> -
> - }
> - if (!WARN_ON(!(*hvp))) {
> - msr.enable = 1;
> - wrmsrq(HV_X64_MSR_VP_ASSIST_PAGE, msr.as_uint64);
> - }
> -
> /* Allow Hyper-V stimer vector to be injected from Hypervisor. */
> if (ms_hyperv.misc_features & HV_STIMER_DIRECT_MODE_AVAILABLE)
> apic_update_vector(cpu, HYPERV_STIMER0_VECTOR, true);
> @@ -286,23 +236,6 @@ static int hv_cpu_die(unsigned int cpu)
>
> hv_common_cpu_die(cpu);
>
> - if (hv_vp_assist_page && hv_vp_assist_page[cpu]) {
> - union hv_vp_assist_msr_contents msr = { 0 };
> - if (hv_root_partition()) {
> - /*
> - * For root partition the VP assist page is mapped to
> - * hypervisor provided page, and thus we unmap the
> - * page here and nullify it, so that in future we have
> - * correct page address mapped in hv_cpu_init.
> - */
> - memunmap(hv_vp_assist_page[cpu]);
> - hv_vp_assist_page[cpu] = NULL;
> - rdmsrq(HV_X64_MSR_VP_ASSIST_PAGE, msr.as_uint64);
> - msr.enable = 0;
> - }
> - wrmsrq(HV_X64_MSR_VP_ASSIST_PAGE, msr.as_uint64);
> - }
> -
> if (hv_reenlightenment_cb == NULL)
> return 0;
>
> @@ -460,21 +393,6 @@ void __init hyperv_init(void)
> if (hv_common_init())
> return;
>
> - /*
> - * The VP assist page is useless to a TDX guest: the only use we
> - * would have for it is lazy EOI, which can not be used with TDX.
> - */
> - if (hv_isolation_type_tdx())
> - hv_vp_assist_page = NULL;
> - else
> - hv_vp_assist_page = kzalloc_objs(*hv_vp_assist_page, nr_cpu_ids);
> - if (!hv_vp_assist_page) {
> - ms_hyperv.hints &= ~HV_X64_ENLIGHTENED_VMCS_RECOMMENDED;
> -
> - if (!hv_isolation_type_tdx())
> - goto common_free;
> - }
> -
> if (ms_hyperv.paravisor_present && hv_isolation_type_snp()) {
> /* Negotiate GHCB Version. */
> if (!hv_ghcb_negotiate_protocol())
> @@ -483,7 +401,7 @@ void __init hyperv_init(void)
>
> hv_ghcb_pg = alloc_percpu(union hv_ghcb *);
> if (!hv_ghcb_pg)
> - goto free_vp_assist_page;
> + goto free_ghcb_page;
> }
>
> cpuhp = cpuhp_setup_state(CPUHP_AP_HYPERV_ONLINE, "x86/hyperv_init:online",
> @@ -613,10 +531,6 @@ void __init hyperv_init(void)
> cpuhp_remove_state(CPUHP_AP_HYPERV_ONLINE);
> free_ghcb_page:
> free_percpu(hv_ghcb_pg);
> -free_vp_assist_page:
> - kfree(hv_vp_assist_page);
> - hv_vp_assist_page = NULL;
> -common_free:
> hv_common_free();
> }
>
> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> index 6b67ac616789..d1ebc0ebd08f 100644
> --- a/drivers/hv/hv_common.c
> +++ b/drivers/hv/hv_common.c
> @@ -28,7 +28,9 @@
> #include <linux/slab.h>
> #include <linux/dma-map-ops.h>
> #include <linux/set_memory.h>
> +#include <linux/vmalloc.h>
> #include <hyperv/hvhdk.h>
> +#include <hyperv/hvgdk.h>
> #include <asm/mshyperv.h>
Need to add
#include <linux/io.h>
because of the memremap() and related calls that have been added.
io.h is probably being #include'd indirectly, but it is better to #include
it directly.
>
> u64 hv_current_partition_id = HV_PARTITION_ID_SELF;
> @@ -78,6 +80,8 @@ static struct ctl_table_header *hv_ctl_table_hdr;
> u8 * __percpu *hv_synic_eventring_tail;
> EXPORT_SYMBOL_GPL(hv_synic_eventring_tail);
>
> +struct hv_vp_assist_page **hv_vp_assist_page;
> +EXPORT_SYMBOL_GPL(hv_vp_assist_page);
> /*
> * Hyper-V specific initialization and shutdown code that is
> * common across all architectures. Called from architecture
> @@ -92,6 +96,9 @@ void __init hv_common_free(void)
> if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE)
> hv_kmsg_dump_unregister();
>
> + kfree(hv_vp_assist_page);
> + hv_vp_assist_page = NULL;
> +
> kfree(hv_vp_index);
> hv_vp_index = NULL;
>
> @@ -394,6 +401,23 @@ int __init hv_common_init(void)
> for (i = 0; i < nr_cpu_ids; i++)
> hv_vp_index[i] = VP_INVAL;
>
> + /*
> + * The VP assist page is useless to a TDX guest: the only use we
> + * would have for it is lazy EOI, which can not be used with TDX.
> + */
> + if (hv_isolation_type_tdx()) {
> + hv_vp_assist_page = NULL;
> + } else {
> + hv_vp_assist_page = kzalloc_objs(*hv_vp_assist_page, nr_cpu_ids);
> + if (!hv_vp_assist_page) {
> +#ifdef CONFIG_X86_64
> + ms_hyperv.hints &= ~HV_X64_ENLIGHTENED_VMCS_RECOMMENDED;
> +#endif
> + hv_common_free();
> + return -ENOMEM;
Given that "failure to allocate memory" now returns an error that is
essentially fatal to hyperv_init(), is it still necessary to clear the flag in
ms_hyperv.hints? I'd love to see that #ifdef go away. It's the only
#ifdef in hv_common.c, and I had worked hard in the past to avoid
such #ifdef's. :-)
> + }
> + }
> +
> return 0;
> }
>
> @@ -471,6 +495,8 @@ void __init ms_hyperv_late_init(void)
>
> int hv_common_cpu_init(unsigned int cpu)
> {
> + union hv_vp_assist_msr_contents msr = { 0 };
> + struct hv_vp_assist_page **hvp;
> void **inputarg, **outputarg;
> u8 **synic_eventring_tail;
> u64 msr_vp_index;
> @@ -542,6 +568,50 @@ int hv_common_cpu_init(unsigned int cpu)
> ret = -ENOMEM;
The Sashiko AI comment here about a bug when ret is set to -ENOMEM
seems valid to me.
> }
>
> + if (!hv_vp_assist_page)
> + return ret;
> +
> + hvp = &hv_vp_assist_page[cpu];
> + if (hv_root_partition()) {
> + /*
> + * For root partition we get the hypervisor provided VP assist
> + * page, instead of allocating a new page.
> + */
> + msr.as_uint64 = hv_get_msr(HV_SYN_REG_VP_ASSIST_PAGE);
> + *hvp = memremap(msr.pfn << HV_VP_ASSIST_PAGE_ADDRESS_SHIFT,
> + PAGE_SIZE, MEMREMAP_WB);
The Sashiko AI comment about potentially memremap'ing 64K instead of 4K can
be ignored. We know that the root partition can only run with a 4K page size,
and that is enforced in drivers/hv/Kconfig.
HV_VP_ASSIST_PAGE_ADDRESS_SHIFT is defined in asm-generic/mshyperv.h.
But there is also HV_X64_MSR_VP_ASSIST_PAGE_ADDRESS_SHIFT in hvgdk_mini.h.
Is there a clean way to eliminate the duplication?
> + } else {
> + /*
> + * The VP assist page is an "overlay" page (see Hyper-V TLFS's
> + * Section 5.2.1 "GPA Overlay Pages"). Here it must be zeroed
> + * out to make sure we always write the EOI MSR in
> + * hv_apic_eoi_write() *after* the EOI optimization is disabled
> + * in hv_cpu_die(), otherwise a CPU may not be stopped in the
> + * case of CPU offlining and the VM will hang.
> + */
Somewhere in the comment above, I'd suggest adding a short "on x86/x64"
qualifier, as the comment doesn't apply on arm64 since it doesn't support
the AutoEOI optimization. Maybe "Here it must be zeroed out to make sure
that on x86/x64 we always write the EOI MSR in ....".
> + if (!*hvp) {
> + *hvp = __vmalloc(PAGE_SIZE, GFP_KERNEL | __GFP_ZERO);
The Sashiko AI comment about using "flags" instead of GFP_KERNEL seems valid.
> +
> + /*
> + * Hyper-V should never specify a VM that is a Confidential
> + * VM and also running in the root partition. Root partition
> + * is blocked to run in Confidential VM. So only decrypt assist
> + * page in non-root partition here.
> + */
> + if (*hvp && !ms_hyperv.paravisor_present && hv_isolation_type_snp()) {
> + WARN_ON_ONCE(set_memory_decrypted((unsigned long)(*hvp), 1));
> + memset(*hvp, 0, PAGE_SIZE);
> + }
> + }
> +
> + if (*hvp)
> + msr.pfn = vmalloc_to_pfn(*hvp);
The Sashiko AI comment about page size here seems valid. But what are the rules
about arm64 page sizes that are supported for VTL2, and how does they relate
to VTL0 allowing 4K, 16K, and 64K page size? What combinations are allowed?
For example, can a VTL2 built with 4K page size run with a VTL0 built with
64K page size? It would be nice to have the rules recorded somewhere in a
code comment, but I'm not sure of the best place.
But regardless of the rules, I'd suggest future-proofing by using
"page_to_hvpfn(vmalloc_to_page(*hvp))" so that the PFN generated is always
in terms of 4K page size as the Hyper-V host expects.
> + }
> + if (!WARN_ON(!(*hvp))) {
> + msr.enable = 1;
> + hv_set_msr(HV_SYN_REG_VP_ASSIST_PAGE, msr.as_uint64);
> + }
> +
> return ret;
> }
>
> @@ -566,6 +636,24 @@ int hv_common_cpu_die(unsigned int cpu)
> *synic_eventring_tail = NULL;
> }
>
> + if (hv_vp_assist_page && hv_vp_assist_page[cpu]) {
> + union hv_vp_assist_msr_contents msr = { 0 };
> +
> + if (hv_root_partition()) {
> + /*
> + * For root partition the VP assist page is mapped to
> + * hypervisor provided page, and thus we unmap the
> + * page here and nullify it, so that in future we have
> + * correct page address mapped in hv_cpu_init.
> + */
> + memunmap(hv_vp_assist_page[cpu]);
> + hv_vp_assist_page[cpu] = NULL;
> + msr.as_uint64 = hv_get_msr(HV_SYN_REG_VP_ASSIST_PAGE);
> + msr.enable = 0;
> + }
> + hv_set_msr(HV_SYN_REG_VP_ASSIST_PAGE, msr.as_uint64);
> + }
> +
> return 0;
> }
>
> diff --git a/include/asm-generic/mshyperv.h b/include/asm-generic/mshyperv.h
> index d37b68238c97..108f135d4fd9 100644
> --- a/include/asm-generic/mshyperv.h
> +++ b/include/asm-generic/mshyperv.h
> @@ -25,6 +25,7 @@
> #include <linux/nmi.h>
> #include <asm/ptrace.h>
> #include <hyperv/hvhdk.h>
> +#include <hyperv/hvgdk.h>
>
> #define VTPM_BASE_ADDRESS 0xfed40000
>
> @@ -299,6 +300,8 @@ do { \
> #define hv_status_debug(status, fmt, ...) \
> hv_status_printk(debug, status, fmt, ##__VA_ARGS__)
>
> +extern struct hv_vp_assist_page **hv_vp_assist_page;
This "extern" statement is added here so it is visible to both x86/x64 and arm64.
And that's correct.
But there is still some VP assist page stuff that has been left in the arch/x86
version of mshyperv.h. That other stuff, including the inline function
hv_get_vp_assist_page(), should also be moved to asm-generic/mshyperv.h.
Given that the VP assist page support is now fully generic and not x86/x64
specific, it shouldn't occur anywhere in the arch/x86 version of mshyperv.h.
> +
> const char *hv_result_to_string(u64 hv_status);
> int hv_result_to_errno(u64 status);
> void hyperv_report_panic(struct pt_regs *regs, long err, bool in_die);
> @@ -377,6 +380,7 @@ static inline int hv_deposit_memory(u64 partition_id, u64 status)
> return hv_deposit_memory_node(NUMA_NO_NODE, partition_id, status);
> }
>
> +#define HV_VP_ASSIST_PAGE_ADDRESS_SHIFT 12
> #if IS_ENABLED(CONFIG_HYPERV_VTL_MODE)
> u8 __init get_vtl(void);
> #else
> diff --git a/include/hyperv/hvgdk_mini.h b/include/hyperv/hvgdk_mini.h
> index 056ef7b6b360..be697ddb211a 100644
> --- a/include/hyperv/hvgdk_mini.h
> +++ b/include/hyperv/hvgdk_mini.h
> @@ -149,6 +149,7 @@ struct hv_u128 {
> #define HV_X64_MSR_VP_ASSIST_PAGE_ADDRESS_SHIFT 12
> #define HV_X64_MSR_VP_ASSIST_PAGE_ADDRESS_MASK \
> (~((1ull << HV_X64_MSR_VP_ASSIST_PAGE_ADDRESS_SHIFT) - 1))
> +#define HV_SYN_REG_VP_ASSIST_PAGE (HV_X64_MSR_VP_ASSIST_PAGE)
>
> /* Hyper-V Enlightened VMCS version mask in nested features CPUID */
> #define HV_X64_ENLIGHTENED_VMCS_VERSION 0xff
> @@ -1185,6 +1186,7 @@ enum hv_register_name {
>
> #define HV_MSR_STIMER0_CONFIG (HV_REGISTER_STIMER0_CONFIG)
> #define HV_MSR_STIMER0_COUNT (HV_REGISTER_STIMER0_COUNT)
> +#define HV_SYN_REG_VP_ASSIST_PAGE (HV_REGISTER_VP_ASSIST_PAGE)
This defines a new register name prefix "HV_SYN_REG_" that isn't used
anywhere else. The prefixes for Hyper-V register names are already complex
to account to x86/x64 and arm64 differences, and the fact the x86/x64 has
synthetic MSRs, while arm64 does not. So introducing another prefix is
undesirable. Couldn't this just be HV_MSR_VP_ASSIST_PAGE using the
same structure as HV_MSR_STIMER0_COUNT (for example)?
>
> #endif /* CONFIG_ARM64 */
>
> --
> 2.43.0
>
^ permalink raw reply
* RE: [PATCH 01/11] arch: arm64: Export arch_smp_send_reschedule for mshv_vtl module
From: Michael Kelley @ 2026-04-01 16:54 UTC (permalink / raw)
To: Naman Jain, K . Y . Srinivasan, Haiyang Zhang, Wei Liu,
Dexuan Cui, Long Li, Catalin Marinas, Will Deacon,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
x86@kernel.org, H . Peter Anvin, Arnd Bergmann, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti
Cc: Marc Zyngier, Timothy Hayes, Lorenzo Pieralisi, mrigendrachaubey,
ssengar@linux.microsoft.com, Michael Kelley,
linux-hyperv@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
linux-riscv@lists.infradead.org
In-Reply-To: <20260316121241.910764-2-namjain@linux.microsoft.com>
From: Naman Jain <namjain@linux.microsoft.com> Sent: Monday, March 16, 2026 5:13 AM
>
Nit: For the patch "Subject", the most common prefix for the file
arch/arm64/kernel/smp.c is "arm64: smp:". I'd suggest using that
prefix for historical consistency.
> mshv_vtl_main.c calls smp_send_reschedule() which expands to
> arch_smp_send_reschedule(). When CONFIG_MSHV_VTL=m, the module cannot
> access this symbol since it is not exported on arm64.
>
> smp_send_reschedule() is used in mshv_vtl_cancel() to interrupt a vCPU
> thread running on another CPU. When a vCPU is looping in
> mshv_vtl_ioctl_return_to_lower_vtl(), it checks a per-CPU cancel flag
> before each VTL0 entry. Setting cancel=1 alone is not enough if the
> target CPU thread is sleeping - the IPI from smp_send_reschedule() kicks
> the remote CPU out of idle/sleep so it re-checks the cancel flag and
> exits the loop promptly.
>
> Other architectures (riscv, loongarch, powerpc) already export this
> symbol. Add the same EXPORT_SYMBOL_GPL for arm64. This is required
> for adding arm64 support in MSHV_VTL.
>
> Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
> ---
> arch/arm64/kernel/smp.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index 1aa324104afb..26b1a4456ceb 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -1152,6 +1152,7 @@ void arch_smp_send_reschedule(int cpu)
> {
> smp_cross_call(cpumask_of(cpu), IPI_RESCHEDULE);
> }
> +EXPORT_SYMBOL_GPL(arch_smp_send_reschedule);
>
> #ifdef CONFIG_ARM64_ACPI_PARKING_PROTOCOL
> void arch_send_wakeup_ipi(unsigned int cpu)
> --
> 2.43.0
>
The "Subject" nit notwithstanding,
Reviewed-by: Michael Kelley <mhklinux@outlook.com>
^ permalink raw reply
* RE: [PATCH 00/11] Drivers: hv: Add ARM64 support in mshv_vtl
From: Michael Kelley @ 2026-04-01 16:54 UTC (permalink / raw)
To: Naman Jain, Michael Kelley, K . Y . Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Long Li, Catalin Marinas, Will Deacon,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen,
x86@kernel.org, H . Peter Anvin, Arnd Bergmann, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Alexandre Ghiti
Cc: Marc Zyngier, Timothy Hayes, Lorenzo Pieralisi, mrigendrachaubey,
ssengar@linux.microsoft.com, linux-hyperv@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, linux-arch@vger.kernel.org,
linux-riscv@lists.infradead.org
In-Reply-To: <a8f856a0-49a8-4041-9036-4e9ade79532b@linux.microsoft.com>
From: Naman Jain <namjain@linux.microsoft.com> Sent: Tuesday, March 17, 2026 9:23 PM
>
> On 3/18/2026 3:33 AM, Michael Kelley wrote:
> > From: Naman Jain <namjain@linux.microsoft.com> Sent: Monday, March 16, 2026 5:13 AM
> >>
> >> The series intends to add support for ARM64 to mshv_vtl driver.
No need to be tentative. :-) Just write as:
"The series adds support for ARM64 to the mshv_vtl driver."
> >> For this, common Hyper-V code is refactored, necessary support is added,
> >> mshv_vtl_main.c is refactored and then finally support is added in
> >> Kconfig.
> >>
> >> Based on commit 1f318b96cc84 ("Linux 7.0-rc3")
> >
> > There's now an online LLM-based tool that is automatically reviewing
> > kernel patches. For this patch set, the results are here:
> >
> > https://sashiko.dev/#/patchset/20260316121241.910764-1-namjain%40linux.microsoft.com
> >
> > It has flagged several things that are worth checking, but I haven't
> > reviewed them to see if they are actually valid.
> >
> > FWIW, the announcement about sashiko.dev is here:
> >
> > https://lore.kernel.org/lkml/7ia4o6kmpj5s.fsf@castle.c.googlers.com/
> >
> > Michael
>
>
> Thanks for sharing Michael,
> I'll check it out and do the needful.
>
I've done a full review of this patch set and provided comments in the
individual patches. Some of my comments reference the Sashiko AI
comments, but there are still some Sashiko AI comments to consider
that I haven't referenced.
FWIW, the Sashiko AI comments are quite good -- it found some things
here that I missed on my own, and in my earlier reviews of the original VTL
code. :-(
Michael
^ permalink raw reply
* Re: [PATCH v2] Drivers: hv: mshv: fix integer overflow in memory region overlap check
From: Junrui Luo @ 2026-04-01 15:23 UTC (permalink / raw)
To: Stanislav Kinsburskii
Cc: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Nuno Das Neves, Anirudh Rayabharam, Mukesh Rathor, Muminul Islam,
Praveen K Paladugu, Jinank Jain, linux-hyperv@vger.kernel.org,
linux-kernel@vger.kernel.org, Yuhao Jiang, Roman Kisel,
stable@vger.kernel.org
In-Reply-To: <acrnkcG5_u0RCydx@skinsburskii.localdomain>
Hi Stanislav,
Thanks for the review.
On Mon, Mar 30, 2026 at 02:13:53PM -0700, Stanislav Kinsburskii wrote:
> Minor nit: just "end" or even "tmp" would be sufficient, since it's only
> used for the overflow checks. "new_region_end" is a bit verbose and it's
> not really "new" per se.
I will rename it to “region_end" in v3.
> This is a PFN, so the check should be against MAX_PHYSMEM_BITS -
> PAGE_SHIFT, right?
HVPFN_DOWN() is defined as (x) >> HV_HYP_PAGE_SHIFT, so
HVPFN_DOWN(1ULL << MAX_PHYSMEM_BITS) already expands to
1ULL << (MAX_PHYSMEM_BITS - HV_HYP_PAGE_SHIFT).
Thanks,
Junrui Luo
^ permalink raw reply
* [PATCH 2/2] hv: vmbus: Remove vmbus_irq_initialized
From: Sebastian Andrzej Siewior @ 2026-04-01 15:15 UTC (permalink / raw)
To: linux-hyperv, linux-rt-devel
Cc: K. Y. Srinivasan, Dexuan Cui, Haiyang Zhang, Jan Kiszka, Long Li,
Michael Kelley, Wei Liu, Sebastian Andrzej Siewior
In-Reply-To: <20260401151517.1743555-1-bigeasy@linutronix.de>
vmbus_irq_initialized is only true if the registration of the per-CPU
threads succeeded. If it failed, the whole registration aborts and the
vmbus_exit() path is never called.
Remove vmbus_irq_initialized.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
drivers/hv/vmbus_drv.c | 14 ++++----------
1 file changed, 4 insertions(+), 10 deletions(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index e44275370ac2a..7417841cd1f70 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1392,8 +1392,6 @@ static void run_vmbus_irqd(unsigned int cpu)
__vmbus_isr();
}
-static bool vmbus_irq_initialized;
-
static struct smp_hotplug_thread vmbus_irq_threads = {
.store = &vmbus_irqd,
.setup = vmbus_irqd_setup,
@@ -1513,11 +1511,10 @@ static int vmbus_bus_init(void)
* the VMbus interrupt handler.
*/
- if (IS_ENABLED(CONFIG_PREEMPT_RT) && !vmbus_irq_initialized) {
+ if (IS_ENABLED(CONFIG_PREEMPT_RT)) {
ret = smpboot_register_percpu_thread(&vmbus_irq_threads);
if (ret)
goto err_kthread;
- vmbus_irq_initialized = true;
}
if (vmbus_irq == -1) {
@@ -1561,10 +1558,8 @@ static int vmbus_bus_init(void)
else
free_percpu_irq(vmbus_irq, &vmbus_evt);
err_setup:
- if (IS_ENABLED(CONFIG_PREEMPT_RT) && vmbus_irq_initialized) {
+ if (IS_ENABLED(CONFIG_PREEMPT_RT))
smpboot_unregister_percpu_thread(&vmbus_irq_threads);
- vmbus_irq_initialized = false;
- }
err_kthread:
bus_unregister(&hv_bus);
return ret;
@@ -3033,10 +3028,9 @@ static void __exit vmbus_exit(void)
hv_remove_vmbus_handler();
else
free_percpu_irq(vmbus_irq, &vmbus_evt);
- if (IS_ENABLED(CONFIG_PREEMPT_RT) && vmbus_irq_initialized) {
+ if (IS_ENABLED(CONFIG_PREEMPT_RT))
smpboot_unregister_percpu_thread(&vmbus_irq_threads);
- vmbus_irq_initialized = false;
- }
+
for_each_online_cpu(cpu) {
struct hv_per_cpu_context *hv_cpu
= per_cpu_ptr(hv_context.cpu_context, cpu);
--
2.53.0
^ permalink raw reply related
* [PATCH 0/2] hv: vmbus: Small cleanups
From: Sebastian Andrzej Siewior @ 2026-04-01 15:15 UTC (permalink / raw)
To: linux-hyperv, linux-rt-devel
Cc: K. Y. Srinivasan, Dexuan Cui, Haiyang Zhang, Jan Kiszka, Long Li,
Michael Kelley, Wei Liu, Sebastian Andrzej Siewior
Replacing the lockdep_hardirq_threaded() annotation which does not
belong in drivers and removing the vmbus_irq_initialized which seems
redundant.
Sebastian Andrzej Siewior (2):
hv: vmbus: Replace lockdep_hardirq_threaded() with lockdep annotation
hv: vmbus: Remove vmbus_irq_initialized
drivers/hv/vmbus_drv.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
--
2.53.0
^ permalink raw reply
* [PATCH 1/2] hv: vmbus: Replace lockdep_hardirq_threaded() with lockdep annotation
From: Sebastian Andrzej Siewior @ 2026-04-01 15:15 UTC (permalink / raw)
To: linux-hyperv, linux-rt-devel
Cc: K. Y. Srinivasan, Dexuan Cui, Haiyang Zhang, Jan Kiszka, Long Li,
Michael Kelley, Wei Liu, Sebastian Andrzej Siewior
In-Reply-To: <20260401151517.1743555-1-bigeasy@linutronix.de>
lockdep_hardirq_threaded() is supposed to be used within IRQ core code
and not within drivers. It is not obvious from within the driver, that
this is the only interrupt service routing and that it is not shared
handler.
Replace lockdep_hardirq_threaded() with a lockdep annotation limiting
threaded context on PREEMPT_RT to __vmbus_isr().
Fixes: f8e6343b7a89c ("Drivers: hv: vmbus: Use kthread for vmbus interrupts on PREEMPT_RT")
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
drivers/hv/vmbus_drv.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index bc4fc1951ae1c..e44275370ac2a 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1407,8 +1407,11 @@ void vmbus_isr(void)
if (IS_ENABLED(CONFIG_PREEMPT_RT)) {
vmbus_irqd_wake();
} else {
- lockdep_hardirq_threaded();
+ static DEFINE_WAIT_OVERRIDE_MAP(vmbus_map, LD_WAIT_CONFIG);
+
+ lock_map_acquire_try(&vmbus_map);
__vmbus_isr();
+ lock_map_release(&vmbus_map);
}
}
EXPORT_SYMBOL_FOR_MODULES(vmbus_isr, "mshv_vtl");
--
2.53.0
^ permalink raw reply related
* [PATCH] Drivers: hv: mshv_vtl: Fix vmemmap_shift exceeding MAX_FOLIO_ORDER
From: Naman Jain @ 2026-04-01 5:40 UTC (permalink / raw)
To: K . Y . Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Michael Kelley
Cc: Saurabh Sengar, Naman Jain, linux-hyperv, linux-kernel
When registering VTL0 memory via MSHV_ADD_VTL0_MEMORY, the kernel
computes pgmap->vmemmap_shift as the number of trailing zeros in the
OR of start_pfn and last_pfn, intending to use the largest compound
page order both endpoints are aligned to.
However, this value is not clamped to MAX_FOLIO_ORDER, so a
sufficiently aligned range (e.g. physical range 0x800000000000-
0x800080000000, corresponding to start_pfn=0x800000000 with 35
trailing zeros) can produce a shift larger than what
memremap_pages() accepts, triggering a WARN and returning -EINVAL:
WARNING: ... memremap_pages+0x512/0x650
requested folio size unsupported
The MAX_FOLIO_ORDER check was added by
commit 646b67d57589 ("mm/memremap: reject unreasonable folio/compound
page sizes in memremap_pages()").
When CONFIG_HAVE_GIGANTIC_FOLIOS=y, CONFIG_SPARSEMEM_VMEMMAP=y, and
CONFIG_HUGETLB_PAGE is not set, MAX_FOLIO_ORDER resolves to
(PUD_SHIFT - PAGE_SHIFT) = 18. Any range whose PFN alignment exceeds
order 18 hits this path.
Fix this by clamping vmemmap_shift to MAX_FOLIO_ORDER so we always
request the largest order the kernel supports, rather than an
out-of-range value.
Also fix the error path to propagate the actual error code from
devm_memremap_pages() instead of hard-coding -EFAULT, which was
masking the real -EINVAL return.
Fixes: 7bfe3b8ea6e3 ("Drivers: hv: Introduce mshv_vtl driver")
Cc: <stable@vger.kernel.org>
Signed-off-by: Naman Jain <namjain@linux.microsoft.com>
---
drivers/hv/mshv_vtl_main.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/hv/mshv_vtl_main.c b/drivers/hv/mshv_vtl_main.c
index 5856975f32e12..255fed3a740c1 100644
--- a/drivers/hv/mshv_vtl_main.c
+++ b/drivers/hv/mshv_vtl_main.c
@@ -405,8 +405,12 @@ static int mshv_vtl_ioctl_add_vtl0_mem(struct mshv_vtl *vtl, void __user *arg)
/*
* Determine the highest page order that can be used for the given memory range.
* This works best when the range is aligned; i.e. both the start and the length.
+ * Clamp to MAX_FOLIO_ORDER to avoid a WARN in memremap_pages() when the range
+ * alignment exceeds the maximum supported folio order for this kernel config.
*/
- pgmap->vmemmap_shift = count_trailing_zeros(vtl0_mem.start_pfn | vtl0_mem.last_pfn);
+ pgmap->vmemmap_shift = min_t(unsigned long,
+ count_trailing_zeros(vtl0_mem.start_pfn | vtl0_mem.last_pfn),
+ MAX_FOLIO_ORDER);
dev_dbg(vtl->module_dev,
"Add VTL0 memory: start: 0x%llx, end_pfn: 0x%llx, page order: %lu\n",
vtl0_mem.start_pfn, vtl0_mem.last_pfn, pgmap->vmemmap_shift);
@@ -415,7 +419,7 @@ static int mshv_vtl_ioctl_add_vtl0_mem(struct mshv_vtl *vtl, void __user *arg)
if (IS_ERR(addr)) {
dev_err(vtl->module_dev, "devm_memremap_pages error: %ld\n", PTR_ERR(addr));
kfree(pgmap);
- return -EFAULT;
+ return PTR_ERR(addr);
}
/* Don't free pgmap, since it has to stick around until the memory
base-commit: 36ece9697e89016181e5ae87510e40fb31d86f2b
--
2.43.0
^ permalink raw reply related
* Re: [PATCH net-next] net: mana: hardening: Validate adapter_mtu from MANA_QUERY_DEV_CONFIG
From: patchwork-bot+netdevbpf @ 2026-04-01 3:20 UTC (permalink / raw)
To: Erni Sri Satya Vennela
Cc: kys, haiyangz, wei.liu, decui, longli, andrew+netdev, davem,
edumazet, kuba, pabeni, ssengar, dipayanroy, gargaditya,
shirazsaleem, kees, linux-hyperv, netdev, linux-kernel
In-Reply-To: <20260326173101.2010514-1-ernis@linux.microsoft.com>
Hello:
This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:
On Thu, 26 Mar 2026 10:30:56 -0700 you wrote:
> As a part of MANA hardening for CVM, validate the adapter_mtu value
> returned from the MANA_QUERY_DEV_CONFIG HWC command.
>
> The adapter_mtu value is used to compute ndev->max_mtu via:
> gc->adapter_mtu - ETH_HLEN. If hardware returns a bogus adapter_mtu
> smaller than ETH_HLEN (e.g. 0), the unsigned subtraction wraps to a
> huge value, silently allowing oversized MTU settings.
>
> [...]
Here is the summary with links:
- [net-next] net: mana: hardening: Validate adapter_mtu from MANA_QUERY_DEV_CONFIG
https://git.kernel.org/netdev/net-next/c/d7709812e13d
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply
* Re: [PATCH 05/12] PCI: use generic driver_override infrastructure
From: Alex Williamson @ 2026-03-31 19:18 UTC (permalink / raw)
To: Danilo Krummrich, Jason Gunthorpe
Cc: Russell King, Greg Kroah-Hartman, Rafael J. Wysocki,
Ioana Ciornei, Nipun Gupta, Nikhil Agarwal, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Bjorn Helgaas,
Armin Wolf, Bjorn Andersson, Mathieu Poirier, Vineeth Vijayan,
Peter Oberparleiter, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Harald Freudenberger, Holger Dengler, Mark Brown,
Michael S. Tsirkin, Jason Wang, Xuan Zhuo, Eugenio Pérez,
Juergen Gross, Stefano Stabellini, Oleksandr Tyshchenko,
Christophe Leroy (CS GROUP), linux-kernel, driver-core,
linuxppc-dev, linux-hyperv, linux-pci, platform-driver-x86,
linux-arm-msm, linux-remoteproc, linux-s390, linux-spi,
virtualization, kvm, xen-devel, linux-arm-kernel, Gui-Dong Han,
alex
In-Reply-To: <DHGTLY0FJWD2.2VLT6NQWF97YY@kernel.org>
On Tue, 31 Mar 2026 10:22:14 +0200
"Danilo Krummrich" <dakr@kernel.org> wrote:
> On Tue Mar 31, 2026 at 10:06 AM CEST, Danilo Krummrich wrote:
> > On Mon Mar 30, 2026 at 10:10 PM CEST, Alex Williamson wrote:
> >> On Mon, 30 Mar 2026 19:38:41 +0200
> >> "Danilo Krummrich" <dakr@kernel.org> wrote:
> >>
> >>> (Cc: Jason)
> >>>
> >>> On Tue Mar 24, 2026 at 1:59 AM CET, Danilo Krummrich wrote:
> >>> > diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> >>> > index d43745fe4c84..460852f79f29 100644
> >>> > --- a/drivers/vfio/pci/vfio_pci_core.c
> >>> > +++ b/drivers/vfio/pci/vfio_pci_core.c
> >>> > @@ -1987,9 +1987,8 @@ static int vfio_pci_bus_notifier(struct notifier_block *nb,
> >>> > pdev->is_virtfn && physfn == vdev->pdev) {
> >>> > pci_info(vdev->pdev, "Captured SR-IOV VF %s driver_override\n",
> >>> > pci_name(pdev));
> >>> > - pdev->driver_override = kasprintf(GFP_KERNEL, "%s",
> >>> > - vdev->vdev.ops->name);
> >>> > - WARN_ON(!pdev->driver_override);
> >>> > + WARN_ON(device_set_driver_override(&pdev->dev,
> >>> > + vdev->vdev.ops->name));
> >>>
> >>> Technically, this is a change in behavior. If vdev->vdev.ops->name is NULL, it
> >>> will trigger the WARN_ON(), whereas before it would have just written "(null)"
> >>> into driver_override.
> >>
> >> It's worse than that. Looking at the implementation in [1], we have:
> >>
> >> +static inline int device_set_driver_override(struct device *dev, const char *s)
> >> +{
> >> + return __device_set_driver_override(dev, s, strlen(s));
> >> +}
> >>
> >> So if name is NULL, we oops in strlen() before we even hit the -EINVAL
> >> and WARN_ON().
> >
> > This was changed in v2 [2] and the actual code in-tree is
> >
> > static inline int device_set_driver_override(struct device *dev, const char *s)
> > {
> > return __device_set_driver_override(dev, s, s ? strlen(s) : 0);
> > }
> >
> > so it does indeed return -EINVAL for a NULL pointer.
Ok, good.
> >> I don't believe we have any vfio-pci variant drivers where the name is
> >> NULL, but kasprintf() handling NULL as "(null)" was a consideration in
> >> this design, that even if there is no name the device is sequestered
> >> with a driver_override that won't match an actual driver.
> >>
> >>> I assume that vfio_pci_core drivers are expected to set the name in struct
> >>> vfio_device_ops in the first place and this code (silently) relies on this
> >>> invariant?
> >>
> >> We do expect that, but it was previously safe either way to make sure
> >> VFs are only bound to the same ops driver or barring that, at least
> >> don't perform a standard driver match. The last thing we want to
> >> happen automatically is for a user owned PF to create SR-IOV VFs that
> >> automatically bind to native kernel drivers.
> >>
> >>> Alex, Jason: Should we keep this hunk above as is and check for a proper name in
> >>> struct vfio_device_ops in vfio_pci_core_register_device() with a subsequent
> >>> patch?
> >>
> >> Given the oops, my preference would be to roll it in here. This change
> >> is what makes it a requirement that name cannot be NULL, where this was
> >> safely handled with kasprintf().
> >
> > Again, no oops here. :)
> >
> > I still think it makes more sense to fail early in
> > vfio_pci_core_register_device(), rather than silently accept "(null)" in
> > driver_override. It also doesn't seem unreasonable with only the WARN_ON(), but
> > I can also just add vdev->vdev.ops->name ?: "(null)".
>
> (Or just skip the call if !vdev->vdev.ops->name, as a user will read "(null)"
> from sysfs either way.)
Hmm, I suppose they would, but there's a fundamental difference between
driver_override being set to "(null)" vs being NULL and only
interpreted as "(null)" via show. The former would prevent ID table
matching, the latter would not. The former is what was intended here,
without realizing both look the same through sysfs and present a
difficult debugging challenge.
Given no oops for strlen() now and no known in-kernel drivers with a
NULL name, I can post a patch that rejects a NULL name in the ops
structure in vfio_pci_core_register_device(), avoiding the entire
situation.
For this,
Acked-by: Alex Williamson <alex@shazbot.org>
Thanks,
Alex
^ permalink raw reply
* Re: [PATCH net-next] net: mana: hardening: Reject zero max_num_queues from MANA_QUERY_VPORT_CONFIG
From: Erni Sri Satya Vennela @ 2026-03-31 18:02 UTC (permalink / raw)
To: Paolo Abeni
Cc: kys, haiyangz, wei.liu, decui, longli, andrew+netdev, davem,
edumazet, kuba, ssengar, dipayanroy, gargaditya, shirazsaleem,
kees, linux-hyperv, netdev, linux-kernel
In-Reply-To: <f0bc585f-b1d2-46e6-b0eb-801881862692@redhat.com>
> +
> > + if (*max_sq == 0 || *max_rq == 0) {
> > + netdev_err(apc->ndev, "Invalid max queues from vPort config\n");
> > + return -EPROTO;
>
> AI review says:
>
> Will returning -EPROTO here expose a pre-existing resource leak in the
> driver's teardown path?
> If mana_query_vport_cfg() returns an error, mana_init_port() fails and
> mana_probe_port() frees the ndev, leaving ac->ports[i] as NULL. In
> mana_probe(), the port initialization loop breaks upon this error, but
> the err variable is then overwritten:
>
> mana_probe() {
> ...
> for (i = 0; i < ac->num_ports; i++) {
> err = mana_probe_port(ac, i, &ac->ports[i]);
> if (err) {
> dev_err(dev, "Probe Failed for port %d\n", i);
> break;
> }
> }
>
> err = add_adev(gd, "eth");
> ...
> }
>
> If add_adev() succeeds, mana_probe() completes successfully instead of
> failing, masking the earlier error while leaving ac->ports[0] as NULL.
> Later, when the driver is unloaded or if add_adev() fails and triggers
> immediate cleanup, mana_remove() is called. It iterates over ac->ports
> and, upon encountering the NULL device, immediately executes goto out:
>
> mana_remove() {
> ...
> for (i = 0; i < ac->num_ports; i++) {
> ndev = ac->ports[i];
> if (!ndev) {
> if (i == 0)
> ...
> goto out;
> }
> ...
> }
>
> mana_destroy_eq(ac);
> out:
> ...
> }
>
> Because the out label in mana_remove() is located after the
> mana_destroy_eq(ac) call, jumping there completely skips destroying the
> event queues allocated earlier by mana_create_eq(ac).
> In a Confidential Virtual Machine context, could an untrusted hypervisor
> repeatedly return invalid configs to continuously leak guest memory and
> hardware queues?
Thankyou for the review.
Since these issues are pre-existing, I will send it in a separate
patchset.
The patchset will also include the issues reported in:
[PATCH net-next] net: mana: hardening: Validate adapter_mtu from
MANA_QUERY_DEV_CONFIG
- Vennela
^ permalink raw reply
* Re: [PATCH net-next] net: mana: hardening: Validate adapter_mtu from MANA_QUERY_DEV_CONFIG
From: Erni Sri Satya Vennela @ 2026-03-31 18:00 UTC (permalink / raw)
To: Paolo Abeni
Cc: kys, haiyangz, wei.liu, decui, longli, andrew+netdev, davem,
edumazet, kuba, ssengar, dipayanroy, gargaditya, shirazsaleem,
kees, linux-hyperv, netdev, linux-kernel
In-Reply-To: <4ceecee2-ea5a-4026-ad38-66c0a7d263cd@redhat.com>
> > - if (resp.hdr.response.msg_version >= GDMA_MESSAGE_V2)
> > + if (resp.hdr.response.msg_version >= GDMA_MESSAGE_V2) {
> > + if (resp.adapter_mtu < ETH_MIN_MTU + ETH_HLEN) {
> > + dev_err(dev, "Adapter MTU too small: %u\n",
> > + resp.adapter_mtu);
> > + return -EPROTO;
>
> AI review says:
>
> If this returns -EPROTO, does the caller mana_probe() jump to an error
> label and call mana_remove()?
> If so, mana_remove() unconditionally calls
> disable_work_sync(&ac->link_change_work) and
> cancel_delayed_work_sync(&ac->gf_stats_work).
> Since mana_query_device_cfg() is called before INIT_WORK() and
> INIT_DELAYED_WORK() in the probe sequence, wouldn't this result in
> calling sync cancellation functions on uninitialized, zeroed work
> structures?
> This can lead to a WARN_ON(!work->func) in __flush_work(), or debug
> object warnings if CONFIG_DEBUG_OBJECTS_WORK is enabled.
> While this initialization issue appears to already exist for other early
> error paths, this new error path can also trigger it.
Thankyou for the review.
Since the issue is pre-existing. I will send it in a separate patchset.
The patchset will include the the issue reported with:
[PATCH net-next] net: mana: hardening: Reject zero max_num_queues from
MANA_QUERY_VPORT_CONFIG
- Vennela
^ permalink raw reply
* Re: [PATCH net-next v4] net: mana: Expose hardware diagnostic info via debugfs
From: Erni Sri Satya Vennela @ 2026-03-31 17:58 UTC (permalink / raw)
To: Jakub Kicinski
Cc: kys, haiyangz, wei.liu, decui, longli, andrew+netdev, davem,
edumazet, pabeni, kotaranov, horms, shradhagupta, shirazsaleem,
dipayanroy, yury.norov, kees, ssengar, gargaditya, linux-hyperv,
netdev, linux-kernel, linux-rdma
In-Reply-To: <20260330152318.144c1b30@kernel.org>
On Mon, Mar 30, 2026 at 03:23:18PM -0700, Jakub Kicinski wrote:
> On Mon, 30 Mar 2026 12:09:52 -0700 Erni Sri Satya Vennela wrote:
> > Just a quick follow‑up on this. Since these issues were pre‑existing and
> > not introduced by this patch, would you prefer that I send them as a
> > separate fix patch, or fold the fixes into the current patch?
>
> Anything that's pre-existing should be a separate patch, before any new
> code. If the bug exists only in net-next - earlier in the same series,
> if the bug exists in net - posted separately for the net tree.
Thankyou for the explanation.
I will send it in a separate patch.
- Vennela
^ permalink raw reply
* Re: [PATCH 06/12] platform/wmi: use generic driver_override infrastructure
From: Ilpo Järvinen @ 2026-03-31 15:50 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Russell King, Greg Kroah-Hartman, Rafael J. Wysocki,
Ioana Ciornei, Nipun Gupta, Nikhil Agarwal, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Bjorn Helgaas,
Armin Wolf, Bjorn Andersson, Mathieu Poirier, Vineeth Vijayan,
Peter Oberparleiter, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Harald Freudenberger, Holger Dengler, Mark Brown,
Michael S. Tsirkin, Jason Wang, Xuan Zhuo, Eugenio Pérez,
Alex Williamson, Juergen Gross, Stefano Stabellini,
Oleksandr Tyshchenko, Christophe Leroy (CS GROUP), LKML,
driver-core, linuxppc-dev, linux-hyperv, linux-pci,
platform-driver-x86, linux-arm-msm, linux-remoteproc, linux-s390,
linux-spi, virtualization, kvm, xen-devel, linux-arm-kernel,
Gui-Dong Han
In-Reply-To: <f819b7d8-5c80-4463-9afa-933a2ddc8ab3@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 635 bytes --]
On Tue, 31 Mar 2026, Danilo Krummrich wrote:
> On 3/31/26 5:02 PM, Ilpo Järvinen wrote:
> > I tried applying this to platform-drivers tree but it failed to compile so
> > I ended up dropping the changed.
>
> As the cover letter mentions, it sits on top of v7.0-rc5, did you consider this?
I noticed it but I just assumed you were working on top of that, not that
there's something past -rc1 that is required.
> I can also pick it up via the driver-core tree.
If there's some post -rc1 material this depends, it's probably better that
way.
Acked-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
--
i.
^ permalink raw reply
* Re: [PATCH 06/12] platform/wmi: use generic driver_override infrastructure
From: Danilo Krummrich @ 2026-03-31 15:43 UTC (permalink / raw)
To: Ilpo Järvinen
Cc: Russell King, Greg Kroah-Hartman, Rafael J. Wysocki,
Ioana Ciornei, Nipun Gupta, Nikhil Agarwal, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Bjorn Helgaas,
Armin Wolf, Bjorn Andersson, Mathieu Poirier, Vineeth Vijayan,
Peter Oberparleiter, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Harald Freudenberger, Holger Dengler, Mark Brown,
Michael S. Tsirkin, Jason Wang, Xuan Zhuo, Eugenio Pérez,
Alex Williamson, Juergen Gross, Stefano Stabellini,
Oleksandr Tyshchenko, Christophe Leroy (CS GROUP), LKML,
driver-core, linuxppc-dev, linux-hyperv, linux-pci,
platform-driver-x86, linux-arm-msm, linux-remoteproc, linux-s390,
linux-spi, virtualization, kvm, xen-devel, linux-arm-kernel,
Gui-Dong Han
In-Reply-To: <f15629e4-ef8f-b1b6-0158-064f40f111da@linux.intel.com>
On 3/31/26 5:02 PM, Ilpo Järvinen wrote:
> I tried applying this to platform-drivers tree but it failed to compile so
> I ended up dropping the changed.
As the cover letter mentions, it sits on top of v7.0-rc5, did you consider this?
I can also pick it up via the driver-core tree.
Thanks,
Danilo
^ permalink raw reply
* Re: [PATCH 06/12] platform/wmi: use generic driver_override infrastructure
From: Ilpo Järvinen @ 2026-03-31 15:02 UTC (permalink / raw)
To: Danilo Krummrich
Cc: Russell King, Greg Kroah-Hartman, Rafael J. Wysocki,
Ioana Ciornei, Nipun Gupta, Nikhil Agarwal, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Bjorn Helgaas,
Armin Wolf, Bjorn Andersson, Mathieu Poirier, Vineeth Vijayan,
Peter Oberparleiter, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Sven Schnelle,
Harald Freudenberger, Holger Dengler, Mark Brown,
Michael S. Tsirkin, Jason Wang, Xuan Zhuo, Eugenio Pérez,
Alex Williamson, Juergen Gross, Stefano Stabellini,
Oleksandr Tyshchenko, Christophe Leroy (CS GROUP), LKML,
driver-core, linuxppc-dev, linux-hyperv, linux-pci,
platform-driver-x86, linux-arm-msm, linux-remoteproc, linux-s390,
linux-spi, virtualization, kvm, xen-devel, linux-arm-kernel,
Gui-Dong Han
In-Reply-To: <20260324005919.2408620-7-dakr@kernel.org>
On Tue, 24 Mar 2026, Danilo Krummrich wrote:
> When a driver is probed through __driver_attach(), the bus' match()
> callback is called without the device lock held, thus accessing the
> driver_override field without a lock, which can cause a UAF.
>
> Fix this by using the driver-core driver_override infrastructure taking
> care of proper locking internally.
>
> Note that calling match() from __driver_attach() without the device lock
> held is intentional. [1]
>
> Link: https://lore.kernel.org/driver-core/DGRGTIRHA62X.3RY09D9SOK77P@kernel.org/ [1]
> Reported-by: Gui-Dong Han <hanguidong02@gmail.com>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220789
> Fixes: 12046f8c77e0 ("platform/x86: wmi: Add driver_override support")
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
> drivers/platform/wmi/core.c | 36 +++++-------------------------------
> include/linux/wmi.h | 4 ----
> 2 files changed, 5 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/platform/wmi/core.c b/drivers/platform/wmi/core.c
> index b8e6b9a421c6..750e3619724e 100644
> --- a/drivers/platform/wmi/core.c
> +++ b/drivers/platform/wmi/core.c
> @@ -842,39 +842,11 @@ static ssize_t expensive_show(struct device *dev,
> }
> static DEVICE_ATTR_RO(expensive);
>
> -static ssize_t driver_override_show(struct device *dev, struct device_attribute *attr,
> - char *buf)
> -{
> - struct wmi_device *wdev = to_wmi_device(dev);
> - ssize_t ret;
> -
> - device_lock(dev);
> - ret = sysfs_emit(buf, "%s\n", wdev->driver_override);
> - device_unlock(dev);
> -
> - return ret;
> -}
> -
> -static ssize_t driver_override_store(struct device *dev, struct device_attribute *attr,
> - const char *buf, size_t count)
> -{
> - struct wmi_device *wdev = to_wmi_device(dev);
> - int ret;
> -
> - ret = driver_set_override(dev, &wdev->driver_override, buf, count);
> - if (ret < 0)
> - return ret;
> -
> - return count;
> -}
> -static DEVICE_ATTR_RW(driver_override);
> -
> static struct attribute *wmi_attrs[] = {
> &dev_attr_modalias.attr,
> &dev_attr_guid.attr,
> &dev_attr_instance_count.attr,
> &dev_attr_expensive.attr,
> - &dev_attr_driver_override.attr,
> NULL
> };
> ATTRIBUTE_GROUPS(wmi);
> @@ -943,7 +915,6 @@ static void wmi_dev_release(struct device *dev)
> {
> struct wmi_block *wblock = dev_to_wblock(dev);
>
> - kfree(wblock->dev.driver_override);
> kfree(wblock);
> }
>
> @@ -952,10 +923,12 @@ static int wmi_dev_match(struct device *dev, const struct device_driver *driver)
> const struct wmi_driver *wmi_driver = to_wmi_driver(driver);
> struct wmi_block *wblock = dev_to_wblock(dev);
> const struct wmi_device_id *id = wmi_driver->id_table;
> + int ret;
>
> /* When driver_override is set, only bind to the matching driver */
> - if (wblock->dev.driver_override)
> - return !strcmp(wblock->dev.driver_override, driver->name);
> + ret = device_match_driver_override(dev, driver);
> + if (ret >= 0)
> + return ret;
>
> if (id == NULL)
> return 0;
> @@ -1076,6 +1049,7 @@ static struct class wmi_bus_class = {
> static const struct bus_type wmi_bus_type = {
> .name = "wmi",
> .dev_groups = wmi_groups,
> + .driver_override = true,
> .match = wmi_dev_match,
> .uevent = wmi_dev_uevent,
> .probe = wmi_dev_probe,
> diff --git a/include/linux/wmi.h b/include/linux/wmi.h
> index 75cb0c7cfe57..14fb644e1701 100644
> --- a/include/linux/wmi.h
> +++ b/include/linux/wmi.h
> @@ -18,16 +18,12 @@
> * struct wmi_device - WMI device structure
> * @dev: Device associated with this WMI device
> * @setable: True for devices implementing the Set Control Method
> - * @driver_override: Driver name to force a match; do not set directly,
> - * because core frees it; use driver_set_override() to
> - * set or clear it.
> *
> * This represents WMI devices discovered by the WMI driver core.
> */
> struct wmi_device {
> struct device dev;
> bool setable;
> - const char *driver_override;
> };
>
> /**
>
Hi,
I tried applying this to platform-drivers tree but it failed to compile so
I ended up dropping the changed.
--
i.
^ permalink raw reply
* Re: [PATCH net-next] net: mana: hardening: Reject zero max_num_queues from MANA_QUERY_VPORT_CONFIG
From: Paolo Abeni @ 2026-03-31 9:33 UTC (permalink / raw)
To: Erni Sri Satya Vennela, kys, haiyangz, wei.liu, decui, longli,
andrew+netdev, davem, edumazet, kuba, ssengar, dipayanroy,
gargaditya, shirazsaleem, kees, linux-hyperv, netdev,
linux-kernel
In-Reply-To: <20260326174815.2012137-1-ernis@linux.microsoft.com>
On 3/26/26 6:48 PM, Erni Sri Satya Vennela wrote:
> As a part of MANA hardening for CVM, validate that max_num_sq and
> max_num_rq returned by MANA_QUERY_VPORT_CONFIG are not zero. These
> values flow into apc->num_queues, which is used as an allocation count
> and loop bound. A zero value would result in zero-size allocations and
> incorrect driver behavior.
>
> Return -EPROTO if either value is zero.
>
> Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com>
> ---
> drivers/net/ethernet/microsoft/mana/mana_en.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index b39e8b920791..a4197b4b0597 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -1249,6 +1249,12 @@ static int mana_query_vport_cfg(struct mana_port_context *apc, u32 vport_index,
>
> *max_sq = resp.max_num_sq;
> *max_rq = resp.max_num_rq;
> +
> + if (*max_sq == 0 || *max_rq == 0) {
> + netdev_err(apc->ndev, "Invalid max queues from vPort config\n");
> + return -EPROTO;
AI review says:
Will returning -EPROTO here expose a pre-existing resource leak in the
driver's teardown path?
If mana_query_vport_cfg() returns an error, mana_init_port() fails and
mana_probe_port() frees the ndev, leaving ac->ports[i] as NULL. In
mana_probe(), the port initialization loop breaks upon this error, but
the err variable is then overwritten:
mana_probe() {
...
for (i = 0; i < ac->num_ports; i++) {
err = mana_probe_port(ac, i, &ac->ports[i]);
if (err) {
dev_err(dev, "Probe Failed for port %d\n", i);
break;
}
}
err = add_adev(gd, "eth");
...
}
If add_adev() succeeds, mana_probe() completes successfully instead of
failing, masking the earlier error while leaving ac->ports[0] as NULL.
Later, when the driver is unloaded or if add_adev() fails and triggers
immediate cleanup, mana_remove() is called. It iterates over ac->ports
and, upon encountering the NULL device, immediately executes goto out:
mana_remove() {
...
for (i = 0; i < ac->num_ports; i++) {
ndev = ac->ports[i];
if (!ndev) {
if (i == 0)
...
goto out;
}
...
}
mana_destroy_eq(ac);
out:
...
}
Because the out label in mana_remove() is located after the
mana_destroy_eq(ac) call, jumping there completely skips destroying the
event queues allocated earlier by mana_create_eq(ac).
In a Confidential Virtual Machine context, could an untrusted hypervisor
repeatedly return invalid configs to continuously leak guest memory and
hardware queues?
^ permalink raw reply
* Re: [PATCH net-next] net: mana: hardening: Validate adapter_mtu from MANA_QUERY_DEV_CONFIG
From: Paolo Abeni @ 2026-03-31 9:28 UTC (permalink / raw)
To: Erni Sri Satya Vennela, kys, haiyangz, wei.liu, decui, longli,
andrew+netdev, davem, edumazet, kuba, ssengar, dipayanroy,
gargaditya, shirazsaleem, kees, linux-hyperv, netdev,
linux-kernel
In-Reply-To: <20260326173101.2010514-1-ernis@linux.microsoft.com>
On 3/26/26 6:30 PM, Erni Sri Satya Vennela wrote:
> As a part of MANA hardening for CVM, validate the adapter_mtu value
> returned from the MANA_QUERY_DEV_CONFIG HWC command.
>
> The adapter_mtu value is used to compute ndev->max_mtu via:
> gc->adapter_mtu - ETH_HLEN. If hardware returns a bogus adapter_mtu
> smaller than ETH_HLEN (e.g. 0), the unsigned subtraction wraps to a
> huge value, silently allowing oversized MTU settings.
>
> Add a validation check to reject adapter_mtu values below
> ETH_MIN_MTU + ETH_HLEN, returning -EPROTO to fail the device
> configuration early with a clear error message.
>
> Signed-off-by: Erni Sri Satya Vennela <ernis@linux.microsoft.com>
> ---
> drivers/net/ethernet/microsoft/mana/mana_en.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index b39e8b920791..bd07d17a6017 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -1207,10 +1207,16 @@ static int mana_query_device_cfg(struct mana_context *ac, u32 proto_major_ver,
>
> *max_num_vports = resp.max_num_vports;
>
> - if (resp.hdr.response.msg_version >= GDMA_MESSAGE_V2)
> + if (resp.hdr.response.msg_version >= GDMA_MESSAGE_V2) {
> + if (resp.adapter_mtu < ETH_MIN_MTU + ETH_HLEN) {
> + dev_err(dev, "Adapter MTU too small: %u\n",
> + resp.adapter_mtu);
> + return -EPROTO;
AI review says:
If this returns -EPROTO, does the caller mana_probe() jump to an error
label and call mana_remove()?
If so, mana_remove() unconditionally calls
disable_work_sync(&ac->link_change_work) and
cancel_delayed_work_sync(&ac->gf_stats_work).
Since mana_query_device_cfg() is called before INIT_WORK() and
INIT_DELAYED_WORK() in the probe sequence, wouldn't this result in
calling sync cancellation functions on uninitialized, zeroed work
structures?
This can lead to a WARN_ON(!work->func) in __flush_work(), or debug
object warnings if CONFIG_DEBUG_OBJECTS_WORK is enabled.
While this initialization issue appears to already exist for other early
error paths, this new error path can also trigger it.
^ permalink raw reply
* Re: [PATCH v2 00/16] Update drivers to use ib_copy_validate_udata_in()
From: Leon Romanovsky @ 2026-03-31 8:35 UTC (permalink / raw)
To: Abhijit Gangurde, Allen Hubbe,
Broadcom internal kernel review list, Bernard Metzler, Bryan Tan,
Cheng Xu, Gal Pressman, Junxian Huang, Kai Shen,
Konstantin Taranov, Krzysztof Czurylo, linux-hyperv, linux-rdma,
Michal Kalderon, Michael Margolin, Nelson Escobar, Satish Kharat,
Selvin Xavier, Yossi Leybovich, Chengchang Tang, Tatyana Nikolova,
Vishnu Dasa, Yishai Hadas, Zhu Yanjun, Jason Gunthorpe
Cc: Long Li, patches
In-Reply-To: <0-v2-f4ac6f418bd6+12c5-rdma_udata_req_jgg@nvidia.com>
On Wed, 25 Mar 2026 18:26:46 -0300, Jason Gunthorpe wrote:
> Progress the uAPI work by shifting nearly all drivers to use
> ib_copy_validate_udata_in() and its variations.
>
> These helpers are easier to use and enforce a tighter uAPI protocol
> for the udata.
>
> v2:
> - Drop EFA patch, rename the field instead
> - Fix the mlx5 mw change, userspace doesn't use the udata struct at all
> v1: https://patch.msgid.link/r/0-v1-2b86f54cda42+7d-rdma_udata_req_jgg@nvidia.com
>
> [...]
Applied, thanks!
[01/16] RDMA: Consolidate patterns with offsetofend() to ib_copy_validate_udata_in()
(no commit info)
[02/16] RDMA: Consolidate patterns with offsetof() to ib_copy_validate_udata_in()
(no commit info)
[03/16] RDMA: Consolidate patterns with sizeof() to ib_copy_validate_udata_in()
(no commit info)
[04/16] RDMA: Use ib_copy_validate_udata_in() for implicit full structs
(no commit info)
[05/16] RDMA/pvrdma: Use ib_copy_validate_udata_in() for srq
(no commit info)
[06/16] RDMA/mlx5: Use ib_copy_validate_udata_in() for SRQ
(no commit info)
[07/16] RDMA/mlx5: Use ib_copy_validate_udata_in() for MW
(no commit info)
[08/16] RDMA/mlx4: Use ib_copy_validate_udata_in()
(no commit info)
[09/16] RDMA/mlx4: Use ib_copy_validate_udata_in() for QP
(no commit info)
[10/16] RDMA/hns: Use ib_copy_validate_udata_in()
(no commit info)
[11/16] RDMA: Use ib_copy_validate_udata_in_cm() for zero comp_mask
(no commit info)
[12/16] RDMA/mlx5: Pull comp_mask validation into ib_copy_validate_udata_in_cm()
(no commit info)
[13/16] RDMA/hns: Add missing comp_mask check in create_qp
(no commit info)
[14/16] RDMA/irdma: Add missing comp_mask check in alloc_ucontext
(no commit info)
[15/16] RDMA: Remove redundant = {} for udata req structs
(no commit info)
[16/16] RDMA/hns: Remove the duplicate calls to ib_copy_validate_udata_in()
(no commit info)
Best regards,
--
Leon Romanovsky <leon@kernel.org>
^ permalink raw reply
* Re: [PATCH 05/12] PCI: use generic driver_override infrastructure
From: Danilo Krummrich @ 2026-03-31 8:22 UTC (permalink / raw)
To: Alex Williamson
Cc: Jason Gunthorpe, Russell King, Greg Kroah-Hartman,
Rafael J. Wysocki, Ioana Ciornei, Nipun Gupta, Nikhil Agarwal,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Bjorn Helgaas, Armin Wolf, Bjorn Andersson, Mathieu Poirier,
Vineeth Vijayan, Peter Oberparleiter, Heiko Carstens,
Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
Sven Schnelle, Harald Freudenberger, Holger Dengler, Mark Brown,
Michael S. Tsirkin, Jason Wang, Xuan Zhuo, Eugenio Pérez,
Juergen Gross, Stefano Stabellini, Oleksandr Tyshchenko,
Christophe Leroy (CS GROUP), linux-kernel, driver-core,
linuxppc-dev, linux-hyperv, linux-pci, platform-driver-x86,
linux-arm-msm, linux-remoteproc, linux-s390, linux-spi,
virtualization, kvm, xen-devel, linux-arm-kernel, Gui-Dong Han
In-Reply-To: <DHGT9XCG8Y96.3IB1EI6FF1ZDZ@kernel.org>
On Tue Mar 31, 2026 at 10:06 AM CEST, Danilo Krummrich wrote:
> On Mon Mar 30, 2026 at 10:10 PM CEST, Alex Williamson wrote:
>> On Mon, 30 Mar 2026 19:38:41 +0200
>> "Danilo Krummrich" <dakr@kernel.org> wrote:
>>
>>> (Cc: Jason)
>>>
>>> On Tue Mar 24, 2026 at 1:59 AM CET, Danilo Krummrich wrote:
>>> > diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
>>> > index d43745fe4c84..460852f79f29 100644
>>> > --- a/drivers/vfio/pci/vfio_pci_core.c
>>> > +++ b/drivers/vfio/pci/vfio_pci_core.c
>>> > @@ -1987,9 +1987,8 @@ static int vfio_pci_bus_notifier(struct notifier_block *nb,
>>> > pdev->is_virtfn && physfn == vdev->pdev) {
>>> > pci_info(vdev->pdev, "Captured SR-IOV VF %s driver_override\n",
>>> > pci_name(pdev));
>>> > - pdev->driver_override = kasprintf(GFP_KERNEL, "%s",
>>> > - vdev->vdev.ops->name);
>>> > - WARN_ON(!pdev->driver_override);
>>> > + WARN_ON(device_set_driver_override(&pdev->dev,
>>> > + vdev->vdev.ops->name));
>>>
>>> Technically, this is a change in behavior. If vdev->vdev.ops->name is NULL, it
>>> will trigger the WARN_ON(), whereas before it would have just written "(null)"
>>> into driver_override.
>>
>> It's worse than that. Looking at the implementation in [1], we have:
>>
>> +static inline int device_set_driver_override(struct device *dev, const char *s)
>> +{
>> + return __device_set_driver_override(dev, s, strlen(s));
>> +}
>>
>> So if name is NULL, we oops in strlen() before we even hit the -EINVAL
>> and WARN_ON().
>
> This was changed in v2 [2] and the actual code in-tree is
>
> static inline int device_set_driver_override(struct device *dev, const char *s)
> {
> return __device_set_driver_override(dev, s, s ? strlen(s) : 0);
> }
>
> so it does indeed return -EINVAL for a NULL pointer.
>
>> I don't believe we have any vfio-pci variant drivers where the name is
>> NULL, but kasprintf() handling NULL as "(null)" was a consideration in
>> this design, that even if there is no name the device is sequestered
>> with a driver_override that won't match an actual driver.
>>
>>> I assume that vfio_pci_core drivers are expected to set the name in struct
>>> vfio_device_ops in the first place and this code (silently) relies on this
>>> invariant?
>>
>> We do expect that, but it was previously safe either way to make sure
>> VFs are only bound to the same ops driver or barring that, at least
>> don't perform a standard driver match. The last thing we want to
>> happen automatically is for a user owned PF to create SR-IOV VFs that
>> automatically bind to native kernel drivers.
>>
>>> Alex, Jason: Should we keep this hunk above as is and check for a proper name in
>>> struct vfio_device_ops in vfio_pci_core_register_device() with a subsequent
>>> patch?
>>
>> Given the oops, my preference would be to roll it in here. This change
>> is what makes it a requirement that name cannot be NULL, where this was
>> safely handled with kasprintf().
>
> Again, no oops here. :)
>
> I still think it makes more sense to fail early in
> vfio_pci_core_register_device(), rather than silently accept "(null)" in
> driver_override. It also doesn't seem unreasonable with only the WARN_ON(), but
> I can also just add vdev->vdev.ops->name ?: "(null)".
(Or just skip the call if !vdev->vdev.ops->name, as a user will read "(null)"
from sysfs either way.)
> Please let me know what you prefer.
>
> - Danilo
>
>> [1] https://lore.kernel.org/all/20260302002729.19438-2-dakr@kernel.org/
>
> [2] https://lore.kernel.org/driver-core/20260303115720.48783-1-dakr@kernel.org/
^ permalink raw reply
* Re: [PATCH 05/12] PCI: use generic driver_override infrastructure
From: Danilo Krummrich @ 2026-03-31 8:06 UTC (permalink / raw)
To: Alex Williamson
Cc: Jason Gunthorpe, Russell King, Greg Kroah-Hartman,
Rafael J. Wysocki, Ioana Ciornei, Nipun Gupta, Nikhil Agarwal,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Bjorn Helgaas, Armin Wolf, Bjorn Andersson, Mathieu Poirier,
Vineeth Vijayan, Peter Oberparleiter, Heiko Carstens,
Vasily Gorbik, Alexander Gordeev, Christian Borntraeger,
Sven Schnelle, Harald Freudenberger, Holger Dengler, Mark Brown,
Michael S. Tsirkin, Jason Wang, Xuan Zhuo, Eugenio Pérez,
Juergen Gross, Stefano Stabellini, Oleksandr Tyshchenko,
Christophe Leroy (CS GROUP), linux-kernel, driver-core,
linuxppc-dev, linux-hyperv, linux-pci, platform-driver-x86,
linux-arm-msm, linux-remoteproc, linux-s390, linux-spi,
virtualization, kvm, xen-devel, linux-arm-kernel, Gui-Dong Han
In-Reply-To: <20260330141050.2cb47bd9@shazbot.org>
On Mon Mar 30, 2026 at 10:10 PM CEST, Alex Williamson wrote:
> On Mon, 30 Mar 2026 19:38:41 +0200
> "Danilo Krummrich" <dakr@kernel.org> wrote:
>
>> (Cc: Jason)
>>
>> On Tue Mar 24, 2026 at 1:59 AM CET, Danilo Krummrich wrote:
>> > diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
>> > index d43745fe4c84..460852f79f29 100644
>> > --- a/drivers/vfio/pci/vfio_pci_core.c
>> > +++ b/drivers/vfio/pci/vfio_pci_core.c
>> > @@ -1987,9 +1987,8 @@ static int vfio_pci_bus_notifier(struct notifier_block *nb,
>> > pdev->is_virtfn && physfn == vdev->pdev) {
>> > pci_info(vdev->pdev, "Captured SR-IOV VF %s driver_override\n",
>> > pci_name(pdev));
>> > - pdev->driver_override = kasprintf(GFP_KERNEL, "%s",
>> > - vdev->vdev.ops->name);
>> > - WARN_ON(!pdev->driver_override);
>> > + WARN_ON(device_set_driver_override(&pdev->dev,
>> > + vdev->vdev.ops->name));
>>
>> Technically, this is a change in behavior. If vdev->vdev.ops->name is NULL, it
>> will trigger the WARN_ON(), whereas before it would have just written "(null)"
>> into driver_override.
>
> It's worse than that. Looking at the implementation in [1], we have:
>
> +static inline int device_set_driver_override(struct device *dev, const char *s)
> +{
> + return __device_set_driver_override(dev, s, strlen(s));
> +}
>
> So if name is NULL, we oops in strlen() before we even hit the -EINVAL
> and WARN_ON().
This was changed in v2 [2] and the actual code in-tree is
static inline int device_set_driver_override(struct device *dev, const char *s)
{
return __device_set_driver_override(dev, s, s ? strlen(s) : 0);
}
so it does indeed return -EINVAL for a NULL pointer.
> I don't believe we have any vfio-pci variant drivers where the name is
> NULL, but kasprintf() handling NULL as "(null)" was a consideration in
> this design, that even if there is no name the device is sequestered
> with a driver_override that won't match an actual driver.
>
>> I assume that vfio_pci_core drivers are expected to set the name in struct
>> vfio_device_ops in the first place and this code (silently) relies on this
>> invariant?
>
> We do expect that, but it was previously safe either way to make sure
> VFs are only bound to the same ops driver or barring that, at least
> don't perform a standard driver match. The last thing we want to
> happen automatically is for a user owned PF to create SR-IOV VFs that
> automatically bind to native kernel drivers.
>
>> Alex, Jason: Should we keep this hunk above as is and check for a proper name in
>> struct vfio_device_ops in vfio_pci_core_register_device() with a subsequent
>> patch?
>
> Given the oops, my preference would be to roll it in here. This change
> is what makes it a requirement that name cannot be NULL, where this was
> safely handled with kasprintf().
Again, no oops here. :)
I still think it makes more sense to fail early in
vfio_pci_core_register_device(), rather than silently accept "(null)" in
driver_override. It also doesn't seem unreasonable with only the WARN_ON(), but
I can also just add vdev->vdev.ops->name ?: "(null)".
Please let me know what you prefer.
- Danilo
> [1] https://lore.kernel.org/all/20260302002729.19438-2-dakr@kernel.org/
[2] https://lore.kernel.org/driver-core/20260303115720.48783-1-dakr@kernel.org/
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox