On Fri, Oct 30, 2026 at 2:33 PM <[email protected]> wrote: > > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) > to consider: > - [Medium] RDMA/irdma: wrong DMA direction in irdma_setup_umode_cq() via > ib_umem_get_cq_buf() > -- > > commit 8981c5869a8245f4b87a31412a16b517815f3a39 > Author: David Hu <[email protected]> > > RDMA/irdma: Support UMEM attribute for CQ creation > > This commit adds support for one-step user-mode CQ buffer creation using > UVERBS_ATTR_CREATE_CQ_BUF_UMEM and IRDMA_IB_ATTR_CREATE_CQ_SHADOW_BUF_UMEM, > sets up hardware PBLEs directly from the pinned UMEMs, and adds UMEM and PBLE > cleanup in irdma_cq_free_rsrc() and irdma_resize_cq(). > > > diff --git a/drivers/infiniband/hw/irdma/verbs.c > > b/drivers/infiniband/hw/irdma/verbs.c > > index 9f4905fdb791a..41e56ab8aa6df 100644 > > --- a/drivers/infiniband/hw/irdma/verbs.c > > +++ b/drivers/infiniband/hw/irdma/verbs.c > > [ ... ] > > > @@ -2767,26 +2793,87 @@ static int irdma_setup_umode_cq(struct irdma_device > > *iwdev, > > if (err_code) > > return err_code; > > > > - spin_lock_irqsave(&ucontext->cq_reg_mem_list_lock, flags); > > - iwcq->iwpbl = irdma_get_pbl((unsigned long)req.user_cq_buf, > > - &ucontext->cq_reg_mem_list); > > - spin_unlock_irqrestore(&ucontext->cq_reg_mem_list_lock, flags); > > - if (!iwcq->iwpbl) > > - return -EPROTO; > > + cqe_64byte_ena = dev->hw_attrs.uk_attrs.feature_flags & > > IRDMA_FEATURE_64_BYTE_CQE ? > > + true : false; > > + cqe_size = cqe_64byte_ena ? 64 : 32; > > + > > + if (uverbs_attr_is_valid(attrs, UVERBS_ATTR_CREATE_CQ_BUF_UMEM)) { > > + struct ib_umem *umem; > > + struct irdma_mem_reg_req cq_req = {}; > > + u8 shadow_pgcnt = (rf->sc_dev.hw_attrs.uk_attrs.feature_flags > > & > > + IRDMA_FEATURE_CQ_RESIZE) ? 0 : > > IRDMA_SHADOW_PGCNT; > > + u32 min_size; > > + > > + cq_req.cq_pages = DIV_ROUND_UP(ukinfo->cq_size * cqe_size, > > + IRDMA_HW_PAGE_SIZE); > > + min_size = (cq_req.cq_pages + shadow_pgcnt) << > > IRDMA_HW_PAGE_SHIFT; > > + > > + umem = ib_umem_get_cq_buf(&iwdev->ibdev, attrs, min_size, > > + IB_ACCESS_LOCAL_WRITE); > > [Severity: Medium] > When IRDMA_FEATURE_CQ_RESIZE is not set in > rf->sc_dev.hw_attrs.uk_attrs.feature_flags, the UMEM passed via > UVERBS_ATTR_CREATE_CQ_BUF_UMEM backs both the CQ ring and the trailing shadow > area page (shadow_pgcnt is IRDMA_SHADOW_PGCNT, and irdma_setup_umode_cq() > later sets info->shadow_area_pa = cqmr->shadow from that same UMEM). > > However, ib_umem_get_cq_buf() in drivers/infiniband/core/umem.c hardcodes > DMA_FROM_DEVICE instead of deriving DMA_BIDIRECTIONAL from > IB_ACCESS_LOCAL_WRITE: > > drivers/infiniband/core/umem.c:ib_umem_get_cq_buf() { > return ib_umem_get_from_attrs(device, attrs, > UVERBS_ATTR_CREATE_CQ_BUF_UMEM, > uverbs_create_cq_get_buffer_desc, > size, access, DMA_FROM_DEVICE); > } > > Since the CPU writes the CQ head index to the shadow area and hardware reads > shadow_area_pa via DMA, mapping this UMEM with DMA_FROM_DEVICE omits IOMMU > read permissions (and skips CPU-to-device bounce buffer syncs). > > Should irdma_setup_umode_cq() use ib_umem_get_attr() with > UVERBS_ATTR_CREATE_CQ_BUF_UMEM when IRDMA_FEATURE_CQ_RESIZE is not set (or > unconditionally, as noted in the commit message) so the buffer is mapped with > DMA_BIDIRECTIONAL?
This finding is not practical. The AI finding alludes to a very narrow scenario on older HW (Gen 1, iWarp) without CQ resize. With iommu on, `DMA_FROM_DEVICE` leads to lack of IOMMU_READ for NIC iommu group, and leads to iommu faults, while NIC tries to fetch CQ consumer index from shadow area in dram. This will only happen for VA, since dma-buf in iWarp is rejected. However, `UVERBS_ATTR_CREATE_CQ_BUF_UMEM` will never land in kernel in case of VA, since `fill_attr_in_buf_umem` (in ucreate_cq) skips VA. Further, switching to `ib_umem_get_attr()` would also mean mapping DMA_BIDIRECTIONAL for cq buf for Gen2, and Gen3 (when cq resize is possible), defeating the standards established in ib core. > > -- > Sashiko AI review · > https://sashiko.dev/#/patchset/[email protected]?part=5
