Re: [PATCH V3 08/16] i3c: mipi-i3c-hci: Avoid restarting DMA ring after aborting wrong transfer
Adrian Hunter <[email protected]>
| Newsgroups | org.infradead.lists.linux-i3c,org.kernel.vger.linux-kernel |
|---|---|
| Organization | Intel Finland Oy, Registered Address: c/o Alberga Business Park, 6 krs, Bertel Jungin Aukio 5, 02600 Espoo, Business Identity Code: 0357606 - 4, Domiciled in Helsinki |
| Message-ID | <[email protected]> |
On 12/05/2026 19:50, Frank Li wrote: > On Mon, May 04, 2026 at 02:33:44PM +0300, Adrian Hunter wrote: >> Software ABORT of the DMA ring is used to recover from transfer list >> timeouts, but it is inherently racy. The intended transfer list may >> complete just before the ABORT takes effect, causing the subsequent >> transfer list to be aborted instead. >> >> In this case, an incomplete transfer list may remain in the ring and has >> not yet been processed by hci_dma_dequeue_xfer(). Restarting the DMA >> ring at that point can lead to unpredictable results. >> >> Detect when the next queued transfer is not the first entry of a transfer >> list and does not belong to the list currently being dequeued. In that >> case, skip restarting the DMA ring and defer recovery until a subsequent >> call to hci_dma_dequeue_xfer(), which will safely restart the ring once >> the incomplete list is handled. >> >> Signed-off-by: Adrian Hunter <[email protected]> >> --- >> >> >> Changes in V3: >> >> None >> >> Changes in V2: >> >> Renamed completing_xfer to final_xfer >> >> >> drivers/i3c/master/mipi-i3c-hci/dma.c | 15 +++++++++++++++ >> drivers/i3c/master/mipi-i3c-hci/hci.h | 1 + >> 2 files changed, 16 insertions(+) >> >> diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c >> index 899fdf6555a8..268f54b32101 100644 >> --- a/drivers/i3c/master/mipi-i3c-hci/dma.c >> +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c >> @@ -503,6 +503,7 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci, >> u32 *ring_data = rh->xfer + rh->xfer_struct_sz * enqueue_ptr; >> >> xfer->final_xfer = xfer_list + n - 1; >> + xfer->xfer_list_pos = i; >> >> /* store cmd descriptor */ >> *ring_data++ = xfer->cmd_desc[0]; >> @@ -669,6 +670,20 @@ static bool hci_dma_dequeue_xfer(struct i3c_hci *hci, >> } >> } >> >> + /* >> + * A software ABORT may race with transfer completion and abort the next >> + * transfer list instead. Detect that case, and do not restart the ring. >> + * It will be handled by a subsequent dequeue. >> + */ >> + if (!did_unqueue) { >> + struct hci_xfer *xfer = rh->src_xfers[rh->done_ptr]; >> + >> + if (xfer && xfer->xfer_list_pos && xfer->final_xfer != xfer_list->final_xfer) { >> + spin_unlock_irq(&hci->lock); > > Is it possible to use auto cleanup to handle lock()? No, there are 2 paths that do not return directly after unlocking and do different things. > > Frank >> + return false; >> + } >> + } >> + >> /* restart the ring */ >> reinit_completion(&rh->op_done); >> mipi_i3c_hci_resume(hci); >> diff --git a/drivers/i3c/master/mipi-i3c-hci/hci.h b/drivers/i3c/master/mipi-i3c-hci/hci.h >> index f07fc627d4d2..83d4f13a68a3 100644 >> --- a/drivers/i3c/master/mipi-i3c-hci/hci.h >> +++ b/drivers/i3c/master/mipi-i3c-hci/hci.h >> @@ -107,6 +107,7 @@ struct hci_xfer { >> struct hci_xfer *final_xfer; >> int ring_number; >> int ring_entry; >> + int xfer_list_pos; >> }; >> }; >> }; >> -- >> 2.51.0 >> -- linux-i3c mailing list [email protected] http://lists.infradead.org/mailman/listinfo/linux-i3c