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

[Jarkko Nikula <jarkko.nikula@linux.intel.com>]

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@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 int i3c_hci_alloc_safe_xfer_buf(struct i3c_hci *hci,
-				       struct hci_xfer *xfer)
-{
-	if (hci->io != &mipi_i3c_hci_dma ||
-	    xfer->data == NULL || !is_vmalloc_addr(xfer->data))
-		return 0;
-
-	if (xfer->rnw)
-		xfer->bounce_buf = kzalloc(xfer->data_len, GFP_KERNEL);
-	else
-		xfer->bounce_buf = kmemdup(xfer->data,
-					   xfer->data_len, GFP_KERNEL);
-
-	return xfer->bounce_buf == NULL ? -ENOMEM : 0;
-}
-
-static void i3c_hci_free_safe_xfer_buf(struct i3c_hci *hci,
-				       struct hci_xfer *xfer)
-{
-	if (hci->io != &mipi_i3c_hci_dma || xfer->bounce_buf == NULL)
-		return;
-
-	if (xfer->rnw)
-		memcpy(xfer->data, xfer->bounce_buf, xfer->data_len);
-
-	kfree(xfer->bounce_buf);
-}
-
 static int i3c_hci_priv_xfers(struct i3c_dev_desc *dev,
 			      struct i3c_priv_xfer *i3c_xfers,
 			      int nxfers)
@@ -333,9 +305,6 @@ static int i3c_hci_priv_xfers(struct i3c_dev_desc *dev,
 		}
 		hci->cmd->prep_i3c_xfer(hci, dev, &xfer[i]);
 		xfer[i].cmd_desc[0] |= CMD_0_ROC;
-		ret = i3c_hci_alloc_safe_xfer_buf(hci, &xfer[i]);
-		if (ret)
-			goto out;
 	}
 	last = i - 1;
 	xfer[last].cmd_desc[0] |= CMD_0_TOC;
@@ -359,9 +328,6 @@ static int i3c_hci_priv_xfers(struct i3c_dev_desc *dev,
 	}
 
 out:
-	for (i = 0; i < nxfers; i++)
-		i3c_hci_free_safe_xfer_buf(hci, &xfer[i]);
-
 	hci_free_xfer(xfer, nxfers);
 	return ret;
 }
diff --git a/drivers/i3c/master/mipi-i3c-hci/dma.c b/drivers/i3c/master/mipi-i3c-hci/dma.c
index 491dfe70b660..0311c84f5b4e 100644
--- a/drivers/i3c/master/mipi-i3c-hci/dma.c
+++ b/drivers/i3c/master/mipi-i3c-hci/dma.c
@@ -339,6 +339,44 @@ static int hci_dma_init(struct i3c_hci *hci)
 	return ret;
 }
 
+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;
+}
+
+static void hci_dma_free_safe_xfer_buf(struct i3c_hci *hci,
+				       struct hci_xfer *xfer)
+{
+	if (xfer->bounce_buf == NULL)
+		return;
+
+	if (xfer->rnw)
+		memcpy(xfer->data, xfer->bounce_buf, xfer->data_len);
+
+	kfree(xfer->bounce_buf);
+}
+
 static void hci_dma_unmap_xfer(struct i3c_hci *hci,
 			       struct hci_xfer *xfer_list, unsigned int n)
 {
@@ -352,6 +390,7 @@ static void hci_dma_unmap_xfer(struct i3c_hci *hci,
 		dma_unmap_single(&hci->master.dev,
 				 xfer->data_dma, xfer->data_len,
 				 xfer->rnw ? DMA_FROM_DEVICE : DMA_TO_DEVICE);
+		hci_dma_free_safe_xfer_buf(hci, xfer);
 	}
 }
 
@@ -391,7 +430,12 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
 
 		/* 2nd and 3rd words of Data Buffer Descriptor Structure */
 		if (xfer->data) {
-			buf = xfer->bounce_buf ? xfer->bounce_buf : xfer->data;
+			buf = hci_dma_alloc_safe_xfer_buf(hci, xfer);
+			if (buf == NULL) {
+				hci_dma_unmap_xfer(hci, xfer_list, i);
+				return -ENOMEM;
+			}
+
 			xfer->data_dma =
 				dma_map_single(&hci->master.dev,
 					       buf,
@@ -401,6 +445,7 @@ static int hci_dma_queue_xfer(struct i3c_hci *hci,
 						  DMA_TO_DEVICE);
 			if (dma_mapping_error(&hci->master.dev,
 					      xfer->data_dma)) {
+				hci_dma_free_safe_xfer_buf(hci, xfer);
 				hci_dma_unmap_xfer(hci, xfer_list, i);
 				return -ENOMEM;
 			}
-- 
2.47.2


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