[PATCH 03/12] i3c: mipi-i3c-hci: Fix race in DMA ring enqueue for parallel xfers
Adrian Hunter
adrian.hunter at intel.com
Fri Feb 27 06:11:40 PST 2026
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.
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
More information about the linux-i3c
mailing list