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_...@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