Linux-HyperV List
 help / color / mirror / Atom feed
* [PATCH v3 5/6] arm64: hyperv: Route hypercalls through RSI host call in CCA Realms
From: Kameron Carr @ 2026-07-21 20:11 UTC (permalink / raw)
  To: kys, haiyangz, wei.liu, decui, longli, mhklinux
  Cc: catalin.marinas, will, mark.rutland, lpieralisi, sudeep.holla,
	arnd, thuth, linux-hyperv, linux-arm-kernel, linux-kernel,
	linux-arch
In-Reply-To: <20260721201148.1441143-1-kameroncarr@linux.microsoft.com>

Modify the five hypercall wrapper functions to check is_realm_world()
and use the per-CPU rsi_host_call structure when inside a Realm.

Signed-off-by: Kameron Carr <kameroncarr@linux.microsoft.com>
---
 arch/arm64/hyperv/hv_core.c | 155 ++++++++++++++++++++++++++++--------
 1 file changed, 121 insertions(+), 34 deletions(-)

diff --git a/arch/arm64/hyperv/hv_core.c b/arch/arm64/hyperv/hv_core.c
index e33a9e3c366a1..77cba08fca132 100644
--- a/arch/arm64/hyperv/hv_core.c
+++ b/arch/arm64/hyperv/hv_core.c
@@ -13,9 +13,41 @@
 #include <linux/mm.h>
 #include <linux/arm-smccc.h>
 #include <linux/module.h>
+#include <linux/smp.h>
 #include <asm-generic/bug.h>
 #include <hyperv/hvhdk.h>
 #include <asm/mshyperv.h>
+#include <asm/rsi.h>
+
+/*
+ * hv_do_rsi_hypercall - Helper function to invoke a hypercall from a
+ * Realm world using the RSI interface.
+ */
+static u64 hv_do_rsi_hypercall(u64 control, u64 input1, u64 input2)
+{
+	struct rsi_host_call *hostcall;
+	unsigned long flags;
+	u64 ret;
+
+	if (!hv_hostcall_array)
+		return HV_STATUS_INVALID_HYPERCALL_INPUT;
+
+	local_irq_save(flags);
+	hostcall = &hv_hostcall_array[smp_processor_id()];
+	memset(hostcall, 0, sizeof(*hostcall));
+	hostcall->gprs[0] = HV_FUNC_ID;
+	hostcall->gprs[1] = control;
+	hostcall->gprs[2] = input1;
+	hostcall->gprs[3] = input2;
+
+	if (rsi_host_call(virt_to_phys(hostcall)) == RSI_SUCCESS)
+		ret = hostcall->gprs[0];
+	else
+		ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
+
+	local_irq_restore(flags);
+	return ret;
+}
 
 /*
  * hv_do_hypercall- Invoke the specified hypercall
@@ -29,8 +61,11 @@ u64 hv_do_hypercall(u64 control, void *input, void *output)
 	input_address = input ? virt_to_phys(input) : 0;
 	output_address = output ? virt_to_phys(output) : 0;
 
-	arm_smccc_1_1_hvc(HV_FUNC_ID, control,
-			  input_address, output_address, &res);
+	if (is_realm_world())
+		return hv_do_rsi_hypercall(control, input_address, output_address);
+
+	arm_smccc_1_1_hvc(HV_FUNC_ID, control, input_address,
+			  output_address, &res);
 	return res.a0;
 }
 EXPORT_SYMBOL_GPL(hv_do_hypercall);
@@ -48,6 +83,9 @@ u64 hv_do_fast_hypercall8(u16 code, u64 input)
 
 	control = (u64)code | HV_HYPERCALL_FAST_BIT;
 
+	if (is_realm_world())
+		return hv_do_rsi_hypercall(control, input, 0);
+
 	arm_smccc_1_1_hvc(HV_FUNC_ID, control, input, &res);
 	return res.a0;
 }
@@ -65,6 +103,9 @@ u64 hv_do_fast_hypercall16(u16 code, u64 input1, u64 input2)
 
 	control = (u64)code | HV_HYPERCALL_FAST_BIT;
 
+	if (is_realm_world())
+		return hv_do_rsi_hypercall(control, input1, input2);
+
 	arm_smccc_1_1_hvc(HV_FUNC_ID, control, input1, input2, &res);
 	return res.a0;
 }
@@ -76,24 +117,44 @@ EXPORT_SYMBOL_GPL(hv_do_fast_hypercall16);
 void hv_set_vpreg(u32 msr, u64 value)
 {
 	struct arm_smccc_res res;
+	struct rsi_host_call *hostcall;
+	unsigned long flags;
+	u64 status;
 
-	arm_smccc_1_1_hvc(HV_FUNC_ID,
-		HVCALL_SET_VP_REGISTERS | HV_HYPERCALL_FAST_BIT |
-			HV_HYPERCALL_REP_COMP_1,
-		HV_PARTITION_ID_SELF,
-		HV_VP_INDEX_SELF,
-		msr,
-		0,
-		value,
-		0,
-		&res);
+	if (is_realm_world()) {
+		local_irq_save(flags);
+		hostcall = &hv_hostcall_array[smp_processor_id()];
+		memset(hostcall, 0, sizeof(*hostcall));
+		hostcall->gprs[0] = HV_FUNC_ID;
+		hostcall->gprs[1] = HVCALL_SET_VP_REGISTERS |
+				    HV_HYPERCALL_FAST_BIT |
+				    HV_HYPERCALL_REP_COMP_1;
+		hostcall->gprs[2] = HV_PARTITION_ID_SELF;
+		hostcall->gprs[3] = HV_VP_INDEX_SELF;
+		hostcall->gprs[4] = msr;
+		hostcall->gprs[6] = value;
+
+		if (rsi_host_call(virt_to_phys(hostcall)) == RSI_SUCCESS)
+			status = hostcall->gprs[0];
+		else
+			status = HV_STATUS_INVALID_HYPERCALL_INPUT;
+		local_irq_restore(flags);
+	} else {
+		arm_smccc_1_1_hvc(HV_FUNC_ID,
+				  HVCALL_SET_VP_REGISTERS |
+					  HV_HYPERCALL_FAST_BIT |
+					  HV_HYPERCALL_REP_COMP_1,
+				  HV_PARTITION_ID_SELF, HV_VP_INDEX_SELF, msr,
+				  0, value, 0, &res);
+		status = res.a0;
+	}
 
 	/*
-	 * Something is fundamentally broken in the hypervisor if
-	 * setting a VP register fails. There's really no way to
-	 * continue as a guest VM, so panic.
+	 * Something is fundamentally broken in the hypervisor (or, in a
+	 * Realm, the RMM denied the host call) if setting a VP register
+	 * fails. There's really no way to continue as a guest VM, so panic.
 	 */
-	BUG_ON(!hv_result_success(res.a0));
+	BUG_ON(!hv_result_success(status));
 }
 EXPORT_SYMBOL_GPL(hv_set_vpreg);
 
@@ -108,29 +169,55 @@ void hv_get_vpreg_128(u32 msr, struct hv_get_vp_registers_output *result)
 {
 	struct arm_smccc_1_2_regs args;
 	struct arm_smccc_1_2_regs res;
+	struct rsi_host_call *hostcall;
+	unsigned long flags;
+	u64 status;
 
-	args.a0 = HV_FUNC_ID;
-	args.a1 = HVCALL_GET_VP_REGISTERS | HV_HYPERCALL_FAST_BIT |
-			HV_HYPERCALL_REP_COMP_1;
-	args.a2 = HV_PARTITION_ID_SELF;
-	args.a3 = HV_VP_INDEX_SELF;
-	args.a4 = msr;
+	if (is_realm_world()) {
+		local_irq_save(flags);
+		hostcall = &hv_hostcall_array[smp_processor_id()];
+		memset(hostcall, 0, sizeof(*hostcall));
 
-	/*
-	 * Use the SMCCC 1.2 interface because the results are in registers
-	 * beyond X0-X3.
-	 */
-	arm_smccc_1_2_hvc(&args, &res);
+		hostcall->gprs[0] = HV_FUNC_ID;
+		hostcall->gprs[1] = HVCALL_GET_VP_REGISTERS |
+				    HV_HYPERCALL_FAST_BIT |
+				    HV_HYPERCALL_REP_COMP_1;
+		hostcall->gprs[2] = HV_PARTITION_ID_SELF;
+		hostcall->gprs[3] = HV_VP_INDEX_SELF;
+		hostcall->gprs[4] = msr;
+
+		if (rsi_host_call(virt_to_phys(hostcall)) == RSI_SUCCESS) {
+			status = hostcall->gprs[0];
+			result->as64.low = hostcall->gprs[6];
+			result->as64.high = hostcall->gprs[7];
+		} else {
+			status = HV_STATUS_INVALID_HYPERCALL_INPUT;
+		}
+		local_irq_restore(flags);
+	} else {
+		args.a0 = HV_FUNC_ID;
+		args.a1 = HVCALL_GET_VP_REGISTERS | HV_HYPERCALL_FAST_BIT |
+			  HV_HYPERCALL_REP_COMP_1;
+		args.a2 = HV_PARTITION_ID_SELF;
+		args.a3 = HV_VP_INDEX_SELF;
+		args.a4 = msr;
+
+		/*
+		 * Use the SMCCC 1.2 interface because the results are in
+		 * registers beyond X0-X3.
+		 */
+		arm_smccc_1_2_hvc(&args, &res);
+		status = res.a0;
+		result->as64.low = res.a6;
+		result->as64.high = res.a7;
+	}
 
 	/*
-	 * Something is fundamentally broken in the hypervisor if
-	 * getting a VP register fails. There's really no way to
-	 * continue as a guest VM, so panic.
+	 * Something is fundamentally broken in the hypervisor (or, in a
+	 * Realm, the RMM denied the host call) if getting a VP register
+	 * fails. There's really no way to continue as a guest VM, so panic.
 	 */
-	BUG_ON(!hv_result_success(res.a0));
-
-	result->as64.low = res.a6;
-	result->as64.high = res.a7;
+	BUG_ON(!hv_result_success(status));
 }
 EXPORT_SYMBOL_GPL(hv_get_vpreg_128);
 
-- 
2.45.4


^ permalink raw reply related

* [PATCH v3 6/6] arm64: hyperv: Implement hv_is_isolation_supported() for CCA Realms
From: Kameron Carr @ 2026-07-21 20:11 UTC (permalink / raw)
  To: kys, haiyangz, wei.liu, decui, longli, mhklinux
  Cc: catalin.marinas, will, mark.rutland, lpieralisi, sudeep.holla,
	arnd, thuth, linux-hyperv, linux-arm-kernel, linux-kernel,
	linux-arch
In-Reply-To: <20260721201148.1441143-1-kameroncarr@linux.microsoft.com>

Provide an arm64 implementation of hv_is_isolation_supported() that
overrides the __weak default in drivers/hv/hv_common.c.

The implementation deliberately does not depend on
hv_is_hyperv_initialized() because hv_common_init() consults
hv_is_isolation_supported() before hyperv_initialized is set.

Signed-off-by: Kameron Carr <kameroncarr@linux.microsoft.com>
---
 arch/arm64/hyperv/mshyperv.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/hyperv/mshyperv.c b/arch/arm64/hyperv/mshyperv.c
index 8519f694f2897..e28612255a970 100644
--- a/arch/arm64/hyperv/mshyperv.c
+++ b/arch/arm64/hyperv/mshyperv.c
@@ -163,3 +163,8 @@ bool hv_isolation_type_cca(void)
 {
 	return is_realm_world();
 }
+
+bool hv_is_isolation_supported(void)
+{
+	return is_realm_world();
+}
-- 
2.45.4


^ permalink raw reply related

* Re: [PATCH v3 1/6] arm64: rsi: Add RSI host call structure and helper function
From: sashiko-bot @ 2026-07-21 20:22 UTC (permalink / raw)
  To: Kameron Carr; +Cc: linux-hyperv
In-Reply-To: <20260721201148.1441143-2-kameroncarr@linux.microsoft.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `rsi_host_call` structure uses native integer types instead of explicit little-endian types, which can cause an ABI mismatch on big-endian kernels.
--

--- Patch [1]: [PATCH v3 1/6] arm64: rsi: Add RSI host call structure and helper function ---
commit 47a2fc3b0f72dc993f44827b221ab5eb02bbcb36
Author: Kameron Carr <kameroncarr@linux.microsoft.com>

arm64: rsi: Add RSI host call structure and helper function

This commit introduces the rsi_host_call structure and a helper function to
invoke SMC_RSI_HOST_CALL. This allows Hyper-V guest code to route hypercalls
through the RSI interface when running inside an Arm CCA Realm.

[ ... ]

> diff --git a/arch/arm64/include/asm/rsi_smc.h b/arch/arm64/include/asm/rsi_smc.h
> index e19253f96c940..9cc57b5be0c02 100644
> --- a/arch/arm64/include/asm/rsi_smc.h
> +++ b/arch/arm64/include/asm/rsi_smc.h
> @@ -142,6 +142,13 @@ struct realm_config {
>  	 */
>  } __aligned(0x1000);
>  
> +struct rsi_host_call {
> +	u16 immediate;
> +	u8 _padding[6];
> +	u64 gprs[31];
> +} __aligned(256);

[Severity: Medium]
Should this structure use explicit little-endian types (__le16, __le64)
instead of native integer types?

If a big-endian kernel constructs this structure in memory and issues the
SMC, won't the Realm Management Monitor read byte-swapped values? The CCA
RMM specification dictates that memory structures shared between the Realm
and the RMM must be little-endian, so using native types could cause an ABI
mismatch.

> +static_assert(sizeof(struct rsi_host_call) == 256);
> +
>  #endif /* __ASSEMBLER__ */
>  
>  /*

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721201148.1441143-1-kameroncarr@linux.microsoft.com?part=1

^ permalink raw reply

* Re: [PATCH v3 4/6] Drivers: hv: Mark shared memory as decrypted for CCA Realms
From: sashiko-bot @ 2026-07-21 20:24 UTC (permalink / raw)
  To: Kameron Carr; +Cc: linux-hyperv
In-Reply-To: <20260721201148.1441143-5-kameroncarr@linux.microsoft.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Using `kmalloc()` for memory passed to `set_memory_decrypted()` causes silent memory corruption on x86 and initialization failures on ARM64 due to a lack of strict page alignment and KASAN/MTE pointer tagging.
--

--- Patch [4]: [PATCH v3 4/6] Drivers: hv: Mark shared memory as decrypted for CCA Realms ---
commit 21b5c4cec77eb56971c17ededee7133820fad287
Author: Kameron Carr <kameroncarr@linux.microsoft.com>

Drivers: hv: Mark shared memory as decrypted for CCA Realms

This patch extends confidential VM isolation memory decryption to Arm CCA Realm
guests during hypercall memory initialization. It introduces a helper for CCA
isolation and updates the hypercall buffer allocation path to use it.

> diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
> index 6b67ac6167891..0f9412fcb9571 100644
> --- a/drivers/hv/hv_common.c
> +++ b/drivers/hv/hv_common.c
[ ... ]
> @@ -489,7 +490,7 @@ int hv_common_cpu_init(unsigned int cpu)
>  	 * online and then taken offline
>  	 */
>  	if (!*inputarg) {
> -		mem = kmalloc_array(pgcount, HV_HYP_PAGE_SIZE, flags);
> +		mem = kmalloc(alloc_size, flags);
>  		if (!mem)
>  			return -ENOMEM;
>  
> @@ -499,14 +500,16 @@ int hv_common_cpu_init(unsigned int cpu)
>  		}
>  
>  		if (!ms_hyperv.paravisor_present &&
> -		    (hv_isolation_type_snp() || hv_isolation_type_tdx())) {
> -			ret = set_memory_decrypted((unsigned long)mem, pgcount);
> +		    (hv_isolation_type_snp() || hv_isolation_type_tdx() ||
> +		     hv_isolation_type_cca())) {
> +			ret = set_memory_decrypted((unsigned long)mem,
> +						   alloc_size >> PAGE_SHIFT);

[Severity: High]
This isn't a bug introduced by this patch, but is it safe to pass memory 
allocated by kmalloc to set_memory_decrypted() in hv_common_cpu_init()?

When kmalloc is used with SLUB_DEBUG, the returned pointer is not strictly 
page-aligned. Because set_memory_decrypted() operates on page frames and
rounds down to the page boundary, this could silently decrypt adjacent slab
objects or metadata on x86 architectures.

Additionally, on arm64 systems with MTE enabled, kmalloc returns a tagged 
pointer. Passing a tagged pointer to set_memory_decrypted() causes its
internal __is_lm_address() check to fail, leading to initialization failures
during boot or CPU hotplug for CCA guests.

Would it be better to allocate this memory using the page allocator, such as 
with __get_free_pages(), to ensure the address is strictly page-aligned and 
untagged?

>  			if (ret) {
>  				/* It may be unsafe to free 'mem' */
>  				return ret;
>  			}
>  
> -			memset(mem, 0x00, pgcount * HV_HYP_PAGE_SIZE);
> +			memset(mem, 0x00, alloc_size);
>  		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721201148.1441143-1-kameroncarr@linux.microsoft.com?part=4

^ permalink raw reply

* Re: [PATCH v3 2/6] firmware: smccc: Detect hypervisor via RSI host call in CCA Realms
From: sashiko-bot @ 2026-07-21 20:24 UTC (permalink / raw)
  To: Kameron Carr; +Cc: linux-hyperv
In-Reply-To: <20260721201148.1441143-3-kameroncarr@linux.microsoft.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing compiler memory barrier causes dead-store elimination and constant-folding of the RSI host call buffer.
--

--- Patch [2]: [PATCH v3 2/6] firmware: smccc: Detect hypervisor via RSI host call in CCA Realms ---
commit 5f8398cb1bc3327a58d70463ef37d994ffacca02
Author: Kameron Carr <kameroncarr@linux.microsoft.com>

firmware: smccc: Detect hypervisor via RSI host call in CCA Realms

This commit modifies arm_smccc_hypervisor_has_uuid() to use rsi_host_call()
when operating in a Realm. It introduces arm_smccc_realm_get_hypervisor_uuid()
which uses a spinlock-protected file-static buffer to make the call.

> diff --git a/drivers/firmware/smccc/smccc.c b/drivers/firmware/smccc/smccc.c
> index bdee057db2fd3..a876b7aa2dc99 100644
> --- a/drivers/firmware/smccc/smccc.c
> +++ b/drivers/firmware/smccc/smccc.c
[ ... ]
> @@ -67,12 +73,45 @@ s32 arm_smccc_get_soc_id_revision(void)
>  }
>  EXPORT_SYMBOL_GPL(arm_smccc_get_soc_id_revision);
>  
> +#ifdef CONFIG_ARM64
> +static struct rsi_host_call uuid_hc;
> +static DEFINE_SPINLOCK(uuid_hc_lock);
> +
> +/*
> + * Helper function to get the hypervisor UUID via an RsiHostCall.
> + */
> +static void arm_smccc_realm_get_hypervisor_uuid(struct arm_smccc_res *res)
> +{
> +	guard(spinlock_irqsave)(&uuid_hc_lock);
> +
> +	memset(&uuid_hc, 0, sizeof(uuid_hc));
> +	uuid_hc.gprs[0] = ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID;
> +
> +	if (rsi_host_call(__pa_symbol(&uuid_hc)) != RSI_SUCCESS) {
> +		res->a0 = SMCCC_RET_NOT_SUPPORTED;
> +		return;
> +	}
> +
> +	res->a0 = uuid_hc.gprs[0];

[Severity: High]
Does this code need a compiler memory barrier before reading back the results?

Because __pa_symbol() uses RELOC_HIDE() to explicitly hide the pointer's
provenance from the compiler's alias analysis, the compiler may not realize
that rsi_host_call() modifies uuid_hc.

Without a barrier(), could the compiler constant-fold the reads to their
statically-known previous values? If so, this could result in res->a0 being
optimized to ARM_SMCCC_VENDOR_HYP_CALL_UID_FUNC_ID instead of the value
returned by the hypervisor, and potentially allow the stores during
initialization to be dead-store eliminated.

> +	res->a1 = uuid_hc.gprs[1];
> +	res->a2 = uuid_hc.gprs[2];
> +	res->a3 = uuid_hc.gprs[3];
> +}
> +#endif

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721201148.1441143-1-kameroncarr@linux.microsoft.com?part=2

^ permalink raw reply

* Re: [PATCH v3 3/6] arm64: hyperv: Add per-CPU RSI host call infrastructure for CCA Realms
From: sashiko-bot @ 2026-07-21 20:25 UTC (permalink / raw)
  To: Kameron Carr; +Cc: linux-hyperv
In-Reply-To: <20260721201148.1441143-4-kameroncarr@linux.microsoft.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] kcalloc does not guarantee the 256-byte alignment required by the RSI host call structure, leading to RSI hypercall failures and boot crashes.
- [Low] Unnecessary complex error handling and cleanup logic for an early boot memory allocation failure.
--

--- Patch [3]: [PATCH v3 3/6] arm64: hyperv: Add per-CPU RSI host call infrastructure for CCA Realms ---
commit d6de874ec14c2f54fe70b4639d2b20bb51fd11c9
Author: Kameron Carr <kameroncarr@linux.microsoft.com>

arm64: hyperv: Add per-CPU RSI host call infrastructure for CCA Realms

This patch introduces a per-CPU array of rsi_host_call structures to
support Hyper-V hypercalls in Arm CCA Realm guests. The array is
allocated dynamically during hyperv_init() so that non-Realm guests pay
no memory overhead.

> diff --git a/arch/arm64/hyperv/mshyperv.c b/arch/arm64/hyperv/mshyperv.c
> index 4fdc26ade1d74..d9010e64ddd7c 100644
> --- a/arch/arm64/hyperv/mshyperv.c
> +++ b/arch/arm64/hyperv/mshyperv.c
[ ... ]
> @@ -85,6 +90,20 @@ static int __init hyperv_init(void)
>  	if (!hyperv_detect_via_acpi() && !hyperv_detect_via_smccc())
>  		return 0;
>  
> +	/*
> +	 * The RSI host-call buffers are only ever used when
> +	 * is_realm_world() is true. Skip the allocation on non-Realm
> +	 * guests. A single contiguous array of nr_cpu_ids entries is
> +	 * allocated; each CPU indexes into it by its processor ID.
> +	 */
> +	if (is_realm_world()) {
> +		hv_hostcall_array = kcalloc(nr_cpu_ids,
> +					    sizeof(struct rsi_host_call),
> +					    GFP_KERNEL);

[Severity: Critical]
Does using kcalloc() violate the __aligned(256) constraint on struct
rsi_host_call?

If slab debugging (e.g., redzoning) or KASAN is enabled, the allocator
offsets the returned pointer, breaking natural power-of-two alignment.

When the unaligned IPA is passed to the RMM via SMC, it gets rejected,
causing the guest to trigger a BUG_ON() inside hv_set_vpreg() during early
boot.

Would it be safer to use alloc_percpu() here to guarantee the required
alignment natively?

> +		if (!hv_hostcall_array)
> +			return -ENOMEM;

[Severity: Low]
This isn't a bug, but since hyperv_init() runs early in the boot process, is
it necessary to handle this memory allocation failure gracefully?

At this stage, physical memory should be available, and an allocation failure
typically means the system cannot boot anyway. It seems the explicit error
handling and cleanup logic add unnecessary complexity.

> +	}
> +
>  	/* Setup the guest ID */
>  	guest_id = hv_generate_guest_id(LINUX_VERSION_CODE);
>  	hv_set_vpreg(HV_REGISTER_GUEST_OS_ID, guest_id);
[ ... ]
> @@ -106,13 +125,13 @@ static int __init hyperv_init(void)
>  
>  	ret = hv_common_init();
>  	if (ret)
> -		return ret;
> +		goto free_hostcall_mem;
>  
>  	ret = cpuhp_setup_state(CPUHP_AP_HYPERV_ONLINE, "arm64/hyperv_init:online",
>  				hv_common_cpu_init, hv_common_cpu_die);
>  	if (ret < 0) {
>  		hv_common_free();
> -		return ret;
> +		goto free_hostcall_mem;
>  	}
>  
>  	if (ms_hyperv.priv_high & HV_ACCESS_PARTITION_ID)
> @@ -125,6 +144,11 @@ static int __init hyperv_init(void)
>  
>  	hyperv_initialized = true;
>  	return 0;
> +
> +free_hostcall_mem:
> +	kfree(hv_hostcall_array);
> +	hv_hostcall_array = NULL;
> +	return ret;

[Severity: Low]
This isn't a bug, but following up on the previous question, could we avoid
adding this entire cleanup block since this is early boot code?

>  }
>  
>  early_initcall(hyperv_init);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721201148.1441143-1-kameroncarr@linux.microsoft.com?part=3

^ permalink raw reply

* Re: [PATCH v3 5/6] arm64: hyperv: Route hypercalls through RSI host call in CCA Realms
From: sashiko-bot @ 2026-07-21 20:27 UTC (permalink / raw)
  To: Kameron Carr; +Cc: linux-hyperv
In-Reply-To: <20260721201148.1441143-6-kameroncarr@linux.microsoft.com>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The globally allocated `hv_hostcall_array` is used for RSI host calls, but it is not guaranteed to be 256-byte aligned, which is strictly required by the RMM.
- [High] Using a single shared per-CPU buffer for hypercalls without masking NMIs/FIQs destroys re-entrancy, risking data corruption.
- [High] Hypercalls passing memory buffers will fail because the guest passes Protected IPAs to the host instead of Shared IPAs.
--

--- Patch [5]: [PATCH v3 5/6] arm64: hyperv: Route hypercalls through RSI host call in CCA Realms ---
commit a89ea311002c08edfbe0b6f9d1e3ca3c6d94b198
Author: Kameron Carr <kameroncarr@linux.microsoft.com>

arm64: hyperv: Route hypercalls through RSI host call in CCA Realms

This patch modifies the hypercall wrapper functions to check for
is_realm_world(). When running inside a Realm, it routes the hypercalls
through the RSI interface using a per-CPU rsi_host_call structure.

> diff --git a/arch/arm64/hyperv/hv_core.c b/arch/arm64/hyperv/hv_core.c
> index e33a9e3c366a1..77cba08fca132 100644
> --- a/arch/arm64/hyperv/hv_core.c
> +++ b/arch/arm64/hyperv/hv_core.c

[ ... ]

> +static u64 hv_do_rsi_hypercall(u64 control, u64 input1, u64 input2)
> +{
> +	struct rsi_host_call *hostcall;
> +	unsigned long flags;
> +	u64 ret;
> +
> +	if (!hv_hostcall_array)
> +		return HV_STATUS_INVALID_HYPERCALL_INPUT;
> +
> +	local_irq_save(flags);
> +	hostcall = &hv_hostcall_array[smp_processor_id()];

[Severity: High]
Since local_irq_save() doesn't mask FIQs or pseudo-NMIs on ARM64, is this 
shared per-CPU buffer safe from re-entrancy?

If a pseudo-NMI like a perf interrupt or hardware watchdog interrupts an 
ongoing hypercall and issues a hypercall itself (e.g., hyperv_report_panic 
calling hv_set_vpreg), wouldn't it overwrite this same hostcall buffer and 
corrupt the state?

> +	memset(hostcall, 0, sizeof(*hostcall));
> +	hostcall->gprs[0] = HV_FUNC_ID;
> +	hostcall->gprs[1] = control;
> +	hostcall->gprs[2] = input1;
> +	hostcall->gprs[3] = input2;
> +
> +	if (rsi_host_call(virt_to_phys(hostcall)) == RSI_SUCCESS)

[Severity: High]
Does the allocation of hv_hostcall_array guarantee the 256-byte alignment 
required by the RMM ABI?

If it is allocated using kcalloc() during init, that only guarantees 
alignment up to ARCH_KMALLOC_MINALIGN. When slab debugging like SLUB_DEBUG 
redzoning is enabled, the objects can be shifted by redzone padding, 
completely breaking the 256-byte alignment and causing the RMM to reject the 
call.

> +		ret = hostcall->gprs[0];
> +	else
> +		ret = HV_STATUS_INVALID_HYPERCALL_INPUT;
> +
> +	local_irq_restore(flags);
> +	return ret;
> +}

[ ... ]

> @@ -29,8 +61,11 @@ u64 hv_do_hypercall(u64 control, void *input, void *output)
>  	input_address = input ? virt_to_phys(input) : 0;
>  	output_address = output ? virt_to_phys(output) : 0;
>  
> -	arm_smccc_1_1_hvc(HV_FUNC_ID, control,
> -			  input_address, output_address, &res);
> +	if (is_realm_world())
> +		return hv_do_rsi_hypercall(control, input_address, output_address);

[Severity: High]
Will the Hyper-V host be able to access these input and output buffers?

In a CCA Realm, the physical address space is divided by the prot_ns_shared 
bit, and virt_to_phys() returns a Protected IPA without this bit set.

If the host requires access to these buffers, does this need to pass a 
Shared IPA instead of the base Protected IPA?

> +
> +	arm_smccc_1_1_hvc(HV_FUNC_ID, control, input_address,
> +			  output_address, &res);
>  	return res.a0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721201148.1441143-1-kameroncarr@linux.microsoft.com?part=5

^ permalink raw reply

* Re: [RFC PATCH] x86/apic: Fix lost IRQ during forced vector migration on Hyper-V
From: Thomas Gleixner @ 2026-07-21 21:10 UTC (permalink / raw)
  To: Naman Jain, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H . Peter Anvin
  Cc: Neeraj Upadhyay, linux-kernel, linux-hyperv, Michael Kelley
In-Reply-To: <20260710054007.288807-1-namjain@linux.microsoft.com>

On Fri, Jul 10 2026 at 05:40, Naman Jain wrote:
>
> Fixes: e84cf6aa501c5 ("x86/apic/vector: Handle vector release on CPU unplug correctly")

Errm. This does not fix anything in the common code. It fixes a hyperv
PCI/MSI problem.

>  	} else {
> +		/*
> +		 * The outgoing CPU cannot use the deferred cleanup-vector
> +		 * mechanism, so its vector is freed inline below. On Hyper-V the
> +		 * MSI retarget hypercall is asynchronous, so an interrupt raised
> +		 * inside the stop_machine window can be posted to the outgoing
> +		 * CPU's old vIRR after the free. Two complementary steps handle
> +		 * that (see also the retrigger at the end of the function):
> +		 *
> +		 *  - Retrigger on the new target so a raced completion is drained
> +		 *    there rather than lost. The retarget is asynchronous, so the
> +		 *    outgoing IRR is not authoritative and the IPI is issued
> +		 *    unconditionally; a spurious ISR is harmless to
> +		 *    completion-draining handlers (they find an empty queue).
> +		 *  - Mark the freed slot VECTOR_RETRIGGERED so a late stray is
> +		 *    absorbed by reevaluate_vector() instead of logging "No irq
> +		 *    handler" while the CPU still takes interrupts during
> +		 *    teardown; __setup_vector_irq() resets it on re-online.
> +		 *
> +		 * This mirrors msi_set_affinity()'s protection, which the Hyper-V
> +		 * MSI chip bypasses via IRQCHIP_MOVE_DEFERRED.

What has msi_set_affinity() and IRQCHIP_MOVE_DEFERRED to do with this?

  - msi_set_affinity() solves a completely different problem. It handles
    the case when the PCI/MSI device does not provide masking of the MSI
    interrupt across the message change.

    Q: What exactly is mirrored here?
    A: Nothing

  - In the context of CPU hot-unplug IRQCHIP_MOVE_DEFERRED is not
    deferring anything simply because there is no way to do the deferred
    (in interrupt context) move on the original target CPU.

    fixup_irqs()
       irq_migrate_all_off_this_cpu()
          migrate_one_irq()
            irq_do_set_affinity()

    The only check there which is related to IRQCHIP_MOVE_DEFERRED is
    the masking decision in migrate_one_irq(), but that has nothing to
    do with the semantics of IRQCHIP_MOVE_DEFERRED.

    That flag tells the interrupt core that regular affinity changes
    have to be delayed into the context of the next device interrupt,
    which is obviously not possible for interrupt startup and forced
    migration on CPU hot-unplug, which both ignore that restriction. The
    only effect in CPU hot-unplug migrations is that the interrupt is
    masked at the outermost domain level across the affinity change
    operation because that's an requirement for msi_set_affinity() and
    similar workarounds (loongarch) to avoid the multi vector dance.

    The fact that hyper-V invokes the retarget hypercall in context of
    the unmask() callback has nothing to with that flag and due to the
    lack of explanation why the flag is needed in the first place.

I'm all for commenting complex issues, but comments have to be
technically correct, which excludes both LLM and human hallucinations.
The same applies to the change log which carries the same fairy tale.

Aside of that, doing the re-targeting in the unmask() callback
unconditionally is pointless overhead. Why?

    disable_irq($IRQ);             // Sets the disabled flag, but does not mask

    --> $IRQ is raised
    handle_edge_irq($IRQ)
    if (disabled($IRQ))
      mask_irq($IRQ);              // Masks at the PCI/MSI level

    enable_irq($IRQ)
    if (masked($IRQ))
      unmask_irq($IRQ)
        issue_hypercall(RETARGET)  // Pointless exercise when affinity
                                   // has not changed
        unmask_pci_msi();          // Unmasks at the PCI/MSI level

Not that I care at all, but I have no idea why pci-hyperv has to do the
affinity change in the unmask() callback and why it needs to take the
overhead which comes with the deferred affinity changes in general.

> +		 * restricted to edge MSI vectors on Hyper-V (msi_desc present):
> +		 * only the MSI retarget hypercall is asynchronous, so edge IOAPIC
> +		 * lines (retargeted synchronously via the RTE) and level-triggered
> +		 * lines never see this race and must not be force-injected.
> +		 */
> +		if (hypervisor_is_type(X86_HYPER_MS_HYPERV) &&
> +		    apicd->vector >= FIRST_EXTERNAL_VECTOR &&
> +		    !irqd_is_level_type(irqd) && irq_data_get_msi_desc(irqd))

TBH. This is a tasteless layering violation. We are not cluttering
common code with hypervisor specific MSI workarounds. The non-remapped
MSI workaround is not implemented here either.

> +			hv_retrigger = true;
>  		apic_free_vector(apicd->cpu, apicd->vector, managed);
> +		/*
> +		 * apic_free_vector() releases the matrix bit but leaves the
> +		 * outgoing CPU's vector_irq[] slot pointing at the stale desc, so
> +		 * the marker is written unconditionally here.

What's exactly unconditional about

       if (cond)
          write()

?

> +              						(Unlike
> +		 * msi_set_affinity(), which marks a genuinely unused slot and
> +		 * therefore guards with IS_ERR_OR_NULL, the slot here still holds
> +		 * this irq's old desc.)

What has msi_set_affinity() to do with this? It sets RETRIGGER for a
very different reason. These references are confusing at best. Simply
explain WHY you need to do this and explain it correctly.

> +		 */
> +		if (hv_retrigger)
> +			per_cpu(vector_irq, apicd->cpu)[apicd->vector] = VECTOR_RETRIGGERED;
>  	}
>  
>  setnew:
> @@ -190,6 +231,13 @@ static void chip_data_update(struct irq_data *irqd, unsigned int newvec, unsigne
>  	BUG_ON(!IS_ERR_OR_NULL(per_cpu(vector_irq, newcpu)[newvec]));
>  	per_cpu(vector_irq, newcpu)[newvec] = desc;
>  	apic_update_irq_cfg(irqd, newvec, newcpu);
> +	/*
> +	 * Drain any completion that raced onto the freed vIRR by retriggering

What's a freed vIRR and which completion? This code deals with vector
management and not with hyperV MSI implementation specific completions
and it does not care whether it operates on a physical or a virtual
APIC.

> +	 * on the new target (see the else-branch above). Issued after the new

'See the else-branch above' is as useful as 'See some random explanation
elsewhere'. It's obvious where to look for that information as the
hv_retrigger condition must be set by something to make the condition true.

> +	 * mapping is installed so the handler is present when it is serviced.
> +	 */
> +	if (hv_retrigger)
> +		__apic_send_IPI(newcpu, newvec);

You are claiming above that the MSI retarget hypercall is asynchronous,
but you fail to specify in which way.

  - If you refer to the retarget invocation in the unmask() callback,
    then that happens still before the new target can handle it.

  - If it is truly asynchronous, i.e. the hypervisor handles it at some
    arbitrary point in time, then the retriggered vector on the new
    target CPU can be handled _before_ the retarget is in effect. In
    that case a subsequent interrupt might still end up on the dead CPU,
    no?

IOW, your or your LLM's elaborate comment is useless word salad as it
does not explain the problem in a coherent and comprehensible way. Quite
to the contrary it is more confusing than helpful.

But as I told you already, this is not a problem of incoherent comments,
this is simply the wrong place to address that.

Looking at the pci-hyperv implementation made me immediately notice a
bug, which exists since the driver was merged:

  The driver does not set the irq_retrigger() callback for the
  interrupt chip.

That means that fixup_irqs() can't do anything even when it observes the
IRR bit set. And that matches the incoherent problem description as far
as I can tell from trying to oracle something out of the word salad.

Great that your AI ass-istant did not figure that out and you blindly
trusted the nonsense it hallucinated.

The uncompiled and therefore untested below should exactly do what you
want to achieve without having hyperV specific hackery in the common
code.

If it does not, then you need to provide a coherent technical
explanation why it is not sufficient.

Thanks,

        tglx

---
Subject: TBD

  Instead of adding the missing callback to the hyperv driver, which
  would be the trivial "fix", it changes the x86 fixup_irqs()
  implementation. Why?

  That makes the code more resilient and allows to remove quite a bit of
  initializations of interrupt chips all over the place.

Not-Signed-off-yet-by: Thomas Gleixner <tglx@kernel.org>
Assisted-by: Human Intelligence
---
 arch/x86/kernel/irq.c |   23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

--- a/arch/x86/kernel/irq.c
+++ b/arch/x86/kernel/irq.c
@@ -466,11 +466,6 @@ DEFINE_IDTENTRY_SYSVEC(sysvec_posted_msi
 /* A cpu has been removed from cpu_online_mask.  Reset irq affinities. */
 void fixup_irqs(void)
 {
-	unsigned int vector;
-	struct irq_desc *desc;
-	struct irq_data *data;
-	struct irq_chip *chip;
-
 	irq_migrate_all_off_this_cpu();
 
 	/*
@@ -489,22 +484,18 @@ void fixup_irqs(void)
 	 * vector_lock because the cpu is already marked !online, so
 	 * nothing else will touch it.
 	 */
-	for (vector = FIRST_EXTERNAL_VECTOR; vector < NR_VECTORS; vector++) {
-		if (IS_ERR_OR_NULL(__this_cpu_read(vector_irq[vector])))
+	for (unsigned int vector = FIRST_EXTERNAL_VECTOR; vector < NR_VECTORS; vector++) {
+		struct irq_desc *desc = __this_cpu_read(vector_irq[vector]);
+
+		if (IS_ERR_OR_NULL(desc))
 			continue;
 
 		if (is_vector_pending(vector)) {
-			desc = __this_cpu_read(vector_irq[vector]);
-
-			raw_spin_lock(&desc->lock);
-			data = irq_desc_get_irq_data(desc);
-			chip = irq_data_get_irq_chip(data);
-			if (chip->irq_retrigger) {
-				chip->irq_retrigger(data);
+			guard(raw_spinlock)(&desc->lock);
+			if (irq_chip_retrigger_hierarchy(&desc->irq_data))
 				__this_cpu_write(vector_irq[vector], VECTOR_RETRIGGERED);
-			}
-			raw_spin_unlock(&desc->lock);
 		}
+
 		if (__this_cpu_read(vector_irq[vector]) != VECTOR_RETRIGGERED)
 			__this_cpu_write(vector_irq[vector], VECTOR_UNUSED);
 	}

^ permalink raw reply

* [PATCH net-next v2 0/7] net: mana: harden the HWC and add dynamic queue depth
From: Long Li @ 2026-07-21 23:43 UTC (permalink / raw)
  To: Long Li, Konstantin Taranov, Jakub Kicinski, David S . Miller,
	Paolo Abeni, Eric Dumazet, Andrew Lunn, Jason Gunthorpe,
	Leon Romanovsky, Haiyang Zhang, K . Y . Srinivasan, Wei Liu,
	Dexuan Cui, shradhagupta, Simon Horman, ernis, stephen
  Cc: netdev, linux-rdma, linux-hyperv, linux-kernel

This series hardens the MANA Hardware Channel (HWC) control-plane path
and then builds on that to support a dynamic HWC queue depth.

The HWC is the command channel the driver uses to talk to the device.
Today it is created at a fixed depth of one outstanding request, and
several of its lookup and teardown paths predate the RCU and DMA-lifetime
rules they now need to follow.  Raising the queue depth and allowing
concurrent commands makes those latent races reachable, so the fixes come
first and the feature builds on them.

Patches 1-5 are fixes for pre-existing HWC bugs, each with a Fixes: tag:

  1: cq_table was a plain pointer array freed with no grace period while
     the EQ interrupt handler dereferenced it; put it under RCU.
  2: the HWC RQ and SQ were sized with each other's message size, so a
     response could overflow the RQ buffer and the RX slot stride was
     computed with the wrong size.
  3: comp_buf was freed before the EQ was destroyed, so a late completion
     handler could touch freed memory.
  4: the RX path consumed device-supplied lengths and indices without
     validation; validate them before use (this matters for confidential
     VMs, where the DMA buffer is shared with the host).
  5: a failed mana_hwc_establish_channel() could leave live MST entries
     while the driver freed the queue buffers, and destroy_channel() freed
     the TXQ/RXQ before the EQ was quiesced; add a setup_active teardown
     gate and destroy the CQ first.

Patches 6-7 add the feature:

  6: replace the depth-1 semaphore with a slot bitmap and per-slot
     synchronization so several management commands can be in flight,
     with teardown that drains in-flight senders before freeing the HWC.
  7: bootstrap the HWC at depth 1, query the device maximum and, if it is
     larger, tear down and rebuild the queues at that depth.  The reported
     dimensions are validated before they size DMA allocations, and the
     capability is advertised so firmware enables it only when the driver
     supports it.

Changes since v1:
 - patch 4: bounds-check the SGE pointer derived from the device-supplied
   inline_oob_size_div4 before dereferencing it, so a corrupted OOB size
   cannot push the read past the RQ buffer (found in review).
 - patch 6: fix a use-after-free of gdma_context during teardown -- track
   in-flight senders with a gc->hwc_lock-protected counter and drain it
   with wait_event_lock_irq(), so the last sender's wake_up() cannot touch
   gc after it is freed (found in review).
 - patch 6: move the in-flight sender force-complete and drain ahead of
   the teardown/FLR block, so the FLR-failure early return can no longer
   skip the drain and strand a sender (found in review).
 - Rebased on current net-next; kept all patches within 80 columns.
 - v1: https://lore.kernel.org/netdev/20260715032942.3945317-1-longli@microsoft.com/

Long Li (7):
  net: mana: RCU-protect gc->cq_table lookups against concurrent CQ
    destroy
  net: mana: fix HWC RQ/SQ buffer size swap
  net: mana: free HWC comp_buf after destroying the EQ
  net: mana: validate hardware-supplied values in the HWC RX path
  net: mana: fix HWC teardown safety with setup_active flag and destroy
    ordering
  net: mana: support concurrent HWC requests with proper synchronization
  net: mana: add dynamic HWC queue depth with reinit path

 drivers/infiniband/hw/mana/cq.c               |  46 +-
 .../net/ethernet/microsoft/mana/gdma_main.c   |  80 +-
 .../net/ethernet/microsoft/mana/hw_channel.c  | 742 ++++++++++++++++--
 drivers/net/ethernet/microsoft/mana/mana_en.c |  22 +-
 include/net/mana/gdma.h                       |  48 +-
 include/net/mana/hw_channel.h                 |  44 +-
 6 files changed, 890 insertions(+), 92 deletions(-)


base-commit: e354f7d60f14a3eacd5ec7b607346a5612f655ec
-- 
2.43.0


^ permalink raw reply

* [PATCH net-next v2 1/7] net: mana: RCU-protect gc->cq_table lookups against concurrent CQ destroy
From: Long Li @ 2026-07-21 23:43 UTC (permalink / raw)
  To: Long Li, Konstantin Taranov, Jakub Kicinski, David S . Miller,
	Paolo Abeni, Eric Dumazet, Andrew Lunn, Jason Gunthorpe,
	Leon Romanovsky, Haiyang Zhang, K . Y . Srinivasan, Wei Liu,
	Dexuan Cui, shradhagupta, Simon Horman, ernis, stephen
  Cc: netdev, linux-rdma, linux-hyperv, linux-kernel
In-Reply-To: <20260721234339.1476932-1-longli@microsoft.com>

The EQ interrupt handler (mana_gd_process_eqe) looks up the completing CQ
in gc->cq_table[cq_id] and runs its callback, concurrently with CQ
teardown on another CPU that clears the slot and frees the CQ.  cq_table
was a plain pointer array freed with no grace period, so the two race
into a use-after-free:

  CPU A (mana_gd_intr, hard IRQ)        CPU B (CQ destroy)
  ----------------------------------    ------------------------------
  cq = gc->cq_table[cq_id];  // valid
                                        gc->cq_table[id] = NULL;
                                        kfree(cq);          // freed
  cq->cq.callback(ctx, cq);  // use-after-free

The handler's existing rcu_read_lock() only guards the per-IRQ EQ list
traversal; cq_table was never under any RCU contract, and a read-side
lock is inert unless the freer also defers the free past a grace period.

Put cq_table under RCU: annotate the base pointer and entries __rcu, read
with rcu_dereference() in the handler, publish with rcu_assign_pointer(),
and on teardown clear the slot then synchronize_rcu() before freeing the
CQ.  The grace period blocks until every in-flight handler has dropped
the old pointer, so the kfree() can no longer race the callback.

Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
Signed-off-by: Long Li <longli@microsoft.com>
---
 drivers/infiniband/hw/mana/cq.c               | 46 ++++++++++++++++---
 .../net/ethernet/microsoft/mana/gdma_main.c   | 25 ++++++++--
 .../net/ethernet/microsoft/mana/hw_channel.c  | 29 ++++++++----
 drivers/net/ethernet/microsoft/mana/mana_en.c | 22 +++++++--
 include/net/mana/gdma.h                       | 21 ++++++++-
 5 files changed, 119 insertions(+), 24 deletions(-)

diff --git a/drivers/infiniband/hw/mana/cq.c b/drivers/infiniband/hw/mana/cq.c
index f2547989f422..2bf4be21cede 100644
--- a/drivers/infiniband/hw/mana/cq.c
+++ b/drivers/infiniband/hw/mana/cq.c
@@ -131,12 +131,20 @@ static void mana_ib_cq_handler(void *ctx, struct gdma_queue *gdma_cq)
 int mana_ib_install_cq_cb(struct mana_ib_dev *mdev, struct mana_ib_cq *cq)
 {
 	struct gdma_context *gc = mdev_to_gc(mdev);
+	struct gdma_queue __rcu **cq_table;
 	struct gdma_queue *gdma_cq;
 
-	if (cq->queue.id >= gc->max_num_cqs)
+	/* No rcu_read_lock(): install/remove run within the IB device
+	 * lifetime, which mana_rdma_remove() (ib_unregister_device) drains
+	 * before the base cq_table can be freed.  See gdma_context::cq_table
+	 * in gdma.h for why "true" is sound.
+	 */
+	cq_table = rcu_dereference_protected(gc->cq_table, true);
+	if (!cq_table || cq->queue.id >= gc->max_num_cqs)
 		return -EINVAL;
+
 	/* Create CQ table entry, sharing a CQ between WQs is not supported */
-	if (gc->cq_table[cq->queue.id])
+	if (rcu_access_pointer(cq_table[cq->queue.id]))
 		return -EINVAL;
 	if (cq->queue.kmem)
 		gdma_cq = cq->queue.kmem;
@@ -149,23 +157,49 @@ int mana_ib_install_cq_cb(struct mana_ib_dev *mdev, struct mana_ib_cq *cq)
 	gdma_cq->type = GDMA_CQ;
 	gdma_cq->cq.callback = mana_ib_cq_handler;
 	gdma_cq->id = cq->queue.id;
-	gc->cq_table[cq->queue.id] = gdma_cq;
+	rcu_assign_pointer(cq_table[cq->queue.id], gdma_cq);
 	return 0;
 }
 
 void mana_ib_remove_cq_cb(struct mana_ib_dev *mdev, struct mana_ib_cq *cq)
 {
 	struct gdma_context *gc = mdev_to_gc(mdev);
+	struct gdma_queue __rcu **cq_table;
+	struct gdma_queue *gdma_cq;
 
-	if (cq->queue.id >= gc->max_num_cqs || cq->queue.id == INVALID_QUEUE_ID)
+	if (cq->queue.id == INVALID_QUEUE_ID)
 		return;
 
 	if (cq->queue.kmem)
 	/* Then it will be cleaned and removed by the mana */
 		return;
 
-	kfree(gc->cq_table[cq->queue.id]);
-	gc->cq_table[cq->queue.id] = NULL;
+	/* No rcu_read_lock(): like mana_ib_install_cq_cb(), this runs within
+	 * the IB device lifetime that mana_rdma_remove() drains before the
+	 * base cq_table can be freed.  See gdma_context::cq_table in gdma.h.
+	 */
+	cq_table = rcu_dereference_protected(gc->cq_table, true);
+	if (!cq_table || cq->queue.id >= gc->max_num_cqs)
+		return;
+	/* Removers for a given CQ are serialized by the IB core, so the slot
+	 * is read and cleared without rcu_read_lock() or atomicity: a CQ is
+	 * never torn down while a live QP references it (cq->usecnt), nor
+	 * while the QP-create that installed the entry is still running (that
+	 * create holds a reference on the CQ uobject across its error path,
+	 * before usecnt is taken).  Any double-remove is therefore sequential
+	 * -- the later caller sees the NULL stored below and returns.
+	 */
+	gdma_cq = rcu_dereference_protected(cq_table[cq->queue.id], true);
+	if (!gdma_cq)
+		return;  /* already removed by a prior teardown path */
+
+	rcu_assign_pointer(cq_table[cq->queue.id], NULL);
+
+	/* Wait for in-flight EQ handlers that may have loaded the old
+	 * pointer via rcu_dereference() to finish before freeing.
+	 */
+	synchronize_rcu();
+	kfree(gdma_cq);
 }
 
 int mana_ib_arm_cq(struct ib_cq *ibcq, enum ib_cq_notify_flags flags)
diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
index a38d4bb74621..9edebf7c097c 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
@@ -761,6 +761,7 @@ static void mana_gd_process_eqe(struct gdma_queue *eq)
 	union gdma_eqe_info eqe_info;
 	enum gdma_eqe_type type;
 	struct gdma_event event;
+	struct gdma_queue __rcu **cq_table;
 	struct gdma_queue *cq;
 	struct gdma_eqe *eqe;
 	u32 cq_id;
@@ -772,10 +773,11 @@ static void mana_gd_process_eqe(struct gdma_queue *eq)
 	switch (type) {
 	case GDMA_EQE_COMPLETION:
 		cq_id = eqe->details[0] & 0xFFFFFF;
-		if (WARN_ON_ONCE(cq_id >= gc->max_num_cqs))
+		cq_table = rcu_dereference(gc->cq_table);
+		if (WARN_ON_ONCE(cq_id >= gc->max_num_cqs || !cq_table))
 			break;
 
-		cq = gc->cq_table[cq_id];
+		cq = rcu_dereference(cq_table[cq_id]);
 		if (WARN_ON_ONCE(!cq || cq->type != GDMA_CQ || cq->id != cq_id))
 			break;
 
@@ -1082,15 +1084,28 @@ static void mana_gd_create_cq(const struct gdma_queue_spec *spec,
 static void mana_gd_destroy_cq(struct gdma_context *gc,
 			       struct gdma_queue *queue)
 {
+	struct gdma_queue __rcu **cq_table;
 	u32 id = queue->id;
 
-	if (id >= gc->max_num_cqs)
+	/* No rcu_read_lock() here: mana_gd_destroy_cq() runs only on the
+	 * CQ-destroy/teardown path, where the base cq_table is stable.  See
+	 * the lifecycle note on gdma_context::cq_table in gdma.h for why the
+	 * "true" predicate is sound.
+	 */
+	cq_table = rcu_dereference_protected(gc->cq_table, true);
+	if (!cq_table || id >= gc->max_num_cqs)
 		return;
 
-	if (!gc->cq_table[id])
+	if (!rcu_access_pointer(cq_table[id]))
 		return;
 
-	gc->cq_table[id] = NULL;
+	rcu_assign_pointer(cq_table[id], NULL);
+
+	/* Wait for in-flight EQ handlers that may have loaded the old
+	 * pointer via rcu_dereference() to finish before the caller
+	 * frees the CQ memory.
+	 */
+	synchronize_rcu();
 }
 
 int mana_gd_create_hwc_queue(struct gdma_dev *gd,
diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index e3c24d50dad0..409e20caeccd 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -674,6 +674,7 @@ static int mana_hwc_establish_channel(struct gdma_context *gc, u16 *q_depth,
 	struct gdma_queue *sq = hwc->txq->gdma_wq;
 	struct gdma_queue *eq = hwc->cq->gdma_eq;
 	struct gdma_queue *cq = hwc->cq->gdma_cq;
+	struct gdma_queue __rcu **cq_table;
 	int err;
 
 	init_completion(&hwc->hwc_init_eqe_comp);
@@ -698,11 +699,15 @@ static int mana_hwc_establish_channel(struct gdma_context *gc, u16 *q_depth,
 	if (WARN_ON(cq->id >= gc->max_num_cqs))
 		return -EPROTO;
 
-	gc->cq_table = vcalloc(gc->max_num_cqs, sizeof(struct gdma_queue *));
-	if (!gc->cq_table)
+	cq_table = vcalloc(gc->max_num_cqs, sizeof(*cq_table));
+	if (!cq_table)
 		return -ENOMEM;
 
-	gc->cq_table[cq->id] = cq;
+	rcu_assign_pointer(cq_table[cq->id], cq);
+	/* Publish the fully-initialised table last; pairs with the
+	 * rcu_dereference(gc->cq_table) in mana_gd_process_eqe().
+	 */
+	rcu_assign_pointer(gc->cq_table, cq_table);
 
 	return 0;
 }
@@ -811,6 +816,7 @@ int mana_hwc_create_channel(struct gdma_context *gc)
 void mana_hwc_destroy_channel(struct gdma_context *gc)
 {
 	struct hw_channel_context *hwc = gc->hwc.driver_data;
+	struct gdma_queue __rcu **old_cq_table;
 
 	if (!hwc)
 		return;
@@ -818,10 +824,8 @@ void mana_hwc_destroy_channel(struct gdma_context *gc)
 	/* gc->max_num_cqs is set in mana_hwc_init_event_handler(). If it's
 	 * non-zero, the HWC worked and we should tear down the HWC here.
 	 */
-	if (gc->max_num_cqs > 0) {
+	if (gc->max_num_cqs > 0)
 		mana_smc_teardown_hwc(&gc->shm_channel, false);
-		gc->max_num_cqs = 0;
-	}
 
 	if (hwc->txq)
 		mana_hwc_destroy_wq(hwc, hwc->txq);
@@ -832,6 +836,14 @@ void mana_hwc_destroy_channel(struct gdma_context *gc)
 	if (hwc->cq)
 		mana_hwc_destroy_cq(hwc->gdma_dev->gdma_context, hwc->cq);
 
+	/* Reset only after mana_hwc_destroy_cq() above has run with a valid
+	 * max_num_cqs so mana_gd_destroy_cq() clears the CQ table slot and
+	 * waits out in-flight EQ handlers (synchronize_rcu) before the CQ is
+	 * freed.  Clearing it earlier would make that path early-return and
+	 * skip the slot clear, leaving a dangling cq_table entry.
+	 */
+	gc->max_num_cqs = 0;
+
 	kfree(hwc->caller_ctx);
 	hwc->caller_ctx = NULL;
 
@@ -848,8 +860,9 @@ void mana_hwc_destroy_channel(struct gdma_context *gc)
 	gc->hwc.driver_data = NULL;
 	gc->hwc.gdma_context = NULL;
 
-	vfree(gc->cq_table);
-	gc->cq_table = NULL;
+	old_cq_table = rcu_replace_pointer(gc->cq_table, NULL, true);
+	synchronize_rcu();
+	vfree(old_cq_table);
 }
 
 int mana_hwc_send_request(struct hw_channel_context *hwc, u32 req_len,
diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c b/drivers/net/ethernet/microsoft/mana/mana_en.c
index a8c329bdbacf..5508746590e7 100644
--- a/drivers/net/ethernet/microsoft/mana/mana_en.c
+++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
@@ -2648,6 +2648,7 @@ static int mana_create_txq(struct mana_port_context *apc,
 	struct mana_obj_spec cq_spec;
 	struct gdma_queue_spec spec;
 	struct gdma_context *gc;
+	struct gdma_queue __rcu **cq_table;
 	struct mana_txq *txq;
 	struct mana_cq *cq;
 	u32 txq_size;
@@ -2753,12 +2754,18 @@ static int mana_create_txq(struct mana_port_context *apc,
 
 		cq->gdma_id = cq->gdma_cq->id;
 
-		if (WARN_ON(cq->gdma_id >= gc->max_num_cqs)) {
+		/* No rcu_read_lock(): mana_create_txq runs under RTNL during
+		 * netdev bring-up, inside the netdev lifetime that
+		 * mana_remove() drains before the base cq_table can be freed.
+		 * See gdma_context::cq_table in gdma.h for why "true" is sound.
+		 */
+		cq_table = rcu_dereference_protected(gc->cq_table, true);
+		if (WARN_ON(!cq_table || cq->gdma_id >= gc->max_num_cqs)) {
 			err = -EINVAL;
 			goto out;
 		}
 
-		gc->cq_table[cq->gdma_id] = cq->gdma_cq;
+		rcu_assign_pointer(cq_table[cq->gdma_id], cq->gdma_cq);
 
 		mana_create_txq_debugfs(apc, i);
 
@@ -2986,6 +2993,7 @@ static struct mana_rxq *mana_create_rxq(struct mana_port_context *apc,
 	struct gdma_queue_spec spec;
 	struct mana_cq *cq = NULL;
 	struct gdma_context *gc;
+	struct gdma_queue __rcu **cq_table;
 	u32 cq_size, rq_size;
 	struct mana_rxq *rxq;
 	int err;
@@ -3075,12 +3083,18 @@ static struct mana_rxq *mana_create_rxq(struct mana_port_context *apc,
 	if (err)
 		goto out;
 
-	if (WARN_ON(cq->gdma_id >= gc->max_num_cqs)) {
+	/* No rcu_read_lock(): mana_create_rxq runs under RTNL during netdev
+	 * bring-up, inside the netdev lifetime that mana_remove() drains
+	 * before the base cq_table can be freed.  See gdma_context::cq_table
+	 * in gdma.h for why "true" is sound.
+	 */
+	cq_table = rcu_dereference_protected(gc->cq_table, true);
+	if (WARN_ON(!cq_table || cq->gdma_id >= gc->max_num_cqs)) {
 		err = -EINVAL;
 		goto out;
 	}
 
-	gc->cq_table[cq->gdma_id] = cq->gdma_cq;
+	rcu_assign_pointer(cq_table[cq->gdma_id], cq->gdma_cq);
 
 	netif_napi_add_weight_locked(ndev, &cq->napi, mana_poll, 1);
 
diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h
index 8529cef0d7c4..da52701e7816 100644
--- a/include/net/mana/gdma.h
+++ b/include/net/mana/gdma.h
@@ -430,7 +430,26 @@ struct gdma_context {
 
 	/* This maps a CQ index to the queue structure. */
 	unsigned int		max_num_cqs;
-	struct gdma_queue	**cq_table;
+	/* Both the base pointer and each entry are RCU-managed.  The fast
+	 * path (mana_gd_process_eqe) reads the base via rcu_dereference()
+	 * under rcu_read_lock(), so the table is freed with
+	 * rcu_assign_pointer(NULL) + synchronize_rcu() and an in-flight
+	 * reader can never observe freed memory.
+	 *
+	 * The slow paths -- mana_gd_destroy_cq() and the CQ install/remove
+	 * callers (mana_create_txq/_rxq, mana_ib_install/remove_cq_cb) --
+	 * instead read the base with rcu_dereference_protected(cq_table,
+	 * true).  The bare "true" is justified by teardown ordering, not by
+	 * a lock: the base table is replaced+freed only by
+	 * mana_hwc_destroy_channel() (and the create-time reinit), and every
+	 * teardown path first runs mana_remove() + mana_rdma_remove(), which
+	 * synchronously drain the netdev and the IB device
+	 * (unregister_netdevice / ib_unregister_device) that bound all
+	 * install/remove callers; the reinit case runs before either
+	 * consumer is probed.  So no slow-path caller can run while the base
+	 * table is being freed.
+	 */
+	struct gdma_queue	__rcu * __rcu *cq_table;
 
 	/* Protect eq_test_event and test_event_eq_id  */
 	struct mutex		eq_test_event_mutex;
-- 
2.43.0


^ permalink raw reply related

* [PATCH net-next v2 2/7] net: mana: fix HWC RQ/SQ buffer size swap
From: Long Li @ 2026-07-21 23:43 UTC (permalink / raw)
  To: Long Li, Konstantin Taranov, Jakub Kicinski, David S . Miller,
	Paolo Abeni, Eric Dumazet, Andrew Lunn, Jason Gunthorpe,
	Leon Romanovsky, Haiyang Zhang, K . Y . Srinivasan, Wei Liu,
	Dexuan Cui, shradhagupta, Simon Horman, ernis, stephen
  Cc: netdev, linux-rdma, linux-hyperv, linux-kernel
In-Reply-To: <20260721234339.1476932-1-longli@microsoft.com>

The HWC RQ receives responses and the SQ sends requests, but
mana_hwc_init_queues() sized the RQ with max_req_msg_size and the SQ with
max_resp_msg_size -- backwards.  A response larger than the undersized RQ
buffer could overflow it, and mana_hwc_rx_event_handler() recovered the
RX slot index by dividing by the wrong size (max_req_msg_size).

Size the RQ by max_resp_msg_size and the SQ by max_req_msg_size, store
max_resp_msg_size in hw_channel_context, and use it as the RX slot
stride.

Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
Signed-off-by: Long Li <longli@microsoft.com>
---
 drivers/net/ethernet/microsoft/mana/hw_channel.c | 7 ++++---
 include/net/mana/hw_channel.h                    | 1 +
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index 409e20caeccd..3f011ebbe7b3 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -263,7 +263,7 @@ static void mana_hwc_rx_event_handler(void *ctx, u32 gdma_rxq_id,
 
 	/* Select the RX work request for virtual address and for reposting. */
 	rq_base_addr = hwc_rxq->msg_buf->mem_info.dma_handle;
-	rx_req_idx = (sge->address - rq_base_addr) / hwc->max_req_msg_size;
+	rx_req_idx = (sge->address - rq_base_addr) / hwc->max_resp_msg_size;
 
 	if (rx_req_idx >= hwc_rxq->msg_buf->num_reqs) {
 		dev_err(hwc->dev, "HWC RX: wrong rx_req_idx=%llu, num_reqs=%u\n",
@@ -733,14 +733,14 @@ static int mana_hwc_init_queues(struct hw_channel_context *hwc, u16 q_depth,
 		goto out;
 	}
 
-	err = mana_hwc_create_wq(hwc, GDMA_RQ, q_depth, max_req_msg_size,
+	err = mana_hwc_create_wq(hwc, GDMA_RQ, q_depth, max_resp_msg_size,
 				 hwc->cq, &hwc->rxq);
 	if (err) {
 		dev_err(hwc->dev, "Failed to create HWC RQ: %d\n", err);
 		goto out;
 	}
 
-	err = mana_hwc_create_wq(hwc, GDMA_SQ, q_depth, max_resp_msg_size,
+	err = mana_hwc_create_wq(hwc, GDMA_SQ, q_depth, max_req_msg_size,
 				 hwc->cq, &hwc->txq);
 	if (err) {
 		dev_err(hwc->dev, "Failed to create HWC SQ: %d\n", err);
@@ -749,6 +749,7 @@ static int mana_hwc_init_queues(struct hw_channel_context *hwc, u16 q_depth,
 
 	hwc->num_inflight_msg = q_depth;
 	hwc->max_req_msg_size = max_req_msg_size;
+	hwc->max_resp_msg_size = max_resp_msg_size;
 
 	return 0;
 out:
diff --git a/include/net/mana/hw_channel.h b/include/net/mana/hw_channel.h
index 16feb39616c1..73671f479399 100644
--- a/include/net/mana/hw_channel.h
+++ b/include/net/mana/hw_channel.h
@@ -181,6 +181,7 @@ struct hw_channel_context {
 
 	u16 num_inflight_msg;
 	u32 max_req_msg_size;
+	u32 max_resp_msg_size;
 
 	u16 hwc_init_q_depth_max;
 	u32 hwc_init_max_req_msg_size;
-- 
2.43.0


^ permalink raw reply related

* [PATCH net-next v2 3/7] net: mana: free HWC comp_buf after destroying the EQ
From: Long Li @ 2026-07-21 23:43 UTC (permalink / raw)
  To: Long Li, Konstantin Taranov, Jakub Kicinski, David S . Miller,
	Paolo Abeni, Eric Dumazet, Andrew Lunn, Jason Gunthorpe,
	Leon Romanovsky, Haiyang Zhang, K . Y . Srinivasan, Wei Liu,
	Dexuan Cui, shradhagupta, Simon Horman, ernis, stephen
  Cc: netdev, linux-rdma, linux-hyperv, linux-kernel
In-Reply-To: <20260721234339.1476932-1-longli@microsoft.com>

mana_hwc_destroy_cq() freed hwc_cq->comp_buf before destroying the CQ and
EQ.  comp_buf is dereferenced by mana_hwc_comp_event(), which the EQ
interrupt handler invokes; freeing it while the EQ was still registered
let a late handler touch freed memory.

Destroy the CQ and EQ first -- the EQ teardown deregisters the IRQ and
fences in-flight handlers -- then free comp_buf and hwc_cq.

Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
Signed-off-by: Long Li <longli@microsoft.com>
---
 drivers/net/ethernet/microsoft/mana/hw_channel.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index 3f011ebbe7b3..2239fdeda57c 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -384,14 +384,20 @@ static void mana_hwc_comp_event(void *ctx, struct gdma_queue *q_self)
 
 static void mana_hwc_destroy_cq(struct gdma_context *gc, struct hwc_cq *hwc_cq)
 {
-	kfree(hwc_cq->comp_buf);
-
 	if (hwc_cq->gdma_cq)
 		mana_gd_destroy_queue(gc, hwc_cq->gdma_cq);
 
+	/* comp_buf is reached only by mana_hwc_comp_event(), which the
+	 * EQ handler invokes via cq_table[id].  The CQ destroy above
+	 * already cleared that slot and ran synchronize_rcu(), so no
+	 * handler can reach comp_buf once it returns.  Destroying the EQ
+	 * here additionally tears down the IRQ (defense in depth) before
+	 * comp_buf and hwc_cq are freed below.
+	 */
 	if (hwc_cq->gdma_eq)
 		mana_gd_destroy_queue(gc, hwc_cq->gdma_eq);
 
+	kfree(hwc_cq->comp_buf);
 	kfree(hwc_cq);
 }
 
-- 
2.43.0


^ permalink raw reply related

* [PATCH net-next v2 4/7] net: mana: validate hardware-supplied values in the HWC RX path
From: Long Li @ 2026-07-21 23:43 UTC (permalink / raw)
  To: Long Li, Konstantin Taranov, Jakub Kicinski, David S . Miller,
	Paolo Abeni, Eric Dumazet, Andrew Lunn, Jason Gunthorpe,
	Leon Romanovsky, Haiyang Zhang, K . Y . Srinivasan, Wei Liu,
	Dexuan Cui, shradhagupta, Simon Horman, ernis, stephen
  Cc: netdev, linux-rdma, linux-hyperv, linux-kernel
In-Reply-To: <20260721234339.1476932-1-longli@microsoft.com>

mana_hwc_rx_event_handler() and mana_hwc_handle_resp() consumed lengths
and indices taken straight from device DMA without validation.  A buggy
firmware or a malicious host (in a confidential VM, where the DMA buffer
is shared) could drive a wrong or reused in-flight request to completion
or index out of bounds.  Validate before use:

  - bounds-check the SGE pointer located via the device-supplied
    inline_oob_size_div4 before dereferencing it, so a corrupted OOB
    size cannot push the read past the RQ buffer;
  - match the SGE address against the address the driver posted for that
    slot, not just an in-range index -- an in-range but wrong SGE would
    otherwise truncate onto a neighbouring slot and read a stale response;
  - require the response to cover a full gdma_resp_hdr before reading
    hwc_msg_id, so a short response cannot complete a slot with stale
    bytes left by the buffer's previous occupant;
  - bounds-check hwc_msg_id in mana_hwc_handle_resp() before indexing the
    inflight bitmap and caller_ctx;
  - reject a resp_len larger than the RX buffer.

Repost the RX WQE on every validation early-return so a rejected response
does not permanently shrink the posted RQ depth.  The two paths that
cannot identify the slot (an out-of-bounds SGE pointer, or an SGE address
matching no posted slot) intentionally leak a single WQE rather than risk
reposting the wrong one.

Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
Signed-off-by: Long Li <longli@microsoft.com>
---
 .../net/ethernet/microsoft/mana/hw_channel.c  | 82 ++++++++++++++++++-
 1 file changed, 78 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index 2239fdeda57c..515c45c4a890 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -83,6 +83,17 @@ static void mana_hwc_handle_resp(struct hw_channel_context *hwc, u32 resp_len,
 	struct hwc_caller_ctx *ctx;
 	int err;
 
+	/* Validate msg_id is in range before using it to index bitmap
+	 * and caller_ctx array.  Malicious firmware could send
+	 * out-of-range msg_id causing out-of-bounds access.
+	 */
+	if (msg_id >= hwc->num_inflight_msg) {
+		dev_err(hwc->dev, "hwc_rx: msg_id %u >= max %u\n",
+			msg_id, hwc->num_inflight_msg);
+		mana_hwc_post_rx_wqe(hwc->rxq, rx_req);
+		return;
+	}
+
 	if (!test_bit(msg_id, hwc->inflight_msg_res.map)) {
 		dev_err(hwc->dev, "hwc_rx: invalid msg_id = %u\n", msg_id);
 		mana_hwc_post_rx_wqe(hwc->rxq, rx_req);
@@ -90,6 +101,18 @@ static void mana_hwc_handle_resp(struct hw_channel_context *hwc, u32 resp_len,
 	}
 
 	ctx = hwc->caller_ctx + msg_id;
+
+	/* Reject responses larger than the RX DMA buffer — the SGE
+	 * limits what hardware can DMA, so an oversized resp_len
+	 * indicates a firmware bug.  Fail rather than silently
+	 * truncating.
+	 */
+	if (resp_len > rx_req->buf_len) {
+		dev_err(hwc->dev, "HWC RX: resp_len %u > buf_len %u\n",
+			resp_len, rx_req->buf_len);
+		resp_len = 0;
+	}
+
 	err = mana_hwc_verify_resp_msg(ctx, resp_msg, resp_len);
 	if (err)
 		goto out;
@@ -261,19 +284,69 @@ static void mana_hwc_rx_event_handler(void *ctx, u32 gdma_rxq_id,
 
 	sge = (struct gdma_sge *)(wqe + 8 + dma_oob->inline_oob_size_div4 * 4);
 
-	/* Select the RX work request for virtual address and for reposting. */
+	/* inline_oob_size_div4 is read from the WQE in device-accessible RQ
+	 * memory, so a malicious host in a CVM (or buggy firmware) could set
+	 * it to push the SGE past the RQ buffer.  Bounds-check the SGE it
+	 * locates before dereferencing sge->address below.  A validly posted
+	 * WQE keeps the SGE inside the ring (worst case ends exactly at the
+	 * buffer boundary); reject anything that would read past it.  The
+	 * slot cannot be trusted here, so leak this RX WQE rather than repost
+	 * the wrong one -- as in the SGE-address mismatch path below.
+	 */
+	if ((u8 *)(sge + 1) > (u8 *)rq->queue_mem_ptr + rq->queue_size) {
+		dev_err(hwc->dev, "HWC RX: SGE past RQ buffer, oob_div4=%u\n",
+			dma_oob->inline_oob_size_div4);
+		return;
+	}
+
+	/* Recover the originating RX slot from the SGE address.  Of the three
+	 * terms here only sge->address lives in device-accessible RQ memory;
+	 * rq_base_addr and max_resp_msg_size are driver-private constants.  An
+	 * in-range but wrong/unaligned SGE (corrupted WQE, or a malicious host
+	 * in a CVM) would otherwise truncate onto a neighbouring slot, letting
+	 * us read a stale response that could complete the wrong, reused
+	 * in-flight request.  Require the index to be in range AND the address
+	 * to exactly match the value the driver posted for that slot.
+	 */
 	rq_base_addr = hwc_rxq->msg_buf->mem_info.dma_handle;
 	rx_req_idx = (sge->address - rq_base_addr) / hwc->max_resp_msg_size;
 
-	if (rx_req_idx >= hwc_rxq->msg_buf->num_reqs) {
-		dev_err(hwc->dev, "HWC RX: wrong rx_req_idx=%llu, num_reqs=%u\n",
-			rx_req_idx, hwc_rxq->msg_buf->num_reqs);
+	if (rx_req_idx >= hwc_rxq->queue_depth) {
+		/* Cannot trust which WQE this is, so we cannot safely repost
+		 * it; leak one RX WQE and bail.  An out-of-range index means
+		 * a corrupted SGE from hardware (or host tampering), an
+		 * unrecoverable device error.
+		 */
+		dev_err(hwc->dev, "HWC RX: SGE idx %llu out of range\n",
+			rx_req_idx);
 		return;
 	}
 
 	rx_req = &hwc_rxq->msg_buf->reqs[rx_req_idx];
+	if (sge->address != (u64)rx_req->buf_sge_addr) {
+		/* In-range index but the address does not match what the
+		 * driver posted for that slot; the same unrecoverable case,
+		 * so leak this WQE rather than repost the wrong one.
+		 */
+		dev_err(hwc->dev, "HWC RX: invalid SGE address %llx (idx=%llu)\n",
+			sge->address, rx_req_idx);
+		return;
+	}
+
 	resp = (struct gdma_resp_hdr *)rx_req->buf_va;
 
+	/* Validate resp_len covers the response header before reading
+	 * hwc_msg_id.  A short response leaves stale data from the
+	 * previous buffer occupant, which could match a live slot and
+	 * complete the wrong request.
+	 */
+	if (rx_oob->tx_oob_data_size < sizeof(*resp)) {
+		dev_err(hwc->dev, "HWC RX: short resp_len=%u\n",
+			rx_oob->tx_oob_data_size);
+		mana_hwc_post_rx_wqe(hwc_rxq, rx_req);
+		return;
+	}
+
 	/* Read msg_id once from DMA buffer to prevent TOCTOU:
 	 * DMA memory is shared/unencrypted in CVMs - host can
 	 * modify it between reads.
@@ -281,6 +354,7 @@ static void mana_hwc_rx_event_handler(void *ctx, u32 gdma_rxq_id,
 	msg_id = READ_ONCE(resp->response.hwc_msg_id);
 	if (msg_id >= hwc->num_inflight_msg) {
 		dev_err(hwc->dev, "HWC RX: wrong msg_id=%u\n", msg_id);
+		mana_hwc_post_rx_wqe(hwc_rxq, rx_req);
 		return;
 	}
 
-- 
2.43.0


^ permalink raw reply related

* [PATCH net-next v2 5/7] net: mana: fix HWC teardown safety with setup_active flag and destroy ordering
From: Long Li @ 2026-07-21 23:43 UTC (permalink / raw)
  To: Long Li, Konstantin Taranov, Jakub Kicinski, David S . Miller,
	Paolo Abeni, Eric Dumazet, Andrew Lunn, Jason Gunthorpe,
	Leon Romanovsky, Haiyang Zhang, K . Y . Srinivasan, Wei Liu,
	Dexuan Cui, shradhagupta, Simon Horman, ernis, stephen
  Cc: netdev, linux-rdma, linux-hyperv, linux-kernel
In-Reply-To: <20260721234339.1476932-1-longli@microsoft.com>

Two teardown hazards let the hardware touch memory the driver freed.

First, once mana_smc_setup_hwc() succeeds the device has active MST
entries and can DMA into the HWC queue buffers.  If a later step in
mana_hwc_establish_channel() fails, the caller had no reliable way to
know teardown was required and could free those buffers while the
mappings were still live -- a DMA-after-free.  max_num_cqs was used as a
"HWC is up" proxy, but it is only set when the init EQE arrives.

Add a setup_active flag, set the moment setup_hwc activates MST entries.
establish_channel() now tears down on any later failure and clears
setup_active once teardown succeeds; destroy_channel() gates teardown on
setup_active.  max_num_cqs is no longer reset: it is an immutable bound
(see gdma.h) and cq_table == NULL is the sole teardown signal.

Second, destroy_channel() freed the TXQ/RXQ buffers while the HWC EQ was
still on the interrupt dispatch list, so an in-flight interrupt could run
the handler against freed buffers:

  CPU A (mana_gd_intr, hard IRQ)        CPU B (destroy_channel)
  ----------------------------------    ------------------------------
                                        free TXQ/RXQ DMA buffers
  handler accesses RQ/TXQ buffers       (EQ still registered)

Destroy the CQ first: mana_hwc_destroy_cq() -> mana_gd_deregister_irq()
removes the EQ via list_del_rcu() + synchronize_rcu(), after which no
handler can reach the queues; only then free the TXQ and RXQ.

Fixes: ca9c54d2d6a5 ("net: mana: Add a driver for Microsoft Azure Network Adapter (MANA)")
Signed-off-by: Long Li <longli@microsoft.com>
---
 .../net/ethernet/microsoft/mana/hw_channel.c  | 102 ++++++++++++++----
 include/net/mana/gdma.h                       |   8 +-
 include/net/mana/hw_channel.h                 |   8 ++
 3 files changed, 96 insertions(+), 22 deletions(-)

diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index 515c45c4a890..5bc873d548d5 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -4,6 +4,7 @@
 #include <net/mana/gdma.h>
 #include <net/mana/mana.h>
 #include <net/mana/hw_channel.h>
+#include <linux/pci.h>
 #include <linux/vmalloc.h>
 
 static int mana_hwc_get_msg_index(struct hw_channel_context *hwc, u16 *msg_id)
@@ -768,20 +769,33 @@ static int mana_hwc_establish_channel(struct gdma_context *gc, u16 *q_depth,
 	if (err)
 		return err;
 
-	if (!wait_for_completion_timeout(&hwc->hwc_init_eqe_comp, 60 * HZ))
-		return -ETIMEDOUT;
+	/* setup_hwc activated MST entries — hardware can now DMA into
+	 * our queue buffers.  If anything below fails, we must tear
+	 * down before returning so the caller doesn't need to track
+	 * whether setup_hwc succeeded.
+	 */
+	hwc->setup_active = true;
+
+	if (!wait_for_completion_timeout(&hwc->hwc_init_eqe_comp, 60 * HZ)) {
+		err = -ETIMEDOUT;
+		goto teardown;
+	}
 
 	*q_depth = hwc->hwc_init_q_depth_max;
 	*max_req_msg_size = hwc->hwc_init_max_req_msg_size;
 	*max_resp_msg_size = hwc->hwc_init_max_resp_msg_size;
 
 	/* Both were set in mana_hwc_init_event_handler(). */
-	if (WARN_ON(cq->id >= gc->max_num_cqs))
-		return -EPROTO;
+	if (WARN_ON(cq->id >= gc->max_num_cqs)) {
+		err = -EPROTO;
+		goto teardown;
+	}
 
 	cq_table = vcalloc(gc->max_num_cqs, sizeof(*cq_table));
-	if (!cq_table)
-		return -ENOMEM;
+	if (!cq_table) {
+		err = -ENOMEM;
+		goto teardown;
+	}
 
 	rcu_assign_pointer(cq_table[cq->id], cq);
 	/* Publish the fully-initialised table last; pairs with the
@@ -790,6 +804,16 @@ static int mana_hwc_establish_channel(struct gdma_context *gc, u16 *q_depth,
 	rcu_assign_pointer(gc->cq_table, cq_table);
 
 	return 0;
+
+teardown:
+	{
+		int td_err = mana_smc_teardown_hwc(&gc->shm_channel, false);
+
+		if (!td_err)
+			hwc->setup_active = false;
+
+		return td_err ? td_err : err;
+	}
 }
 
 static int mana_hwc_init_queues(struct hw_channel_context *hwc, u16 q_depth,
@@ -902,11 +926,58 @@ void mana_hwc_destroy_channel(struct gdma_context *gc)
 	if (!hwc)
 		return;
 
-	/* gc->max_num_cqs is set in mana_hwc_init_event_handler(). If it's
-	 * non-zero, the HWC worked and we should tear down the HWC here.
+	/* Tear down the HWC if setup_hwc previously activated MST entries.
+	 * This is the definitive flag — unlike max_num_cqs which depends
+	 * on the init EQE arriving.
 	 */
-	if (gc->max_num_cqs > 0)
-		mana_smc_teardown_hwc(&gc->shm_channel, false);
+	if (hwc->setup_active) {
+		int td_err = mana_smc_teardown_hwc(&gc->shm_channel, false);
+
+		if (td_err) {
+			dev_err(gc->dev, "HWC teardown failed: %d, issuing FLR\n",
+				td_err);
+
+			/* On systems without IOMMU, freeing DMA memory with
+			 * active hardware MST mappings risks memory corruption.
+			 * Issue FLR to force-reset the device and invalidate
+			 * all hardware state including MST entries.
+			 */
+			td_err = pcie_flr(to_pci_dev(gc->dev));
+			if (td_err) {
+				/* Device wedged: teardown and FLR
+				 * both failed, so hardware may still
+				 * DMA into our queue buffers.  Leak
+				 * all HWC DMA buffers instead of
+				 * freeing memory the device can still
+				 * reach; without an IOMMU the freed
+				 * pages could be reused under live
+				 * DMA.
+				 */
+
+				dev_warn(gc->dev,
+					 "Leaked HWC DMA buffers (CQ/RQ/TXQ) to prevent memory corruption. Device is no longer usable.\n");
+
+				/* Do not call destroy_cq/wq below;
+				 * they would dma_free_coherent().
+				 * Leave hwc/cq/rxq/txq allocated.
+				 */
+				return;
+			}
+
+			dev_info(gc->dev, "FLR succeeded, hardware state cleared\n");
+		}
+
+		hwc->setup_active = false;
+	}
+
+	/* Tear down the HWC CQ object first — mana_hwc_destroy_cq()
+	 * both unpublishes the CQ from cq_table (+synchronize_rcu) and
+	 * deregisters the HWC EQ from the interrupt handler list (via
+	 * mana_gd_deregister_irq + synchronize_rcu), guaranteeing no
+	 * interrupt handler can access RQ/TXQ buffers after this point.
+	 */
+	if (hwc->cq)
+		mana_hwc_destroy_cq(hwc->gdma_dev->gdma_context, hwc->cq);
 
 	if (hwc->txq)
 		mana_hwc_destroy_wq(hwc, hwc->txq);
@@ -914,17 +985,6 @@ void mana_hwc_destroy_channel(struct gdma_context *gc)
 	if (hwc->rxq)
 		mana_hwc_destroy_wq(hwc, hwc->rxq);
 
-	if (hwc->cq)
-		mana_hwc_destroy_cq(hwc->gdma_dev->gdma_context, hwc->cq);
-
-	/* Reset only after mana_hwc_destroy_cq() above has run with a valid
-	 * max_num_cqs so mana_gd_destroy_cq() clears the CQ table slot and
-	 * waits out in-flight EQ handlers (synchronize_rcu) before the CQ is
-	 * freed.  Clearing it earlier would make that path early-return and
-	 * skip the slot clear, leaving a dangling cq_table entry.
-	 */
-	gc->max_num_cqs = 0;
-
 	kfree(hwc->caller_ctx);
 	hwc->caller_ctx = NULL;
 
diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h
index da52701e7816..9ca7cf523366 100644
--- a/include/net/mana/gdma.h
+++ b/include/net/mana/gdma.h
@@ -428,7 +428,13 @@ struct gdma_context {
 	/* L2 MTU */
 	u16 adapter_mtu;
 
-	/* This maps a CQ index to the queue structure. */
+	/* Size of cq_table, i.e. the largest valid CQ index + 1.  Set once
+	 * when cq_table is allocated and treated as immutable for the
+	 * table's lifetime (a bound only) -- it is never reset on teardown.
+	 * cq_table == NULL is the sole "table torn down" signal, so every
+	 * cq_table[id] access must guard with both !cq_table (gone) and
+	 * id >= max_num_cqs (out of bounds).
+	 */
 	unsigned int		max_num_cqs;
 	/* Both the base pointer and each entry are RCU-managed.  The fast
 	 * path (mana_gd_process_eqe) reads the base via rcu_dereference()
diff --git a/include/net/mana/hw_channel.h b/include/net/mana/hw_channel.h
index 73671f479399..684dcec8e612 100644
--- a/include/net/mana/hw_channel.h
+++ b/include/net/mana/hw_channel.h
@@ -200,6 +200,14 @@ struct hw_channel_context {
 	u32 pf_dest_vrcq_id;
 	u32 hwc_timeout;
 
+	/* Set after mana_smc_setup_hwc() succeeds (hardware has active
+	 * MST entries).  On recoverable paths (establish_channel)
+	 * cleared only after successful teardown so a retry remains
+	 * possible.  On the terminal destroy_channel path, cleared
+	 * unconditionally since hwc is about to be freed.
+	 */
+	bool setup_active;
+
 	struct hwc_caller_ctx *caller_ctx;
 };
 
-- 
2.43.0


^ permalink raw reply related

* [PATCH net-next v2 6/7] net: mana: support concurrent HWC requests with proper synchronization
From: Long Li @ 2026-07-21 23:43 UTC (permalink / raw)
  To: Long Li, Konstantin Taranov, Jakub Kicinski, David S . Miller,
	Paolo Abeni, Eric Dumazet, Andrew Lunn, Jason Gunthorpe,
	Leon Romanovsky, Haiyang Zhang, K . Y . Srinivasan, Wei Liu,
	Dexuan Cui, shradhagupta, Simon Horman, ernis, stephen
  Cc: netdev, linux-rdma, linux-hyperv, linux-kernel
In-Reply-To: <20260721234339.1476932-1-longli@microsoft.com>

The HWC serialized all management commands behind a depth-1 semaphore, a
bottleneck when many commands must be issued concurrently.  Allowing
multiple in-flight requests exposes several shared-state races that this
patch addresses together.

  - Replace the semaphore with a waitqueue + bitmap scheme so senders
    acquire message slots in parallel and sleep only when all slots are
    busy.

  - A response is delivered by handle_resp() in CQ interrupt context
    while the issuing sender may concurrently time out and return; both
    touch the same caller_ctx slot (output_buf, output_buflen, error):

      CPU A (sender timeout)            CPU B (handle_resp, IRQ)
      ------------------------------    ------------------------------
      read/clear output_buf             write response into output_buf,
                                        complete the slot

    Add a per-slot spinlock to make these mutually exclusive, a per-slot
    refcount so the last of {sender, handle_resp} releases the bitmap
    slot (no double-release, no stuck slot), and an embedded completion
    the sender waits on.  The sender always NULLs output_buf after
    waking, so a late handle_resp() skips the copy.

  - Add a per-queue lock to hwc_wq; mana_gd_post_and_ring() is not safe
    to call concurrently on the same queue.

  - Add channel_up (cleared under the bitmap lock in destroy_channel) and
    hwc_timed_out flags so new slot acquisitions are rejected during
    teardown and after a timeout.

  - destroy_channel() must not free the HWC while senders are still in
    flight.  Count active senders in a gc->hwc_lock-protected counter;
    destroy_channel() force-completes the in-flight slots (-ENODEV), then
    drains the counter to zero with wait_event_lock_irq() before freeing.
    The counter changes only under hwc_lock and the last sender's
    wake_up() runs under that lock, so evaluating the drain condition
    under hwc_lock guarantees the waking sender has already dropped the
    lock -- finished touching gc -- before the drain returns; it cannot
    race the later free of gc.  The waitqueue itself lives on
    gdma_context, not hwc, so the wake never dereferences freed hwc.

  - The sender looks up the channel via gc->hwc.driver_data, which
    destroy_channel() clears and then frees.  Without serialization the
    lookup and the reference can straddle the free:

      CPU A (mana_gd_send_request)      CPU B (destroy_channel)
      ------------------------------    ------------------------------
      hwc = gc->hwc.driver_data;  // ok
                                        driver_data = NULL;
                                        wait active_senders == 0;  // 0!
                                        kfree(hwc);
      hwc->active_senders++;            // use-after-free

    Guard driver_data with a new gc->hwc_lock spinlock, taken by the
    readers (mana_gd_send_request, mana_need_log, mana_serv_reset) and by
    the publish/clear, so "load the pointer + take a sender reference" is
    atomic against the clear.  After the clear a sender either already
    holds a reference (and is waited for) or observes NULL and returns
    -ENODEV.  These are all control-plane paths (HWC commands sleep,
    reset runs on a workqueue), so a plain spinlock -- not RCU -- is
    sufficient.

This adds the concurrency infrastructure at the bootstrap queue depth of
1; the next patch raises it to the device-reported maximum.

Signed-off-by: Long Li <longli@microsoft.com>
---
 .../net/ethernet/microsoft/mana/gdma_main.c   |  55 ++-
 .../net/ethernet/microsoft/mana/hw_channel.c  | 352 +++++++++++++++---
 include/net/mana/gdma.h                       |  15 +
 include/net/mana/hw_channel.h                 |  33 +-
 4 files changed, 408 insertions(+), 47 deletions(-)

diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c b/drivers/net/ethernet/microsoft/mana/gdma_main.c
index 9edebf7c097c..17711ee37c8b 100644
--- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
+++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
@@ -161,6 +161,8 @@ static int mana_gd_init_registers(struct pci_dev *pdev)
 bool mana_need_log(struct gdma_context *gc, int err)
 {
 	struct hw_channel_context *hwc;
+	bool need_log = true;
+	unsigned long flags;
 
 	if (err != -ETIMEDOUT)
 		return true;
@@ -168,11 +170,13 @@ bool mana_need_log(struct gdma_context *gc, int err)
 	if (!gc)
 		return true;
 
+	spin_lock_irqsave(&gc->hwc_lock, flags);
 	hwc = gc->hwc.driver_data;
 	if (hwc && hwc->hwc_timeout == 0)
-		return false;
+		need_log = false;
+	spin_unlock_irqrestore(&gc->hwc_lock, flags);
 
-	return true;
+	return need_log;
 }
 
 static int mana_gd_query_max_resources(struct pci_dev *pdev)
@@ -367,9 +371,27 @@ static int mana_gd_detect_devices(struct pci_dev *pdev)
 int mana_gd_send_request(struct gdma_context *gc, u32 req_len, const void *req,
 			 u32 resp_len, void *resp)
 {
-	struct hw_channel_context *hwc = gc->hwc.driver_data;
+	struct hw_channel_context *hwc;
+	unsigned long flags;
+	int err;
+
+	spin_lock_irqsave(&gc->hwc_lock, flags);
+	hwc = gc->hwc.driver_data;
+	if (!hwc) {
+		spin_unlock_irqrestore(&gc->hwc_lock, flags);
+		return -ENODEV;
+	}
+	hwc->active_senders++;
+	spin_unlock_irqrestore(&gc->hwc_lock, flags);
+
+	err = mana_hwc_send_request(hwc, req_len, req, resp_len, resp);
 
-	return mana_hwc_send_request(hwc, req_len, req, resp_len, resp);
+	spin_lock_irqsave(&gc->hwc_lock, flags);
+	if (--hwc->active_senders == 0)
+		wake_up(&gc->hwc_drain_waitq);
+	spin_unlock_irqrestore(&gc->hwc_lock, flags);
+
+	return err;
 }
 EXPORT_SYMBOL_NS(mana_gd_send_request, "NET_MANA");
 
@@ -622,6 +644,7 @@ static void mana_serv_reset(struct pci_dev *pdev)
 {
 	struct gdma_context *gc = pci_get_drvdata(pdev);
 	struct hw_channel_context *hwc;
+	unsigned long flags;
 	int ret;
 
 	if (!gc) {
@@ -631,14 +654,17 @@ static void mana_serv_reset(struct pci_dev *pdev)
 		return;
 	}
 
+	spin_lock_irqsave(&gc->hwc_lock, flags);
 	hwc = gc->hwc.driver_data;
 	if (!hwc) {
+		spin_unlock_irqrestore(&gc->hwc_lock, flags);
 		dev_err(&pdev->dev, "MANA service: no HWC\n");
 		goto out;
 	}
 
 	/* HWC is not responding in this case, so don't wait */
 	hwc->hwc_timeout = 0;
+	spin_unlock_irqrestore(&gc->hwc_lock, flags);
 
 	dev_info(&pdev->dev, "MANA reset cycle start\n");
 
@@ -1200,6 +1226,16 @@ static int mana_gd_create_dma_region(struct gdma_dev *gd,
 	if (!MANA_PAGE_ALIGNED(gmi->virt_addr))
 		return -EINVAL;
 
+	/* No RCU needed: this runs only on the data-path queue-creation
+	 * path (mana_gd_create_mana_eq/mana_gd_create_mana_wq_cq, called
+	 * by mana_en under RTNL and by mana_ib RDMA verbs, or during
+	 * init).  Every teardown path — mana_gd_remove, mana_gd_suspend,
+	 * and the HWC reset/service path (which goes through
+	 * mana_gd_suspend) — drains those consumers via mana_rdma_remove()
+	 * + mana_remove() before mana_hwc_destroy_channel() clears
+	 * gc->hwc.driver_data, so no concurrent destroy can race with
+	 * this dereference.
+	 */
 	hwc = gc->hwc.driver_data;
 	req_msg_size = struct_size(req, page_addr_list, num_page);
 	if (req_msg_size > hwc->max_req_msg_size)
@@ -1389,7 +1425,17 @@ int mana_gd_verify_vf_version(struct pci_dev *pdev)
 	struct hw_channel_context *hwc;
 	int err;
 
+	/* No RCU needed: this runs only inside mana_gd_setup, on the
+	 * probe and resume paths.  The PCI/PM core holds device_lock
+	 * across .probe/.resume and .remove/.suspend, so setup cannot
+	 * overlap teardown of the same device.  The HWC reset/service
+	 * path is additionally serialized by GC_IN_SERVICE and runs
+	 * suspend (destroy) then resume (this) sequentially in one work
+	 * item.  driver_data was just set by mana_hwc_create_channel
+	 * earlier in this same setup call, so it is live here.
+	 */
 	hwc = gc->hwc.driver_data;
+
 	mana_gd_init_req_hdr(&req.hdr, GDMA_VERIFY_VF_DRIVER_VERSION,
 			     sizeof(req), sizeof(resp));
 
@@ -2379,6 +2425,7 @@ static int mana_gd_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	mutex_init(&gc->eq_test_event_mutex);
 	mutex_init(&gc->gic_mutex);
+	spin_lock_init(&gc->hwc_lock);
 	pci_set_drvdata(pdev, gc);
 	gc->bar0_pa = pci_resource_start(pdev, 0);
 	gc->bar0_size = pci_resource_len(pdev, 0);
diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index 5bc873d548d5..616fb088c559 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -7,25 +7,52 @@
 #include <linux/pci.h>
 #include <linux/vmalloc.h>
 
+/* Acquire a free message slot from the inflight bitmap.  Returns
+ * -ENODEV if the channel is torn down, or -ETIMEDOUT if a prior HWC
+ * command has timed out (preserving the error code callers expect).
+ */
 static int mana_hwc_get_msg_index(struct hw_channel_context *hwc, u16 *msg_id)
 {
 	struct gdma_resource *r = &hwc->inflight_msg_res;
 	unsigned long flags;
 	u32 index;
 
-	down(&hwc->sema);
+	for (;;) {
+		spin_lock_irqsave(&r->lock, flags);
 
-	spin_lock_irqsave(&r->lock, flags);
+		if (!hwc->channel_up || hwc->hwc_timed_out) {
+			spin_unlock_irqrestore(&r->lock, flags);
+			return hwc->channel_up ? -ETIMEDOUT : -ENODEV;
+		}
 
-	index = find_first_zero_bit(hwc->inflight_msg_res.map,
-				    hwc->inflight_msg_res.size);
+		index = find_first_zero_bit(r->map, r->size);
+		if (index < r->size) {
+			struct hwc_caller_ctx *ctx;
+
+			bitmap_set(r->map, index, 1);
+			ctx = &hwc->caller_ctx[index];
+			reinit_completion(&ctx->comp_event);
+			refcount_set(&ctx->refcnt, 1);
+			ctx->msg_id = index;
+			ctx->error = -EINPROGRESS;
+			spin_unlock_irqrestore(&r->lock, flags);
+			break;
+		}
+		spin_unlock_irqrestore(&r->lock, flags);
 
-	bitmap_set(hwc->inflight_msg_res.map, index, 1);
+		wait_event(hwc->msg_waitq,
+			   !hwc->channel_up ||
+			   hwc->hwc_timed_out ||
+			   !bitmap_full(r->map, r->size));
 
-	spin_unlock_irqrestore(&r->lock, flags);
+		if (!hwc->channel_up)
+			return -ENODEV;
 
-	*msg_id = index;
+		if (hwc->hwc_timed_out)
+			return -ETIMEDOUT;
+	}
 
+	*msg_id = index;
 	return 0;
 }
 
@@ -35,10 +62,17 @@ static void mana_hwc_put_msg_index(struct hw_channel_context *hwc, u16 msg_id)
 	unsigned long flags;
 
 	spin_lock_irqsave(&r->lock, flags);
-	bitmap_clear(hwc->inflight_msg_res.map, msg_id, 1);
+	bitmap_clear(r->map, msg_id, 1);
 	spin_unlock_irqrestore(&r->lock, flags);
 
-	up(&hwc->sema);
+	wake_up(&hwc->msg_waitq);
+}
+
+static void hwc_ctx_put(struct hw_channel_context *hwc,
+			struct hwc_caller_ctx *ctx)
+{
+	if (refcount_dec_and_test(&ctx->refcnt))
+		mana_hwc_put_msg_index(hwc, ctx->msg_id);
 }
 
 static int mana_hwc_verify_resp_msg(const struct hwc_caller_ctx *caller_ctx,
@@ -114,22 +148,32 @@ static void mana_hwc_handle_resp(struct hw_channel_context *hwc, u32 resp_len,
 		resp_len = 0;
 	}
 
-	err = mana_hwc_verify_resp_msg(ctx, resp_msg, resp_len);
-	if (err)
-		goto out;
+	spin_lock(&ctx->lock);
 
-	ctx->status_code = resp_msg->status;
+	err = mana_hwc_verify_resp_msg(ctx, resp_msg, resp_len);
 
-	memcpy(ctx->output_buf, resp_msg, resp_len);
-out:
-	ctx->error = err;
+	if (!err && ctx->output_buf) {
+		ctx->status_code = resp_msg->status;
+		memcpy(ctx->output_buf, resp_msg, resp_len);
+		ctx->error = 0;
+	} else if (ctx->output_buf) {
+		/* Only overwrite error if the sender hasn't timed out
+		 * or been force-completed by destroy.  When output_buf
+		 * is NULL, a terminal error (-ENODEV or timeout) has
+		 * already been set — preserve it so the sender doesn't
+		 * see a spurious success.
+		 */
+		ctx->error = err;
+	}
 
-	/* Must post rx wqe before complete(), otherwise the next rx may
-	 * hit no_wqe error.
+	/* Post RX WQE before completing — the next response may arrive
+	 * immediately and needs a posted buffer.
 	 */
 	mana_hwc_post_rx_wqe(hwc->rxq, rx_req);
-
 	complete(&ctx->comp_event);
+	spin_unlock(&ctx->lock);
+
+	hwc_ctx_put(hwc, ctx);
 }
 
 static void mana_hwc_init_event_handler(void *ctx, struct gdma_queue *q_self,
@@ -641,6 +685,7 @@ static int mana_hwc_create_wq(struct hw_channel_context *hwc,
 	hwc_wq->gdma_wq = queue;
 	hwc_wq->queue_depth = q_depth;
 	hwc_wq->hwc_cq = hwc_cq;
+	spin_lock_init(&hwc_wq->lock);
 
 	err = mana_hwc_alloc_dma_buf(hwc, q_depth, max_msg_size,
 				     &hwc_wq->msg_buf);
@@ -658,7 +703,7 @@ static int mana_hwc_create_wq(struct hw_channel_context *hwc,
 	return err;
 }
 
-static int mana_hwc_post_tx_wqe(const struct hwc_wq *hwc_txq,
+static int mana_hwc_post_tx_wqe(struct hwc_wq *hwc_txq,
 				struct hwc_work_request *req,
 				u32 dest_virt_rq_id, u32 dest_virt_rcq_id,
 				bool dest_pf)
@@ -697,7 +742,11 @@ static int mana_hwc_post_tx_wqe(const struct hwc_wq *hwc_txq,
 	req->wqe_req.inline_oob_data = tx_oob;
 	req->wqe_req.client_data_unit = 0;
 
+	/* Serialize WQE posting — multiple senders may call concurrently. */
+	spin_lock(&hwc_txq->lock);
 	err = mana_gd_post_and_ring(hwc_txq->gdma_wq, &req->wqe_req, NULL);
+	spin_unlock(&hwc_txq->lock);
+
 	if (err)
 		dev_err(dev, "Failed to post WQE on HWC SQ: %d\n", err);
 	return err;
@@ -708,7 +757,7 @@ static int mana_hwc_init_inflight_msg(struct hw_channel_context *hwc,
 {
 	int err;
 
-	sema_init(&hwc->sema, num_msg);
+	init_waitqueue_head(&hwc->msg_waitq);
 
 	err = mana_gd_alloc_res_map(num_msg, &hwc->inflight_msg_res);
 	if (err)
@@ -738,18 +787,34 @@ static int mana_hwc_test_channel(struct hw_channel_context *hwc, u16 q_depth,
 	if (!ctx)
 		return -ENOMEM;
 
-	for (i = 0; i < q_depth; ++i)
+	for (i = 0; i < q_depth; ++i) {
+		spin_lock_init(&ctx[i].lock);
 		init_completion(&ctx[i].comp_event);
+	}
 
 	hwc->caller_ctx = ctx;
 
-	return mana_gd_test_eq(gc, hwc->cq->gdma_eq);
+	/* channel_up must be set before the test EQ request, because
+	 * the request goes through mana_hwc_get_msg_index() which
+	 * checks channel_up.  caller_ctx is allocated above, so
+	 * concurrent access to a NULL caller_ctx is not possible.
+	 */
+	hwc->channel_up = true;
+
+	err = mana_gd_test_eq(gc, hwc->cq->gdma_eq);
+	if (err)
+		hwc->channel_up = false;
+
+	return err;
 }
 
 static int mana_hwc_establish_channel(struct gdma_context *gc, u16 *q_depth,
 				      u32 *max_req_msg_size,
 				      u32 *max_resp_msg_size)
 {
+	/* No RCU needed: called only from mana_hwc_create_channel
+	 * during init, before the channel is published to senders.
+	 */
 	struct hw_channel_context *hwc = gc->hwc.driver_data;
 	struct gdma_queue *rq = hwc->rxq->gdma_wq;
 	struct gdma_queue *sq = hwc->txq->gdma_wq;
@@ -866,6 +931,7 @@ int mana_hwc_create_channel(struct gdma_context *gc)
 	u32 max_req_msg_size, max_resp_msg_size;
 	struct gdma_dev *gd = &gc->hwc;
 	struct hw_channel_context *hwc;
+	unsigned long flags;
 	u16 q_depth_max;
 	int err;
 
@@ -874,10 +940,11 @@ int mana_hwc_create_channel(struct gdma_context *gc)
 		return -ENOMEM;
 
 	gd->gdma_context = gc;
-	gd->driver_data = hwc;
 	hwc->gdma_dev = gd;
 	hwc->dev = gc->dev;
 	hwc->hwc_timeout = HW_CHANNEL_WAIT_RESOURCE_TIMEOUT_MS;
+	hwc->active_senders = 0;
+	init_waitqueue_head(&gc->hwc_drain_waitq);
 
 	/* HWC's instance number is always 0. */
 	gd->dev_id.as_uint32 = 0;
@@ -886,6 +953,15 @@ int mana_hwc_create_channel(struct gdma_context *gc)
 	gd->pdid = INVALID_PDID;
 	gd->doorbell = INVALID_DOORBELL;
 
+	/* Publish driver_data last, under hwc_lock: the lock orders the hwc
+	 * initialisation above before the pointer becomes visible and
+	 * serialises the publish against the control-plane readers in
+	 * mana_gd_send_request(), mana_need_log() and mana_serv_reset().
+	 */
+	spin_lock_irqsave(&gc->hwc_lock, flags);
+	gc->hwc.driver_data = hwc;
+	spin_unlock_irqrestore(&gc->hwc_lock, flags);
+
 	/* mana_hwc_init_queues() only creates the required data structures,
 	 * and doesn't touch the HWC device.
 	 */
@@ -920,15 +996,118 @@ int mana_hwc_create_channel(struct gdma_context *gc)
 
 void mana_hwc_destroy_channel(struct gdma_context *gc)
 {
+	/* This is the only destroy entry point.  driver_data is read
+	 * plainly here (teardown is serialised against other teardown);
+	 * it is cleared under hwc_lock below before hwc is freed.
+	 */
 	struct hw_channel_context *hwc = gc->hwc.driver_data;
 	struct gdma_queue __rcu **old_cq_table;
+	unsigned long flags;
 
 	if (!hwc)
 		return;
 
+	/* Prevent new requests from starting and wake any
+	 * threads waiting for a free msg slot.  Set channel_up under
+	 * the bitmap lock so get_msg_index() cannot acquire a slot
+	 * and increment active_senders after this point.
+	 *
+	 * If channel_up is already false (e.g. init failed before
+	 * the channel was established), skip the lock — it may not
+	 * have been initialized yet, and no senders can be active.
+	 */
+	if (hwc->channel_up) {
+		spin_lock_irqsave(&hwc->inflight_msg_res.lock, flags);
+		hwc->channel_up = false;
+		spin_unlock_irqrestore(&hwc->inflight_msg_res.lock, flags);
+		wake_up_all(&hwc->msg_waitq);
+	}
+
+	/* Clear the pointer under hwc_lock so new callers in
+	 * mana_gd_send_request() see NULL and return -ENODEV.  The lock
+	 * makes the readers' "load driver_data + active_senders++"
+	 * atomic against this store, so once it returns no new sender can
+	 * take a reference; the active_senders drain below waits out those
+	 * that already did, before their hwc is freed.
+	 */
+	spin_lock_irqsave(&gc->hwc_lock, flags);
+	gc->hwc.driver_data = NULL;
+	spin_unlock_irqrestore(&gc->hwc_lock, flags);
+
+	/* Force-complete any in-flight senders so they observe -ENODEV,
+	 * return, and drop their references.  This runs before the HWC
+	 * hardware teardown below, so a live interrupt may still deliver
+	 * a real response via handle_resp() concurrently — that is safe
+	 * because the per-slot refcount model tolerates a concurrent
+	 * complete() and both paths (handle_resp and this loop) drop
+	 * their refs without double-releasing the slot.
+	 */
+	if (hwc->caller_ctx) {
+		struct hwc_caller_ctx *ctx;
+		int i;
+
+		for (i = 0; i < hwc->num_inflight_msg; i++) {
+			if (!test_bit(i, hwc->inflight_msg_res.map))
+				continue;
+
+			ctx = &hwc->caller_ctx[i];
+
+			/* Wake senders blocked on wait_for_completion.
+			 * Set error under lock to avoid racing with
+			 * handle_resp() which writes error under the
+			 * same lock.  The sender NULLs output_buf
+			 * after waking — doing it here would race
+			 * with a sender that hasn't set output_buf yet.
+			 */
+			spin_lock_irqsave(&ctx->lock, flags);
+			ctx->error = -ENODEV;
+			complete(&ctx->comp_event);
+			spin_unlock_irqrestore(&ctx->lock, flags);
+		}
+	}
+
+	/* Wait for all sender threads to finish and drop their refs
+	 * before touching the hardware or freeing anything.  Draining
+	 * here -- ahead of the teardown/FLR block below -- guarantees no
+	 * in-flight sender is still running when this function returns on
+	 * any path, including the FLR-failure early return; otherwise a
+	 * stranded sender would dereference gc->hwc_lock /
+	 * gc->hwc_drain_waitq after the caller frees gc.
+	 * After this, only slots held by timed-out senders whose
+	 * handle_resp() never ran remain in the bitmap.
+	 *
+	 * active_senders is only ever modified under hwc_lock, and the
+	 * last sender's wake_up() runs under that lock before it is
+	 * released.  Evaluating the condition under hwc_lock therefore
+	 * guarantees that once we observe 0 the waking sender has
+	 * already dropped the lock -- i.e. finished touching gc -- so it
+	 * cannot race the caller freeing gc after this returns.
+	 */
+	spin_lock_irq(&gc->hwc_lock);
+	wait_event_lock_irq(gc->hwc_drain_waitq,
+			    hwc->active_senders == 0, gc->hwc_lock);
+	spin_unlock_irq(&gc->hwc_lock);
+
 	/* Tear down the HWC if setup_hwc previously activated MST entries.
 	 * This is the definitive flag — unlike max_num_cqs which depends
 	 * on the init EQE arriving.
+	 *
+	 * The return value is intentionally not checked.  This is the
+	 * terminal cleanup path (device removal, suspend, or init
+	 * failure) — resources must be freed regardless.  If teardown
+	 * fails, hardware may still have active MST entries, but:
+	 *
+	 *  - Interrupts: mana_hwc_destroy_cq() below calls
+	 *    mana_gd_deregister_irq() which removes the HWC EQ from
+	 *    the interrupt dispatch list via list_del_rcu() +
+	 *    synchronize_rcu().  After that, no interrupt handler can
+	 *    invoke handle_resp() or access CQ/RQ buffers — even if
+	 *    the IRQ is shared with data path queues.
+	 *
+	 *  - DMA: mana_hwc_destroy_wq() frees DMA buffers via
+	 *    dma_free_coherent() which unmaps the IOVA from the
+	 *    IOMMU.  Any stale hardware DMA to the old address
+	 *    faults at the IOMMU, not in kernel memory.
 	 */
 	if (hwc->setup_active) {
 		int td_err = mana_smc_teardown_hwc(&gc->shm_channel, false);
@@ -947,7 +1126,9 @@ void mana_hwc_destroy_channel(struct gdma_context *gc)
 				/* Device wedged: teardown and FLR
 				 * both failed, so hardware may still
 				 * DMA into our queue buffers.  Leak
-				 * all HWC DMA buffers instead of
+				 * all HWC DMA buffers -- and the EQ
+				 * IRQ registration, whose safe removal
+				 * needs those queues -- instead of
 				 * freeing memory the device can still
 				 * reach; without an IOMMU the freed
 				 * pages could be reused under live
@@ -972,9 +1153,12 @@ void mana_hwc_destroy_channel(struct gdma_context *gc)
 
 	/* Tear down the HWC CQ object first — mana_hwc_destroy_cq()
 	 * both unpublishes the CQ from cq_table (+synchronize_rcu) and
-	 * deregisters the HWC EQ from the interrupt handler list (via
-	 * mana_gd_deregister_irq + synchronize_rcu), guaranteeing no
-	 * interrupt handler can access RQ/TXQ buffers after this point.
+	 * deregisters the HWC EQ from the interrupt handler's RCU list
+	 * (via mana_gd_deregister_irq + synchronize_rcu), guaranteeing
+	 * no interrupt handler can access RQ/TXQ buffers after this
+	 * point.  The active_senders drain above ensures no sender is
+	 * accessing the CQ via txq->hwc_cq when it is destroyed.  Then
+	 * destroy TXQ and RQ safely.
 	 */
 	if (hwc->cq)
 		mana_hwc_destroy_cq(hwc->gdma_dev->gdma_context, hwc->cq);
@@ -985,6 +1169,23 @@ void mana_hwc_destroy_channel(struct gdma_context *gc)
 	if (hwc->rxq)
 		mana_hwc_destroy_wq(hwc, hwc->rxq);
 
+	/* Release any slots still held — these belong to timed-out
+	 * senders where handle_resp() never ran (refcount = 1 with
+	 * handle_resp's ref still outstanding).
+	 */
+	if (hwc->caller_ctx) {
+		struct hwc_caller_ctx *ctx;
+		int i;
+
+		for (i = 0; i < hwc->num_inflight_msg; i++) {
+			if (!test_bit(i, hwc->inflight_msg_res.map))
+				continue;
+
+			ctx = &hwc->caller_ctx[i];
+			hwc_ctx_put(hwc, ctx);
+		}
+	}
+
 	kfree(hwc->caller_ctx);
 	hwc->caller_ctx = NULL;
 
@@ -998,7 +1199,6 @@ void mana_hwc_destroy_channel(struct gdma_context *gc)
 	hwc->hwc_timeout = 0;
 
 	kfree(hwc);
-	gc->hwc.driver_data = NULL;
 	gc->hwc.gdma_context = NULL;
 
 	old_cq_table = rcu_replace_pointer(gc->cq_table, NULL, true);
@@ -1014,13 +1214,17 @@ int mana_hwc_send_request(struct hw_channel_context *hwc, u32 req_len,
 	struct hwc_wq *txq = hwc->txq;
 	struct gdma_req_hdr *req_msg;
 	struct hwc_caller_ctx *ctx;
+	unsigned long flags;
 	u32 dest_vrcq = 0;
 	u32 dest_vrq = 0;
 	u32 command;
+	u32 status;
 	u16 msg_id;
 	int err;
 
-	mana_hwc_get_msg_index(hwc, &msg_id);
+	err = mana_hwc_get_msg_index(hwc, &msg_id);
+	if (err)
+		return err;
 
 	tx_wr = &txq->msg_buf->reqs[msg_id];
 
@@ -1032,8 +1236,11 @@ int mana_hwc_send_request(struct hw_channel_context *hwc, u32 req_len,
 	}
 
 	ctx = hwc->caller_ctx + msg_id;
+
+	spin_lock_irqsave(&ctx->lock, flags);
 	ctx->output_buf = resp;
 	ctx->output_buflen = resp_len;
+	spin_unlock_irqrestore(&ctx->lock, flags);
 
 	req_msg = (struct gdma_req_hdr *)tx_wr->buf_va;
 	if (req)
@@ -1049,8 +1256,14 @@ int mana_hwc_send_request(struct hw_channel_context *hwc, u32 req_len,
 		dest_vrcq = hwc->pf_dest_vrcq_id;
 	}
 
+	/* Take handle_resp's ref before posting — hardware can respond
+	 * immediately after the doorbell ring.
+	 */
+	refcount_inc(&ctx->refcnt);
+
 	err = mana_hwc_post_tx_wqe(txq, tx_wr, dest_vrq, dest_vrcq, false);
 	if (err) {
+		refcount_dec(&ctx->refcnt);
 		dev_err(hwc->dev, "HWC: Failed to post send WQE: %d\n", err);
 		goto out;
 	}
@@ -1061,31 +1274,86 @@ int mana_hwc_send_request(struct hw_channel_context *hwc, u32 req_len,
 			dev_err(hwc->dev, "Command 0x%x timed out: %u ms\n",
 				command, hwc->hwc_timeout);
 
-		/* Reduce further waiting if HWC no response */
+		/* NULL out output_buf so a late handle_resp() won't write
+		 * into the caller's buffer after the sender returns, then
+		 * check whether handle_resp() already delivered a valid
+		 * response between the timeout firing and this lock
+		 * acquisition — ctx->error != -EINPROGRESS means it ran.
+		 */
+		spin_lock_irqsave(&ctx->lock, flags);
+		ctx->output_buf = NULL;
+		err = ctx->error;
+		status = ctx->status_code;
+		spin_unlock_irqrestore(&ctx->lock, flags);
+
+		if (err != -EINPROGRESS) {
+			/* handle_resp() delivered a valid response just after
+			 * the timeout fired.  The hardware is alive, so use
+			 * the response and leave the channel usable; do not
+			 * latch hwc_timed_out or degrade hwc_timeout for what
+			 * turned out to be a transient race.
+			 */
+			hwc_ctx_put(hwc, ctx);
+			goto check_status;
+		}
+
+		/* Genuine timeout: no response arrived.  Reduce further
+		 * waiting, and mark the channel timed out under the bitmap
+		 * lock so get_msg_index() cannot acquire new slots after this.
+		 */
 		if (hwc->hwc_timeout > 1)
 			hwc->hwc_timeout = 1;
 
+		spin_lock_irqsave(&hwc->inflight_msg_res.lock, flags);
+		hwc->hwc_timed_out = true;
+		spin_unlock_irqrestore(&hwc->inflight_msg_res.lock, flags);
+		wake_up_all(&hwc->msg_waitq);
+
 		err = -ETIMEDOUT;
-		goto out;
+		hwc_ctx_put(hwc, ctx);
+		goto done;
 	}
 
-	if (ctx->error) {
-		err = ctx->error;
-		goto out;
-	}
+	/* NULL output_buf so a late handle_resp() won't memcpy into
+	 * the caller's buffer after the sender exits.  Read error and
+	 * status_code under the same lock — after hwc_ctx_put the slot
+	 * may be reused and these fields overwritten.
+	 */
+	spin_lock_irqsave(&ctx->lock, flags);
+	ctx->output_buf = NULL;
+	err = ctx->error;
+	status = ctx->status_code;
+	spin_unlock_irqrestore(&ctx->lock, flags);
+	hwc_ctx_put(hwc, ctx);
+
+check_status:
+	if (err)
+		goto done;
 
-	if (ctx->status_code && ctx->status_code != GDMA_STATUS_MORE_ENTRIES) {
-		if (ctx->status_code == GDMA_STATUS_CMD_UNSUPPORTED) {
+	if (status && status != GDMA_STATUS_MORE_ENTRIES) {
+		if (status == GDMA_STATUS_CMD_UNSUPPORTED) {
 			err = -EOPNOTSUPP;
-			goto out;
+			goto done;
 		}
+
 		if (command != MANA_QUERY_PHY_STAT)
 			dev_err(hwc->dev, "Command 0x%x failed with status: 0x%x\n",
-				command, ctx->status_code);
+				command, status);
 		err = -EPROTO;
-		goto out;
+		goto done;
 	}
+
+	err = 0;
+	goto done;
 out:
-	mana_hwc_put_msg_index(hwc, msg_id);
+	/* Pre-post error paths: no WQE was submitted so handle_resp()
+	 * cannot race here.  refcount is 1 (no second ref taken).
+	 */
+	ctx = hwc->caller_ctx + msg_id;
+	spin_lock_irqsave(&ctx->lock, flags);
+	ctx->output_buf = NULL;
+	spin_unlock_irqrestore(&ctx->lock, flags);
+	hwc_ctx_put(hwc, ctx);
+done:
 	return err;
 }
diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h
index 9ca7cf523366..01e845237b6a 100644
--- a/include/net/mana/gdma.h
+++ b/include/net/mana/gdma.h
@@ -481,6 +481,21 @@ struct gdma_context {
 	/* Hardware communication channel (HWC) */
 	struct gdma_dev		hwc;
 
+	/* destroy_channel() waits here for all HWC senders to exit.
+	 * Lives on gc (not hwc) so wake_up() after the last sender's
+	 * atomic_dec doesn't dereference freed hwc memory.
+	 */
+	wait_queue_head_t	hwc_drain_waitq;
+
+	/* Serializes hwc.driver_data (the hw_channel_context pointer)
+	 * between the control-plane readers in mana_gd_send_request(),
+	 * mana_need_log() and mana_serv_reset() and the publish/clear in
+	 * mana_hwc_create_channel()/mana_hwc_destroy_channel().  All users
+	 * are control-plane (HWC commands sleep; reset runs on a workqueue),
+	 * so a plain spinlock -- not RCU -- is sufficient.
+	 */
+	spinlock_t		hwc_lock;
+
 	/* Azure network adapter */
 	struct gdma_dev		mana;
 
diff --git a/include/net/mana/hw_channel.h b/include/net/mana/hw_channel.h
index 684dcec8e612..30bc236286dc 100644
--- a/include/net/mana/hw_channel.h
+++ b/include/net/mana/hw_channel.h
@@ -164,6 +164,9 @@ struct hwc_wq {
 	u16 queue_depth;
 
 	struct hwc_cq *hwc_cq;
+
+	/* Serializes concurrent mana_gd_post_and_ring() calls. */
+	spinlock_t lock;
 };
 
 struct hwc_caller_ctx {
@@ -173,6 +176,17 @@ struct hwc_caller_ctx {
 
 	u32 error; /* Linux error code */
 	u32 status_code;
+
+	/* Protects output_buf against concurrent access from
+	 * handle_resp() (CQ interrupt) and the sender timeout path.
+	 */
+	spinlock_t lock;
+
+	/* Tracks sender + handle_resp ownership.  The last put
+	 * (refcount reaches 0) releases the bitmap slot.
+	 */
+	refcount_t refcnt;
+	u16 msg_id;
 };
 
 struct hw_channel_context {
@@ -193,13 +207,24 @@ struct hw_channel_context {
 	struct hwc_wq *txq;
 	struct hwc_cq *cq;
 
-	struct semaphore sema;
 	struct gdma_resource inflight_msg_res;
+	/* Waitqueue for senders blocked on a full inflight bitmap. */
+	wait_queue_head_t msg_waitq;
 
 	u32 pf_dest_vrq_id;
 	u32 pf_dest_vrcq_id;
 	u32 hwc_timeout;
 
+	/* Set after channel is fully established; cleared on teardown to
+	 * abort waiters in mana_hwc_get_msg_index() and reject new sends.
+	 */
+	bool channel_up;
+
+	/* Set on first HWC timeout.  Causes get_msg_index() to return
+	 * -ETIMEDOUT instead of waiting, draining all queued senders.
+	 */
+	bool hwc_timed_out;
+
 	/* Set after mana_smc_setup_hwc() succeeds (hardware has active
 	 * MST entries).  On recoverable paths (establish_channel)
 	 * cleared only after successful teardown so a retry remains
@@ -208,6 +233,12 @@ struct hw_channel_context {
 	 */
 	bool setup_active;
 
+	/* Count of in-flight mana_gd_send_request() callers.  Protected
+	 * by gc->hwc_lock; the last sender to drop it to zero wakes
+	 * gc->hwc_drain_waitq for the mana_hwc_destroy_channel() drain.
+	 */
+	unsigned int active_senders;
+
 	struct hwc_caller_ctx *caller_ctx;
 };
 
-- 
2.43.0


^ permalink raw reply related

* [PATCH net-next v2 7/7] net: mana: add dynamic HWC queue depth with reinit path
From: Long Li @ 2026-07-21 23:43 UTC (permalink / raw)
  To: Long Li, Konstantin Taranov, Jakub Kicinski, David S . Miller,
	Paolo Abeni, Eric Dumazet, Andrew Lunn, Jason Gunthorpe,
	Leon Romanovsky, Haiyang Zhang, K . Y . Srinivasan, Wei Liu,
	Dexuan Cui, shradhagupta, Simon Horman, ernis, stephen
  Cc: netdev, linux-rdma, linux-hyperv, linux-kernel
In-Reply-To: <20260721234339.1476932-1-longli@microsoft.com>

The HWC is first established at a bootstrap queue depth of 1.  Query the
device's maximum supported depth and, if larger, tear down and rebuild
the HWC queues at that depth before re-establishing the channel, so more
management commands can be in flight.  Advertise
GDMA_DRV_CAP_FLAG_1_DYN_HWC_QUEUE_DEPTH so the firmware enables this only
when the driver supports it.

mana_hwc_destroy_queues() tears down the CQ first, which deregisters the
EQ IRQ (mana_gd_deregister_irq() + synchronize_rcu()) so no interrupt
handler can touch the queues, then the TXQ, RXQ and inflight resources.

Validate the device-reported dimensions before they size DMA
allocations: enforce the request/response header minimums, ensure
q_depth * max_msg_size plus alignment fits in u32, and cap CQ depth to
U16_MAX/2.  Carry the depth as u32 -- the device field is 24-bit, so
truncating to u16 on receipt could wrap a large value to a small depth
and silently pass these checks.

If reinit fails, fall back to the bootstrap-depth channel when its
teardown succeeded; if teardown also failed, hardware mappings may still
be active, so abort channel creation instead.

Signed-off-by: Long Li <longli@microsoft.com>
---
 .../net/ethernet/microsoft/mana/hw_channel.c  | 190 +++++++++++++++++-
 include/net/mana/gdma.h                       |   4 +
 include/net/mana/hw_channel.h                 |   2 +-
 3 files changed, 191 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/microsoft/mana/hw_channel.c b/drivers/net/ethernet/microsoft/mana/hw_channel.c
index 616fb088c559..0481d4044342 100644
--- a/drivers/net/ethernet/microsoft/mana/hw_channel.c
+++ b/drivers/net/ethernet/microsoft/mana/hw_channel.c
@@ -214,7 +214,12 @@ static void mana_hwc_init_event_handler(void *ctx, struct gdma_queue *q_self,
 			break;
 
 		case HWC_INIT_DATA_QUEUE_DEPTH:
-			hwc->hwc_init_q_depth_max = (u16)val;
+			/* HWC_INIT_DATA_QUEUE_DEPTH is a 24-bit field.  Keep
+			 * the full device-reported value here; it is clamped
+			 * and validated in mana_hwc_create_channel() rather
+			 * than silently truncated to u16.
+			 */
+			hwc->hwc_init_q_depth_max = val;
 			break;
 
 		case HWC_INIT_DATA_MAX_REQUEST:
@@ -808,7 +813,7 @@ static int mana_hwc_test_channel(struct hw_channel_context *hwc, u16 q_depth,
 	return err;
 }
 
-static int mana_hwc_establish_channel(struct gdma_context *gc, u16 *q_depth,
+static int mana_hwc_establish_channel(struct gdma_context *gc, u32 *q_depth,
 				      u32 *max_req_msg_size,
 				      u32 *max_resp_msg_size)
 {
@@ -886,6 +891,12 @@ static int mana_hwc_init_queues(struct hw_channel_context *hwc, u16 q_depth,
 {
 	int err;
 
+	/* CQ depth is q_depth * 2 (SQ + RQ) passed as u16 to create_cq.
+	 * Cap to prevent u16 truncation.
+	 */
+	if (q_depth > U16_MAX / 2)
+		q_depth = U16_MAX / 2;
+
 	err = mana_hwc_init_inflight_msg(hwc, q_depth);
 	if (err)
 		return err;
@@ -926,13 +937,62 @@ static int mana_hwc_init_queues(struct hw_channel_context *hwc, u16 q_depth,
 	return err;
 }
 
+/* Tear down all HWC queues and free associated resources.  Used on
+ * the reinit-with-higher-queue-depth path and reinit fallback.
+ *
+ * PRECONDITION: must be called only during channel bring-up in
+ * mana_hwc_create_channel(), before the channel is published to
+ * senders.  There the setup thread is effectively single-threaded
+ * (serialized against teardown by the PCI/PM device_lock or, on the
+ * service path, GC_IN_SERVICE, with the data path not yet probed),
+ * channel_up is still false, caller_ctx is not yet allocated, and
+ * active_senders is 0 — so no concurrent request/response user can
+ * touch these queues.  That is why this skips the hwc_lock-protected
+ * driver_data clear + active_senders drain that
+ * mana_hwc_destroy_channel() needs for the runtime teardown race;
+ * only the CQ-first ordering below (to fence off a pending interrupt)
+ * is required.  Calling this on a live, published channel would be a
+ * use-after-free.
+ */
+static void mana_hwc_destroy_queues(struct hw_channel_context *hwc)
+{
+	struct gdma_context *gc = hwc->gdma_dev->gdma_context;
+
+	/* Destroy CQ first to deregister the EQ from the interrupt
+	 * handler list before freeing caller_ctx, TXQ, or RXQ memory.
+	 * A pending interrupt handler could still reach handle_resp()
+	 * which dereferences caller_ctx.
+	 */
+	if (hwc->cq) {
+		mana_hwc_destroy_cq(gc, hwc->cq);
+		hwc->cq = NULL;
+	}
+
+	kfree(hwc->caller_ctx);
+	hwc->caller_ctx = NULL;
+
+	if (hwc->txq) {
+		mana_hwc_destroy_wq(hwc, hwc->txq);
+		hwc->txq = NULL;
+	}
+
+	if (hwc->rxq) {
+		mana_hwc_destroy_wq(hwc, hwc->rxq);
+		hwc->rxq = NULL;
+	}
+
+	mana_gd_free_res_map(&hwc->inflight_msg_res);
+	hwc->num_inflight_msg = 0;
+}
+
 int mana_hwc_create_channel(struct gdma_context *gc)
 {
 	u32 max_req_msg_size, max_resp_msg_size;
 	struct gdma_dev *gd = &gc->hwc;
 	struct hw_channel_context *hwc;
+	struct gdma_queue __rcu **old_cq_table;
 	unsigned long flags;
-	u16 q_depth_max;
+	u32 q_depth_max;
 	int err;
 
 	hwc = kzalloc_obj(*hwc);
@@ -980,8 +1040,130 @@ int mana_hwc_create_channel(struct gdma_context *gc)
 		goto out;
 	}
 
+	/* The channel was bootstrapped at a minimal queue depth.  If the
+	 * device reports a higher maximum, tear down and rebuild with
+	 * the larger depth so more HWC commands can be in flight.
+	 */
+	if (q_depth_max > HW_CHANNEL_VF_BOOTSTRAP_QUEUE_DEPTH) {
+		/* q_depth_max now carries the full device-reported value
+		 * (HWC_INIT_DATA_QUEUE_DEPTH is 24-bit).  Clamp it to the
+		 * depth the rest of the driver supports — create_cq() takes
+		 * q_depth * 2 as a u16 — before the overflow check below, so
+		 * an over-large but otherwise-valid depth is reduced to the
+		 * maximum instead of wrapping or being rejected.
+		 */
+		if (q_depth_max > U16_MAX / 2)
+			q_depth_max = U16_MAX / 2;
+
+		/* Sanity-check device-reported values before using them
+		 * to size DMA allocations.  Enforce protocol minimums
+		 * for message sizes and check that q_depth * max_msg_size
+		 * plus alignment headroom fits in u32 (for
+		 * mana_hwc_alloc_dma_buf's MANA_PAGE_ALIGN).
+		 */
+		if (!max_req_msg_size || !max_resp_msg_size ||
+		    max_req_msg_size < sizeof(struct gdma_req_hdr) ||
+		    max_resp_msg_size < sizeof(struct gdma_resp_hdr) ||
+		    (u64)q_depth_max * max_req_msg_size >
+			U32_MAX - MANA_PAGE_SIZE ||
+		    (u64)q_depth_max * max_resp_msg_size >
+			U32_MAX - MANA_PAGE_SIZE) {
+			dev_err(hwc->dev,
+				"HWC: invalid dims q=%u req=%u resp=%u\n",
+				q_depth_max, max_req_msg_size,
+				max_resp_msg_size);
+			q_depth_max = HW_CHANNEL_VF_BOOTSTRAP_QUEUE_DEPTH;
+			goto skip_reinit;
+		}
+
+		err = mana_smc_teardown_hwc(&gc->shm_channel, false);
+		if (err) {
+			/* Keep using the bootstrap-depth channel. */
+			dev_err(hwc->dev,
+				"Failed to teardown HWC for reinit: %d\n",
+				err);
+			q_depth_max = HW_CHANNEL_VF_BOOTSTRAP_QUEUE_DEPTH;
+			goto skip_reinit;
+		}
+
+		hwc->setup_active = false;
+
+		/* Destroy queues first — mana_gd_destroy_cq inside
+		 * unpublishes the CQ from cq_table via
+		 * rcu_assign_pointer(NULL) + synchronize_rcu.
+		 * Must happen while cq_table is still valid.
+		 */
+		mana_hwc_destroy_queues(hwc);
+
+		old_cq_table = rcu_replace_pointer(gc->cq_table, NULL, true);
+		synchronize_rcu();
+		vfree(old_cq_table);
+
+		err = mana_hwc_init_queues(hwc, q_depth_max,
+					   max_req_msg_size,
+					   max_resp_msg_size);
+		if (err) {
+			dev_err(hwc->dev, "Failed to reinit HWC: %d\n", err);
+			goto reinit_fallback;
+		}
+
+		err = mana_hwc_establish_channel(gc, &q_depth_max,
+						 &max_req_msg_size,
+						 &max_resp_msg_size);
+		if (err) {
+			dev_err(hwc->dev, "Failed to re-establish HWC: %d\n",
+				err);
+			/* establish_channel does internal teardown on
+			 * failure.  If teardown succeeded (setup_active
+			 * cleared), MST entries are invalidated and we
+			 * can try the bootstrap fallback.  If teardown
+			 * also failed (setup_active still set), hardware
+			 * mappings may still be active — skip fallback.
+			 */
+			if (hwc->setup_active)
+				goto out;
+			goto reinit_fallback;
+		}
+	}
+
+	goto skip_reinit;
+
+reinit_fallback:
+	/* Restore bootstrap-depth channel so the device remains functional.
+	 * Free cq_table if it was allocated by a partially successful
+	 * establish attempt.
+	 */
+	dev_warn(hwc->dev, "HWC reinit failed, falling back to bootstrap depth\n");
+
+	mana_hwc_destroy_queues(hwc);
+
+	old_cq_table = rcu_replace_pointer(gc->cq_table, NULL, true);
+	synchronize_rcu();
+	vfree(old_cq_table);
+
+	err = mana_hwc_init_queues(hwc, HW_CHANNEL_VF_BOOTSTRAP_QUEUE_DEPTH,
+				   HW_CHANNEL_MAX_REQUEST_SIZE,
+				   HW_CHANNEL_MAX_RESPONSE_SIZE);
+	if (err) {
+		dev_err(hwc->dev, "Failed to restore bootstrap HWC: %d\n", err);
+		goto out;
+	}
+
+	err = mana_hwc_establish_channel(gc, &q_depth_max, &max_req_msg_size,
+					 &max_resp_msg_size);
+	if (err) {
+		dev_err(hwc->dev, "Failed to re-establish bootstrap HWC: %d\n",
+			err);
+		goto out;
+	}
+
+skip_reinit:
+
+	/* No RCU needed: still in mana_hwc_create_channel, the
+	 * pointer has not been published to concurrent senders yet.
+	 */
 	err = mana_hwc_test_channel(gc->hwc.driver_data,
-				    HW_CHANNEL_VF_BOOTSTRAP_QUEUE_DEPTH,
+				    hwc->num_inflight_msg,
 				    max_req_msg_size, max_resp_msg_size);
 	if (err) {
 		dev_err(hwc->dev, "Failed to test HWC: %d\n", err);
diff --git a/include/net/mana/gdma.h b/include/net/mana/gdma.h
index 01e845237b6a..a4eac6f7c366 100644
--- a/include/net/mana/gdma.h
+++ b/include/net/mana/gdma.h
@@ -708,6 +708,9 @@ enum {
 /* Driver supports dynamic interrupt moderation - DIM */
 #define GDMA_DRV_CAP_FLAG_1_DYN_INTERRUPT_MODERATION BIT(28)
 
+/* Driver supports dynamic queue depth for HWC */
+#define GDMA_DRV_CAP_FLAG_1_DYN_HWC_QUEUE_DEPTH BIT(29)
+
 #define GDMA_DRV_CAP_FLAGS1 \
 	(GDMA_DRV_CAP_FLAG_1_EQ_SHARING_MULTI_VPORT | \
 	 GDMA_DRV_CAP_FLAG_1_NAPI_WKDONE_FIX | \
@@ -723,6 +726,7 @@ enum {
 	 GDMA_DRV_CAP_FLAG_1_PROBE_RECOVERY | \
 	 GDMA_DRV_CAP_FLAG_1_HANDLE_STALL_SQ_RECOVERY | \
 	 GDMA_DRV_CAP_FLAG_1_HWC_TIMEOUT_RECOVERY | \
+	 GDMA_DRV_CAP_FLAG_1_DYN_HWC_QUEUE_DEPTH | \
 	 GDMA_DRV_CAP_FLAG_1_EQ_MSI_UNSHARE_MULTI_VPORT | \
 	 GDMA_DRV_CAP_FLAG_1_DYN_INTERRUPT_MODERATION)
 
diff --git a/include/net/mana/hw_channel.h b/include/net/mana/hw_channel.h
index 30bc236286dc..eec5d3aa23e1 100644
--- a/include/net/mana/hw_channel.h
+++ b/include/net/mana/hw_channel.h
@@ -197,7 +197,7 @@ struct hw_channel_context {
 	u32 max_req_msg_size;
 	u32 max_resp_msg_size;
 
-	u16 hwc_init_q_depth_max;
+	u32 hwc_init_q_depth_max;
 	u32 hwc_init_max_req_msg_size;
 	u32 hwc_init_max_resp_msg_size;
 
-- 
2.43.0


^ permalink raw reply related

* RE: [PATCH rdma-next] RDMA/mana_ib: drain QP references after partial table insertion
From: Konstantin Taranov @ 2026-07-22  9:18 UTC (permalink / raw)
  To: Leon Romanovsky, Long Li, Jason Gunthorpe, Shiraz Saleem
  Cc: linux-rdma@vger.kernel.org, linux-hyperv@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <20260721-if-mana-table-store-qp-qids-partiall-v1-1-8fb3d2d2b559@nvidia.com>

> From: Leon Romanovsky <leonro@nvidia.com>
> 
> mana_table_store_ud_qp() publishes a QP at its send-queue id before
> inserting the receive-queue id, dropping the XArray lock between the two
> xa_insert_irq() calls. A concurrent completion handler can look up the QP and
> take a transient reference. When the second insertion fails, the rollback
> erased only the send-queue entry and returned, leaving both the initial table
> reference and the transient reference outstanding while RDMA core frees the
> QP, causing a use-after-free.
> 
> Drain the reference as normal destruction does: drop the initial reference and
> wait for qp->free, releasing the QP only after every concurrent lookup returns
> its reference.
> 
> Fixes: 8001e9257eca ("RDMA/mana_ib: extend mana QP table")
> Signed-off-by: Leon Romanovsky <leonro@nvidia.com>

Reviewed-by: Konstantin Taranov <kotaranov@microsoft.com> 

> ---
> Konstantin,
> 
> I saw this in one of the Sashiko runs, and I believe the AI is correct. The
> reference can be acquired through the GDMA_EQE_RNIC_QP_FATAL event.
> 

I doubt that it is possible for EQEs. The eqe only can mention an RQ and we first try to insert an SQ.
As well as when we drain GSI QP, we also look up with an RQ.
I am also trying to think whether CQE with SQ QID can cause it, which would reveal a HW bug/race.
But I do agree that the patch makes sense and we should have it, regardless of whether it is possible.
Thanks a lot for the patch. I have sent recently a UC-related patch that changes the same lines.
I will rebase it on top of your change.

> Thanks
> ---
>  drivers/infiniband/hw/mana/qp.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/mana/qp.c
> b/drivers/infiniband/hw/mana/qp.c index b5ff07e34eb7..c52f3ec14032
> 100644
> --- a/drivers/infiniband/hw/mana/qp.c
> +++ b/drivers/infiniband/hw/mana/qp.c
> @@ -472,6 +472,12 @@ static void mana_table_remove_rc_qp(struct
> mana_ib_dev *mdev, struct mana_ib_qp
>  	xa_erase_irq(&mdev->qp_table_wq, qp->ibqp.qp_num);  }
> 
> +static void mana_table_drain_qp_ref(struct mana_ib_qp *qp) {
> +	mana_put_qp_ref(qp);
> +	wait_for_completion(&qp->free);
> +}
> +
>  static int mana_table_store_ud_qp(struct mana_ib_dev *mdev, struct
> mana_ib_qp *qp)  {
>  	u32 qids = qp->ud_qp.queues[MANA_UD_SEND_QUEUE].id |
> MANA_SENDQ_MASK; @@ -490,6 +496,7 @@ static int
> mana_table_store_ud_qp(struct mana_ib_dev *mdev, struct mana_ib_qp *q
> 
>  remove_sq:
>  	xa_erase_irq(&mdev->qp_table_wq, qids);
> +	mana_table_drain_qp_ref(qp);
>  	return err;
>  }
> 
> @@ -537,8 +544,7 @@ static void mana_table_remove_qp(struct
> mana_ib_dev *mdev,
>  			  qp->ibqp.qp_type);
>  		return;
>  	}
> -	mana_put_qp_ref(qp);
> -	wait_for_completion(&qp->free);
> +	mana_table_drain_qp_ref(qp);
>  }
> 
>  static int mana_ib_create_rc_qp(struct ib_qp *ibqp, struct ib_pd *ibpd,
> 
> ---
> base-commit: 74f49255492a62658f36bf2578d7916f1c6ffad1
> change-id: 20260721-if-mana-table-store-qp-qids-partiall-afa14f023261
> 
> Best regards,
> --
> Leon Romanovsky <leonro@nvidia.com>


^ permalink raw reply

* Re: [RFC PATCH] x86/apic: Fix lost IRQ during forced vector migration on Hyper-V
From: Naman Jain @ 2026-07-22  9:29 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H . Peter Anvin
  Cc: Neeraj Upadhyay, linux-kernel, linux-hyperv, Michael Kelley
In-Reply-To: <874ihskq1h.ffs@fw13>



On 7/22/2026 2:40 AM, Thomas Gleixner wrote:
> On Fri, Jul 10 2026 at 05:40, Naman Jain wrote:
>>
>> Fixes: e84cf6aa501c5 ("x86/apic/vector: Handle vector release on CPU unplug correctly")
> 
> Errm. This does not fix anything in the common code. It fixes a hyperv
> PCI/MSI problem.
> >>   	} else {
>> +		/*
>> +		 * The outgoing CPU cannot use the deferred cleanup-vector
>> +		 * mechanism, so its vector is freed inline below. On Hyper-V the
>> +		 * MSI retarget hypercall is asynchronous, so an interrupt raised
>> +		 * inside the stop_machine window can be posted to the outgoing
>> +		 * CPU's old vIRR after the free. Two complementary steps handle
>> +		 * that (see also the retrigger at the end of the function):
>> +		 *
>> +		 *  - Retrigger on the new target so a raced completion is drained
>> +		 *    there rather than lost. The retarget is asynchronous, so the
>> +		 *    outgoing IRR is not authoritative and the IPI is issued
>> +		 *    unconditionally; a spurious ISR is harmless to
>> +		 *    completion-draining handlers (they find an empty queue).
>> +		 *  - Mark the freed slot VECTOR_RETRIGGERED so a late stray is
>> +		 *    absorbed by reevaluate_vector() instead of logging "No irq
>> +		 *    handler" while the CPU still takes interrupts during
>> +		 *    teardown; __setup_vector_irq() resets it on re-online.
>> +		 *
>> +		 * This mirrors msi_set_affinity()'s protection, which the Hyper-V
>> +		 * MSI chip bypasses via IRQCHIP_MOVE_DEFERRED.
> 
> What has msi_set_affinity() and IRQCHIP_MOVE_DEFERRED to do with this?
> 
>    - msi_set_affinity() solves a completely different problem. It handles
>      the case when the PCI/MSI device does not provide masking of the MSI
>      interrupt across the message change.
> 
>      Q: What exactly is mirrored here?
>      A: Nothing
> 
>    - In the context of CPU hot-unplug IRQCHIP_MOVE_DEFERRED is not
>      deferring anything simply because there is no way to do the deferred
>      (in interrupt context) move on the original target CPU.
> 
>      fixup_irqs()
>         irq_migrate_all_off_this_cpu()
>            migrate_one_irq()
>              irq_do_set_affinity()
> 
>      The only check there which is related to IRQCHIP_MOVE_DEFERRED is
>      the masking decision in migrate_one_irq(), but that has nothing to
>      do with the semantics of IRQCHIP_MOVE_DEFERRED.
> 
>      That flag tells the interrupt core that regular affinity changes
>      have to be delayed into the context of the next device interrupt,
>      which is obviously not possible for interrupt startup and forced
>      migration on CPU hot-unplug, which both ignore that restriction. The
>      only effect in CPU hot-unplug migrations is that the interrupt is
>      masked at the outermost domain level across the affinity change
>      operation because that's an requirement for msi_set_affinity() and
>      similar workarounds (loongarch) to avoid the multi vector dance.
> 
>      The fact that hyper-V invokes the retarget hypercall in context of
>      the unmask() callback has nothing to with that flag and due to the
>      lack of explanation why the flag is needed in the first place.
> 
> I'm all for commenting complex issues, but comments have to be
> technically correct, which excludes both LLM and human hallucinations.
> The same applies to the change log which carries the same fairy tale.
> 
> Aside of that, doing the re-targeting in the unmask() callback
> unconditionally is pointless overhead. Why?
> 
>      disable_irq($IRQ);             // Sets the disabled flag, but does not mask
> 
>      --> $IRQ is raised
>      handle_edge_irq($IRQ)
>      if (disabled($IRQ))
>        mask_irq($IRQ);              // Masks at the PCI/MSI level
> 
>      enable_irq($IRQ)
>      if (masked($IRQ))
>        unmask_irq($IRQ)
>          issue_hypercall(RETARGET)  // Pointless exercise when affinity
>                                     // has not changed
>          unmask_pci_msi();          // Unmasks at the PCI/MSI level
> 
> Not that I care at all, but I have no idea why pci-hyperv has to do the
> affinity change in the unmask() callback and why it needs to take the
> overhead which comes with the deferred affinity changes in general.
> 
>> +		 * restricted to edge MSI vectors on Hyper-V (msi_desc present):
>> +		 * only the MSI retarget hypercall is asynchronous, so edge IOAPIC
>> +		 * lines (retargeted synchronously via the RTE) and level-triggered
>> +		 * lines never see this race and must not be force-injected.
>> +		 */
>> +		if (hypervisor_is_type(X86_HYPER_MS_HYPERV) &&
>> +		    apicd->vector >= FIRST_EXTERNAL_VECTOR &&
>> +		    !irqd_is_level_type(irqd) && irq_data_get_msi_desc(irqd))
> 
> TBH. This is a tasteless layering violation. We are not cluttering
> common code with hypervisor specific MSI workarounds. The non-remapped
> MSI workaround is not implemented here either.
> 
>> +			hv_retrigger = true;
>>   		apic_free_vector(apicd->cpu, apicd->vector, managed);
>> +		/*
>> +		 * apic_free_vector() releases the matrix bit but leaves the
>> +		 * outgoing CPU's vector_irq[] slot pointing at the stale desc, so
>> +		 * the marker is written unconditionally here.
> 
> What's exactly unconditional about
> 
>         if (cond)
>            write()
> 
> ?
> 
>> +              						(Unlike
>> +		 * msi_set_affinity(), which marks a genuinely unused slot and
>> +		 * therefore guards with IS_ERR_OR_NULL, the slot here still holds
>> +		 * this irq's old desc.)
> 
> What has msi_set_affinity() to do with this? It sets RETRIGGER for a
> very different reason. These references are confusing at best. Simply
> explain WHY you need to do this and explain it correctly.
> 
>> +		 */
>> +		if (hv_retrigger)
>> +			per_cpu(vector_irq, apicd->cpu)[apicd->vector] = VECTOR_RETRIGGERED;
>>   	}
>>   
>>   setnew:
>> @@ -190,6 +231,13 @@ static void chip_data_update(struct irq_data *irqd, unsigned int newvec, unsigne
>>   	BUG_ON(!IS_ERR_OR_NULL(per_cpu(vector_irq, newcpu)[newvec]));
>>   	per_cpu(vector_irq, newcpu)[newvec] = desc;
>>   	apic_update_irq_cfg(irqd, newvec, newcpu);
>> +	/*
>> +	 * Drain any completion that raced onto the freed vIRR by retriggering
> 
> What's a freed vIRR and which completion? This code deals with vector
> management and not with hyperV MSI implementation specific completions
> and it does not care whether it operates on a physical or a virtual
> APIC.
> 
>> +	 * on the new target (see the else-branch above). Issued after the new
> 
> 'See the else-branch above' is as useful as 'See some random explanation
> elsewhere'. It's obvious where to look for that information as the
> hv_retrigger condition must be set by something to make the condition true.
> 
>> +	 * mapping is installed so the handler is present when it is serviced.
>> +	 */
>> +	if (hv_retrigger)
>> +		__apic_send_IPI(newcpu, newvec);
> 
> You are claiming above that the MSI retarget hypercall is asynchronous,
> but you fail to specify in which way.
> 
>    - If you refer to the retarget invocation in the unmask() callback,
>      then that happens still before the new target can handle it.
> 
>    - If it is truly asynchronous, i.e. the hypervisor handles it at some
>      arbitrary point in time, then the retriggered vector on the new
>      target CPU can be handled _before_ the retarget is in effect. In
>      that case a subsequent interrupt might still end up on the dead CPU,
>      no?
> 
> IOW, your or your LLM's elaborate comment is useless word salad as it
> does not explain the problem in a coherent and comprehensible way. Quite
> to the contrary it is more confusing than helpful.
> 
> But as I told you already, this is not a problem of incoherent comments,
> this is simply the wrong place to address that.
> 
> Looking at the pci-hyperv implementation made me immediately notice a
> bug, which exists since the driver was merged:
> 
>    The driver does not set the irq_retrigger() callback for the
>    interrupt chip.
> 
> That means that fixup_irqs() can't do anything even when it observes the
> IRR bit set. And that matches the incoherent problem description as far
> as I can tell from trying to oracle something out of the word salad.
> 
> Great that your AI ass-istant did not figure that out and you blindly
> trusted the nonsense it hallucinated.
> 
> The uncompiled and therefore untested below should exactly do what you
> want to achieve without having hyperV specific hackery in the common
> code.
> 
> If it does not, then you need to provide a coherent technical
> explanation why it is not sufficient.
> 
> Thanks,
> 
>          tglx
> 
> ---


Thanks a lot for reviewing the code. I was actually debugging hangs and 
timeouts when we test stress-FIO with hotplug. Occasionally I saw "No 
irq handler" logs in those hangs, and started debugging in that 
direction. I had tried adding the hyper-V irq_retrigger() callback, but 
the hang still reproduced. This kept me under the impression that this 
change did not fix the issue and led me to over-engineer this fix in the 
wrong place. Recently, I was able to root cause the hang to some other 
reason (using different NVME scheduler).

I completely agree with your comments, and now that I know how to fix 
this, do you suggest me to -

1. add irq_retrigger callback in pci-hyperv.c
or
2. go with your fixup-irq() change to no longer need drivers to add the 
above? And then see what all stale irq_retrigger inits can be removed

Going with 1) would be easier for me to fix this bug, and back port it 
to older kernels, with proper Fixes tag (pointing to some pci-hyperv.c 
change).
Then later, we can go with 2) separately as a general enhancement (not a 
Fix).

Regards,
Naman

> Subject: TBD
> 
>    Instead of adding the missing callback to the hyperv driver, which
>    would be the trivial "fix", it changes the x86 fixup_irqs()
>    implementation. Why?
> 
>    That makes the code more resilient and allows to remove quite a bit of
>    initializations of interrupt chips all over the place.
> 
> Not-Signed-off-yet-by: Thomas Gleixner <tglx@kernel.org>
> Assisted-by: Human Intelligence
> ---
>   arch/x86/kernel/irq.c |   23 +++++++----------------
>   1 file changed, 7 insertions(+), 16 deletions(-)
> 
> --- a/arch/x86/kernel/irq.c
> +++ b/arch/x86/kernel/irq.c
> @@ -466,11 +466,6 @@ DEFINE_IDTENTRY_SYSVEC(sysvec_posted_msi
>   /* A cpu has been removed from cpu_online_mask.  Reset irq affinities. */
>   void fixup_irqs(void)
>   {
> -	unsigned int vector;
> -	struct irq_desc *desc;
> -	struct irq_data *data;
> -	struct irq_chip *chip;
> -
>   	irq_migrate_all_off_this_cpu();
>   
>   	/*
> @@ -489,22 +484,18 @@ void fixup_irqs(void)
>   	 * vector_lock because the cpu is already marked !online, so
>   	 * nothing else will touch it.
>   	 */
> -	for (vector = FIRST_EXTERNAL_VECTOR; vector < NR_VECTORS; vector++) {
> -		if (IS_ERR_OR_NULL(__this_cpu_read(vector_irq[vector])))
> +	for (unsigned int vector = FIRST_EXTERNAL_VECTOR; vector < NR_VECTORS; vector++) {
> +		struct irq_desc *desc = __this_cpu_read(vector_irq[vector]);
> +
> +		if (IS_ERR_OR_NULL(desc))
>   			continue;
>   
>   		if (is_vector_pending(vector)) {
> -			desc = __this_cpu_read(vector_irq[vector]);
> -
> -			raw_spin_lock(&desc->lock);
> -			data = irq_desc_get_irq_data(desc);
> -			chip = irq_data_get_irq_chip(data);
> -			if (chip->irq_retrigger) {
> -				chip->irq_retrigger(data);
> +			guard(raw_spinlock)(&desc->lock);
> +			if (irq_chip_retrigger_hierarchy(&desc->irq_data))
>   				__this_cpu_write(vector_irq[vector], VECTOR_RETRIGGERED);
> -			}
> -			raw_spin_unlock(&desc->lock);
>   		}
> +
>   		if (__this_cpu_read(vector_irq[vector]) != VECTOR_RETRIGGERED)
>   			__this_cpu_write(vector_irq[vector], VECTOR_UNUSED);
>   	}


^ permalink raw reply

* Re: [PATCH rdma-next] RDMA/mana_ib: drain QP references after partial table insertion
From: Leon Romanovsky @ 2026-07-22  9:34 UTC (permalink / raw)
  To: Konstantin Taranov
  Cc: Long Li, Jason Gunthorpe, Shiraz Saleem,
	linux-rdma@vger.kernel.org, linux-hyperv@vger.kernel.org,
	linux-kernel@vger.kernel.org
In-Reply-To: <DU8PR83MB0975C74A6A47AB8EAE06AB29B4C12@DU8PR83MB0975.EURPRD83.prod.outlook.com>

On Wed, Jul 22, 2026 at 09:18:12AM +0000, Konstantin Taranov wrote:
> > From: Leon Romanovsky <leonro@nvidia.com>
> > 
> > mana_table_store_ud_qp() publishes a QP at its send-queue id before
> > inserting the receive-queue id, dropping the XArray lock between the two
> > xa_insert_irq() calls. A concurrent completion handler can look up the QP and
> > take a transient reference. When the second insertion fails, the rollback
> > erased only the send-queue entry and returned, leaving both the initial table
> > reference and the transient reference outstanding while RDMA core frees the
> > QP, causing a use-after-free.
> > 
> > Drain the reference as normal destruction does: drop the initial reference and
> > wait for qp->free, releasing the QP only after every concurrent lookup returns
> > its reference.
> > 
> > Fixes: 8001e9257eca ("RDMA/mana_ib: extend mana QP table")
> > Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> 
> Reviewed-by: Konstantin Taranov <kotaranov@microsoft.com> 
> 
> > ---
> > Konstantin,
> > 
> > I saw this in one of the Sashiko runs, and I believe the AI is correct. The
> > reference can be acquired through the GDMA_EQE_RNIC_QP_FATAL event.
> > 
> 
> I doubt that it is possible for EQEs. The eqe only can mention an RQ and we first try to insert an SQ.
> As well as when we drain GSI QP, we also look up with an RQ.

GDMA_EQE_RNIC_QP_FATAL is my interpretation, and not AI :).

Thanks

^ permalink raw reply

* Re: [PATCH rdma-next] RDMA/mana_ib: drain QP references after partial table insertion
From: Leon Romanovsky @ 2026-07-22  9:35 UTC (permalink / raw)
  To: Long Li, Konstantin Taranov, Jason Gunthorpe, Shiraz Saleem,
	Leon Romanovsky
  Cc: linux-rdma, linux-hyperv, linux-kernel
In-Reply-To: <20260721-if-mana-table-store-qp-qids-partiall-v1-1-8fb3d2d2b559@nvidia.com>


On Tue, 21 Jul 2026 18:00:01 +0300, Leon Romanovsky wrote:
> mana_table_store_ud_qp() publishes a QP at its send-queue id before
> inserting the receive-queue id, dropping the XArray lock between the two
> xa_insert_irq() calls. A concurrent completion handler can look up the QP
> and take a transient reference. When the second insertion fails, the
> rollback erased only the send-queue entry and returned, leaving both the
> initial table reference and the transient reference outstanding while RDMA
> core frees the QP, causing a use-after-free.
> 
> [...]

Applied, thanks!

[1/1] RDMA/mana_ib: drain QP references after partial table insertion
      https://git.kernel.org/rdma/rdma/c/97f7c2262c28eb

Best regards,
-- 
Leon Romanovsky <leon@kernel.org>


^ permalink raw reply

* Re: [RFC PATCH] x86/apic: Fix lost IRQ during forced vector migration on Hyper-V
From: Thomas Gleixner @ 2026-07-22 13:15 UTC (permalink / raw)
  To: Naman Jain, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H . Peter Anvin
  Cc: Neeraj Upadhyay, linux-kernel, linux-hyperv, Michael Kelley
In-Reply-To: <12557e3a-5feb-4601-ad53-cf4846052edd@linux.microsoft.com>

Naman!

On Wed, Jul 22 2026 at 14:59, Naman Jain wrote:
> I completely agree with your comments, and now that I know how to fix 
> this, do you suggest me to -
>
> 1. add irq_retrigger callback in pci-hyperv.c
> or
> 2. go with your fixup-irq() change to no longer need drivers to add the 
> above? And then see what all stale irq_retrigger inits can be removed
>
> Going with 1) would be easier for me to fix this bug, and back port it 
> to older kernels, with proper Fixes tag (pointing to some pci-hyperv.c 
> change).
> Then later, we can go with 2) separately as a general enhancement (not a 
> Fix).

This should be one patch series:

  1) Add the callback to pci-hyperv, i.e. in hv_pcie_init_dev_msi_info()

     +    chip->irq_retrigger = irq_chip_retrigger_hierarchy;

     That sets it on the outermost domain interrupt chip (PCI/MSI
     device domain), which is what fixup_irqs() looks at.

     This one needs a Fixes: tag (The first commit which introduced the
     driver) and a Cc: stable@... so it will be backported.

  2) Change fixup_irqs() to use irq_chip_retrigger_hierarchy()

     Feel free to pick up the patch I gave you, add a proper change log
     and a Suggested-by: Thomas .... tag.

  3) Remove the initialization of the outermost chip from arch/x86/* and
     drivers/iommu/amd/* as that's not longer required then.

Thanks,

        tglx



^ permalink raw reply

* Re: [PATCH v2] Drivers: hv: vmbus: add VTL2 redirect connection ID
From: Tianyu Lan @ 2026-07-22 13:22 UTC (permalink / raw)
  To: Hardik Garg
  Cc: K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui,
	Saurabh Sengar, Michael Kelley, linux-hyperv, linux-kernel
In-Reply-To: <20260717001837.635756-1-hargar@linux.microsoft.com>

On Fri, Jul 17, 2026 at 8:18 AM Hardik Garg <hargar@linux.microsoft.com> wrote:
>
> VMBus sends CHANNELMSG_INITIATE_CONTACT through a Hyper-V message
> connection ID. Older protocol versions use VMBUS_MESSAGE_CONNECTION_ID,
> while protocol version 5.0 and newer normally use
> VMBUS_MESSAGE_CONNECTION_ID_4.
>
> For a VTL2 kernel using VMBus protocol 5.0 or newer, the host
> may expect INITIATE_CONTACT on either the redirect connection ID or
> VMBUS_MESSAGE_CONNECTION_ID_4. There is no capability indication that
> identifies which ID is active, so the driver must determine it at runtime.
>
> During VMBus negotiation, the redirect ID is tried first because it is
> used by VTL2 configurations with VMBus redirection enabled. If the
> redirect ID is unavailable, the host rejects it synchronously with
> HV_STATUS_INVALID_CONNECTION_ID, allowing fallback to the standard ID.
>
> Return a distinct error for an invalid Initiate Contact connection ID so
> this fallback does not mask other post-message failures or
> protocol-version rejections. Preserve the existing connection ID
> selection for older protocol versions or when running below VTL2.
>
> Signed-off-by: Hardik Garg <hargar@linux.microsoft.com>

Reviewed-by: Tianyu Lan <Tianyu.Lan@microsoft.com>




-- 
Thanks
Tianyu Lan

^ permalink raw reply

* Re: [RFC PATCH] x86/apic: Fix lost IRQ during forced vector migration on Hyper-V
From: Naman Jain @ 2026-07-22 15:14 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
	H . Peter Anvin
  Cc: Neeraj Upadhyay, linux-kernel, linux-hyperv, Michael Kelley
In-Reply-To: <87y0f3jhd6.ffs@fw13>



On 7/22/2026 6:45 PM, Thomas Gleixner wrote:
> Naman!
> 
> On Wed, Jul 22 2026 at 14:59, Naman Jain wrote:
>> I completely agree with your comments, and now that I know how to fix
>> this, do you suggest me to -
>>
>> 1. add irq_retrigger callback in pci-hyperv.c
>> or
>> 2. go with your fixup-irq() change to no longer need drivers to add the
>> above? And then see what all stale irq_retrigger inits can be removed
>>
>> Going with 1) would be easier for me to fix this bug, and back port it
>> to older kernels, with proper Fixes tag (pointing to some pci-hyperv.c
>> change).
>> Then later, we can go with 2) separately as a general enhancement (not a
>> Fix).
> 
> This should be one patch series:
> 
>    1) Add the callback to pci-hyperv, i.e. in hv_pcie_init_dev_msi_info()
> 
>       +    chip->irq_retrigger = irq_chip_retrigger_hierarchy;
> 
>       That sets it on the outermost domain interrupt chip (PCI/MSI
>       device domain), which is what fixup_irqs() looks at.
> 
>       This one needs a Fixes: tag (The first commit which introduced the
>       driver) and a Cc: stable@... so it will be backported.
> 
>    2) Change fixup_irqs() to use irq_chip_retrigger_hierarchy()
> 
>       Feel free to pick up the patch I gave you, add a proper change log
>       and a Suggested-by: Thomas .... tag.
> 
>    3) Remove the initialization of the outermost chip from arch/x86/* and
>       drivers/iommu/amd/* as that's not longer required then.
> 
> Thanks,
> 
>          tglx
> 

Understood. Thanks.

Regards,
Naman


^ permalink raw reply

* Re: [PATCH v9 8/8] drm/gpusvm: Use hmm_range_fault_unlocked_timeout() for range faults
From: Stanislav Kinsburskii @ 2026-07-22 18:52 UTC (permalink / raw)
  To: Matthew Brost
  Cc: airlied, akhilesh, akpm, corbet, dakr, david, decui, haiyangz,
	jgg, kees, kys, leon, liam, lizhi.hou, ljs, longli, lyude,
	maarten.lankhorst, mamin506, mhocko, mripard, nouveau, ogabbay,
	oleg, rppt, shuah, simona, skhan, surenb, tzimmermann, vbabka,
	wei.liu, dri-devel, intel-xe, linux-mm, linux-doc, linux-hyperv,
	linux-kernel, linux-kselftest, linux-rdma
In-Reply-To: <al7CGPdJGM+asOFK@gsse-cloud1.jf.intel.com>

On Mon, Jul 20, 2026 at 05:49:28PM -0700, Matthew Brost wrote:
> On Wed, Jul 15, 2026 at 11:16:52AM -0700, Stanislav Kinsburskii wrote:
> > Several GPU SVM paths take mmap_read_lock() only to call hmm_range_fault()
> > and open-code mmu interval sequence setup before each HMM walk. They also
> > retry -EBUSY until HMM_RANGE_DEFAULT_TIMEOUT expires.
> > 
> > Use hmm_range_fault_unlocked_timeout() for those faults. The HMM helper now
> > owns mmap_lock acquisition and refreshes range->notifier_seq for its
> > internal retries, while GPU SVM keeps its existing driver-lock validation
> > with mmu_interval_read_retry() after a successful fault.
> > 
> > Pass HMM_RANGE_DEFAULT_TIMEOUT as the helper retry budget for each HMM
> > fault attempt. This scopes the timeout to repeated HMM notifier retries
> > while preserving the outer retry loops that restart when the interval is
> > invalidated before GPU SVM updates or consumes the mapping state.
> > 
> 
> This part doesn't seem right for get_pages(), see below.
> 
> > Leave drm_gpusvm_check_pages() on hmm_range_fault() because that path is
> > called with the mmap lock already held by its caller.
> > 
> > Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> > Signed-off-by: Stanislav Kinsburskii <skinsburskii@gmail.com>
> > ---
> >  drivers/gpu/drm/drm_gpusvm.c |   61 +++++-------------------------------------
> >  1 file changed, 7 insertions(+), 54 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_gpusvm.c b/drivers/gpu/drm/drm_gpusvm.c
> > index 958cb605aedd..de5bbfe58ee9 100644
> > --- a/drivers/gpu/drm/drm_gpusvm.c
> > +++ b/drivers/gpu/drm/drm_gpusvm.c
> > @@ -773,8 +773,7 @@ enum drm_gpusvm_scan_result drm_gpusvm_scan_mm(struct drm_gpusvm_range *range,
> >  		.end = end,
> >  		.dev_private_owner = dev_private_owner,
> >  	};
> > -	unsigned long timeout =
> > -		jiffies + msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
> > +	unsigned long timeout = msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
> >  	enum drm_gpusvm_scan_result state = DRM_GPUSVM_SCAN_UNPOPULATED, new_state;
> >  	unsigned long *pfns;
> >  	unsigned long npages = npages_in_range(start, end);
> > @@ -788,22 +787,7 @@ enum drm_gpusvm_scan_result drm_gpusvm_scan_mm(struct drm_gpusvm_range *range,
> >  	hmm_range.hmm_pfns = pfns;
> >  
> >  retry:
> > -	hmm_range.notifier_seq = mmu_interval_read_begin(notifier);
> > -	mmap_read_lock(range->gpusvm->mm);
> > -
> > -	while (true) {
> > -		err = hmm_range_fault(&hmm_range);
> > -		if (err == -EBUSY) {
> > -			if (time_after(jiffies, timeout))
> > -				break;
> > -
> > -			hmm_range.notifier_seq =
> > -				mmu_interval_read_begin(notifier);
> > -			continue;
> > -		}
> > -		break;
> > -	}
> > -	mmap_read_unlock(range->gpusvm->mm);
> > +	err = hmm_range_fault_unlocked_timeout(&hmm_range, timeout);
> >  	if (err)
> >  		goto err_free;
> >  
> > @@ -1406,8 +1390,7 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
> >  		.dev_private_owner = ctx->device_private_page_owner,
> >  	};
> >  	void *zdd;
> > -	unsigned long timeout =
> > -		jiffies + msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
> > +	unsigned long timeout = msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
> >  	unsigned long i, j;
> >  	unsigned long npages = npages_in_range(pages_start, pages_end);
> >  	unsigned long num_dma_mapped;
> > @@ -1422,9 +1405,6 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
> >  	struct dma_iova_state *state = &svm_pages->state;
> >  
> >  retry:
> > -	if (time_after(jiffies, timeout))
> > -		return -EBUSY;
> > -
> 
> I think that by deleting the code above, you have changed this function's
> semantics by removing the hard cap of HMM_RANGE_DEFAULT_TIMEOUT. This
> code was added because, on some non-production platforms, the timing in
> this function could cause it to livelock.
> 
> Is there any reason this was remove aside from timeout variable not
> being a deadline now? You likely should add the deadline back in.
> 

Indeed, this one can be called from a kernel thread context as well.
I'll revert the change in the next revision.

Thanks,
Stanislav


> Matt
> 
> >  	hmm_range.notifier_seq = mmu_interval_read_begin(notifier);
> >  	if (drm_gpusvm_pages_valid_unlocked(gpusvm, svm_pages))
> >  		goto set_seqno;
> > @@ -1439,21 +1419,7 @@ int drm_gpusvm_get_pages(struct drm_gpusvm *gpusvm,
> >  	}
> >  
> >  	hmm_range.hmm_pfns = pfns;
> > -	while (true) {
> > -		mmap_read_lock(mm);
> > -		err = hmm_range_fault(&hmm_range);
> > -		mmap_read_unlock(mm);
> > -
> > -		if (err == -EBUSY) {
> > -			if (time_after(jiffies, timeout))
> > -				break;
> > -
> > -			hmm_range.notifier_seq =
> > -				mmu_interval_read_begin(notifier);
> > -			continue;
> > -		}
> > -		break;
> > -	}
> > +	err = hmm_range_fault_unlocked_timeout(&hmm_range, timeout);
> >  	mmput(mm);
> >  	if (err)
> >  		goto err_free;
> > @@ -1720,8 +1686,7 @@ int drm_gpusvm_range_evict(struct drm_gpusvm *gpusvm,
> >  		.end = drm_gpusvm_range_end(range),
> >  		.dev_private_owner = NULL,
> >  	};
> > -	unsigned long timeout =
> > -		jiffies + msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
> > +	unsigned long timeout = msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
> >  	unsigned long *pfns;
> >  	unsigned long npages = npages_in_range(drm_gpusvm_range_start(range),
> >  					       drm_gpusvm_range_end(range));
> > @@ -1736,24 +1701,12 @@ int drm_gpusvm_range_evict(struct drm_gpusvm *gpusvm,
> >  		return -ENOMEM;
> >  
> >  	hmm_range.hmm_pfns = pfns;
> > -	while (!time_after(jiffies, timeout)) {
> > -		hmm_range.notifier_seq = mmu_interval_read_begin(notifier);
> > -		if (time_after(jiffies, timeout)) {
> > -			err = -ETIME;
> > -			break;
> > -		}
> > -
> > -		mmap_read_lock(mm);
> > -		err = hmm_range_fault(&hmm_range);
> > -		mmap_read_unlock(mm);
> > -		if (err != -EBUSY)
> > -			break;
> > -	}
> > +	err = hmm_range_fault_unlocked_timeout(&hmm_range, timeout);
> >  
> >  	kvfree(pfns);
> >  	mmput(mm);
> >  
> > -	return err;
> > +	return err == -EBUSY ? -ETIME : err;
> >  }
> >  EXPORT_SYMBOL_GPL(drm_gpusvm_range_evict);
> >  
> > 
> > 

^ permalink raw reply

* Re: [RFC PATCH 1/2] Drivers: hv: vmbus: add vmbus_establish_gpadl_caller_decrypted()
From: sashiko-bot @ 2026-07-22 19:57 UTC (permalink / raw)
  To: Kameron Carr; +Cc: linux-hyperv
In-Reply-To: <20260721195633.1438361-2-kameroncarr@linux.microsoft.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but vmbus_teardown_gpadl() silently overwrites the error code from vmbus_post_msg(), causing it to falsely report success when GPADL teardown fails, leading to use-after-free memory corruption.
--

--- Patch [1]: [RFC PATCH 1/2] Drivers: hv: vmbus: add vmbus_establish_gpadl_caller_decrypted() ---
commit 93ae4b1e666b7bf013113501ce3aaa80b55f1c28
Author: Kameron Carr <kameroncarr@linux.microsoft.com>

    Drivers: hv: vmbus: add vmbus_establish_gpadl_caller_decrypted()
    
    Refactor vmbus_establish_gpadl() to separate the encryption lifecycle
    from the rest of the GPADL establishment logic. Add a new 
    vmbus_establish_gpadl_caller_decrypted() for callers that want
    to decrypt their own buffers.

> diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c
> index 6821f225248b1..0166367a4df37 100644
> --- a/drivers/hv/channel.c
> +++ b/drivers/hv/channel.c
[ ... ]
> +int vmbus_establish_gpadl_caller_decrypted(struct vmbus_channel *channel,
> +					   void *kbuffer, u32 size,
> +					   struct vmbus_gpadl *gpadl)
> +{
> +	int ret = __vmbus_establish_gpadl(channel, HV_GPADL_BUFFER, kbuffer,
> +					  size, 0U, gpadl);
> +
> +	/* Caller owns @kbuffer's encryption; teardown must not touch it. */
> +	gpadl->decrypted = false;

[Severity: High]
This isn't a bug introduced by this patch, but does setting gpadl->decrypted
to false here interact with a pre-existing issue in vmbus_teardown_gpadl()
that masks teardown failures?

In vmbus_teardown_gpadl(), if vmbus_post_msg() fails (e.g., due to host
resource starvation), the error code in the ret variable is overwritten
unconditionally before the function returns:

drivers/hv/channel.c:vmbus_teardown_gpadl() {
    ...
	ret = vmbus_post_msg(msg,
			     sizeof(struct vmbus_channel_gpadl_teardown), true);
	trace_vmbus_teardown_gpadl(msg, ret);

	if (ret)
		goto post_msg_err;
    ...
post_msg_err:
    ...
	if (gpadl->decrypted)
		ret = set_memory_encrypted((unsigned long)gpadl->buffer,
					PFN_UP(gpadl->size));
	else
		ret = 0;
    ...
	return ret;
}

Since gpadl->decrypted is set to false for these caller-managed GPADLs,
if vmbus_post_msg() fails during teardown, the else branch will set ret = 0.

Could this cause vmbus_teardown_gpadl() to falsely report success when
the teardown actually failed? If the caller then frees the buffer while
the hypervisor still holds a mapping to it, could it lead to memory
corruption across the VM boundary?

> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(vmbus_establish_gpadl_caller_decrypted);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721195633.1438361-1-kameroncarr@linux.microsoft.com?part=1

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox