Skip to contentSkip to conversation

mmc: sdhci: Fix ADMA for PAGE_SIZE >= 64KiB

PATCH In review Adrian Hunter 3 participants 5 messages Nov 15, 2021 – Nov 18, 2021

Subsystems (2)
MMC, SD and SDIO Secure digital host controller interface (SDHCI) driver
lore
Share & tools
lore

Adrian Hunter

author maintainer
#0 lore raw

PATCH 1/1 mmc: sdhci: Fix ADMA for PAGE_SIZE >= 64KiB

The block layer forces a minimum segment size of PAGE_SIZE, so a segment can be too big for the ADMA table, if PAGE_SIZE >= 64KiB. Fix by writing multiple descriptors, noting that the ADMA table is sized for 4KiB chunks anyway, so it will be big enough.

Reported-and-tested-by Bough Chen <[email protected]>
Signed-off-by Adrian Hunter <[email protected]>
---
 drivers/mmc/host/sdhci.c | 21 ++++++++++++++++++---
 drivers/mmc/host/sdhci.h |  4 +++-
 2 files changed, 21 insertions(+), 4 deletions(-)
drivers/mmc/host/sdhci.c +18 -3
index 269c86569402..07c6da1f2f0f 100644
@@ -771,7 +771,19 @@ static void sdhci_adma_table_pre(struct sdhci_host *host,
 			len -= offset;
 		}
 
-		BUG_ON(len > 65536);
+		/*
+		 * The block layer forces a minimum segment size of PAGE_SIZE,
+		 * so 'len' can be too big here if PAGE_SIZE >= 64KiB. Write
+		 * multiple descriptors, noting that the ADMA table is sized
+		 * for 4KiB chunks anyway, so it will be big enough.
+		 */
+		while (len > host->max_adma) {
+			int n = 32 * 1024; /* 32KiB*/
+
+			__sdhci_adma_write_desc(host, &desc, addr, n, ADMA2_TRAN_VALID);
+			addr += n;
+			len -= n;
+		}
 
 		/* tran, valid */
 		if (len)

@@ -3968,6 +3980,7 @@ struct sdhci_host *sdhci_alloc_host(struct device *dev,
 	 * descriptor for each segment, plus 1 for a nop end descriptor.
 	 */
 	host->adma_table_cnt = SDHCI_MAX_SEGS * 2 + 1;
+	host->max_adma = 65536;
 
 	host->max_timeout_count = 0xE;
 

@@ -4633,10 +4646,12 @@ int sdhci_setup_host(struct sdhci_host *host)
 	 * be larger than 64 KiB though.
 	 */
 	if (host->flags & SDHCI_USE_ADMA) {
-		if (host->quirks & SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC)
+		if (host->quirks & SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC) {
+			host->max_adma = 65532; /* 32-bit alignment */
 			mmc->max_seg_size = 65535;
-		else
+		} else {
 			mmc->max_seg_size = 65536;
+		}
 	} else {
 		mmc->max_seg_size = mmc->max_req_size;
 	}
drivers/mmc/host/sdhci.h +3 -1
index bb883553d3b4..d7929d725730 100644
@@ -340,7 +340,8 @@ struct sdhci_adma2_64_desc {
 
 /*
  * Maximum segments assuming a 512KiB maximum requisition size and a minimum
- * 4KiB page size.
+ * 4KiB page size. Note this also allows enough for multiple descriptors in
+ * case of PAGE_SIZE >= 64KiB.
  */
 #define SDHCI_MAX_SEGS		128
 

@@ -543,6 +544,7 @@ struct sdhci_host {
 	unsigned int blocks;	/* remaining PIO blocks */
 
 	int sg_count;		/* Mapped sg entries */
+	int max_adma;		/* Max. length in ADMA descriptor */
 
 	void *adma_table;	/* ADMA descriptor table */
 	void *align_buffer;	/* Bounce buffer */
Signature · 1 line
2.25.1
#1 lore raw
Quoted 8 lines · Adrian Hunter

The block layer forces a minimum segment size of PAGE_SIZE, so a segment can be too big for the ADMA table, if PAGE_SIZE >= 64KiB. Fix by writing multiple descriptors, noting that the ADMA table is sized for 4KiB chunks anyway, so it will be big enough.

Reported-and-tested-by Bough Chen <[email protected]>
Signed-off-by Adrian Hunter <[email protected]>

Applied for next, thanks!

Should we perhaps tag this for stable and/or queue it as a fix for v5.16?

Kind regards
Uffe

Quoted 78 lines
---
 drivers/mmc/host/sdhci.c | 21 ++++++++++++++++++---
 drivers/mmc/host/sdhci.h |  4 +++-
 2 files changed, 21 insertions(+), 4 deletions(-)
drivers/mmc/host/sdhci.c +18 -3
index 269c86569402..07c6da1f2f0f 100644
@@ -771,7 +771,19 @@ static void sdhci_adma_table_pre(struct sdhci_host *host,
                        len -= offset;
                }
 
-               BUG_ON(len > 65536);
+               /*
+                * The block layer forces a minimum segment size of PAGE_SIZE,
+                * so 'len' can be too big here if PAGE_SIZE >= 64KiB. Write
+                * multiple descriptors, noting that the ADMA table is sized
+                * for 4KiB chunks anyway, so it will be big enough.
+                */
+               while (len > host->max_adma) {
+                       int n = 32 * 1024; /* 32KiB*/
+
+                       __sdhci_adma_write_desc(host, &desc, addr, n, ADMA2_TRAN_VALID);
+                       addr += n;
+                       len -= n;
+               }
 
                /* tran, valid */
                if (len)

@@ -3968,6 +3980,7 @@ struct sdhci_host *sdhci_alloc_host(struct device *dev,
         * descriptor for each segment, plus 1 for a nop end descriptor.
         */
        host->adma_table_cnt = SDHCI_MAX_SEGS * 2 + 1;
+       host->max_adma = 65536;
 
        host->max_timeout_count = 0xE;
 

@@ -4633,10 +4646,12 @@ int sdhci_setup_host(struct sdhci_host *host)
         * be larger than 64 KiB though.
         */
        if (host->flags & SDHCI_USE_ADMA) {
-               if (host->quirks & SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC)
+               if (host->quirks & SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC) {
+                       host->max_adma = 65532; /* 32-bit alignment */
                        mmc->max_seg_size = 65535;
-               else
+               } else {
                        mmc->max_seg_size = 65536;
+               }
        } else {
                mmc->max_seg_size = mmc->max_req_size;
        }
drivers/mmc/host/sdhci.h +3 -2
index bb883553d3b4..d7929d725730 100644
@@ -340,7 +340,8 @@ struct sdhci_adma2_64_desc {
 
 /*
  * Maximum segments assuming a 512KiB maximum requisition size and a minimum
- * 4KiB page size.
+ * 4KiB page size. Note this also allows enough for multiple descriptors in
+ * case of PAGE_SIZE >= 64KiB.
  */
 #define SDHCI_MAX_SEGS         128
 

@@ -543,6 +544,7 @@ struct sdhci_host {
        unsigned int blocks;    /* remaining PIO blocks */
 
        int sg_count;           /* Mapped sg entries */
+       int max_adma;           /* Max. length in ADMA descriptor */
 
        void *adma_table;       /* ADMA descriptor table */
        void *align_buffer;     /* Bounce buffer */
--

2.25.1

author maintainer
#2 lore raw
Quoted 7 lines · Ulf Hansson
Quoted 6 lines · Adrian Hunter

The block layer forces a minimum segment size of PAGE_SIZE, so a segment can be too big for the ADMA table, if PAGE_SIZE >= 64KiB. Fix by writing multiple descriptors, noting that the ADMA table is sized for 4KiB chunks anyway, so it will be big enough.

Quoted 6 lines
Quoted 2 lines
Reported-and-tested-by Bough Chen <[email protected]>
Signed-off-by Adrian Hunter <[email protected]>

Applied for next, thanks!

Should we perhaps tag this for stable and/or queue it as a fix for v5.16?

Could do. I thought I would leave it up to Haibo Chen.

Quoted 84 lines

Kind regards
Uffe

Quoted 78 lines
---
 drivers/mmc/host/sdhci.c | 21 ++++++++++++++++++---
 drivers/mmc/host/sdhci.h |  4 +++-
 2 files changed, 21 insertions(+), 4 deletions(-)
drivers/mmc/host/sdhci.c +18 -3
index 269c86569402..07c6da1f2f0f 100644
@@ -771,7 +771,19 @@ static void sdhci_adma_table_pre(struct sdhci_host *host,
                        len -= offset;
                }
 
-               BUG_ON(len > 65536);
+               /*
+                * The block layer forces a minimum segment size of PAGE_SIZE,
+                * so 'len' can be too big here if PAGE_SIZE >= 64KiB. Write
+                * multiple descriptors, noting that the ADMA table is sized
+                * for 4KiB chunks anyway, so it will be big enough.
+                */
+               while (len > host->max_adma) {
+                       int n = 32 * 1024; /* 32KiB*/
+
+                       __sdhci_adma_write_desc(host, &desc, addr, n, ADMA2_TRAN_VALID);
+                       addr += n;
+                       len -= n;
+               }
 
                /* tran, valid */
                if (len)

@@ -3968,6 +3980,7 @@ struct sdhci_host *sdhci_alloc_host(struct device *dev,
         * descriptor for each segment, plus 1 for a nop end descriptor.
         */
        host->adma_table_cnt = SDHCI_MAX_SEGS * 2 + 1;
+       host->max_adma = 65536;
 
        host->max_timeout_count = 0xE;
 

@@ -4633,10 +4646,12 @@ int sdhci_setup_host(struct sdhci_host *host)
         * be larger than 64 KiB though.
         */
        if (host->flags & SDHCI_USE_ADMA) {
-               if (host->quirks & SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC)
+               if (host->quirks & SDHCI_QUIRK_BROKEN_ADMA_ZEROLEN_DESC) {
+                       host->max_adma = 65532; /* 32-bit alignment */
                        mmc->max_seg_size = 65535;
-               else
+               } else {
                        mmc->max_seg_size = 65536;
+               }
        } else {
                mmc->max_seg_size = mmc->max_req_size;
        }
drivers/mmc/host/sdhci.h +3 -2
index bb883553d3b4..d7929d725730 100644
@@ -340,7 +340,8 @@ struct sdhci_adma2_64_desc {
 
 /*
  * Maximum segments assuming a 512KiB maximum requisition size and a minimum
- * 4KiB page size.
+ * 4KiB page size. Note this also allows enough for multiple descriptors in
+ * case of PAGE_SIZE >= 64KiB.
  */
 #define SDHCI_MAX_SEGS         128
 

@@ -543,6 +544,7 @@ struct sdhci_host {
        unsigned int blocks;    /* remaining PIO blocks */
 
        int sg_count;           /* Mapped sg entries */
+       int max_adma;           /* Max. length in ADMA descriptor */
 
        void *adma_table;       /* ADMA descriptor table */
        void *align_buffer;     /* Bounce buffer */
--

2.25.1

#3 lore raw
Quoted 9 lines · Adrian Hunter
Quoted 7 lines · Ulf Hansson
Quoted 6 lines · Adrian Hunter

The block layer forces a minimum segment size of PAGE_SIZE, so a segment can be too big for the ADMA table, if PAGE_SIZE >= 64KiB. Fix by writing multiple descriptors, noting that the ADMA table is sized for 4KiB chunks anyway, so it will be big enough.

Quoted 8 lines
Quoted 6 lines
Quoted 2 lines
Reported-and-tested-by Bough Chen <[email protected]>
Signed-off-by Adrian Hunter <[email protected]>

Applied for next, thanks!

Should we perhaps tag this for stable and/or queue it as a fix for v5.16?

Could do. I thought I would leave it up to Haibo Chen.

I have moved it to the fixes branch and added a stable tag. Haibo, please tell me if you think that isn't needed.

[...]

Kind regards
Uffe

#4 lore raw
Quoted 23 lines

-----Original Message-----
From: Ulf Hansson [mailto:[email protected]]
Sent: 2021年11月17日 17:38
To: Adrian Hunter <[email protected]>
Cc: Bough Chen <[email protected]>; linux-mmc <[email protected]>
Subject: Re: [PATCH] mmc: sdhci: Fix ADMA for PAGE_SIZE >= 64KiB

Quoted 3 lines · Adrian Hunter
Quoted 1 line · Ulf Hansson

On Mon, 15 Nov 2021 at 09:24, Adrian Hunter <[email protected]>

Quoted 9 lines · wrote:
Quoted 9 lines
Quoted 8 lines

The block layer forces a minimum segment size of PAGE_SIZE, so a segment can be too big for the ADMA table, if PAGE_SIZE >= 64KiB. Fix by writing multiple descriptors, noting that the ADMA table is sized for 4KiB chunks anyway, so it will be big enough.

Reported-and-tested-by Bough Chen <[email protected]>
Signed-off-by Adrian Hunter <[email protected]>
Quoted 9 lines
Quoted 5 lines
Quoted 3 lines

Applied for next, thanks!

Should we perhaps tag this for stable and/or queue it as a fix for v5.16?

Could do. I thought I would leave it up to Haibo Chen.

I have moved it to the fixes branch and added a stable tag. Haibo, please tell me if you think that isn't needed.

I'm okay with this.

Best Regards
Haibo Chen

Quoted 4 lines

[...]

Kind regards
Uffe