[PATCH v2 3/4] i3c: mipi-i3c-hci: Use physical device pointer with DMA API
Frank Li
Frank.li at nxp.com
Tue Aug 19 07:14:39 PDT 2025
On Tue, Aug 19, 2025 at 09:27:04AM +0300, Jarkko Nikula wrote:
> On 8/18/25 7:28 PM, Frank Li wrote:
> > On Fri, Aug 15, 2025 at 05:12:41PM +0300, Jarkko Nikula wrote:
> > > DMA transfer faults on Intel hardware when the IOMMU is enabled and
> > > driver initialization will fail when attempting to do the first transfer:
> > >
> > > DMAR: DRHD: handling fault status reg 2
> > > DMAR: [DMA Read NO_PASID] Request device [00:11.0] fault addr 0x676e3000 [fault reason 0x71] SM: Present bit in first-level paging entry is clear
> > > i3c mipi-i3c-hci.0: ring 0: Transfer Aborted
> > > mipi-i3c-hci mipi-i3c-hci.0: probe with driver mipi-i3c-hci failed with error -62
> > >
> > > Reason for this is that the IOMMU setup is done for the physical devices
> > > only and not for the virtual I3C Controller device object.
> > >
> > > Therefore use the pointer to a physical device object with the DMA API.
> > >
> > > Due to a data corruption observation when the device DMA is IOMMU
> > > mapped, a properly sized receive bounce buffer is required if transfer
> > > length is not a multiple of DWORDs.
> > >
> > > Reported-by: Prabhakaran, Krishna <krishna.prabhakaran at intel.com>
> > > Signed-off-by: Jarkko Nikula <jarkko.nikula at linux.intel.com>
> > > ---
> > > drivers/i3c/master/mipi-i3c-hci/dma.c | 46 +++++++++++++++++++--------
> > > 1 file changed, 33 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
> > > index 295671daae09..8cea91d4d3bf 100644
> > > --- a/drivers/i3c/master/mipi-i3c-hci/dma.c
> > > +++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
> > > @@ -14,6 +14,7 @@
> > > #include <linux/errno.h>
> > > #include <linux/i3c/master.h>
> > > #include <linux/io.h>
> > > +#include <linux/pci.h>
> > >
> > > #include "hci.h"
> > > #include "cmd.h"
> > > @@ -138,6 +139,7 @@ struct hci_rh_data {
> > > };
> > >
> > > struct hci_rings_data {
> > > + struct device *sysdev;
> > > unsigned int total;
> > > struct hci_rh_data headers[] __counted_by(total);
> > > };
> > > @@ -165,20 +167,20 @@ static void hci_dma_cleanup(struct i3c_hci *hci)
> > > rh_reg_write(IBI_SETUP, 0);
> > >
> > > if (rh->xfer)
> > > - dma_free_coherent(&hci->master.dev,
> > > + dma_free_coherent(rings->sysdev,
> > > rh->xfer_struct_sz * rh->xfer_entries,
> > > rh->xfer, rh->xfer_dma);
> > > if (rh->resp)
> > > - dma_free_coherent(&hci->master.dev,
> > > + dma_free_coherent(rings->sysdev,
> > > rh->resp_struct_sz * rh->xfer_entries,
> > > rh->resp, rh->resp_dma);
> > > kfree(rh->src_xfers);
> > > if (rh->ibi_status)
> > > - dma_free_coherent(&hci->master.dev,
> > > + dma_free_coherent(rings->sysdev,
> > > rh->ibi_status_sz * rh->ibi_status_entries,
> > > rh->ibi_status, rh->ibi_status_dma);
> > > if (rh->ibi_data_dma)
> > > - dma_unmap_single(&hci->master.dev, rh->ibi_data_dma,
> > > + dma_unmap_single(rings->sysdev, rh->ibi_data_dma,
> > > rh->ibi_chunk_sz * rh->ibi_chunks_total,
> > > DMA_FROM_DEVICE);
> > > kfree(rh->ibi_data);
> > > @@ -194,11 +196,23 @@ static int hci_dma_init(struct i3c_hci *hci)
> > > {
> > > struct hci_rings_data *rings;
> > > struct hci_rh_data *rh;
> > > + struct device *sysdev;
> > > u32 regval;
> > > unsigned int i, nr_rings, xfers_sz, resps_sz;
> > > unsigned int ibi_status_ring_sz, ibi_data_ring_sz;
> > > int ret;
> > >
> > > + /*
> > > + * Set pointer to a physical device that does DMA and has IOMMU setup
> > > + * done for it in case of enabled IOMMU and use it with the DMA API.
> > > + * Here such device is either
> > > + * "mipi-i3c-hci" platform device (OF/ACPI enumeration) parent or
> > > + * grandparent (PCI enumeration).
> > > + */
> > > + sysdev = hci->master.dev.parent;
> > > + if (sysdev->parent && dev_is_pci(sysdev->parent))
> > > + sysdev = sysdev->parent;
> > > +
> > > regval = rhs_reg_read(CONTROL);
> > > nr_rings = FIELD_GET(MAX_HEADER_COUNT_CAP, regval);
> > > dev_info(&hci->master.dev, "%d DMA rings available\n", nr_rings);
> > > @@ -213,6 +227,7 @@ static int hci_dma_init(struct i3c_hci *hci)
> > > return -ENOMEM;
> > > hci->io_data = rings;
> > > rings->total = nr_rings;
> > > + rings->sysdev = sysdev;
> > >
> > > regval = FIELD_PREP(MAX_HEADER_COUNT, rings->total);
> > > rhs_reg_write(CONTROL, regval);
> > > @@ -239,9 +254,9 @@ static int hci_dma_init(struct i3c_hci *hci)
> > > xfers_sz = rh->xfer_struct_sz * rh->xfer_entries;
> > > resps_sz = rh->resp_struct_sz * rh->xfer_entries;
> > >
> > > - rh->xfer = dma_alloc_coherent(&hci->master.dev, xfers_sz,
> > > + rh->xfer = dma_alloc_coherent(rings->sysdev, xfers_sz,
> > > &rh->xfer_dma, GFP_KERNEL);
> > > - rh->resp = dma_alloc_coherent(&hci->master.dev, resps_sz,
> > > + rh->resp = dma_alloc_coherent(rings->sysdev, resps_sz,
> > > &rh->resp_dma, GFP_KERNEL);
> > > rh->src_xfers =
> > > kmalloc_array(rh->xfer_entries, sizeof(*rh->src_xfers),
> > > @@ -295,16 +310,16 @@ static int hci_dma_init(struct i3c_hci *hci)
> > > ibi_data_ring_sz = rh->ibi_chunk_sz * rh->ibi_chunks_total;
> > >
> > > rh->ibi_status =
> > > - dma_alloc_coherent(&hci->master.dev, ibi_status_ring_sz,
> > > + dma_alloc_coherent(rings->sysdev, ibi_status_ring_sz,
> > > &rh->ibi_status_dma, GFP_KERNEL);
> > > rh->ibi_data = kmalloc(ibi_data_ring_sz, GFP_KERNEL);
> > > ret = -ENOMEM;
> > > if (!rh->ibi_status || !rh->ibi_data)
> > > goto err_out;
> > > rh->ibi_data_dma =
> > > - dma_map_single(&hci->master.dev, rh->ibi_data,
> > > + dma_map_single(rings->sysdev, rh->ibi_data,
> > > ibi_data_ring_sz, DMA_FROM_DEVICE);
> > > - if (dma_mapping_error(&hci->master.dev, rh->ibi_data_dma)) {
> > > + if (dma_mapping_error(rings->sysdev, rh->ibi_data_dma)) {
> > > rh->ibi_data_dma = 0;
> > > ret = -ENOMEM;
> > > goto err_out;
> > > @@ -369,6 +384,7 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
> > > 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];
> > > @@ -387,10 +403,13 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
> > >
> > > /* 2nd and 3rd words of Data Buffer Descriptor Structure */
> > > if (xfer->data) {
> > > - xfer->dma = i3c_master_dma_map_single(&hci->master.dev,
> > > + need_bounce = device_iommu_mapped(rings->sysdev) &&
> > > + xfer->rnw &&
> >
> > why only read need bounce? if iommu have not map enough size, still fault
> > for write.
> >
> > such as you try to map IO space [0xffd,0x1000). when hardware try access
> > 0x1000, iommu will capture this type error. Maybe it is hard to meet
> > allocate io space at close end of page, you have not found this problem.
> >
> Yes, only read has been problematic so far on our HW when the device is
> IOMMU mapped and write works fine with or without IOMMU. In my opinnion it's
> better to have another patch if such issue is seen on some other HW.
Okay.
Frank
>
> --
> 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