[PATCH 03/12] i3c: mipi-i3c-hci: Fix race in DMA ring enqueue for parallel xfers
Frank Li
Frank.li at nxp.com
Fri Feb 27 08:09:21 PST 2026
On Fri, Feb 27, 2026 at 04:11:40PM +0200, Adrian Hunter wrote:
> The I3C subsystem allows multiple transfers to be queued concurrently.
> However, the MIPI I3C HCI driver’s DMA enqueue path, hci_dma_queue_xfer(),
> lacks sufficient serialization.
>
> In particular, the allocation of the enqueue_ptr and its subsequent update
> in the RING_OPERATION1 register, must be done atomically. Otherwise, for
> example, it would be possible for 2 transfers to be allocated the same
> enqueue_ptr.
>
> Extend the use of the existing spinlock for that purpose. Keep a count of
> the number of xfers enqueued so that it is easy to determine if the ring
> has enough space.
>
> Refactor hci_dma_queue_xfer() to do all DMA mapping first, so that DMA
> mapping is not done under spinlock.
Can you move this part code into prepare patch? like add helper
hci_dma_map_xfer_list()
Frank
>
> Fixes: 9ad9a52cce282 ("i3c/master: introduce the mipi-i3c-hci driver")
> Cc: stable at vger.kernel.org
> Signed-off-by: Adrian Hunter <adrian.hunter at intel.com>
> ---
> drivers/i3c/master/mipi-i3c-hci/dma.c | 79 ++++++++++++++++-----------
> 1 file changed, 48 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
> index b903a2da1fd1..f60654fbe58e 100644
> --- a/drivers/i3c/master/mipi-i3c-hci/dma.c
> +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
> @@ -129,7 +129,7 @@ struct hci_rh_data {
> dma_addr_t xfer_dma, resp_dma, ibi_status_dma, ibi_data_dma;
> unsigned int xfer_entries, ibi_status_entries, ibi_chunks_total;
> unsigned int xfer_struct_sz, resp_struct_sz, ibi_status_sz, ibi_chunk_sz;
> - unsigned int done_ptr, ibi_chunk_ptr;
> + unsigned int done_ptr, ibi_chunk_ptr, xfer_space;
> struct hci_xfer **src_xfers;
> spinlock_t lock;
> struct completion op_done;
> @@ -261,6 +261,7 @@ static void hci_dma_init_rh(struct i3c_hci *hci, struct hci_rh_data *rh, int i)
>
> rh->done_ptr = 0;
> rh->ibi_chunk_ptr = 0;
> + rh->xfer_space = rh->xfer_entries;
> }
>
> static void hci_dma_init_rings(struct i3c_hci *hci)
> @@ -439,26 +440,63 @@ static void hci_dma_unmap_xfer(struct i3c_hci *hci,
> }
> }
>
> +static struct i3c_dma *hci_dma_map_xfer(struct device *dev, struct hci_xfer *xfer)
> +{
> + enum dma_data_direction dir = xfer->rnw ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
> + bool need_bounce = device_iommu_mapped(dev) && xfer->rnw && (xfer->data_len & 3);
> +
> + return i3c_master_dma_map_single(dev, xfer->data, xfer->data_len, need_bounce, dir);
> +}
> +
> +static int hci_dma_map_xfer_list(struct i3c_hci *hci, struct device *dev,
> + struct hci_xfer *xfer_list, int n)
> +{
> + for (int i = 0; i < n; i++) {
> + struct hci_xfer *xfer = xfer_list + i;
> +
> + if (!xfer->data)
> + continue;
> +
> + xfer->dma = hci_dma_map_xfer(dev, xfer);
> + if (!xfer->dma) {
> + hci_dma_unmap_xfer(hci, xfer_list, i);
> + return -ENOMEM;
> + }
> + }
> +
> + return 0;
> +}
> +
> static int hci_dma_queue_xfer(struct i3c_hci *hci,
> struct hci_xfer *xfer_list, int n)
> {
> struct hci_rings_data *rings = hci->io_data;
> struct hci_rh_data *rh;
> unsigned int i, ring, enqueue_ptr;
> - u32 op1_val, op2_val;
> + u32 op1_val;
> + int ret;
> +
> + ret = hci_dma_map_xfer_list(hci, rings->sysdev, xfer_list, n);
> + if (ret)
> + return ret;
>
> /* For now we only use ring 0 */
> ring = 0;
> rh = &rings->headers[ring];
>
> + spin_lock_irq(&rh->lock);
> +
> + if (n > rh->xfer_space) {
> + spin_unlock_irq(&rh->lock);
> + hci_dma_unmap_xfer(hci, xfer_list, n);
> + return -EBUSY;
> + }
> +
> op1_val = rh_reg_read(RING_OPERATION1);
> enqueue_ptr = FIELD_GET(RING_OP1_CR_ENQ_PTR, op1_val);
> for (i = 0; i < n; i++) {
> struct hci_xfer *xfer = xfer_list + i;
> u32 *ring_data = rh->xfer + rh->xfer_struct_sz * enqueue_ptr;
> - enum dma_data_direction dir = xfer->rnw ? DMA_FROM_DEVICE :
> - DMA_TO_DEVICE;
> - bool need_bounce;
>
> /* store cmd descriptor */
> *ring_data++ = xfer->cmd_desc[0];
> @@ -477,18 +515,6 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
>
> /* 2nd and 3rd words of Data Buffer Descriptor Structure */
> if (xfer->data) {
> - need_bounce = device_iommu_mapped(rings->sysdev) &&
> - xfer->rnw &&
> - xfer->data_len != ALIGN(xfer->data_len, 4);
> - xfer->dma = i3c_master_dma_map_single(rings->sysdev,
> - xfer->data,
> - xfer->data_len,
> - need_bounce,
> - dir);
> - if (!xfer->dma) {
> - hci_dma_unmap_xfer(hci, xfer_list, i);
> - return -ENOMEM;
> - }
> *ring_data++ = lower_32_bits(xfer->dma->addr);
> *ring_data++ = upper_32_bits(xfer->dma->addr);
> } else {
> @@ -503,22 +529,10 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
> xfer->ring_entry = enqueue_ptr;
>
> enqueue_ptr = (enqueue_ptr + 1) % rh->xfer_entries;
> -
> - /*
> - * We may update the hardware view of the enqueue pointer
> - * only if we didn't reach its dequeue pointer.
> - */
> - op2_val = rh_reg_read(RING_OPERATION2);
> - if (enqueue_ptr == FIELD_GET(RING_OP2_CR_DEQ_PTR, op2_val)) {
> - /* the ring is full */
> - hci_dma_unmap_xfer(hci, xfer_list, i + 1);
> - return -EBUSY;
> - }
> }
>
> - /* take care to update the hardware enqueue pointer atomically */
> - spin_lock_irq(&rh->lock);
> - op1_val = rh_reg_read(RING_OPERATION1);
> + rh->xfer_space -= n;
> +
> op1_val &= ~RING_OP1_CR_ENQ_PTR;
> op1_val |= FIELD_PREP(RING_OP1_CR_ENQ_PTR, enqueue_ptr);
> rh_reg_write(RING_OPERATION1, op1_val);
> @@ -586,6 +600,7 @@ static void hci_dma_xfer_done(struct i3c_hci *hci, struct hci_rh_data *rh)
> {
> u32 op1_val, op2_val, resp, *ring_resp;
> unsigned int tid, done_ptr = rh->done_ptr;
> + unsigned int done_cnt = 0;
> struct hci_xfer *xfer;
>
> for (;;) {
> @@ -617,10 +632,12 @@ static void hci_dma_xfer_done(struct i3c_hci *hci, struct hci_rh_data *rh)
>
> done_ptr = (done_ptr + 1) % rh->xfer_entries;
> rh->done_ptr = done_ptr;
> + done_cnt += 1;
> }
>
> /* take care to update the software dequeue pointer atomically */
> spin_lock(&rh->lock);
> + rh->xfer_space += done_cnt;
> op1_val = rh_reg_read(RING_OPERATION1);
> op1_val &= ~RING_OP1_CR_SW_DEQ_PTR;
> op1_val |= FIELD_PREP(RING_OP1_CR_SW_DEQ_PTR, done_ptr);
> --
> 2.51.0
>
>
> --
> linux-i3c mailing list
> linux-i3c at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c
More information about the linux-i3c
mailing list