Re: [PATCH v1 14/15] RDMA/xsc: add the RDMA send path

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 13 potential issue(s) to consider:
- [Critical] RDMA/xsc: stack OOB read in build_qp1_send_v2() from xsc_ib_drain_sq()
- [Critical] RDMA/xsc: hdr_buf DMA buffer overflow on MAD/UD send in xsc_ib_post_send()
- [High] RDMA/xsc: broken inline ds_data_num and zero-length SGEs in xsc_ib_post_send()
- [High] RDMA/xsc: completion deadlock and lost wakeup in xsc_ib_drain_sq() and rq
- [High] RDMA/xsc: stale zero_length_read state corrupts completions in xsc_poll_one()
- [High] RDMA/xsc: missing endian conversions on ctrl->msg_len and crc in xsc_ib_post_send()
- [High] RDMA/xsc: missing le64_to_cpu() on data_seg->va in xsc_handle_rdma_mad_resp_recv()
- [High] RDMA/xsc: unsynchronized WQ access in xsc_generate_flush_err_cqe() and post paths
- [Medium] RDMA/xsc: broken IB_WR_SEND_WITH_IMM header packing in build_qp1_send_v2()
- [Low] RDMA/xsc: duplicate crc32_le() implementation and inaccurate IOMMU comment
- [Low] RDMA/xsc: unbalanced flush_wqe_cnt for MAD sends in xsc_ib_post_send()
- [Low] RDMA/xsc: wrong format specifier %ld for size_t in build_qp1_send_v2()
- [Low] RDMA/xsc: comment in xsc_ib.h falsely claims QP1 creation is refused on UNKNOWN IOMMU
--

commit c77226a0e2a3291230f2f8d59b1b33bb5ef256ca
Author: Xin Tian <tianx@yunsilicon.com>

RDMA/xsc: add the RDMA send path

This commit implements the RDMA send path for the xsc driver, including
xsc_ib_post_send() for RC and QP1/UD traffic, zero-length RDMA read
handling, and the xsc_ib_drain_sq() and xsc_ib_drain_rq() helpers.

> diff --git a/drivers/infiniband/hw/xsc/cq.c b/drivers/infiniband/hw/xsc/cq.c
> index 36d122f8d46fb..a5ab8d00d9a6b 100644
> --- a/drivers/infiniband/hw/xsc/cq.c
> +++ b/drivers/infiniband/hw/xsc/cq.c
[ ... ]
> @@ -173,7 +173,13 @@ static void xsc_handle_rdma_mad_resp_recv(struct xsc_ib_cq *cq,
>  	handle_responder(wc, cqe, *cur_qp, opcode);
>  
>  	data_seg = get_seg_wqe(get_recv_wqe(*cur_qp, idx), 0);
> -	recv = xsc_ib_recv_mad_sg_virt_addr(&dev->ib_dev, wc, data_seg->va);
> +	recv = xsc_ib_mad_sg_virt_addr(&dev->ib_dev, data_seg->va);

[Severity: High]
In xsc_handle_rdma_mad_resp_recv(), data_seg->va is a __le64 field populated
in xsc_ib_post_recv() via WR_LE_64(data_seg->va, wr->sg_list[i].addr).

Should data_seg->va be converted with le64_to_cpu() before passing it as a
dma_addr_t to xsc_ib_mad_sg_virt_addr() so that the DMA address is not
byte-swapped on big-endian architectures?

> diff --git a/drivers/infiniband/hw/xsc/main.c b/drivers/infiniband/hw/xsc/main.c
> index 138ca1a80083b..0574687982c24 100644
> --- a/drivers/infiniband/hw/xsc/main.c
> +++ b/drivers/infiniband/hw/xsc/main.c
> @@ -474,6 +474,22 @@ static int init_node_data(struct xsc_ib_dev *dev)
>  	return 0;
>  }
>  
> +static void crc_table_init(struct xsc_ib_dev *dev)
> +{
> +	u32 c, i, j;
> +
> +	for (i = 0; i < 256; i++) {
> +		c = i;
> +		for (j = 0; j < 8; j++) {
> +			if (c & 1)
> +				c = 0xedb88320L ^ (c >> 1);
> +			else
> +				c = c >> 1;
> +		}
> +		dev->crc_32_table[i] = c;
> +	}
> +}

[Severity: Low]
This isn't a bug, but drivers/infiniband/hw/xsc/qp.c already includes
linux/crc32.h, which provides crc32_le() using the same 0xedb88320 polynomial.

Could xsc_icrc_hdr() and xsc_ib_post_send() use crc32_le() directly instead of
maintaining dev->crc_32_table[], crc_table_init(), and xsc_crc32()?

> diff --git a/drivers/infiniband/hw/xsc/qp.c b/drivers/infiniband/hw/xsc/qp.c
> index 0dfb102949070..46ca3dd95df35 100644
> --- a/drivers/infiniband/hw/xsc/qp.c
> +++ b/drivers/infiniband/hw/xsc/qp.c
[ ... ]
> @@ -1437,13 +1532,538 @@ static int xsc_wq_overflow(struct xsc_ib_wq *wq, int nreq)
>  	return cur + nreq >= wq->max_post;
>  }
>  
> +static inline void xsc_post_send_db(struct xsc_ib_qp *qp,
> +				    struct xsc_core_device *xdev,
> +				    int nreq)
> +{
> +	u32 next_pid;
> +
> +	if (unlikely(!nreq))
> +		return;
> +
> +	qp->sq.head += nreq;
> +
> +	next_pid = qp->sq.head << (qp->sq.wqe_shift - XSC_BASE_WQE_SHIFT);
> +	xsc_update_tx_db(xdev, qp->doorbell_qpn, next_pid);
> +}

[Severity: High]
Can xsc_post_send_db() and xsc_ib_post_recv() race with
xsc_generate_flush_err_cqe() during QP drain or error transitions?

xsc_ib_post_send() holds only qp->sq.lock while populating qp->sq.wrid[idx],
qp->sq.wqe_head[idx], qp->sq.need_flush[idx], and qp->sq.wr_opcode[idx], and
then xsc_post_send_db() advances qp->sq.head before the barrier in
xsc_update_tx_db(). Similarly, xsc_ib_post_recv() holds only qp->rq.lock.

Meanwhile, in drivers/infiniband/hw/xsc/cq.c, xsc_ib_poll_cq() calls
xsc_generate_flush_err_cqe() while holding only cq->lock and
cq->err_state_qp_lock:

drivers/infiniband/hw/xsc/cq.c:xsc_generate_flush_err_cqe() {
	...
		while (wq->tail != wq->head) {
			idx = wq->tail & (wq->wqe_cnt - 1);
			if (err_qp_node->is_sq && !wq->need_flush[idx]) {
				wq->tail++;
				continue;
			} else {
				gen_flush_err_cqe(err_qp_node, &(core_to_xqp(xqp)->ibqp), wq,
						  idx, wc + *npolled + sw_npolled);
				++sw_npolled;
			}
	...
}

When xsc_ib_drain_sq() or xsc_ib_drain_rq() moves the QP to IB_QPS_ERR first
and then posts the drain WR, could xsc_generate_flush_err_cqe() on another CPU
observe the updated wq->head alongside a stale wq->need_flush[idx] or
wq->wrid[idx], skipping the drain WR or invoking a stale wr_cqe callback?

[ ... ]
> +/* Routine for sending QP1 packets for RoCE V2 */
> +static int build_qp1_send_v2(struct xsc_ib_dev *dev,
> +			     struct xsc_ib_qp *qp,
> +			     const struct ib_send_wr *wr,
> +			     struct ib_sge *sge,
> +			     int payload_size, u32 *crc)
> +{
> +	struct xsc_ib_ah *ah = container_of(ud_wr((struct ib_send_wr *)wr)->ah, struct xsc_ib_ah,
> +					    ibah);
> +	const struct ib_gid_attr *sgid_attr = ah->ibah.sgid_attr;

[Severity: Critical]
Does draining a GSI or UD send queue via xsc_ib_drain_sq() read past the end
of the stack-allocated work request in build_qp1_send_v2()?

In xsc_ib_drain_sq(), swr is declared on the stack as a base struct ib_send_wr
with .opcode = IB_WR_RDMA_WRITE:

xsc_ib_drain_sq()
  -> xsc_ib_post_send(qp, &swr, &bad_swr)
       -> case IB_QPT_UD / IB_QPT_GSI:
            -> build_qp1_send_v2(dev, qp, wr, &sg, msg_len, &crc)

Because xsc_ib_post_send() does not check for drain WRs or validate wr->opcode
in the IB_QPT_UD and IB_QPT_GSI cases, build_qp1_send_v2() casts &swr via
ud_wr(wr)->ah, reading past the end of struct ib_send_wr on the stack and
dereferencing ah->ibah.sgid_attr.

Also, for IB_QPT_SMI or IB_QPT_RAW_PACKET QPs, doesn't xsc_ib_post_send() hit
the default branch and fail xsc_ib_drain_sq() with -EPERM?

[ ... ]
> +	is_eth = true;
> +	is_vlan = (vlan_id && (vlan_id < 0x1000)) ? true : false;
> +
> +	ib_ud_header_init(payload_size, !is_eth, is_eth, is_vlan, false,
> +			  ip_version, true, 0, &qp->qp1_hdr);
[ ... ]
> +	/* BTH */
> +	if (wr->opcode == IB_WR_SEND_WITH_IMM) {
> +		qp->qp1_hdr.bth.opcode = IB_OPCODE_UD_SEND_ONLY_WITH_IMMEDIATE;
> +		qp->qp1_hdr.immediate_present = 1;
> +	} else {
> +		qp->qp1_hdr.bth.opcode = IB_OPCODE_UD_SEND_ONLY;
> +	}

[Severity: Medium]
When wr->opcode == IB_WR_SEND_WITH_IMM in build_qp1_send_v2(),
ib_ud_header_init() is called with immediate_present = 0, and
qp->qp1_hdr.immediate_present is later set to 1 without setting
qp->qp1_hdr.immediate_data or increasing sge->length by 4 bytes.

Doesn't ib_ud_header_pack() then write 4 uninitialized bytes of immediate_data
at the end of the header that are excluded from the IP/UDP lengths and
sge->length, and then overwritten when xsc_ib_post_send() copies the payload
to mad_send_base + sg.length?

[ ... ]
> +	rc = xsc_icrc_hdr(dev, hdr_buf, sge->length - sizeof(struct ib_unpacked_eth), crc);
> +	if (rc) {
> +		ibdev_err(&dev->ib_dev, "CRC error: hdr size %ld\n",
> +			  sge->length - sizeof(struct ib_unpacked_eth));
> +	}
> +	return rc;
> +}

[Severity: Low]
In build_qp1_send_v2(), sge->length - sizeof(struct ib_unpacked_eth) has type
size_t. Should the format specifier be %zu instead of %ld to avoid -Wformat
warnings on 32-bit COMPILE_TEST builds?

[ ... ]
> +		for (i = 0; i < wr->num_sge; ++i) {
> +			if (likely(wr->sg_list[i].length))
> +				msg_len += wr->sg_list[i].length;
> +		}
> +		ctrl->msg_len = msg_len;

[Severity: High]
In struct xsc_send_wqe_ctrl_seg, ctrl->msg_len is declared as __le32.

In xsc_ib_post_send(), msg_len is assigned to ctrl->msg_len without
cpu_to_le32(), and in the IB_QPT_UD / IB_QPT_GSI branch below:

	crc = xsc_crc32(dev, crc, mad_send_base + sg.length, ctrl->msg_len);
	ctrl->msg_len += sg.length;
	seg_index++;

	put_unaligned(~crc, (u32 *)&mad_send_base[ctrl->msg_len]);
	ctrl->msg_len += sizeof(crc);
	sg.length = ctrl->msg_len;

ctrl->msg_len is used directly in host-endian arithmetic and array indexing,
and ~crc is written in host byte order with put_unaligned() instead of
put_unaligned_le32(). Won't this corrupt the WQE length, buffer offset, and
ICRC on big-endian systems?

[ ... ]
> +			if (unlikely(wr->opcode == IB_WR_RDMA_READ && msg_len == 0)) {
> +				ibdev_dbg(&dev->ib_dev, "post zero-length read wr\n");
> +				/* HW rejects zero-length reads; poll backfills the
> +				 * READ opcode from qp->sq.zero_length_read[]
> +				 */
> +				ctrl_val |= FIELD_PREP(XSC_WQE_CTRL_SEG_DATA0_MSG_OPCODE_MASK,
> +						       dev->opcode_map[IB_WR_RDMA_WRITE]);
> +				qp->sq.zero_length_read[idx] = 1;
> +			} else {
> +				ctrl_val |= FIELD_PREP(XSC_WQE_CTRL_SEG_DATA0_MSG_OPCODE_MASK,
> +						       dev->opcode_map[wr->opcode]);
> +			}
> +			break;

[Severity: High]
Can qp->sq.zero_length_read[idx] remain set across WQE slot reuse and corrupt
a later completion in xsc_poll_one()?

In xsc_ib_post_send(), qp->sq.zero_length_read[idx] is set to 1 for any
zero-length IB_WR_RDMA_READ, and the else branch does not clear it to 0. In
xsc_poll_one(), wq->zero_length_read[idx] is only cleared when a normal or
error hardware CQE is polled for that idx.

If a zero-length RDMA read is posted unsignaled (!ce), or if it is completed
via xsc_generate_lost_cqe() or software flush in gen_flush_err_cqe(), or if
set_data_inl_seg() fails later in xsc_ib_post_send(), zero_length_read[idx]
stays 1 and will overwrite the completion opcode of a subsequent signaled SEND
or WRITE that reuses slot idx.

> +		case IB_QPT_UD:
> +		case IB_QPT_GSI:
> +			ctrl_val |= FIELD_PREP(XSC_WQE_CTRL_SEG_DATA0_MSG_OPCODE_MASK,
> +					       xsc_ib_opcode->mad);
> +			ds_data_num++;
> +			data_seg = get_seg_wqe(ctrl, seg_index);
> +			mad_send_base = (u8 *)qp->sq.hdr_buf +
> +				MAX_QP1_SQ_HDR_SIZE_V2 * qp->sq.mad_index;
> +
> +			err = build_qp1_send_v2(dev, qp, wr, &sg, msg_len, &crc);
> +			if (err) {
> +				*bad_wr = wr;
> +				goto out;
> +			}
> +
> +			cur_p = mad_send_base + sg.length;
> +			for (i = 0; i < wr->num_sge; ++i) {
> +				if (likely(wr->sg_list[i].length)) {
> +					vaddr = xsc_ib_mad_sg_virt_addr(&dev->ib_dev,
> +									wr->sg_list[i].addr);
> +					if (unlikely(!vaddr)) {
> +						ibdev_err(&dev->ib_dev,
> +							  "cannot map MAD sg entry %d to a virtual address\n",
> +							  i);
> +						err = -EOPNOTSUPP;
> +						*bad_wr = wr;
> +						goto out;
> +					}
> +					memcpy(cur_p, vaddr, wr->sg_list[i].length);
> +				}
> +				cur_p += wr->sg_list[i].length;
> +			}
> +			crc = xsc_crc32(dev, crc, mad_send_base + sg.length, ctrl->msg_len);
> +			ctrl->msg_len += sg.length;
> +			seg_index++;
> +
> +			put_unaligned(~crc, (u32 *)&mad_send_base[ctrl->msg_len]);

[Severity: Critical]
Can this memcpy() and put_unaligned() overflow the 512-byte slot in
qp->sq.hdr_buf?

In create_kernel_qp(), qp->sq.hdr_buf is allocated with MAD_QUEUE_DEPTH slots
of MAX_QP1_SQ_HDR_SIZE_V2 (512) bytes each, while xsc_port_immutable() sets
immutable->max_mad_size = IB_MGMT_MAD_SIZE * 2 (512 bytes).

In xsc_ib_post_send(), build_qp1_send_v2() writes up to 86 bytes of RoCEv2
headers into mad_send_base, and then xsc_ib_post_send() copies msg_len bytes
of SGE payload plus 4 bytes of ICRC immediately after sg.length without
checking that sg.length + msg_len + sizeof(crc) fits within
MAX_QP1_SQ_HDR_SIZE_V2. A 512-byte MAD (or any UD payload over 422 bytes) will
write up to 602 bytes into the 512-byte slot and past the end of
qp->sq.hdr_buf on the last slot.

[ ... ]
> +		ctrl_val |= FIELD_PREP(XSC_WQE_CTRL_SEG_DATA0_DS_DATA_NUM_MASK, ds_data_num);
> +		ctrl->data0 |= cpu_to_le32(ctrl_val);
> +		ctrl->data3 = cpu_to_le32((wr->send_flags & IB_SEND_SOLICITED ?
> +					   XSC_WQE_CTRL_SEG_DATA3_SE : 0) |
> +					  (ce ? XSC_WQE_CTRL_SEG_DATA3_CE : 0));
> +
> +		if (wr->send_flags & IB_SEND_INLINE && wr->num_sge) {
> +			unsigned int segs_used;
> +
> +			err = set_data_inl_seg(qp, wr, ctrl, seg_index, &segs_used);
> +			if (unlikely(err)) {
> +				*bad_wr = wr;
> +				ibdev_err(&dev->ib_dev, "inline layout failed, err %d\n", err);
> +				goto out;
> +			}
> +			ds_data_num = segs_used - 1;
> +		} else {
> +			for (i = 0; i < sg_n; ++i, ++seg_index) {
> +				if (likely(sgl[i].length)) {
> +					data_seg = get_seg_wqe(ctrl, seg_index);
> +					set_local_data_seg(dev->xdev, data_seg, &sgl[i]);
> +				}
> +			}
> +		}

[Severity: High]
There appear to be three issues with how ds_data_num and the data segments are
populated here in xsc_ib_post_send():

1. ctrl->data0 is written with ds_data_num before the IB_SEND_INLINE block
   runs, so ds_data_num = segs_used - 1 after set_data_inl_seg() is never
   written to ctrl->data0.

2. For IB_QPT_UD and IB_QPT_GSI, sg_n is set to 0 after copying the payload
   into qp->sq.hdr_buf, but the inline check tests wr->num_sge instead of
   sg_n, which would cause set_data_inl_seg() to append extra inline segments
   on UD/GSI QPs.

3. In the non-inline else loop, seg_index is incremented on every iteration
   even when sgl[i].length == 0 and set_local_data_seg() is skipped, while
   ds_data_num still includes zero-length SGEs, leaving zeroed segment holes
   in the WQE.

> +		qp->sq.wrid[idx] = wr->wr_id;
> +		qp->sq.wqe_head[idx] = qp->sq.head + nreq;
> +		qp->sq.cur_post += 1;
> +		if (ce) {
> +			atomic_inc(&qp->sq.flush_wqe_cnt);
> +			qp->sq.need_flush[idx] = 1;
> +			qp->sq.prev_ce_wqe[idx] = qp->sq.last_post_ce_wqe;
> +			qp->sq.last_post_ce_wqe = idx;
> +		}
> +		qp->sq.wr_opcode[idx] = wr->opcode;

[Severity: Low]
When ce is true in xsc_ib_post_send(), qp->sq.flush_wqe_cnt is incremented for
all QP types including IB_QPT_GSI, but in drivers/infiniband/hw/xsc/cq.c,
xsc_poll_one() skips decrementing it when a MAD send completes:

drivers/infiniband/hw/xsc/cq.c:xsc_poll_one() {
	...
		if (opcode != XSC_OPCODE_RDMA_MAD_REQ_SEND)
			atomic_dec(&wq->flush_wqe_cnt);
	...
}

Doesn't this cause qp->sq.flush_wqe_cnt to grow on every signaled MAD send?

[ ... ]
> +void xsc_ib_drain_rq(struct ib_qp *qp)
> +{
> +	struct ib_cq *cq = qp->recv_cq;
> +	struct xsc_ib_cq *xcq = to_xcq(cq);
> +	struct xsc_ib_qp *xqp = to_xqp(qp);
> +	struct ib_qp_attr attr = { .qp_state = IB_QPS_ERR };
> +	struct xsc_ib_drain_cqe rdrain;
> +	const struct ib_recv_wr rwr = {
> +		.next = NULL,
> +		{.wr_cqe = &rdrain.cqe,},
> +	};
> +	const struct ib_recv_wr *bad_rwr;
> +	int ret;
> +	struct xsc_ib_dev *dev = to_xdev(qp->device);
> +
> +	ret = ib_modify_qp(qp, &attr, IB_QP_STATE);
> +	if (ret) {
> +		ibdev_err(&dev->ib_dev, "failed to drain recv queue: %d\n", ret);
> +		return;
> +	}
> +
> +	rdrain.cqe.done = xsc_ib_drain_rq_done;
> +	init_completion(&rdrain.done);
> +
> +	ret = xsc_ib_post_recv(qp, &rwr, &bad_rwr);
> +	if (ret) {
> +		ibdev_err(&dev->ib_dev, "failed to drain recv queue: %d\n", ret);
> +		return;
> +	}
> +	xqp->post_recv_invalid = true;
> +
> +	if (cq->poll_ctx == IB_POLL_DIRECT)
> +		while (wait_for_completion_timeout(&rdrain.done, HZ / 10) <= 0)
> +			ib_process_cq_direct(cq, -1);
> +	else
> +		xcq->xcq.comp(&xcq->xcq);
> +	wait_for_completion(&rdrain.done);
> +}
> +
> +void xsc_ib_drain_sq(struct ib_qp *qp)
> +{
> +	struct ib_cq *cq = qp->send_cq;
> +	struct xsc_ib_cq *xcq = to_xcq(cq);
> +	struct xsc_ib_qp *xqp = to_xqp(qp);
> +	struct ib_qp_attr attr = { .qp_state = IB_QPS_ERR };
> +	struct xsc_ib_drain_cqe sdrain;
> +	const struct ib_send_wr *bad_swr;
> +	const struct ib_send_wr swr = {
> +		.next = NULL,
> +		{ .wr_cqe	= &sdrain.cqe, },
> +		.opcode	= IB_WR_RDMA_WRITE,
> +		.send_flags = IB_SEND_SIGNALED,
> +	};
> +	int ret;
> +	struct xsc_ib_dev *dev = to_xdev(qp->device);
> +
> +	ret = ib_modify_qp(qp, &attr, IB_QP_STATE);
> +	if (ret) {
> +		ibdev_err(&dev->ib_dev, "failed to drain send queue: %d\n", ret);
> +		return;
> +	}
> +
> +	sdrain.cqe.done = xsc_ib_drain_sq_done;
> +	init_completion(&sdrain.done);
> +
> +	ret = xsc_ib_post_send(qp, &swr, &bad_swr);
> +	if (ret) {
> +		ibdev_err(&dev->ib_dev, "failed to drain send queue: %d\n", ret);
> +		return;
> +	}
> +	xqp->post_send_invalid = true;
> +
> +	if (cq->poll_ctx == IB_POLL_DIRECT)
> +		while (wait_for_completion_timeout(&sdrain.done, HZ / 10) <= 0)
> +			ib_process_cq_direct(cq, -1);
> +	else
> +		xcq->xcq.comp(&xcq->xcq);
> +	wait_for_completion(&sdrain.done);
> +}

[Severity: High]
Can xsc_ib_drain_rq() and xsc_ib_drain_sq() hang when waiting for the drain
completion?

1. When cq->poll_ctx == IB_POLL_DIRECT, wait_for_completion_timeout() in the
   while loop condition consumes rdrain.done / sdrain.done when it returns a
   positive value, and then the unconditional wait_for_completion() after the
   if/else block waits a second time on the already-consumed completion.
   Additionally, ib_modify_qp() transitions the QP to IB_QPS_ERR, which calls
   xsc_ib_trigger_cq_comp_handler() and triggers the WARN_ONCE() in
   ib_cq_completion_direct() for IB_POLL_DIRECT CQs.

2. For IB_POLL_SOFTIRQ CQs, ib_modify_qp() schedules cq->iop before the drain
   WR is posted. If ib_poll_handler() finishes polling before the drain WR is
   posted, and xcq->xcq.comp(&xcq->xcq) runs before ib_poll_handler() calls
   irq_poll_complete(), irq_poll_sched() is a no-op while IRQ_POLL_F_SCHED is
   still set. When ib_poll_handler() then calls ib_req_notify_cq() ->
   xsc_ib_arm_cq(), xsc_cqe_is_empty() only checks hardware CQEs via
   get_sw_cqe() and ignores software flush entries on cq->err_state_qp_list,
   so cq->iop is never rescheduled and wait_for_completion() hangs.

3. If the SQ or RQ is already full when drain is called, xsc_wq_overflow() in
   xsc_ib_post_send() / xsc_ib_post_recv() will reject the drain WR with
   -ENOMEM and return early without draining the queue.

> diff --git a/drivers/infiniband/hw/xsc/xsc_ib.h b/drivers/infiniband/hw/xsc/xsc_ib.h
> index 126fcf7892a12..fe111b2c61370 100644
> --- a/drivers/infiniband/hw/xsc/xsc_ib.h
> +++ b/drivers/infiniband/hw/xsc/xsc_ib.h
[ ... ]
> @@ -402,6 +407,23 @@ static inline enum ib_mtu xsc_net_to_ib_mtu(unsigned int mtu)
>  		return 0;
>  }
>  
> +/*
> + * MMIO doorbells and MAD payloads are accessed by the CPU, so the driver
> + * needs the kernel virtual address behind a DMA address.  There is no such
> + * conversion API, so the address space is classified once at probe time and
> + * the matching trick is used:
> + *
> + *   NORMAL  - an IOMMU DMA domain is attached; iova_to_phys() is available.
> + *   DISABLE - no IOMMU translation, DMA address == physical address.
> + *   UNKNOWN - address translation exists but is not owned by the IOMMU
> + *             layer (forced software bounce buffering, or a hypervisor that
> + *             translates DMA addresses), so neither trick works.
> + *
> + * UNKNOWN is not supported: MAD and doorbell traffic cannot be addressed by
> + * the CPU there, and QP1 (SMI/GSI) creation is refused as a result, which
> + * disables SA/CM on such a system.  Systems with an IOMMU or without any
> + * translation are unaffected.
> + */

[Severity: Low]
This comment states that QP1 (SMI/GSI) creation is refused when the IOMMU
state is UNKNOWN, but xsc_ib_create_qp() in drivers/infiniband/hw/xsc/qp.c
does not check dev->iommu_state and instead allows QP1 creation to succeed,
failing later in xsc_ib_post_send() and xsc_handle_rdma_mad_resp_recv().

Should either xsc_ib_create_qp() reject IB_QPT_GSI when dev->iommu_state ==
XSC_IB_IOMMU_MAP_UNKNOWN_DOMAIN, or this comment be updated to match the code?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104155650.37053-1-tianx@yunsilicon.com?part=14