[RFC PATCH] mmc: meson-mx-sdhc: add DMA bounce buffer support

[RFC PATCH] mmc: meson-mx-sdhc: add DMA bounce buffer support

[ZHENPENG YUAN]

The Meson MX SDHC controller uses a single DMA address for each
transfer. Since the driver leaves max_segs at the MMC core default
of 1, large I/O requests are split into single-segment requests,
typically 4 KiB each.

On an Amlogic S805, this limits eMMC sequential read throughput
to approximately 15 MiB/s despite using HS200 timing.

Add a 128 KiB DMA bounce buffer for multi-segment requests and
advertise up to 64 scatterlist segments. Single-segment requests
continue to use direct DMA. If buffer allocation fails, fall back
to the original single-segment limit.

Testing on a Xunlei OneCloud (Meson8b, Samsung 8GME4R eMMC)
running Linux 7.3.1:

  Block size       Before       After
  4 KiB            14.47        14.69 MiB/s
  64 KiB           15.06        63.21 MiB/s
  128 KiB          15.08        75.13 MiB/s
  1 MiB            15.07        75.93 MiB/s

For a 128 MiB sequential read, block requests decreased from
32768 to 1024.

Direct I/O SHA256 readback verification and fio CRC32C write
verification passed. No MMC errors were observed.

Assisted-by: LLM
Signed-off-by: ZHENPENG YUAN <yuanzhenpeng210@gmail.com>
---
 drivers/mmc/host/meson-mx-sdhc-mmc.c | 165 ++++++++++++++++++++++++---
 1 file changed, 149 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/host/meson-mx-sdhc-mmc.c b/drivers/mmc/host/meson-mx-sdhc-mmc.c
index fb49ea712..c62583a80 100644
--- a/drivers/mmc/host/meson-mx-sdhc-mmc.c
+++ b/drivers/mmc/host/meson-mx-sdhc-mmc.c
@@ -16,6 +16,7 @@
 #include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
+#include <linux/scatterlist.h>
 #include <linux/types.h>
 
 #include <linux/mmc/host.h>
@@ -29,6 +30,12 @@
 #define MESON_SDHC_MAX_BLK_SIZE					512
 #define MESON_SDHC_NUM_TUNING_TRIES				10
 
+/*
+ * A 128 KiB request spans at most 33 pages when it is not page aligned.
+ * The extra entries give the block layer room to merge a full request.
+ */
+#define MESON_SDHC_MAX_SEGS					64
+
 #define MESON_SDHC_WAIT_CMD_READY_SLEEP_US			1
 #define MESON_SDHC_WAIT_CMD_READY_TIMEOUT_US			100000
 #define MESON_SDHC_WAIT_BEFORE_SEND_SLEEP_US			1
@@ -48,6 +55,17 @@ struct meson_mx_sdhc_host {
 	struct mmc_command		*cmd;
 	int				error;
 
+	/*
+	 * The controller has a single DMA address register, so requests that
+	 * are not physically contiguous have to be bounced through this
+	 * buffer.
+	 */
+	void				*bounce_buf;
+	size_t				bounce_buf_size;
+	dma_addr_t			dma_addr;
+	unsigned int			dma_len;
+	bool				dma_bounce;
+
 	struct regmap			*regmap;
 
 	struct clk			*pclk;
@@ -220,8 +238,7 @@ static void meson_mx_sdhc_start_cmd(struct mmc_host *mmc,
 			   FIELD_PREP(MESON_SDHC_CTRL_PACK_LEN, pack_len));
 
 	if (cmd->data)
-		regmap_write(host->regmap, MESON_SDHC_ADDR,
-			     sg_dma_address(cmd->data->sg));
+		regmap_write(host->regmap, MESON_SDHC_ADDR, host->dma_addr);
 
 	meson_mx_sdhc_wait_cmd_ready(mmc);
 
@@ -362,32 +379,97 @@ static void meson_mx_sdhc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 
 static int meson_mx_sdhc_map_dma(struct mmc_host *mmc, struct mmc_request *mrq)
 {
+	struct meson_mx_sdhc_host *host = mmc_priv(mmc);
 	struct mmc_data *data = mrq->data;
-	unsigned int dma_len;
+	enum dma_data_direction dir;
+	struct scatterlist *sg;
+	unsigned int len, total_len;
+	int i, nents;
 
 	if (!data)
 		return 0;
 
-	dma_len = dma_map_sg(mmc_dev(mmc), data->sg, data->sg_len,
-			     mmc_get_dma_dir(data));
-	if (!dma_len) {
-		dev_err(mmc_dev(mmc), "dma_map_sg failed\n");
+	dir = mmc_get_dma_dir(data);
+	len = data->blocks * data->blksz;
+	host->dma_len = len;
+	host->dma_bounce = false;
+
+	/*
+	 * A request that consists of a single scatterlist entry is already
+	 * contiguous from the controller's point of view and can be
+	 * programmed directly.  This is also the only case where transfers
+	 * larger than the advertised max_req_size may occur (SDIO).
+	 */
+	if (data->sg_len == 1 && data->sg->length >= len) {
+		nents = dma_map_sg(mmc_dev(mmc), data->sg, data->sg_len, dir);
+		if (!nents) {
+			dev_err(mmc_dev(mmc), "dma_map_sg failed\n");
+			return -ENOMEM;
+		}
+
+		host->dma_addr = sg_dma_address(data->sg);
+		return 0;
+	}
+
+	total_len = 0;
+	for_each_sg(data->sg, sg, data->sg_len, i)
+		total_len += sg->length;
+
+	if (!host->bounce_buf || len > host->bounce_buf_size ||
+	    total_len < len) {
+		dev_err(mmc_dev(mmc),
+			"cannot DMA %u bytes in %u segments\n",
+			len, data->sg_len);
+		return -EINVAL;
+	}
+
+	if (dir == DMA_TO_DEVICE) {
+		if (sg_copy_to_buffer(data->sg, data->sg_len, host->bounce_buf,
+				      len) != len) {
+			dev_err(mmc_dev(mmc),
+				"failed to copy data into the bounce buffer\n");
+			return -EIO;
+		}
+	}
+
+	host->dma_addr = dma_map_single(mmc_dev(mmc), host->bounce_buf, len,
+					dir);
+	if (dma_mapping_error(mmc_dev(mmc), host->dma_addr)) {
+		dev_err(mmc_dev(mmc), "dma_map_single failed\n");
 		return -ENOMEM;
 	}
 
+	host->dma_bounce = true;
+
 	return 0;
 }
 
+static void meson_mx_sdhc_unmap_dma(struct meson_mx_sdhc_host *host,
+				    struct mmc_data *data)
+{
+	struct device *dev = mmc_dev(host->mmc);
+
+	if (host->dma_bounce) {
+		dma_unmap_single(dev, host->dma_addr, host->dma_len,
+				 mmc_get_dma_dir(data));
+		host->dma_bounce = false;
+	} else {
+		dma_unmap_sg(dev, data->sg, data->sg_len,
+			     mmc_get_dma_dir(data));
+	}
+}
+
 static void meson_mx_sdhc_request(struct mmc_host *mmc, struct mmc_request *mrq)
 {
 	struct meson_mx_sdhc_host *host = mmc_priv(mmc);
 	struct mmc_command *cmd = mrq->cmd;
+	int ret = 0;
 
 	if (!host->error)
-		host->error = meson_mx_sdhc_map_dma(mmc, mrq);
+		ret = meson_mx_sdhc_map_dma(mmc, mrq);
 
-	if (host->error) {
-		cmd->error = host->error;
+	if (host->error || ret) {
+		cmd->error = host->error ? host->error : ret;
 		mmc_request_done(mmc, mrq);
 		return;
 	}
@@ -596,11 +678,6 @@ static irqreturn_t meson_mx_sdhc_irq_thread(int irq, void *irq_data)
 			regmap_update_bits(host->regmap, MESON_SDHC_PDMA, val,
 					   val);
 		}
-
-		dma_unmap_sg(mmc_dev(host->mmc), cmd->data->sg,
-			     cmd->data->sg_len, mmc_get_dma_dir(cmd->data));
-
-		cmd->data->bytes_xfered = cmd->data->blksz * cmd->data->blocks;
 	}
 
 	meson_mx_sdhc_wait_cmd_ready(host->mmc);
@@ -625,6 +702,40 @@ static irqreturn_t meson_mx_sdhc_irq_thread(int irq, void *irq_data)
 		 */
 		meson_mx_sdhc_clear_fifo(host->mmc);
 
+	if (cmd->data) {
+		struct mmc_data *data = cmd->data;
+		size_t len = data->blksz * data->blocks;
+		bool bounced = host->dma_bounce;
+
+		/*
+		 * Unmap the data after the controller has been quiesced so
+		 * that neither the reset nor the FIFO flush can race with an
+		 * ongoing DMA transfer.  dma_unmap_single() (for bounced
+		 * reads) also invalidates the CPU caches, so the copy below
+		 * sees the data written by the controller.
+		 */
+		meson_mx_sdhc_unmap_dma(host, data);
+
+		if (!data->error) {
+			if (bounced && data->flags & MMC_DATA_READ) {
+				size_t copied;
+
+				copied = sg_copy_from_buffer(data->sg,
+							     data->sg_len,
+							     host->bounce_buf,
+							     len);
+				if (copied != len) {
+					dev_err(mmc_dev(host->mmc),
+						"failed to copy data from the bounce buffer\n");
+					data->error = -EIO;
+				}
+			}
+
+			if (!data->error)
+				data->bytes_xfered = len;
+		}
+	}
+
 	meson_mx_sdhc_request_done(host);
 
 	return IRQ_HANDLED;
@@ -813,8 +924,17 @@ static int meson_mx_sdhc_probe(struct platform_device *pdev)
 
 	mmc->max_req_size = SZ_128K;
 	mmc->max_seg_size = mmc->max_req_size;
-	mmc->max_blk_count = FIELD_GET(MESON_SDHC_SEND_TOTAL_PACK, ~0);
 	mmc->max_blk_size = MESON_SDHC_MAX_BLK_SIZE;
+	/*
+	 * The controller has a single DMA address register.  Allowing the
+	 * block layer to merge more segments is only safe because requests
+	 * consisting of multiple segments are bounced through a contiguous
+	 * buffer.  Keep every request within max_req_size so that it always
+	 * fits into that buffer; SDIO uses max_blk_count to split transfers
+	 * as well.
+	 */
+	mmc->max_blk_count = mmc->max_req_size / mmc->max_blk_size;
+	mmc->max_segs = MESON_SDHC_MAX_SEGS;
 	mmc->max_busy_timeout = 30 * MSEC_PER_SEC;
 	mmc->f_min = clk_round_rate(host->sd_clk, 1);
 	mmc->f_max = clk_round_rate(host->sd_clk, ULONG_MAX);
@@ -823,6 +943,19 @@ static int meson_mx_sdhc_probe(struct platform_device *pdev)
 	mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY | MMC_CAP_HW_RESET;
 	mmc->ops = &meson_mx_sdhc_ops;
 
+	host->bounce_buf = devm_kmalloc(dev, mmc->max_req_size, GFP_KERNEL);
+	if (host->bounce_buf) {
+		host->bounce_buf_size = mmc->max_req_size;
+	} else {
+		/*
+		 * Without the bounce buffer only physically contiguous
+		 * requests can be handled, so fall back to the old limit.
+		 */
+		mmc->max_segs = 1;
+		dev_warn(dev,
+			 "failed to allocate the DMA bounce buffer, using single-segment transfers\n");
+	}
+
 	ret = mmc_of_parse(mmc);
 	if (ret)
 		goto err_disable_pclk;
-- 
2.56.0

[RFC PATCH] mmc: meson-mx-sdhc: add DMA bounce buffer support

[ZHENPENG YUAN]

The Meson MX SDHC controller uses a single DMA address for each
transfer. Since the driver leaves max_segs at the MMC core default
of 1, large I/O requests are split into single-segment requests,
typically 4 KiB each.

On an Amlogic S805, this limits eMMC sequential read throughput
to approximately 15 MiB/s despite using HS200 timing.

Add a 128 KiB DMA bounce buffer for multi-segment requests and
advertise up to 64 scatterlist segments. Single-segment requests
continue to use direct DMA. If buffer allocation fails, fall back
to the original single-segment limit.

Testing on a Xunlei OneCloud (Meson8b, Samsung 8GME4R eMMC)
running Linux 7.3.1:

  Block size       Before       After
  4 KiB            14.47        14.69 MiB/s
  64 KiB           15.06        63.21 MiB/s
  128 KiB          15.08        75.13 MiB/s
  1 MiB            15.07        75.93 MiB/s

For a 128 MiB sequential read, block requests decreased from
32768 to 1024.

Direct I/O SHA256 readback verification and fio CRC32C write
verification passed. No MMC errors were observed.

Assisted-by: LLM
Signed-off-by: ZHENPENG YUAN <yuanzhenpeng210@gmail.com>
---
 drivers/mmc/host/meson-mx-sdhc-mmc.c | 165 ++++++++++++++++++++++++---
 1 file changed, 149 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/host/meson-mx-sdhc-mmc.c b/drivers/mmc/host/meson-mx-sdhc-mmc.c
index fb49ea712..c62583a80 100644
--- a/drivers/mmc/host/meson-mx-sdhc-mmc.c
+++ b/drivers/mmc/host/meson-mx-sdhc-mmc.c
@@ -16,6 +16,7 @@
 #include <linux/property.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
+#include <linux/scatterlist.h>
 #include <linux/types.h>
 
 #include <linux/mmc/host.h>
@@ -29,6 +30,12 @@
 #define MESON_SDHC_MAX_BLK_SIZE					512
 #define MESON_SDHC_NUM_TUNING_TRIES				10
 
+/*
+ * A 128 KiB request spans at most 33 pages when it is not page aligned.
+ * The extra entries give the block layer room to merge a full request.
+ */
+#define MESON_SDHC_MAX_SEGS					64
+
 #define MESON_SDHC_WAIT_CMD_READY_SLEEP_US			1
 #define MESON_SDHC_WAIT_CMD_READY_TIMEOUT_US			100000
 #define MESON_SDHC_WAIT_BEFORE_SEND_SLEEP_US			1
@@ -48,6 +55,17 @@ struct meson_mx_sdhc_host {
 	struct mmc_command		*cmd;
 	int				error;
 
+	/*
+	 * The controller has a single DMA address register, so requests that
+	 * are not physically contiguous have to be bounced through this
+	 * buffer.
+	 */
+	void				*bounce_buf;
+	size_t				bounce_buf_size;
+	dma_addr_t			dma_addr;
+	unsigned int			dma_len;
+	bool				dma_bounce;
+
 	struct regmap			*regmap;
 
 	struct clk			*pclk;
@@ -220,8 +238,7 @@ static void meson_mx_sdhc_start_cmd(struct mmc_host *mmc,
 			   FIELD_PREP(MESON_SDHC_CTRL_PACK_LEN, pack_len));
 
 	if (cmd->data)
-		regmap_write(host->regmap, MESON_SDHC_ADDR,
-			     sg_dma_address(cmd->data->sg));
+		regmap_write(host->regmap, MESON_SDHC_ADDR, host->dma_addr);
 
 	meson_mx_sdhc_wait_cmd_ready(mmc);
 
@@ -362,32 +379,97 @@ static void meson_mx_sdhc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 
 static int meson_mx_sdhc_map_dma(struct mmc_host *mmc, struct mmc_request *mrq)
 {
+	struct meson_mx_sdhc_host *host = mmc_priv(mmc);
 	struct mmc_data *data = mrq->data;
-	unsigned int dma_len;
+	enum dma_data_direction dir;
+	struct scatterlist *sg;
+	unsigned int len, total_len;
+	int i, nents;
 
 	if (!data)
 		return 0;
 
-	dma_len = dma_map_sg(mmc_dev(mmc), data->sg, data->sg_len,
-			     mmc_get_dma_dir(data));
-	if (!dma_len) {
-		dev_err(mmc_dev(mmc), "dma_map_sg failed\n");
+	dir = mmc_get_dma_dir(data);
+	len = data->blocks * data->blksz;
+	host->dma_len = len;
+	host->dma_bounce = false;
+
+	/*
+	 * A request that consists of a single scatterlist entry is already
+	 * contiguous from the controller's point of view and can be
+	 * programmed directly.  This is also the only case where transfers
+	 * larger than the advertised max_req_size may occur (SDIO).
+	 */
+	if (data->sg_len == 1 && data->sg->length >= len) {
+		nents = dma_map_sg(mmc_dev(mmc), data->sg, data->sg_len, dir);
+		if (!nents) {
+			dev_err(mmc_dev(mmc), "dma_map_sg failed\n");
+			return -ENOMEM;
+		}
+
+		host->dma_addr = sg_dma_address(data->sg);
+		return 0;
+	}
+
+	total_len = 0;
+	for_each_sg(data->sg, sg, data->sg_len, i)
+		total_len += sg->length;
+
+	if (!host->bounce_buf || len > host->bounce_buf_size ||
+	    total_len < len) {
+		dev_err(mmc_dev(mmc),
+			"cannot DMA %u bytes in %u segments\n",
+			len, data->sg_len);
+		return -EINVAL;
+	}
+
+	if (dir == DMA_TO_DEVICE) {
+		if (sg_copy_to_buffer(data->sg, data->sg_len, host->bounce_buf,
+				      len) != len) {
+			dev_err(mmc_dev(mmc),
+				"failed to copy data into the bounce buffer\n");
+			return -EIO;
+		}
+	}
+
+	host->dma_addr = dma_map_single(mmc_dev(mmc), host->bounce_buf, len,
+					dir);
+	if (dma_mapping_error(mmc_dev(mmc), host->dma_addr)) {
+		dev_err(mmc_dev(mmc), "dma_map_single failed\n");
 		return -ENOMEM;
 	}
 
+	host->dma_bounce = true;
+
 	return 0;
 }
 
+static void meson_mx_sdhc_unmap_dma(struct meson_mx_sdhc_host *host,
+				    struct mmc_data *data)
+{
+	struct device *dev = mmc_dev(host->mmc);
+
+	if (host->dma_bounce) {
+		dma_unmap_single(dev, host->dma_addr, host->dma_len,
+				 mmc_get_dma_dir(data));
+		host->dma_bounce = false;
+	} else {
+		dma_unmap_sg(dev, data->sg, data->sg_len,
+			     mmc_get_dma_dir(data));
+	}
+}
+
 static void meson_mx_sdhc_request(struct mmc_host *mmc, struct mmc_request *mrq)
 {
 	struct meson_mx_sdhc_host *host = mmc_priv(mmc);
 	struct mmc_command *cmd = mrq->cmd;
+	int ret = 0;
 
 	if (!host->error)
-		host->error = meson_mx_sdhc_map_dma(mmc, mrq);
+		ret = meson_mx_sdhc_map_dma(mmc, mrq);
 
-	if (host->error) {
-		cmd->error = host->error;
+	if (host->error || ret) {
+		cmd->error = host->error ? host->error : ret;
 		mmc_request_done(mmc, mrq);
 		return;
 	}
@@ -596,11 +678,6 @@ static irqreturn_t meson_mx_sdhc_irq_thread(int irq, void *irq_data)
 			regmap_update_bits(host->regmap, MESON_SDHC_PDMA, val,
 					   val);
 		}
-
-		dma_unmap_sg(mmc_dev(host->mmc), cmd->data->sg,
-			     cmd->data->sg_len, mmc_get_dma_dir(cmd->data));
-
-		cmd->data->bytes_xfered = cmd->data->blksz * cmd->data->blocks;
 	}
 
 	meson_mx_sdhc_wait_cmd_ready(host->mmc);
@@ -625,6 +702,40 @@ static irqreturn_t meson_mx_sdhc_irq_thread(int irq, void *irq_data)
 		 */
 		meson_mx_sdhc_clear_fifo(host->mmc);
 
+	if (cmd->data) {
+		struct mmc_data *data = cmd->data;
+		size_t len = data->blksz * data->blocks;
+		bool bounced = host->dma_bounce;
+
+		/*
+		 * Unmap the data after the controller has been quiesced so
+		 * that neither the reset nor the FIFO flush can race with an
+		 * ongoing DMA transfer.  dma_unmap_single() (for bounced
+		 * reads) also invalidates the CPU caches, so the copy below
+		 * sees the data written by the controller.
+		 */
+		meson_mx_sdhc_unmap_dma(host, data);
+
+		if (!data->error) {
+			if (bounced && data->flags & MMC_DATA_READ) {
+				size_t copied;
+
+				copied = sg_copy_from_buffer(data->sg,
+							     data->sg_len,
+							     host->bounce_buf,
+							     len);
+				if (copied != len) {
+					dev_err(mmc_dev(host->mmc),
+						"failed to copy data from the bounce buffer\n");
+					data->error = -EIO;
+				}
+			}
+
+			if (!data->error)
+				data->bytes_xfered = len;
+		}
+	}
+
 	meson_mx_sdhc_request_done(host);
 
 	return IRQ_HANDLED;
@@ -813,8 +924,17 @@ static int meson_mx_sdhc_probe(struct platform_device *pdev)
 
 	mmc->max_req_size = SZ_128K;
 	mmc->max_seg_size = mmc->max_req_size;
-	mmc->max_blk_count = FIELD_GET(MESON_SDHC_SEND_TOTAL_PACK, ~0);
 	mmc->max_blk_size = MESON_SDHC_MAX_BLK_SIZE;
+	/*
+	 * The controller has a single DMA address register.  Allowing the
+	 * block layer to merge more segments is only safe because requests
+	 * consisting of multiple segments are bounced through a contiguous
+	 * buffer.  Keep every request within max_req_size so that it always
+	 * fits into that buffer; SDIO uses max_blk_count to split transfers
+	 * as well.
+	 */
+	mmc->max_blk_count = mmc->max_req_size / mmc->max_blk_size;
+	mmc->max_segs = MESON_SDHC_MAX_SEGS;
 	mmc->max_busy_timeout = 30 * MSEC_PER_SEC;
 	mmc->f_min = clk_round_rate(host->sd_clk, 1);
 	mmc->f_max = clk_round_rate(host->sd_clk, ULONG_MAX);
@@ -823,6 +943,19 @@ static int meson_mx_sdhc_probe(struct platform_device *pdev)
 	mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY | MMC_CAP_HW_RESET;
 	mmc->ops = &meson_mx_sdhc_ops;
 
+	host->bounce_buf = devm_kmalloc(dev, mmc->max_req_size, GFP_KERNEL);
+	if (host->bounce_buf) {
+		host->bounce_buf_size = mmc->max_req_size;
+	} else {
+		/*
+		 * Without the bounce buffer only physically contiguous
+		 * requests can be handled, so fall back to the old limit.
+		 */
+		mmc->max_segs = 1;
+		dev_warn(dev,
+			 "failed to allocate the DMA bounce buffer, using single-segment transfers\n");
+	}
+
 	ret = mmc_of_parse(mmc);
 	if (ret)
 		goto err_disable_pclk;
-- 
2.56.0


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

Re: [RFC PATCH] mmc: meson-mx-sdhc: add DMA bounce buffer support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] mmc: meson-mx-sdhc: missing cmd->error check in meson_mx_sdhc_irq_thread()
--

commit dab15b5e51f3b15121298d5095440b6a71ad6e0f
Author: ZHENPENG YUAN <yuanzhenpeng210@gmail.com>

mmc: meson-mx-sdhc: add DMA bounce buffer support

This commit adds a 128 KiB DMA bounce buffer for multi-segment scatterlist
requests in the Meson MX SDHC driver and increases max_segs to 64. Single
segment requests continue to use direct DMA, while multi-segment transfers
are copied through the bounce buffer.

> diff --git a/drivers/mmc/host/meson-mx-sdhc-mmc.c b/drivers/mmc/host/meson-mx-sdhc-mmc.c
> index fb49ea71289e6..c62583a804f49 100644
> --- a/drivers/mmc/host/meson-mx-sdhc-mmc.c
> +++ b/drivers/mmc/host/meson-mx-sdhc-mmc.c

[ ... ]

> @@ -625,6 +702,40 @@ static irqreturn_t meson_mx_sdhc_irq_thread(int irq, void *irq_data)
>  		 */
>  		meson_mx_sdhc_clear_fifo(host->mmc);
>  
> +	if (cmd->data) {
> +		struct mmc_data *data = cmd->data;
> +		size_t len = data->blksz * data->blocks;
> +		bool bounced = host->dma_bounce;
> +
> +		/*
> +		 * Unmap the data after the controller has been quiesced so
> +		 * that neither the reset nor the FIFO flush can race with an
> +		 * ongoing DMA transfer.  dma_unmap_single() (for bounced
> +		 * reads) also invalidates the CPU caches, so the copy below
> +		 * sees the data written by the controller.
> +		 */
> +		meson_mx_sdhc_unmap_dma(host, data);
> +
> +		if (!data->error) {
> +			if (bounced && data->flags & MMC_DATA_READ) {
> +				size_t copied;
> +
> +				copied = sg_copy_from_buffer(data->sg,
> +							     data->sg_len,
> +							     host->bounce_buf,
> +							     len);
> +				if (copied != len) {
> +					dev_err(mmc_dev(host->mmc),
> +						"failed to copy data from the bounce buffer\n");
> +					data->error = -EIO;
> +				}
> +			}
> +
> +			if (!data->error)
> +				data->bytes_xfered = len;
> +		}
> +	}

[Severity: Critical]
Should this block in meson_mx_sdhc_irq_thread() check !cmd->error in addition
to !data->error before copying from host->bounce_buf and setting
data->bytes_xfered?

When a command with cmd->data encounters a FIFO overrun/underrun or a command
response CRC or timeout error, meson_mx_sdhc_irq() sets cmd->error while
leaving cmd->data->error as 0:

drivers/mmc/host/meson-mx-sdhc-mmc.c:meson_mx_sdhc_irq() {
	if (ista & MESON_SDHC_ISTA_RXFIFO_FULL ||
	    ista & MESON_SDHC_ISTA_TXFIFO_EMPTY)
		cmd->error = -EIO;
	else if (ista & MESON_SDHC_ISTA_RESP_ERR_CRC)
		cmd->error = -EILSEQ;
	else if (ista & MESON_SDHC_ISTA_RESP_TIMEOUT)
		cmd->error = -ETIMEDOUT;

	if (cmd->data) {
		if (ista & MESON_SDHC_ISTA_DATA_ERR_CRC)
			cmd->data->error = -EILSEQ;
		else if (ista & MESON_SDHC_ISTA_DATA_TIMEOUT)
			cmd->data->error = -ETIMEDOUT;
	}
}

In meson_mx_sdhc_irq_thread(), if cmd->error is -EIO or -ETIMEDOUT,
meson_mx_sdhc_reset() resets the controller. However, because the post-unmap
completion check only tests !data->error:

1. On a bounced multi-segment read, sg_copy_from_buffer() still copies len
   bytes from host->bounce_buf into data->sg even though the transfer failed
   or was aborted, overwriting the caller's scatterlist with uninitialized
   devm_kmalloc() heap memory or stale data from an earlier transfer.

2. data->bytes_xfered is set to len despite cmd->error being non-zero. In
   mmc_blk_mq_poll_completion() -> mmc_blk_mq_rw_recovery(), if CMD13 status
   polling finds the card in the transfer state without card-side error bits,
   recovery sees a non-zero brq->data.bytes_xfered and returns early:

drivers/mmc/core/block.c:mmc_blk_mq_rw_recovery() {
	...
	if (brq->data.bytes_xfered)
		return;
}

Could this cause mmc_blk_mq_complete_rq() to complete a failed read or write
request with BLK_STS_OK, resulting in silent data corruption or data loss?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261031200925.3364-1-yuanzhenpeng210@gmail.com?part=1

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

Re: [RFC PATCH] mmc: meson-mx-sdhc: add DMA bounce buffer support

[Martin Blumenstingl]

Hi,

Thank you for this patch!

On Sat, Oct 31, 2026 at 9:09 PM ZHENPENG YUAN <yuanzhenpeng210@gmail.com> wrote:
>
> The Meson MX SDHC controller uses a single DMA address for each
> transfer. Since the driver leaves max_segs at the MMC core default
> of 1, large I/O requests are split into single-segment requests,
> typically 4 KiB each.
>
> On an Amlogic S805, this limits eMMC sequential read throughput
> to approximately 15 MiB/s despite using HS200 timing.
>
> Add a 128 KiB DMA bounce buffer for multi-segment requests and
FWIW, while reviewing your patch I found that Amlogic actually uses a
512KiB bounce buffer in their downstream driver: [0]

> advertise up to 64 scatterlist segments. Single-segment requests
> continue to use direct DMA. If buffer allocation fails, fall back
> to the original single-segment limit.
>
> Testing on a Xunlei OneCloud (Meson8b, Samsung 8GME4R eMMC)
> running Linux 7.3.1:
>
>   Block size       Before       After
>   4 KiB            14.47        14.69 MiB/s
>   64 KiB           15.06        63.21 MiB/s
>   128 KiB          15.08        75.13 MiB/s
>   1 MiB            15.07        75.93 MiB/s
>
> For a 128 MiB sequential read, block requests decreased from
> 32768 to 1024.
That's a nice performance improvement!

> Direct I/O SHA256 readback verification and fio CRC32C write
> verification passed. No MMC errors were observed.
>
> Assisted-by: LLM
> Signed-off-by: ZHENPENG YUAN <yuanzhenpeng210@gmail.com>
Could you please compare your patch to simply setting mmc->caps2 |=
MMC_CAP2_MERGE_CAPABLE; ?
From what I understand this will allow physically contiguous SG
entries to be merged by mmc core.

Ulf: the code here looks similar to the sdhci.c implementation. I'm
wondering if there were any plans to somehow move this to MMC core.


Best regards,
Martin


[0] https://github.com/endlessm/linux-meson/blob/d6e13c220931110fe676ede6da69fc61a7cb04b6/arch/arm/mach-meson8/include/mach/sd.h#L627

Re: [RFC PATCH] mmc: meson-mx-sdhc: add DMA bounce buffer support

[Martin Blumenstingl]

Hi,

Thank you for this patch!

On Sat, Oct 31, 2026 at 9:09 PM ZHENPENG YUAN <yuanzhenpeng210@gmail.com> wrote:
>
> The Meson MX SDHC controller uses a single DMA address for each
> transfer. Since the driver leaves max_segs at the MMC core default
> of 1, large I/O requests are split into single-segment requests,
> typically 4 KiB each.
>
> On an Amlogic S805, this limits eMMC sequential read throughput
> to approximately 15 MiB/s despite using HS200 timing.
>
> Add a 128 KiB DMA bounce buffer for multi-segment requests and
FWIW, while reviewing your patch I found that Amlogic actually uses a
512KiB bounce buffer in their downstream driver: [0]

> advertise up to 64 scatterlist segments. Single-segment requests
> continue to use direct DMA. If buffer allocation fails, fall back
> to the original single-segment limit.
>
> Testing on a Xunlei OneCloud (Meson8b, Samsung 8GME4R eMMC)
> running Linux 7.3.1:
>
>   Block size       Before       After
>   4 KiB            14.47        14.69 MiB/s
>   64 KiB           15.06        63.21 MiB/s
>   128 KiB          15.08        75.13 MiB/s
>   1 MiB            15.07        75.93 MiB/s
>
> For a 128 MiB sequential read, block requests decreased from
> 32768 to 1024.
That's a nice performance improvement!

> Direct I/O SHA256 readback verification and fio CRC32C write
> verification passed. No MMC errors were observed.
>
> Assisted-by: LLM
> Signed-off-by: ZHENPENG YUAN <yuanzhenpeng210@gmail.com>
Could you please compare your patch to simply setting mmc->caps2 |=
MMC_CAP2_MERGE_CAPABLE; ?
From what I understand this will allow physically contiguous SG
entries to be merged by mmc core.

Ulf: the code here looks similar to the sdhci.c implementation. I'm
wondering if there were any plans to somehow move this to MMC core.


Best regards,
Martin


[0] https://github.com/endlessm/linux-meson/blob/d6e13c220931110fe676ede6da69fc61a7cb04b6/arch/arm/mach-meson8/include/mach/sd.h#L627

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

Re: [RFC PATCH] mmc: meson-mx-sdhc: add DMA bounce buffer support

[HELPMEEADICE]

Hi Martin,

Thank you again for your feedback!

I've completed the comparison between the original driver,
MMC_CAP2_MERGE_CAPABLE, and the 128 KiB bounce buffer implementation
on my Xunlei OneCloud (Amlogic S805).

The results show that MMC_CAP2_MERGE_CAPABLE alone does not improve
performance on this platform.

Runtime instrumentation confirmed that:

- dma_get_merge_boundary() returns 0.
- host->can_dma_map_merge remains 0.
- The block queue's max_segments remains 1.
- The merge-only variant behaves essentially the same as the original driver.

Here are the sequential read results (MiB/s, mean of 5 runs):

                 Baseline   Merge-only   Bounce
    4 KiB          11.81       12.20      13.01
   64 KiB          12.88       12.80      57.52
  128 KiB          12.97       12.80      67.68
    1 MiB          13.11       13.05      69.43

The bounce buffer improves sequential read throughput by up to 5.3x
and sequential write throughput by up to 2.1x.

For the 128 KiB sequential read test, instrumentation showed that the
number of MMC data requests decreased from 70,599 to 2,465, with the
average request size increasing from approximately 4 KiB to 118 KiB.

I also tested DMA mapping with 33 physically non-contiguous pages.
dma_map_sg() returned 33 DMA segments, confirming that these pages
cannot be merged into a single DMA segment on this platform.

For safety, the tested merge-only variant included a check to reject
DMA mappings with more than one segment. The pure one-line variant was
not executed.

Additionally, I addressed the issue identified by Sashiko by checking
cmd->error before copying bounced read data back and reporting
bytes_xfered.

All 135 fio jobs completed successfully. SHA256 and CRC32C integrity
checks passed, with no MMC errors observed. Fault injection testing
has not yet been performed.

Based on these results, the bounce buffer appears necessary to support
larger MMC requests on this hardware.

I plan to incorporate the error handling fix into the next revision of
the patch.

Please let me know if you have any further suggestions.

Best regards,
Zhenpeng


HELPMEEADICE <yuanzhenpeng210@gmail.com> 于2026年11月1日周日 16:46写道：
>
> Hi Martin,
>
> Thank you again for your feedback!
>
> I've completed the comparison between the original driver, MMC_CAP2_MERGE_CAPABLE, and the 128 KiB bounce buffer implementation on my Xunlei OneCloud (Amlogic S805).
>
> The results show that MMC_CAP2_MERGE_CAPABLE alone does not improve performance on this platform.
>
> Runtime instrumentation confirmed that:
>
> dma_get_merge_boundary() returns 0.
>
> host->can_dma_map_merge remains 0.
>
> The block queue's max_segments remains 1.
>
> The merge-only variant behaves essentially the same as the original driver.
>
> Here are the sequential read results (MiB/s, mean of 5 runs):
>
>              Baseline   Merge-only   Bounce
> 4 KiB          11.81       12.20      13.01
>
> 64 KiB 12.88 12.80 57.52
> 128 KiB 12.97 12.80 67.68
> 1 MiB 13.11 13.05 69.43
>
> The bounce buffer improves sequential read throughput by up to 5.3x and sequential write throughput by up to 2.1x.
>
> For the 128 KiB sequential read test, instrumentation showed that the number of MMC data requests decreased from 70,599 to 2,465, with the average request size increasing from approximately 4 KiB to 118 KiB.
>
> I also tested DMA mapping with 33 physically non-contiguous pages. dma_map_sg() returned 33 DMA segments, confirming that these pages cannot be merged into a single DMA segment on this platform.
>
> For safety, the tested merge-only variant included a check to reject DMA mappings with more than one segment. The pure one-line variant was not executed.
>
> Additionally, I addressed the issue identified by Sashiko by checking cmd->error before copying bounced read data back and reporting bytes_xfered.
>
> All 135 fio jobs completed successfully. SHA256 and CRC32C integrity checks passed, with no MMC errors observed. Fault injection testing has not yet been performed.
>
> Based on these results, the bounce buffer appears necessary to support larger MMC requests on this hardware.
>
> I plan to incorporate the error handling fix into the next revision of the patch.
>
> Please let me know if you have any further suggestions.
>
> Best regards,
> Zhenpeng
>
>
> Martin Blumenstingl <martin.blumenstingl@googlemail.com> 于2026年11月1日周日 05:17写道：
>>
>> Hi,
>>
>> Thank you for this patch!
>>
>> On Sat, Oct 31, 2026 at 9:09 PM ZHENPENG YUAN <yuanzhenpeng210@gmail.com> wrote:
>> >
>> > The Meson MX SDHC controller uses a single DMA address for each
>> > transfer. Since the driver leaves max_segs at the MMC core default
>> > of 1, large I/O requests are split into single-segment requests,
>> > typically 4 KiB each.
>> >
>> > On an Amlogic S805, this limits eMMC sequential read throughput
>> > to approximately 15 MiB/s despite using HS200 timing.
>> >
>> > Add a 128 KiB DMA bounce buffer for multi-segment requests and
>> FWIW, while reviewing your patch I found that Amlogic actually uses a
>> 512KiB bounce buffer in their downstream driver: [0]
>>
>> > advertise up to 64 scatterlist segments. Single-segment requests
>> > continue to use direct DMA. If buffer allocation fails, fall back
>> > to the original single-segment limit.
>> >
>> > Testing on a Xunlei OneCloud (Meson8b, Samsung 8GME4R eMMC)
>> > running Linux 7.3.1:
>> >
>> >   Block size       Before       After
>> >   4 KiB            14.47        14.69 MiB/s
>> >   64 KiB           15.06        63.21 MiB/s
>> >   128 KiB          15.08        75.13 MiB/s
>> >   1 MiB            15.07        75.93 MiB/s
>> >
>> > For a 128 MiB sequential read, block requests decreased from
>> > 32768 to 1024.
>> That's a nice performance improvement!
>>
>> > Direct I/O SHA256 readback verification and fio CRC32C write
>> > verification passed. No MMC errors were observed.
>> >
>> > Assisted-by: LLM
>> > Signed-off-by: ZHENPENG YUAN <yuanzhenpeng210@gmail.com>
>> Could you please compare your patch to simply setting mmc->caps2 |=
>> MMC_CAP2_MERGE_CAPABLE; ?
>> From what I understand this will allow physically contiguous SG
>> entries to be merged by mmc core.
>>
>> Ulf: the code here looks similar to the sdhci.c implementation. I'm
>> wondering if there were any plans to somehow move this to MMC core.
>>
>>
>> Best regards,
>> Martin
>>
>>
>> [0] https://github.com/endlessm/linux-meson/blob/d6e13c220931110fe676ede6da69fc61a7cb04b6/arch/arm/mach-meson8/include/mach/sd.h#L627

Re: [RFC PATCH] mmc: meson-mx-sdhc: add DMA bounce buffer support

[HELPMEEADICE]

Hi Martin,

Thank you again for your feedback!

I've completed the comparison between the original driver,
MMC_CAP2_MERGE_CAPABLE, and the 128 KiB bounce buffer implementation
on my Xunlei OneCloud (Amlogic S805).

The results show that MMC_CAP2_MERGE_CAPABLE alone does not improve
performance on this platform.

Runtime instrumentation confirmed that:

- dma_get_merge_boundary() returns 0.
- host->can_dma_map_merge remains 0.
- The block queue's max_segments remains 1.
- The merge-only variant behaves essentially the same as the original driver.

Here are the sequential read results (MiB/s, mean of 5 runs):

                 Baseline   Merge-only   Bounce
    4 KiB          11.81       12.20      13.01
   64 KiB          12.88       12.80      57.52
  128 KiB          12.97       12.80      67.68
    1 MiB          13.11       13.05      69.43

The bounce buffer improves sequential read throughput by up to 5.3x
and sequential write throughput by up to 2.1x.

For the 128 KiB sequential read test, instrumentation showed that the
number of MMC data requests decreased from 70,599 to 2,465, with the
average request size increasing from approximately 4 KiB to 118 KiB.

I also tested DMA mapping with 33 physically non-contiguous pages.
dma_map_sg() returned 33 DMA segments, confirming that these pages
cannot be merged into a single DMA segment on this platform.

For safety, the tested merge-only variant included a check to reject
DMA mappings with more than one segment. The pure one-line variant was
not executed.

Additionally, I addressed the issue identified by Sashiko by checking
cmd->error before copying bounced read data back and reporting
bytes_xfered.

All 135 fio jobs completed successfully. SHA256 and CRC32C integrity
checks passed, with no MMC errors observed. Fault injection testing
has not yet been performed.

Based on these results, the bounce buffer appears necessary to support
larger MMC requests on this hardware.

I plan to incorporate the error handling fix into the next revision of
the patch.

Please let me know if you have any further suggestions.

Best regards,
Zhenpeng


HELPMEEADICE <yuanzhenpeng210@gmail.com> 于2026年11月1日周日 16:46写道：
>
> Hi Martin,
>
> Thank you again for your feedback!
>
> I've completed the comparison between the original driver, MMC_CAP2_MERGE_CAPABLE, and the 128 KiB bounce buffer implementation on my Xunlei OneCloud (Amlogic S805).
>
> The results show that MMC_CAP2_MERGE_CAPABLE alone does not improve performance on this platform.
>
> Runtime instrumentation confirmed that:
>
> dma_get_merge_boundary() returns 0.
>
> host->can_dma_map_merge remains 0.
>
> The block queue's max_segments remains 1.
>
> The merge-only variant behaves essentially the same as the original driver.
>
> Here are the sequential read results (MiB/s, mean of 5 runs):
>
>              Baseline   Merge-only   Bounce
> 4 KiB          11.81       12.20      13.01
>
> 64 KiB 12.88 12.80 57.52
> 128 KiB 12.97 12.80 67.68
> 1 MiB 13.11 13.05 69.43
>
> The bounce buffer improves sequential read throughput by up to 5.3x and sequential write throughput by up to 2.1x.
>
> For the 128 KiB sequential read test, instrumentation showed that the number of MMC data requests decreased from 70,599 to 2,465, with the average request size increasing from approximately 4 KiB to 118 KiB.
>
> I also tested DMA mapping with 33 physically non-contiguous pages. dma_map_sg() returned 33 DMA segments, confirming that these pages cannot be merged into a single DMA segment on this platform.
>
> For safety, the tested merge-only variant included a check to reject DMA mappings with more than one segment. The pure one-line variant was not executed.
>
> Additionally, I addressed the issue identified by Sashiko by checking cmd->error before copying bounced read data back and reporting bytes_xfered.
>
> All 135 fio jobs completed successfully. SHA256 and CRC32C integrity checks passed, with no MMC errors observed. Fault injection testing has not yet been performed.
>
> Based on these results, the bounce buffer appears necessary to support larger MMC requests on this hardware.
>
> I plan to incorporate the error handling fix into the next revision of the patch.
>
> Please let me know if you have any further suggestions.
>
> Best regards,
> Zhenpeng
>
>
> Martin Blumenstingl <martin.blumenstingl@googlemail.com> 于2026年11月1日周日 05:17写道：
>>
>> Hi,
>>
>> Thank you for this patch!
>>
>> On Sat, Oct 31, 2026 at 9:09 PM ZHENPENG YUAN <yuanzhenpeng210@gmail.com> wrote:
>> >
>> > The Meson MX SDHC controller uses a single DMA address for each
>> > transfer. Since the driver leaves max_segs at the MMC core default
>> > of 1, large I/O requests are split into single-segment requests,
>> > typically 4 KiB each.
>> >
>> > On an Amlogic S805, this limits eMMC sequential read throughput
>> > to approximately 15 MiB/s despite using HS200 timing.
>> >
>> > Add a 128 KiB DMA bounce buffer for multi-segment requests and
>> FWIW, while reviewing your patch I found that Amlogic actually uses a
>> 512KiB bounce buffer in their downstream driver: [0]
>>
>> > advertise up to 64 scatterlist segments. Single-segment requests
>> > continue to use direct DMA. If buffer allocation fails, fall back
>> > to the original single-segment limit.
>> >
>> > Testing on a Xunlei OneCloud (Meson8b, Samsung 8GME4R eMMC)
>> > running Linux 7.3.1:
>> >
>> >   Block size       Before       After
>> >   4 KiB            14.47        14.69 MiB/s
>> >   64 KiB           15.06        63.21 MiB/s
>> >   128 KiB          15.08        75.13 MiB/s
>> >   1 MiB            15.07        75.93 MiB/s
>> >
>> > For a 128 MiB sequential read, block requests decreased from
>> > 32768 to 1024.
>> That's a nice performance improvement!
>>
>> > Direct I/O SHA256 readback verification and fio CRC32C write
>> > verification passed. No MMC errors were observed.
>> >
>> > Assisted-by: LLM
>> > Signed-off-by: ZHENPENG YUAN <yuanzhenpeng210@gmail.com>
>> Could you please compare your patch to simply setting mmc->caps2 |=
>> MMC_CAP2_MERGE_CAPABLE; ?
>> From what I understand this will allow physically contiguous SG
>> entries to be merged by mmc core.
>>
>> Ulf: the code here looks similar to the sdhci.c implementation. I'm
>> wondering if there were any plans to somehow move this to MMC core.
>>
>>
>> Best regards,
>> Martin
>>
>>
>> [0] https://github.com/endlessm/linux-meson/blob/d6e13c220931110fe676ede6da69fc61a7cb04b6/arch/arm/mach-meson8/include/mach/sd.h#L627

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