From: Jamin Lin via <qemu-devel@nongnu.org>
To: "Philippe Mathieu-Daudé" <philmd@linaro.org>,
"Bin Meng" <bmeng.cn@gmail.com>,
"open list:SD (Secure Card)" <qemu-block@nongnu.org>,
"open list:All patches CC here" <qemu-devel@nongnu.org>
Cc: <jamin_lin@aspeedtech.com>, <troy_lee@aspeedtech.com>,
<yunlin.tang@aspeedtech.com>
Subject: [RFC.PATCH v1 1/2] sd:sdhci: Fix boundary_count overflow in sdhci_sdma_transfer_multi_blocks
Date: Wed, 11 Dec 2024 17:51:09 +0800 [thread overview]
Message-ID: <20241211095111.1421928-2-jamin_lin@aspeedtech.com> (raw)
In-Reply-To: <20241211095111.1421928-1-jamin_lin@aspeedtech.com>
How to reproduce it:
1. The value of "s->blksie" was 0x7200. The bits[14:12] was "111", so the buffer
boundary was 0x80000.(512Kbytes). This SDMA buffer boundary the same as
u-boot default value.
The bit[11:0] is "001000000000", so the block size is 0x200.(512bytes)
2. The SDMA address was 0x83123456 which was not page aligned and
"s->sdmasysad % boundary_chk" was 0x23456. The value of boundary_count was
0x5cbaa.("boundary_chk - (s->sdmasysad % boundary_chk)" -->
"(0x80000 - 0x23456)")
However, boundary_count did not aligned the block size 512 bytes and the SDMA
address is not page aligned(0x80000), so the following if-statement never be true,
```
if (((boundary_count + begin) < block_size) && page_aligned)
````
Finally, it caused boundary_count overflow because its data type was uint32_t.
Ex: the last boundary_count was 0x1aa and "0x1aa - 0x200" became "0xffffffaa".
It is the wrong behavior.
To fix it, it seems we can directly check the "boundary_count < blocksize"
instead of "boundary_count < blocksize && page_aligned"
Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
hw/sd/sdhci.c | 18 ++++--------------
1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 37875c02c3..47d96b935b 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -590,7 +590,6 @@ static void sdhci_write_dataport(SDHCIState *s, uint32_t value, unsigned size)
/* Multi block SDMA transfer */
static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s)
{
- bool page_aligned = false;
unsigned int begin;
const uint16_t block_size = s->blksize & BLOCK_SIZE_MASK;
uint32_t boundary_chk = 1 << (((s->blksize & ~BLOCK_SIZE_MASK) >> 12) + 12);
@@ -601,15 +600,6 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s)
return;
}
- /*
- * XXX: Some sd/mmc drivers (for example, u-boot-slp) do not account for
- * possible stop at page boundary if initial address is not page aligned,
- * allow them to work properly
- */
- if ((s->sdmasysad % boundary_chk) == 0) {
- page_aligned = true;
- }
-
s->prnsts |= SDHC_DATA_INHIBIT | SDHC_DAT_LINE_ACTIVE;
if (s->trnmod & SDHC_TRNS_READ) {
s->prnsts |= SDHC_DOING_READ;
@@ -618,7 +608,7 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s)
sdbus_read_data(&s->sdbus, s->fifo_buffer, block_size);
}
begin = s->data_count;
- if (((boundary_count + begin) < block_size) && page_aligned) {
+ if (((boundary_count + begin) < block_size)) {
s->data_count = boundary_count + begin;
boundary_count = 0;
} else {
@@ -634,7 +624,7 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s)
if (s->data_count == block_size) {
s->data_count = 0;
}
- if (page_aligned && boundary_count == 0) {
+ if (boundary_count == 0) {
break;
}
}
@@ -642,7 +632,7 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s)
s->prnsts |= SDHC_DOING_WRITE;
while (s->blkcnt) {
begin = s->data_count;
- if (((boundary_count + begin) < block_size) && page_aligned) {
+ if (((boundary_count + begin) < block_size)) {
s->data_count = boundary_count + begin;
boundary_count = 0;
} else {
@@ -659,7 +649,7 @@ static void sdhci_sdma_transfer_multi_blocks(SDHCIState *s)
s->blkcnt--;
}
}
- if (page_aligned && boundary_count == 0) {
+ if (boundary_count == 0) {
break;
}
}
--
2.34.1
next prev parent reply other threads:[~2024-12-11 9:52 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-12-11 9:51 [RFC.PATCH v1 0/2] sd:sdhci Fix data transfer did not complete Jamin Lin via
2024-12-11 9:51 ` Jamin Lin via [this message]
2024-12-11 9:51 ` [RFC.PATCH v1 2/2] sd:sdhci: Fix data transfer did not complete if data size is bigger then SDMA Buffer Boundary Jamin Lin via
2024-12-13 3:14 ` [RFC.PATCH v1 0/2] sd:sdhci Fix data transfer did not complete Jamin Lin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20241211095111.1421928-2-jamin_lin@aspeedtech.com \
--to=qemu-devel@nongnu.org \
--cc=bmeng.cn@gmail.com \
--cc=jamin_lin@aspeedtech.com \
--cc=philmd@linaro.org \
--cc=qemu-block@nongnu.org \
--cc=troy_lee@aspeedtech.com \
--cc=yunlin.tang@aspeedtech.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.