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

[PATCH v3 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/12k setup by backporting this series to
6.6 kernel. 

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 v3:
- Added missing qrtr_node_rewrite_ctrl() for QRTR_TYPE_RESUME_TX
- Fixed the types of ath{11k/12k}_pci_get_qrtr_node_id() and helpers to use
  'unsigned int' to match qrtr_host_node_id()
- Collected tags
- Link to v2: https://patch.msgid.link/20260921-qrtr-multi-ep-v2-0-27dd80d841a0@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                    | 62 +++++++++++++++++++++++++++++++----
 net/qrtr/mhi.c                        |  4 ++-
 10 files changed, 137 insertions(+), 8 deletions(-)
---
base-commit: 8830e65ed46de41f849eefb8ba227d4852c460f6
change-id: 20260918-qrtr-multi-ep-c27c2bc54ee1

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

[PATCH v3 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 | 62 ++++++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 55 insertions(+), 7 deletions(-)

diff --git a/net/qrtr/af_qrtr.c b/net/qrtr/af_qrtr.c
index 78347c937af7..6387b85a7a2c 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 || type == QRTR_TYPE_RESUME_TX)
+		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,30 @@ 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 ||
+	    cb->type == QRTR_TYPE_RESUME_TX)
+		qrtr_node_rewrite_ctrl(node, cb->type, skb);
+
 	if (cb->type == QRTR_TYPE_RESUME_TX) {
 		qrtr_tx_resume(node, skb);
 	} else {
@@ -646,6 +693,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 v3 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 v3 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.

Tested-on: 2x WCN6855 hw2.1 PCI WLAN.HSP.1.1-04685-QCAHSPSWPL_V1_V2_SILICONZ_IOE-1

Tested-by: Juha-Matti Tilli <juha-matti.tilli@iki.fi>
Reviewed-by: Vasanthakumar Thiagarajan <vasanthakumar.thiagarajan@oss.qualcomm.com>
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..f432fa4890be 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);
+	unsigned 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 unsigned 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..d40c057e130c 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 unsigned 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..9484b0eee1a9 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;
+	unsigned 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 v3 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.

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

Tested-by: Juha-Matti Tilli <juha-matti.tilli@iki.fi>
Reviewed-by: Vasanthakumar Thiagarajan <vasanthakumar.thiagarajan@oss.qualcomm.com>
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..b8592c3d84e7 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);
+	unsigned 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 unsigned 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..9b6108c1abdc 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 unsigned 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..e1e75cc9d144 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;
+	unsigned 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 v3 4/4] wifi: ath12k: Connect to the QMI server belonging to the device owned by this driver

[Raj Kumar Bhagat]

On 22-09-2026 19:11, 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-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
> 
> Tested-by: Juha-Matti Tilli<juha-matti.tilli@iki.fi>
> Reviewed-by: Vasanthakumar Thiagarajan<vasanthakumar.thiagarajan@oss.qualcomm.com>
> Signed-off-by: Manivannan Sadhasivam<manivannan.sadhasivam@oss.qualcomm.com>

Tested-on: QCN9274 hw2.0 PCI WLAN.WBE.1.6-01243-QCAHKSWPL_SILICONZ-1
Tested-by: Raj Kumar Bhagat <raj.bhagat@oss.qualcomm.com>

Validated in ipq9574-rdp433 with three QCN9274 PCIe devices.

Re: [PATCH v3 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:41, 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-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
> 
> Tested-by: Juha-Matti Tilli <juha-matti.tilli@iki.fi>
> Reviewed-by: Vasanthakumar Thiagarajan <vasanthakumar.thiagarajan@oss.qualcomm.com>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

Ok, you can retain my Tested-by tag despite the changes to v3.

Backported to kernel 6.18.53, all regdb issues are gone, and when I
attached antennas to the secondary WiFi card, the statistics fetching
dmesg issues are gone now.

I have two cards, both of them work simultaneously, on the v3 patchset.

The only remaining issue I have is this on the access point card:

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

Google's AI (Gemini) tells me it's not worrying if the connection works
(like it does for me).

Thank you for the effort! I'll take the 6.18.53 LTS version into use
with these patches.

BR, Juha-Matti

Re: [PATCH v3 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 4 potential
issue(s) to consider.

Critical: 0 · High: 2 · Medium: 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] qrtr_endpoint_post() (net/qrtr/af_qrtr.c:570) replaces the
  previously unconditional `qrtr_node_assign(node, cb->src_node)` with…
- [High] net: qrtr: bypass of node ID rewrite for short DEL_CLIENT
  packets
- [Medium] The lazy initialization of node->ep_nid in
  qrtr_endpoint_post() is an unlocked test-and-set: `if (node->ep_nid ==…
- [Low] The new gate `if (node->nid == QRTR_EP_NID_AUTO || node->nid ==
  cb->src_node)` at af_qrtr.c:570 reads node->nid with plain loads…

--

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

This adds a per-node 'ep_nid' field holding the node id as received from
the remote endpoint, keeps 'nid' as the host-assigned radix tree key, and
translates between the two in qrtr_endpoint_post() and
qrtr_node_enqueue(). A few questions below about the changes to the node
id learning logic and about the locking around the new field.

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

[ ... ]

> @@ -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 {
> +		/* 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 || type == QRTR_TYPE_RESUME_TX)
> +		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);
> +}
> +

[Severity: High]
Does this length check skip the rewrite for DEL_CLIENT packets that
carry only the client part of the union?

sizeof(struct qrtr_ctrl_pkt) is 20 bytes because of the server member.
A DEL_CLIENT only needs cmd, client.node and client.port, which is 12
bytes. qrtr_endpoint_post() enforces the full struct size only for
QRTR_TYPE_NEW_SERVER and QRTR_TYPE_RESUME_TX. A 12 byte DEL_CLIENT from
the remote therefore passes validation and is queued to the control
port, but this function returns before it touches pkt->client.node.

qrtr_endpoint_post() has already translated cb->src_node from ep_nid to
nid a few lines earlier. The name service then sees a sender that does
not match the payload:

net/qrtr/ns.c:ctrl_cmd_del_client() {
	...
	/* Don't accept spoofed messages */
	if (from->sq_node != node_id)
		return -EINVAL;
	...
}

Here from->sq_node is node->nid and node_id is still node->ep_nid, so
the command is rejected. As a result:

- the lookups owned by that remote port are never removed;
- the server registered on that port under nid is never deleted;
- local servers are never told that the remote client went away.

Local clients keep being pointed at a server that no longer exists.
The packet length is chosen by the remote firmware or by a qrtr-tun
writer, so this does not depend on well-behaved peers.

This only matters once an endpoint registers with a nid that differs
from the id the remote uses, which is what this series enables.

Would it be better to check against the end of the member being
rewritten? That would mean offsetofend(struct qrtr_ctrl_pkt,
client.node) for DEL_CLIENT and RESUME_TX, and offsetofend(struct
qrtr_ctrl_pkt, server.node) for the server types.

Or, if short DEL_CLIENT packets are not supposed to exist, should
qrtr_endpoint_post() reject them up front instead of delivering an
untranslated payload to the control port?

[ ... ]

> @@ -531,16 +564,30 @@ 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 this test-and-set of node->ep_nid safe against concurrent callers?

node->nid is initialized under qrtr_nodes_lock inside qrtr_node_assign(),
but ep_nid is written here with no lock, and the two fields are never
published as a pair. qrtr_endpoint_post() is an exported entry point that
qrtr_tun_write_iter() in net/qrtr/tun.c calls straight from write(2):

net/qrtr/tun.c:qrtr_tun_write_iter() {
	...
	ret = qrtr_endpoint_post(&tun->ep, kbuf, len);
	...
}

Two threads writing the same /dev/qrtr-tun descriptor reach this on the
same qrtr_node with no serialization:

CPU0 qrtr_endpoint_post(src=A)
    reads node->ep_nid == QRTR_EP_NID_AUTO

CPU1 qrtr_endpoint_post(src=B)
    reads node->ep_nid == QRTR_EP_NID_AUTO
    WRITE_ONCE(node->ep_nid, B)
    qrtr_node_assign(node, B) -> node->nid = B

CPU0
    WRITE_ONCE(node->ep_nid, A)

Doesn't that leave (nid = B, ep_nid = A) permanently for an endpoint that
passed QRTR_EP_NID_AUTO, which then switches on all the translation paths
-- the cb->src_node relabelling below, qrtr_node_rewrite_ctrl(), and the
dst_node rewrite in qrtr_node_enqueue()? The enqueue path would then put
ep_nid on the wire for a destination that the peer knows as nid.

Note node->ep_lock cannot be used for this, since qrtr_endpoint_post() is
also called from atomic context (net/qrtr/mhi.c downlink callback), so the
pair would have to be published under the IRQ-safe qrtr_nodes_lock or with
a cmpxchg.

On the reader side, the patch mixes annotations for the same field:
qrtr_node_enqueue() uses READ_ONCE(node->ep_nid) in the control-port case
but a plain load in the dst_node case, and qrtr_node_rewrite_ctrl() loads
ep_nid twice without annotation:

	if (node->nid == node->ep_nid)
		return;
	...
	if (le32_to_cpu(*nid) == node->ep_nid)
		*nid = cpu_to_le32(node->nid);

Can a concurrent update between those two loads let the function pass the
equality test and then rewrite the payload with a value inconsistent with
the cb->src_node translation done a few lines earlier?

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

[Severity: High]
Does this gate drop the src_node based learning of bridged nodes?

Before this patch the call was unconditional, so every distinct src_node
seen on an endpoint was inserted into qrtr_nodes. Once node->nid is
latched -- which for a QRTR_EP_NID_AUTO endpoint happens on the very first
received packet, and for a host-assigned endpoint already happens in
qrtr_endpoint_register() -- the condition can only be true for that one
id, so no further source node id is ever inserted.

That src_node learning came in with commit e04df98adf7d7d, and the
multi-key-per-node model is still assumed elsewhere in the file:

net/qrtr/af_qrtr.c:__qrtr_node_release() {
	...
	/* If the node is a bridge for other nodes, there are possibly
	 * multiple entries pointing to our released node, delete them all.
	 */
	...
}

The QRTR_TYPE_NEW_SERVER branch below only learns pkt->server.node, so a
bridged node that never announces a server is never inserted. For such a
node, what happens to a local socket that already received a packet from
it? qrtr_recvmsg() hands that sq_node to userspace, but the reply path
looks the id up again:

net/qrtr/af_qrtr.c:qrtr_sendmsg() {
	...
		node = qrtr_node_lookup(addr->sq_node);
		if (!node) {
			release_sock(sk);
			return -ECONNRESET;
		}
	...
}

And if the received data had confirm_rx set, qrtr_recvmsg() ->
qrtr_send_resume_tx() hits the same lookup:

net/qrtr/af_qrtr.c:qrtr_send_resume_tx() {
	...
	node = qrtr_node_lookup(remote.sq_node);
	if (!node)
		return -EINVAL;
	...
}

so no QRTR_TYPE_RESUME_TX goes out and the remote's pending counter stays
at the high watermark, stalling transmission.

Given that, is the commit message statement still accurate?

    "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."

The ids here are learned from remote packets, so this looks reachable with
ordinary traffic on the existing tun, smd and mhi endpoints.

[Severity: Low]
Should this decision be made inside qrtr_nodes_lock instead of as an
unlocked pre-check?

node->nid is written under qrtr_nodes_lock with WRITE_ONCE in
qrtr_node_assign(), and the other lockless readers in this file use
READ_ONCE, but the gate here uses plain loads and the check is not atomic
with the radix_tree_insert() plus nid latch it guards. With
node->nid == QRTR_EP_NID_AUTO and two concurrent qrtr_endpoint_post()
calls on the same node:

CPU0 (src=3)                      CPU1 (src=7)
  gate: nid == AUTO -> true         gate: nid == AUTO -> true
  qrtr_node_assign(node, 3)
    radix_tree_insert(.., 3, node)
    node->nid = 3
                                    qrtr_node_assign(node, 7)
                                      radix_tree_insert(.., 7, node)
                                      (inner re-check leaves nid = 3)

Doesn't that alias one node under two keys again and permanently occupy
key 7 -- radix_tree_insert()'s -EEXIST return is discarded -- so a
genuinely distinct remote with id 7 can no longer be inserted? That is the
aliasing the change is meant to prevent. Moving the test inside
qrtr_node_assign() under the spinlock would also remove the plain reads of
a WRITE_ONCE-written field that KCSAN would flag.

>  
>  	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 ||
> +	    cb->type == QRTR_TYPE_RESUME_TX)
> +		qrtr_node_rewrite_ctrl(node, cb->type, skb);
> +
>  	if (cb->type == QRTR_TYPE_RESUME_TX) {
>  		qrtr_tx_resume(node, skb);
>  	} else {

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260922-qrtr-multi-ep-v3-0-c00831e88957%40oss.qualcomm.com

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

[netdev-bot+sashiko]

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

Critical: 0 · High: 2 · Medium: 0 · 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] The reserved host node-id namespace introduced by this patch
  (`QRTR_NODE_HOST_BASE` = 0x10000, `qrtr_host_node_id()`) is only…
- [High] `qrtr_endpoint_register()` now performs the `qrtr_nodes` radix-
  tree insertion at registration time (because net/qrtr/mhi.c:131-132 is…

--

net: qrtr: Assign unique node id for MHI endpoints

This adds a new header, include/net/qrtr.h, defining QRTR_NODE_HOST_BASE
(0x10000) and a helper qrtr_host_node_id() that derives a host assigned
node id from a per-device index, and switches the MHI endpoint driver to
register with qrtr_host_node_id(mhi_dev->mhi_cntrl->index) instead of
QRTR_EP_NID_AUTO.

> 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 "range 0 to 0xffff" property actually enforced anywhere, or is it
only a convention documented in this comment?

In qrtr_endpoint_post() the v1 path takes the full 32-bit id straight from
the wire with no upper bound:

net/qrtr/af_qrtr.c:qrtr_endpoint_post() {
	...
	cb->src_node = le32_to_cpu(v1->src_node_id);
	...
}

and the NEW_SERVER handling will insert any 32-bit value into the global
qrtr_nodes tree:

net/qrtr/af_qrtr.c:qrtr_endpoint_post() {
	...
	if (cb->type == QRTR_TYPE_NEW_SERVER) {
		/* Remote node endpoint can bridge other distant nodes */
		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);
	}
	...
}

Can a remote endpoint (firmware over MHI, or a writer on /dev/qrtr-tun,
whose bytes reach qrtr_endpoint_post() unmodified via
qrtr_tun_write_iter()) therefore claim 0x10000 + controller_index before or
after the genuine MHI endpoint registers?

qrtr_node_assign() drops the insert result while still setting node->nid:

net/qrtr/af_qrtr.c: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);
	spin_unlock_irqrestore(&qrtr_nodes_lock, flags);
}

so if the id is already taken, does qrtr_node_lookup() resolve it to the
squatting node, sending unicast qrtr_sendmsg() and name service traffic
aimed at the host assigned id to the wrong endpoint while the real one
believes it owns that id?

Note also that qrtr_node_rewrite_ctrl() only rewrites ids equal to
node->ep_nid, so a bridged server.node of 0x10000 + k reaches the name
service unchanged and is stored as a server on that node id.

This matters for the rest of the series: the ath11k and ath12k changes use
the host node id as the device identity filter, for example

drivers/net/wireless/ath/ath12k/qmi.c:ath12k_qmi_ops_new_server() {
	...
	node_id = ath12k_hif_get_qrtr_node_id(ab);
	if (node_id && service->node != node_id)
		return 0;
	...
}

If the id can be advertised by another endpoint, does the "device may
connect to the firmware running on the other device" case the series aims
to close remain reachable?  Would rejecting remote supplied ids >=
QRTR_NODE_HOST_BASE in qrtr_endpoint_post()/qrtr_node_assign() make the
documented invariant real?

> +
> +/* 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]
This is the first caller to pass a concrete nid rather than
QRTR_EP_NID_AUTO, which moves the qrtr_nodes insertion into the
registration path.  Should the failure of that insertion be detected here?

qrtr_endpoint_register() calls qrtr_node_assign() and ignores the outcome,
then publishes the endpoint and returns 0 unconditionally:

net/qrtr/af_qrtr.c:qrtr_endpoint_register() {
	...
	qrtr_node_assign(node, nid);

	mutex_lock(&qrtr_node_lock);
	list_add(&node->item, &qrtr_all_nodes);
	mutex_unlock(&qrtr_node_lock);
	ep->node = node;
	...
	return 0;
}

and qrtr_node_assign() discards radix_tree_insert()'s return value while
setting node->nid anyway:

net/qrtr/af_qrtr.c:qrtr_node_assign() {
	...
	radix_tree_insert(&qrtr_nodes, nid, node);
	if (node->nid == QRTR_EP_NID_AUTO)
		WRITE_ONCE(node->nid, nid);
	...
}

Are there two reachable failure modes here?

The tree is declared static RADIX_TREE(qrtr_nodes, GFP_ATOMIC) and the
insert runs under spin_lock_irqsave() with no radix_tree_preload(), so
inserting key 0x10000 needs to extend the tree height and can return
-ENOMEM under memory pressure.

-EEXIST looks reachable too: mhi_unregister_controller() does
ida_free(&mhi_controller_ida, mhi_cntrl->index), so indexes and the derived
node ids get recycled, while the previous qrtr_node's radix slots are only
removed in __qrtr_node_release() on the last kref drop, which a sender
blocked in qrtr_tx_wait() can delay.

In either case node->nid is 0x10000 + index but qrtr_nodes has no entry (or
a stale one) for it, and qcom_mhi_qrtr_probe() still returns 0.  Is the
state permanent?  The condition in qrtr_endpoint_post() that used to retry
the insert

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

can never be true for a host assigned nid, since 0x10000 + index never
equals the remote's own id.

That would leave every unicast qrtr_sendmsg() to this node id returning
-ECONNRESET from qrtr_node_lookup() for the life of the device, the receive
path still stamping cb->src_node = node->nid so servers are advertised on
an unroutable id, and qrtr_endpoint_unregister() (which walks radix slots
pointing at the node to build the QRTR_TYPE_BYE notification) emitting no
BYE at all.

Would it be better for qrtr_node_assign() to return the insert error and
qrtr_endpoint_register() to propagate it so probe fails visibly?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260922-qrtr-multi-ep-v3-0-c00831e88957%40oss.qualcomm.com

Re: [PATCH v3 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 1 potential
issue(s) to consider.

Critical: 0 · High: 1 (1) · Medium: 0 · 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] Asymmetric QMI node-id filtering in ath11k:
  ath11k_qmi_ops_new_server()…

Pre-existing issues:
- [High] ath11k_qmi_init_service()
  (drivers/net/wireless/ath/ath11k/qmi.c:3328-3364) never calls…

--

wifi: ath11k: 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
ath11k_qmi_ops_new_server() to skip WLFW servers advertised by other
devices.  A node id of zero keeps the previous unfiltered behaviour.

> diff --git a/drivers/net/wireless/ath/ath11k/qmi.c b/drivers/net/wireless/ath/ath11k/qmi.c
> index 7dc07339b9579..9484b0eee1a9f 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;
> +	unsigned 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 the skip path return a negative errno instead of 0?

The QMI core only discards the service record when the callback 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);
	...
}

and include/linux/soc/qcom/qmi.h documents the consequence:

 * @new_server:		inform client of a new_server lookup-result, returning
 *                      successfully from this call causes the library to call
 *                      @del_server as the service is removed from the
 *                      lookup-result.

So with return 0 the foreign device's server stays in this handle's
lookup_results.  The counterpart callback has no node id check:

drivers/net/wireless/ath/ath11k/qmi.c:ath11k_qmi_ops_del_server() {
	...
	ath11k_dbg(ab, ATH11K_DBG_QMI, "wifi fw del server\n");
	ath11k_qmi_driver_event_post(qmi, ATH11K_QMI_EVENT_SERVER_EXIT, NULL);
}

When the twin device's WLFW server goes away (its firmware crash/SSR,
unbind, MHI power down, or a QRTR BYE for its node),
qmi_recv_del_server()/qmi_recv_bye() match the retained entry and call
ath11k_qmi_ops_del_server() on this device's handle, which reaches:

drivers/net/wireless/ath/ath11k/qmi.c:ath11k_qmi_driver_event_work() {
	...
		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 force a healthy device into crash recovery even though its own
firmware and MHI channels never went away, with no matching
SERVER_ARRIVE for its own node to clear the flags?  That would leave the
cross-device coupling the commit message describes in place on the
teardown half of the qmi_ops pair.

Would returning -ENODEV here (matching the existing kernel_connect()
failure return) so the foreign svc is never tracked, and/or applying the
same node_id && service->node != node_id test in
ath11k_qmi_ops_del_server(), be the intended behaviour?

Two smaller points on the same hunk: each retained foreign entry also
causes an extra event post during qmi_handle_release() in
ath11k_qmi_deinit_service(), and the silent return 0 leaves no
ath11k_dbg() trace, so a mis-derived node id (no server ever accepted,
device never comes up) is hard to diagnose.  Would an ath11k_dbg() line
on the skip path help here?

The same pattern is repeated in the following ath12k commit "wifi:
ath12k: Connect to the QMI server belonging to the device owned by this
driver"; nothing later in the series changes the return value here or
adds a filter to ath11k_qmi_ops_del_server().

[ ... ]

One more observation in the same file, outside the diff:

[Severity: High]
This isn't a bug introduced by this patch, but ath11k_qmi_init_service()
does not call qmi_handle_release() on the two failure paths that run
after qmi_handle_init() has already succeeded:

drivers/net/wireless/ath/ath11k/qmi.c:ath11k_qmi_init_service() {
	...
	ab->qmi.event_wq = alloc_ordered_workqueue("ath11k_qmi_driver_event", 0);
	if (!ab->qmi.event_wq) {
		ath11k_err(ab, "failed to allocate workqueue\n");
		return -EFAULT;
	}
	...
	if (ret < 0) {
		ath11k_warn(ab, "failed to add qmi lookup: %d\n", ret);
		destroy_workqueue(ab->qmi.event_wq);
		return ret;
	}
	...
}

By that point qmi_handle_init() has allocated qmi->recv_buf, created
qmi->wq, and created a bound QRTR socket via qmi_sock_create(), which
installs:

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

ath11k_core_soc_create() then propagates the error without any QMI
cleanup:

drivers/net/wireless/ath/ath11k/core.c:ath11k_core_soc_create() {
	...
	ret = ath11k_qmi_init_service(ab);
	if (ret) {
		ath11k_err(ab, "failed to initialize qmi :%d\n", ret);
		return ret;
	}
	...
}

and ath11k_pci_probe() unwinds and frees the containing ath11k_base.

Does this leak the recv_buf, the ordered workqueue and the QRTR socket,
and leave the still-bound socket's sk_user_data and sk_data_ready
pointing at the freed ath11k_base, so that qmi_data_ready() dereferences
freed memory for any datagram delivered to that port?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260922-qrtr-multi-ep-v3-0-c00831e88957%40oss.qualcomm.com

Re: [PATCH v3 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 1 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · 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] Asymmetric node-id filtering in ath12k QMI:
  ath12k_qmi_ops_new_server()…

--

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

This patch adds a get_qrtr_node_id() HIF callback that returns the QRTR node
id derived from the MHI controller index, and uses it in the QMI new_server
callback to skip WLFW services that live on a different node. A node id of
zero keeps the previous unfiltered behaviour.

> diff --git a/drivers/net/wireless/ath/ath12k/qmi.c b/drivers/net/wireless/ath/ath12k/qmi.c
> index 280e50a1f31d8..e1e75cc9d1441 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;
> +	unsigned 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?

In drivers/soc/qcom/qmi_interface.c, qmi_recv_new_server() only discards the
service record when the callback returns a negative value:

	ret = ops->new_server(qmi, svc);
	if (ret < 0)
		kfree(svc);
	else
		list_add(&svc->list_node, &qmi->lookup_results);

With the return 0 above, the other device's WLFW service stays in this
handle's lookup_results even though this device never connected to it.

The matching teardown path in the same qmi_ops table has no node id check:

drivers/net/wireless/ath/ath12k/qmi.c:ath12k_qmi_ops_del_server() {
	struct ath12k_qmi *qmi = container_of(qmi_hdl, struct ath12k_qmi, handle);
	struct ath12k_base *ab = qmi->ab;

	ath12k_dbg(ab, ATH12K_DBG_QMI, "qmi wifi fw del server\n");
	ath12k_qmi_driver_event_post(qmi, ATH12K_QMI_EVENT_SERVER_EXIT, NULL);
}

and qmi_recv_del_server() invokes it for every retained entry whose node
matches, with qmi_recv_bye() passing port == -1 to match all ports on the
dying node:

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 (port != -1 && svc->port != port)
			continue;

		if (ops->del_server)
			ops->del_server(qmi, svc);
	...
}

net/qrtr/ns.c:server_del() notifies every lookup matching service/instance
regardless of node, so device A's handle does receive DEL_SERVER or BYE for
device B's node when device B's firmware crashes or that card is unbound or
hot-removed.

Can that spuriously fault the healthy device? The event worker reacts to
SERVER_EXIT unconditionally:

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);
	...
}

ATH12K_FLAG_CRASH_FLUSH makes CE sends, WMI sends, data TX
(ath12k_wifi7_dp_tx()) and mgmt TX return -ESHUTDOWN, and it makes
ath12k_core_stop() skip ath12k_dp_reoq_lut_addr_reset() and
ath12k_qmi_firmware_stop() on a device whose firmware is still running:

drivers/net/wireless/ath/ath12k/core.c:ath12k_core_stop() {
	if (!test_bit(ATH12K_FLAG_CRASH_FLUSH, &ab->dev_flags)) {
		ath12k_dp_reoq_lut_addr_reset(ath12k_ab_to_dp(ab));
		ath12k_qmi_firmware_stop(ab);
	}
	...
}

The flag is only cleared on a later FW_READY event or on core reconfigure,
so the untouched device can stay in this state indefinitely.

Would returning a negative errno here (so the foreign service is never
tracked), and/or adding the same service->node check to
ath12k_qmi_ops_del_server(), cover the teardown direction as well?

There is a second effect from the retained entries: each filtered
NEW_SERVER adds another node of the foreign device into lookup_results,
so a foreign device in a firmware restart loop accumulates duplicate
entries that each later produce their own SERVER_EXIT post.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260922-qrtr-multi-ep-v3-0-c00831e88957%40oss.qualcomm.com