[RFC PATCH 00/17] hw/sd: Rework models for eMMC support

[RFC PATCH 00/17] hw/sd: Rework models for eMMC support

[Cédric Le Goater]

Hello Philippe,

I am restarting the discussion we started in : 

  http://patchwork.ozlabs.org/project/qemu-devel/list/?series=250563

This series adds an extension for a new eMMC device using the
framework you put in place. It's not perfect but we are getting close.
The SPI variant would need its own class I suppose and there are more
cleanups to be done in the set of commands. Please comment !

Thanks,

C.

Cédric Le Goater (2):
  hw/sd: Add eMMC support
  hw/sd: Fix SET_BLOCK_COUNT command argument

Joel Stanley (3):
  hw/sd: Add sd_cmd_SEND_TUNING_BLOCK() handler
  hw/sd: Support boot area in emmc image
  hw/sd: Subtract bootarea size from blk

Philippe Mathieu-Daudé (9):
  hw/sd: When card is in wrong state, log which state it is
  hw/sd: Move proto_name to SDProto structure
  hw/sd: Introduce sd_cmd_handler type
  hw/sd: Add sd_cmd_illegal() handler
  hw/sd: Add sd_cmd_unimplemented() handler
  hw/sd: Add sd_cmd_GO_IDLE_STATE() handler
  hw/sd: Add sd_cmd_SEND_OP_CMD() handler
  hw/sd: Add sd_cmd_ALL_SEND_CID() handler
  hw/sd: Add sd_cmd_SEND_RELATIVE_ADDR() handler

Sai Pavan Boddu (3):
  hw/sd: Update CMD1 definition for MMC
  hw/sd: Add CMD21 tuning sequence
  hw/sd: Add mmc switch function support

 hw/sd/sdmmc-internal.h |  97 +++++++
 include/hw/sd/sd.h     |  10 +
 hw/sd/sd.c             | 585 +++++++++++++++++++++++++++++++++--------
 hw/sd/sdmmc-internal.c |   2 +-
 4 files changed, 583 insertions(+), 111 deletions(-)

-- 
2.34.1

[RFC PATCH 01/17] hw/sd: When card is in wrong state, log which state it is

[Cédric Le Goater]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

We report the card is in an inconsistent state, but don't precise
in which state it is. Add this information, as it is useful when
debugging problems.

Since we will reuse this code, extract as sd_invalid_state_for_cmd()
helper.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Message-Id: <20210624142209.1193073-2-f4bug@amsat.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index cd67a7bac8e2..e7fbb937c277 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -967,6 +967,14 @@ static bool address_in_range(SDState *sd, const char *desc,
     return true;
 }
 
+static sd_rsp_type_t sd_invalid_state_for_cmd(SDState *sd, SDRequest req)
+{
+    qemu_log_mask(LOG_GUEST_ERROR, "SD: CMD%i in a wrong state: %s\n",
+                  req.cmd, sd_state_name(sd->state));
+
+    return sd_illegal;
+}
+
 static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 {
     uint32_t rca = 0x0000;
@@ -1535,9 +1543,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         return sd_illegal;
     }
 
-    qemu_log_mask(LOG_GUEST_ERROR, "SD: CMD%i in a wrong state: %s\n",
-                  req.cmd, sd_state_name(sd->state));
-    return sd_illegal;
+    return sd_invalid_state_for_cmd(sd, req);
 }
 
 static sd_rsp_type_t sd_app_command(SDState *sd,
-- 
2.34.1

[RFC PATCH 02/17] hw/sd: Move proto_name to SDProto structure

[Cédric Le Goater]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Introduce a new structure to hold the bus protocol specific
fields: SDProto. The first field is the protocol name.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Message-Id: <20210624142209.1193073-4-f4bug@amsat.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index e7fbb937c277..45aa6a77de5e 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -88,6 +88,10 @@ enum SDCardStates {
     sd_disconnect_state,
 };
 
+typedef struct SDProto {
+    const char *name;
+} SDProto;
+
 struct SDState {
     DeviceState parent_obj;
 
@@ -112,6 +116,7 @@ struct SDState {
 
     /* Runtime changeables */
 
+    const SDProto *proto;   /* Bus protocol */
     uint32_t mode;    /* current card mode, one of SDCardModes */
     int32_t state;    /* current card state, one of SDCardStates */
     uint32_t vhs;
@@ -138,7 +143,6 @@ struct SDState {
     qemu_irq readonly_cb;
     qemu_irq inserted_cb;
     QEMUTimer *ocr_power_timer;
-    const char *proto_name;
     bool enable;
     uint8_t dat_lines;
     bool cmd_line;
@@ -969,8 +973,8 @@ static bool address_in_range(SDState *sd, const char *desc,
 
 static sd_rsp_type_t sd_invalid_state_for_cmd(SDState *sd, SDRequest req)
 {
-    qemu_log_mask(LOG_GUEST_ERROR, "SD: CMD%i in a wrong state: %s\n",
-                  req.cmd, sd_state_name(sd->state));
+    qemu_log_mask(LOG_GUEST_ERROR, "%s: CMD%i in a wrong state: %s\n",
+                  sd->proto->name, req.cmd, sd_state_name(sd->state));
 
     return sd_illegal;
 }
@@ -984,7 +988,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
      * However there is no ACMD55, so we want to trace this particular case.
      */
     if (req.cmd != 55 || sd->expecting_acmd) {
-        trace_sdcard_normal_command(sd->proto_name,
+        trace_sdcard_normal_command(sd->proto->name,
                                     sd_cmd_name(req.cmd), req.cmd,
                                     req.arg, sd_state_name(sd->state));
     }
@@ -1549,7 +1553,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 static sd_rsp_type_t sd_app_command(SDState *sd,
                                     SDRequest req)
 {
-    trace_sdcard_app_command(sd->proto_name, sd_acmd_name(req.cmd),
+    trace_sdcard_app_command(sd->proto->name, sd_acmd_name(req.cmd),
                              req.cmd, req.arg, sd_state_name(sd->state));
     sd->card_status |= APP_CMD;
     switch (req.cmd) {
@@ -1843,7 +1847,7 @@ void sd_write_byte(SDState *sd, uint8_t value)
     if (sd->card_status & (ADDRESS_ERROR | WP_VIOLATION))
         return;
 
-    trace_sdcard_write_data(sd->proto_name,
+    trace_sdcard_write_data(sd->proto->name,
                             sd_acmd_name(sd->current_cmd),
                             sd->current_cmd, value);
     switch (sd->current_cmd) {
@@ -1999,7 +2003,7 @@ uint8_t sd_read_byte(SDState *sd)
 
     io_len = (sd->ocr & (1 << 30)) ? 512 : sd->blk_len;
 
-    trace_sdcard_read_data(sd->proto_name,
+    trace_sdcard_read_data(sd->proto->name,
                            sd_acmd_name(sd->current_cmd),
                            sd->current_cmd, io_len);
     switch (sd->current_cmd) {
@@ -2118,6 +2122,14 @@ void sd_enable(SDState *sd, bool enable)
     sd->enable = enable;
 }
 
+static const SDProto sd_proto_spi = {
+    .name = "SPI",
+};
+
+static const SDProto sd_proto_sd = {
+    .name = "SD",
+};
+
 static void sd_instance_init(Object *obj)
 {
     SDState *sd = SD_CARD(obj);
@@ -2138,7 +2150,7 @@ static void sd_realize(DeviceState *dev, Error **errp)
     SDState *sd = SD_CARD(dev);
     int ret;
 
-    sd->proto_name = sd->spi ? "SPI" : "SD";
+    sd->proto = sd->spi ? &sd_proto_spi : &sd_proto_sd;
 
     switch (sd->spec_version) {
     case SD_PHY_SPECv1_10_VERS
-- 
2.34.1

[RFC PATCH 03/17] hw/sd: Introduce sd_cmd_handler type

[Cédric Le Goater]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Add 2 command handler arrays in SDProto, for CMD and ACMD.
Have sd_normal_command() / sd_app_command() use these arrays:
if an command handler is registered, call it, otherwise fall
back to current code base.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Message-Id: <20210624142209.1193073-5-f4bug@amsat.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 45aa6a77de5e..a7c51913df82 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -88,8 +88,12 @@ enum SDCardStates {
     sd_disconnect_state,
 };
 
+typedef sd_rsp_type_t (*sd_cmd_handler)(SDState *sd, SDRequest req);
+
 typedef struct SDProto {
     const char *name;
+    sd_cmd_handler cmd[SDMMC_CMD_MAX];
+    sd_cmd_handler acmd[SDMMC_CMD_MAX];
 } SDProto;
 
 struct SDState {
@@ -1012,6 +1016,10 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         return sd_illegal;
     }
 
+    if (sd->proto->cmd[req.cmd]) {
+        return sd->proto->cmd[req.cmd](sd, req);
+    }
+
     switch (req.cmd) {
     /* Basic commands (Class 0 and Class 1) */
     case 0:	/* CMD0:   GO_IDLE_STATE */
@@ -1556,6 +1564,11 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
     trace_sdcard_app_command(sd->proto->name, sd_acmd_name(req.cmd),
                              req.cmd, req.arg, sd_state_name(sd->state));
     sd->card_status |= APP_CMD;
+
+    if (sd->proto->acmd[req.cmd]) {
+        return sd->proto->acmd[req.cmd](sd, req);
+    }
+
     switch (req.cmd) {
     case 6:	/* ACMD6:  SET_BUS_WIDTH */
         if (sd->spi) {
-- 
2.34.1

[RFC PATCH 04/17] hw/sd: Add sd_cmd_illegal() handler

[Cédric Le Goater]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Log illegal commands as GUEST_ERROR.

Note: we are logging back the SDIO commands (CMD5, CMD52-54).

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Message-Id: <20210624142209.1193073-6-f4bug@amsat.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 57 ++++++++++++++++++++++--------------------------------
 1 file changed, 23 insertions(+), 34 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index a7c51913df82..6c677de4aadb 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -983,6 +983,14 @@ static sd_rsp_type_t sd_invalid_state_for_cmd(SDState *sd, SDRequest req)
     return sd_illegal;
 }
 
+static sd_rsp_type_t sd_cmd_illegal(SDState *sd, SDRequest req)
+{
+    qemu_log_mask(LOG_GUEST_ERROR, "%s: Unknown CMD%i\n",
+                  sd->proto->name, req.cmd);
+
+    return sd_illegal;
+}
+
 static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 {
     uint32_t rca = 0x0000;
@@ -1035,15 +1043,10 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         break;
 
     case 1:	/* CMD1:   SEND_OP_CMD */
-        if (!sd->spi)
-            goto bad_cmd;
-
         sd->state = sd_transfer_state;
         return sd_r1;
 
     case 2:	/* CMD2:   ALL_SEND_CID */
-        if (sd->spi)
-            goto bad_cmd;
         switch (sd->state) {
         case sd_ready_state:
             sd->state = sd_identification_state;
@@ -1055,8 +1058,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         break;
 
     case 3:	/* CMD3:   SEND_RELATIVE_ADDR */
-        if (sd->spi)
-            goto bad_cmd;
         switch (sd->state) {
         case sd_identification_state:
         case sd_standby_state:
@@ -1070,8 +1071,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         break;
 
     case 4:	/* CMD4:   SEND_DSR */
-        if (sd->spi)
-            goto bad_cmd;
         switch (sd->state) {
         case sd_standby_state:
             break;
@@ -1081,9 +1080,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
-    case 5: /* CMD5: reserved for SDIO cards */
-        return sd_illegal;
-
     case 6:	/* CMD6:   SWITCH_FUNCTION */
         switch (sd->mode) {
         case sd_data_transfer_mode:
@@ -1099,8 +1095,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         break;
 
     case 7:	/* CMD7:   SELECT/DESELECT_CARD */
-        if (sd->spi)
-            goto bad_cmd;
         switch (sd->state) {
         case sd_standby_state:
             if (sd->rca != rca)
@@ -1230,8 +1224,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         break;
 
     case 15:	/* CMD15:  GO_INACTIVE_STATE */
-        if (sd->spi)
-            goto bad_cmd;
         switch (sd->mode) {
         case sd_data_transfer_mode:
             if (sd->rca != rca)
@@ -1338,8 +1330,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         break;
 
     case 26:	/* CMD26:  PROGRAM_CID */
-        if (sd->spi)
-            goto bad_cmd;
         switch (sd->state) {
         case sd_transfer_state:
             sd->state = sd_receivingdata_state;
@@ -1489,15 +1479,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
-    case 52 ... 54:
-        /* CMD52, CMD53, CMD54: reserved for SDIO cards
-         * (see the SDIO Simplified Specification V2.0)
-         * Handle as illegal command but do not complain
-         * on stderr, as some OSes may use these in their
-         * probing for presence of an SDIO card.
-         */
-        return sd_illegal;
-
     /* Application specific commands (Class 8) */
     case 55:	/* CMD55:  APP_CMD */
         switch (sd->state) {
@@ -1538,19 +1519,12 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         break;
 
     case 58:    /* CMD58:   READ_OCR (SPI) */
-        if (!sd->spi) {
-            goto bad_cmd;
-        }
         return sd_r3;
 
     case 59:    /* CMD59:   CRC_ON_OFF (SPI) */
-        if (!sd->spi) {
-            goto bad_cmd;
-        }
         return sd_r1;
 
     default:
-    bad_cmd:
         qemu_log_mask(LOG_GUEST_ERROR, "SD: Unknown CMD%i\n", req.cmd);
         return sd_illegal;
     }
@@ -2137,10 +2111,25 @@ void sd_enable(SDState *sd, bool enable)
 
 static const SDProto sd_proto_spi = {
     .name = "SPI",
+    .cmd = {
+        [2 ... 4]   = sd_cmd_illegal,
+        [5]         = sd_cmd_illegal,
+        [7]         = sd_cmd_illegal,
+        [15]        = sd_cmd_illegal,
+        [26]        = sd_cmd_illegal,
+        [52 ... 54] = sd_cmd_illegal,
+    },
 };
 
 static const SDProto sd_proto_sd = {
     .name = "SD",
+    .cmd = {
+        [1]         = sd_cmd_illegal,
+        [5]         = sd_cmd_illegal,
+        [52 ... 54] = sd_cmd_illegal,
+        [58]        = sd_cmd_illegal,
+        [59]        = sd_cmd_illegal,
+    },
 };
 
 static void sd_instance_init(Object *obj)
-- 
2.34.1

[RFC PATCH 05/17] hw/sd: Add sd_cmd_unimplemented() handler

[Cédric Le Goater]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20210624142209.1193073-7-f4bug@amsat.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 6c677de4aadb..fd32aeb7fe82 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -991,6 +991,15 @@ static sd_rsp_type_t sd_cmd_illegal(SDState *sd, SDRequest req)
     return sd_illegal;
 }
 
+/* Commands that are recognised but not yet implemented. */
+static sd_rsp_type_t sd_cmd_unimplemented(SDState *sd, SDRequest req)
+{
+    qemu_log_mask(LOG_UNIMP, "%s: CMD%i not implemented\n",
+                  sd->proto->name, req.cmd);
+
+    return sd_illegal;
+}
+
 static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 {
     uint32_t rca = 0x0000;
@@ -1545,9 +1554,6 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
 
     switch (req.cmd) {
     case 6:	/* ACMD6:  SET_BUS_WIDTH */
-        if (sd->spi) {
-            goto unimplemented_spi_cmd;
-        }
         switch (sd->state) {
         case sd_transfer_state:
             sd->sd_status[0] &= 0x3f;
@@ -1678,12 +1684,6 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
     default:
         /* Fall back to standard commands.  */
         return sd_normal_command(sd, req);
-
-    unimplemented_spi_cmd:
-        /* Commands that are recognised but not yet implemented in SPI mode.  */
-        qemu_log_mask(LOG_UNIMP, "SD: CMD%i not implemented in SPI mode\n",
-                      req.cmd);
-        return sd_illegal;
     }
 
     qemu_log_mask(LOG_GUEST_ERROR, "SD: ACMD%i in a wrong state\n", req.cmd);
@@ -2119,6 +2119,9 @@ static const SDProto sd_proto_spi = {
         [26]        = sd_cmd_illegal,
         [52 ... 54] = sd_cmd_illegal,
     },
+    .cmd = {
+        [6]         = sd_cmd_unimplemented,
+    },
 };
 
 static const SDProto sd_proto_sd = {
-- 
2.34.1

[RFC PATCH 06/17] hw/sd: Add sd_cmd_GO_IDLE_STATE() handler

[Cédric Le Goater]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Message-Id: <20210624142209.1193073-8-f4bug@amsat.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index fd32aeb7fe82..30429e8954d3 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1000,6 +1000,16 @@ static sd_rsp_type_t sd_cmd_unimplemented(SDState *sd, SDRequest req)
     return sd_illegal;
 }
 
+static sd_rsp_type_t sd_cmd_GO_IDLE_STATE(SDState *sd, SDRequest req)
+{
+    if (sd->state != sd_inactive_state) {
+        sd->state = sd_idle_state;
+        sd_reset(DEVICE(sd));
+    }
+
+    return sd->spi ? sd_r1 : sd_r0;
+}
+
 static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 {
     uint32_t rca = 0x0000;
@@ -1039,18 +1049,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 
     switch (req.cmd) {
     /* Basic commands (Class 0 and Class 1) */
-    case 0:	/* CMD0:   GO_IDLE_STATE */
-        switch (sd->state) {
-        case sd_inactive_state:
-            return sd->spi ? sd_r1 : sd_r0;
-
-        default:
-            sd->state = sd_idle_state;
-            sd_reset(DEVICE(sd));
-            return sd->spi ? sd_r1 : sd_r0;
-        }
-        break;
-
     case 1:	/* CMD1:   SEND_OP_CMD */
         sd->state = sd_transfer_state;
         return sd_r1;
@@ -2112,6 +2110,7 @@ void sd_enable(SDState *sd, bool enable)
 static const SDProto sd_proto_spi = {
     .name = "SPI",
     .cmd = {
+        [0]         = sd_cmd_GO_IDLE_STATE,
         [2 ... 4]   = sd_cmd_illegal,
         [5]         = sd_cmd_illegal,
         [7]         = sd_cmd_illegal,
@@ -2127,6 +2126,7 @@ static const SDProto sd_proto_spi = {
 static const SDProto sd_proto_sd = {
     .name = "SD",
     .cmd = {
+        [0]         = sd_cmd_GO_IDLE_STATE,
         [1]         = sd_cmd_illegal,
         [5]         = sd_cmd_illegal,
         [52 ... 54] = sd_cmd_illegal,
-- 
2.34.1

[RFC PATCH 07/17] hw/sd: Add sd_cmd_SEND_OP_CMD() handler

[Cédric Le Goater]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20210624142209.1193073-9-f4bug@amsat.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 30429e8954d3..a6ce54433388 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1010,6 +1010,13 @@ static sd_rsp_type_t sd_cmd_GO_IDLE_STATE(SDState *sd, SDRequest req)
     return sd->spi ? sd_r1 : sd_r0;
 }
 
+static sd_rsp_type_t sd_cmd_SEND_OP_CMD(SDState *sd, SDRequest req)
+{
+    sd->state = sd_transfer_state;
+
+    return sd_r1;
+}
+
 static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 {
     uint32_t rca = 0x0000;
@@ -1049,10 +1056,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 
     switch (req.cmd) {
     /* Basic commands (Class 0 and Class 1) */
-    case 1:	/* CMD1:   SEND_OP_CMD */
-        sd->state = sd_transfer_state;
-        return sd_r1;
-
     case 2:	/* CMD2:   ALL_SEND_CID */
         switch (sd->state) {
         case sd_ready_state:
@@ -1602,11 +1605,6 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
         break;
 
     case 41:	/* ACMD41: SD_APP_OP_COND */
-        if (sd->spi) {
-            /* SEND_OP_CMD */
-            sd->state = sd_transfer_state;
-            return sd_r1;
-        }
         if (sd->state != sd_idle_state) {
             break;
         }
@@ -2111,6 +2109,7 @@ static const SDProto sd_proto_spi = {
     .name = "SPI",
     .cmd = {
         [0]         = sd_cmd_GO_IDLE_STATE,
+        [1]         = sd_cmd_SEND_OP_CMD,
         [2 ... 4]   = sd_cmd_illegal,
         [5]         = sd_cmd_illegal,
         [7]         = sd_cmd_illegal,
@@ -2120,6 +2119,7 @@ static const SDProto sd_proto_spi = {
     },
     .cmd = {
         [6]         = sd_cmd_unimplemented,
+        [41]        = sd_cmd_SEND_OP_CMD,
     },
 };
 
-- 
2.34.1

Re: [RFC PATCH 07/17] hw/sd: Add sd_cmd_SEND_OP_CMD() handler

[Philippe Mathieu-Daudé via]

On 18/3/22 14:28, Cédric Le Goater wrote:
> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> Message-Id: <20210624142209.1193073-9-f4bug@amsat.org>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   hw/sd/sd.c | 18 +++++++++---------
>   1 file changed, 9 insertions(+), 9 deletions(-)

> @@ -2111,6 +2109,7 @@ static const SDProto sd_proto_spi = {
>       .name = "SPI",
>       .cmd = {
>           [0]         = sd_cmd_GO_IDLE_STATE,
> +        [1]         = sd_cmd_SEND_OP_CMD,
>           [2 ... 4]   = sd_cmd_illegal,
>           [5]         = sd_cmd_illegal,
>           [7]         = sd_cmd_illegal,
> @@ -2120,6 +2119,7 @@ static const SDProto sd_proto_spi = {
>       },
>       .cmd = {
>           [6]         = sd_cmd_unimplemented,
> +        [41]        = sd_cmd_SEND_OP_CMD,
>       },
>   };
>   

I missed adding the cmd_abbrev[1] entry.

Re: [RFC PATCH 07/17] hw/sd: Add sd_cmd_SEND_OP_CMD() handler

[Cédric Le Goater]

On 5/9/22 23:12, Philippe Mathieu-Daudé wrote:
> On 18/3/22 14:28, Cédric Le Goater wrote:
>> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> Message-Id: <20210624142209.1193073-9-f4bug@amsat.org>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>   hw/sd/sd.c | 18 +++++++++---------
>>   1 file changed, 9 insertions(+), 9 deletions(-)
> 
>> @@ -2111,6 +2109,7 @@ static const SDProto sd_proto_spi = {
>>       .name = "SPI",
>>       .cmd = {
>>           [0]         = sd_cmd_GO_IDLE_STATE,
>> +        [1]         = sd_cmd_SEND_OP_CMD,
>>           [2 ... 4]   = sd_cmd_illegal,
>>           [5]         = sd_cmd_illegal,
>>           [7]         = sd_cmd_illegal,
>> @@ -2120,6 +2119,7 @@ static const SDProto sd_proto_spi = {
>>       },
>>       .cmd = {
>>           [6]         = sd_cmd_unimplemented,
>> +        [41]        = sd_cmd_SEND_OP_CMD,
>>       },
>>   };
> 
> I missed adding the cmd_abbrev[1] entry.

Will you resend ?

Thanks,

C.

Re: [RFC PATCH 07/17] hw/sd: Add sd_cmd_SEND_OP_CMD() handler

[Philippe Mathieu-Daudé via]

On 10/5/22 08:57, Cédric Le Goater wrote:
> On 5/9/22 23:12, Philippe Mathieu-Daudé wrote:
>> On 18/3/22 14:28, Cédric Le Goater wrote:
>>> From: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> Message-Id: <20210624142209.1193073-9-f4bug@amsat.org>
>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>> ---
>>>   hw/sd/sd.c | 18 +++++++++---------
>>>   1 file changed, 9 insertions(+), 9 deletions(-)
>>
>>> @@ -2111,6 +2109,7 @@ static const SDProto sd_proto_spi = {
>>>       .name = "SPI",
>>>       .cmd = {
>>>           [0]         = sd_cmd_GO_IDLE_STATE,
>>> +        [1]         = sd_cmd_SEND_OP_CMD,
>>>           [2 ... 4]   = sd_cmd_illegal,
>>>           [5]         = sd_cmd_illegal,
>>>           [7]         = sd_cmd_illegal,
>>> @@ -2120,6 +2119,7 @@ static const SDProto sd_proto_spi = {
>>>       },
>>>       .cmd = {
>>>           [6]         = sd_cmd_unimplemented,
>>> +        [41]        = sd_cmd_SEND_OP_CMD,
>>>       },
>>>   };
>>
>> I missed adding the cmd_abbrev[1] entry.
> 
> Will you resend ?

Yes.

[RFC PATCH 08/17] hw/sd: Add sd_cmd_ALL_SEND_CID() handler

[Cédric Le Goater]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Message-Id: <20210624142209.1193073-10-f4bug@amsat.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index a6ce54433388..5ee71d75a6d7 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1017,6 +1017,17 @@ static sd_rsp_type_t sd_cmd_SEND_OP_CMD(SDState *sd, SDRequest req)
     return sd_r1;
 }
 
+static sd_rsp_type_t sd_cmd_ALL_SEND_CID(SDState *sd, SDRequest req)
+{
+    if (sd->state != sd_ready_state) {
+        return sd_invalid_state_for_cmd(sd, req);
+    }
+
+    sd->state = sd_identification_state;
+
+    return sd_r2_i;
+}
+
 static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 {
     uint32_t rca = 0x0000;
@@ -1056,17 +1067,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 
     switch (req.cmd) {
     /* Basic commands (Class 0 and Class 1) */
-    case 2:	/* CMD2:   ALL_SEND_CID */
-        switch (sd->state) {
-        case sd_ready_state:
-            sd->state = sd_identification_state;
-            return sd_r2_i;
-
-        default:
-            break;
-        }
-        break;
-
     case 3:	/* CMD3:   SEND_RELATIVE_ADDR */
         switch (sd->state) {
         case sd_identification_state:
@@ -2128,6 +2128,7 @@ static const SDProto sd_proto_sd = {
     .cmd = {
         [0]         = sd_cmd_GO_IDLE_STATE,
         [1]         = sd_cmd_illegal,
+        [2]         = sd_cmd_ALL_SEND_CID,
         [5]         = sd_cmd_illegal,
         [52 ... 54] = sd_cmd_illegal,
         [58]        = sd_cmd_illegal,
-- 
2.34.1

[RFC PATCH 09/17] hw/sd: Add sd_cmd_SEND_RELATIVE_ADDR() handler

[Cédric Le Goater]

From: Philippe Mathieu-Daudé <f4bug@amsat.org>

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Message-Id: <20210624142209.1193073-11-f4bug@amsat.org>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 5ee71d75a6d7..25e86c893bba 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1028,6 +1028,20 @@ static sd_rsp_type_t sd_cmd_ALL_SEND_CID(SDState *sd, SDRequest req)
     return sd_r2_i;
 }
 
+static sd_rsp_type_t sd_cmd_SEND_RELATIVE_ADDR(SDState *sd, SDRequest req)
+{
+    switch (sd->state) {
+    case sd_identification_state:
+    case sd_standby_state:
+        sd->state = sd_standby_state;
+        sd_set_rca(sd);
+        return sd_r6;
+
+    default:
+        return sd_invalid_state_for_cmd(sd, req);
+    }
+}
+
 static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 {
     uint32_t rca = 0x0000;
@@ -1067,19 +1081,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 
     switch (req.cmd) {
     /* Basic commands (Class 0 and Class 1) */
-    case 3:	/* CMD3:   SEND_RELATIVE_ADDR */
-        switch (sd->state) {
-        case sd_identification_state:
-        case sd_standby_state:
-            sd->state = sd_standby_state;
-            sd_set_rca(sd);
-            return sd_r6;
-
-        default:
-            break;
-        }
-        break;
-
     case 4:	/* CMD4:   SEND_DSR */
         switch (sd->state) {
         case sd_standby_state:
@@ -2129,6 +2130,7 @@ static const SDProto sd_proto_sd = {
         [0]         = sd_cmd_GO_IDLE_STATE,
         [1]         = sd_cmd_illegal,
         [2]         = sd_cmd_ALL_SEND_CID,
+        [3]         = sd_cmd_SEND_RELATIVE_ADDR,
         [5]         = sd_cmd_illegal,
         [52 ... 54] = sd_cmd_illegal,
         [58]        = sd_cmd_illegal,
-- 
2.34.1

[RFC PATCH 10/17] hw/sd: Add sd_cmd_SEND_TUNING_BLOCK() handler

[Cédric Le Goater]

From: Joel Stanley <joel@jms.id.au>

Signed-off-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 28 +++++++++++++++++-----------
 1 file changed, 17 insertions(+), 11 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 25e86c893bba..602ed6eb0701 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1042,6 +1042,22 @@ static sd_rsp_type_t sd_cmd_SEND_RELATIVE_ADDR(SDState *sd, SDRequest req)
     }
 }
 
+static sd_rsp_type_t sd_cmd_SEND_TUNING_BLOCK(SDState *sd, SDRequest req)
+{
+        if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
+            return sd_invalid_state_for_cmd(sd, req);
+        }
+
+        if (sd->state != sd_transfer_state) {
+            return sd_invalid_state_for_cmd(sd, req);
+        }
+
+        sd->state = sd_sendingdata_state;
+        sd->data_offset = 0;
+
+        return sd_r1;
+}
+
 static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 {
     uint32_t rca = 0x0000;
@@ -1285,17 +1301,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
-    case 19:    /* CMD19: SEND_TUNING_BLOCK (SD) */
-        if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
-            break;
-        }
-        if (sd->state == sd_transfer_state) {
-            sd->state = sd_sendingdata_state;
-            sd->data_offset = 0;
-            return sd_r1;
-        }
-        break;
-
     case 23:    /* CMD23: SET_BLOCK_COUNT */
         if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
             break;
@@ -2132,6 +2137,7 @@ static const SDProto sd_proto_sd = {
         [2]         = sd_cmd_ALL_SEND_CID,
         [3]         = sd_cmd_SEND_RELATIVE_ADDR,
         [5]         = sd_cmd_illegal,
+        [19]        = sd_cmd_SEND_TUNING_BLOCK,
         [52 ... 54] = sd_cmd_illegal,
         [58]        = sd_cmd_illegal,
         [59]        = sd_cmd_illegal,
-- 
2.34.1

Re: [RFC PATCH 10/17] hw/sd: Add sd_cmd_SEND_TUNING_BLOCK() handler

[Philippe Mathieu-Daudé via]

On 18/3/22 14:28, Cédric Le Goater wrote:
> From: Joel Stanley <joel@jms.id.au>
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   hw/sd/sd.c | 28 +++++++++++++++++-----------
>   1 file changed, 17 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 25e86c893bba..602ed6eb0701 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -1042,6 +1042,22 @@ static sd_rsp_type_t sd_cmd_SEND_RELATIVE_ADDR(SDState *sd, SDRequest req)
>       }
>   }
>   
> +static sd_rsp_type_t sd_cmd_SEND_TUNING_BLOCK(SDState *sd, SDRequest req)
> +{
> +        if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
> +            return sd_invalid_state_for_cmd(sd, req);

There was a bug here, this should be:

                return sd_illegal;

(see 
https://lore.kernel.org/qemu-devel/20220509141320.98374-1-philippe.mathieu.daude@gmail.com/. 
I can fix upon applying).

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> +        }
> +
> +        if (sd->state != sd_transfer_state) {
> +            return sd_invalid_state_for_cmd(sd, req);
> +        }
> +
> +        sd->state = sd_sendingdata_state;
> +        sd->data_offset = 0;
> +
> +        return sd_r1;
> +}
> +
>   static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
>   {
>       uint32_t rca = 0x0000;
> @@ -1285,17 +1301,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
>           }
>           break;
>   
> -    case 19:    /* CMD19: SEND_TUNING_BLOCK (SD) */
> -        if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
> -            break;
> -        }
> -        if (sd->state == sd_transfer_state) {
> -            sd->state = sd_sendingdata_state;
> -            sd->data_offset = 0;
> -            return sd_r1;
> -        }
> -        break;
> -
>       case 23:    /* CMD23: SET_BLOCK_COUNT */
>           if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
>               break;
> @@ -2132,6 +2137,7 @@ static const SDProto sd_proto_sd = {
>           [2]         = sd_cmd_ALL_SEND_CID,
>           [3]         = sd_cmd_SEND_RELATIVE_ADDR,
>           [5]         = sd_cmd_illegal,
> +        [19]        = sd_cmd_SEND_TUNING_BLOCK,
>           [52 ... 54] = sd_cmd_illegal,
>           [58]        = sd_cmd_illegal,
>           [59]        = sd_cmd_illegal,

Re: [RFC PATCH 10/17] hw/sd: Add sd_cmd_SEND_TUNING_BLOCK() handler

[Cédric Le Goater]

On 5/9/22 23:05, Philippe Mathieu-Daudé wrote:
> On 18/3/22 14:28, Cédric Le Goater wrote:
>> From: Joel Stanley <joel@jms.id.au>
>>
>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>   hw/sd/sd.c | 28 +++++++++++++++++-----------
>>   1 file changed, 17 insertions(+), 11 deletions(-)
>>
>> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
>> index 25e86c893bba..602ed6eb0701 100644
>> --- a/hw/sd/sd.c
>> +++ b/hw/sd/sd.c
>> @@ -1042,6 +1042,22 @@ static sd_rsp_type_t sd_cmd_SEND_RELATIVE_ADDR(SDState *sd, SDRequest req)
>>       }
>>   }
>> +static sd_rsp_type_t sd_cmd_SEND_TUNING_BLOCK(SDState *sd, SDRequest req)
>> +{
>> +        if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
>> +            return sd_invalid_state_for_cmd(sd, req);
> 
> There was a bug here, this should be:
> 
>                 return sd_illegal;
> 
> (see https://lore.kernel.org/qemu-devel/20220509141320.98374-1-philippe.mathieu.daude@gmail.com/. I can fix upon applying).

I missed that.

> 
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Thanks,

C.

>> +        }
>> +
>> +        if (sd->state != sd_transfer_state) {
>> +            return sd_invalid_state_for_cmd(sd, req);
>> +        }
>> +
>> +        sd->state = sd_sendingdata_state;
>> +        sd->data_offset = 0;
>> +
>> +        return sd_r1;
>> +}
>> +
>>   static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
>>   {
>>       uint32_t rca = 0x0000;
>> @@ -1285,17 +1301,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
>>           }
>>           break;
>> -    case 19:    /* CMD19: SEND_TUNING_BLOCK (SD) */
>> -        if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
>> -            break;
>> -        }
>> -        if (sd->state == sd_transfer_state) {
>> -            sd->state = sd_sendingdata_state;
>> -            sd->data_offset = 0;
>> -            return sd_r1;
>> -        }
>> -        break;
>> -
>>       case 23:    /* CMD23: SET_BLOCK_COUNT */
>>           if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
>>               break;
>> @@ -2132,6 +2137,7 @@ static const SDProto sd_proto_sd = {
>>           [2]         = sd_cmd_ALL_SEND_CID,
>>           [3]         = sd_cmd_SEND_RELATIVE_ADDR,
>>           [5]         = sd_cmd_illegal,
>> +        [19]        = sd_cmd_SEND_TUNING_BLOCK,
>>           [52 ... 54] = sd_cmd_illegal,
>>           [58]        = sd_cmd_illegal,
>>           [59]        = sd_cmd_illegal,
> 

[RFC PATCH 11/17] hw/sd: Add eMMC support

[Cédric Le Goater]

The initial eMMC support from Vincent Palatin was largely reworked to
match the current SD framework. The parameters mimick a real 4GB eMMC,
but it can be set to various sizes.

This adds a new QOM object class for EMMC devices.

Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
Link: https://lore.kernel.org/r/1311635951-11047-5-git-send-email-vpalatin@chromium.org
[ jms: - Forward ported to QEMU 5.2 ]
Signed-off-by: Joel Stanley <joel@jms.id.au>
[ clg: - ported on aspeed-7.0 patchset
       - HPI activation ]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sdmmc-internal.h |  97 +++++++++++++++++++
 include/hw/sd/sd.h     |   9 ++
 hw/sd/sd.c             | 205 ++++++++++++++++++++++++++++++++++++++++-
 hw/sd/sdmmc-internal.c |   2 +-
 4 files changed, 311 insertions(+), 2 deletions(-)

diff --git a/hw/sd/sdmmc-internal.h b/hw/sd/sdmmc-internal.h
index d8bf17d204fc..2b98f117cd8f 100644
--- a/hw/sd/sdmmc-internal.h
+++ b/hw/sd/sdmmc-internal.h
@@ -37,4 +37,101 @@ const char *sd_cmd_name(uint8_t cmd);
  */
 const char *sd_acmd_name(uint8_t cmd);
 
+/*
+ * EXT_CSD fields
+ */
+
+#define EXT_CSD_CMDQ_MODE_EN            15      /* R/W */
+#define EXT_CSD_FLUSH_CACHE             32      /* W */
+#define EXT_CSD_CACHE_CTRL              33      /* R/W */
+#define EXT_CSD_POWER_OFF_NOTIFICATION  34      /* R/W */
+#define EXT_CSD_PACKED_FAILURE_INDEX    35      /* RO */
+#define EXT_CSD_PACKED_CMD_STATUS       36      /* RO */
+#define EXT_CSD_EXP_EVENTS_STATUS       54      /* RO, 2 bytes */
+#define EXT_CSD_EXP_EVENTS_CTRL         56      /* R/W, 2 bytes */
+#define EXT_CSD_DATA_SECTOR_SIZE        61      /* R */
+#define EXT_CSD_GP_SIZE_MULT            143     /* R/W */
+#define EXT_CSD_PARTITION_SETTING_COMPLETED 155 /* R/W */
+#define EXT_CSD_PARTITION_ATTRIBUTE     156     /* R/W */
+#define EXT_CSD_PARTITION_SUPPORT       160     /* RO */
+#define EXT_CSD_HPI_MGMT                161     /* R/W */
+#define EXT_CSD_RST_N_FUNCTION          162     /* R/W */
+#define EXT_CSD_BKOPS_EN                163     /* R/W */
+#define EXT_CSD_BKOPS_START             164     /* W */
+#define EXT_CSD_SANITIZE_START          165     /* W */
+#define EXT_CSD_WR_REL_PARAM            166     /* RO */
+#define EXT_CSD_RPMB_MULT               168     /* RO */
+#define EXT_CSD_FW_CONFIG               169     /* R/W */
+#define EXT_CSD_BOOT_WP                 173     /* R/W */
+#define EXT_CSD_ERASE_GROUP_DEF         175     /* R/W */
+#define EXT_CSD_PART_CONFIG             179     /* R/W */
+#define EXT_CSD_ERASED_MEM_CONT         181     /* RO */
+#define EXT_CSD_BUS_WIDTH               183     /* R/W */
+#define EXT_CSD_STROBE_SUPPORT          184     /* RO */
+#define EXT_CSD_HS_TIMING               185     /* R/W */
+#define EXT_CSD_POWER_CLASS             187     /* R/W */
+#define EXT_CSD_REV                     192     /* RO */
+#define EXT_CSD_STRUCTURE               194     /* RO */
+#define EXT_CSD_CARD_TYPE               196     /* RO */
+#define EXT_CSD_DRIVER_STRENGTH         197     /* RO */
+#define EXT_CSD_OUT_OF_INTERRUPT_TIME   198     /* RO */
+#define EXT_CSD_PART_SWITCH_TIME        199     /* RO */
+#define EXT_CSD_PWR_CL_52_195           200     /* RO */
+#define EXT_CSD_PWR_CL_26_195           201     /* RO */
+#define EXT_CSD_PWR_CL_52_360           202     /* RO */
+#define EXT_CSD_PWR_CL_26_360           203     /* RO */
+#define EXT_CSD_SEC_CNT                 212     /* RO, 4 bytes */
+#define EXT_CSD_S_A_TIMEOUT             217     /* RO */
+#define EXT_CSD_S_C_VCCQ                219     /* RO */
+#define EXT_CSD_S_C_VCC                 220     /* RO */
+#define EXT_CSD_REL_WR_SEC_C            222     /* RO */
+#define EXT_CSD_HC_WP_GRP_SIZE          221     /* RO */
+#define EXT_CSD_ERASE_TIMEOUT_MULT      223     /* RO */
+#define EXT_CSD_HC_ERASE_GRP_SIZE       224     /* RO */
+#define EXT_CSD_ACC_SIZE                225     /* RO */
+#define EXT_CSD_BOOT_MULT               226     /* RO */
+#define EXT_CSD_BOOT_INFO               228     /* RO */
+#define EXT_CSD_SEC_TRIM_MULT           229     /* RO */
+#define EXT_CSD_SEC_ERASE_MULT          230     /* RO */
+#define EXT_CSD_SEC_FEATURE_SUPPORT     231     /* RO */
+#define EXT_CSD_TRIM_MULT               232     /* RO */
+#define EXT_CSD_PWR_CL_200_195          236     /* RO */
+#define EXT_CSD_PWR_CL_200_360          237     /* RO */
+#define EXT_CSD_PWR_CL_DDR_52_195       238     /* RO */
+#define EXT_CSD_PWR_CL_DDR_52_360       239     /* RO */
+#define EXT_CSD_BKOPS_STATUS            246     /* RO */
+#define EXT_CSD_POWER_OFF_LONG_TIME     247     /* RO */
+#define EXT_CSD_GENERIC_CMD6_TIME       248     /* RO */
+#define EXT_CSD_CACHE_SIZE              249     /* RO, 4 bytes */
+#define EXT_CSD_PWR_CL_DDR_200_360      253     /* RO */
+#define EXT_CSD_FIRMWARE_VERSION        254     /* RO, 8 bytes */
+#define EXT_CSD_PRE_EOL_INFO            267     /* RO */
+#define EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_A      268     /* RO */
+#define EXT_CSD_DEVICE_LIFE_TIME_EST_TYP_B      269     /* RO */
+#define EXT_CSD_CMDQ_DEPTH              307     /* RO */
+#define EXT_CSD_CMDQ_SUPPORT            308     /* RO */
+#define EXT_CSD_SUPPORTED_MODE          493     /* RO */
+#define EXT_CSD_TAG_UNIT_SIZE           498     /* RO */
+#define EXT_CSD_DATA_TAG_SUPPORT        499     /* RO */
+#define EXT_CSD_MAX_PACKED_WRITES       500     /* RO */
+#define EXT_CSD_MAX_PACKED_READS        501     /* RO */
+#define EXT_CSD_BKOPS_SUPPORT           502     /* RO */
+#define EXT_CSD_HPI_FEATURES            503     /* RO */
+#define EXT_CSD_S_CMD_SET               504     /* RO */
+
+/*
+ * EXT_CSD field definitions
+ */
+
+#define EXT_CSD_WR_REL_PARAM_EN         (1 << 2)
+#define EXT_CSD_WR_REL_PARAM_EN_RPMB_REL_WR     (1 << 4)
+
+#define EXT_CSD_PART_CONFIG_ACC_MASK    (0x7)
+#define EXT_CSD_PART_CONFIG_ACC_DEFAULT (0x0)
+#define EXT_CSD_PART_CONFIG_ACC_BOOT0   (0x1)
+
+#define EXT_CSD_PART_CONFIG_EN_MASK     (0x7 << 3)
+#define EXT_CSD_PART_CONFIG_EN_BOOT0    (0x1 << 3)
+#define EXT_CSD_PART_CONFIG_EN_USER     (0x7 << 3)
+
 #endif
diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index 47360ba4ee98..dfe661079b8a 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -93,6 +93,11 @@ typedef struct {
 #define TYPE_SD_CARD "sd-card"
 OBJECT_DECLARE_TYPE(SDState, SDCardClass, SD_CARD)
 
+#define TYPE_EMMC "emmc"
+DECLARE_INSTANCE_CHECKER(SDState, EMMC, TYPE_EMMC)
+
+struct SDProto;
+
 struct SDCardClass {
     /*< private >*/
     DeviceClass parent_class;
@@ -124,6 +129,10 @@ struct SDCardClass {
     void (*enable)(SDState *sd, bool enable);
     bool (*get_inserted)(SDState *sd);
     bool (*get_readonly)(SDState *sd);
+
+    const struct SDProto *proto;
+    uint8_t spec_version;
+    void (*set_csd)(SDState *sd, uint64_t size);
 };
 
 #define TYPE_SD_BUS "sd-bus"
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 602ed6eb0701..5d7f04adf5a4 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -144,6 +144,7 @@ struct SDState {
     uint64_t data_start;
     uint32_t data_offset;
     uint8_t data[512];
+    uint8_t ext_csd[512];
     qemu_irq readonly_cb;
     qemu_irq inserted_cb;
     QEMUTimer *ocr_power_timer;
@@ -390,8 +391,85 @@ static const uint8_t sd_csd_rw_mask[16] = {
     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xfc, 0xfe,
 };
 
+static void mmc_set_ext_csd(SDState *sd, uint64_t size)
+{
+    uint32_t sectcount = size >> HWBLOCK_SHIFT;
+
+    memset(sd->ext_csd, 0, sizeof(sd->ext_csd));
+
+    sd->ext_csd[EXT_CSD_S_CMD_SET] = 0x1; /* supported command sets */
+    sd->ext_csd[EXT_CSD_HPI_FEATURES] = 0x3; /* HPI features  */
+    sd->ext_csd[EXT_CSD_BKOPS_SUPPORT] = 0x1; /* Background operations */
+    sd->ext_csd[241] = 0xA; /* 1st initialization time after partitioning */
+    sd->ext_csd[EXT_CSD_TRIM_MULT] = 0x1; /* Trim multiplier */
+    sd->ext_csd[EXT_CSD_SEC_FEATURE_SUPPORT] = 0x15; /* Secure feature */
+    sd->ext_csd[EXT_CSD_SEC_ERASE_MULT] = 0x96; /* Secure erase support */
+    sd->ext_csd[EXT_CSD_SEC_TRIM_MULT] = 0x96; /* Secure TRIM multiplier */
+    sd->ext_csd[EXT_CSD_BOOT_INFO] = 0x7; /* Boot information */
+    sd->ext_csd[EXT_CSD_BOOT_MULT] = 0x8; /* Boot partition size. 128KB unit */
+    sd->ext_csd[EXT_CSD_ACC_SIZE] = 0x6; /* Access size */
+    sd->ext_csd[EXT_CSD_HC_ERASE_GRP_SIZE] = 0x4; /* HC Erase unit size */
+    sd->ext_csd[EXT_CSD_ERASE_TIMEOUT_MULT] = 0x1; /* HC erase timeout */
+    sd->ext_csd[EXT_CSD_REL_WR_SEC_C] = 0x1; /* Reliable write sector count */
+    sd->ext_csd[EXT_CSD_HC_WP_GRP_SIZE] = 0x4; /* HC write protect group size */
+    sd->ext_csd[EXT_CSD_S_C_VCC] = 0x8; /* Sleep current VCC  */
+    sd->ext_csd[EXT_CSD_S_C_VCCQ] = 0x7; /* Sleep current VCCQ */
+    sd->ext_csd[EXT_CSD_S_A_TIMEOUT] = 0x11; /* Sleep/Awake timeout */
+    sd->ext_csd[215] = (sectcount >> 24) & 0xff; /* Sector count */
+    sd->ext_csd[214] = (sectcount >> 16) & 0xff; /* ... */
+    sd->ext_csd[213] = (sectcount >> 8) & 0xff;  /* ... */
+    sd->ext_csd[EXT_CSD_SEC_CNT] = (sectcount & 0xff);       /* ... */
+    sd->ext_csd[210] = 0xa; /* Min write perf for 8bit@52Mhz */
+    sd->ext_csd[209] = 0xa; /* Min read perf for 8bit@52Mhz  */
+    sd->ext_csd[208] = 0xa; /* Min write perf for 4bit@52Mhz */
+    sd->ext_csd[207] = 0xa; /* Min read perf for 4bit@52Mhz */
+    sd->ext_csd[206] = 0xa; /* Min write perf for 4bit@26Mhz */
+    sd->ext_csd[205] = 0xa; /* Min read perf for 4bit@26Mhz */
+    sd->ext_csd[EXT_CSD_PART_SWITCH_TIME] = 0x1;
+    sd->ext_csd[EXT_CSD_OUT_OF_INTERRUPT_TIME] = 0x1;
+    sd->ext_csd[EXT_CSD_CARD_TYPE] = 0x7;
+    sd->ext_csd[EXT_CSD_STRUCTURE] = 0x2;
+    sd->ext_csd[EXT_CSD_REV] = 0x5;
+    sd->ext_csd[EXT_CSD_RPMB_MULT] = 0x1; /* RPMB size */
+    sd->ext_csd[EXT_CSD_PARTITION_SUPPORT] = 0x3;
+    sd->ext_csd[159] = 0x00; /* Max enhanced area size */
+    sd->ext_csd[158] = 0x00; /* ... */
+    sd->ext_csd[157] = 0xEC; /* ... */
+}
+
+static void sd_emmc_set_csd(SDState *sd, uint64_t size)
+{
+    sd->csd[0] = 0xd0;
+    sd->csd[1] = 0x0f;
+    sd->csd[2] = 0x00;
+    sd->csd[3] = 0x32;
+    sd->csd[4] = 0x0f;
+    if (size <= 2 * GiB) {
+        /* use 1k blocks */
+        uint32_t csize1k = (size >> (CMULT_SHIFT + 10)) - 1;
+        sd->csd[5] = 0x5a;
+        sd->csd[6] = 0x80 | ((csize1k >> 10) & 0xf);
+        sd->csd[7] = (csize1k >> 2) & 0xff;
+    } else { /* >= 2GB : size stored in ext CSD, block addressing */
+        sd->csd[5] = 0x59;
+        sd->csd[6] = 0x8f;
+        sd->csd[7] = 0xff;
+        sd->ocr = FIELD_DP32(sd->ocr, OCR, CARD_CAPACITY, 1);
+    }
+    sd->csd[8] = 0xff;
+    sd->csd[9] = 0xff;
+    sd->csd[10] = 0xf7;
+    sd->csd[11] = 0xfe;
+    sd->csd[12] = 0x49;
+    sd->csd[13] = 0x10;
+    sd->csd[14] = 0x00;
+    sd->csd[15] = (sd_crc7(sd->csd, 15) << 1) | 1;
+    mmc_set_ext_csd(sd, size);
+}
+
 static void sd_set_csd(SDState *sd, uint64_t size)
 {
+    SDCardClass *sc = SD_CARD_GET_CLASS(sd);
     int hwblock_shift = HWBLOCK_SHIFT;
     uint32_t csize;
     uint32_t sectsize = (1 << (SECTOR_SHIFT + 1)) - 1;
@@ -403,7 +481,9 @@ static void sd_set_csd(SDState *sd, uint64_t size)
     }
     csize = (size >> (CMULT_SHIFT + hwblock_shift)) - 1;
 
-    if (size <= SDSC_MAX_CAPACITY) { /* Standard Capacity SD */
+    if (sc->set_csd) {
+        sc->set_csd(sd, size);
+    } else if (size <= SDSC_MAX_CAPACITY) { /* Standard Capacity SD */
         sd->csd[0] = 0x00;	/* CSD structure */
         sd->csd[1] = 0x26;	/* Data read access-time-1 */
         sd->csd[2] = 0x00;	/* Data read access-time-2 */
@@ -1028,6 +1108,25 @@ static sd_rsp_type_t sd_cmd_ALL_SEND_CID(SDState *sd, SDRequest req)
     return sd_r2_i;
 }
 
+static void sd_emmc_set_rca(SDState *sd, uint16_t value)
+{
+    sd->rca = value;
+}
+
+static sd_rsp_type_t sd_emmc_cmd_SEND_RELATIVE_ADDR(SDState *sd, SDRequest req)
+{
+    switch (sd->state) {
+    case sd_identification_state:
+    case sd_standby_state:
+        sd->state = sd_standby_state;
+        sd_emmc_set_rca(sd, req.arg >> 16);
+        return sd_r1;
+
+    default:
+        return sd_invalid_state_for_cmd(sd, req);
+    }
+}
+
 static sd_rsp_type_t sd_cmd_SEND_RELATIVE_ADDR(SDState *sd, SDRequest req)
 {
     switch (sd->state) {
@@ -2003,6 +2102,14 @@ uint8_t sd_read_byte(SDState *sd)
             sd->state = sd_transfer_state;
         break;
 
+    case 8:     /* CMD8: SEND_EXT_CSD on MMC */
+        ret = sd->data[sd->data_offset++];
+
+        if (sd->data_offset >= sizeof(sd->ext_csd)) {
+            sd->state = sd_transfer_state;
+        }
+        break;
+
     case 9:	/* CMD9:   SEND_CSD */
     case 10:	/* CMD10:  SEND_CID */
         ret = sd->data[sd->data_offset ++];
@@ -2144,6 +2251,75 @@ static const SDProto sd_proto_sd = {
     },
 };
 
+static sd_rsp_type_t sd_emmc_cmd_SEND_OP_CMD(SDState *sd, SDRequest req)
+{
+    sd->state = sd_ready_state;
+    return sd_r3;
+}
+
+static sd_rsp_type_t sd_emmc_cmd_ALL_SEND_CID(SDState *sd, SDRequest req)
+{
+    if (sd->state != sd_ready_state && sd->state != sd_idle_state) {
+        return sd_invalid_state_for_cmd(sd, req);
+    }
+
+    sd->state = sd_identification_state;
+
+    return sd_r2_i;
+}
+
+static sd_rsp_type_t sd_emmc_cmd_SEND_EXT_CSD(SDState *sd, SDRequest req)
+{
+    uint64_t addr = (sd->ocr & (1 << 30)) ? (uint64_t) req.arg << 9 : req.arg;
+
+    switch (sd->state) {
+    case sd_transfer_state:
+        /* MMC : Sends the EXT_CSD register as a Block of data */
+        sd->state = sd_sendingdata_state;
+        memcpy(sd->data, sd->ext_csd, sizeof(sd->ext_csd));
+        sd->data_start = addr;
+        sd->data_offset = 0;
+        return sd_r1;
+    default:
+        return sd_invalid_state_for_cmd(sd, req);
+    }
+}
+
+static sd_rsp_type_t sd_emmc_cmd_APP_CMD(SDState *sd, SDRequest req)
+{
+    return sd_r0;
+}
+
+static sd_rsp_type_t sd_emmc_cmd_SEND_TUNING_BLOCK(SDState *sd, SDRequest req)
+{
+    if (sd->state != sd_transfer_state) {
+        sd_invalid_state_for_cmd(sd, req);
+    }
+
+    sd->state = sd_sendingdata_state;
+    sd->data_offset = 0;
+    return sd_r1;
+}
+
+static const SDProto sd_proto_emmc = {
+    .name = "eMMC",
+    .cmd = {
+        [0]         = sd_cmd_GO_IDLE_STATE,
+        [1]         = sd_emmc_cmd_SEND_OP_CMD,
+        [2]         = sd_emmc_cmd_ALL_SEND_CID,
+        [3]         = sd_emmc_cmd_SEND_RELATIVE_ADDR,
+        [5]         = sd_cmd_illegal,
+        [8]         = sd_emmc_cmd_SEND_EXT_CSD,
+        [19]        = sd_cmd_SEND_TUNING_BLOCK,
+        [21]        = sd_emmc_cmd_SEND_TUNING_BLOCK,
+        [41]        = sd_cmd_illegal,
+        [52 ... 54] = sd_cmd_illegal,
+        [55]        = sd_emmc_cmd_APP_CMD,
+        [58]        = sd_cmd_illegal,
+        [59]        = sd_cmd_illegal,
+    },
+};
+
 static void sd_instance_init(Object *obj)
 {
     SDState *sd = SD_CARD(obj);
@@ -2162,10 +2338,19 @@ static void sd_instance_finalize(Object *obj)
 static void sd_realize(DeviceState *dev, Error **errp)
 {
     SDState *sd = SD_CARD(dev);
+    SDCardClass *sc = SD_CARD_GET_CLASS(sd);
     int ret;
 
     sd->proto = sd->spi ? &sd_proto_spi : &sd_proto_sd;
 
+    if (sc->proto) {
+        sd->proto = sc->proto;
+    }
+
+    if (sc->spec_version) {
+        sd->spec_version = sc->spec_version;
+    }
+
     switch (sd->spec_version) {
     case SD_PHY_SPECv1_10_VERS
      ... SD_PHY_SPECv3_01_VERS:
@@ -2261,9 +2446,27 @@ static const TypeInfo sd_info = {
     .instance_finalize = sd_instance_finalize,
 };
 
+static void emmc_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+    SDCardClass *sc = SD_CARD_CLASS(klass);
+
+    dc->desc = "eMMC";
+    sc->proto = &sd_proto_emmc;
+    sc->spec_version = SD_PHY_SPECv3_01_VERS; /* eMMC requirement */
+    sc->set_csd = sd_emmc_set_csd;
+}
+
+static const TypeInfo emmc_info = {
+    .name = TYPE_EMMC,
+    .parent = TYPE_SD_CARD,
+    .class_init = emmc_class_init,
+ };
+
 static void sd_register_types(void)
 {
     type_register_static(&sd_info);
+    type_register_static(&emmc_info);
 }
 
 type_init(sd_register_types)
diff --git a/hw/sd/sdmmc-internal.c b/hw/sd/sdmmc-internal.c
index 2053def3f10b..8648a7808dcc 100644
--- a/hw/sd/sdmmc-internal.c
+++ b/hw/sd/sdmmc-internal.c
@@ -14,7 +14,7 @@
 const char *sd_cmd_name(uint8_t cmd)
 {
     static const char *cmd_abbrev[SDMMC_CMD_MAX] = {
-         [0]    = "GO_IDLE_STATE",
+         [0]    = "GO_IDLE_STATE",           [1]    = "SEND_OP_CMD",
          [2]    = "ALL_SEND_CID",            [3]    = "SEND_RELATIVE_ADDR",
          [4]    = "SET_DSR",                 [5]    = "IO_SEND_OP_COND",
          [6]    = "SWITCH_FUNC",             [7]    = "SELECT/DESELECT_CARD",
-- 
2.34.1

Re: [RFC PATCH 11/17] hw/sd: Add eMMC support

[Jerome Forissier]

Hi Cédric,

On 3/18/22 14:28, Cédric Le Goater wrote:
> The initial eMMC support from Vincent Palatin was largely reworked to
> match the current SD framework. The parameters mimick a real 4GB eMMC,
> but it can be set to various sizes.
> 
> This adds a new QOM object class for EMMC devices.

That is interesting. Is RPMB emulation implemented? I doesn't look like
so, although...


> +    sd->ext_csd[EXT_CSD_RPMB_MULT] = 0x1; /* RPMB size */

...this indicates that the device has one block (128 KB) of RPMB if I'm
not mistaken.


I would be quite interested in testing and possibly helping out
implement RPMB, although I must admit don't know much about QEMU
internals. I have written some quick & dirty emulation code for RPMB at
the Linux ioctl() level, see [1]. This code is useful for CI testing of
the OP-TEE OS project [2], but having a lower level emulation in QEMU
would be much better since the Linux kernel would "see" the device and
report it in sysfs etc.

[1] https://github.com/OP-TEE/optee_client/blob/3.16.0/tee-supplicant/src/rpmb.c#L494-L571
[2] https://github.com/OP-TEE/optee_os/

Thanks,
-- 
Jerome

Re: [RFC PATCH 11/17] hw/sd: Add eMMC support

[Cédric Le Goater]

Hello Jerome,

On 3/28/22 14:10, Jerome Forissier wrote:
> Hi Cédric,
> 
> On 3/18/22 14:28, Cédric Le Goater wrote:
>> The initial eMMC support from Vincent Palatin was largely reworked to
>> match the current SD framework. The parameters mimick a real 4GB eMMC,
>> but it can be set to various sizes.
>>
>> This adds a new QOM object class for EMMC devices.
> 
> That is interesting. 

This series is really a pre-pre-alpha-0. It's a mixed of changes from
Joel and I on top of Vincent Palatin patches sent ~10 years ago. I
reworked them recently to take into account a proposal of Philippe to
improve the sd models and possibly add eMMC support. They are good
enough to boot a rainier-bmc board.

> Is RPMB emulation implemented? I doesn't look like so, although...

no. it's not.

> 
> 
>> +    sd->ext_csd[EXT_CSD_RPMB_MULT] = 0x1; /* RPMB size */
> 
> ...this indicates that the device has one block (128 KB) of RPMB if I'm
> not mistaken.
> 
> 
> I would be quite interested in testing and possibly helping out
> implement RPMB, although I must admit don't know much about QEMU
> internals. I have written some quick & dirty emulation code for RPMB at
> the Linux ioctl() level, see [1]. This code is useful for CI testing of
> the OP-TEE OS project [2], but having a lower level emulation in QEMU
> would be much better since the Linux kernel would "see" the device and
> report it in sysfs etc.
> 
> [1] https://github.com/OP-TEE/optee_client/blob/3.16.0/tee-supplicant/src/rpmb.c#L494-L571
> [2] https://github.com/OP-TEE/optee_os/


You can give these patches a try using this branch:

   https://github.com/legoater/qemu/commits/aspeed-7.0

Using the above QEMU, I would run a rainier-bmc machine which requires eMMC
support :

   qemu-system-arm -M rainier-bmc \
	-nic user,mac=C0:FF:EE:00:00:02,hostfwd=::2222-:22 \
	-drive file=/path/to/mmc-p10bmc.qcow2,format=qcow2,if=sd,id=sd0,index=2 \
         -nographic -nodefaults -snapshot -serial mon:stdio
	
The bootable qcow2 image :

   https://www.kaod.org/qemu/aspeed/mmc-p10bmc.qcow2

was created from :

   https://jenkins.openbmc.org/view/latest/job/latest-master/label=docker-builder,target=p10bmc/lastSuccessfulBuild/artifact/openbmc/build/tmp/deploy/images/p10bmc/

Booting from the eMMC is a topic by itself but instead of booting
from the eMMC, I would use a custom kernel and userspace.

Compile an AST2600 kernel or grab :

   https://www.kaod.org/qemu/aspeed/zImage
   https://www.kaod.org/qemu/aspeed/aspeed-bmc-ibm-rainier.dtb

Compile a buildroot image or grab :

   https://www.kaod.org/qemu/aspeed/rootfs.cpio.xz
   
and run :

   qemu-system-arm -M rainier-bmc \
	-kernel /path/to/linux/build_ast2600/arch/arm/boot/zImage \
	-initrd /path/to/rootfs.cpio.xz \
	-dtb /path/to/linux/build_ast2600/arch/arm/boot/dts/aspeed-bmc-ibm-rainier.dtb \
	-append 'console=ttyS4,115200n8 root=/dev/ram ro' \
	-nic user,mac=C0:FF:EE:00:00:02,hostfwd=::2222-:22 \
	-drive file=/path/to/mmc-p10bmc.qcow2,format=qcow2,if=sd,id=sd0,index=2 \
         -nographic -nodefaults -serial mon:stdio
  
root/0penBmc to login.

 From there, modify the kernel and the buildroot image, you can scp
some binary, do your testing and possibly send patches to improve QEMU
support !

C.

Re: [RFC PATCH 11/17] hw/sd: Add eMMC support

[Philippe Mathieu-Daudé via]

Hi Cédric,

On 18/3/22 14:28, Cédric Le Goater wrote:
> The initial eMMC support from Vincent Palatin was largely reworked to
> match the current SD framework. The parameters mimick a real 4GB eMMC,
> but it can be set to various sizes.
> 
> This adds a new QOM object class for EMMC devices.
> 
> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
> Link: https://lore.kernel.org/r/1311635951-11047-5-git-send-email-vpalatin@chromium.org
> [ jms: - Forward ported to QEMU 5.2 ]
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> [ clg: - ported on aspeed-7.0 patchset
>         - HPI activation ]
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   hw/sd/sdmmc-internal.h |  97 +++++++++++++++++++
>   include/hw/sd/sd.h     |   9 ++
>   hw/sd/sd.c             | 205 ++++++++++++++++++++++++++++++++++++++++-
>   hw/sd/sdmmc-internal.c |   2 +-
>   4 files changed, 311 insertions(+), 2 deletions(-)

Do you mind splitting as:

- Add TYPE_EMMC, emmc_class_init and sd_proto_emmc[] with
   already existing handlers (1 patch)

- Add new handlers, from smaller to sd_emmc_set_csd(),
   and finally mmc_set_ext_csd() with the EXT_CSD definitions
   (various patches).

Otherwise LGTM!

What is your test suite?

Thanks,

Phil.

Re: [RFC PATCH 11/17] hw/sd: Add eMMC support

[Cédric Le Goater]

[ ... ]


> Do you mind splitting as:
> 
> - Add TYPE_EMMC, emmc_class_init and sd_proto_emmc[] with
>    already existing handlers (1 patch)
> 
> - Add new handlers, from smaller to sd_emmc_set_csd(),
>    and finally mmc_set_ext_csd() with the EXT_CSD definitions
>    (various patches).

Sure, I will reorganize the patchset. It will ease the review.

> Otherwise LGTM!
> 
> What is your test suite?

Not simple.

Get the latest FW images for the rainier platform from :

   https://jenkins.openbmc.org/view/latest/job/latest-master/label=docker-builder,target=p10bmc/lastSuccessfulBuild/artifact/openbmc/build/tmp/deploy/images/p10bmc/

and build a bootable eMMC with such a sequence :

   dd of=$mmc if=/dev/zero bs=1M count=128
   dd of=$mmc if=${fw_dir}/u-boot-spl.bin conv=notrunc
   dd of=$mmc if=${fw_dir}/u-boot.bin conv=notrunc bs=1K seek=64
   dd of=$mmc if=${fw_dir}/u-boot-env.bin conv=notrunc bs=1K seek=$((896 + 64))
     
   xzdec $wicxz | dd status=progress of=$mmc conv=notrunc bs=1M seek=2
   truncate --size 16G $mmc

We could have a smaller image with a buildroot rootfs.

Then, boot with:

   qemu-system-arm -M rainier-bmc -net nic,netdev=net0 -netdev user \
	-drive file=$mmc.qcow2,format=qcow2,if=sd,id=sd0,index=2

The Aspeed machines (AST2600) can boot from flash or eMMC and the above
command line does the trick. It is not optimal.


Thanks,

C.

Re: [RFC PATCH 11/17] hw/sd: Add eMMC support

[Cédric Le Goater]

On 5/10/22 09:15, Cédric Le Goater wrote:
> [ ... ]
> 
> 
>> Do you mind splitting as:
>>
>> - Add TYPE_EMMC, emmc_class_init and sd_proto_emmc[] with
>>    already existing handlers (1 patch)
>>
>> - Add new handlers, from smaller to sd_emmc_set_csd(),
>>    and finally mmc_set_ext_csd() with the EXT_CSD definitions
>>    (various patches).
> 
> Sure, I will reorganize the patchset. It will ease the review.

I just did in https://github.com/legoater/qemu/commits/aspeed-7.1

The commit logs need more information, at least a reference to the specs.

> 
>> Otherwise LGTM!
>>
>> What is your test suite?
> 
> Not simple.
> 
> Get the latest FW images for the rainier platform from :
> 
>    https://jenkins.openbmc.org/view/latest/job/latest-master/label=docker-builder,target=p10bmc/lastSuccessfulBuild/artifact/openbmc/build/tmp/deploy/images/p10bmc/
> 
> and build a bootable eMMC with such a sequence :
> 
>    dd of=$mmc if=/dev/zero bs=1M count=128
>    dd of=$mmc if=${fw_dir}/u-boot-spl.bin conv=notrunc
>    dd of=$mmc if=${fw_dir}/u-boot.bin conv=notrunc bs=1K seek=64
>    dd of=$mmc if=${fw_dir}/u-boot-env.bin conv=notrunc bs=1K seek=$((896 + 64))
>    xzdec $wicxz | dd status=progress of=$mmc conv=notrunc bs=1M seek=2
>    truncate --size 16G $mmc
> 
> We could have a smaller image with a buildroot rootfs.
> 
> Then, boot with:
> 
>    qemu-system-arm -M rainier-bmc -net nic,netdev=net0 -netdev user \
>      -drive file=$mmc.qcow2,format=qcow2,if=sd,id=sd0,index=2

and you will need my branch for that.

Thanks,

C.



> The Aspeed machines (AST2600) can boot from flash or eMMC and the above
> command line does the trick. It is not optimal.
> 
> 
> Thanks,
> 
> C.

Re: [RFC PATCH 11/17] hw/sd: Add eMMC support

[Philippe Mathieu-Daudé via]

Hi Cédric,

On 18/3/22 14:28, Cédric Le Goater wrote:
> The initial eMMC support from Vincent Palatin was largely reworked to
> match the current SD framework. The parameters mimick a real 4GB eMMC,
> but it can be set to various sizes.
> 
> This adds a new QOM object class for EMMC devices.
> 
> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
> Link: https://lore.kernel.org/r/1311635951-11047-5-git-send-email-vpalatin@chromium.org
> [ jms: - Forward ported to QEMU 5.2 ]
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> [ clg: - ported on aspeed-7.0 patchset
>         - HPI activation ]
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   hw/sd/sdmmc-internal.h |  97 +++++++++++++++++++
>   include/hw/sd/sd.h     |   9 ++
>   hw/sd/sd.c             | 205 ++++++++++++++++++++++++++++++++++++++++-
>   hw/sd/sdmmc-internal.c |   2 +-
>   4 files changed, 311 insertions(+), 2 deletions(-)

> +static const SDProto sd_proto_emmc = {

What about renaming as:

                     ... emmc_proto = {

> +    .name = "eMMC",
> +    .cmd = {
> +        [0]         = sd_cmd_GO_IDLE_STATE,
> +        [1]         = sd_emmc_cmd_SEND_OP_CMD,

                        = emmc_cmd_SEND_OP_CMD,

> +        [2]         = sd_emmc_cmd_ALL_SEND_CID,

                          ...

?

> +        [3]         = sd_emmc_cmd_SEND_RELATIVE_ADDR,
> +        [5]         = sd_cmd_illegal,
> +        [8]         = sd_emmc_cmd_SEND_EXT_CSD,
> +        [19]        = sd_cmd_SEND_TUNING_BLOCK,
> +        [21]        = sd_emmc_cmd_SEND_TUNING_BLOCK,
> +        [41]        = sd_cmd_illegal,
> +        [52 ... 54] = sd_cmd_illegal,
> +        [55]        = sd_emmc_cmd_APP_CMD,
> +        [58]        = sd_cmd_illegal,
> +        [59]        = sd_cmd_illegal,
> +    },
> +};

Re: [RFC PATCH 11/17] hw/sd: Add eMMC support

[Cédric Le Goater]

On 5/30/22 19:02, Philippe Mathieu-Daudé wrote:
> Hi Cédric,
> 
> On 18/3/22 14:28, Cédric Le Goater wrote:
>> The initial eMMC support from Vincent Palatin was largely reworked to
>> match the current SD framework. The parameters mimick a real 4GB eMMC,
>> but it can be set to various sizes.
>>
>> This adds a new QOM object class for EMMC devices.
>>
>> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
>> Link: https://lore.kernel.org/r/1311635951-11047-5-git-send-email-vpalatin@chromium.org
>> [ jms: - Forward ported to QEMU 5.2 ]
>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>> [ clg: - ported on aspeed-7.0 patchset
>>         - HPI activation ]
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>   hw/sd/sdmmc-internal.h |  97 +++++++++++++++++++
>>   include/hw/sd/sd.h     |   9 ++
>>   hw/sd/sd.c             | 205 ++++++++++++++++++++++++++++++++++++++++-
>>   hw/sd/sdmmc-internal.c |   2 +-
>>   4 files changed, 311 insertions(+), 2 deletions(-)
> 
>> +static const SDProto sd_proto_emmc = {
> 
> What about renaming as:
> 
>                      ... emmc_proto = {

yes. These are internal functions. Fine with me.

Thanks,

C.

> 
>> +    .name = "eMMC",
>> +    .cmd = {
>> +        [0]         = sd_cmd_GO_IDLE_STATE,
>> +        [1]         = sd_emmc_cmd_SEND_OP_CMD,
> 
>                         = emmc_cmd_SEND_OP_CMD,
> 
>> +        [2]         = sd_emmc_cmd_ALL_SEND_CID,
> 
>                           ...
> 
> ?
> 
>> +        [3]         = sd_emmc_cmd_SEND_RELATIVE_ADDR,
>> +        [5]         = sd_cmd_illegal,
>> +        [8]         = sd_emmc_cmd_SEND_EXT_CSD,
>> +        [19]        = sd_cmd_SEND_TUNING_BLOCK,
>> +        [21]        = sd_emmc_cmd_SEND_TUNING_BLOCK,
>> +        [41]        = sd_cmd_illegal,
>> +        [52 ... 54] = sd_cmd_illegal,
>> +        [55]        = sd_emmc_cmd_APP_CMD,
>> +        [58]        = sd_cmd_illegal,
>> +        [59]        = sd_cmd_illegal,
>> +    },
>> +};

Re: [RFC PATCH 11/17] hw/sd: Add eMMC support

[Philippe Mathieu-Daudé via]

On 18/3/22 14:28, Cédric Le Goater wrote:
> The initial eMMC support from Vincent Palatin was largely reworked to
> match the current SD framework. The parameters mimick a real 4GB eMMC,
> but it can be set to various sizes.
> 
> This adds a new QOM object class for EMMC devices.
> 
> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
> Link: https://lore.kernel.org/r/1311635951-11047-5-git-send-email-vpalatin@chromium.org
> [ jms: - Forward ported to QEMU 5.2 ]
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> [ clg: - ported on aspeed-7.0 patchset
>         - HPI activation ]
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   hw/sd/sdmmc-internal.h |  97 +++++++++++++++++++
>   include/hw/sd/sd.h     |   9 ++
>   hw/sd/sd.c             | 205 ++++++++++++++++++++++++++++++++++++++++-
>   hw/sd/sdmmc-internal.c |   2 +-
>   4 files changed, 311 insertions(+), 2 deletions(-)


> +static void emmc_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +    SDCardClass *sc = SD_CARD_CLASS(klass);
> +
> +    dc->desc = "eMMC";
> +    sc->proto = &sd_proto_emmc;
> +    sc->spec_version = SD_PHY_SPECv3_01_VERS; /* eMMC requirement */
> +    sc->set_csd = sd_emmc_set_csd;
> +}
> +
> +static const TypeInfo emmc_info = {
> +    .name = TYPE_EMMC,
> +    .parent = TYPE_SD_CARD,

Hmm this is odd to have the model inheriting features from SD_CARD but 
then behaving differently (one could enumerate QDEV objects implementing
TYPE_SD_CARD then use them expecting they match the SD card protocol).

Why do you need to have TYPE_SD_CARD as parent?

Could we simply duplicate sd_class_init() assignations instead? That
would likely make it easier to modify eMMC handlers.

> +    .class_init = emmc_class_init,
> + };

Re: [RFC PATCH 11/17] hw/sd: Add eMMC support

[Cédric Le Goater]

On 5/30/22 19:40, Philippe Mathieu-Daudé wrote:
> On 18/3/22 14:28, Cédric Le Goater wrote:
>> The initial eMMC support from Vincent Palatin was largely reworked to
>> match the current SD framework. The parameters mimick a real 4GB eMMC,
>> but it can be set to various sizes.
>>
>> This adds a new QOM object class for EMMC devices.
>>
>> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
>> Link: https://lore.kernel.org/r/1311635951-11047-5-git-send-email-vpalatin@chromium.org
>> [ jms: - Forward ported to QEMU 5.2 ]
>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>> [ clg: - ported on aspeed-7.0 patchset
>>         - HPI activation ]
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>   hw/sd/sdmmc-internal.h |  97 +++++++++++++++++++
>>   include/hw/sd/sd.h     |   9 ++
>>   hw/sd/sd.c             | 205 ++++++++++++++++++++++++++++++++++++++++-
>>   hw/sd/sdmmc-internal.c |   2 +-
>>   4 files changed, 311 insertions(+), 2 deletions(-)
> 
> 
>> +static void emmc_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +    SDCardClass *sc = SD_CARD_CLASS(klass);
>> +
>> +    dc->desc = "eMMC";
>> +    sc->proto = &sd_proto_emmc;
>> +    sc->spec_version = SD_PHY_SPECv3_01_VERS; /* eMMC requirement */
>> +    sc->set_csd = sd_emmc_set_csd;
>> +}
>> +
>> +static const TypeInfo emmc_info = {
>> +    .name = TYPE_EMMC,
>> +    .parent = TYPE_SD_CARD,
> 
> Hmm this is odd to have the model inheriting features from SD_CARD but then behaving differently (one could enumerate QDEV objects implementing
> TYPE_SD_CARD then use them expecting they match the SD card protocol).
> 
> Why do you need to have TYPE_SD_CARD as parent?

Simply for the initialization.
> Could we simply duplicate sd_class_init() assignations instead? That
> would likely make it easier to modify eMMC handlers.

May be we lack a base abstract class ?

It would clean up this section in the realize routine :

    sd->proto = sd->spi ? &sd_proto_spi : &sd_proto_sd;

     if (sc->proto) {
         sd->proto = sc->proto;
     }

Thanks,

C.

>> +    .class_init = emmc_class_init,
>> + };

Re: [RFC PATCH 11/17] hw/sd: Add eMMC support

[Philippe Mathieu-Daudé via]

On 31/5/22 07:58, Cédric Le Goater wrote:
> On 5/30/22 19:40, Philippe Mathieu-Daudé wrote:
>> On 18/3/22 14:28, Cédric Le Goater wrote:
>>> The initial eMMC support from Vincent Palatin was largely reworked to
>>> match the current SD framework. The parameters mimick a real 4GB eMMC,
>>> but it can be set to various sizes.
>>>
>>> This adds a new QOM object class for EMMC devices.
>>>
>>> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
>>> Link: 
>>> https://lore.kernel.org/r/1311635951-11047-5-git-send-email-vpalatin@chromium.org 
>>>
>>> [ jms: - Forward ported to QEMU 5.2 ]
>>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>>> [ clg: - ported on aspeed-7.0 patchset
>>>         - HPI activation ]
>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>> ---
>>>   hw/sd/sdmmc-internal.h |  97 +++++++++++++++++++
>>>   include/hw/sd/sd.h     |   9 ++
>>>   hw/sd/sd.c             | 205 ++++++++++++++++++++++++++++++++++++++++-
>>>   hw/sd/sdmmc-internal.c |   2 +-
>>>   4 files changed, 311 insertions(+), 2 deletions(-)
>>
>>
>>> +static void emmc_class_init(ObjectClass *klass, void *data)
>>> +{
>>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>>> +    SDCardClass *sc = SD_CARD_CLASS(klass);
>>> +
>>> +    dc->desc = "eMMC";
>>> +    sc->proto = &sd_proto_emmc;
>>> +    sc->spec_version = SD_PHY_SPECv3_01_VERS; /* eMMC requirement */
>>> +    sc->set_csd = sd_emmc_set_csd;
>>> +}
>>> +
>>> +static const TypeInfo emmc_info = {
>>> +    .name = TYPE_EMMC,
>>> +    .parent = TYPE_SD_CARD,
>>
>> Hmm this is odd to have the model inheriting features from SD_CARD but 
>> then behaving differently (one could enumerate QDEV objects implementing
>> TYPE_SD_CARD then use them expecting they match the SD card protocol).
>>
>> Why do you need to have TYPE_SD_CARD as parent?
> 
> Simply for the initialization.
>> Could we simply duplicate sd_class_init() assignations instead? That
>> would likely make it easier to modify eMMC handlers.
> 
> May be we lack a base abstract class ?

I've been thinking about it but maybe not enough. I'll revisit.

> It would clean up this section in the realize routine :
> 
>     sd->proto = sd->spi ? &sd_proto_spi : &sd_proto_sd;
> 
>      if (sc->proto) {
>          sd->proto = sc->proto;
>      }

In v2 I moved the 'proto' field from instance to class, so we don't need
this hack anymore.

Re: [RFC PATCH 11/17] hw/sd: Add eMMC support

[Cédric Le Goater]

On 5/31/22 10:03, Philippe Mathieu-Daudé wrote:
> On 31/5/22 07:58, Cédric Le Goater wrote:
>> On 5/30/22 19:40, Philippe Mathieu-Daudé wrote:
>>> On 18/3/22 14:28, Cédric Le Goater wrote:
>>>> The initial eMMC support from Vincent Palatin was largely reworked to
>>>> match the current SD framework. The parameters mimick a real 4GB eMMC,
>>>> but it can be set to various sizes.
>>>>
>>>> This adds a new QOM object class for EMMC devices.
>>>>
>>>> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
>>>> Link: https://lore.kernel.org/r/1311635951-11047-5-git-send-email-vpalatin@chromium.org
>>>> [ jms: - Forward ported to QEMU 5.2 ]
>>>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>>>> [ clg: - ported on aspeed-7.0 patchset
>>>>         - HPI activation ]
>>>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>>>> ---
>>>>   hw/sd/sdmmc-internal.h |  97 +++++++++++++++++++
>>>>   include/hw/sd/sd.h     |   9 ++
>>>>   hw/sd/sd.c             | 205 ++++++++++++++++++++++++++++++++++++++++-
>>>>   hw/sd/sdmmc-internal.c |   2 +-
>>>>   4 files changed, 311 insertions(+), 2 deletions(-)
>>>
>>>
>>>> +static void emmc_class_init(ObjectClass *klass, void *data)
>>>> +{
>>>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>>>> +    SDCardClass *sc = SD_CARD_CLASS(klass);
>>>> +
>>>> +    dc->desc = "eMMC";
>>>> +    sc->proto = &sd_proto_emmc;
>>>> +    sc->spec_version = SD_PHY_SPECv3_01_VERS; /* eMMC requirement */
>>>> +    sc->set_csd = sd_emmc_set_csd;
>>>> +}
>>>> +
>>>> +static const TypeInfo emmc_info = {
>>>> +    .name = TYPE_EMMC,
>>>> +    .parent = TYPE_SD_CARD,
>>>
>>> Hmm this is odd to have the model inheriting features from SD_CARD but then behaving differently (one could enumerate QDEV objects implementing
>>> TYPE_SD_CARD then use them expecting they match the SD card protocol).
>>>
>>> Why do you need to have TYPE_SD_CARD as parent?
>>
>> Simply for the initialization.
>>> Could we simply duplicate sd_class_init() assignations instead? That
>>> would likely make it easier to modify eMMC handlers.
>>
>> May be we lack a base abstract class ?
> 
> I've been thinking about it but maybe not enough. I'll revisit.
> 
>> It would clean up this section in the realize routine :
>>
>>     sd->proto = sd->spi ? &sd_proto_spi : &sd_proto_sd;
>>
>>      if (sc->proto) {
>>          sd->proto = sc->proto;
>>      }
> 
> In v2 I moved the 'proto' field from instance to class, so we don't need
> this hack anymore.

Indeed :

    static void sd_realize(DeviceState *dev, Error **errp)
    {
        SDState *sd = SD_CARD(dev);
        SDCardClass *sc = SD_CARD_GET_CLASS(sd);
        int ret;
    
        sc->proto = sd->spi ? &sd_proto_spi : &sd_proto_sd;
        ...

but this is assigning a class attribute from an instance :/

C.

Re: [RFC PATCH 11/17] hw/sd: Add eMMC support

[Philippe Mathieu-Daudé via]

On 18/3/22 14:28, Cédric Le Goater wrote:
> The initial eMMC support from Vincent Palatin was largely reworked to
> match the current SD framework. The parameters mimick a real 4GB eMMC,
> but it can be set to various sizes.
> 
> This adds a new QOM object class for EMMC devices.
> 
> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
> Link: https://lore.kernel.org/r/1311635951-11047-5-git-send-email-vpalatin@chromium.org
> [ jms: - Forward ported to QEMU 5.2 ]
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> [ clg: - ported on aspeed-7.0 patchset
>         - HPI activation ]
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   hw/sd/sdmmc-internal.h |  97 +++++++++++++++++++
>   include/hw/sd/sd.h     |   9 ++
>   hw/sd/sd.c             | 205 ++++++++++++++++++++++++++++++++++++++++-
>   hw/sd/sdmmc-internal.c |   2 +-
>   4 files changed, 311 insertions(+), 2 deletions(-)

>   static void sd_instance_init(Object *obj)
>   {
>       SDState *sd = SD_CARD(obj);
> @@ -2162,10 +2338,19 @@ static void sd_instance_finalize(Object *obj)
>   static void sd_realize(DeviceState *dev, Error **errp)
>   {
>       SDState *sd = SD_CARD(dev);
> +    SDCardClass *sc = SD_CARD_GET_CLASS(sd);
>       int ret;
>   
>       sd->proto = sd->spi ? &sd_proto_spi : &sd_proto_sd;
>   
> +    if (sc->proto) {
> +        sd->proto = sc->proto;
> +    }
> +
> +    if (sc->spec_version) {
> +        sd->spec_version = sc->spec_version;
> +    }
> +
>       switch (sd->spec_version) {
>       case SD_PHY_SPECv1_10_VERS
>        ... SD_PHY_SPECv3_01_VERS:


Instead I'd use:

-- >8 --
@@ -2301,14 +2297,26 @@ static const TypeInfo sd_info = {
      .instance_finalize = sd_instance_finalize,
  };

+static void emmc_realize(DeviceState *dev, Error **errp)
+{
+    SDState *sd = SD_CARD(dev);
+
+    if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
+            error_setg(errp, "Minimum spec for eMMC is v3.01");
+            return;
+    }
+
+    sd_realize(dev, errp);
+}
+
  static void emmc_class_init(ObjectClass *klass, void *data)
  {
      DeviceClass *dc = DEVICE_CLASS(klass);
      SDCardClass *sc = SD_CARD_CLASS(klass);

      dc->desc = "eMMC";
+    dc->realize = emmc_realize;
      sc->proto = &sd_proto_emmc;
  }

---

Re: [RFC PATCH 11/17] hw/sd: Add eMMC support

[Cédric Le Goater]

On 5/30/22 20:29, Philippe Mathieu-Daudé wrote:
> On 18/3/22 14:28, Cédric Le Goater wrote:
>> The initial eMMC support from Vincent Palatin was largely reworked to
>> match the current SD framework. The parameters mimick a real 4GB eMMC,
>> but it can be set to various sizes.
>>
>> This adds a new QOM object class for EMMC devices.
>>
>> Signed-off-by: Vincent Palatin <vpalatin@chromium.org>
>> Link: https://lore.kernel.org/r/1311635951-11047-5-git-send-email-vpalatin@chromium.org
>> [ jms: - Forward ported to QEMU 5.2 ]
>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>> [ clg: - ported on aspeed-7.0 patchset
>>         - HPI activation ]
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>   hw/sd/sdmmc-internal.h |  97 +++++++++++++++++++
>>   include/hw/sd/sd.h     |   9 ++
>>   hw/sd/sd.c             | 205 ++++++++++++++++++++++++++++++++++++++++-
>>   hw/sd/sdmmc-internal.c |   2 +-
>>   4 files changed, 311 insertions(+), 2 deletions(-)
> 
>>   static void sd_instance_init(Object *obj)
>>   {
>>       SDState *sd = SD_CARD(obj);
>> @@ -2162,10 +2338,19 @@ static void sd_instance_finalize(Object *obj)
>>   static void sd_realize(DeviceState *dev, Error **errp)
>>   {
>>       SDState *sd = SD_CARD(dev);
>> +    SDCardClass *sc = SD_CARD_GET_CLASS(sd);
>>       int ret;
>>       sd->proto = sd->spi ? &sd_proto_spi : &sd_proto_sd;
>> +    if (sc->proto) {
>> +        sd->proto = sc->proto;
>> +    }
>> +
>> +    if (sc->spec_version) {
>> +        sd->spec_version = sc->spec_version;
>> +    }
>> +
>>       switch (sd->spec_version) {
>>       case SD_PHY_SPECv1_10_VERS
>>        ... SD_PHY_SPECv3_01_VERS:
> 
> 
> Instead I'd use:


Yes. This is better.

Thanks,

C.

> -- >8 --
> @@ -2301,14 +2297,26 @@ static const TypeInfo sd_info = {
>       .instance_finalize = sd_instance_finalize,
>   };
> 
> +static void emmc_realize(DeviceState *dev, Error **errp)
> +{
> +    SDState *sd = SD_CARD(dev);
> +
> +    if (sd->spec_version < SD_PHY_SPECv3_01_VERS) {
> +            error_setg(errp, "Minimum spec for eMMC is v3.01");
> +            return;
> +    }
> +
> +    sd_realize(dev, errp);
> +}
> +
>   static void emmc_class_init(ObjectClass *klass, void *data)
>   {
>       DeviceClass *dc = DEVICE_CLASS(klass);
>       SDCardClass *sc = SD_CARD_CLASS(klass);
> 
>       dc->desc = "eMMC";
> +    dc->realize = emmc_realize;
>       sc->proto = &sd_proto_emmc;
>   }
> 
> ---
> 
> 

[RFC PATCH 12/17] hw/sd: Fix SET_BLOCK_COUNT command argument

[Cédric Le Goater]

The number of blocks is defined in the lower bits [15:0].

WIP. This needs to be more precise on the spec version.

Signed-off-by: Cédric Le Goater <clg@kaod.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 5d7f04adf5a4..517699c5fc98 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1406,7 +1406,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         switch (sd->state) {
         case sd_transfer_state:
-            sd->multi_blk_cnt = req.arg;
+            sd->multi_blk_cnt = req.arg & 0xFFFF;
             return sd_r1;
 
         default:
-- 
2.34.1

[RFC PATCH 13/17] hw/sd: Update CMD1 definition for MMC

[Cédric Le Goater]

From: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>

Add support to Power up the card and send response r3 in case of MMC.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
[ clg: - ported on aspeed-7.0 patchset ]
Signed-off-by: Cédric Le Goater <clg@kaod.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 517699c5fc98..289764a38bf3 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2253,8 +2253,8 @@ static const SDProto sd_proto_sd = {
 
 static sd_rsp_type_t sd_emmc_cmd_SEND_OP_CMD(SDState *sd, SDRequest req)
 {
-    sd->state = sd_ready_state;
-    return sd_r3;
+    sd_ocr_powerup(sd);
+    return sd->state == sd_idle_state ? sd_r3 : sd_r0;
 }
 
 static sd_rsp_type_t sd_emmc_cmd_ALL_SEND_CID(SDState *sd, SDRequest req)
-- 
2.34.1

[RFC PATCH 14/17] hw/sd: Add CMD21 tuning sequence

[Cédric Le Goater]

From: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>

MMC cards support different tuning sequence for entering HS200 mode.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
[ clg: - ported on aspeed-7.0 patchset ]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 289764a38bf3..3a3b9d62e1ba 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -2071,6 +2071,30 @@ static const uint8_t sd_tuning_block_pattern[SD_TUNING_BLOCK_SIZE] = {
     0xbb, 0xff, 0xf7, 0xff,         0xf7, 0x7f, 0x7b, 0xde,
 };
 
+#define EXCSD_BUS_WIDTH_OFFSET 183
+#define BUS_WIDTH_8_MASK    0x4
+#define BUS_WIDTH_4_MASK    0x2
+#define MMC_TUNING_BLOCK_SIZE   128
+
+static const uint8_t mmc_tuning_block_pattern[128] = {
+       0xff, 0xff, 0x00, 0xff, 0xff, 0xff, 0x00, 0x00,
+       0xff, 0xff, 0xcc, 0xcc, 0xcc, 0x33, 0xcc, 0xcc,
+       0xcc, 0x33, 0x33, 0xcc, 0xcc, 0xcc, 0xff, 0xff,
+       0xff, 0xee, 0xff, 0xff, 0xff, 0xee, 0xee, 0xff,
+       0xff, 0xff, 0xdd, 0xff, 0xff, 0xff, 0xdd, 0xdd,
+       0xff, 0xff, 0xff, 0xbb, 0xff, 0xff, 0xff, 0xbb,
+       0xbb, 0xff, 0xff, 0xff, 0x77, 0xff, 0xff, 0xff,
+       0x77, 0x77, 0xff, 0x77, 0xbb, 0xdd, 0xee, 0xff,
+       0xff, 0xff, 0xff, 0x00, 0xff, 0xff, 0xff, 0x00,
+       0x00, 0xff, 0xff, 0xcc, 0xcc, 0xcc, 0x33, 0xcc,
+       0xcc, 0xcc, 0x33, 0x33, 0xcc, 0xcc, 0xcc, 0xff,
+       0xff, 0xff, 0xee, 0xff, 0xff, 0xff, 0xee, 0xee,
+       0xff, 0xff, 0xff, 0xdd, 0xff, 0xff, 0xff, 0xdd,
+       0xdd, 0xff, 0xff, 0xff, 0xbb, 0xff, 0xff, 0xff,
+       0xbb, 0xbb, 0xff, 0xff, 0xff, 0x77, 0xff, 0xff,
+       0xff, 0x77, 0x77, 0xff, 0x77, 0xbb, 0xdd, 0xee,
+};
+
 uint8_t sd_read_byte(SDState *sd)
 {
     /* TODO: Append CRCs */
@@ -2165,6 +2189,21 @@ uint8_t sd_read_byte(SDState *sd)
         ret = sd_tuning_block_pattern[sd->data_offset++];
         break;
 
+    case 21:    /* CMD21: SEND_TUNING_BLOCK (MMC) */
+        if (sd->data_offset >= MMC_TUNING_BLOCK_SIZE - 1) {
+            sd->state = sd_transfer_state;
+        }
+        if (sd->ext_csd[EXCSD_BUS_WIDTH_OFFSET] & BUS_WIDTH_8_MASK) {
+            ret = mmc_tuning_block_pattern[sd->data_offset++];
+        } else {
+            /* Return LSB Nibbles of two byte from the 8bit tuning block
+             * for 4bit mode
+             */
+            ret = mmc_tuning_block_pattern[sd->data_offset++] & 0x0F;
+            ret |= (mmc_tuning_block_pattern[sd->data_offset++] & 0x0F) << 4;
+        }
+        break;
+
     case 22:	/* ACMD22: SEND_NUM_WR_BLOCKS */
         ret = sd->data[sd->data_offset ++];
 
-- 
2.34.1

[RFC PATCH 15/17] hw/sd: Add mmc switch function support

[Cédric Le Goater]

From: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>

switch operation in mmc cards, updated the ext_csd register to
request changes in card operations. Here we implement similar
sequence but requests are mostly dummy and make no change.

Implement SWITCH_ERROR if the write operation offset goes beyond length
of ext_csd.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
[ clg: - ported on aspeed-7.0 patchset ]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 3a3b9d62e1ba..d3bda6c2aa6b 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -538,6 +538,7 @@ static void sd_set_rca(SDState *sd)
 FIELD(CSR, AKE_SEQ_ERROR,               3,  1)
 FIELD(CSR, APP_CMD,                     5,  1)
 FIELD(CSR, FX_EVENT,                    6,  1)
+FIELD(CSR, SWITCH_ERROR,                7,  1)
 FIELD(CSR, READY_FOR_DATA,              8,  1)
 FIELD(CSR, CURRENT_STATE,               9,  4)
 FIELD(CSR, ERASE_RESET,                13,  1)
@@ -935,6 +936,43 @@ static uint32_t sd_wpbits(SDState *sd, uint64_t addr)
     return ret;
 }
 
+enum {
+    MMC_CMD6_ACCESS_COMMAND_SET = 0,
+    MMC_CMD6_ACCESS_SET_BITS,
+    MMC_CMD6_ACCESS_CLEAR_BITS,
+    MMC_CMD6_ACCESS_WRITE_BYTE,
+};
+
+static void mmc_function_switch(SDState *sd, uint32_t arg)
+{
+    uint32_t access = extract32(arg, 24, 2);
+    uint32_t index = extract32(arg, 16, 8);
+    uint32_t value = extract32(arg, 8, 8);
+    uint8_t b = sd->ext_csd[index];
+
+    switch (access) {
+    case MMC_CMD6_ACCESS_COMMAND_SET:
+        qemu_log_mask(LOG_UNIMP, "MMC Command set switching not supported\n");
+        return;
+    case MMC_CMD6_ACCESS_SET_BITS:
+        b |= value;
+        break;
+    case MMC_CMD6_ACCESS_CLEAR_BITS:
+        b &= ~value;
+        break;
+    case MMC_CMD6_ACCESS_WRITE_BYTE:
+        b = value;
+        break;
+    }
+
+    if (index >= 192) {
+        sd->card_status |= R_CSR_SWITCH_ERROR_MASK;
+        return;
+    }
+
+    sd->ext_csd[index] = b;
+}
+
 static void sd_function_switch(SDState *sd, uint32_t arg)
 {
     int i, mode, new_func;
@@ -2340,6 +2378,19 @@ static sd_rsp_type_t sd_emmc_cmd_SEND_TUNING_BLOCK(SDState *sd, SDRequest req)
     return sd_r1;
 }
 
+static sd_rsp_type_t sd_emmc_cmd_SWITCH_FUNCTION(SDState *sd, SDRequest req)
+{
+    switch (sd->state) {
+    case sd_transfer_state:
+        sd->state = sd_programming_state;
+        mmc_function_switch(sd, req.arg);
+        sd->state = sd_transfer_state;
+        return sd_r1b;
+    default:
+        return sd_invalid_state_for_cmd(sd, req);
+    }
+}
+
 static const SDProto sd_proto_emmc = {
     .name = "eMMC",
     .cmd = {
@@ -2348,6 +2399,7 @@ static const SDProto sd_proto_emmc = {
         [2]         = sd_emmc_cmd_ALL_SEND_CID,
         [3]         = sd_emmc_cmd_SEND_RELATIVE_ADDR,
         [5]         = sd_cmd_illegal,
+        [6]         = sd_emmc_cmd_SWITCH_FUNCTION,
         [8]         = sd_emmc_cmd_SEND_EXT_CSD,
         [19]        = sd_cmd_SEND_TUNING_BLOCK,
         [21]        = sd_emmc_cmd_SEND_TUNING_BLOCK,
-- 
2.34.1

[RFC PATCH 16/17] hw/sd: Support boot area in emmc image

[Cédric Le Goater]

From: Joel Stanley <joel@jms.id.au>

This assumes a specially constructued image:

  dd if=/dev/zero of=mmc-bootarea.img count=2 bs=1M
  dd if=u-boot-spl.bin of=mmc-bootarea.img conv=notrunc
  dd if=u-boot.bin of=mmc-bootarea.img conv=notrunc count=64 bs=1K
  cat mmc-bootarea.img obmc-phosphor-image.wic > mmc.img
  truncate --size 16GB mmc.img
  truncate --size 128MB mmc-bootarea.img

For now this still requires a mtd image to load the SPL:

  qemu-system-arm -M tacoma-bmc -nographic \
   -global driver=sd-card,property=emmc,value=true \
   -drive file=mmc.img,if=sd,index=2 \
   -drive file=mmc-bootarea.img,if=mtd,format=raw

Signed-off-by: Joel Stanley <joel@jms.id.au>
[clg: - changes on the definition names ]
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 include/hw/sd/sd.h |  1 +
 hw/sd/sd.c         | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index dfe661079b8a..02cfc8cecfac 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -133,6 +133,7 @@ struct SDCardClass {
     const struct SDProto *proto;
     uint8_t spec_version;
     void (*set_csd)(SDState *sd, uint64_t size);
+    uint32_t (*bootpart_offset)(SDState *sd);
 };
 
 #define TYPE_SD_BUS "sd-bus"
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index d3bda6c2aa6b..e975fcf18512 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -839,9 +839,40 @@ void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert)
     qemu_set_irq(insert, sd->blk ? blk_is_inserted(sd->blk) : 0);
 }
 
+/*
+ * This requires a disk image that has two boot partitions inserted at the
+ * beginning of it. The size of the boot partitions are configured in the
+ * ext_csd structure, which is hardcoded in qemu. They are currently set to
+ * 1MB each.
+ */
+static uint32_t sd_emmc_bootpart_offset(SDState *sd)
+{
+    unsigned int access = sd->ext_csd[EXT_CSD_PART_CONFIG] &
+        EXT_CSD_PART_CONFIG_ACC_MASK;
+    unsigned int boot_capacity = sd->ext_csd[EXT_CSD_BOOT_MULT] << 17;
+
+    switch (access) {
+    case EXT_CSD_PART_CONFIG_ACC_DEFAULT:
+        return boot_capacity * 2;
+    case EXT_CSD_PART_CONFIG_ACC_BOOT0:
+        return 0;
+    case EXT_CSD_PART_CONFIG_ACC_BOOT0 + 1:
+        return boot_capacity * 1;
+    default:
+         g_assert_not_reached();
+    }
+}
+
+static uint32_t sd_bootpart_offset(SDState *sd)
+{
+    SDCardClass *sc = SD_CARD_GET_CLASS(sd);
+    return sc->bootpart_offset ? sc->bootpart_offset(sd) : 0;
+}
+
 static void sd_blk_read(SDState *sd, uint64_t addr, uint32_t len)
 {
     trace_sdcard_read_block(addr, len);
+    addr += sd_bootpart_offset(sd);
     if (!sd->blk || blk_pread(sd->blk, addr, sd->data, len) < 0) {
         fprintf(stderr, "sd_blk_read: read error on host side\n");
     }
@@ -850,6 +881,7 @@ static void sd_blk_read(SDState *sd, uint64_t addr, uint32_t len)
 static void sd_blk_write(SDState *sd, uint64_t addr, uint32_t len)
 {
     trace_sdcard_write_block(addr, len);
+    addr += sd_bootpart_offset(sd);
     if (!sd->blk || blk_pwrite(sd->blk, addr, sd->data, len, 0) < 0) {
         fprintf(stderr, "sd_blk_write: write error on host side\n");
     }
@@ -2546,6 +2578,7 @@ static void emmc_class_init(ObjectClass *klass, void *data)
     sc->proto = &sd_proto_emmc;
     sc->spec_version = SD_PHY_SPECv3_01_VERS; /* eMMC requirement */
     sc->set_csd = sd_emmc_set_csd;
+    sc->bootpart_offset = sd_emmc_bootpart_offset;
 }
 
 static const TypeInfo emmc_info = {
-- 
2.34.1

[RFC PATCH 17/17] hw/sd: Subtract bootarea size from blk

[Cédric Le Goater]

From: Joel Stanley <joel@jms.id.au>

The userdata size is derived from the file the user passes on the
command line, but we must take into account the boot areas.

Signed-off-by: Joel Stanley <joel@jms.id.au>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/sd/sd.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index e975fcf18512..2ac721bea026 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -644,6 +644,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;
 
@@ -655,6 +656,11 @@ static void sd_reset(DeviceState *dev)
     }
     size = sect << 9;
 
+    if (sc->bootpart_offset) {
+        unsigned int boot_capacity = sd->ext_csd[EXT_CSD_BOOT_MULT] << 17;
+        size -= boot_capacity * 2;
+    }
+
     sect = sd_addr_to_wpnum(size) + 1;
 
     sd->state = sd_idle_state;
-- 
2.34.1

Re: [RFC PATCH 17/17] hw/sd: Subtract bootarea size from blk

[Philippe Mathieu-Daudé via]

Hi Joel,

On 18/3/22 14:28, Cédric Le Goater wrote:
> From: Joel Stanley <joel@jms.id.au>
> 
> The userdata size is derived from the file the user passes on the
> command line, but we must take into account the boot areas.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>   hw/sd/sd.c | 6 ++++++
>   1 file changed, 6 insertions(+)

> @@ -655,6 +656,11 @@ static void sd_reset(DeviceState *dev)
>       }
>       size = sect << 9;
>   
> +    if (sc->bootpart_offset) {
> +        unsigned int boot_capacity = sd->ext_csd[EXT_CSD_BOOT_MULT] << 17;

What about adding a tiny helper?

static unsigned sd_boot_capacity_bytes(SDState *sd)
{
     return sd->ext_csd[EXT_CSD_BOOT_MULT] << 18;
}

> +        size -= boot_capacity * 2;
> +    }
> +
>       sect = sd_addr_to_wpnum(size) + 1;
>   
>       sd->state = sd_idle_state;

Re: [RFC PATCH 00/17] hw/sd: Rework models for eMMC support

[Cédric Le Goater]

Hello,

On 3/18/22 14:28, Cédric Le Goater wrote:
> Hello Philippe,
> 
> I am restarting the discussion we started in :
> 
>    http://patchwork.ozlabs.org/project/qemu-devel/list/?series=250563
> 
> This series adds an extension for a new eMMC device using the
> framework you put in place. It's not perfect but we are getting close.
> The SPI variant would need its own class I suppose and there are more
> cleanups to be done in the set of commands. Please comment !


Since patch 01-09 are nearly all reviewed, may be we could start by
merging those ?

Thanks,

C.