[PATCH 1/3] i3c: mipi-i3c-hci: Make bounce buffer code generic to all DMA transfers

[Frank Li]

On Mon, Jun 09, 2025 at 05:15:44PM +0300, Jarkko Nikula wrote:
> On 6/6/25 6:02 PM, Frank Li wrote:
> > On Fri, Jun 06, 2025 at 10:16:44AM +0300, Jarkko Nikula wrote:
> > > Hi
> > >
> > > On 6/5/25 6:13 PM, Frank Li wrote:
> > > > On Thu, Jun 05, 2025 at 05:07:19PM +0300, Jarkko Nikula wrote:
> > > > > Hi
> > > > >
> > > > > On 6/4/25 6:00 PM, Frank Li wrote:
> > > > > > On Wed, Jun 04, 2025 at 03:55:11PM +0300, Jarkko Nikula wrote:
> > > > > > > Move DMA bounce buffer code for I3C private transfers to be generic for
> > > > > > > all DMA transfers, and round up the receive bounce buffer size to a
> > > > > > > multiple of DWORDs.
> > > > > > >
> > > > > > > It was observed that when the device DMA is IOMMU mapped and the receive
> > > > > > > length is not a multiple of DWORDs, the last DWORD is padded with stale
> > > > > > > data from the RX FIFO, corrupting 1-3 bytes beyond the expected data.
> > > > > > >
> > > > > > > A similar issue, though less severe, occurs when an I3C target returns
> > > > > > > less data than requested. In this case, the padding does not exceed the
> > > > > > > requested number of bytes, assuming the device DMA is not IOMMU mapped.
> > > > > > >
> > > > > > > Therefore, all I3C private transfer, CCC command payload and I2C
> > > > > > > transfer receive buffers must be properly sized for the DMA being IOMMU
> > > > > > > mapped. Even if those buffers are already DMA safe, their size may not
> > > > > > > be, and I don't have a clear idea how to guarantee this other than
> > > > > > > using a local bounce buffer.
> > > > > > >
> > > > > > > To prepare for the device DMA being IOMMU mapped and to address the
> > > > > > > above issue, implement a local, properly sized bounce buffer for all
> > > > > > > DMA transfers. For now, allocate it only when the buffer is in the
> > > > > > > vmalloc() area to avoid unnecessary copying with CCC commands and
> > > > > > > DMA-safe I2C transfers.
> > > > > > >
> > > > > > > Signed-off-by: Jarkko Nikula <jarkko.nikula at linux.intel.com>
> > > > > > > ---
> > > > > > >     drivers/i3c/master/mipi-i3c-hci/core.c | 34 -------------------
> > > > > > >     drivers/i3c/master/mipi-i3c-hci/dma.c  | 47 +++++++++++++++++++++++++-
> > > > > > >     2 files changed, 46 insertions(+), 35 deletions(-)
> > > > > > >
> > > > > > > diff --git a/drivers/i3c/master/mipi-i3c-hci/core.c b/drivers/i3c/master/mipi-i3c-hci/core.c
> > > > > > > index bc4538694540..24c5e7d5b439 100644
> > > > > > > --- a/drivers/i3c/master/mipi-i3c-hci/core.c
> > > > > > > +++ b/drivers/i3c/master/mipi-i3c-hci/core.c
> > > > > > > @@ -272,34 +272,6 @@ static int i3c_hci_daa(struct i3c_master_controller *m)
> > > > > > >     	return hci->cmd->perform_daa(hci);
> > > > > > >     }
> > > > > > >
> > > > > > ...
> > > > > > >     }
> > > > > > >
> > > > > > > +static void *hci_dma_alloc_safe_xfer_buf(struct i3c_hci *hci,
> > > > > > > +					 struct hci_xfer *xfer)
> > > > > > > +{
> > > > > > > +	if (!is_vmalloc_addr(xfer->data))
> > > > > > > +		return xfer->data;
> > > > > > > +
> > > > > > > +	if (xfer->rnw)
> > > > > > > +		/*
> > > > > > > +		 * Round up the receive bounce buffer length to a multiple of
> > > > > > > +		 * DWORDs. Independently of buffer alignment, DMA_FROM_DEVICE
> > > > > > > +		 * transfers may corrupt the last DWORD when transfer length is
> > > > > > > +		 * not a multiple of DWORDs. This was observed when the device
> > > > > > > +		 * DMA is IOMMU mapped or when an I3C target device returns
> > > > > > > +		 * less data than requested. Latter case is less severe and
> > > > > > > +		 * does not exceed the requested number of bytes, assuming the
> > > > > > > +		 * device DMA is not IOMMU mapped.
> > > > > > > +		 */
> > > > > > > +		xfer->bounce_buf = kzalloc(ALIGN(xfer->data_len, 4),
> > > > > > > +					   GFP_KERNEL);
> > > > > > > +	else
> > > > > > > +		xfer->bounce_buf = kmemdup(xfer->data, xfer->data_len,
> > > > > > > +					   GFP_KERNEL);
> > > > > > > +
> > > > > > > +	return xfer->bounce_buf;
> > > > > >
> > > > > > Why need use bounce_buf? why not use dma_map_single()?, which will check
> > > > > > alignment and size to decide if use swiotlb as bounce buffer.
> > > > > >
> > > > > We do pass the buffer to the dma_map_single(). I've understood swiotlb is
> > > > > transparently used when the DMA cannot directly access the memory but that's
> > > > > not the case here.
> > > >
> > > > why not? even though you have to use internal buf, why not use
> > > > dma_alloc_coherent().
> > > >
> > > I don't think it's suitable for "smallish" per transfer allocations/mappings
> > > and buffer is not accessed concurrently by the CPU and DMA during the
> > > transfer.
> >
> > I still can't understand why need this special handle for i3c. Basic it use
> > dma transfer to transfer data. Can you simple descript hci's data flow?
> >
> Unfortunately my knowledge doesn't go deeply enough in HW but I could guess
> it's somewhere in MIPI I3C HCI DMA engine implementation and/or HCI IP to
> memory bridge/bus integration.
>
> If it would be a cache line issue then I would think we would see corruption
> independently is the DMA IOMMU mapped or not and in larger number of bytes
> than 1-3 last bytes.
>
> Also another finding that if I3C target returns less data than expected then
> there is also padding in the last DWORD with the real data. But not more
> than requested, i.e. 2 bytes expected, target returns 1, only the 2nd byte
> contains stale data and bytes 3-4 untouched. Or 8 bytes expected, target
> returns 1, bytes 2-4 contain stale data and bytes 5-8 untouched.
>
> So something around trailing bytes handling in the HW when the transfer
> length is not multiple of DWORDs as far as I could guess.

That's reason why should not manually handle dma buffer here. It is
small memory and less proformance requirement.

If you use dma provided api, you should not take care such detial. DMA
buffer at least one cache line size. otherwise, dma may currupt some data,
which is hard to find.

Frank

>
> --
> linux-i3c mailing list
> linux-i3c at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c