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)

 MAINTAINERS                               |   1 +
 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                                | 198 ++++++++++++++++------
 hw/sd/sdhci.c                             |   6 +-
 hw/sd/ssi-sd.c                            | 100 ++---------
 hw/sd/trace-events                        |   4 +-
 tests/functional/meson.build              |   1 +
 tests/functional/test_riscv64_sifive_u.py |  51 ++++++
 13 files changed, 249 insertions(+), 165 deletions(-)
 create mode 100755 tests/functional/test_riscv64_sifive_u.py

-- 
2.49.0

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

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.

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

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.

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.

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~

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!