[PULL 00/16] SD/MMC patches for 2024-07-06

[PULL 00/16] SD/MMC patches for 2024-07-06

[Philippe Mathieu-Daudé]

The following changes since commit f2cb4026fccfe073f84a4b440e41d3ed0c3134f6:

  Merge tag 'pull-maintainer-july24-050724-1' of https://gitlab.com/stsquad/qemu into staging (2024-07-05 09:15:48 -0700)

are available in the Git repository at:

  https://github.com/philmd/qemu.git tags/sdmmc-20240706

for you to fetch changes up to 0bcea3f74b04cdc23ecd6822bea7e46a55eb4be7:

  hw/sd/sdcard: Extract TYPE_SDMMC_COMMON from TYPE_SD_CARD (2024-07-05 23:55:02 +0200)

----------------------------------------------------------------
SD/MMC patches queue

- Use published card address (RCA) in qtest/npcm7xx_sdhci
- Have cards use random RCA
- Use SD spec v3.01 by default
- Convert GEN_CMD to sd_generic_read/write_byte style
- Extract SDMMC_COMMON abstract QDev parent from SD_CARD
- Few housekeeping

----------------------------------------------------------------

Philippe Mathieu-Daudé (16):
  hw/sd/sdhci: Log non-sequencial access as GUEST_ERROR
  hw/sd/npcm7xx_sdhci: Use TYPE_SYSBUS_SDHCI definition
  tests/qtest/npcm7xx_sdhci: Access the card using its published address
  hw/sd/sdcard: Generate random RCA value
  hw/sd/sdcard: Remove leftover comment about removed 'spi' Property
  hw/sd/sdcard: Use spec v3.01 by default
  hw/sd/sdcard: Rename sd_cmd_SEND_OP_COND handler
  hw/sd/sdcard: Add sd_cmd_GEN_CMD handler (CMD56)
  hw/sd/sdcard: Remove sd_none enum from sd_cmd_type_t
  hw/sd/sdcard: Remove noise from sd_acmd_name()
  hw/sd/sdcard: Remove noise from sd_cmd_name()
  hw/sd/sdcard: Remove default case in read/write on DAT lines
  hw/sd/sdcard: Trace length of data read on DAT lines
  hw/sd/sdcard: Cover more SDCardStates
  hw/sd/sdcard: Introduce set_csd/set_cid handlers
  hw/sd/sdcard: Extract TYPE_SDMMC_COMMON from TYPE_SD_CARD

 hw/sd/sdmmc-internal.h           |   3 +
 include/hw/sd/sd.h               |   3 +-
 tests/qtest/libqos/sdhci-cmd.h   |   2 +
 hw/core/machine.c                |   1 +
 hw/sd/core.c                     |  29 ++---
 hw/sd/npcm7xx_sdhci.c            |   3 +-
 hw/sd/sd.c                       | 177 +++++++++++++++----------------
 hw/sd/sdhci.c                    |   5 +-
 tests/qtest/npcm7xx_sdhci-test.c |   8 +-
 hw/sd/trace-events               |   3 +-
 10 files changed, 124 insertions(+), 110 deletions(-)

-- 
2.41.0

[PULL 01/16] hw/sd/sdhci: Log non-sequencial access as GUEST_ERROR

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Tested-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Message-Id: <20240702140842.54242-3-philmd@linaro.org>
---
 hw/sd/sdhci.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 27673e1c70..d02c3e3963 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -983,8 +983,9 @@ static inline bool
 sdhci_buff_access_is_sequential(SDHCIState *s, unsigned byte_num)
 {
     if ((s->data_count & 0x3) != byte_num) {
-        trace_sdhci_error("Non-sequential access to Buffer Data Port register"
-                          "is prohibited\n");
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "SDHCI: Non-sequential access to Buffer Data Port"
+                      " register is prohibited\n");
         return false;
     }
     return true;
-- 
2.41.0

[PULL 02/16] hw/sd/npcm7xx_sdhci: Use TYPE_SYSBUS_SDHCI definition

[Philippe Mathieu-Daudé]

Use the macro instead of two explicit string literals.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Message-Id: <20240702140842.54242-2-philmd@linaro.org>
---
 hw/sd/npcm7xx_sdhci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/sd/npcm7xx_sdhci.c b/hw/sd/npcm7xx_sdhci.c
index e93dab8dbd..fb51821e11 100644
--- a/hw/sd/npcm7xx_sdhci.c
+++ b/hw/sd/npcm7xx_sdhci.c
@@ -16,6 +16,7 @@
 
 #include "qemu/osdep.h"
 
+#include "hw/sd/sdhci.h"
 #include "hw/sd/npcm7xx_sdhci.h"
 #include "migration/vmstate.h"
 #include "sdhci-internal.h"
@@ -162,7 +163,7 @@ static void npcm7xx_sdhci_instance_init(Object *obj)
 {
     NPCM7xxSDHCIState *s = NPCM7XX_SDHCI(obj);
 
-    object_initialize_child(OBJECT(s), "generic-sdhci", &s->sdhci,
+    object_initialize_child(OBJECT(s), TYPE_SYSBUS_SDHCI, &s->sdhci,
                             TYPE_SYSBUS_SDHCI);
 }
 
-- 
2.41.0

[PULL 03/16] tests/qtest/npcm7xx_sdhci: Access the card using its published address

[Philippe Mathieu-Daudé]

Currently setup_sd_card() asks the card its address,
but discard the response and use hardcoded 0x4567.

Set the SDHC_CMD_RESPONSE bit to have the controller
record the bus response, and read the response from
the RSPREG0 register. Then we can select the card with
its real address.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Message-Id: <20240702140842.54242-4-philmd@linaro.org>
---
 tests/qtest/libqos/sdhci-cmd.h   | 2 ++
 tests/qtest/npcm7xx_sdhci-test.c | 8 ++++++--
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/libqos/sdhci-cmd.h b/tests/qtest/libqos/sdhci-cmd.h
index 9e61dd4944..90efa028ef 100644
--- a/tests/qtest/libqos/sdhci-cmd.h
+++ b/tests/qtest/libqos/sdhci-cmd.h
@@ -22,6 +22,7 @@
 #define SDHC_ARGUMENT 0x08
 #define SDHC_TRNMOD 0x0C
 #define SDHC_CMDREG 0x0E
+#define SDHC_RSPREG0 0x10
 #define SDHC_BDATA 0x20
 #define SDHC_PRNSTS 0x24
 #define SDHC_BLKGAP 0x2A
@@ -38,6 +39,7 @@
 #define SDHC_TRNS_MULTI 0x0020
 
 /* CMD Reg */
+#define SDHC_CMD_RESPONSE (3 << 0)
 #define SDHC_CMD_DATA_PRESENT (1 << 5)
 #define SDHC_ALL_SEND_CID (2 << 8)
 #define SDHC_SEND_RELATIVE_ADDR (3 << 8)
diff --git a/tests/qtest/npcm7xx_sdhci-test.c b/tests/qtest/npcm7xx_sdhci-test.c
index 5d68540e52..01f237a816 100644
--- a/tests/qtest/npcm7xx_sdhci-test.c
+++ b/tests/qtest/npcm7xx_sdhci-test.c
@@ -30,6 +30,8 @@ char *sd_path;
 
 static QTestState *setup_sd_card(void)
 {
+    uint16_t rca;
+
     QTestState *qts = qtest_initf(
         "-machine kudo-bmc "
         "-device sd-card,drive=drive0 "
@@ -43,8 +45,10 @@ static QTestState *setup_sd_card(void)
     sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0, 0, SDHC_APP_CMD);
     sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0x41200000, 0, (41 << 8));
     sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0, 0, SDHC_ALL_SEND_CID);
-    sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0, 0, SDHC_SEND_RELATIVE_ADDR);
-    sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0x45670000, 0,
+    sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0, 0, SDHC_SEND_RELATIVE_ADDR
+                                                    | SDHC_CMD_RESPONSE);
+    rca = qtest_readl(qts, NPCM7XX_MMC_BA + SDHC_RSPREG0) >> 16;
+    sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, rca << 16, 0,
                    SDHC_SELECT_DESELECT_CARD);
 
     return qts;
-- 
2.41.0

[PULL 04/16] hw/sd/sdcard: Generate random RCA value

[Philippe Mathieu-Daudé]

Rather than using the obscure 0x4567 magic value,
use a real random one.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Tested-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Message-Id: <20240702140842.54242-5-philmd@linaro.org>
---
 hw/sd/sd.c         | 11 ++++++++---
 hw/sd/trace-events |  1 +
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 552957b2e5..b158402704 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -46,6 +46,7 @@
 #include "qemu/error-report.h"
 #include "qemu/timer.h"
 #include "qemu/log.h"
+#include "qemu/guest-random.h"
 #include "qemu/module.h"
 #include "sdmmc-internal.h"
 #include "trace.h"
@@ -515,9 +516,10 @@ static void sd_set_csd(SDState *sd, uint64_t size)
 
 /* Relative Card Address register */
 
-static void sd_set_rca(SDState *sd)
+static void sd_set_rca(SDState *sd, uint16_t value)
 {
-    sd->rca += 0x4567;
+    trace_sdcard_set_rca(value);
+    sd->rca = value;
 }
 
 static uint16_t sd_req_get_rca(SDState *s, SDRequest req)
@@ -1212,11 +1214,14 @@ static sd_rsp_type_t sd_cmd_ALL_SEND_CID(SDState *sd, SDRequest req)
 /* CMD3 */
 static sd_rsp_type_t sd_cmd_SEND_RELATIVE_ADDR(SDState *sd, SDRequest req)
 {
+    uint16_t random_rca;
+
     switch (sd->state) {
     case sd_identification_state:
     case sd_standby_state:
         sd->state = sd_standby_state;
-        sd_set_rca(sd);
+        qemu_guest_getrandom_nofail(&random_rca, sizeof(random_rca));
+        sd_set_rca(sd, random_rca);
         return sd_r6;
 
     default:
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index 43eaeba149..6a51b0e906 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -43,6 +43,7 @@ sdcard_response(const char *rspdesc, int rsplen) "%s (sz:%d)"
 sdcard_powerup(void) ""
 sdcard_inquiry_cmd41(void) ""
 sdcard_reset(void) ""
+sdcard_set_rca(uint16_t value) "new RCA: 0x%04x"
 sdcard_set_blocklen(uint16_t length) "block len 0x%03x"
 sdcard_set_block_count(uint32_t cnt) "block cnt 0x%"PRIx32
 sdcard_inserted(bool readonly) "read_only: %u"
-- 
2.41.0

[PULL 05/16] hw/sd/sdcard: Remove leftover comment about removed 'spi' Property

[Philippe Mathieu-Daudé]

Commit c3287c0f70 ("hw/sd: Introduce a "sd-card" SPI variant
model") removed the 'spi' property. Remove the comment left
over.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>
Tested-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Message-Id: <20240703085907.66775-2-philmd@linaro.org>
---
 hw/sd/sd.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index b158402704..53767beaf8 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2473,10 +2473,6 @@ static Property sd_properties[] = {
     DEFINE_PROP_UINT8("spec_version", SDState,
                       spec_version, SD_PHY_SPECv2_00_VERS),
     DEFINE_PROP_DRIVE("drive", SDState, blk),
-    /* We do not model the chip select pin, so allow the board to select
-     * whether card should be in SSI or MMC/SD mode.  It is also up to the
-     * board to ensure that ssi transfers only occur when the chip select
-     * is asserted.  */
     DEFINE_PROP_END_OF_LIST()
 };
 
-- 
2.41.0

[PULL 06/16] hw/sd/sdcard: Use spec v3.01 by default

[Philippe Mathieu-Daudé]

Recent SDHCI expect cards to support the v3.01 spec
to negociate lower I/O voltage. Select it by default.

Versioned machine types with a version of 9.0 or
earlier retain the old default (spec v2.00).

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Tested-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Tested-by: Cédric Le Goater <clg@redhat.com>
Message-Id: <20240703134356.85972-2-philmd@linaro.org>
---
 hw/core/machine.c | 1 +
 hw/sd/sd.c        | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/core/machine.c b/hw/core/machine.c
index f4cba6496c..bc38cad7f2 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -39,6 +39,7 @@ GlobalProperty hw_compat_9_0[] = {
     {"scsi-disk-base", "migrate-emulated-scsi-request", "false" },
     {"vfio-pci", "skip-vsc-check", "false" },
     { "virtio-pci", "x-pcie-pm-no-soft-reset", "off" },
+    {"sd-card", "spec_version", "2" },
 };
 const size_t hw_compat_9_0_len = G_N_ELEMENTS(hw_compat_9_0);
 
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 53767beaf8..a08a452d81 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2471,7 +2471,7 @@ static void sd_realize(DeviceState *dev, Error **errp)
 
 static Property sd_properties[] = {
     DEFINE_PROP_UINT8("spec_version", SDState,
-                      spec_version, SD_PHY_SPECv2_00_VERS),
+                      spec_version, SD_PHY_SPECv3_01_VERS),
     DEFINE_PROP_DRIVE("drive", SDState, blk),
     DEFINE_PROP_END_OF_LIST()
 };
-- 
2.41.0

[PULL 07/16] hw/sd/sdcard: Rename sd_cmd_SEND_OP_COND handler

[Philippe Mathieu-Daudé]

The correct command name is 'SD SEND_OP_COND',
rename accordingly.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Tested-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Tested-by: Cédric Le Goater <clg@redhat.com>
Message-Id: <20240703134356.85972-4-philmd@linaro.org>
---
 hw/sd/sd.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index a08a452d81..8618c2bcf0 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1689,7 +1689,7 @@ static sd_rsp_type_t sd_acmd_SET_WR_BLK_ERASE_COUNT(SDState *sd, SDRequest req)
 }
 
 /* ACMD41 */
-static sd_rsp_type_t sd_acmd_SD_APP_OP_COND(SDState *sd, SDRequest req)
+static sd_rsp_type_t sd_cmd_SEND_OP_COND(SDState *sd, SDRequest req)
 {
     if (sd->state != sd_idle_state) {
         return sd_invalid_state_for_cmd(sd, req);
@@ -2392,7 +2392,7 @@ static const SDProto sd_proto_sd = {
         [13] = {8,  sd_adtc, "SD_STATUS", sd_acmd_SD_STATUS},
         [22] = {8,  sd_adtc, "SEND_NUM_WR_BLOCKS", sd_acmd_SEND_NUM_WR_BLOCKS},
         [23] = {8,  sd_ac,   "SET_WR_BLK_ERASE_COUNT", sd_acmd_SET_WR_BLK_ERASE_COUNT},
-        [41] = {8,  sd_bcr,  "SD_APP_OP_COND", sd_acmd_SD_APP_OP_COND},
+        [41] = {8,  sd_bcr,  "SEND_OP_COND", sd_cmd_SEND_OP_COND},
         [42] = {8,  sd_ac,   "SET_CLR_CARD_DETECT", sd_acmd_SET_CLR_CARD_DETECT},
         [51] = {8,  sd_adtc, "SEND_SCR", sd_acmd_SEND_SCR},
     },
-- 
2.41.0

[PULL 08/16] hw/sd/sdcard: Add sd_cmd_GEN_CMD handler (CMD56)

[Philippe Mathieu-Daudé]

"General command" (GEN_CMD, CMD56) is described as:

  GEN_CMD is the same as the single block read or write
  commands (CMD24 or CMD17). The difference is that [...]
  the data block is not a memory payload data but has a
  vendor specific format and meaning.

Thus this block must not be stored overwriting data block
on underlying storage drive. Handle as RAZ/WI.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Tested-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Tested-by: Cédric Le Goater <clg@redhat.com>
Message-Id: <20240703134356.85972-3-philmd@linaro.org>
---
 hw/sd/sd.c | 54 ++++++++++++++++++++----------------------------------
 1 file changed, 20 insertions(+), 34 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 8618c2bcf0..10f2764a53 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -243,7 +243,6 @@ static const char *sd_cmd_name(SDState *sd, uint8_t cmd)
                                             [25]    = "WRITE_MULTIPLE_BLOCK",
         [26]    = "MANUF_RSVD",
         [40]    = "DPS_spec",
-        [56]    = "GEN_CMD",
         [60]    = "MANUF_RSVD",             [61]    = "MANUF_RSVD",
         [62]    = "MANUF_RSVD",             [63]    = "MANUF_RSVD",
     };
@@ -902,9 +901,6 @@ static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len)
     }
 }
 
-#define APP_READ_BLOCK(a, len)  memset(sd->data, 0xec, len)
-#define APP_WRITE_BLOCK(a, len)
-
 static void sd_erase(SDState *sd)
 {
     uint64_t erase_start = sd->erase_start;
@@ -1641,6 +1637,22 @@ static sd_rsp_type_t sd_cmd_APP_CMD(SDState *sd, SDRequest req)
     return sd_r1;
 }
 
+/* CMD56 */
+static sd_rsp_type_t sd_cmd_GEN_CMD(SDState *sd, SDRequest req)
+{
+    if (sd->state != sd_transfer_state) {
+        return sd_invalid_state_for_cmd(sd, req);
+    }
+
+    /* Vendor specific command: our model is RAZ/WI */
+    if (req.arg & 1) {
+        memset(sd->data, 0, sizeof(sd->data));
+        return sd_cmd_to_sendingdata(sd, req, 0, NULL, 0);
+    } else {
+        return sd_cmd_to_receivingdata(sd, req, 0, 0);
+    }
+}
+
 /* CMD58 */
 static sd_rsp_type_t spi_cmd_READ_OCR(SDState *sd, SDRequest req)
 {
@@ -1836,22 +1848,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
     case 26:  /* CMD26:  PROGRAM_CID */
         return sd_cmd_to_receivingdata(sd, req, 0, sizeof(sd->cid));
 
-    /* Application specific commands (Class 8) */
-    case 56:  /* CMD56:  GEN_CMD */
-        switch (sd->state) {
-        case sd_transfer_state:
-            sd->data_offset = 0;
-            if (req.arg & 1)
-                sd->state = sd_sendingdata_state;
-            else
-                sd->state = sd_receivingdata_state;
-            return sd_r1;
-
-        default:
-            break;
-        }
-        break;
-
     default:
         qemu_log_mask(LOG_GUEST_ERROR, "SD: Unknown CMD%i\n", req.cmd);
         return sd_illegal;
@@ -2187,11 +2183,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
         break;
 
     case 56:  /* CMD56:  GEN_CMD */
-        sd->data[sd->data_offset ++] = value;
-        if (sd->data_offset >= sd->blk_len) {
-            APP_WRITE_BLOCK(sd->data_start, sd->data_offset);
-            sd->state = sd_transfer_state;
-        }
+        sd_generic_write_byte(sd, value);
         break;
 
     default:
@@ -2233,6 +2225,7 @@ uint8_t sd_read_byte(SDState *sd)
     case 22: /* ACMD22: SEND_NUM_WR_BLOCKS */
     case 30: /* CMD30:  SEND_WRITE_PROT */
     case 51: /* ACMD51: SEND_SCR */
+    case 56: /* CMD56:  GEN_CMD */
         sd_generic_read_byte(sd, &ret);
         break;
 
@@ -2260,15 +2253,6 @@ uint8_t sd_read_byte(SDState *sd)
         }
         break;
 
-    case 56:  /* CMD56:  GEN_CMD */
-        if (sd->data_offset == 0)
-            APP_READ_BLOCK(sd->data_start, sd->blk_len);
-        ret = sd->data[sd->data_offset ++];
-
-        if (sd->data_offset >= sd->blk_len)
-            sd->state = sd_transfer_state;
-        break;
-
     default:
         qemu_log_mask(LOG_GUEST_ERROR, "%s: unknown command\n", __func__);
         return 0x00;
@@ -2323,6 +2307,7 @@ static const SDProto sd_proto_spi = {
         [52] = {9,  sd_spi, "IO_RW_DIRECT", sd_cmd_optional},
         [53] = {9,  sd_spi, "IO_RW_EXTENDED", sd_cmd_optional},
         [55] = {8,  sd_spi, "APP_CMD", sd_cmd_APP_CMD},
+        [56] = {8,  sd_spi, "GEN_CMD", sd_cmd_GEN_CMD},
         [57] = {10, sd_spi, "DIRECT_SECURE_WRITE", sd_cmd_optional},
         [58] = {0,  sd_spi, "READ_OCR", spi_cmd_READ_OCR},
         [59] = {0,  sd_spi, "CRC_ON_OFF", spi_cmd_CRC_ON_OFF},
@@ -2383,6 +2368,7 @@ static const SDProto sd_proto_sd = {
         [52] = {9,  sd_bc,   "IO_RW_DIRECT", sd_cmd_optional},
         [53] = {9,  sd_bc,   "IO_RW_EXTENDED", sd_cmd_optional},
         [55] = {8,  sd_ac,   "APP_CMD", sd_cmd_APP_CMD},
+        [56] = {8,  sd_adtc, "GEN_CMD", sd_cmd_GEN_CMD},
         [57] = {10, sd_adtc, "DIRECT_SECURE_WRITE", sd_cmd_optional},
         [58] = {11, sd_adtc, "READ_EXTR_MULTI", sd_cmd_optional},
         [59] = {11, sd_adtc, "WRITE_EXTR_MULTI", sd_cmd_optional},
-- 
2.41.0

[PULL 09/16] hw/sd/sdcard: Remove sd_none enum from sd_cmd_type_t

[Philippe Mathieu-Daudé]

All handlers using the 'sd_none' enum got converted,
remove it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Tested-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Tested-by: Cédric Le Goater <clg@redhat.com>
Message-Id: <20240628070216.92609-76-philmd@linaro.org>
---
 include/hw/sd/sd.h | 1 -
 hw/sd/sd.c         | 7 +------
 2 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index 29c76935a0..c1a35ab420 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -76,7 +76,6 @@ typedef enum  {
 } sd_uhs_mode_t;
 
 typedef enum {
-    sd_none = 0,
     sd_spi,
     sd_bc,     /* broadcast -- no response */
     sd_bcr,    /* broadcast with response */
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 10f2764a53..43f60cf089 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -524,17 +524,12 @@ static void sd_set_rca(SDState *sd, uint16_t value)
 static uint16_t sd_req_get_rca(SDState *s, SDRequest req)
 {
     switch (s->proto->cmd[req.cmd].type) {
-    case sd_none:
-        /* Called from legacy code not ported to SDProto array */
-        assert(!s->proto->cmd[req.cmd].handler);
-        /* fall-through */
     case sd_ac:
     case sd_adtc:
         return req.arg >> 16;
     case sd_spi:
-        g_assert_not_reached();
     default:
-        return 0;
+        g_assert_not_reached();
     }
 }
 
-- 
2.41.0

[PULL 10/16] hw/sd/sdcard: Remove noise from sd_acmd_name()

[Philippe Mathieu-Daudé]

These ACMD names weren't really useful, "UNKNOWN_ACMD" is simpler.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Tested-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Tested-by: Cédric Le Goater <clg@redhat.com>
Message-Id: <20240628070216.92609-77-philmd@linaro.org>
---
 hw/sd/sd.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 43f60cf089..274a917ba6 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -257,23 +257,13 @@ static const char *sd_cmd_name(SDState *sd, uint8_t cmd)
 
 static const char *sd_acmd_name(SDState *sd, uint8_t cmd)
 {
-    static const char *acmd_abbrev[SDMMC_CMD_MAX] = {
-        [14] = "DPS_spec",                  [15] = "DPS_spec",
-        [16] = "DPS_spec",
-        [18] = "SECU_spec",
-        [52] = "SECU_spec",                 [53] = "SECU_spec",
-        [54] = "SECU_spec",
-        [56] = "SECU_spec",                 [57] = "SECU_spec",
-        [58] = "SECU_spec",                 [59] = "SECU_spec",
-    };
     const SDProto *sdp = sd->proto;
 
     if (sdp->acmd[cmd].handler) {
-        assert(!acmd_abbrev[cmd]);
         return sdp->acmd[cmd].name;
     }
 
-    return acmd_abbrev[cmd] ? acmd_abbrev[cmd] : "UNKNOWN_ACMD";
+    return "UNKNOWN_ACMD";
 }
 
 static uint8_t sd_get_dat_lines(SDState *sd)
-- 
2.41.0

[PULL 11/16] hw/sd/sdcard: Remove noise from sd_cmd_name()

[Philippe Mathieu-Daudé]

These CMD names weren't really useful, "UNKNOWN_CMD" is simpler.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Tested-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Tested-by: Cédric Le Goater <clg@redhat.com>
Message-Id: <20240628070216.92609-78-philmd@linaro.org>
---
 hw/sd/sd.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 274a917ba6..bc1a574b62 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -239,12 +239,7 @@ static const char *sd_cmd_name(SDState *sd, uint8_t cmd)
 {
     static const char *cmd_abbrev[SDMMC_CMD_MAX] = {
         [18]    = "READ_MULTIPLE_BLOCK",
-                                            [21]    = "DPS_spec",
                                             [25]    = "WRITE_MULTIPLE_BLOCK",
-        [26]    = "MANUF_RSVD",
-        [40]    = "DPS_spec",
-        [60]    = "MANUF_RSVD",             [61]    = "MANUF_RSVD",
-        [62]    = "MANUF_RSVD",             [63]    = "MANUF_RSVD",
     };
     const SDProto *sdp = sd->proto;
 
-- 
2.41.0

[PULL 12/16] hw/sd/sdcard: Remove default case in read/write on DAT lines

[Philippe Mathieu-Daudé]

All read/write on DAT lines are explicitly handled.
Reaching this point would be a programming error:
replace by an assertion.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Tested-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Tested-by: Cédric Le Goater <clg@redhat.com>
Message-Id: <20240628070216.92609-79-philmd@linaro.org>
---
 hw/sd/sd.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index bc1a574b62..3f495f91fe 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1949,7 +1949,6 @@ int sd_do_command(SDState *sd, SDRequest *req,
         /* Valid command, we can update the 'state before command' bits.
          * (Do this now so they appear in r1 responses.)
          */
-        sd->current_cmd = req->cmd;
         sd->card_status = FIELD_DP32(sd->card_status, CSR,
                                      CURRENT_STATE, last_state);
     }
@@ -2014,6 +2013,8 @@ send_response:
     qemu_hexdump(stderr, "Response", response, rsplen);
 #endif
 
+    sd->current_cmd = rtype == sd_illegal ? 0 : req->cmd;
+
     return rsplen;
 }
 
@@ -2167,8 +2168,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
         break;
 
     default:
-        qemu_log_mask(LOG_GUEST_ERROR, "%s: unknown command\n", __func__);
-        break;
+        g_assert_not_reached();
     }
 }
 
@@ -2234,8 +2234,7 @@ uint8_t sd_read_byte(SDState *sd)
         break;
 
     default:
-        qemu_log_mask(LOG_GUEST_ERROR, "%s: unknown command\n", __func__);
-        return 0x00;
+        g_assert_not_reached();
     }
 
     return ret;
-- 
2.41.0

[PULL 13/16] hw/sd/sdcard: Trace length of data read on DAT lines

[Philippe Mathieu-Daudé]

Some commands expect less than BLOCK_LENGTH.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Tested-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Tested-by: Cédric Le Goater <clg@redhat.com>
Message-Id: <20240628070216.92609-80-philmd@linaro.org>
---
 hw/sd/sd.c         | 4 ++--
 hw/sd/trace-events | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 3f495f91fe..9e6b9c9c63 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2193,8 +2193,8 @@ uint8_t sd_read_byte(SDState *sd)
     io_len = sd_blk_len(sd);
 
     trace_sdcard_read_data(sd->proto->name,
-                           sd->last_cmd_name,
-                           sd->current_cmd, sd->data_offset, io_len);
+                           sd->last_cmd_name, sd->current_cmd,
+                           sd->data_offset, sd->data_size, io_len);
     switch (sd->current_cmd) {
     case 6:  /* CMD6:   SWITCH_FUNCTION */
     case 9:  /* CMD9:   SEND_CSD */
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index 6a51b0e906..5dfe6be7b7 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -55,7 +55,7 @@ sdcard_req_addr(uint32_t req_arg, uint64_t addr) "req 0x%" PRIx32 " addr 0x%" PR
 sdcard_read_block(uint64_t addr, uint32_t len) "addr 0x%" PRIx64 " size 0x%x"
 sdcard_write_block(uint64_t addr, uint32_t len) "addr 0x%" PRIx64 " size 0x%x"
 sdcard_write_data(const char *proto, const char *cmd_desc, uint8_t cmd, uint32_t offset, uint8_t value) "%s %20s/ CMD%02d ofs %"PRIu32" value 0x%02x"
-sdcard_read_data(const char *proto, const char *cmd_desc, uint8_t cmd, uint32_t offset, uint32_t length) "%s %20s/ CMD%02d ofs %"PRIu32" len %" PRIu32
+sdcard_read_data(const char *proto, const char *cmd_desc, uint8_t cmd, uint32_t offset, uint64_t size, uint32_t blklen) "%s %20s/ CMD%02d ofs %"PRIu32" size %"PRIu64" blklen %" PRIu32
 sdcard_set_voltage(uint16_t millivolts) "%u mV"
 
 # pxa2xx_mmci.c
-- 
2.41.0

[PULL 14/16] hw/sd/sdcard: Cover more SDCardStates

[Philippe Mathieu-Daudé]

So far eMMC will only use sd_sleep_state, but
all all states specified for completeness.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Tested-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Tested-by: Cédric Le Goater <clg@redhat.com>
Message-Id: <20240628070216.92609-81-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 9e6b9c9c63..ecb1b2f405 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -76,7 +76,9 @@ enum SDCardModes {
 };
 
 enum SDCardStates {
+    sd_waitirq_state        = -2, /* emmc */
     sd_inactive_state       = -1,
+
     sd_idle_state           = 0,
     sd_ready_state          = 1,
     sd_identification_state = 2,
@@ -86,6 +88,9 @@ enum SDCardStates {
     sd_receivingdata_state  = 6,
     sd_programming_state    = 7,
     sd_disconnect_state     = 8,
+    sd_bus_test_state       = 9,  /* emmc */
+    sd_sleep_state          = 10, /* emmc */
+    sd_io_state             = 15  /* sd */
 };
 
 #define SDMMC_CMD_MAX 64
@@ -203,13 +208,19 @@ static const char *sd_state_name(enum SDCardStates state)
         [sd_standby_state]          = "standby",
         [sd_transfer_state]         = "transfer",
         [sd_sendingdata_state]      = "sendingdata",
+        [sd_bus_test_state]         = "bus-test",
         [sd_receivingdata_state]    = "receivingdata",
         [sd_programming_state]      = "programming",
         [sd_disconnect_state]       = "disconnect",
+        [sd_sleep_state]            = "sleep",
+        [sd_io_state]               = "i/o"
     };
     if (state == sd_inactive_state) {
         return "inactive";
     }
+    if (state == sd_waitirq_state) {
+        return "wait-irq";
+    }
     assert(state < ARRAY_SIZE(state_name));
     return state_name[state];
 }
-- 
2.41.0

[PULL 15/16] hw/sd/sdcard: Introduce set_csd/set_cid handlers

[Philippe Mathieu-Daudé]

In preparation of introducing eMMC support which have
different CSD/CID structures, introduce a pair of handlers
in SDCardClass.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Tested-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Tested-by: Cédric Le Goater <clg@redhat.com>
Message-Id: <20240628070216.92609-82-philmd@linaro.org>
---
 include/hw/sd/sd.h | 2 ++
 hw/sd/sd.c         | 7 +++++--
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index c1a35ab420..0d6d9e452b 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -127,6 +127,8 @@ struct SDCardClass {
     void (*enable)(SDState *sd, bool enable);
     bool (*get_inserted)(SDState *sd);
     bool (*get_readonly)(SDState *sd);
+    void (*set_cid)(SDState *sd);
+    void (*set_csd)(SDState *sd, uint64_t size);
 
     const struct SDProto *proto;
 };
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index ecb1b2f405..058a70a0f8 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -680,6 +680,7 @@ static inline uint64_t sd_addr_to_wpnum(uint64_t addr)
 static void sd_reset(DeviceState *dev)
 {
     SDState *sd = SD_CARD(dev);
+    SDCardClass *sc = SD_CARD_GET_CLASS(sd);
     uint64_t size;
     uint64_t sect;
 
@@ -700,8 +701,8 @@ static void sd_reset(DeviceState *dev)
     sd->size = size;
     sd_set_ocr(sd);
     sd_set_scr(sd);
-    sd_set_cid(sd);
-    sd_set_csd(sd, size);
+    sc->set_cid(sd);
+    sc->set_csd(sd, size);
     sd_set_cardstatus(sd);
     sd_set_sdstatus(sd);
 
@@ -2475,6 +2476,8 @@ static void sd_class_init(ObjectClass *klass, void *data)
     sc->enable = sd_enable;
     sc->get_inserted = sd_get_inserted;
     sc->get_readonly = sd_get_readonly;
+    sc->set_cid = sd_set_cid;
+    sc->set_csd = sd_set_csd;
     sc->proto = &sd_proto_sd;
 }
 
-- 
2.41.0

[PULL 16/16] hw/sd/sdcard: Extract TYPE_SDMMC_COMMON from TYPE_SD_CARD

[Philippe Mathieu-Daudé]

In order to keep eMMC model simpler to maintain,
extract common properties and the common code from
class_init to the (internal) TYPE_SDMMC_COMMON.

Update the corresponding QOM cast macros.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Tested-by: Andrew Jeffery <andrew@codeconstruct.com.au>
Tested-by: Cédric Le Goater <clg@redhat.com>
Message-Id: <20240703134356.85972-6-philmd@linaro.org>
---
 hw/sd/sdmmc-internal.h |  3 +++
 hw/sd/core.c           | 29 ++++++++++++------------
 hw/sd/sd.c             | 51 +++++++++++++++++++++++++++++-------------
 3 files changed, 53 insertions(+), 30 deletions(-)

diff --git a/hw/sd/sdmmc-internal.h b/hw/sd/sdmmc-internal.h
index cc0b69e834..91eb5b6b2f 100644
--- a/hw/sd/sdmmc-internal.h
+++ b/hw/sd/sdmmc-internal.h
@@ -11,6 +11,9 @@
 #ifndef SDMMC_INTERNAL_H
 #define SDMMC_INTERNAL_H
 
+#define TYPE_SDMMC_COMMON "sdmmc-common"
+DECLARE_OBJ_CHECKERS(SDState, SDCardClass, SDMMC_COMMON, TYPE_SDMMC_COMMON)
+
 /*
  * EXT_CSD Modes segment
  *
diff --git a/hw/sd/core.c b/hw/sd/core.c
index 52d5d90045..4b30218b52 100644
--- a/hw/sd/core.c
+++ b/hw/sd/core.c
@@ -24,6 +24,7 @@
 #include "hw/sd/sd.h"
 #include "qemu/module.h"
 #include "qapi/error.h"
+#include "sdmmc-internal.h"
 #include "trace.h"
 
 static inline const char *sdbus_name(SDBus *sdbus)
@@ -39,7 +40,7 @@ static SDState *get_card(SDBus *sdbus)
     if (!kid) {
         return NULL;
     }
-    return SD_CARD(kid->child);
+    return SDMMC_COMMON(kid->child);
 }
 
 uint8_t sdbus_get_dat_lines(SDBus *sdbus)
@@ -48,7 +49,7 @@ uint8_t sdbus_get_dat_lines(SDBus *sdbus)
     uint8_t dat_lines = 0b1111; /* 4 bit bus width */
 
     if (slave) {
-        SDCardClass *sc = SD_CARD_GET_CLASS(slave);
+        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(slave);
 
         if (sc->get_dat_lines) {
             dat_lines = sc->get_dat_lines(slave);
@@ -65,7 +66,7 @@ bool sdbus_get_cmd_line(SDBus *sdbus)
     bool cmd_line = true;
 
     if (slave) {
-        SDCardClass *sc = SD_CARD_GET_CLASS(slave);
+        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(slave);
 
         if (sc->get_cmd_line) {
             cmd_line = sc->get_cmd_line(slave);
@@ -82,7 +83,7 @@ void sdbus_set_voltage(SDBus *sdbus, uint16_t millivolts)
 
     trace_sdbus_set_voltage(sdbus_name(sdbus), millivolts);
     if (card) {
-        SDCardClass *sc = SD_CARD_GET_CLASS(card);
+        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
 
         assert(sc->set_voltage);
         sc->set_voltage(card, millivolts);
@@ -95,7 +96,7 @@ int sdbus_do_command(SDBus *sdbus, SDRequest *req, uint8_t *response)
 
     trace_sdbus_command(sdbus_name(sdbus), req->cmd, req->arg);
     if (card) {
-        SDCardClass *sc = SD_CARD_GET_CLASS(card);
+        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
 
         return sc->do_command(card, req, response);
     }
@@ -109,7 +110,7 @@ void sdbus_write_byte(SDBus *sdbus, uint8_t value)
 
     trace_sdbus_write(sdbus_name(sdbus), value);
     if (card) {
-        SDCardClass *sc = SD_CARD_GET_CLASS(card);
+        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
 
         sc->write_byte(card, value);
     }
@@ -121,7 +122,7 @@ void sdbus_write_data(SDBus *sdbus, const void *buf, size_t length)
     const uint8_t *data = buf;
 
     if (card) {
-        SDCardClass *sc = SD_CARD_GET_CLASS(card);
+        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
 
         for (size_t i = 0; i < length; i++) {
             trace_sdbus_write(sdbus_name(sdbus), data[i]);
@@ -136,7 +137,7 @@ uint8_t sdbus_read_byte(SDBus *sdbus)
     uint8_t value = 0;
 
     if (card) {
-        SDCardClass *sc = SD_CARD_GET_CLASS(card);
+        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
 
         value = sc->read_byte(card);
     }
@@ -151,7 +152,7 @@ void sdbus_read_data(SDBus *sdbus, void *buf, size_t length)
     uint8_t *data = buf;
 
     if (card) {
-        SDCardClass *sc = SD_CARD_GET_CLASS(card);
+        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
 
         for (size_t i = 0; i < length; i++) {
             data[i] = sc->read_byte(card);
@@ -165,7 +166,7 @@ bool sdbus_receive_ready(SDBus *sdbus)
     SDState *card = get_card(sdbus);
 
     if (card) {
-        SDCardClass *sc = SD_CARD_GET_CLASS(card);
+        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
 
         return sc->receive_ready(card);
     }
@@ -178,7 +179,7 @@ bool sdbus_data_ready(SDBus *sdbus)
     SDState *card = get_card(sdbus);
 
     if (card) {
-        SDCardClass *sc = SD_CARD_GET_CLASS(card);
+        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
 
         return sc->data_ready(card);
     }
@@ -191,7 +192,7 @@ bool sdbus_get_inserted(SDBus *sdbus)
     SDState *card = get_card(sdbus);
 
     if (card) {
-        SDCardClass *sc = SD_CARD_GET_CLASS(card);
+        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
 
         return sc->get_inserted(card);
     }
@@ -204,7 +205,7 @@ bool sdbus_get_readonly(SDBus *sdbus)
     SDState *card = get_card(sdbus);
 
     if (card) {
-        SDCardClass *sc = SD_CARD_GET_CLASS(card);
+        SDCardClass *sc = SDMMC_COMMON_GET_CLASS(card);
 
         return sc->get_readonly(card);
     }
@@ -250,7 +251,7 @@ void sdbus_reparent_card(SDBus *from, SDBus *to)
         return;
     }
 
-    sc = SD_CARD_GET_CLASS(card);
+    sc = SDMMC_COMMON_GET_CLASS(card);
     readonly = sc->get_readonly(card);
 
     sdbus_set_inserted(from, false);
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 058a70a0f8..d6a07f0ade 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -679,8 +679,8 @@ static inline uint64_t sd_addr_to_wpnum(uint64_t addr)
 
 static void sd_reset(DeviceState *dev)
 {
-    SDState *sd = SD_CARD(dev);
-    SDCardClass *sc = SD_CARD_GET_CLASS(sd);
+    SDState *sd = SDMMC_COMMON(dev);
+    SDCardClass *sc = SDMMC_COMMON_GET_CLASS(sd);
     uint64_t size;
     uint64_t sect;
 
@@ -2377,8 +2377,8 @@ static const SDProto sd_proto_sd = {
 
 static void sd_instance_init(Object *obj)
 {
-    SDState *sd = SD_CARD(obj);
-    SDCardClass *sc = SD_CARD_GET_CLASS(sd);
+    SDState *sd = SDMMC_COMMON(obj);
+    SDCardClass *sc = SDMMC_COMMON_GET_CLASS(sd);
 
     sd->proto = sc->proto;
     sd->last_cmd_name = "UNSET";
@@ -2388,14 +2388,14 @@ static void sd_instance_init(Object *obj)
 
 static void sd_instance_finalize(Object *obj)
 {
-    SDState *sd = SD_CARD(obj);
+    SDState *sd = SDMMC_COMMON(obj);
 
     timer_free(sd->ocr_power_timer);
 }
 
 static void sd_realize(DeviceState *dev, Error **errp)
 {
-    SDState *sd = SD_CARD(dev);
+    SDState *sd = SDMMC_COMMON(dev);
     int ret;
 
     switch (sd->spec_version) {
@@ -2446,20 +2446,23 @@ static void sd_realize(DeviceState *dev, Error **errp)
     }
 }
 
-static Property sd_properties[] = {
-    DEFINE_PROP_UINT8("spec_version", SDState,
-                      spec_version, SD_PHY_SPECv3_01_VERS),
+static Property sdmmc_common_properties[] = {
     DEFINE_PROP_DRIVE("drive", SDState, blk),
     DEFINE_PROP_END_OF_LIST()
 };
 
-static void sd_class_init(ObjectClass *klass, void *data)
+static Property sd_properties[] = {
+    DEFINE_PROP_UINT8("spec_version", SDState,
+                      spec_version, SD_PHY_SPECv3_01_VERS),
+    DEFINE_PROP_END_OF_LIST()
+};
+
+static void sdmmc_common_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SDCardClass *sc = SD_CARD_CLASS(klass);
+    SDCardClass *sc = SDMMC_COMMON_CLASS(klass);
 
-    dc->realize = sd_realize;
-    device_class_set_props(dc, sd_properties);
+    device_class_set_props(dc, sdmmc_common_properties);
     dc->vmsd = &sd_vmstate;
     dc->reset = sd_reset;
     dc->bus_type = TYPE_SD_BUS;
@@ -2476,6 +2479,16 @@ static void sd_class_init(ObjectClass *klass, void *data)
     sc->enable = sd_enable;
     sc->get_inserted = sd_get_inserted;
     sc->get_readonly = sd_get_readonly;
+}
+
+static void sd_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    SDCardClass *sc = SDMMC_COMMON_CLASS(klass);
+
+    dc->realize = sd_realize;
+    device_class_set_props(dc, sd_properties);
+
     sc->set_cid = sd_set_cid;
     sc->set_csd = sd_set_csd;
     sc->proto = &sd_proto_sd;
@@ -2490,7 +2503,7 @@ static void sd_class_init(ObjectClass *klass, void *data)
 static void sd_spi_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
-    SDCardClass *sc = SD_CARD_CLASS(klass);
+    SDCardClass *sc = SDMMC_COMMON_CLASS(klass);
 
     dc->desc = "SD SPI";
     sc->proto = &sd_proto_spi;
@@ -2498,14 +2511,20 @@ static void sd_spi_class_init(ObjectClass *klass, void *data)
 
 static const TypeInfo sd_types[] = {
     {
-        .name           = TYPE_SD_CARD,
+        .name           = TYPE_SDMMC_COMMON,
         .parent         = TYPE_DEVICE,
+        .abstract       = true,
         .instance_size  = sizeof(SDState),
         .class_size     = sizeof(SDCardClass),
-        .class_init     = sd_class_init,
+        .class_init     = sdmmc_common_class_init,
         .instance_init  = sd_instance_init,
         .instance_finalize = sd_instance_finalize,
     },
+    {
+        .name           = TYPE_SD_CARD,
+        .parent         = TYPE_SDMMC_COMMON,
+        .class_init     = sd_class_init,
+    },
     {
         .name           = TYPE_SD_CARD_SPI,
         .parent         = TYPE_SD_CARD,
-- 
2.41.0

Re: [PULL 00/16] SD/MMC patches for 2024-07-06

[Richard Henderson]

On 7/5/24 15:04, Philippe Mathieu-Daudé wrote:
> The following changes since commit f2cb4026fccfe073f84a4b440e41d3ed0c3134f6:
> 
>    Merge tag 'pull-maintainer-july24-050724-1' ofhttps://gitlab.com/stsquad/qemu  into staging (2024-07-05 09:15:48 -0700)
> 
> are available in the Git repository at:
> 
>    https://github.com/philmd/qemu.git  tags/sdmmc-20240706
> 
> for you to fetch changes up to 0bcea3f74b04cdc23ecd6822bea7e46a55eb4be7:
> 
>    hw/sd/sdcard: Extract TYPE_SDMMC_COMMON from TYPE_SD_CARD (2024-07-05 23:55:02 +0200)
> 
> ----------------------------------------------------------------
> SD/MMC patches queue
> 
> - Use published card address (RCA) in qtest/npcm7xx_sdhci
> - Have cards use random RCA
> - Use SD spec v3.01 by default
> - Convert GEN_CMD to sd_generic_read/write_byte style
> - Extract SDMMC_COMMON abstract QDev parent from SD_CARD
> - Few housekeeping

Applied, thanks.  Please update https://wiki.qemu.org/ChangeLog/9.1 as appropriate.

r~

Re: [PULL 08/16] hw/sd/sdcard: Add sd_cmd_GEN_CMD handler (CMD56)

[Peter Maydell]

On Fri, 5 Jul 2024 at 23:06, Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
>
> "General command" (GEN_CMD, CMD56) is described as:
>
>   GEN_CMD is the same as the single block read or write
>   commands (CMD24 or CMD17). The difference is that [...]
>   the data block is not a memory payload data but has a
>   vendor specific format and meaning.
>
> Thus this block must not be stored overwriting data block
> on underlying storage drive. Handle as RAZ/WI.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Tested-by: Andrew Jeffery <andrew@codeconstruct.com.au>
> Tested-by: Cédric Le Goater <clg@redhat.com>
> Message-Id: <20240703134356.85972-3-philmd@linaro.org>


> @@ -2187,11 +2183,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
>          break;
>
>      case 56:  /* CMD56:  GEN_CMD */
> -        sd->data[sd->data_offset ++] = value;
> -        if (sd->data_offset >= sd->blk_len) {
> -            APP_WRITE_BLOCK(sd->data_start, sd->data_offset);
> -            sd->state = sd_transfer_state;
> -        }
> +        sd_generic_write_byte(sd, value);
>          break;
>
>      default:

Hi; Coverity notes that we almost always check the return
value from sd_generic_write_byte(), but that we don't do
that in this new callsite. Is there something we should do
if it returns false here, or should we just mark the
issue in coverity as a false positive? (CID 1550397)

thanks
-- PMM