* [PATCH-for-10.1 v2 01/11] hw/sd/sdcard: Do not ignore errors in sd_cmd_to_sendingdata()
2025-08-04 13:33 [PATCH-for-10.1 v2 00/11] hw/sd: Fix SD cards in SPI mode Philippe Mathieu-Daudé
@ 2025-08-04 13:33 ` Philippe Mathieu-Daudé
2025-08-04 13:33 ` [PATCH-for-10.1 v2 02/11] hw/sd/sdcard: Factor sd_response_size() out Philippe Mathieu-Daudé
` (12 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-08-04 13:33 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: Philippe Mathieu-Daudé, Ben Dooks, qemu-riscv, qemu-block,
qemu-arm, Guenter Roeck, Bin Meng, Richard Henderson
Unfortunately when adding sd_cmd_to_sendingdata() in commit
f486bf7d109 we neglected to return any possible error. Fix.
Fixes: f486bf7d109 ("hw/sd/sdcard: Introduce sd_cmd_to_sendingdata and sd_generic_read_byte")
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/sd/sd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index c275fdda2d0..0bb385268ed 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1305,7 +1305,7 @@ static sd_rsp_type_t sd_cmd_to_sendingdata(SDState *sd, SDRequest req,
const void *data, size_t size)
{
if (sd->state != sd_transfer_state) {
- sd_invalid_state_for_cmd(sd, req);
+ return sd_invalid_state_for_cmd(sd, req);
}
sd->state = sd_sendingdata_state;
--
2.49.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH-for-10.1 v2 02/11] hw/sd/sdcard: Factor sd_response_size() out
2025-08-04 13:33 [PATCH-for-10.1 v2 00/11] hw/sd: Fix SD cards in SPI mode Philippe Mathieu-Daudé
2025-08-04 13:33 ` [PATCH-for-10.1 v2 01/11] hw/sd/sdcard: Do not ignore errors in sd_cmd_to_sendingdata() Philippe Mathieu-Daudé
@ 2025-08-04 13:33 ` Philippe Mathieu-Daudé
2025-08-04 22:55 ` Richard Henderson
2025-08-04 13:33 ` [PATCH-for-10.1 v2 03/11] hw/sd/sdbus: Provide buffer size to sdbus_do_command() Philippe Mathieu-Daudé
` (11 subsequent siblings)
13 siblings, 1 reply; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-08-04 13:33 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: Philippe Mathieu-Daudé, Ben Dooks, qemu-riscv, qemu-block,
qemu-arm, Guenter Roeck, Bin Meng
Set @rsplen once before switching to fill the response buffer.
This will allow to assert in a single place that the buffer is
big enough to be filled with the response.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/sd/sd.c | 36 +++++++++++++++++++++++++++++-------
1 file changed, 29 insertions(+), 7 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 0bb385268ed..76ce54664f2 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -729,6 +729,33 @@ static int sd_req_crc_validate(SDRequest *req)
return sd_crc7(buffer, 5) != req->crc; /* TODO */
}
+static size_t sd_response_size(SDState *sd, sd_rsp_type_t rtype)
+{
+ switch (rtype) {
+ case sd_r1:
+ case sd_r1b:
+ return 4;
+
+ case sd_r2_i:
+ case sd_r2_s:
+ return 16;
+
+ case sd_r3:
+ case sd_r7:
+ return 4;
+
+ case sd_r6:
+ return 4;
+
+ case sd_r0:
+ case sd_illegal:
+ return 0;
+
+ default:
+ g_assert_not_reached();
+ }
+}
+
static void sd_response_r1_make(SDState *sd, uint8_t *response)
{
stl_be_p(response, sd->card_status);
@@ -2203,36 +2230,32 @@ static int sd_do_command(SDState *sd, SDRequest *req,
}
send_response:
+ rsplen = sd_response_size(sd, rtype);
+
switch (rtype) {
case sd_r1:
case sd_r1b:
sd_response_r1_make(sd, response);
- rsplen = 4;
break;
case sd_r2_i:
memcpy(response, sd->cid, sizeof(sd->cid));
- rsplen = 16;
break;
case sd_r2_s:
memcpy(response, sd->csd, sizeof(sd->csd));
- rsplen = 16;
break;
case sd_r3:
sd_response_r3_make(sd, response);
- rsplen = 4;
break;
case sd_r6:
sd_response_r6_make(sd, response);
- rsplen = 4;
break;
case sd_r7:
sd_response_r7_make(sd, response);
- rsplen = 4;
break;
case sd_r0:
@@ -2244,7 +2267,6 @@ send_response:
sd->data_offset = 0;
/* fall-through */
case sd_illegal:
- rsplen = 0;
break;
default:
g_assert_not_reached();
--
2.49.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH-for-10.1 v2 02/11] hw/sd/sdcard: Factor sd_response_size() out
2025-08-04 13:33 ` [PATCH-for-10.1 v2 02/11] hw/sd/sdcard: Factor sd_response_size() out Philippe Mathieu-Daudé
@ 2025-08-04 22:55 ` Richard Henderson
0 siblings, 0 replies; 21+ messages in thread
From: Richard Henderson @ 2025-08-04 22:55 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Peter Maydell, qemu-devel
Cc: Ben Dooks, qemu-riscv, qemu-block, qemu-arm, Guenter Roeck,
Bin Meng
On 8/4/25 23:33, Philippe Mathieu-Daudé wrote:
> Set @rsplen once before switching to fill the response buffer.
> This will allow to assert in a single place that the buffer is
> big enough to be filled with the response.
>
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
> hw/sd/sd.c | 36 +++++++++++++++++++++++++++++-------
> 1 file changed, 29 insertions(+), 7 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH-for-10.1 v2 03/11] hw/sd/sdbus: Provide buffer size to sdbus_do_command()
2025-08-04 13:33 [PATCH-for-10.1 v2 00/11] hw/sd: Fix SD cards in SPI mode Philippe Mathieu-Daudé
2025-08-04 13:33 ` [PATCH-for-10.1 v2 01/11] hw/sd/sdcard: Do not ignore errors in sd_cmd_to_sendingdata() Philippe Mathieu-Daudé
2025-08-04 13:33 ` [PATCH-for-10.1 v2 02/11] hw/sd/sdcard: Factor sd_response_size() out Philippe Mathieu-Daudé
@ 2025-08-04 13:33 ` Philippe Mathieu-Daudé
2025-08-04 22:56 ` Richard Henderson
2025-08-04 13:33 ` [PATCH-for-10.1 v2 04/11] hw/sd/sdcard: Fill SPI response bits in card code Philippe Mathieu-Daudé
` (10 subsequent siblings)
13 siblings, 1 reply; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-08-04 13:33 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: Philippe Mathieu-Daudé, Ben Dooks, qemu-riscv, qemu-block,
qemu-arm, Guenter Roeck, Bin Meng, Beniamino Galvani,
Strahinja Jankovic
We provide to sdbus_do_command() a pointer to a buffer to be
filled with a varying number of bytes. By not providing the
buffer size, the callee can not check the buffer is big enough.
Pass the buffer size as argument to follow good practices.
sdbus_do_command() doesn't return any error, only the size filled
in the buffer. Convert the returned type to unsigned and remove
the few unreachable lines in callers.
This allow to check for possible overflow in sd_do_command().
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
include/hw/sd/sd.h | 23 +++++++++++++++++++++--
hw/sd/allwinner-sdhost.c | 7 ++-----
hw/sd/bcm2835_sdhost.c | 7 ++-----
hw/sd/core.c | 5 +++--
hw/sd/omap_mmc.c | 5 +++--
hw/sd/pl181.c | 6 ++----
hw/sd/sd.c | 6 ++++--
hw/sd/sdhci.c | 6 +++---
hw/sd/ssi-sd.c | 12 +++++++-----
9 files changed, 47 insertions(+), 30 deletions(-)
diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index d6bad175131..55d363f58fb 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -96,7 +96,17 @@ struct SDCardClass {
DeviceClass parent_class;
/*< public >*/
- int (*do_command)(SDState *sd, SDRequest *req, uint8_t *response);
+ /**
+ * Process a SD command request.
+ * @sd: card
+ * @req: command request
+ * @resp: buffer to receive the command response
+ * @respsz: size of @resp buffer
+ *
+ * Return: size of the response
+ */
+ size_t (*do_command)(SDState *sd, SDRequest *req,
+ uint8_t *resp, size_t respsz);
/**
* Write a byte to a SD card.
* @sd: card
@@ -153,7 +163,16 @@ struct SDBusClass {
void sdbus_set_voltage(SDBus *sdbus, uint16_t millivolts);
uint8_t sdbus_get_dat_lines(SDBus *sdbus);
bool sdbus_get_cmd_line(SDBus *sdbus);
-int sdbus_do_command(SDBus *sd, SDRequest *req, uint8_t *response);
+/**
+ * sdbus_do_command: Process a SD command request
+ * @sd: card
+ * @req: command request
+ * @resp: buffer to receive the command response
+ * @respsz: size of @resp buffer
+ *
+ * Return: size of the response
+ */
+size_t sdbus_do_command(SDBus *sd, SDRequest *req, uint8_t *resp, size_t respsz);
/**
* Write a byte to a SD bus.
* @sd: bus
diff --git a/hw/sd/allwinner-sdhost.c b/hw/sd/allwinner-sdhost.c
index b31da5c399c..9d61b372e70 100644
--- a/hw/sd/allwinner-sdhost.c
+++ b/hw/sd/allwinner-sdhost.c
@@ -233,7 +233,7 @@ static void allwinner_sdhost_send_command(AwSdHostState *s)
{
SDRequest request;
uint8_t resp[16];
- int rlen;
+ size_t rlen;
/* Auto clear load flag */
s->command &= ~SD_CMDR_LOAD;
@@ -246,10 +246,7 @@ static void allwinner_sdhost_send_command(AwSdHostState *s)
request.arg = s->command_arg;
/* Send request to SD bus */
- rlen = sdbus_do_command(&s->sdbus, &request, resp);
- if (rlen < 0) {
- goto error;
- }
+ rlen = sdbus_do_command(&s->sdbus, &request, resp, sizeof(resp));
/* If the command has a response, store it in the response registers */
if ((s->command & SD_CMDR_RESPONSE)) {
diff --git a/hw/sd/bcm2835_sdhost.c b/hw/sd/bcm2835_sdhost.c
index 29debdf59e4..f7cef7bb1cd 100644
--- a/hw/sd/bcm2835_sdhost.c
+++ b/hw/sd/bcm2835_sdhost.c
@@ -113,15 +113,12 @@ static void bcm2835_sdhost_send_command(BCM2835SDHostState *s)
{
SDRequest request;
uint8_t rsp[16];
- int rlen;
+ size_t rlen;
request.cmd = s->cmd & SDCMD_CMD_MASK;
request.arg = s->cmdarg;
- rlen = sdbus_do_command(&s->sdbus, &request, rsp);
- if (rlen < 0) {
- goto error;
- }
+ rlen = sdbus_do_command(&s->sdbus, &request, rsp, sizeof(rsp));
if (!(s->cmd & SDCMD_NO_RESPONSE)) {
if (rlen == 0 || (rlen == 4 && (s->cmd & SDCMD_LONG_RESPONSE))) {
goto error;
diff --git a/hw/sd/core.c b/hw/sd/core.c
index 4b30218b520..d3c9017445e 100644
--- a/hw/sd/core.c
+++ b/hw/sd/core.c
@@ -90,7 +90,8 @@ void sdbus_set_voltage(SDBus *sdbus, uint16_t millivolts)
}
}
-int sdbus_do_command(SDBus *sdbus, SDRequest *req, uint8_t *response)
+size_t sdbus_do_command(SDBus *sdbus, SDRequest *req,
+ uint8_t *resp, size_t respsz)
{
SDState *card = get_card(sdbus);
@@ -98,7 +99,7 @@ int sdbus_do_command(SDBus *sdbus, SDRequest *req, uint8_t *response)
if (card) {
SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
- return sc->do_command(card, req, response);
+ return sc->do_command(card, req, resp, respsz);
}
return 0;
diff --git a/hw/sd/omap_mmc.c b/hw/sd/omap_mmc.c
index b7648d41cc5..5a1d25defaa 100644
--- a/hw/sd/omap_mmc.c
+++ b/hw/sd/omap_mmc.c
@@ -130,7 +130,8 @@ static void omap_mmc_command(OMAPMMCState *host, int cmd, int dir,
sd_rsp_type_t resptype, int init)
{
uint32_t rspstatus, mask;
- int rsplen, timeout;
+ size_t rsplen;
+ int timeout;
SDRequest request;
uint8_t response[16];
@@ -157,7 +158,7 @@ static void omap_mmc_command(OMAPMMCState *host, int cmd, int dir,
request.arg = host->arg;
request.crc = 0; /* FIXME */
- rsplen = sdbus_do_command(&host->sdbus, &request, response);
+ rsplen = sdbus_do_command(&host->sdbus, &request, response, sizeof(response));
/* TODO: validate CRCs */
switch (resptype) {
diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c
index b8fc9f86f13..5d56ead4d91 100644
--- a/hw/sd/pl181.c
+++ b/hw/sd/pl181.c
@@ -173,14 +173,12 @@ static void pl181_do_command(PL181State *s)
{
SDRequest request;
uint8_t response[16];
- int rlen;
+ size_t rlen;
request.cmd = s->cmd & PL181_CMD_INDEX;
request.arg = s->cmdarg;
trace_pl181_command_send(request.cmd, request.arg);
- rlen = sdbus_do_command(&s->sdbus, &request, response);
- if (rlen < 0)
- goto error;
+ rlen = sdbus_do_command(&s->sdbus, &request, response, sizeof(response));
if (s->cmd & PL181_CMD_RESPONSE) {
if (rlen == 0 || (rlen == 4 && (s->cmd & PL181_CMD_LONGRESP)))
goto error;
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 76ce54664f2..069107a2e70 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2166,8 +2166,9 @@ static bool cmd_valid_while_locked(SDState *sd, unsigned cmd)
return cmd_class == 0 || cmd_class == 7;
}
-static int sd_do_command(SDState *sd, SDRequest *req,
- uint8_t *response) {
+static size_t sd_do_command(SDState *sd, SDRequest *req,
+ uint8_t *response, size_t respsz)
+{
int last_state;
sd_rsp_type_t rtype;
int rsplen;
@@ -2231,6 +2232,7 @@ static int sd_do_command(SDState *sd, SDRequest *req,
send_response:
rsplen = sd_response_size(sd, rtype);
+ assert(rsplen <= respsz);
switch (rtype) {
case sd_r1:
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 226ff133ff9..3c897e54b72 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -337,7 +337,7 @@ static void sdhci_send_command(SDHCIState *s)
{
SDRequest request;
uint8_t response[16];
- int rlen;
+ size_t rlen;
bool timeout = false;
s->errintsts = 0;
@@ -346,7 +346,7 @@ static void sdhci_send_command(SDHCIState *s)
request.arg = s->argument;
trace_sdhci_send_command(request.cmd, request.arg);
- rlen = sdbus_do_command(&s->sdbus, &request, response);
+ rlen = sdbus_do_command(&s->sdbus, &request, response, sizeof(response));
if (s->cmdreg & SDHC_CMD_RESPONSE) {
if (rlen == 4) {
@@ -400,7 +400,7 @@ static void sdhci_end_transfer(SDHCIState *s)
request.cmd = 0x0C;
request.arg = 0;
trace_sdhci_end_transfer(request.cmd, request.arg);
- sdbus_do_command(&s->sdbus, &request, response);
+ sdbus_do_command(&s->sdbus, &request, response, sizeof(response));
/* Auto CMD12 response goes to the upper Response register */
s->rspreg[3] = ldl_be_p(response);
}
diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index 6c90a86ab41..3025f8f15f4 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -146,8 +146,9 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
/* manually issue cmd12 to stop the transfer */
request.cmd = 12;
request.arg = 0;
- s->arglen = sdbus_do_command(&s->sdbus, &request, longresp);
- if (s->arglen <= 0) {
+ s->arglen = sdbus_do_command(&s->sdbus, &request,
+ longresp, sizeof(longresp));
+ if (s->arglen == 0) {
s->arglen = 1;
/* a zero value indicates the card is busy */
s->response[0] = 0;
@@ -171,8 +172,9 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
request.cmd = s->cmd;
request.arg = ldl_be_p(s->cmdarg);
DPRINTF("CMD%d arg 0x%08x\n", s->cmd, request.arg);
- s->arglen = sdbus_do_command(&s->sdbus, &request, longresp);
- if (s->arglen <= 0) {
+ s->arglen = sdbus_do_command(&s->sdbus, &request,
+ longresp, sizeof(longresp));
+ if (s->arglen == 0) {
s->arglen = 1;
s->response[0] = 4;
DPRINTF("SD command failed\n");
@@ -333,7 +335,7 @@ static int ssi_sd_post_load(void *opaque, int version_id)
return -EINVAL;
}
if (s->mode == SSI_SD_CMDARG &&
- (s->arglen < 0 || s->arglen >= ARRAY_SIZE(s->cmdarg))) {
+ (s->arglen >= ARRAY_SIZE(s->cmdarg))) {
return -EINVAL;
}
if (s->mode == SSI_SD_RESPONSE &&
--
2.49.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH-for-10.1 v2 03/11] hw/sd/sdbus: Provide buffer size to sdbus_do_command()
2025-08-04 13:33 ` [PATCH-for-10.1 v2 03/11] hw/sd/sdbus: Provide buffer size to sdbus_do_command() Philippe Mathieu-Daudé
@ 2025-08-04 22:56 ` Richard Henderson
0 siblings, 0 replies; 21+ messages in thread
From: Richard Henderson @ 2025-08-04 22:56 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Peter Maydell, qemu-devel
Cc: Ben Dooks, qemu-riscv, qemu-block, qemu-arm, Guenter Roeck,
Bin Meng, Beniamino Galvani, Strahinja Jankovic
On 8/4/25 23:33, Philippe Mathieu-Daudé wrote:
> We provide to sdbus_do_command() a pointer to a buffer to be
> filled with a varying number of bytes. By not providing the
> buffer size, the callee can not check the buffer is big enough.
> Pass the buffer size as argument to follow good practices.
>
> sdbus_do_command() doesn't return any error, only the size filled
> in the buffer. Convert the returned type to unsigned and remove
> the few unreachable lines in callers.
>
> This allow to check for possible overflow in sd_do_command().
>
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
> include/hw/sd/sd.h | 23 +++++++++++++++++++++--
> hw/sd/allwinner-sdhost.c | 7 ++-----
> hw/sd/bcm2835_sdhost.c | 7 ++-----
> hw/sd/core.c | 5 +++--
> hw/sd/omap_mmc.c | 5 +++--
> hw/sd/pl181.c | 6 ++----
> hw/sd/sd.c | 6 ++++--
> hw/sd/sdhci.c | 6 +++---
> hw/sd/ssi-sd.c | 12 +++++++-----
> 9 files changed, 47 insertions(+), 30 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH-for-10.1 v2 04/11] hw/sd/sdcard: Fill SPI response bits in card code
2025-08-04 13:33 [PATCH-for-10.1 v2 00/11] hw/sd: Fix SD cards in SPI mode Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2025-08-04 13:33 ` [PATCH-for-10.1 v2 03/11] hw/sd/sdbus: Provide buffer size to sdbus_do_command() Philippe Mathieu-Daudé
@ 2025-08-04 13:33 ` Philippe Mathieu-Daudé
2025-08-04 13:33 ` [PATCH-for-10.1 v2 05/11] hw/sd/sdcard: Implement SPI R2 return value Philippe Mathieu-Daudé
` (9 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-08-04 13:33 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: Philippe Mathieu-Daudé, Ben Dooks, qemu-riscv, qemu-block,
qemu-arm, Guenter Roeck, Bin Meng
ssi-sd.c contains the SPI link layer adaptation,
while sd.c contains all the SD card internal details.
We already handle the response values in sd.c, but
missed the SPI case. Complete them (fill R1, prepend
R1 in R3/R7 and always return something in SPI mode).
Remove all the duplication in ssi-sd.c.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/sd/sd.c | 32 ++++++++++++++++---
hw/sd/ssi-sd.c | 87 ++++----------------------------------------------
2 files changed, 35 insertions(+), 84 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 069107a2e70..cbcc180f6a4 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -734,22 +734,24 @@ static size_t sd_response_size(SDState *sd, sd_rsp_type_t rtype)
switch (rtype) {
case sd_r1:
case sd_r1b:
- return 4;
+ return sd_is_spi(sd) ? 1 : 4;
case sd_r2_i:
case sd_r2_s:
+ assert(!sd_is_spi(sd));
return 16;
case sd_r3:
case sd_r7:
- return 4;
+ return sd_is_spi(sd) ? 5 : 4;
case sd_r6:
+ assert(!sd_is_spi(sd));
return 4;
case sd_r0:
case sd_illegal:
- return 0;
+ return sd_is_spi(sd) ? 1 : 0;
default:
g_assert_not_reached();
@@ -758,7 +760,19 @@ static size_t sd_response_size(SDState *sd, sd_rsp_type_t rtype)
static void sd_response_r1_make(SDState *sd, uint8_t *response)
{
- stl_be_p(response, sd->card_status);
+ if (sd_is_spi(sd)) {
+ response[0] = sd->state == sd_idle_state
+ && !FIELD_EX32(sd->ocr, OCR, CARD_POWER_UP);
+ response[0] |= FIELD_EX32(sd->card_status, CSR, ERASE_RESET) << 1;
+ response[0] |= FIELD_EX32(sd->card_status, CSR, ILLEGAL_COMMAND) << 2;
+ response[0] |= FIELD_EX32(sd->card_status, CSR, COM_CRC_ERROR) << 3;
+ response[0] |= FIELD_EX32(sd->card_status, CSR, ERASE_SEQ_ERROR) << 4;
+ response[0] |= FIELD_EX32(sd->card_status, CSR, ADDRESS_ERROR) << 5;
+ response[0] |= FIELD_EX32(sd->card_status, CSR, BLOCK_LEN_ERROR) << 6;
+ response[0] |= 0 << 7;
+ } else {
+ stl_be_p(response, sd->card_status);
+ }
/* Clear the "clear on read" status bits */
sd->card_status &= ~CARD_STATUS_C;
@@ -766,6 +780,11 @@ static void sd_response_r1_make(SDState *sd, uint8_t *response)
static void sd_response_r3_make(SDState *sd, uint8_t *response)
{
+ if (sd_is_spi(sd)) {
+ /* Prepend R1 */
+ sd_response_r1_make(sd, response);
+ response++;
+ }
stl_be_p(response, sd->ocr & ACMD41_R3_MASK);
}
@@ -783,6 +802,11 @@ static void sd_response_r6_make(SDState *sd, uint8_t *response)
static void sd_response_r7_make(SDState *sd, uint8_t *response)
{
+ if (sd_is_spi(sd)) {
+ /* Prepend R1 */
+ sd_response_r1_make(sd, response);
+ response++;
+ }
stl_be_p(response, sd->vhs);
}
diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index 3025f8f15f4..2d5c0ad5016 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -70,23 +70,6 @@ struct ssi_sd_state {
#define TYPE_SSI_SD "ssi-sd"
OBJECT_DECLARE_SIMPLE_TYPE(ssi_sd_state, SSI_SD)
-/* State word bits. */
-#define SSI_SDR_LOCKED 0x0001
-#define SSI_SDR_WP_ERASE 0x0002
-#define SSI_SDR_ERROR 0x0004
-#define SSI_SDR_CC_ERROR 0x0008
-#define SSI_SDR_ECC_FAILED 0x0010
-#define SSI_SDR_WP_VIOLATION 0x0020
-#define SSI_SDR_ERASE_PARAM 0x0040
-#define SSI_SDR_OUT_OF_RANGE 0x0080
-#define SSI_SDR_IDLE 0x0100
-#define SSI_SDR_ERASE_RESET 0x0200
-#define SSI_SDR_ILLEGAL_COMMAND 0x0400
-#define SSI_SDR_COM_CRC_ERROR 0x0800
-#define SSI_SDR_ERASE_SEQ_ERROR 0x1000
-#define SSI_SDR_ADDRESS_ERROR 0x2000
-#define SSI_SDR_PARAMETER_ERROR 0x4000
-
/* multiple block write */
#define SSI_TOKEN_MULTI_WRITE 0xfc
/* terminate multiple block write */
@@ -104,7 +87,7 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
{
ssi_sd_state *s = SSI_SD(dev);
SDRequest request;
- uint8_t longresp[16];
+ uint8_t longresp[5];
/*
* Special case: allow CMD12 (STOP TRANSMISSION) while reading data.
@@ -171,74 +154,18 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
/* FIXME: Check CRC. */
request.cmd = s->cmd;
request.arg = ldl_be_p(s->cmdarg);
- DPRINTF("CMD%d arg 0x%08x\n", s->cmd, request.arg);
s->arglen = sdbus_do_command(&s->sdbus, &request,
longresp, sizeof(longresp));
- if (s->arglen == 0) {
- s->arglen = 1;
- s->response[0] = 4;
- DPRINTF("SD command failed\n");
- } else if (s->cmd == 8 || s->cmd == 58) {
- /* CMD8/CMD58 returns R3/R7 response */
- DPRINTF("Returned R3/R7\n");
- s->arglen = 5;
- s->response[0] = 1;
- memcpy(&s->response[1], longresp, 4);
- } else if (s->arglen != 4) {
- BADF("Unexpected response to cmd %d\n", s->cmd);
- /* Illegal command is about as near as we can get. */
- s->arglen = 1;
- s->response[0] = 4;
- } else {
- /* All other commands return status. */
- uint32_t cardstatus;
- uint16_t status;
+ DPRINTF("CMD%d arg 0x%08x = %d\n", s->cmd, request.arg, s->arglen);
+ assert(s->arglen > 0);
/* CMD13 returns a 2-byte statuse work. Other commands
only return the first byte. */
s->arglen = (s->cmd == 13) ? 2 : 1;
+ memcpy(s->response, longresp, s->arglen);
- /* handle R1b */
- if (s->cmd == 28 || s->cmd == 29 || s->cmd == 38) {
- s->stopping = 1;
- }
-
- cardstatus = ldl_be_p(longresp);
- status = 0;
- if (((cardstatus >> 9) & 0xf) < 4)
- status |= SSI_SDR_IDLE;
- if (cardstatus & ERASE_RESET)
- status |= SSI_SDR_ERASE_RESET;
- if (cardstatus & ILLEGAL_COMMAND)
- status |= SSI_SDR_ILLEGAL_COMMAND;
- if (cardstatus & COM_CRC_ERROR)
- status |= SSI_SDR_COM_CRC_ERROR;
- if (cardstatus & ERASE_SEQ_ERROR)
- status |= SSI_SDR_ERASE_SEQ_ERROR;
- if (cardstatus & ADDRESS_ERROR)
- status |= SSI_SDR_ADDRESS_ERROR;
- if (cardstatus & CARD_IS_LOCKED)
- status |= SSI_SDR_LOCKED;
- if (cardstatus & (LOCK_UNLOCK_FAILED | WP_ERASE_SKIP))
- status |= SSI_SDR_WP_ERASE;
- if (cardstatus & SD_ERROR)
- status |= SSI_SDR_ERROR;
- if (cardstatus & CC_ERROR)
- status |= SSI_SDR_CC_ERROR;
- if (cardstatus & CARD_ECC_FAILED)
- status |= SSI_SDR_ECC_FAILED;
- if (cardstatus & WP_VIOLATION)
- status |= SSI_SDR_WP_VIOLATION;
- if (cardstatus & ERASE_PARAM)
- status |= SSI_SDR_ERASE_PARAM;
- if (cardstatus & (OUT_OF_RANGE | CID_CSD_OVERWRITE))
- status |= SSI_SDR_OUT_OF_RANGE;
- /* ??? Don't know what Parameter Error really means, so
- assume it's set if the second byte is nonzero. */
- if (status & 0xff)
- status |= SSI_SDR_PARAMETER_ERROR;
- s->response[0] = status >> 8;
- s->response[1] = status;
- DPRINTF("Card status 0x%02x\n", status);
+ /* handle R1b (busy signal) */
+ if (s->cmd == 28 || s->cmd == 29 || s->cmd == 38) {
+ s->stopping = 1;
}
s->mode = SSI_SD_PREP_RESP;
s->response_pos = 0;
--
2.49.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH-for-10.1 v2 05/11] hw/sd/sdcard: Implement SPI R2 return value
2025-08-04 13:33 [PATCH-for-10.1 v2 00/11] hw/sd: Fix SD cards in SPI mode Philippe Mathieu-Daudé
` (3 preceding siblings ...)
2025-08-04 13:33 ` [PATCH-for-10.1 v2 04/11] hw/sd/sdcard: Fill SPI response bits in card code Philippe Mathieu-Daudé
@ 2025-08-04 13:33 ` Philippe Mathieu-Daudé
2025-08-04 13:34 ` [PATCH-for-10.1 v2 06/11] hw/sd/sdcard: Use complete SEND_OP_COND implementation in SPI mode Philippe Mathieu-Daudé
` (8 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-08-04 13:33 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: Philippe Mathieu-Daudé, Ben Dooks, qemu-riscv, qemu-block,
qemu-arm, Guenter Roeck, Bin Meng
In SPI mode, R2 is a 2-byte value.
Implement in spi_response_r2_make() and
return SPI R2 in the SEND_STATUS commands.
Reported-by: Guenter Roeck <linux@roeck-us.net>
Fixes: 775616c3ae8 ("Partial SD card SPI mode support")
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/sd/sd.c | 38 +++++++++++++++++++++++++++++++++++---
hw/sd/ssi-sd.c | 3 ---
2 files changed, 35 insertions(+), 6 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index cbcc180f6a4..01ec6d951c8 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -61,6 +61,7 @@
typedef enum {
sd_r0 = 0, /* no response */
sd_r1, /* normal response command */
+ spi_r2, /* STATUS */
sd_r2_i, /* CID register */
sd_r2_s, /* CSD register */
sd_r3, /* OCR register */
@@ -247,6 +248,7 @@ static const char *sd_response_name(sd_rsp_type_t rsp)
static const char *response_name[] = {
[sd_r0] = "RESP#0 (no response)",
[sd_r1] = "RESP#1 (normal cmd)",
+ [spi_r2] = "RESP#2 (STATUS reg)",
[sd_r2_i] = "RESP#2 (CID reg)",
[sd_r2_s] = "RESP#2 (CSD reg)",
[sd_r3] = "RESP#3 (OCR reg)",
@@ -736,6 +738,10 @@ static size_t sd_response_size(SDState *sd, sd_rsp_type_t rtype)
case sd_r1b:
return sd_is_spi(sd) ? 1 : 4;
+ case spi_r2:
+ assert(sd_is_spi(sd));
+ return 2;
+
case sd_r2_i:
case sd_r2_s:
assert(!sd_is_spi(sd));
@@ -778,6 +784,22 @@ static void sd_response_r1_make(SDState *sd, uint8_t *response)
sd->card_status &= ~CARD_STATUS_C;
}
+static void spi_response_r2_make(SDState *sd, uint8_t *resp)
+{
+ /* Prepend R1 */
+ sd_response_r1_make(sd, resp);
+
+ resp[1] = FIELD_EX32(sd->card_status, CSR, CARD_IS_LOCKED) << 0;
+ resp[1] |= (FIELD_EX32(sd->card_status, CSR, LOCK_UNLOCK_FAILED)
+ || FIELD_EX32(sd->card_status, CSR, WP_ERASE_SKIP)) << 1;
+ resp[1] |= FIELD_EX32(sd->card_status, CSR, ERROR) << 2;
+ resp[1] |= FIELD_EX32(sd->card_status, CSR, CC_ERROR) << 3;
+ resp[1] |= FIELD_EX32(sd->card_status, CSR, CARD_ECC_FAILED) << 4;
+ resp[1] |= FIELD_EX32(sd->card_status, CSR, WP_VIOLATION) << 5;
+ resp[1] |= FIELD_EX32(sd->card_status, CSR, ERASE_PARAM) << 6;
+ resp[1] |= FIELD_EX32(sd->card_status, CSR, OUT_OF_RANGE) << 7;
+}
+
static void sd_response_r3_make(SDState *sd, uint8_t *response)
{
if (sd_is_spi(sd)) {
@@ -1643,7 +1665,7 @@ static sd_rsp_type_t sd_cmd_SEND_STATUS(SDState *sd, SDRequest req)
}
if (sd_is_spi(sd)) {
- return sd_r2_s;
+ return spi_r2;
}
return sd_req_rca_same(sd, req) ? sd_r1 : sd_r0;
@@ -1957,8 +1979,14 @@ static sd_rsp_type_t sd_acmd_SET_BUS_WIDTH(SDState *sd, SDRequest req)
/* ACMD13 */
static sd_rsp_type_t sd_acmd_SD_STATUS(SDState *sd, SDRequest req)
{
- return sd_cmd_to_sendingdata(sd, req, 0,
- sd->sd_status, sizeof(sd->sd_status));
+ sd_rsp_type_t rsp;
+
+ rsp = sd_cmd_to_sendingdata(sd, req, 0,
+ sd->sd_status, sizeof(sd->sd_status));
+ if (sd_is_spi(sd) && rsp != sd_illegal) {
+ return spi_r2;
+ }
+ return rsp;
}
/* ACMD22 */
@@ -2264,6 +2292,10 @@ send_response:
sd_response_r1_make(sd, response);
break;
+ case spi_r2:
+ spi_response_r2_make(sd, response);
+ break;
+
case sd_r2_i:
memcpy(response, sd->cid, sizeof(sd->cid));
break;
diff --git a/hw/sd/ssi-sd.c b/hw/sd/ssi-sd.c
index 2d5c0ad5016..594dead19ee 100644
--- a/hw/sd/ssi-sd.c
+++ b/hw/sd/ssi-sd.c
@@ -158,9 +158,6 @@ static uint32_t ssi_sd_transfer(SSIPeripheral *dev, uint32_t val)
longresp, sizeof(longresp));
DPRINTF("CMD%d arg 0x%08x = %d\n", s->cmd, request.arg, s->arglen);
assert(s->arglen > 0);
- /* CMD13 returns a 2-byte statuse work. Other commands
- only return the first byte. */
- s->arglen = (s->cmd == 13) ? 2 : 1;
memcpy(s->response, longresp, s->arglen);
/* handle R1b (busy signal) */
--
2.49.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH-for-10.1 v2 06/11] hw/sd/sdcard: Use complete SEND_OP_COND implementation in SPI mode
2025-08-04 13:33 [PATCH-for-10.1 v2 00/11] hw/sd: Fix SD cards in SPI mode Philippe Mathieu-Daudé
` (4 preceding siblings ...)
2025-08-04 13:33 ` [PATCH-for-10.1 v2 05/11] hw/sd/sdcard: Implement SPI R2 return value Philippe Mathieu-Daudé
@ 2025-08-04 13:34 ` Philippe Mathieu-Daudé
2025-08-04 13:34 ` [PATCH-for-10.1 v2 07/11] hw/sd/sdcard: Allow using SWITCH_FUNCTION in more SPI states Philippe Mathieu-Daudé
` (7 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-08-04 13:34 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: Philippe Mathieu-Daudé, Ben Dooks, qemu-riscv, qemu-block,
qemu-arm, Guenter Roeck, Bin Meng
While spi_cmd_SEND_OP_COND() is incomplete, sd_cmd_SEND_OP_COND()
is, except it doesn't return the correct value in SPI mode.
Correct and use, removing the need for spi_cmd_SEND_OP_COND().
Fixes: 775616c3ae8 ("Partial SD card SPI mode support")
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/sd/sd.c | 15 +++++----------
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 01ec6d951c8..df2a272c6a2 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1414,14 +1414,6 @@ static sd_rsp_type_t sd_cmd_GO_IDLE_STATE(SDState *sd, SDRequest req)
return sd_is_spi(sd) ? sd_r1 : sd_r0;
}
-/* CMD1 */
-static sd_rsp_type_t spi_cmd_SEND_OP_COND(SDState *sd, SDRequest req)
-{
- sd->state = sd_transfer_state;
-
- return sd_r1;
-}
-
/* CMD2 */
static sd_rsp_type_t sd_cmd_ALL_SEND_CID(SDState *sd, SDRequest req)
{
@@ -2046,6 +2038,9 @@ static sd_rsp_type_t sd_cmd_SEND_OP_COND(SDState *sd, SDRequest req)
sd->state = sd_ready_state;
}
+ if (sd_is_spi(sd)) {
+ return sd_r1;
+ }
return sd_r3;
}
@@ -2590,7 +2585,7 @@ static const SDProto sd_proto_spi = {
.name = "SPI",
.cmd = {
[0] = {0, sd_spi, "GO_IDLE_STATE", sd_cmd_GO_IDLE_STATE},
- [1] = {0, sd_spi, "SEND_OP_COND", spi_cmd_SEND_OP_COND},
+ [1] = {0, sd_spi, "SEND_OP_COND", sd_cmd_SEND_OP_COND},
[5] = {9, sd_spi, "IO_SEND_OP_COND", sd_cmd_optional},
[6] = {10, sd_spi, "SWITCH_FUNCTION", sd_cmd_SWITCH_FUNCTION},
[8] = {0, sd_spi, "SEND_IF_COND", sd_cmd_SEND_IF_COND},
@@ -2626,7 +2621,7 @@ static const SDProto sd_proto_spi = {
[13] = {8, sd_spi, "SD_STATUS", sd_acmd_SD_STATUS},
[22] = {8, sd_spi, "SEND_NUM_WR_BLOCKS", sd_acmd_SEND_NUM_WR_BLOCKS},
[23] = {8, sd_spi, "SET_WR_BLK_ERASE_COUNT", sd_acmd_SET_WR_BLK_ERASE_COUNT},
- [41] = {8, sd_spi, "SEND_OP_COND", spi_cmd_SEND_OP_COND},
+ [41] = {8, sd_spi, "SEND_OP_COND", sd_cmd_SEND_OP_COND},
[42] = {8, sd_spi, "SET_CLR_CARD_DETECT", sd_acmd_SET_CLR_CARD_DETECT},
[51] = {8, sd_spi, "SEND_SCR", sd_acmd_SEND_SCR},
},
--
2.49.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH-for-10.1 v2 07/11] hw/sd/sdcard: Allow using SWITCH_FUNCTION in more SPI states
2025-08-04 13:33 [PATCH-for-10.1 v2 00/11] hw/sd: Fix SD cards in SPI mode Philippe Mathieu-Daudé
` (5 preceding siblings ...)
2025-08-04 13:34 ` [PATCH-for-10.1 v2 06/11] hw/sd/sdcard: Use complete SEND_OP_COND implementation in SPI mode Philippe Mathieu-Daudé
@ 2025-08-04 13:34 ` Philippe Mathieu-Daudé
2025-08-04 13:34 ` [PATCH-for-10.1 v2 08/11] hw/sd/sdcard: Factor spi_cmd_SEND_CxD() out Philippe Mathieu-Daudé
` (6 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-08-04 13:34 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: Philippe Mathieu-Daudé, Ben Dooks, qemu-riscv, qemu-block,
qemu-arm, Guenter Roeck, Bin Meng
In SPI mode, SWITCH_FUNCTION is valid in all mode
(except the IDLE one).
Fixes: 775616c3ae8 ("Partial SD card SPI mode support")
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/sd/sd.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index df2a272c6a2..a9efa158594 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1488,8 +1488,14 @@ static sd_rsp_type_t sd_cmd_SWITCH_FUNCTION(SDState *sd, SDRequest req)
if (sd->mode != sd_data_transfer_mode) {
return sd_invalid_mode_for_cmd(sd, req);
}
- if (sd->state != sd_transfer_state) {
- return sd_invalid_state_for_cmd(sd, req);
+ if (sd_is_spi(sd)) {
+ if (sd->state == sd_idle_state) {
+ return sd_invalid_state_for_cmd(sd, req);
+ }
+ } else {
+ if (sd->state != sd_transfer_state) {
+ return sd_invalid_state_for_cmd(sd, req);
+ }
}
sd_function_switch(sd, req.arg);
--
2.49.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH-for-10.1 v2 08/11] hw/sd/sdcard: Factor spi_cmd_SEND_CxD() out
2025-08-04 13:33 [PATCH-for-10.1 v2 00/11] hw/sd: Fix SD cards in SPI mode Philippe Mathieu-Daudé
` (6 preceding siblings ...)
2025-08-04 13:34 ` [PATCH-for-10.1 v2 07/11] hw/sd/sdcard: Allow using SWITCH_FUNCTION in more SPI states Philippe Mathieu-Daudé
@ 2025-08-04 13:34 ` Philippe Mathieu-Daudé
2025-08-04 13:34 ` [PATCH-for-10.1 v2 09/11] hw/sd/sdcard: Disable checking STBY mode in SPI SEND_CSD/CID Philippe Mathieu-Daudé
` (5 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-08-04 13:34 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: Philippe Mathieu-Daudé, Ben Dooks, qemu-riscv, qemu-block,
qemu-arm, Guenter Roeck, Bin Meng
spi_cmd_SEND_CSD() and spi_cmd_SEND_CID() are very
similar. Factor the common code as spi_cmd_SEND_CxD().
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/sd/sd.c | 19 ++++++++++---------
1 file changed, 10 insertions(+), 9 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index a9efa158594..ee81dc09991 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1588,14 +1588,19 @@ static sd_rsp_type_t emmc_cmd_SEND_EXT_CSD(SDState *sd, SDRequest req)
sd->ext_csd, sizeof(sd->ext_csd));
}
-/* CMD9 */
-static sd_rsp_type_t spi_cmd_SEND_CSD(SDState *sd, SDRequest req)
+static sd_rsp_type_t spi_cmd_SEND_CxD(SDState *sd, SDRequest req,
+ const void *data, size_t size)
{
if (sd->state != sd_standby_state) {
return sd_invalid_state_for_cmd(sd, req);
}
- return sd_cmd_to_sendingdata(sd, req, sd_req_get_address(sd, req),
- sd->csd, 16);
+ return sd_cmd_to_sendingdata(sd, req, 0, data, size);
+}
+
+/* CMD9 */
+static sd_rsp_type_t spi_cmd_SEND_CSD(SDState *sd, SDRequest req)
+{
+ return spi_cmd_SEND_CxD(sd, req, sd->csd, sizeof(sd->csd));
}
static sd_rsp_type_t sd_cmd_SEND_CSD(SDState *sd, SDRequest req)
@@ -1610,11 +1615,7 @@ static sd_rsp_type_t sd_cmd_SEND_CSD(SDState *sd, SDRequest req)
/* CMD10 */
static sd_rsp_type_t spi_cmd_SEND_CID(SDState *sd, SDRequest req)
{
- if (sd->state != sd_standby_state) {
- return sd_invalid_state_for_cmd(sd, req);
- }
- return sd_cmd_to_sendingdata(sd, req, sd_req_get_address(sd, req),
- sd->cid, 16);
+ return spi_cmd_SEND_CxD(sd, req, sd->cid, sizeof(sd->cid));
}
static sd_rsp_type_t sd_cmd_SEND_CID(SDState *sd, SDRequest req)
--
2.49.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH-for-10.1 v2 09/11] hw/sd/sdcard: Disable checking STBY mode in SPI SEND_CSD/CID
2025-08-04 13:33 [PATCH-for-10.1 v2 00/11] hw/sd: Fix SD cards in SPI mode Philippe Mathieu-Daudé
` (7 preceding siblings ...)
2025-08-04 13:34 ` [PATCH-for-10.1 v2 08/11] hw/sd/sdcard: Factor spi_cmd_SEND_CxD() out Philippe Mathieu-Daudé
@ 2025-08-04 13:34 ` Philippe Mathieu-Daudé
2025-08-04 13:34 ` [PATCH-for-10.1 v2 10/11] hw/sd/sdcard: Remove SDState::mode field Philippe Mathieu-Daudé
` (4 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-08-04 13:34 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: Philippe Mathieu-Daudé, Ben Dooks, qemu-riscv, qemu-block,
qemu-arm, Guenter Roeck, Bin Meng
The card should be in STANDBY mode to process SEND_CSD or SEND_CID,
but is still in IDLE mode.
Unfortunately I don't have enough time to keep debugging this issue,
so disable the check for the time being and the next release, as it
blocks Linux. I'll keep looking.
Reported-by: Guenter Roeck <linux@roeck-us.net>
Reported-by: Ben Dooks <ben.dooks@codethink.co.uk>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/sd/sd.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index ee81dc09991..22f30997713 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1591,9 +1591,20 @@ static sd_rsp_type_t emmc_cmd_SEND_EXT_CSD(SDState *sd, SDRequest req)
static sd_rsp_type_t spi_cmd_SEND_CxD(SDState *sd, SDRequest req,
const void *data, size_t size)
{
+ /*
+ * XXX as of v10.1.0-rc1 command is reached in sd_idle_state,
+ * so disable this check.
if (sd->state != sd_standby_state) {
return sd_invalid_state_for_cmd(sd, req);
}
+ */
+
+ /*
+ * Since SPI returns CSD and CID on the DAT lines,
+ * switch to sd_transfer_state.
+ */
+ sd->state = sd_transfer_state;
+
return sd_cmd_to_sendingdata(sd, req, 0, data, size);
}
--
2.49.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH-for-10.1 v2 10/11] hw/sd/sdcard: Remove SDState::mode field
2025-08-04 13:33 [PATCH-for-10.1 v2 00/11] hw/sd: Fix SD cards in SPI mode Philippe Mathieu-Daudé
` (8 preceding siblings ...)
2025-08-04 13:34 ` [PATCH-for-10.1 v2 09/11] hw/sd/sdcard: Disable checking STBY mode in SPI SEND_CSD/CID Philippe Mathieu-Daudé
@ 2025-08-04 13:34 ` Philippe Mathieu-Daudé
2025-08-04 13:34 ` [PATCH-for-10.1 v2 11/11] tests/functional: Test SD cards in SPI mode (using sifive_u machine) Philippe Mathieu-Daudé
` (3 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-08-04 13:34 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: Philippe Mathieu-Daudé, Ben Dooks, qemu-riscv, qemu-block,
qemu-arm, Guenter Roeck, Bin Meng
SD card mode is a superset of its state (SDState::state),
no need to migrate it.
Use sd_mode() to get the SDCardModes from the SDCardStates.
Fixes: 50a5be6c3d5 ("hw/sd.c: add SD card save/load support")
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
hw/sd/sd.c | 35 +++++++++++++++++------------------
hw/sd/trace-events | 4 ++--
2 files changed, 19 insertions(+), 20 deletions(-)
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 22f30997713..8c290595f01 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -147,7 +147,6 @@ struct SDState {
/* Runtime changeables */
- uint32_t mode; /* current card mode, one of SDCardModes */
int32_t state; /* current card state, one of SDCardStates */
uint32_t vhs;
bool wp_switch;
@@ -315,27 +314,24 @@ static void sd_set_voltage(SDState *sd, uint16_t millivolts)
}
}
-static void sd_set_mode(SDState *sd)
+static enum SDCardModes sd_mode(SDState *sd)
{
switch (sd->state) {
case sd_inactive_state:
- sd->mode = sd_inactive;
- break;
-
+ return sd_inactive;
case sd_idle_state:
case sd_ready_state:
case sd_identification_state:
- sd->mode = sd_card_identification_mode;
- break;
-
+ return sd_card_identification_mode;
case sd_standby_state:
case sd_transfer_state:
case sd_sendingdata_state:
case sd_receivingdata_state:
case sd_programming_state:
case sd_disconnect_state:
- sd->mode = sd_data_transfer_mode;
- break;
+ return sd_data_transfer_mode;
+ default:
+ g_assert_not_reached();
}
}
@@ -1025,7 +1021,7 @@ static const VMStateDescription sd_vmstate = {
.minimum_version_id = 2,
.pre_load = sd_vmstate_pre_load,
.fields = (const VMStateField[]) {
- VMSTATE_UINT32(mode, SDState),
+ VMSTATE_UNUSED(4),
VMSTATE_INT32(state, SDState),
VMSTATE_UINT8_ARRAY(cid, SDState, 16),
VMSTATE_UINT8_ARRAY(csd, SDState, 16),
@@ -1325,7 +1321,7 @@ static sd_rsp_type_t sd_invalid_state_for_cmd(SDState *sd, SDRequest req)
static sd_rsp_type_t sd_invalid_mode_for_cmd(SDState *sd, SDRequest req)
{
qemu_log_mask(LOG_GUEST_ERROR, "%s: CMD%i in a wrong mode: %s (spec %s)\n",
- sd->proto->name, req.cmd, sd_mode_name(sd->mode),
+ sd->proto->name, req.cmd, sd_mode_name(sd_mode(sd)),
sd_version_str(sd->spec_version));
return sd_illegal;
@@ -1485,7 +1481,7 @@ static sd_rsp_type_t emmc_cmd_sleep_awake(SDState *sd, SDRequest req)
/* CMD6 */
static sd_rsp_type_t sd_cmd_SWITCH_FUNCTION(SDState *sd, SDRequest req)
{
- if (sd->mode != sd_data_transfer_mode) {
+ if (sd_mode(sd) != sd_data_transfer_mode) {
return sd_invalid_mode_for_cmd(sd, req);
}
if (sd_is_spi(sd)) {
@@ -1658,7 +1654,7 @@ static sd_rsp_type_t sd_cmd_STOP_TRANSMISSION(SDState *sd, SDRequest req)
/* CMD13 */
static sd_rsp_type_t sd_cmd_SEND_STATUS(SDState *sd, SDRequest req)
{
- if (sd->mode != sd_data_transfer_mode) {
+ if (sd_mode(sd) != sd_data_transfer_mode) {
return sd_invalid_mode_for_cmd(sd, req);
}
@@ -1684,7 +1680,7 @@ static sd_rsp_type_t sd_cmd_SEND_STATUS(SDState *sd, SDRequest req)
/* CMD15 */
static sd_rsp_type_t sd_cmd_GO_INACTIVE_STATE(SDState *sd, SDRequest req)
{
- if (sd->mode != sd_data_transfer_mode) {
+ if (sd_mode(sd) != sd_data_transfer_mode) {
return sd_invalid_mode_for_cmd(sd, req);
}
switch (sd->state) {
@@ -2090,7 +2086,9 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
if (req.cmd != 55 || sd->expecting_acmd) {
trace_sdcard_normal_command(sd->proto->name,
sd->last_cmd_name, req.cmd,
- req.arg, sd_state_name(sd->state));
+ req.arg,
+ sd_mode_name(sd_mode(sd)),
+ sd_state_name(sd->state));
}
/* Not interpreting this as an app command */
@@ -2176,7 +2174,9 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
{
sd->last_cmd_name = sd_acmd_name(sd, req.cmd);
trace_sdcard_app_command(sd->proto->name, sd->last_cmd_name,
- req.cmd, req.arg, sd_state_name(sd->state));
+ req.cmd, req.arg,
+ sd_mode_name(sd_mode(sd)),
+ sd_state_name(sd->state));
sd->card_status |= APP_CMD;
if (sd->proto->acmd[req.cmd].handler) {
@@ -2276,7 +2276,6 @@ static size_t sd_do_command(SDState *sd, SDRequest *req,
}
last_state = sd->state;
- sd_set_mode(sd);
if (sd->expecting_acmd) {
sd->expecting_acmd = false;
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index db0644256d9..8d49840917e 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -37,8 +37,8 @@ sdhci_write_dataport(uint16_t data_count) "write buffer filled with %u bytes of
sdhci_capareg(const char *desc, uint16_t val) "%s: %u"
# sd.c
-sdcard_normal_command(const char *proto, const char *cmd_desc, uint8_t cmd, uint32_t arg, const char *state) "%s %20s/ CMD%02d arg 0x%08x (state %s)"
-sdcard_app_command(const char *proto, const char *acmd_desc, uint8_t acmd, uint32_t arg, const char *state) "%s %23s/ACMD%02d arg 0x%08x (state %s)"
+sdcard_normal_command(const char *proto, const char *cmd_desc, uint8_t cmd, uint32_t arg, const char *mode, const char *state) "%s %20s/ CMD%02d arg 0x%08x (mode %s, state %s)"
+sdcard_app_command(const char *proto, const char *acmd_desc, uint8_t acmd, uint32_t arg, const char *mode, const char *state) "%s %23s/ACMD%02d arg 0x%08x (mode %s, state %s)"
sdcard_response(const char *rspdesc, int rsplen) "%s (sz:%d)"
sdcard_powerup(void) ""
sdcard_inquiry_cmd41(void) ""
--
2.49.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH-for-10.1 v2 11/11] tests/functional: Test SD cards in SPI mode (using sifive_u machine)
2025-08-04 13:33 [PATCH-for-10.1 v2 00/11] hw/sd: Fix SD cards in SPI mode Philippe Mathieu-Daudé
` (9 preceding siblings ...)
2025-08-04 13:34 ` [PATCH-for-10.1 v2 10/11] hw/sd/sdcard: Remove SDState::mode field Philippe Mathieu-Daudé
@ 2025-08-04 13:34 ` Philippe Mathieu-Daudé
2025-08-04 23:03 ` [PATCH-for-10.1 v2 00/11] hw/sd: Fix SD cards in SPI mode Richard Henderson
` (2 subsequent siblings)
13 siblings, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-08-04 13:34 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: Philippe Mathieu-Daudé, Ben Dooks, qemu-riscv, qemu-block,
qemu-arm, Guenter Roeck, Bin Meng, Alistair Francis,
Palmer Dabbelt, Weiwei Li, Daniel Henrique Barboza, Liu Zhiwei
Add a test which uses the sifive_u machine to boot a Linux
kernel from a SD card connected via a SPI interface.
Inspired from the command provided in:
- https://lore.kernel.org/qemu-devel/94b2c5bf-53d0-4c74-8264-f3021916f38c@roeck-us.net/
- https://lore.kernel.org/qemu-devel/840016d0-0d49-4ef4-8372-b62b3bcd0ac6@codethink.co.uk/
Inspired-by: Guenter Roeck <linux@roeck-us.net>
Inspired-by: Ben Dooks <ben.dooks@codethink.co.uk>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
MAINTAINERS | 1 +
tests/functional/meson.build | 1 +
tests/functional/test_riscv64_sifive_u.py | 51 +++++++++++++++++++++++
3 files changed, 53 insertions(+)
create mode 100755 tests/functional/test_riscv64_sifive_u.py
diff --git a/MAINTAINERS b/MAINTAINERS
index 28cea342718..a07086ed762 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1695,6 +1695,7 @@ S: Supported
F: docs/system/riscv/sifive_u.rst
F: hw/*/*sifive*.c
F: include/hw/*/*sifive*.h
+F: tests/functional/test_riscv64_sifive_u.py
AMD Microblaze-V Generic Board
M: Sai Pavan Boddu <sai.pavan.boddu@amd.com>
diff --git a/tests/functional/meson.build b/tests/functional/meson.build
index ecf965adc6c..311c6f18065 100644
--- a/tests/functional/meson.build
+++ b/tests/functional/meson.build
@@ -274,6 +274,7 @@ tests_riscv64_system_quick = [
]
tests_riscv64_system_thorough = [
+ 'riscv64_sifive_u',
'riscv64_tuxrun',
]
diff --git a/tests/functional/test_riscv64_sifive_u.py b/tests/functional/test_riscv64_sifive_u.py
new file mode 100755
index 00000000000..dc4cb8a4a96
--- /dev/null
+++ b/tests/functional/test_riscv64_sifive_u.py
@@ -0,0 +1,51 @@
+#!/usr/bin/env python3
+#
+# Functional test that boots a Linux kernel on a Sifive U machine
+# and checks the console
+#
+# Copyright (c) Linaro Ltd.
+#
+# Author:
+# Philippe Mathieu-Daudé
+#
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+import os
+
+from qemu_test import Asset, LinuxKernelTest
+from qemu_test import skipIfMissingCommands
+
+
+class SifiveU(LinuxKernelTest):
+
+ ASSET_KERNEL = Asset(
+ 'https://storage.tuxboot.com/buildroot/20241119/riscv64/Image',
+ '2bd8132a3bf21570290042324fff48c987f42f2a00c08de979f43f0662ebadba')
+ ASSET_ROOTFS = Asset(
+ ('https://github.com/groeck/linux-build-test/raw/'
+ '9819da19e6eef291686fdd7b029ea00e764dc62f/rootfs/riscv64/'
+ 'rootfs.ext2.gz'),
+ 'b6ed95610310b7956f9bf20c4c9c0c05fea647900df441da9dfe767d24e8b28b')
+
+ def test_riscv64_sifive_u_mmc_spi(self):
+ self.set_machine('sifive_u')
+ kernel_path = self.ASSET_KERNEL.fetch()
+ rootfs_path = self.uncompress(self.ASSET_ROOTFS)
+
+ self.vm.set_console()
+ kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE +
+ 'root=/dev/mmcblk0 rootwait '
+ 'earlycon=sbi console=ttySIF0 '
+ 'panic=-1 noreboot')
+ self.vm.add_args('-kernel', kernel_path,
+ '-drive', f'file={rootfs_path},if=sd,format=raw',
+ '-append', kernel_command_line,
+ '-no-reboot')
+ self.vm.launch()
+ self.wait_for_console_pattern('Boot successful.')
+
+ os.remove(rootfs_path)
+
+
+if __name__ == '__main__':
+ LinuxKernelTest.main()
--
2.49.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH-for-10.1 v2 00/11] hw/sd: Fix SD cards in SPI mode
2025-08-04 13:33 [PATCH-for-10.1 v2 00/11] hw/sd: Fix SD cards in SPI mode Philippe Mathieu-Daudé
` (10 preceding siblings ...)
2025-08-04 13:34 ` [PATCH-for-10.1 v2 11/11] tests/functional: Test SD cards in SPI mode (using sifive_u machine) Philippe Mathieu-Daudé
@ 2025-08-04 23:03 ` Richard Henderson
2025-08-04 23:09 ` Philippe Mathieu-Daudé
2025-08-05 14:29 ` Philippe Mathieu-Daudé
2025-08-06 6:39 ` Michael Tokarev
13 siblings, 1 reply; 21+ messages in thread
From: Richard Henderson @ 2025-08-04 23:03 UTC (permalink / raw)
To: qemu-devel
On 8/4/25 23:33, Philippe Mathieu-Daudé wrote:
> Philippe Mathieu-Daudé (11):
> hw/sd/sdcard: Do not ignore errors in sd_cmd_to_sendingdata()
> hw/sd/sdcard: Factor sd_response_size() out
> hw/sd/sdbus: Provide buffer size to sdbus_do_command()
> hw/sd/sdcard: Fill SPI response bits in card code
> hw/sd/sdcard: Implement SPI R2 return value
> hw/sd/sdcard: Use complete SEND_OP_COND implementation in SPI mode
> hw/sd/sdcard: Allow using SWITCH_FUNCTION in more SPI states
> hw/sd/sdcard: Factor spi_cmd_SEND_CxD() out
> hw/sd/sdcard: Disable checking STBY mode in SPI SEND_CSD/CID
> hw/sd/sdcard: RemoveSDState::mode field
> tests/functional: Test SD cards in SPI mode (using sifive_u machine)
I've reached the limit of what I can review simply by looking at the code, rather than
absorb the specs. The rest looks plausible, and doesn't twig the Spidey sense that
something icky is going on, so
Acked-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH-for-10.1 v2 00/11] hw/sd: Fix SD cards in SPI mode
2025-08-04 23:03 ` [PATCH-for-10.1 v2 00/11] hw/sd: Fix SD cards in SPI mode Richard Henderson
@ 2025-08-04 23:09 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-08-04 23:09 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
On 5/8/25 01:03, Richard Henderson wrote:
> On 8/4/25 23:33, Philippe Mathieu-Daudé wrote:
>> Philippe Mathieu-Daudé (11):
>> hw/sd/sdcard: Do not ignore errors in sd_cmd_to_sendingdata()
>> hw/sd/sdcard: Factor sd_response_size() out
>> hw/sd/sdbus: Provide buffer size to sdbus_do_command()
>> hw/sd/sdcard: Fill SPI response bits in card code
>> hw/sd/sdcard: Implement SPI R2 return value
>> hw/sd/sdcard: Use complete SEND_OP_COND implementation in SPI mode
>> hw/sd/sdcard: Allow using SWITCH_FUNCTION in more SPI states
>> hw/sd/sdcard: Factor spi_cmd_SEND_CxD() out
>> hw/sd/sdcard: Disable checking STBY mode in SPI SEND_CSD/CID
>> hw/sd/sdcard: RemoveSDState::mode field
>> tests/functional: Test SD cards in SPI mode (using sifive_u machine)
>
> I've reached the limit of what I can review simply by looking at the
> code, rather than absorb the specs. The rest looks plausible, and
> doesn't twig the Spidey sense that something icky is going on, so
>
> Acked-by: Richard Henderson <richard.henderson@linaro.org>
Thanks a lot!
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH-for-10.1 v2 00/11] hw/sd: Fix SD cards in SPI mode
2025-08-04 13:33 [PATCH-for-10.1 v2 00/11] hw/sd: Fix SD cards in SPI mode Philippe Mathieu-Daudé
` (11 preceding siblings ...)
2025-08-04 23:03 ` [PATCH-for-10.1 v2 00/11] hw/sd: Fix SD cards in SPI mode Richard Henderson
@ 2025-08-05 14:29 ` Philippe Mathieu-Daudé
2025-08-06 6:39 ` Michael Tokarev
13 siblings, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-08-05 14:29 UTC (permalink / raw)
To: Peter Maydell, qemu-devel
Cc: Ben Dooks, qemu-riscv, qemu-block, qemu-arm, Guenter Roeck,
Bin Meng
On 4/8/25 15:33, Philippe Mathieu-Daudé wrote:
> Philippe Mathieu-Daudé (11):
> hw/sd/sdcard: Do not ignore errors in sd_cmd_to_sendingdata()
> hw/sd/sdcard: Factor sd_response_size() out
> hw/sd/sdbus: Provide buffer size to sdbus_do_command()
> hw/sd/sdcard: Fill SPI response bits in card code
> hw/sd/sdcard: Implement SPI R2 return value
> hw/sd/sdcard: Use complete SEND_OP_COND implementation in SPI mode
> hw/sd/sdcard: Allow using SWITCH_FUNCTION in more SPI states
> hw/sd/sdcard: Factor spi_cmd_SEND_CxD() out
> hw/sd/sdcard: Disable checking STBY mode in SPI SEND_CSD/CID
> hw/sd/sdcard: Remove SDState::mode field
> tests/functional: Test SD cards in SPI mode (using sifive_u machine)
Series queued.
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH-for-10.1 v2 00/11] hw/sd: Fix SD cards in SPI mode
2025-08-04 13:33 [PATCH-for-10.1 v2 00/11] hw/sd: Fix SD cards in SPI mode Philippe Mathieu-Daudé
` (12 preceding siblings ...)
2025-08-05 14:29 ` Philippe Mathieu-Daudé
@ 2025-08-06 6:39 ` Michael Tokarev
2025-08-06 7:22 ` Philippe Mathieu-Daudé
13 siblings, 1 reply; 21+ messages in thread
From: Michael Tokarev @ 2025-08-06 6:39 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Peter Maydell, qemu-devel
Cc: Ben Dooks, qemu-riscv, qemu-block, qemu-arm, Guenter Roeck,
Bin Meng
On 04.08.2025 16:33, Philippe Mathieu-Daudé wrote:
> Since v1:
> - new patch factoring sd_response_size() out (Richard)
>
> This series fix a pair of issues with SD cards used wired
> via a SPI link / controller.
>
> Such mode implementation was minimal. I was testing it with
> the ARM Gumstix machines, but we remove them in the v9.2.0
> release (commit a2ccff4d2bc ), so they bit-rotted.
>
> Although the series looks big, I shrinked it a lot to have
> the minimum amount of meaningful changes. Other changes
> added during debugging will be shared later, as I believe
> they will still be useful to debug other future issues.
>
> The last patch add testing coverage, to avoid further bitrot.
>
> Regards,
>
> Phil.
>
> Philippe Mathieu-Daudé (11):
> hw/sd/sdcard: Do not ignore errors in sd_cmd_to_sendingdata()
> hw/sd/sdcard: Factor sd_response_size() out
> hw/sd/sdbus: Provide buffer size to sdbus_do_command()
> hw/sd/sdcard: Fill SPI response bits in card code
> hw/sd/sdcard: Implement SPI R2 return value
> hw/sd/sdcard: Use complete SEND_OP_COND implementation in SPI mode
> hw/sd/sdcard: Allow using SWITCH_FUNCTION in more SPI states
> hw/sd/sdcard: Factor spi_cmd_SEND_CxD() out
> hw/sd/sdcard: Disable checking STBY mode in SPI SEND_CSD/CID
> hw/sd/sdcard: Remove SDState::mode field
> tests/functional: Test SD cards in SPI mode (using sifive_u machine)
Hi!
Philippe, do you think this series is something which should
go to stable-10.0 (LTS) branch? I'm not sure for the impact if
it is not applied, though, - what do we miss in this case?
Thanks,
/mjt
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH-for-10.1 v2 00/11] hw/sd: Fix SD cards in SPI mode
2025-08-06 6:39 ` Michael Tokarev
@ 2025-08-06 7:22 ` Philippe Mathieu-Daudé
2025-08-07 16:06 ` Guenter Roeck
0 siblings, 1 reply; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-08-06 7:22 UTC (permalink / raw)
To: Michael Tokarev, Peter Maydell, qemu-devel
Cc: Ben Dooks, qemu-riscv, qemu-block, qemu-arm, Guenter Roeck,
Bin Meng
On 6/8/25 08:39, Michael Tokarev wrote:
> Philippe, do you think this series is something which should
> go to stable-10.0 (LTS) branch? I'm not sure for the impact if
> it is not applied, though, - what do we miss in this case?
Only 2 machines use a SD card wired over SPI lines:
$ git grep '"ssi-sd"'
hw/arm/stellaris.c:1302: sddev = ssi_create_peripheral(bus,
"ssi-sd");
hw/riscv/sifive_u.c:671: sd_dev =
ssi_create_peripheral(s->soc.spi2.spi, "ssi-sd");
hw/sd/ssi-sd.c:70:#define TYPE_SSI_SD "ssi-sd"
I don't know them enough to tell if they are that important. This
isn't a security problem. The emulation of the transport (SPI) to the
SD card being broken, guests can not access the emulated card.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH-for-10.1 v2 00/11] hw/sd: Fix SD cards in SPI mode
2025-08-06 7:22 ` Philippe Mathieu-Daudé
@ 2025-08-07 16:06 ` Guenter Roeck
2025-08-08 13:34 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 21+ messages in thread
From: Guenter Roeck @ 2025-08-07 16:06 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: Michael Tokarev, Peter Maydell, qemu-devel, Ben Dooks, qemu-riscv,
qemu-block, qemu-arm, Bin Meng
On Wed, Aug 06, 2025 at 09:22:46AM +0200, Philippe Mathieu-Daudé wrote:
> On 6/8/25 08:39, Michael Tokarev wrote:
>
> > Philippe, do you think this series is something which should
> > go to stable-10.0 (LTS) branch? I'm not sure for the impact if
> > it is not applied, though, - what do we miss in this case?
>
> Only 2 machines use a SD card wired over SPI lines:
>
> $ git grep '"ssi-sd"'
> hw/arm/stellaris.c:1302: sddev = ssi_create_peripheral(bus,
> "ssi-sd");
> hw/riscv/sifive_u.c:671: sd_dev = ssi_create_peripheral(s->soc.spi2.spi,
> "ssi-sd");
> hw/sd/ssi-sd.c:70:#define TYPE_SSI_SD "ssi-sd"
>
> I don't know them enough to tell if they are that important. This
> isn't a security problem. The emulation of the transport (SPI) to the
> SD card being broken, guests can not access the emulated card.
With 10.1.0-rc2, trying to boot v6.16-11744-g9c389564fa8e on sifive_u, I get:
[ 2.503619] riscv-pmu-sbi: 16 firmware and 18 hardware counters
[ 2.503672] riscv-pmu-sbi: Perf sampling/filtering is not supported as sscof extension is not available
qemu-system-riscv64: ../hw/sd/ssi-sd.c:160: ssi_sd_transfer: Assertion `s->arglen > 0' failed.
This is without trying to boot from it. Oddly enough, booting from SD card
does work.
Guenter
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH-for-10.1 v2 00/11] hw/sd: Fix SD cards in SPI mode
2025-08-07 16:06 ` Guenter Roeck
@ 2025-08-08 13:34 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 21+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-08-08 13:34 UTC (permalink / raw)
To: Guenter Roeck
Cc: Michael Tokarev, Peter Maydell, qemu-devel, Ben Dooks, qemu-riscv,
qemu-block, qemu-arm, Bin Meng
On 7/8/25 18:06, Guenter Roeck wrote:
> On Wed, Aug 06, 2025 at 09:22:46AM +0200, Philippe Mathieu-Daudé wrote:
>> On 6/8/25 08:39, Michael Tokarev wrote:
>>
>>> Philippe, do you think this series is something which should
>>> go to stable-10.0 (LTS) branch? I'm not sure for the impact if
>>> it is not applied, though, - what do we miss in this case?
>>
>> Only 2 machines use a SD card wired over SPI lines:
>>
>> $ git grep '"ssi-sd"'
>> hw/arm/stellaris.c:1302: sddev = ssi_create_peripheral(bus,
>> "ssi-sd");
>> hw/riscv/sifive_u.c:671: sd_dev = ssi_create_peripheral(s->soc.spi2.spi,
>> "ssi-sd");
>> hw/sd/ssi-sd.c:70:#define TYPE_SSI_SD "ssi-sd"
>>
>> I don't know them enough to tell if they are that important. This
>> isn't a security problem. The emulation of the transport (SPI) to the
>> SD card being broken, guests can not access the emulated card.
>
> With 10.1.0-rc2, trying to boot v6.16-11744-g9c389564fa8e on sifive_u, I get:
>
> [ 2.503619] riscv-pmu-sbi: 16 firmware and 18 hardware counters
> [ 2.503672] riscv-pmu-sbi: Perf sampling/filtering is not supported as sscof extension is not available
> qemu-system-riscv64: ../hw/sd/ssi-sd.c:160: ssi_sd_transfer: Assertion `s->arglen > 0' failed.
>
> This is without trying to boot from it. Oddly enough, booting from SD card
> does work.
This was a latent bug, thank you for the report.
I'll post a fix & test ASAP.
Regards,
Phil.
^ permalink raw reply [flat|nested] 21+ messages in thread