[PATCH 0/4] Refactoring to support per-instance configurable max segments for dw_mmc

[PATCH 0/4] Refactoring to support per-instance configurable max segments for dw_mmc

[Shawn Lin]

The first patch is a defensive fix that addresses a crash caused by
incorrect max segment numbers provided by host drivers. Patches 2
through 4 then perform a step-by-step refactoring of the dw_mmc
code to support per-instance configurable max segments, which host
drivers can utilize in the future.



Shawn Lin (4):
  mmc: core: mmc: core: Add validation for host-provided max_segs
  mmc: dw_mmc: Move misplaced comment
  mmc: dw_mmc: Add desc_num field for clarity
  mmc: dw_mmc: Convert descriptor ring buffer to per-instance
    configurable

 drivers/mmc/core/queue.c  |  8 +++++++-
 drivers/mmc/host/dw_mmc.c | 27 +++++++++++++--------------
 drivers/mmc/host/dw_mmc.h |  4 +++-
 3 files changed, 23 insertions(+), 16 deletions(-)

-- 
2.7.4

[PATCH 1/4] mmc: core: mmc: core: Add validation for host-provided max_segs

[Shawn Lin]

The max_segs field is of type unsigned short, and if a host driver
sets an excessively large value, it may be truncated to zero. This
can cause mmc_alloc_sg() to call kmalloc_objs() with a zero size
allocation request, which leads to undefined behavior.

Under the SLUB allocator, kmalloc(0) returns a special pointer
(ZERO_SIZE_PTR). The subsequent 'if (sg)' check will evaluate to
true, and sg_init_table() will then attempt to access invalid memory,
resulting in a crash:

dwmmc_rockchip 2a310000.mmc: Successfully tuned phase to 133
mmc1: new UHS-I speed SDR104 SDHC card at address aaaa
Unable to handle kernel paging request at virtual address 0000001ffffffff0
Mem abort info:
ESR = 0x0000000096000004
EC = 0x25: DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
FSC = 0x04: level 0 translation fault
Data abort info:
ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
CM = 0, WnR = 0, TnD = 0, TagAccess = 0
GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
user pgtable: 4k pages, 48-bit VAs, pgdp=0000000102c88000
[0000001ffffffff0] pgd=0000000000000000, p4d=0000000000000000
Internal error: Oops: 0000000096000004 [#1] SMP
Modules linked in:
CPU: 2 UID: 0 PID: 102 Comm: kworker/2:1 Not tainted 7.0.0-rc6-next-20260331-00013-g4d93c25963c5-dirty #80 PREEMPT
Hardware name: Rockchip RK3576 EVB V10 Board (DT)
Workqueue: events_freezable mmc_rescan
pstate: 80000005 (Nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : sg_init_table+0x2c/0x50
lr : sg_init_table+0x24/0x50
sp : ffff8000837db710
x29: ffff8000837db710 x28: 000000000000c000 x27: 0000000000000300
x26: 0000000000000000 x25: 0000000000000040 x24: ffff0000c46a0000
x23: 0000000000000000 x22: ffff0000c0c73c00 x21: 0000000000000010
x20: 0000000000000010 x19: 0000000000000000 x18: 000000000000002c
x17: 0000000000000000 x16: 0000000000000001 x15: 0000000000000000
x14: 0000000000000400 x13: ffff8000837dc000 x12: 0000000000000000
x11: ffff0000c0c73ca0 x10: 0000000000000040 x9 : 459ec1f0abbdbb00
x8 : 0000001fffffffe0 x7 : 0000000000000000 x6 : 000000000000003f
x5 : 0000000000035579 x4 : 0000000000000901 x3 : 0000000000000000
x2 : 0000000000000000 x1 : 0000000000000000 x0 : 0000000000000010
Call trace:
sg_init_table+0x2c/0x50 (P)
mmc_mq_init_request+0x64/0x90
blk_mq_alloc_map_and_rqs+0x3ac/0x480
blk_mq_alloc_set_map_and_rqs+0x98/0x1e0
blk_mq_alloc_tag_set+0x1c0/0x290
mmc_init_queue+0x120/0x370
mmc_blk_alloc_req+0x150/0x420

To prevent this, add a validation check in mmc_mq_init_request() to
detect when sg_len (derived from max_segs) is zero. If sg_len is zero,
we return an error and print an error message, allowing host driver
developers to identify and fix incorrect max_segs configuration.

This is a defensive measure that ensures the MMC core fails gracefully
when host drivers provide invalid max_segs values, rather than crashing
with a page fault.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/core/queue.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
index 39fcb66..c9028e4 100644
--- a/drivers/mmc/core/queue.c
+++ b/drivers/mmc/core/queue.c
@@ -214,8 +214,14 @@ static int mmc_mq_init_request(struct blk_mq_tag_set *set, struct request *req,
 	struct mmc_queue *mq = set->driver_data;
 	struct mmc_card *card = mq->card;
 	struct mmc_host *host = card->host;
+	u16 sg_len = mmc_get_max_segments(host);
 
-	mq_rq->sg = mmc_alloc_sg(mmc_get_max_segments(host), GFP_KERNEL);
+	if (!sg_len) {
+		dev_err(mmc_dev(host), "Wrong max_segs assigned\n");
+		return -EINVAL;
+	}
+
+	mq_rq->sg = mmc_alloc_sg(sg_len, GFP_KERNEL);
 	if (!mq_rq->sg)
 		return -ENOMEM;
 
-- 
2.7.4

[PATCH 2/4] mmc: dw_mmc: Move misplaced comment

[Shawn Lin]

It was originally part of the @cmd_status field description but became
separated and now appears between @ring_size and @dms without proper context.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/host/dw_mmc.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 42e58be..14fb2b3 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -78,8 +78,8 @@ struct dw_mci_dma_slave {
  * @sg_cpu: Virtual address of DMA buffer.
  * @dma_ops: Pointer to DMA callbacks.
  * @cmd_status: Snapshot of SR taken upon completion of the current
- * @ring_size: Buffer size for idma descriptors.
  *	command. Only valid when EVENT_CMD_COMPLETE is pending.
+ * @ring_size: Buffer size for idma descriptors.
  * @dms: structure of slave-dma private data.
  * @phy_regs: physical address of controller's register map
  * @data_status: Snapshot of SR taken upon completion of the current
-- 
2.7.4

[PATCH 3/4] mmc: dw_mmc: Add desc_num field for clarity

[Shawn Lin]

The ring_size field in struct dw_mci is misleadingly named.
Despite its name, it does not represent the size of the descriptor
ring buffer in bytes, but rather the number of descriptors allocated
within the fixed-size ring buffer.

The actual ring buffer size is fixed at PAGE_SIZE (or DESC_RING_BUF_SZ,
which equals PAGE_SIZE). Within this buffer, we allocate either
struct idmac_desc or struct idmac_desc_64addr descriptors, and
ring_size stores the count of these descriptors.

This naming has caused confusion, as it's also used to set
mmc->max_segs (the maximum number of scatter-gather segments),
which logically corresponds to the number of descriptors, not a
size in bytes.

No functional change is introduced by this naming-only patch.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 drivers/mmc/host/dw_mmc.c | 16 ++++++++--------
 drivers/mmc/host/dw_mmc.h |  2 ++
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 20193ee..df6daa6 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -491,12 +491,12 @@ static int dw_mci_idmac_init(struct dw_mci *host)
 
 	if (host->dma_64bit_address == 1) {
 		struct idmac_desc_64addr *p;
-		/* Number of descriptors in the ring buffer */
-		host->ring_size =
+
+		host->desc_num =
 			DESC_RING_BUF_SZ / sizeof(struct idmac_desc_64addr);
 
 		/* Forward link the descriptor list */
-		for (i = 0, p = host->sg_cpu; i < host->ring_size - 1;
+		for (i = 0, p = host->sg_cpu; i < host->desc_num - 1;
 								i++, p++) {
 			p->des6 = (host->sg_dma +
 					(sizeof(struct idmac_desc_64addr) *
@@ -519,13 +519,13 @@ static int dw_mci_idmac_init(struct dw_mci *host)
 
 	} else {
 		struct idmac_desc *p;
-		/* Number of descriptors in the ring buffer */
-		host->ring_size =
+
+		host->desc_num =
 			DESC_RING_BUF_SZ / sizeof(struct idmac_desc);
 
 		/* Forward link the descriptor list */
 		for (i = 0, p = host->sg_cpu;
-		     i < host->ring_size - 1;
+		     i < host->desc_num - 1;
 		     i++, p++) {
 			p->des3 = cpu_to_le32(host->sg_dma +
 					(sizeof(struct idmac_desc) * (i + 1)));
@@ -2858,10 +2858,10 @@ static int dw_mci_init_host(struct dw_mci *host)
 
 	/* Useful defaults if platform data is unset. */
 	if (host->use_dma == TRANS_MODE_IDMAC) {
-		mmc->max_segs = host->ring_size;
+		mmc->max_segs = host->desc_num;
 		mmc->max_blk_size = 65535;
 		mmc->max_seg_size = 0x1000;
-		mmc->max_req_size = mmc->max_seg_size * host->ring_size;
+		mmc->max_req_size = mmc->max_seg_size * host->desc_num;
 		mmc->max_blk_count = mmc->max_req_size / 512;
 	} else if (host->use_dma == TRANS_MODE_EDMAC) {
 		mmc->max_segs = 64;
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 14fb2b3..a05100c 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -80,6 +80,7 @@ struct dw_mci_dma_slave {
  * @cmd_status: Snapshot of SR taken upon completion of the current
  *	command. Only valid when EVENT_CMD_COMPLETE is pending.
  * @ring_size: Buffer size for idma descriptors.
+ * @desc_num: Number of idmac descriptors available.
  * @dms: structure of slave-dma private data.
  * @phy_regs: physical address of controller's register map
  * @data_status: Snapshot of SR taken upon completion of the current
@@ -185,6 +186,7 @@ struct dw_mci {
 	const struct dw_mci_dma_ops	*dma_ops;
 	/* For idmac */
 	unsigned int		ring_size;
+	unsigned short		desc_num;
 
 	/* For edmac */
 	struct dw_mci_dma_slave *dms;
-- 
2.7.4

[PATCH 4/4] mmc: dw_mmc: Convert descriptor ring buffer to per-instance configurable

[Shawn Lin]

Replace the hardcoded DESC_RING_BUF_SZ macro with a per-instance
ring_size member in struct dw_mci. This change provides greater
flexibility and prepares the driver for future configuration options.

Variant host controllers can now override the default ring_size via
the struct dw_mci_drv_data::init() callback, allowing them to tune
the descriptor ring buffer size for their specific use cases. This
is particularly beneficial for improving performance in large-block
sequential read/write scenarios.

Empirical testing shows that increasing ring_size can significantly
improve request efficiency. For example, the block count per request
can increase from 0x800 (1 MiB) to 0x2000 (4 MiB), as demonstrated
by trace data:

    dd-706     [004] .....   106.017566: mmc_request_start: mmc1: start
    struct mmc_request[0000000066f43a37]: ... sbc_arg=0x800

    dd-697     [001] .....    15.227953: mmc_request_start: mmc1: start
    struct mmc_request[00000000d82bf187]: ... sbc_arg=0x2000

While increasing the request size improves sequential I/O throughput,
it also introduces trade-offs: larger requests can delay other pending
I/O operations. Therefore, this configuration should be balanced
according to the specific workload and not hardcoded globally.

The default ring_size is initialized to PAGE_SIZE in dw_mci_alloc_host(),
preserving existing behavior. All buffer size calculations now use
host->ring_size instead of the hardcoded macro.

No functional changes are introduced for existing platforms.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>

---

 drivers/mmc/host/dw_mmc.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index df6daa6..61f10e7 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -50,8 +50,6 @@
 				 SDMMC_IDMAC_INT_FBE | SDMMC_IDMAC_INT_RI | \
 				 SDMMC_IDMAC_INT_TI)
 
-#define DESC_RING_BUF_SZ	PAGE_SIZE
-
 struct idmac_desc_64addr {
 	u32		des0;	/* Control Descriptor */
 #define IDMAC_OWN_CLR64(x) \
@@ -493,7 +491,7 @@ static int dw_mci_idmac_init(struct dw_mci *host)
 		struct idmac_desc_64addr *p;
 
 		host->desc_num =
-			DESC_RING_BUF_SZ / sizeof(struct idmac_desc_64addr);
+			host->ring_size / sizeof(struct idmac_desc_64addr);
 
 		/* Forward link the descriptor list */
 		for (i = 0, p = host->sg_cpu; i < host->desc_num - 1;
@@ -521,7 +519,7 @@ static int dw_mci_idmac_init(struct dw_mci *host)
 		struct idmac_desc *p;
 
 		host->desc_num =
-			DESC_RING_BUF_SZ / sizeof(struct idmac_desc);
+			host->ring_size / sizeof(struct idmac_desc);
 
 		/* Forward link the descriptor list */
 		for (i = 0, p = host->sg_cpu;
@@ -653,7 +651,7 @@ static inline int dw_mci_prepare_desc(struct dw_mci *host, struct mmc_data *data
 err_own_bit:
 	/* restore the descriptor chain as it's polluted */
 	dev_dbg(host->dev, "descriptor is still owned by IDMAC.\n");
-	memset(host->sg_cpu, 0, DESC_RING_BUF_SZ);
+	memset(host->sg_cpu, 0, host->ring_size);
 	dw_mci_idmac_init(host);
 	return -EINVAL;
 }
@@ -2954,7 +2952,7 @@ static void dw_mci_init_dma(struct dw_mci *host)
 
 		/* Alloc memory for sg translation */
 		host->sg_cpu = dmam_alloc_coherent(host->dev,
-						   DESC_RING_BUF_SZ,
+						   host->ring_size,
 						   &host->sg_dma, GFP_KERNEL);
 		if (!host->sg_cpu) {
 			dev_err(host->dev,
@@ -3185,6 +3183,7 @@ struct dw_mci *dw_mci_alloc_host(struct device *dev)
 	host = mmc_priv(mmc);
 	host->mmc = mmc;
 	host->dev = dev;
+	host->ring_size = PAGE_SIZE;
 
 	return host;
 }
-- 
2.7.4