Re: [for-next v2 2/5] RDMA/ionic: support firmware-assigned CQ IDs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] RDMA/ionic: raw loop index passed as udma_mask bitmask in CQ creation
--

commit fe0e581cbc460bad22a8b89a0b2fc1b93d3ab485
Author: Abhijit Gangurde <abhijit.gangurde@amd.com>
Subject: RDMA/ionic: support firmware-assigned CQ IDs

When the LIF advertises qid allocation for CQ, this commit updates the driver
to let the firmware return the completion queue ID instead of allocating it
in the driver bitmap. The CQ creation and destruction are split into distinct
phases to safely handle concurrent allocations.

> diff --git a/drivers/infiniband/hw/ionic/ionic_admin.c b/drivers/infiniband/hw/ionic/ionic_admin.c
> index 37e24450d1298..bd31594b36908 100644
> --- a/drivers/infiniband/hw/ionic/ionic_admin.c
> +++ b/drivers/infiniband/hw/ionic/ionic_admin.c
> @@ -533,12 +576,16 @@ static struct ionic_vcq *ionic_create_rdma_admincq(struct ionic_ibdev *dev,
>  	cq = &vcq->cq[0];
>  
>  	rc = ionic_create_cq_common(vcq, &buf, &attr, NULL, NULL,
> -				    NULL, NULL, 0);
> +				    NULL, 0);
>  	if (rc)
>  		goto err_init;
>  
> -	rc = ionic_rdma_queue_devcmd(dev, &cq->q, cq->cqid, cq->eqid,
> -				     IONIC_CMD_RDMA_CREATE_CQ);
> +	rc = ionic_rdma_cq_devcmd(vcq, &cq->q, &cq->cqid, cq->eqid,
> +				  IONIC_CMD_RDMA_CREATE_CQ, 0);

[Severity: High]
Does ionic_create_rdma_admincq() pass an empty bitmask to the firmware here? 
The udma_mask argument is 0, which evaluates to a bitmask with no bits set. 

Earlier in this function, vcq->udma_mask is initialized to 1. If the firmware 
expects a valid UDMA mask, this might fail or allocate unexpectedly.

[ ... ]

> diff --git a/drivers/infiniband/hw/ionic/ionic_controlpath.c b/drivers/infiniband/hw/ionic/ionic_controlpath.c
> index 37f71fb43811e..7a840ece8259d 100644
> --- a/drivers/infiniband/hw/ionic/ionic_controlpath.c
> +++ b/drivers/infiniband/hw/ionic/ionic_controlpath.c
> @@ -1222,8 +1230,11 @@ int ionic_dealloc_mw(struct ib_mw *ibmw)
>  static int ionic_create_cq_cmd(struct ionic_ibdev *dev,
>  			       struct ionic_ctx *ctx,
>  			       struct ionic_cq *cq,
> -			       struct ionic_tbl_buf *buf)
> +			       struct ionic_tbl_buf *buf,
> +			       int udma_mask,
> +			       int *out_udma_idx)
>  {

[ ... ]

> @@ -1243,13 +1254,57 @@ static int ionic_create_cq_cmd(struct ionic_ibdev *dev,
> +	wr.wqe.cmd.create_cq.udma_mask = udma_mask;

[ ... ]

> @@ -1308,16 +1363,21 @@ int ionic_create_cq(struct ib_cq *ibcq, const struct ib_cq_init_attr *attr,
>  
>  		rc = ionic_create_cq_common(vcq, &buf, attr, ctx, udata,
>  					    &req.cq[udma_idx],
> -					    &resp.cqid[udma_idx],
>  					    udma_idx);
>  		if (rc)
>  			goto err_init;
>  
> -		rc = ionic_create_cq_cmd(dev, ctx, &vcq->cq[udma_idx], &buf);
> +		rc = ionic_create_cq_cmd(dev, ctx, &vcq->cq[udma_idx], &buf,
> +					 udma_idx, NULL);

[Severity: High]
Should ionic_create_cq() be passing BIT(udma_idx) or vcq->udma_mask instead
of the raw loop index here?

By passing udma_idx directly as the udma_mask parameter to 
ionic_create_cq_cmd(), if udma_idx is 0, a mask of 0 is passed. If udma_idx 
is 1, a mask of 1 (which equals BIT(0)) is passed, which could cause the 
firmware to allocate the CQ on UDMA 0 instead of the intended UDMA 1. 

This could break DMA and completion tracking if the software CQ context is 
mapped to the wrong hardware UDMA engine.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922095724.3683011-1-abhijit.gangurde@amd.com?part=2