* [PULL 01/13] hw/sd/sdhci: Fix qemu_log_mask() format string
2020-10-21 17:34 [PULL 00/13] SD/MMC patches for 2020-10-21 Philippe Mathieu-Daudé
@ 2020-10-21 17:34 ` Philippe Mathieu-Daudé
2020-10-21 17:34 ` [PULL 02/13] hw/sd/sdhci: Document the datasheet used Philippe Mathieu-Daudé
` (12 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-21 17:34 UTC (permalink / raw)
To: qemu-devel
Cc: Alexander Bulekov, Richard Henderson, Philippe Mathieu-Daudé
Add missing newline character in qemu_log_mask() format.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Tested-by: Alexander Bulekov <alxndr@bu.edu>
Message-Id: <20200901140411.112150-2-f4bug@amsat.org>
---
hw/sd/sdhci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 69002130839..6d4603f5b19 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1112,7 +1112,7 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
/* Limit block size to the maximum buffer size */
if (extract32(s->blksize, 0, 12) > s->buf_maxsz) {
qemu_log_mask(LOG_GUEST_ERROR, "%s: Size 0x%x is larger than "
- "the maximum buffer 0x%x", __func__, s->blksize,
+ "the maximum buffer 0x%x\n", __func__, s->blksize,
s->buf_maxsz);
s->blksize = deposit32(s->blksize, 0, 12, s->buf_maxsz);
--
2.26.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 02/13] hw/sd/sdhci: Document the datasheet used
2020-10-21 17:34 [PULL 00/13] SD/MMC patches for 2020-10-21 Philippe Mathieu-Daudé
2020-10-21 17:34 ` [PULL 01/13] hw/sd/sdhci: Fix qemu_log_mask() format string Philippe Mathieu-Daudé
@ 2020-10-21 17:34 ` Philippe Mathieu-Daudé
2020-10-21 17:34 ` [PULL 03/13] hw/sd/sdhci: Fix DMA Transfer Block Size field Philippe Mathieu-Daudé
` (11 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-21 17:34 UTC (permalink / raw)
To: qemu-devel
Cc: Alexander Bulekov, Richard Henderson, Philippe Mathieu-Daudé
Add datasheet name in the file header.
We can not add the direct download link since there is a disclaimers
to agree first on the SD Association website (www.sdcard.org).
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Tested-by: Alexander Bulekov <alxndr@bu.edu>
Message-Id: <20200901140411.112150-3-f4bug@amsat.org>
---
hw/sd/sdhci.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 6d4603f5b19..ed717ab549b 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1,6 +1,8 @@
/*
* SD Association Host Standard Specification v2.0 controller emulation
*
+ * Datasheet: PartA2_SD_Host_Controller_Simplified_Specification_Ver2.00.pdf
+ *
* Copyright (c) 2011 Samsung Electronics Co., Ltd.
* Mitsyanko Igor <i.mitsyanko@samsung.com>
* Peter A.G. Crosthwaite <peter.crosthwaite@petalogix.com>
--
2.26.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 03/13] hw/sd/sdhci: Fix DMA Transfer Block Size field
2020-10-21 17:34 [PULL 00/13] SD/MMC patches for 2020-10-21 Philippe Mathieu-Daudé
2020-10-21 17:34 ` [PULL 01/13] hw/sd/sdhci: Fix qemu_log_mask() format string Philippe Mathieu-Daudé
2020-10-21 17:34 ` [PULL 02/13] hw/sd/sdhci: Document the datasheet used Philippe Mathieu-Daudé
@ 2020-10-21 17:34 ` Philippe Mathieu-Daudé
2020-10-21 17:34 ` [PULL 04/13] hw/sd/sdhci: Stop multiple transfers when block count is cleared Philippe Mathieu-Daudé
` (10 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-21 17:34 UTC (permalink / raw)
To: qemu-devel
Cc: Alexander Bulekov, Prasad J Pandit, Philippe Mathieu-Daudé,
qemu-stable
The 'Transfer Block Size' field is 12-bit wide.
See section '2.2.2. Block Size Register (Offset 004h)' in datasheet.
Two different bug reproducer available:
- https://bugs.launchpad.net/qemu/+bug/1892960
- https://ruhr-uni-bochum.sciebo.de/s/NNWP2GfwzYKeKwE?path=%2Fsdhci_oob_write1
Cc: qemu-stable@nongnu.org
Buglink: https://bugs.launchpad.net/qemu/+bug/1892960
Fixes: d7dfca0807a ("hw/sdhci: introduce standard SD host controller")
Reported-by: Alexander Bulekov <alxndr@bu.edu>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Prasad J Pandit <pjp@fedoraproject.org>
Tested-by: Alexander Bulekov <alxndr@bu.edu>
Message-Id: <20200901140411.112150-3-f4bug@amsat.org>
---
hw/sd/sdhci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index ed717ab549b..c849c3d99b8 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1107,7 +1107,7 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
break;
case SDHC_BLKSIZE:
if (!TRANSFERRING_DATA(s->prnsts)) {
- MASKED_WRITE(s->blksize, mask, value);
+ MASKED_WRITE(s->blksize, mask, extract32(value, 0, 12));
MASKED_WRITE(s->blkcnt, mask >> 16, value >> 16);
}
--
2.26.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 04/13] hw/sd/sdhci: Stop multiple transfers when block count is cleared
2020-10-21 17:34 [PULL 00/13] SD/MMC patches for 2020-10-21 Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2020-10-21 17:34 ` [PULL 03/13] hw/sd/sdhci: Fix DMA Transfer Block Size field Philippe Mathieu-Daudé
@ 2020-10-21 17:34 ` Philippe Mathieu-Daudé
2020-10-21 17:34 ` [PULL 05/13] hw/sd/sdhci: Resume pending DMA transfers on MMIO accesses Philippe Mathieu-Daudé
` (9 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-21 17:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexander Bulekov, Philippe Mathieu-Daudé
Clearing BlockCount stops multiple transfers.
See "SD Host Controller Simplified Specification Version 2.00":
- 2.2.3. Block Count Register (Offset 006h)
- Table 2-8 : Determination of Transfer Type
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Alexander Bulekov <alxndr@bu.edu>
Message-Id: <20200903172806.489710-2-f4bug@amsat.org>
---
hw/sd/sdhci.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index c849c3d99b8..61e469bd32f 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -731,6 +731,12 @@ static void sdhci_do_adma(SDHCIState *s)
ADMADescr dscr = {};
int i;
+ if (s->trnmod & SDHC_TRNS_BLK_CNT_EN && !s->blkcnt) {
+ /* Stop Multiple Transfer */
+ sdhci_end_transfer(s);
+ return;
+ }
+
for (i = 0; i < SDHC_ADMA_DESCS_PER_DELAY; ++i) {
s->admaerr &= ~SDHC_ADMAERR_LENGTH_MISMATCH;
@@ -756,7 +762,6 @@ static void sdhci_do_adma(SDHCIState *s)
switch (dscr.attr & SDHC_ADMA_ATTR_ACT_MASK) {
case SDHC_ADMA_ATTR_ACT_TRAN: /* data transfer */
-
if (s->trnmod & SDHC_TRNS_READ) {
while (length) {
if (s->data_count == 0) {
--
2.26.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 05/13] hw/sd/sdhci: Resume pending DMA transfers on MMIO accesses
2020-10-21 17:34 [PULL 00/13] SD/MMC patches for 2020-10-21 Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2020-10-21 17:34 ` [PULL 04/13] hw/sd/sdhci: Stop multiple transfers when block count is cleared Philippe Mathieu-Daudé
@ 2020-10-21 17:34 ` Philippe Mathieu-Daudé
2020-10-21 17:34 ` [PULL 06/13] hw/sd/sdhci: Let sdhci_update_irq() return if IRQ was delivered Philippe Mathieu-Daudé
` (8 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-21 17:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexander Bulekov, Philippe Mathieu-Daudé
If we have pending DMA requests scheduled, process them first.
So far we don't need to implement a bottom half to process them.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Alexander Bulekov <alxndr@bu.edu>
Message-Id: <20200903172806.489710-3-f4bug@amsat.org>
---
hw/sd/sdhci.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 61e469bd32f..4db77decf87 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -948,11 +948,21 @@ sdhci_buff_access_is_sequential(SDHCIState *s, unsigned byte_num)
return true;
}
+static void sdhci_resume_pending_transfer(SDHCIState *s)
+{
+ timer_del(s->transfer_timer);
+ sdhci_data_transfer(s);
+}
+
static uint64_t sdhci_read(void *opaque, hwaddr offset, unsigned size)
{
SDHCIState *s = (SDHCIState *)opaque;
uint32_t ret = 0;
+ if (timer_pending(s->transfer_timer)) {
+ sdhci_resume_pending_transfer(s);
+ }
+
switch (offset & ~0x3) {
case SDHC_SYSAD:
ret = s->sdmasysad;
@@ -1096,6 +1106,10 @@ sdhci_write(void *opaque, hwaddr offset, uint64_t val, unsigned size)
uint32_t value = val;
value <<= shift;
+ if (timer_pending(s->transfer_timer)) {
+ sdhci_resume_pending_transfer(s);
+ }
+
switch (offset & ~0x3) {
case SDHC_SYSAD:
s->sdmasysad = (s->sdmasysad & mask) | value;
--
2.26.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 06/13] hw/sd/sdhci: Let sdhci_update_irq() return if IRQ was delivered
2020-10-21 17:34 [PULL 00/13] SD/MMC patches for 2020-10-21 Philippe Mathieu-Daudé
` (4 preceding siblings ...)
2020-10-21 17:34 ` [PULL 05/13] hw/sd/sdhci: Resume pending DMA transfers on MMIO accesses Philippe Mathieu-Daudé
@ 2020-10-21 17:34 ` Philippe Mathieu-Daudé
2020-10-21 17:34 ` [PULL 07/13] hw/sd/sdhci: Yield if interrupt delivered during multiple transfer Philippe Mathieu-Daudé
` (7 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-21 17:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexander Bulekov, Philippe Mathieu-Daudé
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Alexander Bulekov <alxndr@bu.edu>
Message-Id: <20200903172806.489710-4-f4bug@amsat.org>
---
hw/sd/sdhci.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 4db77decf87..b93ecefd20c 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -218,9 +218,14 @@ static uint8_t sdhci_slotint(SDHCIState *s)
((s->norintsts & SDHC_NIS_REMOVE) && (s->wakcon & SDHC_WKUP_ON_RMV));
}
-static inline void sdhci_update_irq(SDHCIState *s)
+/* Return true if IRQ was pending and delivered */
+static bool sdhci_update_irq(SDHCIState *s)
{
- qemu_set_irq(s->irq, sdhci_slotint(s));
+ bool pending = sdhci_slotint(s);
+
+ qemu_set_irq(s->irq, pending);
+
+ return pending;
}
static void sdhci_raise_insertion_irq(void *opaque)
--
2.26.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 07/13] hw/sd/sdhci: Yield if interrupt delivered during multiple transfer
2020-10-21 17:34 [PULL 00/13] SD/MMC patches for 2020-10-21 Philippe Mathieu-Daudé
` (5 preceding siblings ...)
2020-10-21 17:34 ` [PULL 06/13] hw/sd/sdhci: Let sdhci_update_irq() return if IRQ was delivered Philippe Mathieu-Daudé
@ 2020-10-21 17:34 ` Philippe Mathieu-Daudé
2020-10-21 17:34 ` [PULL 08/13] hw/sd/sdcard: Add trace event for ERASE command (CMD38) Philippe Mathieu-Daudé
` (6 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-21 17:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexander Bulekov, Philippe Mathieu-Daudé
The Descriptor Table has a bit to allow the DMA to generates
Interrupt when the operation of the descriptor line is completed
(see "1.13.4. Descriptor Table" of 'SD Host Controller Simplified
Specification Version 2.00').
If we have pending interrupt and the descriptor requires it
to be generated as soon as it is completed, reschedule pending
transfers and yield to the CPU.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Alexander Bulekov <alxndr@bu.edu>
Message-Id: <20200903172806.489710-5-f4bug@amsat.org>
---
hw/sd/sdhci.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index b93ecefd20c..2f8b74a84f7 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -837,7 +837,10 @@ static void sdhci_do_adma(SDHCIState *s)
s->norintsts |= SDHC_NIS_DMA;
}
- sdhci_update_irq(s);
+ if (sdhci_update_irq(s) && !(dscr.attr & SDHC_ADMA_ATTR_END)) {
+ /* IRQ delivered, reschedule current transfer */
+ break;
+ }
}
/* ADMA transfer terminates if blkcnt == 0 or by END attribute */
--
2.26.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 08/13] hw/sd/sdcard: Add trace event for ERASE command (CMD38)
2020-10-21 17:34 [PULL 00/13] SD/MMC patches for 2020-10-21 Philippe Mathieu-Daudé
` (6 preceding siblings ...)
2020-10-21 17:34 ` [PULL 07/13] hw/sd/sdhci: Yield if interrupt delivered during multiple transfer Philippe Mathieu-Daudé
@ 2020-10-21 17:34 ` Philippe Mathieu-Daudé
2020-10-21 17:34 ` [PULL 09/13] hw/sd/sdcard: Introduce the INVALID_ADDRESS definition Philippe Mathieu-Daudé
` (5 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-21 17:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexander Bulekov, Philippe Mathieu-Daudé
Trace addresses provided to the ERASE command.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Alexander Bulekov <alxndr@bu.edu>
Message-Id: <20201015063824.212980-2-f4bug@amsat.org>
---
hw/sd/sd.c | 2 +-
hw/sd/trace-events | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 00128822224..2606b969e34 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -749,7 +749,7 @@ static void sd_erase(SDState *sd)
uint64_t erase_start = sd->erase_start;
uint64_t erase_end = sd->erase_end;
- trace_sdcard_erase();
+ trace_sdcard_erase(sd->erase_start, sd->erase_end);
if (!sd->erase_start || !sd->erase_end) {
sd->card_status |= ERASE_SEQ_ERROR;
return;
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index a87d7355fb8..96c7ea5e52f 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -46,7 +46,7 @@ sdcard_reset(void) ""
sdcard_set_blocklen(uint16_t length) "0x%03x"
sdcard_inserted(bool readonly) "read_only: %u"
sdcard_ejected(void) ""
-sdcard_erase(void) ""
+sdcard_erase(uint32_t first, uint32_t last) "addr first 0x%" PRIx32" last 0x%" PRIx32
sdcard_lock(void) ""
sdcard_unlock(void) ""
sdcard_read_block(uint64_t addr, uint32_t len) "addr 0x%" PRIx64 " size 0x%x"
--
2.26.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 09/13] hw/sd/sdcard: Introduce the INVALID_ADDRESS definition
2020-10-21 17:34 [PULL 00/13] SD/MMC patches for 2020-10-21 Philippe Mathieu-Daudé
` (7 preceding siblings ...)
2020-10-21 17:34 ` [PULL 08/13] hw/sd/sdcard: Add trace event for ERASE command (CMD38) Philippe Mathieu-Daudé
@ 2020-10-21 17:34 ` Philippe Mathieu-Daudé
2020-10-21 17:34 ` [PULL 10/13] hw/sd/sdcard: Do not use legal address '0' for INVALID_ADDRESS Philippe Mathieu-Daudé
` (4 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-21 17:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexander Bulekov, Philippe Mathieu-Daudé
'0' is used as a value to indicate an invalid (or unset)
address. Use a definition instead of a magic value.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Alexander Bulekov <alxndr@bu.edu>
Message-Id: <20201015063824.212980-3-f4bug@amsat.org>
---
hw/sd/sd.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 2606b969e34..30ae435d669 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -53,6 +53,8 @@
#define SDSC_MAX_CAPACITY (2 * GiB)
+#define INVALID_ADDRESS 0
+
typedef enum {
sd_r0 = 0, /* no response */
sd_r1, /* normal response command */
@@ -575,8 +577,8 @@ static void sd_reset(DeviceState *dev)
sd->wpgrps_size = sect;
sd->wp_groups = bitmap_new(sd->wpgrps_size);
memset(sd->function_group, 0, sizeof(sd->function_group));
- sd->erase_start = 0;
- sd->erase_end = 0;
+ sd->erase_start = INVALID_ADDRESS;
+ sd->erase_end = INVALID_ADDRESS;
sd->size = size;
sd->blk_len = 0x200;
sd->pwd_len = 0;
@@ -750,7 +752,8 @@ static void sd_erase(SDState *sd)
uint64_t erase_end = sd->erase_end;
trace_sdcard_erase(sd->erase_start, sd->erase_end);
- if (!sd->erase_start || !sd->erase_end) {
+ if (sd->erase_start == INVALID_ADDRESS
+ || sd->erase_end == INVALID_ADDRESS) {
sd->card_status |= ERASE_SEQ_ERROR;
return;
}
@@ -763,8 +766,8 @@ static void sd_erase(SDState *sd)
erase_start = sd_addr_to_wpnum(erase_start);
erase_end = sd_addr_to_wpnum(erase_end);
- sd->erase_start = 0;
- sd->erase_end = 0;
+ sd->erase_start = INVALID_ADDRESS;
+ sd->erase_end = INVALID_ADDRESS;
sd->csd[14] |= 0x40;
for (i = erase_start; i <= erase_end; i++) {
--
2.26.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 10/13] hw/sd/sdcard: Do not use legal address '0' for INVALID_ADDRESS
2020-10-21 17:34 [PULL 00/13] SD/MMC patches for 2020-10-21 Philippe Mathieu-Daudé
` (8 preceding siblings ...)
2020-10-21 17:34 ` [PULL 09/13] hw/sd/sdcard: Introduce the INVALID_ADDRESS definition Philippe Mathieu-Daudé
@ 2020-10-21 17:34 ` Philippe Mathieu-Daudé
2020-10-21 17:34 ` [PULL 11/13] hw/sd/sdcard: Reset both start/end addresses on error Philippe Mathieu-Daudé
` (3 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-21 17:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexander Bulekov, Philippe Mathieu-Daudé
As it is legal to WRITE/ERASE the address/block 0,
change the value of this definition to an illegal
address: UINT32_MAX.
Unfortunately this break the migration stream, so
bump the VMState version number. This affects some
ARM boards and the SDHCI_PCI device (which is only
used for testing).
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Alexander Bulekov <alxndr@bu.edu>
Message-Id: <20201015063824.212980-4-f4bug@amsat.org>
---
hw/sd/sd.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 30ae435d669..4c05152f189 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -53,7 +53,7 @@
#define SDSC_MAX_CAPACITY (2 * GiB)
-#define INVALID_ADDRESS 0
+#define INVALID_ADDRESS UINT32_MAX
typedef enum {
sd_r0 = 0, /* no response */
@@ -666,8 +666,8 @@ static int sd_vmstate_pre_load(void *opaque)
static const VMStateDescription sd_vmstate = {
.name = "sd-card",
- .version_id = 1,
- .minimum_version_id = 1,
+ .version_id = 2,
+ .minimum_version_id = 2,
.pre_load = sd_vmstate_pre_load,
.fields = (VMStateField[]) {
VMSTATE_UINT32(mode, SDState),
--
2.26.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 11/13] hw/sd/sdcard: Reset both start/end addresses on error
2020-10-21 17:34 [PULL 00/13] SD/MMC patches for 2020-10-21 Philippe Mathieu-Daudé
` (9 preceding siblings ...)
2020-10-21 17:34 ` [PULL 10/13] hw/sd/sdcard: Do not use legal address '0' for INVALID_ADDRESS Philippe Mathieu-Daudé
@ 2020-10-21 17:34 ` Philippe Mathieu-Daudé
2020-10-21 17:34 ` [PULL 12/13] hw/sd/sdcard: Do not attempt to erase out of range addresses Philippe Mathieu-Daudé
` (2 subsequent siblings)
13 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-21 17:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexander Bulekov, Philippe Mathieu-Daudé
From the Spec "4.3.5 Erase":
The host should adhere to the following command
sequence: ERASE_WR_BLK_START, ERASE_WR_BLK_END and
ERASE (CMD38).
If an erase (CMD38) or address setting (CMD32, 33)
command is received out of sequence, the card shall
set the ERASE_SEQ_ERROR bit in the status register
and reset the whole sequence.
Reset both addresses if the ERASE command occured
out of sequence (one of the start/end address is
not set).
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Alexander Bulekov <alxndr@bu.edu>
Message-Id: <20201015063824.212980-5-f4bug@amsat.org>
---
hw/sd/sd.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 4c05152f189..ee7b64023aa 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -755,6 +755,8 @@ static void sd_erase(SDState *sd)
if (sd->erase_start == INVALID_ADDRESS
|| sd->erase_end == INVALID_ADDRESS) {
sd->card_status |= ERASE_SEQ_ERROR;
+ sd->erase_start = INVALID_ADDRESS;
+ sd->erase_end = INVALID_ADDRESS;
return;
}
--
2.26.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 12/13] hw/sd/sdcard: Do not attempt to erase out of range addresses
2020-10-21 17:34 [PULL 00/13] SD/MMC patches for 2020-10-21 Philippe Mathieu-Daudé
` (10 preceding siblings ...)
2020-10-21 17:34 ` [PULL 11/13] hw/sd/sdcard: Reset both start/end addresses on error Philippe Mathieu-Daudé
@ 2020-10-21 17:34 ` Philippe Mathieu-Daudé
2020-10-21 17:34 ` [PULL 13/13] hw/sd/sdcard: Assert if accessing an illegal group Philippe Mathieu-Daudé
2020-10-22 11:33 ` [PULL 00/13] SD/MMC patches for 2020-10-21 Peter Maydell
13 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-21 17:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexander Bulekov, Philippe Mathieu-Daudé
While the Spec v3 is not very clear, v6 states:
If the host provides an out of range address as an argument
to CMD32 or CMD33, the card shall indicate OUT_OF_RANGE error
in R1 (ERX) for CMD38.
If an address is out of range, do not attempt to erase it:
return R1 with the error bit set.
Buglink: https://bugs.launchpad.net/qemu/+bug/1895310
Reported-by: Alexander Bulekov <alxndr@bu.edu>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Alexander Bulekov <alxndr@bu.edu>
Message-Id: <20201015063824.212980-6-f4bug@amsat.org>
---
hw/sd/sd.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index ee7b64023aa..4454d168e2f 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -766,6 +766,13 @@ static void sd_erase(SDState *sd)
erase_end *= 512;
}
+ if (sd->erase_start > sd->size || sd->erase_end > sd->size) {
+ sd->card_status |= OUT_OF_RANGE;
+ sd->erase_start = INVALID_ADDRESS;
+ sd->erase_end = INVALID_ADDRESS;
+ return;
+ }
+
erase_start = sd_addr_to_wpnum(erase_start);
erase_end = sd_addr_to_wpnum(erase_end);
sd->erase_start = INVALID_ADDRESS;
--
2.26.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PULL 13/13] hw/sd/sdcard: Assert if accessing an illegal group
2020-10-21 17:34 [PULL 00/13] SD/MMC patches for 2020-10-21 Philippe Mathieu-Daudé
` (11 preceding siblings ...)
2020-10-21 17:34 ` [PULL 12/13] hw/sd/sdcard: Do not attempt to erase out of range addresses Philippe Mathieu-Daudé
@ 2020-10-21 17:34 ` Philippe Mathieu-Daudé
2020-10-22 11:33 ` [PULL 00/13] SD/MMC patches for 2020-10-21 Peter Maydell
13 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-10-21 17:34 UTC (permalink / raw)
To: qemu-devel; +Cc: Alexander Bulekov, Philippe Mathieu-Daudé
We can not have more group than 'wpgrps_size'.
Assert if we are accessing a group above this limit.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Alexander Bulekov <alxndr@bu.edu>
Message-Id: <20201015063824.212980-7-f4bug@amsat.org>
---
hw/sd/sd.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 4454d168e2f..c3febed2434 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -780,6 +780,7 @@ static void sd_erase(SDState *sd)
sd->csd[14] |= 0x40;
for (i = erase_start; i <= erase_end; i++) {
+ assert(i < sd->wpgrps_size);
if (test_bit(i, sd->wp_groups)) {
sd->card_status |= WP_ERASE_SKIP;
}
@@ -794,6 +795,7 @@ static uint32_t sd_wpbits(SDState *sd, uint64_t addr)
wpnum = sd_addr_to_wpnum(addr);
for (i = 0; i < 32; i++, wpnum++, addr += WPGROUP_SIZE) {
+ assert(wpnum < sd->wpgrps_size);
if (addr < sd->size && test_bit(wpnum, sd->wp_groups)) {
ret |= (1 << i);
}
--
2.26.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PULL 00/13] SD/MMC patches for 2020-10-21
2020-10-21 17:34 [PULL 00/13] SD/MMC patches for 2020-10-21 Philippe Mathieu-Daudé
` (12 preceding siblings ...)
2020-10-21 17:34 ` [PULL 13/13] hw/sd/sdcard: Assert if accessing an illegal group Philippe Mathieu-Daudé
@ 2020-10-22 11:33 ` Peter Maydell
13 siblings, 0 replies; 15+ messages in thread
From: Peter Maydell @ 2020-10-22 11:33 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: Alexander Bulekov, QEMU Developers
On Wed, 21 Oct 2020 at 18:37, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> The following changes since commit ac793156f650ae2d77834932d72224175ee69086:
>
> Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-2020102=
> 0-1' into staging (2020-10-20 21:11:35 +0100)
>
> are available in the Git repository at:
>
> https://gitlab.com/philmd/qemu.git tags/sd-next-20201021
>
> for you to fetch changes up to 84816fb63e5c57159b469a66052d1b2bc862ef77:
>
> hw/sd/sdcard: Assert if accessing an illegal group (2020-10-21 13:34:27 +02=
> 00)
>
> ----------------------------------------------------------------
> SD/MMC patches
>
> Fix two heap-overflow reported by Alexander Bulekov while fuzzing:
> - https://bugs.launchpad.net/qemu/+bug/1892960
> - https://bugs.launchpad.net/qemu/+bug/1895310
>
> CI jobs results:
> . https://cirrus-ci.com/build/6399328187056128
> . https://gitlab.com/philmd/qemu/-/pipelines/205701966
> . https://travis-ci.org/github/philmd/qemu/builds/737708930
> ----------------------------------------------------------------
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2
for any user-visible changes.
-- PMM
^ permalink raw reply [flat|nested] 15+ messages in thread