[PATCH v2 0/4] net: qrtr: Add support for using identical MHI endpoint devices

[PATCH v2 0/4] net: qrtr: Add support for using identical MHI endpoint devices

[Manivannan Sadhasivam via B4 Relay]

Hi,

This series reworks the QRTR driver to add support for using identical MHI
endpoint devices at the same time. Currently, QRTR driver has a limitation on
using more than one identical MHI endpoint devices like QCS6490 or any Qcom
Endpoint device making use of QRTR at the same time. Because of the fact that
these devices will have the identical QRTR node id, they cannot be used at the
same time, on the same host machine.

This limitation is a blocker for running upstream kernel on devices like Qcom
based routers as they often have multiple identical WLAN Endpoint devices
connected to the Router SoC.

This series supersedes the following 2 series from Juha:
https://lore.kernel.org/all/20260915054207.2513877-1-juha-matti.tilli@iki.fi
https://lore.kernel.org/all/20260915074135.2567411-1-juha-matti.tilli@iki.fi

Compared to the series from Juha, this series simplifies the multi-endpoint
support by computing a unique node id for each MHI endpoint and uses it for the
internal QRTR book keeping. This ensures that even if the MHI endpoint devices
have same node ids, the host QRTR will always use a unique node id for each one
of them.

More details can be found in the individual patches.

Testing
=======

Juha tested this series on multi ath11k setup by backporting this series to
6.6.156 kernel. I'd appreciate a formal tested-by tag from Juha and other folks
CCed to this series.

Merge Strategy
==============

Since both QRTR and ath drivers are falling under the networking tree, the whole
series can be merged through netdev tree.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
Changes in v2:
- Rebased on top of net-next/main
- Used READ/WRITE_ONCE for reading/writing ep_nid
- Link to v1: https://patch.msgid.link/20260918-qrtr-multi-ep-v1-0-8a06caa368d3@oss.qualcomm.com

---
Manivannan Sadhasivam (4):
      net: qrtr: Allow the host QRTR to assign a unique node id
      net: qrtr: Assign unique node id for MHI endpoints
      wifi: ath11k: Connect to the QMI server belonging to the device owned by this driver
      wifi: ath12k: Connect to the QMI server belonging to the device owned by this driver

 MAINTAINERS                           |  1 +
 drivers/net/wireless/ath/ath11k/hif.h |  9 ++++++
 drivers/net/wireless/ath/ath11k/pci.c |  9 ++++++
 drivers/net/wireless/ath/ath11k/qmi.c | 10 ++++++
 drivers/net/wireless/ath/ath12k/hif.h |  9 ++++++
 drivers/net/wireless/ath/ath12k/pci.c |  9 ++++++
 drivers/net/wireless/ath/ath12k/qmi.c | 11 +++++++
 include/net/qrtr.h                    | 21 ++++++++++++
 net/qrtr/af_qrtr.c                    | 61 +++++++++++++++++++++++++++++++----
 net/qrtr/mhi.c                        |  4 ++-
 10 files changed, 136 insertions(+), 8 deletions(-)
---
base-commit: 8830e65ed46de41f849eefb8ba227d4852c460f6
change-id: 20260918-qrtr-multi-ep-c27c2bc54ee1

Best regards,
--  
மணிவண்ணன் சதாசிவம்

[PATCH v2 1/4] net: qrtr: Allow the host QRTR to assign a unique node id

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

Currently, QRTR identifies each remote node by the value of the
'src_node_id' field in the received packet and uses the node id as the key
to store the node to the internal 'qrtr_nodes' radix tree.

But this approach comes with a limitation. When more than one remote node
share the same id, then QRTR can only add the first node to the radix tree
and has to drop the successive nodes with the same node id as the 'key' in
radix tree has to be unique. This prevents connecting identical Qcom PCIe
WLAN devices to a single host at the same time.

To fix this limitation, allow the host QRTR to use the node id (nid)
received from the endpoint driver during qrtr_endpoint_register() as the
unique node id and use it as the 'key' for the internal radix tree. Also,
store the received 'src_id' in a new 'qrtr_node->ep_nid' field and replace
the 'ep_nid' with the unique 'nid' in the QRTR control packets if both
differ i.e., an endpoint driver has passed a unique 'nid' during
qrtr_endpoint_register(). To maintain symmetry, replace the 'nid' with the
'ep_nid' while sending the packet back to the remote node if both differ.

Currently, all QRTR endpoint drivers pass 'QRTR_EP_NID_AUTO' as the 'nid'
during qrtr_endpoint_register(). So 'ep_nid' and 'nid' are same and the
functionality is not changed.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 net/qrtr/af_qrtr.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++-------
 1 file changed, 54 insertions(+), 7 deletions(-)

diff --git a/net/qrtr/af_qrtr.c b/net/qrtr/af_qrtr.c
index 78347c937af7..ad62f3caede8 100644
--- a/net/qrtr/af_qrtr.c
+++ b/net/qrtr/af_qrtr.c
@@ -118,7 +118,8 @@ static DEFINE_XARRAY_ALLOC(qrtr_ports);
  * @ep_lock: lock for endpoint management and callbacks
  * @ep: endpoint
  * @ref: reference count for node
- * @nid: node id
+ * @nid: node id assigned by the host QRTR
+ * @ep_nid: endpoint's own node id as received
  * @qrtr_tx_flow: xarray of qrtr_tx_flow, keyed by node << 32 | port
  * @qrtr_tx_lock: lock for qrtr_tx_flow inserts
  * @hello_sent: hello packet send successful
@@ -131,6 +132,7 @@ struct qrtr_node {
 	struct qrtr_endpoint *ep;
 	struct kref ref;
 	unsigned int nid;
+	unsigned int ep_nid;
 
 	struct xarray qrtr_tx_flow;
 	struct mutex qrtr_tx_lock; /* for qrtr_tx_flow */
@@ -347,6 +349,7 @@ static int qrtr_node_enqueue(struct qrtr_node *node, struct sk_buff *skb,
 {
 	struct qrtr_hdr_v1 *hdr;
 	size_t len = skb->len;
+	unsigned int dst_node;
 	int rc, confirm_rx;
 
 	mutex_lock(&node->ep_lock);
@@ -369,10 +372,14 @@ static int qrtr_node_enqueue(struct qrtr_node *node, struct sk_buff *skb,
 	hdr->src_node_id = cpu_to_le32(from->sq_node);
 	hdr->src_port_id = cpu_to_le32(from->sq_port);
 	if (to->sq_port == QRTR_PORT_CTRL) {
-		hdr->dst_node_id = cpu_to_le32(READ_ONCE(node->nid));
+		hdr->dst_node_id = cpu_to_le32(READ_ONCE(node->ep_nid));
 		hdr->dst_port_id = cpu_to_le32(QRTR_PORT_CTRL);
 	} else {
-		hdr->dst_node_id = cpu_to_le32(to->sq_node);
+		/* Put back the endpoint's own node id */
+		dst_node = to->sq_node;
+		if (dst_node == READ_ONCE(node->nid))
+			dst_node = node->ep_nid;
+		hdr->dst_node_id = cpu_to_le32(dst_node);
 		hdr->dst_port_id = cpu_to_le32(to->sq_port);
 	}
 
@@ -441,6 +448,32 @@ static void qrtr_node_assign(struct qrtr_node *node, unsigned int nid)
 	spin_unlock_irqrestore(&qrtr_nodes_lock, flags);
 }
 
+/* Replace the node id in the control packet with 'node->nid', if both are
+ * different.
+ */
+static void qrtr_node_rewrite_ctrl(struct qrtr_node *node, unsigned int type,
+				   struct sk_buff *skb)
+{
+	struct qrtr_ctrl_pkt *pkt;
+	__le32 *nid;
+
+	if (node->nid == node->ep_nid)
+		return;
+
+	if (skb->len < sizeof(*pkt))
+		return;
+
+	pkt = (struct qrtr_ctrl_pkt *)skb->data;
+	if (type == QRTR_TYPE_DEL_CLIENT)
+		nid = &pkt->client.node;
+	else
+		nid = &pkt->server.node;
+
+	/* Rewrite only the endpoint's node id, not those of bridged nodes */
+	if (le32_to_cpu(*nid) == node->ep_nid)
+		*nid = cpu_to_le32(node->nid);
+}
+
 /**
  * qrtr_endpoint_post() - post incoming data
  * @ep: endpoint handle
@@ -531,16 +564,29 @@ int qrtr_endpoint_post(struct qrtr_endpoint *ep, const void *data, size_t len)
 
 	skb_put_data(skb, data + hdrlen, size);
 
-	qrtr_node_assign(node, cb->src_node);
+	if (node->ep_nid == QRTR_EP_NID_AUTO)
+		WRITE_ONCE(node->ep_nid, cb->src_node);
+
+	if (node->nid == QRTR_EP_NID_AUTO || node->nid == cb->src_node)
+		qrtr_node_assign(node, cb->src_node);
 
 	if (cb->type == QRTR_TYPE_NEW_SERVER) {
 		/* Remote node endpoint can bridge other distant nodes */
-		const struct qrtr_ctrl_pkt *pkt;
+		const struct qrtr_ctrl_pkt *pkt = data + hdrlen;
+		unsigned int server_node = le32_to_cpu(pkt->server.node);
 
-		pkt = data + hdrlen;
-		qrtr_node_assign(node, le32_to_cpu(pkt->server.node));
+		if (server_node != node->ep_nid)
+			qrtr_node_assign(node, server_node);
 	}
 
+	if (cb->src_node == node->ep_nid)
+		cb->src_node = node->nid;
+
+	if (cb->type == QRTR_TYPE_NEW_SERVER ||
+	    cb->type == QRTR_TYPE_DEL_SERVER ||
+	    cb->type == QRTR_TYPE_DEL_CLIENT)
+		qrtr_node_rewrite_ctrl(node, cb->type, skb);
+
 	if (cb->type == QRTR_TYPE_RESUME_TX) {
 		qrtr_tx_resume(node, skb);
 	} else {
@@ -646,6 +692,7 @@ int qrtr_endpoint_register(struct qrtr_endpoint *ep, unsigned int nid)
 	mutex_init(&node->ep_lock);
 	skb_queue_head_init(&node->rx_queue);
 	node->nid = QRTR_EP_NID_AUTO;
+	node->ep_nid = QRTR_EP_NID_AUTO;
 	node->ep = ep;
 
 	node->hello_sent = false;

-- 
2.43.0

[PATCH v2 2/4] net: qrtr: Assign unique node id for MHI endpoints

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

Now that the QRTR driver allows using the unique node id for each device,
which is different from the node id received from the remote node, compute
a unique node id using a new API, qrtr_host_node_id().

This API takes the 'index' parameter and adds it to QRTR_NODE_HOST_BASE,
whose value is outside of the node id range used by the remote nodes (0 to
0xffff).

Finally, call the new API from MHI endpoint driver by passing the MHI
controller index as the parameter. The MHI controller index is unique for
each controller, thereby for each MHI endpoint device.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 MAINTAINERS        |  1 +
 include/net/qrtr.h | 21 +++++++++++++++++++++
 net/qrtr/mhi.c     |  4 +++-
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index df8ab9b82402..9c8bb086be28 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22511,6 +22511,7 @@ QUALCOMM IPC ROUTER (QRTR) DRIVER
 M:	Manivannan Sadhasivam <mani@kernel.org>
 L:	linux-arm-msm@vger.kernel.org
 S:	Maintained
+F:	include/net/qrtr.h
 F:	include/trace/events/qrtr.h
 F:	include/uapi/linux/qrtr.h
 F:	net/qrtr/
diff --git a/include/net/qrtr.h b/include/net/qrtr.h
new file mode 100644
index 000000000000..745d51b4a6ba
--- /dev/null
+++ b/include/net/qrtr.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _NET_QRTR_H
+#define _NET_QRTR_H
+
+#include <linux/types.h>
+
+/* The offset is chosen carefully to not collide with the node ids allocated by
+ * the remote nodes. All the remote nodes use node ids in range 0 to 0xffff.
+ */
+#define QRTR_NODE_HOST_BASE 0x10000u
+
+/* Compute host node id from a per-device index. The index must be unique
+ * among the host assigned endpoints and smaller than QRTR_NODE_HOST_BASE.
+ */
+static inline unsigned int qrtr_host_node_id(unsigned int index)
+{
+	return QRTR_NODE_HOST_BASE + index;
+}
+
+#endif /* _NET_QRTR_H */
diff --git a/net/qrtr/mhi.c b/net/qrtr/mhi.c
index 3990da1a65dc..c920ab61f848 100644
--- a/net/qrtr/mhi.c
+++ b/net/qrtr/mhi.c
@@ -6,6 +6,7 @@
 #include <linux/mhi.h>
 #include <linux/module.h>
 #include <linux/skbuff.h>
+#include <net/qrtr.h>
 #include <net/sock.h>
 
 #include "qrtr.h"
@@ -127,7 +128,8 @@ static int qcom_mhi_qrtr_probe(struct mhi_device *mhi_dev,
 	if (rc)
 		return rc;
 
-	rc = qrtr_endpoint_register(&qdev->ep, QRTR_EP_NID_AUTO);
+	rc = qrtr_endpoint_register(&qdev->ep,
+				    qrtr_host_node_id(mhi_dev->mhi_cntrl->index));
 	if (rc)
 		goto err_unprepare;
 

-- 
2.43.0

[PATCH v2 3/4] wifi: ath11k: Connect to the QMI server belonging to the device owned by this driver

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

When two identical ath11k PCIe devices are connected to the same host, both
advertise the WLFW service with the same service and instance id. The QMI
lookup reports both servers to each device, so a device may connect to the
firmware running on the other device.

But now, QRTR provides each MHI endpoint a unique node id which is
different from the node id announced by the device. So use the same id to
pick the correct server. Add a get_qrtr_node_id() HIF callback that returns
the node id derived from the MHI controller index and zero for transports
that do not assign one. In the new_server callback, skip any service whose
node id does not match. A node id of zero disables the check, so transports
that do not assign one keep their current behavior.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/net/wireless/ath/ath11k/hif.h |  9 +++++++++
 drivers/net/wireless/ath/ath11k/pci.c |  9 +++++++++
 drivers/net/wireless/ath/ath11k/qmi.c | 10 ++++++++++
 3 files changed, 28 insertions(+)

diff --git a/drivers/net/wireless/ath/ath11k/hif.h b/drivers/net/wireless/ath/ath11k/hif.h
index 017fed1b4bd1..0da9025c7ed8 100644
--- a/drivers/net/wireless/ath/ath11k/hif.h
+++ b/drivers/net/wireless/ath/ath11k/hif.h
@@ -32,6 +32,7 @@ struct ath11k_hif_ops {
 	void (*ce_irq_disable)(struct ath11k_base *ab);
 	void (*get_ce_msi_idx)(struct ath11k_base *ab, u32 ce_id, u32 *msi_idx);
 	void (*coredump_download)(struct ath11k_base *ab);
+	int (*get_qrtr_node_id)(struct ath11k_base *ab);
 };
 
 static inline void ath11k_hif_ce_irq_enable(struct ath11k_base *ab)
@@ -159,4 +160,12 @@ static inline void ath11k_hif_coredump_download(struct ath11k_base *ab)
 		ab->hif.ops->coredump_download(ab);
 }
 
+static inline int ath11k_hif_get_qrtr_node_id(struct ath11k_base *ab)
+{
+	if (!ab->hif.ops->get_qrtr_node_id)
+		return 0;
+
+	return ab->hif.ops->get_qrtr_node_id(ab);
+}
+
 #endif /* _HIF_H_ */
diff --git a/drivers/net/wireless/ath/ath11k/pci.c b/drivers/net/wireless/ath/ath11k/pci.c
index a163168f3617..4d327b5cc98a 100644
--- a/drivers/net/wireless/ath/ath11k/pci.c
+++ b/drivers/net/wireless/ath/ath11k/pci.c
@@ -10,6 +10,7 @@
 #include <linux/of.h>
 #include <linux/time.h>
 #include <linux/vmalloc.h>
+#include <net/qrtr.h>
 
 #include "pci.h"
 #include "core.h"
@@ -900,6 +901,13 @@ static int ath11k_pci_start(struct ath11k_base *ab)
 	return 0;
 }
 
+static int ath11k_pci_get_qrtr_node_id(struct ath11k_base *ab)
+{
+	struct ath11k_pci *ab_pci = ath11k_pci_priv(ab);
+
+	return qrtr_host_node_id(ab_pci->mhi_ctrl->index);
+}
+
 static const struct ath11k_hif_ops ath11k_pci_hif_ops = {
 	.start = ath11k_pci_start,
 	.stop = ath11k_pcic_stop,
@@ -921,6 +929,7 @@ static const struct ath11k_hif_ops ath11k_pci_hif_ops = {
 #ifdef CONFIG_DEV_COREDUMP
 	.coredump_download = ath11k_pci_coredump_download,
 #endif
+	.get_qrtr_node_id = ath11k_pci_get_qrtr_node_id,
 };
 
 static void ath11k_pci_read_hw_version(struct ath11k_base *ab, u32 *major, u32 *minor)
diff --git a/drivers/net/wireless/ath/ath11k/qmi.c b/drivers/net/wireless/ath/ath11k/qmi.c
index 7dc07339b957..dadf78f30b89 100644
--- a/drivers/net/wireless/ath/ath11k/qmi.c
+++ b/drivers/net/wireless/ath/ath11k/qmi.c
@@ -3170,8 +3170,18 @@ static int ath11k_qmi_ops_new_server(struct qmi_handle *qmi_hdl,
 	struct ath11k_qmi *qmi = container_of(qmi_hdl, struct ath11k_qmi, handle);
 	struct ath11k_base *ab = qmi->ab;
 	struct sockaddr_qrtr *sq = &qmi->sq;
+	int node_id;
 	int ret;
 
+	/* Identical devices advertise the same QMI services, so connect only to
+	 * the QMI server on this device's node. A node id of 0 means the
+	 * transport has not assigned a unique node id, so accept the server
+	 * unfiltered.
+	 */
+	node_id = ath11k_hif_get_qrtr_node_id(ab);
+	if (node_id && service->node != node_id)
+		return 0;
+
 	sq->sq_family = AF_QIPCRTR;
 	sq->sq_node = service->node;
 	sq->sq_port = service->port;

-- 
2.43.0

[PATCH v2 4/4] wifi: ath12k: Connect to the QMI server belonging to the device owned by this driver

[Manivannan Sadhasivam via B4 Relay]

From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

When two identical ath12k PCIe devices are connected to the same host, both
advertise the WLFW service with the same service and instance id. The QMI
lookup reports both servers to each device, so a device may connect to the
firmware running on the other device.

But now, QRTR provides each MHI endpoint a unique node id which is
different from the node id announced by the device. So use the same id to
pick the correct server. Add a get_qrtr_node_id() HIF callback that returns
the node id derived from the MHI controller index and zero for transports
that do not assign one. In the new_server callback, skip any service whose
node id does not match. A node id of zero disables the check, so transports
that do not assign one keep their current behavior.

Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/net/wireless/ath/ath12k/hif.h |  9 +++++++++
 drivers/net/wireless/ath/ath12k/pci.c |  9 +++++++++
 drivers/net/wireless/ath/ath12k/qmi.c | 11 +++++++++++
 3 files changed, 29 insertions(+)

diff --git a/drivers/net/wireless/ath/ath12k/hif.h b/drivers/net/wireless/ath/ath12k/hif.h
index 4de8667690e9..be9c070cf716 100644
--- a/drivers/net/wireless/ath/ath12k/hif.h
+++ b/drivers/net/wireless/ath/ath12k/hif.h
@@ -32,6 +32,7 @@ struct ath12k_hif_ops {
 	void (*get_ce_msi_idx)(struct ath12k_base *ab, u32 ce_id, u32 *msi_idx);
 	int (*panic_handler)(struct ath12k_base *ab);
 	void (*coredump_download)(struct ath12k_base *ab);
+	int (*get_qrtr_node_id)(struct ath12k_base *ab);
 };
 
 static inline int ath12k_hif_map_service_to_pipe(struct ath12k_base *ab, u16 service_id,
@@ -162,4 +163,12 @@ static inline void ath12k_hif_coredump_download(struct ath12k_base *ab)
 	if (ab->hif.ops->coredump_download)
 		ab->hif.ops->coredump_download(ab);
 }
+
+static inline int ath12k_hif_get_qrtr_node_id(struct ath12k_base *ab)
+{
+	if (!ab->hif.ops->get_qrtr_node_id)
+		return 0;
+
+	return ab->hif.ops->get_qrtr_node_id(ab);
+}
 #endif /* ATH12K_HIF_H */
diff --git a/drivers/net/wireless/ath/ath12k/pci.c b/drivers/net/wireless/ath/ath12k/pci.c
index 6441927b5382..2b1b122bbb3b 100644
--- a/drivers/net/wireless/ath/ath12k/pci.c
+++ b/drivers/net/wireless/ath/ath12k/pci.c
@@ -10,6 +10,7 @@
 #include <linux/pci.h>
 #include <linux/time.h>
 #include <linux/vmalloc.h>
+#include <net/qrtr.h>
 
 #include "pci.h"
 #include "core.h"
@@ -1491,6 +1492,13 @@ static int ath12k_pci_panic_handler(struct ath12k_base *ab)
 	return NOTIFY_OK;
 }
 
+static int ath12k_pci_get_qrtr_node_id(struct ath12k_base *ab)
+{
+	struct ath12k_pci *ab_pci = ath12k_pci_priv(ab);
+
+	return qrtr_host_node_id(ab_pci->mhi_ctrl->index);
+}
+
 static const struct ath12k_hif_ops ath12k_pci_hif_ops = {
 	.start = ath12k_pci_start,
 	.stop = ath12k_pci_stop,
@@ -1512,6 +1520,7 @@ static const struct ath12k_hif_ops ath12k_pci_hif_ops = {
 #ifdef CONFIG_ATH12K_COREDUMP
 	.coredump_download = ath12k_pci_coredump_download,
 #endif
+	.get_qrtr_node_id = ath12k_pci_get_qrtr_node_id,
 };
 
 static enum ath12k_device_family
diff --git a/drivers/net/wireless/ath/ath12k/qmi.c b/drivers/net/wireless/ath/ath12k/qmi.c
index 280e50a1f31d..22b8cbbbf4f4 100644
--- a/drivers/net/wireless/ath/ath12k/qmi.c
+++ b/drivers/net/wireless/ath/ath12k/qmi.c
@@ -9,6 +9,7 @@
 #include "qmi.h"
 #include "core.h"
 #include "debug.h"
+#include "hif.h"
 #include <linux/of.h>
 #include <linux/firmware.h>
 #include <linux/of_address.h>
@@ -3940,8 +3941,18 @@ static int ath12k_qmi_ops_new_server(struct qmi_handle *qmi_hdl,
 	struct ath12k_qmi *qmi = container_of(qmi_hdl, struct ath12k_qmi, handle);
 	struct ath12k_base *ab = qmi->ab;
 	struct sockaddr_qrtr *sq = &qmi->sq;
+	int node_id;
 	int ret;
 
+	/* Identical devices advertise the same QMI services, so connect only to
+	 * the QMI server on this device's node. A node id of 0 means the
+	 * transport has not assigned a unique node id, so accept the server
+	 * unfiltered.
+	 */
+	node_id = ath12k_hif_get_qrtr_node_id(ab);
+	if (node_id && service->node != node_id)
+		return 0;
+
 	sq->sq_family = AF_QIPCRTR;
 	sq->sq_node = service->node;
 	sq->sq_port = service->port;

-- 
2.43.0

Re: [PATCH v2 3/4] wifi: ath11k: Connect to the QMI server belonging to the device owned by this driver

[Juha-Matti Tilli]

On Mon, Sep 21, 2026, at 14:23, Manivannan Sadhasivam via B4 Relay wrote:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> 
> When two identical ath11k PCIe devices are connected to the same host, both
> advertise the WLFW service with the same service and instance id. The QMI
> lookup reports both servers to each device, so a device may connect to the
> firmware running on the other device.
> 
> But now, QRTR provides each MHI endpoint a unique node id which is
> different from the node id announced by the device. So use the same id to
> pick the correct server. Add a get_qrtr_node_id() HIF callback that returns
> the node id derived from the MHI controller index and zero for transports
> that do not assign one. In the new_server callback, skip any service whose
> node id does not match. A node id of zero disables the check, so transports
> that do not assign one keep their current behavior.

Tested-by: Juha-Matti Tilli <juha-matti.tilli@iki.fi>
Tested-on: 2x WCN6855 hw2.1 PCI WLAN.HSP.1.1-04685-QCAHSPSWPL_V1_V2_SILICONZ_IOE-1

That's from the new rebased patches, v2, applied with git am.

Again, I picked the entire QRTR from net-next/main after your changes,
and backported the ath11k and ath12k changes to 6.6.157 kernel. The
conflicts in ath11k and ath12k are really simple to resolve. The QRTR is
better to take as-is, as it's a large and substantially modified piece
of code, and backporting could risk different behavior.

Unfortunately, it seems I'm locked to 6.6 kernel until I manage to boot
a newer kernel. I tested 6.12 series and my perfectly valid gzipped
initrd ext2 file system is recognised as gzipped, but mounting the root
from /dev/ram0 fails due to -EBUSY. Why this happens with 6.12.x but not
6.6.157 is a mystery to me. Prompting two AI models didn't help, they
keep on just repeating the same useless points. So I have to resort to
heavier weapons, printk and maybe even git bisect.

So, because 6.6 kernel in our CPU doesn't work with ath12k due to lack
of interrupts and I can't boot a newer kernel yet, you'll have to either
find someone with two ath12k cards, or just accept to test the ath12k
change with a single card and hope for the best.

My understanding is that 6.12 would support just 1 interrupt in ath12k.
If I could only boot it, that is.

BR, Juha-Matti

Re: [PATCH v2 3/4] wifi: ath11k: Connect to the QMI server belonging to the device owned by this driver

[Vasanthakumar Thiagarajan]

On 9/21/2026 4:53 PM, Manivannan Sadhasivam via B4 Relay wrote:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> 
> When two identical ath11k PCIe devices are connected to the same host, both
> advertise the WLFW service with the same service and instance id. The QMI
> lookup reports both servers to each device, so a device may connect to the
> firmware running on the other device.
> 
> But now, QRTR provides each MHI endpoint a unique node id which is
> different from the node id announced by the device. So use the same id to
> pick the correct server. Add a get_qrtr_node_id() HIF callback that returns
> the node id derived from the MHI controller index and zero for transports
> that do not assign one. In the new_server callback, skip any service whose
> node id does not match. A node id of zero disables the check, so transports
> that do not assign one keep their current behavior.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---
>   drivers/net/wireless/ath/ath11k/hif.h |  9 +++++++++
>   drivers/net/wireless/ath/ath11k/pci.c |  9 +++++++++
>   drivers/net/wireless/ath/ath11k/qmi.c | 10 ++++++++++
>   3 files changed, 28 insertions(+)
> 
> diff --git a/drivers/net/wireless/ath/ath11k/hif.h b/drivers/net/wireless/ath/ath11k/hif.h
> index 017fed1b4bd1..0da9025c7ed8 100644
> --- a/drivers/net/wireless/ath/ath11k/hif.h
> +++ b/drivers/net/wireless/ath/ath11k/hif.h
> @@ -32,6 +32,7 @@ struct ath11k_hif_ops {
>   	void (*ce_irq_disable)(struct ath11k_base *ab);
>   	void (*get_ce_msi_idx)(struct ath11k_base *ab, u32 ce_id, u32 *msi_idx);
>   	void (*coredump_download)(struct ath11k_base *ab);
> +	int (*get_qrtr_node_id)(struct ath11k_base *ab);
>   };
>   
>   static inline void ath11k_hif_ce_irq_enable(struct ath11k_base *ab)
> @@ -159,4 +160,12 @@ static inline void ath11k_hif_coredump_download(struct ath11k_base *ab)
>   		ab->hif.ops->coredump_download(ab);
>   }
>   
> +static inline int ath11k_hif_get_qrtr_node_id(struct ath11k_base *ab)
> +{
> +	if (!ab->hif.ops->get_qrtr_node_id)
> +		return 0;
> +
> +	return ab->hif.ops->get_qrtr_node_id(ab);
> +}
> +
>   #endif /* _HIF_H_ */
> diff --git a/drivers/net/wireless/ath/ath11k/pci.c b/drivers/net/wireless/ath/ath11k/pci.c
> index a163168f3617..4d327b5cc98a 100644
> --- a/drivers/net/wireless/ath/ath11k/pci.c
> +++ b/drivers/net/wireless/ath/ath11k/pci.c
> @@ -10,6 +10,7 @@
>   #include <linux/of.h>
>   #include <linux/time.h>
>   #include <linux/vmalloc.h>
> +#include <net/qrtr.h>
>   
>   #include "pci.h"
>   #include "core.h"
> @@ -900,6 +901,13 @@ static int ath11k_pci_start(struct ath11k_base *ab)
>   	return 0;
>   }
>   
> +static int ath11k_pci_get_qrtr_node_id(struct ath11k_base *ab)
> +{
> +	struct ath11k_pci *ab_pci = ath11k_pci_priv(ab);
> +
> +	return qrtr_host_node_id(ab_pci->mhi_ctrl->index);

nit: since qrtr_host_node_id() returns node_id in unsigned int,
may be maintain that in the return type of the callers as well.
Mostly harmless as it is unlikely that node_id is allocated with
an extremely high id. Either way is fine with me. Same applicable
for ath12k patch as well.


Reviewed-by: Vasanthakumar Thiagarajan <vasanthakumar.thiagarajan@oss.qualcomm.com>

Re: [PATCH v2 4/4] wifi: ath12k: Connect to the QMI server belonging to the device owned by this driver

[Vasanthakumar Thiagarajan]

On 9/21/2026 4:53 PM, Manivannan Sadhasivam via B4 Relay wrote:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> 
> When two identical ath12k PCIe devices are connected to the same host, both
> advertise the WLFW service with the same service and instance id. The QMI
> lookup reports both servers to each device, so a device may connect to the
> firmware running on the other device.
> 
> But now, QRTR provides each MHI endpoint a unique node id which is
> different from the node id announced by the device. So use the same id to
> pick the correct server. Add a get_qrtr_node_id() HIF callback that returns
> the node id derived from the MHI controller index and zero for transports
> that do not assign one. In the new_server callback, skip any service whose
> node id does not match. A node id of zero disables the check, so transports
> that do not assign one keep their current behavior.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

Reviewed-by: Vasanthakumar Thiagarajan <vasanthakumar.thiagarajan@oss.qualcomm.com>

Re: [PATCH v2 4/4] wifi: ath12k: Connect to the QMI server belonging to the device owned by this driver

[Juha-Matti Tilli]

On Mon, Sep 21, 2026, at 14:23, Manivannan Sadhasivam via B4 Relay wrote:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> 
> When two identical ath12k PCIe devices are connected to the same host, both
> advertise the WLFW service with the same service and instance id. The QMI
> lookup reports both servers to each device, so a device may connect to the
> firmware running on the other device.
> 
> But now, QRTR provides each MHI endpoint a unique node id which is
> different from the node id announced by the device. So use the same id to
> pick the correct server. Add a get_qrtr_node_id() HIF callback that returns
> the node id derived from the MHI controller index and zero for transports
> that do not assign one. In the new_server callback, skip any service whose
> node id does not match. A node id of zero disables the check, so transports
> that do not assign one keep their current behavior.

Tested-by: Juha-Matti Tilli <juha-matti.tilli@iki.fi>
Tested-on: 2x WCN7850 hw2.0 WLAN.IOE_HMT.1.1-00018-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1
Tested-on: 1x WCN6855 ath11k + 1x WCN7850 ath12k

I'm having some issues with regdb on ath12k cards so I had to test using
the low-frequency 2.4GHz channels. I tried updating linux-firmware to
20260810 to match my 6.12.110+multi-QRTR kernel, didn't help. The error
is:

ath12k_pci 0001:01:00.0: No reg rules available
ath12k_pci 0001:01:00.0: failed to extract regulatory info from received event
------------[ cut here ]------------
WARNING: CPU: 0 PID: 12 at /drivers/net/wireless/ath/ath12k/wmi.c:5841 ath12k_reg_chan_list_event+0x171/0x1e0 [ath12k]
Modules linked in: xt_gmmark(O) xt_bemark(O) w24e2identificator(O) qrtr_mhi ath12k mhi qmi_helpers mac80211 libarc4 cfg80211
CPU: 0 UID: 0 PID: 12 Comm: kworker/u8:0 Tainted: G           O       6.12.110-yocto-standard-gec77603762c7 #1
Tainted: [O]=OOT_MODULE
Hardware name: Freescale LS1021A
Workqueue: ath12k_qmi_driver_event ath12k_qmi_driver_event_work [ath12k]
Call trace: 
 unwind_backtrace from show_stack+0xb/0xc
 show_stack from dump_stack_lvl+0x35/0x40
 dump_stack_lvl from __warn+0xaf/0xb0
 __warn from warn_slowpath_fmt+0xe5/0xe8
 warn_slowpath_fmt from ath12k_reg_chan_list_event+0x171/0x1e0 [ath12k]
 ath12k_reg_chan_list_event [ath12k] from ath12k_wmi_op_rx+0x219/0xc08 [ath12k]
 ath12k_wmi_op_rx [ath12k] from ath12k_htc_rx_completion_handler+0x215/0x2d4 [ath12k]
 ath12k_htc_rx_completion_handler [ath12k] from ath12k_ce_recv_process_cb+0x149/0x198 [ath12k]
 ath12k_ce_recv_process_cb [ath12k] from ath12k_pci_ce_tasklet+0x13/0x28 [ath12k]
 ath12k_pci_ce_tasklet [ath12k] from tasklet_action_common+0x87/0xa8
 tasklet_action_common from handle_softirqs+0xbf/0x1b4
 handle_softirqs from irq_exit+0x55/0x78
 irq_exit from call_with_stack+0xd/0x10
---[ end trace 0000000000000000 ]---

This error happens with 2x ath12k and 1x ath12k 1x ath11k too.

But, the dual ath12k setup works, and the errors are entirely different
than what QRTR node id clash does.

So, I believe the patch works.

BR, Juha-Matti

Re: [PATCH v2 3/4] wifi: ath11k: Connect to the QMI server belonging to the device owned by this driver

[Manivannan Sadhasivam]

On Tue, Sep 22, 2026 at 04:32:07PM +0530, Vasanthakumar Thiagarajan wrote:
> 
> 
> On 9/21/2026 4:53 PM, Manivannan Sadhasivam via B4 Relay wrote:
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > 
> > When two identical ath11k PCIe devices are connected to the same host, both
> > advertise the WLFW service with the same service and instance id. The QMI
> > lookup reports both servers to each device, so a device may connect to the
> > firmware running on the other device.
> > 
> > But now, QRTR provides each MHI endpoint a unique node id which is
> > different from the node id announced by the device. So use the same id to
> > pick the correct server. Add a get_qrtr_node_id() HIF callback that returns
> > the node id derived from the MHI controller index and zero for transports
> > that do not assign one. In the new_server callback, skip any service whose
> > node id does not match. A node id of zero disables the check, so transports
> > that do not assign one keep their current behavior.
> > 
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > ---
> >   drivers/net/wireless/ath/ath11k/hif.h |  9 +++++++++
> >   drivers/net/wireless/ath/ath11k/pci.c |  9 +++++++++
> >   drivers/net/wireless/ath/ath11k/qmi.c | 10 ++++++++++
> >   3 files changed, 28 insertions(+)
> > 
> > diff --git a/drivers/net/wireless/ath/ath11k/hif.h b/drivers/net/wireless/ath/ath11k/hif.h
> > index 017fed1b4bd1..0da9025c7ed8 100644
> > --- a/drivers/net/wireless/ath/ath11k/hif.h
> > +++ b/drivers/net/wireless/ath/ath11k/hif.h
> > @@ -32,6 +32,7 @@ struct ath11k_hif_ops {
> >   	void (*ce_irq_disable)(struct ath11k_base *ab);
> >   	void (*get_ce_msi_idx)(struct ath11k_base *ab, u32 ce_id, u32 *msi_idx);
> >   	void (*coredump_download)(struct ath11k_base *ab);
> > +	int (*get_qrtr_node_id)(struct ath11k_base *ab);
> >   };
> >   static inline void ath11k_hif_ce_irq_enable(struct ath11k_base *ab)
> > @@ -159,4 +160,12 @@ static inline void ath11k_hif_coredump_download(struct ath11k_base *ab)
> >   		ab->hif.ops->coredump_download(ab);
> >   }
> > +static inline int ath11k_hif_get_qrtr_node_id(struct ath11k_base *ab)
> > +{
> > +	if (!ab->hif.ops->get_qrtr_node_id)
> > +		return 0;
> > +
> > +	return ab->hif.ops->get_qrtr_node_id(ab);
> > +}
> > +
> >   #endif /* _HIF_H_ */
> > diff --git a/drivers/net/wireless/ath/ath11k/pci.c b/drivers/net/wireless/ath/ath11k/pci.c
> > index a163168f3617..4d327b5cc98a 100644
> > --- a/drivers/net/wireless/ath/ath11k/pci.c
> > +++ b/drivers/net/wireless/ath/ath11k/pci.c
> > @@ -10,6 +10,7 @@
> >   #include <linux/of.h>
> >   #include <linux/time.h>
> >   #include <linux/vmalloc.h>
> > +#include <net/qrtr.h>
> >   #include "pci.h"
> >   #include "core.h"
> > @@ -900,6 +901,13 @@ static int ath11k_pci_start(struct ath11k_base *ab)
> >   	return 0;
> >   }
> > +static int ath11k_pci_get_qrtr_node_id(struct ath11k_base *ab)
> > +{
> > +	struct ath11k_pci *ab_pci = ath11k_pci_priv(ab);
> > +
> > +	return qrtr_host_node_id(ab_pci->mhi_ctrl->index);
> 
> nit: since qrtr_host_node_id() returns node_id in unsigned int,
> may be maintain that in the return type of the callers as well.
> Mostly harmless as it is unlikely that node_id is allocated with
> an extremely high id. Either way is fine with me. Same applicable
> for ath12k patch as well.
> 

Thanks for spotting it. For the sake of correctness, it makes sense to use same
type everywhere. I'll fix it in next version.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v2 4/4] wifi: ath12k: Connect to the QMI server belonging to the device owned by this driver

[Manivannan Sadhasivam]

On Tue, Sep 22, 2026 at 02:43:05PM +0300, Juha-Matti Tilli wrote:
> On Mon, Sep 21, 2026, at 14:23, Manivannan Sadhasivam via B4 Relay wrote:
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> > 
> > When two identical ath12k PCIe devices are connected to the same host, both
> > advertise the WLFW service with the same service and instance id. The QMI
> > lookup reports both servers to each device, so a device may connect to the
> > firmware running on the other device.
> > 
> > But now, QRTR provides each MHI endpoint a unique node id which is
> > different from the node id announced by the device. So use the same id to
> > pick the correct server. Add a get_qrtr_node_id() HIF callback that returns
> > the node id derived from the MHI controller index and zero for transports
> > that do not assign one. In the new_server callback, skip any service whose
> > node id does not match. A node id of zero disables the check, so transports
> > that do not assign one keep their current behavior.
> 
> Tested-by: Juha-Matti Tilli <juha-matti.tilli@iki.fi>
> Tested-on: 2x WCN7850 hw2.0 WLAN.IOE_HMT.1.1-00018-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1
> Tested-on: 1x WCN6855 ath11k + 1x WCN7850 ath12k
> 
> I'm having some issues with regdb on ath12k cards so I had to test using
> the low-frequency 2.4GHz channels. I tried updating linux-firmware to
> 20260810 to match my 6.12.110+multi-QRTR kernel, didn't help. The error
> is:
> 
> ath12k_pci 0001:01:00.0: No reg rules available
> ath12k_pci 0001:01:00.0: failed to extract regulatory info from received event
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 12 at /drivers/net/wireless/ath/ath12k/wmi.c:5841 ath12k_reg_chan_list_event+0x171/0x1e0 [ath12k]
> Modules linked in: xt_gmmark(O) xt_bemark(O) w24e2identificator(O) qrtr_mhi ath12k mhi qmi_helpers mac80211 libarc4 cfg80211
> CPU: 0 UID: 0 PID: 12 Comm: kworker/u8:0 Tainted: G           O       6.12.110-yocto-standard-gec77603762c7 #1
> Tainted: [O]=OOT_MODULE
> Hardware name: Freescale LS1021A
> Workqueue: ath12k_qmi_driver_event ath12k_qmi_driver_event_work [ath12k]
> Call trace: 
>  unwind_backtrace from show_stack+0xb/0xc
>  show_stack from dump_stack_lvl+0x35/0x40
>  dump_stack_lvl from __warn+0xaf/0xb0
>  __warn from warn_slowpath_fmt+0xe5/0xe8
>  warn_slowpath_fmt from ath12k_reg_chan_list_event+0x171/0x1e0 [ath12k]
>  ath12k_reg_chan_list_event [ath12k] from ath12k_wmi_op_rx+0x219/0xc08 [ath12k]
>  ath12k_wmi_op_rx [ath12k] from ath12k_htc_rx_completion_handler+0x215/0x2d4 [ath12k]
>  ath12k_htc_rx_completion_handler [ath12k] from ath12k_ce_recv_process_cb+0x149/0x198 [ath12k]
>  ath12k_ce_recv_process_cb [ath12k] from ath12k_pci_ce_tasklet+0x13/0x28 [ath12k]
>  ath12k_pci_ce_tasklet [ath12k] from tasklet_action_common+0x87/0xa8
>  tasklet_action_common from handle_softirqs+0xbf/0x1b4
>  handle_softirqs from irq_exit+0x55/0x78
>  irq_exit from call_with_stack+0xd/0x10
> ---[ end trace 0000000000000000 ]---
> 
> This error happens with 2x ath12k and 1x ath12k 1x ath11k too.
> 
> But, the dual ath12k setup works, and the errors are entirely different
> than what QRTR node id clash does.
> 
> So, I believe the patch works.
> 

Thanks a lot for testing. I asked our WLAN team member to test this series on
multi-ath12k setup and found one issue with QRTR_TYPE_RESUME_TX. After fixing
it, multi-ath12k setup works fine.

I'll submit v3 with that fix. Please give it a shot on this setup, one final
time :) Appreciate your help here.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v2 4/4] wifi: ath12k: Connect to the QMI server belonging to the device owned by this driver

[Juha-Matti Tilli]

On Tue, Sep 22, 2026, at 16:30, Manivannan Sadhasivam wrote:
> On Tue, Sep 22, 2026 at 02:43:05PM +0300, Juha-Matti Tilli wrote:
> > So, I believe the patch works.
> 
> Thanks a lot for testing. I asked our WLAN team member to test this series on
> multi-ath12k setup and found one issue with QRTR_TYPE_RESUME_TX. After fixing
> it, multi-ath12k setup works fine.
> 
> I'll submit v3 with that fix. Please give it a shot on this setup, one final
> time :) Appreciate your help here.

Is this issue perhaps related to this (2x ath12k):

ath12k_pci 0001:01:00.0: dp_tx: failed to find the peer with peer_id 2

I managed to switch to 6.18.53 kernel and now the regulatory DB issue
is gone, but I'm getting this repeatedly to dmesg.

Yet, despite this issue, I see the network and data flows (with v2).

Also the other card gives:

ath12k_pci 0000:01:00.0: failed to pull fw stats: -71
ath12k_pci 0000:01:00.0: time out while waiting for get fw stats

...but it's lacking antennas so could be related to that.

BR, Juha-Matti

Re: [PATCH v2 1/4] net: qrtr: Allow the host QRTR to assign a unique node id

[Youssef Samir]

On 9/21/2026 12:23 PM, Manivannan Sadhasivam via B4 Relay wrote:
> +/* Replace the node id in the control packet with 'node->nid', if both are
> + * different.
> + */
> +static void qrtr_node_rewrite_ctrl(struct qrtr_node *node, unsigned int type,
> +				   struct sk_buff *skb)
> +{
> +	struct qrtr_ctrl_pkt *pkt;
> +	__le32 *nid;
> +
> +	if (node->nid == node->ep_nid)
> +		return;
> +
> +	if (skb->len < sizeof(*pkt))
> +		return;
> +
> +	pkt = (struct qrtr_ctrl_pkt *)skb->data;
> +	if (type == QRTR_TYPE_DEL_CLIENT)
qrtr_tx_wait() will create a qrtr_tx_flow object keyed by qrtr_node->nid.
qrtr_tx_resume will attempt to retrieve it with qrtr_node->ep_nid, which
will fail. This leads to the HTD path pending packets reaching
QRTR_TX_FLOW_HIGH and block indefinitely. QRTR_TYPE_RESUME_TX should
be handled here.> +		nid = &pkt->client.node;
> +	else
> +		nid = &pkt->server.node;
> +
> +	/* Rewrite only the endpoint's node id, not those of bridged nodes */
> +	if (le32_to_cpu(*nid) == node->ep_nid)
> +		*nid = cpu_to_le32(node->nid);
> +}
> +
Thanks,
Youssef

Re: [PATCH v2 2/4] net: qrtr: Assign unique node id for MHI endpoints

[Youssef Samir]

On 9/21/2026 12:23 PM, Manivannan Sadhasivam via B4 Relay wrote:
> +
> +/* The offset is chosen carefully to not collide with the node ids allocated by
> + * the remote nodes. All the remote nodes use node ids in range 0 to 0xffff.
This assumption does not hold up for QRTR V1 headers where the node ids are 32bit.
QAIC devices use BHI_SERIALNUM as the node ID, and it is definitely larger than
0xFFFF.> + */
> +#define QRTR_NODE_HOST_BASE 0x10000u
>
Thanks,
Youssef

Re: [PATCH v2 2/4] net: qrtr: Assign unique node id for MHI endpoints

[Juha-Matti Tilli]

On Tue, Sep 22, 2026, at 20:14, Youssef Samir wrote:
> On 9/21/2026 12:23 PM, Manivannan Sadhasivam via B4 Relay wrote:
> > +
> > +/* The offset is chosen carefully to not collide with the node ids allocated by
> > + * the remote nodes. All the remote nodes use node ids in range 0 to 0xffff.
> This assumption does not hold up for QRTR V1 headers where the node ids are 32bit.
> QAIC devices use BHI_SERIALNUM as the node ID, and it is definitely larger than
> 0xFFFF.> + */

Does that mean we have to go back to my endpoint id patchset?  It's an
option, but a quite complex one.

However, what is the likelihood that serial number would clash with
these host node id values? Maybe just adjusting the comment could be
acceptable.

Also, rewriting QRTR node_id for more than just MHI devices could help
here. If all are rewritten, the problem goes away.

BR, Juha-Matti

Re: [PATCH v2 2/4] net: qrtr: Assign unique node id for MHI endpoints

[Youssef Samir]

On Tue, Sep 22, 2026 at 6:24 PM Juha-Matti Tilli
<juha-matti.tilli@iki.fi> wrote:
>
> On Tue, Sep 22, 2026, at 20:14, Youssef Samir wrote:
> > On 9/21/2026 12:23 PM, Manivannan Sadhasivam via B4 Relay wrote:
> > > +
> > > +/* The offset is chosen carefully to not collide with the node ids allocated by
> > > + * the remote nodes. All the remote nodes use node ids in range 0 to 0xffff.
> > This assumption does not hold up for QRTR V1 headers where the node ids are 32bit.
> > QAIC devices use BHI_SERIALNUM as the node ID, and it is definitely larger than
> > 0xFFFF.> + */
>
> Does that mean we have to go back to my endpoint id patchset?  It's an
> option, but a quite complex one.
I'm not sure. Last time I tested the endpoint ID series was its v5 and it was
functionally acceptable, and other than switching to sendmsg(2) instead of
send(2) and sendto(2), it did not require much changing on the user-space
side. But as you mentioned, it was/is complex compared to this one.
>
> However, what is the likelihood that serial number would clash with
> these host node id values? Maybe just adjusting the comment could be
> acceptable.
The chance of collision is slim, but still not impossible.
I think a solution would be to treat qrtr_host_node_id(idx) as the
preferred id, not
the one that will actually be assigned to the node. And before
assigning the nid,
we check it against the radix tree, and if it exists, we try to probe
the next available
id (so an IDA). This will also require exposing the nid to the user in
sysfs (i.e. /sys/bus/mhi/devices/mhix/mhix_IPCR/nid).

Thanks,
Youssef

Re: [PATCH v2 1/4] net: qrtr: Allow the host QRTR to assign a unique node id

[Manivannan Sadhasivam]

On Tue, Sep 22, 2026 at 06:08:11PM +0100, Youssef Samir wrote:
> 
> 
> On 9/21/2026 12:23 PM, Manivannan Sadhasivam via B4 Relay wrote:
> > +/* Replace the node id in the control packet with 'node->nid', if both are
> > + * different.
> > + */
> > +static void qrtr_node_rewrite_ctrl(struct qrtr_node *node, unsigned int type,
> > +				   struct sk_buff *skb)
> > +{
> > +	struct qrtr_ctrl_pkt *pkt;
> > +	__le32 *nid;
> > +
> > +	if (node->nid == node->ep_nid)
> > +		return;
> > +
> > +	if (skb->len < sizeof(*pkt))
> > +		return;
> > +
> > +	pkt = (struct qrtr_ctrl_pkt *)skb->data;
> > +	if (type == QRTR_TYPE_DEL_CLIENT)
> qrtr_tx_wait() will create a qrtr_tx_flow object keyed by qrtr_node->nid.
> qrtr_tx_resume will attempt to retrieve it with qrtr_node->ep_nid, which
> will fail. This leads to the HTD path pending packets reaching
> QRTR_TX_FLOW_HIGH and block indefinitely. QRTR_TYPE_RESUME_TX should
> be handled here.> +		nid = &pkt->client.node;

You are right! I've already fixed this issue in v3.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v2 1/4] net: qrtr: Allow the host QRTR to assign a unique node id

[netdev-bot+sashiko]

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

Critical: 0 · High: 3 · Medium: 3 · Low: 0 (1)

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] Incomplete node-id translation: QRTR_TYPE_RESUME_TX payloads are
  not translated, so flow-control credits returned by the remote are…
- [High] Lost routing entries for bridged (distant) nodes.
- [High] The host-assigned node id is not validated and the
  radix_tree_insert() failure is ignored, so the very failure mode the…
- [Medium] node->ep_nid is latched unconditionally from the first
  accepted packet's src_node (af_qrtr.c:567-568) with no requirement…
- [Medium] The QRTR_EP_NID_AUTO sentinel ((-1) == 0xffffffff, which
  include/uapi/linux/qrtr.h defines as QRTR_NODE_BCAST) can be…
- [Medium] Unsynchronized one-time initialization of node->ep_nid.

Pre-existing issues:
- [Low] (Assessed as a false positive / hardening note only.)
  qrtr_node_rewrite_ctrl() returns without rewriting when skb->len <…

--

net: qrtr: Allow the host QRTR to assign a unique node id

This patch lets the host assign a unique node id at
qrtr_endpoint_register() time and uses it as the qrtr_nodes radix tree key,
while the id reported by the remote is kept separately in the new
qrtr_node->ep_nid field. Received control packets and headers are rewritten
to translate between the two ids when they differ.

A few questions below about the translation and the routing changes.

> diff --git a/net/qrtr/af_qrtr.c b/net/qrtr/af_qrtr.c
> index 78347c937af76..ad62f3caede8b 100644
> --- a/net/qrtr/af_qrtr.c
> +++ b/net/qrtr/af_qrtr.c

[ ... ]

> @@ -347,6 +349,7 @@ static int qrtr_node_enqueue(struct qrtr_node *node, struct sk_buff *skb,
>  {
>  	struct qrtr_hdr_v1 *hdr;
>  	size_t len = skb->len;
> +	unsigned int dst_node;
>  	int rc, confirm_rx;
>  
>  	mutex_lock(&node->ep_lock);
> @@ -369,10 +372,14 @@ static int qrtr_node_enqueue(struct qrtr_node *node, struct sk_buff *skb,
>  	hdr->src_node_id = cpu_to_le32(from->sq_node);
>  	hdr->src_port_id = cpu_to_le32(from->sq_port);
>  	if (to->sq_port == QRTR_PORT_CTRL) {
> -		hdr->dst_node_id = cpu_to_le32(READ_ONCE(node->nid));
> +		hdr->dst_node_id = cpu_to_le32(READ_ONCE(node->ep_nid));
>  		hdr->dst_port_id = cpu_to_le32(QRTR_PORT_CTRL);
>  	} else {
> -		hdr->dst_node_id = cpu_to_le32(to->sq_node);
> +		/* Put back the endpoint's own node id */
> +		dst_node = to->sq_node;
> +		if (dst_node == READ_ONCE(node->nid))
> +			dst_node = node->ep_nid;
> +		hdr->dst_node_id = cpu_to_le32(dst_node);
>  		hdr->dst_port_id = cpu_to_le32(to->sq_port);
>  	}

[Severity: Medium]
Can the QRTR_EP_NID_AUTO sentinel end up on the wire as a unicast
destination here?

qrtr_endpoint_register() sets node->ep_nid = QRTR_EP_NID_AUTO and inserts
node->nid into qrtr_nodes right away for an explicit-nid endpoint, then
schedules the HELLO work. node->ep_nid is only filled in later, by the
first inbound packet in qrtr_endpoint_post().

In that window a local sendmsg() to sq_node == node->nid reaches the data
branch above, matches dst_node == READ_ONCE(node->nid) and emits ep_nid,
which is still (-1). net/qrtr/qrtr.h has:

    #define QRTR_EP_NID_AUTO (-1)

and include/uapi/linux/qrtr.h has:

    #define QRTR_NODE_BCAST		0xffffffffu

so the destination node id transmitted is the broadcast id. Should both
branches skip transmission (or defer) while ep_nid is still the sentinel?

Also, node->ep_nid is read here with a plain load in the data branch while
qrtr_endpoint_post() publishes it with WRITE_ONCE(). Was the missing
READ_ONCE() on this read intentional, given the CTRL branch just above uses
one?

> @@ -441,6 +448,32 @@ static void qrtr_node_assign(struct qrtr_node *node, unsigned int nid)
>  	spin_unlock_irqrestore(&qrtr_nodes_lock, flags);
>  }
>  
> +/* Replace the node id in the control packet with 'node->nid', if both are
> + * different.
> + */
> +static void qrtr_node_rewrite_ctrl(struct qrtr_node *node, unsigned int type,
> +				   struct sk_buff *skb)
> +{
> +	struct qrtr_ctrl_pkt *pkt;
> +	__le32 *nid;
> +
> +	if (node->nid == node->ep_nid)
> +		return;
> +
> +	if (skb->len < sizeof(*pkt))
> +		return;

[Severity: Low]
This isn't a bug introduced by this patch, but the minimum-size check in
qrtr_endpoint_post() only covers two types:

	if ((cb->type == QRTR_TYPE_NEW_SERVER ||
	     cb->type == QRTR_TYPE_RESUME_TX) &&
	    size < sizeof(struct qrtr_ctrl_pkt))
		goto err;

so an undersized QRTR_TYPE_DEL_SERVER or QRTR_TYPE_DEL_CLIENT takes the
early return above and skips the rewrite while still being delivered.
The truncated payload fields read as zero in qrtr_ns_worker(), so the
outcome matches the pre-patch behaviour, but would it be worth rejecting
undersized DEL_SERVER/DEL_CLIENT the same way NEW_SERVER already is?

> +
> +	pkt = (struct qrtr_ctrl_pkt *)skb->data;
> +	if (type == QRTR_TYPE_DEL_CLIENT)
> +		nid = &pkt->client.node;
> +	else
> +		nid = &pkt->server.node;
> +
> +	/* Rewrite only the endpoint's node id, not those of bridged nodes */
> +	if (le32_to_cpu(*nid) == node->ep_nid)
> +		*nid = cpu_to_le32(node->nid);
> +}
> +
>  /**
>   * qrtr_endpoint_post() - post incoming data
>   * @ep: endpoint handle
> @@ -531,16 +564,29 @@ int qrtr_endpoint_post(struct qrtr_endpoint *ep, const void *data, size_t len)
>  
>  	skb_put_data(skb, data + hdrlen, size);
>  
> -	qrtr_node_assign(node, cb->src_node);
> +	if (node->ep_nid == QRTR_EP_NID_AUTO)
> +		WRITE_ONCE(node->ep_nid, cb->src_node);

[Severity: Medium]
Is there anything here that ties this first packet's src_node to the
endpoint's own node, rather than to a bridged node the endpoint carries?

If the first accepted packet has a bridged source B, ep_nid becomes B for
the lifetime of the link, and no later path corrects it. After that,
qrtr_node_enqueue() addresses control packets to B, data addressed to
node->nid is rewritten to B, and packets from the endpoint's real id E are
neither translated (E != ep_nid) nor inserted into qrtr_nodes by the new
guard below. Should this latch only accept the id seen in the remote's own
HELLO?

[Severity: Medium]
This is a check-then-set outside any lock, while the sibling field is
written under qrtr_nodes_lock in qrtr_node_assign():

	spin_lock_irqsave(&qrtr_nodes_lock, flags);
	radix_tree_insert(&qrtr_nodes, nid, node);
	if (node->nid == QRTR_EP_NID_AUTO)
		WRITE_ONCE(node->nid, nid);

qrtr_endpoint_post() is entered without serialization from
net/qrtr/tun.c:qrtr_tun_write_iter():

	ret = qrtr_endpoint_post(&tun->ep, kbuf, len);

Two concurrent writes to the same /dev/qrtr-tun fd can both observe the
sentinel and both store, with different src_node values.

There is a second ordering question: ep_nid is published before
qrtr_node_assign() sets node->nid, so can a concurrent caller see
ep_nid == E while node->nid is still QRTR_EP_NID_AUTO and then execute

	if (cb->src_node == node->ep_nid)
		cb->src_node = node->nid;

delivering src_node 0xffffffff, or have qrtr_node_rewrite_ctrl() stamp
0xffffffff into a NEW_SERVER/DEL_SERVER/DEL_CLIENT payload?

> +
> +	if (node->nid == QRTR_EP_NID_AUTO || node->nid == cb->src_node)
> +		qrtr_node_assign(node, cb->src_node);

[Severity: High]
Does this stop learning routes to bridged (distant) nodes?

Previously every src_node was inserted into qrtr_nodes unconditionally, and
qrtr_node_assign() inserts the key regardless of node->nid:

	spin_lock_irqsave(&qrtr_nodes_lock, flags);
	radix_tree_insert(&qrtr_nodes, nid, node);
	if (node->nid == QRTR_EP_NID_AUTO)
		WRITE_ONCE(node->nid, nid);

With the new guard, once node->nid == E a packet with src_node == B
(B != E) inserts nothing unless it happens to be QRTR_TYPE_NEW_SERVER. Then:

  - qrtr_sendmsg() replying to sq_node == B hits the
    qrtr_node_lookup() failure path and returns -ECONNRESET
  - qrtr_send_resume_tx() for confirm_rx data received from B does:

	node = qrtr_node_lookup(remote.sq_node);
	if (!node)
		return -EINVAL;

    so the bridged sender never gets its credit back

  - the name service cannot answer NEW_LOOKUP/DEL_* traffic from B

This path is reached with the existing endpoint drivers that all pass
QRTR_EP_NID_AUTO, since node->nid is set from the first packet. Does that
match the commit message statement?

    Currently, all QRTR endpoint drivers pass 'QRTR_EP_NID_AUTO' as the
    'nid' during qrtr_endpoint_register(). So 'ep_nid' and 'nid' are same
    and the functionality is not changed.

>  
>  	if (cb->type == QRTR_TYPE_NEW_SERVER) {
>  		/* Remote node endpoint can bridge other distant nodes */
> -		const struct qrtr_ctrl_pkt *pkt;
> +		const struct qrtr_ctrl_pkt *pkt = data + hdrlen;
> +		unsigned int server_node = le32_to_cpu(pkt->server.node);
>  
> -		pkt = data + hdrlen;
> -		qrtr_node_assign(node, le32_to_cpu(pkt->server.node));
> +		if (server_node != node->ep_nid)
> +			qrtr_node_assign(node, server_node);
>  	}
>  
> +	if (cb->src_node == node->ep_nid)
> +		cb->src_node = node->nid;
> +
> +	if (cb->type == QRTR_TYPE_NEW_SERVER ||
> +	    cb->type == QRTR_TYPE_DEL_SERVER ||
> +	    cb->type == QRTR_TYPE_DEL_CLIENT)
> +		qrtr_node_rewrite_ctrl(node, cb->type, skb);
> +
>  	if (cb->type == QRTR_TYPE_RESUME_TX) {
>  		qrtr_tx_resume(node, skb);
>  	} else {

[Severity: High]
Should QRTR_TYPE_RESUME_TX also be translated here?

qrtr_node_enqueue() keys the flow on to->sq_node, i.e. the host assigned
node->nid:

qrtr_tx_wait():
	unsigned long key = (u64)dest_node << 32 | dest_port;

but it puts node->ep_nid on the wire as hdr->dst_node_id. The remote echoes
its own view of the destination back in RESUME_TX, which is ep_nid; the
host does the same thing in qrtr_send_resume_tx():

	pkt->cmd = cpu_to_le32(QRTR_TYPE_RESUME_TX);
	pkt->client.node = cpu_to_le32(cb->dst_node);

RESUME_TX is not in the list above, and qrtr_tx_resume() consumes the raw
payload:

	u64 remote_node = le32_to_cpu(pkt->client.node);
	u32 remote_port = le32_to_cpu(pkt->client.port);
	...
	key = remote_node << 32 | remote_port;
	flow = xa_load(&node->qrtr_tx_flow, key);

With node->nid != node->ep_nid the lookup uses ep_nid << 32 | port while
the flow was stored under nid << 32 | port, so flow->pending is never
reset. Once QRTR_TX_FLOW_HIGH messages are outstanding, does every further
send then block indefinitely in qrtr_tx_wait()?

	ret = wait_event_interruptible_locked_irq(flow->resume_tx,
						  flow->pending < QRTR_TX_FLOW_HIGH ||
						  flow->tx_failed ||
						  !node->ep);

There is no timeout, and flow->tx_failed is only set on transmit failure.
This would be hit by ordinary QMI bring-up traffic to an MHI endpoint once
that endpoint registers with a host assigned nid.

> @@ -646,6 +692,7 @@ int qrtr_endpoint_register(struct qrtr_endpoint *ep, unsigned int nid)
>  	mutex_init(&node->ep_lock);
>  	skb_queue_head_init(&node->rx_queue);
>  	node->nid = QRTR_EP_NID_AUTO;
> +	node->ep_nid = QRTR_EP_NID_AUTO;
>  	node->ep = ep;

[Severity: High]
Now that the host supplied nid is authoritative, should the insertion
failure be propagated out of qrtr_endpoint_register()?

qrtr_node_assign() drops the return value and commits node->nid anyway:

	spin_lock_irqsave(&qrtr_nodes_lock, flags);
	radix_tree_insert(&qrtr_nodes, nid, node);
	if (node->nid == QRTR_EP_NID_AUTO)
		WRITE_ONCE(node->nid, nid);
	spin_unlock_irqrestore(&qrtr_nodes_lock, flags);

radix_tree_insert() does not overwrite, so on -EEXIST the key keeps
pointing at a different node while the new node believes it owns nid;
-ENOMEM behaves the same. qrtr_endpoint_register() then returns 0 and the
driver is never told its requested id was rejected.

The colliding key can come from the remote: a packet with a matching
src_node while node->nid is still AUTO, or a NEW_SERVER payload whose
server.node matches, both of which are full 32-bit values from the v1
header:

	if (server_node != node->ep_nid)
		qrtr_node_assign(node, server_node);

After that, the new guard

	if (node->nid == QRTR_EP_NID_AUTO || node->nid == cb->src_node)
		qrtr_node_assign(node, cb->src_node);

prevents any later recovery insertion, so qrtr_node_lookup(nid) keeps
resolving to the wrong node for the endpoint's lifetime. Isn't that the
same silent-drop failure mode the commit message says this change removes?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260921-qrtr-multi-ep-v2-0-27dd80d841a0%40oss.qualcomm.com

Re: [PATCH v2 2/4] net: qrtr: Assign unique node id for MHI endpoints

[netdev-bot+sashiko]

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

Critical: 0 · High: 2 · Medium: 3 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] TX flow-control (QRTR_TYPE_RESUME_TX) breaks for MHI endpoints:
  the flow is stored under a key built from the host-assigned node id,…
- [High] The new header asserts an invariant that nothing enforces: 'The
  offset is chosen carefully to not collide with the node ids allocated…
- [Medium] The QRTR_EP_NID_AUTO sentinel (-1, i.e. 0xffffffff ==
  QRTR_NODE_BCAST) can now be emitted as the wire destination node id of…
- [Medium] qrtr_endpoint_register() reports success even when the
  requested host node id was not published in the qrtr_nodes lookup tree.
- [Medium] Node-id reuse races against the deferred removal of radix-tree
  entries, silently leaving a re-probed MHI endpoint unreachable.

--

net: qrtr: Assign unique node id for MHI endpoints

This adds include/net/qrtr.h with QRTR_NODE_HOST_BASE (0x10000) and a
qrtr_host_node_id() helper, then switches the MHI QRTR endpoint driver from
QRTR_EP_NID_AUTO to a host assigned node id derived from the MHI controller
index.  It also adds the new header to the QRTR MAINTAINERS entry.

[ ... ]

> diff --git a/include/net/qrtr.h b/include/net/qrtr.h
> new file mode 100644
> index 0000000000000..745d51b4a6ba6
> --- /dev/null
> +++ b/include/net/qrtr.h
> @@ -0,0 +1,21 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef _NET_QRTR_H
> +#define _NET_QRTR_H
> +
> +#include <linux/types.h>
> +
> +/* The offset is chosen carefully to not collide with the node ids allocated by
> + * the remote nodes. All the remote nodes use node ids in range 0 to 0xffff.
> + */
> +#define QRTR_NODE_HOST_BASE 0x10000u

[Severity: High]
Is the "All the remote nodes use node ids in range 0 to 0xffff" statement
enforced anywhere in the receive path?  The v1 parser in
qrtr_endpoint_post() imports an unrestricted 32 bit value:

	cb->src_node = le32_to_cpu(v1->src_node_id);

and the NEW_SERVER branch hands a fully wire controlled id to
qrtr_node_assign() with no range test and no nid precondition:

	if (node->nid == QRTR_EP_NID_AUTO || node->nid == cb->src_node)
		qrtr_node_assign(node, cb->src_node);

	if (cb->type == QRTR_TYPE_NEW_SERVER) {
		const struct qrtr_ctrl_pkt *pkt = data + hdrlen;
		unsigned int server_node = le32_to_cpu(pkt->server.node);

		if (server_node != node->ep_nid)
			qrtr_node_assign(node, server_node);
	}

qrtr_node_assign() then discards the insertion result while still
committing node->nid:

	spin_lock_irqsave(&qrtr_nodes_lock, flags);
	radix_tree_insert(&qrtr_nodes, nid, node);
	if (node->nid == QRTR_EP_NID_AUTO)
		WRITE_ONCE(node->nid, nid);
	spin_unlock_irqrestore(&qrtr_nodes_lock, flags);

So can a remote (firmware on MHI device 0, or a qrtr-tun endpoint) claim a
key in the new host range, say 0x10001, before the controller with index 1
probes?  When that controller registers, radix_tree_insert() returns
-EEXIST, it is ignored, node->nid is set to 0x10001 anyway, and every
qrtr_node_lookup(0x10001) returns the squatting node instead.  In the
reverse order the squatter's packets get rewritten to appear to come from
another device's host id.

Since the host ids are now derived from probe order, they are predictable.
Should ids >= QRTR_NODE_HOST_BASE be rejected on receive, and should the
radix_tree_insert() error be propagated?

> +
> +/* Compute host node id from a per-device index. The index must be unique
> + * among the host assigned endpoints and smaller than QRTR_NODE_HOST_BASE.
> + */
> +static inline unsigned int qrtr_host_node_id(unsigned int index)
> +{
> +	return QRTR_NODE_HOST_BASE + index;
> +}
> +
> +#endif /* _NET_QRTR_H */

> diff --git a/net/qrtr/mhi.c b/net/qrtr/mhi.c
> index 3990da1a65dc4..c920ab61f8487 100644
> --- a/net/qrtr/mhi.c
> +++ b/net/qrtr/mhi.c

[ ... ]

> @@ -127,7 +128,8 @@ static int qcom_mhi_qrtr_probe(struct mhi_device *mhi_dev,
>  	if (rc)
>  		return rc;
>  
> -	rc = qrtr_endpoint_register(&qdev->ep, QRTR_EP_NID_AUTO);
> +	rc = qrtr_endpoint_register(&qdev->ep,
> +				    qrtr_host_node_id(mhi_dev->mhi_cntrl->index));
>  	if (rc)
>  		goto err_unprepare;

[Severity: High]
With a host assigned id, node->nid (0x10000 + index) differs permanently
from node->ep_nid (set from the wire, 0..0xffff).  Does TX flow control
still work in that configuration?

qrtr_node_enqueue() keys the flow on the host assigned id:

	confirm_rx = qrtr_tx_wait(node, to->sq_node, to->sq_port, type);

qrtr_tx_wait():
	unsigned long key = (u64)dest_node << 32 | dest_port;
	...
	flow = xa_load(&node->qrtr_tx_flow, key);

but the same enqueue path rewrites the wire destination to ep_nid:

	/* Put back the endpoint's own node id */
	dst_node = to->sq_node;
	if (dst_node == READ_ONCE(node->nid))
		dst_node = node->ep_nid;
	hdr->dst_node_id = cpu_to_le32(dst_node);

so the remote is addressed as ep_nid and names itself that way in the
RESUME_TX payload, the same way the kernel's own qrtr_send_resume_tx()
does with pkt->client.node = cpu_to_le32(cb->dst_node).

On receive, only NEW_SERVER, DEL_SERVER and DEL_CLIENT payload node ids get
translated, RESUME_TX is passed through untouched:

	if (cb->type == QRTR_TYPE_NEW_SERVER ||
	    cb->type == QRTR_TYPE_DEL_SERVER ||
	    cb->type == QRTR_TYPE_DEL_CLIENT)
		qrtr_node_rewrite_ctrl(node, cb->type, skb);

	if (cb->type == QRTR_TYPE_RESUME_TX) {
		qrtr_tx_resume(node, skb);

qrtr_tx_resume() therefore computes key = ep_nid << 32 | port:

	u64 remote_node = le32_to_cpu(pkt->client.node);
	u32 remote_port = le32_to_cpu(pkt->client.port);
	...
	key = remote_node << 32 | remote_port;

	flow = xa_load(&node->qrtr_tx_flow, key);
	if (flow) {

Doesn't that xa_load() always miss the flow stored under the nid based key,
leaving flow->pending never reset and no waiter woken?

After QRTR_TX_FLOW_HIGH (10) data messages to one remote port,
qrtr_tx_wait() would then sleep in wait_event_interruptible_locked_irq()
until a signal arrives or node->ep is cleared at unregister, which stalls
QMI traffic to the endpoint.  Should RESUME_TX payloads also be translated
from ep_nid to nid?

[Severity: Medium]
Can this rc check ever observe a failure to publish the requested id?
Passing a concrete nid means qrtr_node_assign() no longer returns at the
"if (nid == QRTR_EP_NID_AUTO) return;" guard, so radix_tree_insert() now
runs at probe time, and its return value is dropped while node->nid is
written unconditionally.

qrtr_nodes is RADIX_TREE(qrtr_nodes, GFP_ATOMIC) and the insert runs under
spin_lock_irqsave(), and keys >= 0x10000 need tree extension, so -ENOMEM is
possible, as is -EEXIST.  In both cases qrtr_endpoint_register() still adds
the node to qrtr_all_nodes, schedules the HELLO work and returns 0, so the
probe succeeds with an endpoint that qrtr_node_lookup() cannot find, and
sends to its id fail with -ECONNRESET in qrtr_sendmsg().

Nothing retries later, since the receive path only re-assigns when

	if (node->nid == QRTR_EP_NID_AUTO || node->nid == cb->src_node)
		qrtr_node_assign(node, cb->src_node);

and neither condition holds once node->nid is 0x10000 + index while the
remote reports its own low id.  Should qrtr_node_assign() return the
insertion error so qrtr_endpoint_register() can propagate it and the
err_unprepare path here can run?

[Severity: Medium]
Can the QRTR_EP_NID_AUTO sentinel now reach the wire as dst_node_id?

qrtr_endpoint_register() sets both ids to the sentinel and then publishes
the node immediately, because nid is no longer AUTO:

	node->nid = QRTR_EP_NID_AUTO;
	node->ep_nid = QRTR_EP_NID_AUTO;
	...
	qrtr_node_assign(node, nid);
	...
	schedule_delayed_work(&node->say_hello, 0);

So qrtr_node_lookup(0x10000 + index) succeeds, and node->hello_sent can
become true, before any packet has been received, i.e. before

	if (node->ep_nid == QRTR_EP_NID_AUTO)
		WRITE_ONCE(node->ep_nid, cb->src_node);

has run.  A sendmsg() in that window reaches

	dst_node = to->sq_node;
	if (dst_node == READ_ONCE(node->nid))
		dst_node = node->ep_nid;
	hdr->dst_node_id = cpu_to_le32(dst_node);

and writes 0xffffffff (QRTR_NODE_BCAST) as the destination node of a data
frame handed to ep->xmit().  Previously the node was not reachable through
qrtr_node_lookup() until the first receive had set both nid and ep_nid.
Should the enqueue path check ep_nid against QRTR_EP_NID_AUTO before using
it as a wire value?

[Severity: Medium]
The MHI controller index is recycled, in mhi_register_controller():

	mhi_cntrl->index = ida_alloc(&mhi_controller_ida, GFP_KERNEL);

and in mhi_unregister_controller():

	ida_free(&mhi_controller_ida, mhi_cntrl->index);

Does that race with the deferred removal of the radix tree slot?
qrtr_endpoint_unregister() only clears node->ep and drops a reference:

	mutex_lock(&node->ep_lock);
	node->ep = NULL;
	mutex_unlock(&node->ep_lock);
	...
	qrtr_node_release(node);

while the slots are deleted only in __qrtr_node_release():

	radix_tree_for_each_slot(slot, &qrtr_nodes, &iter, 0) {
		if (*slot == node)
			radix_tree_iter_delete(&qrtr_nodes, &iter, slot);
	}

qrtr_sendmsg() holds the reference from qrtr_node_lookup() across
qrtr_node_enqueue(), which sleeps in qrtr_tx_wait(), so the final put can
land after MHI remove() has returned and the index has been freed and
handed to the next controller:

CPU0 MHI remove
  qrtr_endpoint_unregister()	/* slot for 0x10000+N still present */
  mhi_unregister_controller()
    ida_free(N)

CPU1
  qrtr_node_enqueue() -> qrtr_tx_wait()	/* still holds old node ref */

CPU0/CPU2 new controller gets index N
  qcom_mhi_qrtr_probe()
    qrtr_endpoint_register(ep, 0x10000 + N)
      qrtr_node_assign() -> radix_tree_insert() == -EEXIST, ignored

Until CPU1 drops its reference, lookups of 0x10000 + N return the dead node
(node->ep == NULL).  After __qrtr_node_release() deletes the slot, is the
new endpoint permanently unreachable, given that the receive path only
re-assigns when node->nid == QRTR_EP_NID_AUTO or node->nid == cb->src_node?

With QRTR_EP_NID_AUTO the id was re-derived from every incoming packet, so
a collision healed itself.  Should the slot be removed synchronously in
qrtr_endpoint_unregister(), or the insert error be checked at register?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260921-qrtr-multi-ep-v2-0-27dd80d841a0%40oss.qualcomm.com

Re: [PATCH v2 3/4] wifi: ath11k: Connect to the QMI server belonging to the device owned by this driver

[netdev-bot+sashiko]

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

Critical: 0 · High: 1 · Medium: 0 · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] Incomplete fix: ath11k_qmi_ops_new_server() returns 0 (success)
  when it decides to skip a WLFW server that belongs to the *other*…
- [Low] Concern that the commit fixes a real user-visible multi-device
  initialization bug (identical ath11k PCIe cards connecting to each…

--

wifi: ath11k: Connect to the QMI server belonging to the device owned by this driver

This adds a get_qrtr_node_id() HIF callback returning the QRTR node id
derived from the MHI controller index, and uses it in the QMI new_server
callback so a device only connects to the WLFW server running on its own
node. A node id of zero keeps the previous unfiltered behaviour.

[Severity: Low]
This isn't a bug, but the commit fixes a user-visible misbehaviour (two
identical ath11k PCIe cards connecting to each other's WLFW firmware) and
carries no Fixes: trailer.

The fix depends on qrtr_host_node_id()/QRTR_NODE_HOST_BASE in
include/net/qrtr.h and on the host-assigned node id plus the
NEW_SERVER/DEL_SERVER node rewrite from the preceding net/qrtr patches, so
a Fixes: tag pointing at the original ath11k commit would invite stable
tooling to pick this up without those dependencies.

Would it help to say so explicitly in the commit message, for example
"depends on the preceding net/qrtr changes; not suitable for stable on its
own", instead of leaving the tag out silently?

> diff --git a/drivers/net/wireless/ath/ath11k/qmi.c b/drivers/net/wireless/ath/ath11k/qmi.c
> index 7dc07339b9579..dadf78f30b897 100644
> --- a/drivers/net/wireless/ath/ath11k/qmi.c
> +++ b/drivers/net/wireless/ath/ath11k/qmi.c
> @@ -3170,8 +3170,18 @@ static int ath11k_qmi_ops_new_server(struct qmi_handle *qmi_hdl,
>  	struct ath11k_qmi *qmi = container_of(qmi_hdl, struct ath11k_qmi, handle);
>  	struct ath11k_base *ab = qmi->ab;
>  	struct sockaddr_qrtr *sq = &qmi->sq;
> +	int node_id;
>  	int ret;
>  
> +	/* Identical devices advertise the same QMI services, so connect only to
> +	 * the QMI server on this device's node. A node id of 0 means the
> +	 * transport has not assigned a unique node id, so accept the server
> +	 * unfiltered.
> +	 */
> +	node_id = ath11k_hif_get_qrtr_node_id(ab);
> +	if (node_id && service->node != node_id)
> +		return 0;
> +

[Severity: High]
Should this return a negative errno rather than 0 when the server belongs
to the other device's node?

The QMI core only drops the service record when ->new_server() returns a
negative value:

drivers/soc/qcom/qmi_interface.c:qmi_recv_new_server() {
	...
	ret = ops->new_server(qmi, svc);
	if (ret < 0)
		kfree(svc);
	else
		list_add(&svc->list_node, &qmi->lookup_results);
}

So returning 0 here keeps the foreign node's WLFW service on this handle's
lookup_results list, even though we deliberately did not connect to it.

When that foreign server later goes away (sibling firmware crash, unbind of
the other card, or its MHI endpoint unregistering), net/qrtr/ns.c
server_del()/ctrl_cmd_bye() call lookup_notify() for every lookup matching
the service/instance, which reaches this device's handle:

drivers/soc/qcom/qmi_interface.c:qmi_recv_del_server() {
	list_for_each_entry_safe(svc, tmp, &qmi->lookup_results, list_node) {
		if (node != -1 && svc->node != node)
			continue;
		...
		if (ops->del_server)
			ops->del_server(qmi, svc);
}

ath11k_qmi_ops_del_server() does no node filtering and posts
ATH11K_QMI_EVENT_SERVER_EXIT unconditionally, which in
ath11k_qmi_driver_event_work() does:

		case ATH11K_QMI_EVENT_SERVER_EXIT:
			set_bit(ATH11K_FLAG_CRASH_FLUSH, &ab->dev_flags);
			set_bit(ATH11K_FLAG_RECOVERY, &ab->dev_flags);

			if (!ab->is_reset)
				ath11k_core_pre_reconfigure_recovery(ab);

Can this drive crash-flush and recovery on the healthy device whose own
firmware never went down?  Since the new filter also rejects the foreign
node's re-announcement, no matching SERVER_ARRIVE follows, so does that
adapter stay stuck with ATH11K_FLAG_CRASH_FLUSH and ATH11K_FLAG_RECOVERY
set until the driver is reloaded?

Would returning a negative errno here (so the core frees the record), and/or
adding the same node id check to ath11k_qmi_ops_del_server(), cover the
del_server side of the cross-device confusion as well?

>  	sq->sq_family = AF_QIPCRTR;
>  	sq->sq_node = service->node;
>  	sq->sq_port = service->port;

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260921-qrtr-multi-ep-v2-0-27dd80d841a0%40oss.qualcomm.com

Re: [PATCH v2 4/4] wifi: ath12k: Connect to the QMI server belonging to the device owned by this driver

[netdev-bot+sashiko]

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

Critical: 0 · High: 1 · Medium: 0 (1) · Low: 1

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] Incomplete node-id filtering in ath12k_qmi_ops_new_server(): the
  foreign device's WLFW server is "skipped" by returning 0, which the…
- [Low] Concern as filed: the patch lacks a Fixes: tag and silently
  depends on a QRTR change that registers MHI endpoints with…

Pre-existing issues:
- [Medium] Pre-existing resource leak (unrelated to this patch's change):
  ath12k_qmi_init_service()…

--

wifi: ath12k: Connect to the QMI server belonging to the device owned by this driver

This adds a get_qrtr_node_id() HIF callback that returns the QRTR node id
derived from the MHI controller index, and uses it in
ath12k_qmi_ops_new_server() so a device only connects to the WLFW server
running on its own node. A node id of zero keeps the previous behaviour for
transports that do not assign one.

> diff --git a/drivers/net/wireless/ath/ath12k/pci.c b/drivers/net/wireless/ath/ath12k/pci.c
> index 6441927b53822..2b1b122bbb3ba 100644
> --- a/drivers/net/wireless/ath/ath12k/pci.c
> +++ b/drivers/net/wireless/ath/ath12k/pci.c

[ ... ]

> @@ -1491,6 +1492,13 @@ static int ath12k_pci_panic_handler(struct ath12k_base *ab)
>  	return NOTIFY_OK;
>  }
>  
> +static int ath12k_pci_get_qrtr_node_id(struct ath12k_base *ab)
> +{
> +	struct ath12k_pci *ab_pci = ath12k_pci_priv(ab);
> +
> +	return qrtr_host_node_id(ab_pci->mhi_ctrl->index);
> +}

[Severity: Low]
This isn't a bug, but could the commit message name the prerequisite change
this depends on?

On PCIe this helper always returns a non-zero id
(QRTR_NODE_HOST_BASE + index), so the new filter in
ath12k_qmi_ops_new_server() is unconditionally active, and it only matches
because net/qrtr/mhi.c now registers endpoints with the same id:

net/qrtr/mhi.c:qcom_mhi_qrtr_probe() {
	rc = qrtr_endpoint_register(&qdev->ep,
				    qrtr_host_node_id(mhi_dev->mhi_cntrl->index));
}

That registration comes from "net: qrtr: Assign unique node id for MHI
endpoints" in this same series, and the message only says "But now, QRTR
provides each MHI endpoint a unique node id". Without the QRTR patch, this
helper returns an id that matches no advertised server and every WLFW
server would be skipped, so spelling out the ordering requirement (and that
this must not be backported alone) would help future backporters.

[ ... ]

> diff --git a/drivers/net/wireless/ath/ath12k/qmi.c b/drivers/net/wireless/ath/ath12k/qmi.c
> index 280e50a1f31d8..22b8cbbbf4f45 100644
> --- a/drivers/net/wireless/ath/ath12k/qmi.c
> +++ b/drivers/net/wireless/ath/ath12k/qmi.c

[ ... ]

> @@ -3940,8 +3941,18 @@ static int ath12k_qmi_ops_new_server(struct qmi_handle *qmi_hdl,
>  	struct ath12k_qmi *qmi = container_of(qmi_hdl, struct ath12k_qmi, handle);
>  	struct ath12k_base *ab = qmi->ab;
>  	struct sockaddr_qrtr *sq = &qmi->sq;
> +	int node_id;
>  	int ret;
>  
> +	/* Identical devices advertise the same QMI services, so connect only to
> +	 * the QMI server on this device's node. A node id of 0 means the
> +	 * transport has not assigned a unique node id, so accept the server
> +	 * unfiltered.
> +	 */
> +	node_id = ath12k_hif_get_qrtr_node_id(ab);
> +	if (node_id && service->node != node_id)
> +		return 0;
> +

[Severity: High]
Should this return a negative errno instead of 0 for the foreign node?

The QMI core only discards the service when new_server() returns negative:

drivers/soc/qcom/qmi_interface.c:qmi_recv_new_server() {
	ret = ops->new_server(qmi, svc);
	if (ret < 0)
		kfree(svc);
	else
		list_add(&svc->list_node, &qmi->lookup_results);
}

With "return 0", the other device's server stays recorded in
qmi->lookup_results on this device's handle, even though this device never
connected to it.

The teardown half of the lifecycle has no node id check. When the peer
device's firmware dies or the peer is unbound, net/qrtr/ns.c:server_del()
notifies every lookup matching service and instance, with no node
filtering:

net/qrtr/ns.c:server_del() {
	list_for_each(li, &qrtr_ns.lookups) {
		lookup = container_of(li, struct qrtr_lookup, li);
		if (lookup->service && lookup->service != srv->service)
			continue;
		if (lookup->instance && lookup->instance != srv->instance)
			continue;

		lookup_notify(&lookup->sq, srv, false);
	}
}

qmi_recv_del_server() then finds the retained foreign entry (also reached
with node/port == -1 from qmi_handle_net_reset() and qmi_handle_release())
and calls into the driver:

drivers/net/wireless/ath/ath12k/qmi.c:ath12k_qmi_ops_del_server() {
	ath12k_dbg(ab, ATH12K_DBG_QMI, "qmi wifi fw del server\n");
	ath12k_qmi_driver_event_post(qmi, ATH12K_QMI_EVENT_SERVER_EXIT, NULL);
}

so this healthy device gets a SERVER_EXIT with no matching ARRIVE:

drivers/net/wireless/ath/ath12k/qmi.c:ath12k_qmi_driver_event_work() {
		case ATH12K_QMI_EVENT_SERVER_EXIT:
			set_bit(ATH12K_FLAG_CRASH_FLUSH, &ab->dev_flags);
}

Can that latch the healthy device into the crash-flush state permanently?
ATH12K_FLAG_CRASH_FLUSH makes ath12k_dp_tx() and management TX return
-ESHUTDOWN, and ath12k_wmi_cmd_send()/ath12k_ce_send() fail the same way:

drivers/net/wireless/ath/ath12k/wifi7/dp_tx.c:ath12k_dp_tx() {
	if (test_bit(ATH12K_FLAG_CRASH_FLUSH, &ab->dev_flags))
		return -ESHUTDOWN;
}

The bit is only cleared on this device's own FW_READY or core start, and
neither happens because this device's firmware never crashed.

Would returning a negative errno (so the core never records the foreign
service) plus the same node id check in ath12k_qmi_ops_del_server() cover
both halves?

[Severity: Medium]
This isn't a bug introduced by this patch, but while looking at the QMI
handle lifecycle in the same file: does ath12k_qmi_init_service() leak the
QMI handle on its two failure paths after qmi_handle_init() succeeded?

drivers/net/wireless/ath/ath12k/qmi.c:ath12k_qmi_init_service() {
	ab->qmi.event_wq = alloc_ordered_workqueue("ath12k_qmi_driver_event", 0);
	if (!ab->qmi.event_wq) {
		ath12k_err(ab, "failed to allocate workqueue\n");
		return -EFAULT;
	}
	...
	if (ret < 0) {
		ath12k_warn(ab, "failed to add qmi lookup\n");
		destroy_workqueue(ab->qmi.event_wq);
		return ret;
	}
}

Neither path calls qmi_handle_release(), so qmi->recv_buf, the
"qmi_msg_handler" workqueue and the kernel QRTR socket created by
qmi_handle_init() stay allocated.

ath12k_core_soc_create() returns before its err_qmi_deinit label, and the
unwind loop in ath12k_core_hw_group_create() skips the device that just
failed:

drivers/net/wireless/ath/ath12k/core.c:ath12k_core_hw_group_create() {
destroy:
	for (i--; i >= 0; i--) {
		ab = ag->ab[i];
		...
		ath12k_core_soc_destroy(ab);
}

Probe then reaches ath12k_core_free() which kfree()s ab, while the leaked
socket still holds sk->sk_user_data pointing at &ab->qmi:

drivers/soc/qcom/qmi_interface.c:qmi_sock_create() {
	sock->sk->sk_user_data = qmi;
	sock->sk->sk_data_ready = qmi_data_ready;
}

Both paths require an allocation failure at probe time, so this is a cold
path only.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260921-qrtr-multi-ep-v2-0-27dd80d841a0%40oss.qualcomm.com