[Qemu-devel] [PATCH v2 00/25] SDCard: housekeeping, improve SPI, introduce new Specs

[Qemu-devel] [PATCH v2 00/25] SDCard: housekeeping, improve SPI, introduce new Specs

[Philippe Mathieu-Daudé]

Hi,

This series intends to simplify the sdcard device:
- add trace points,
- have a more robust SD mode,
- improve the SPI mode,
- introduce the MMC mode (split for a later series)

3 patches as tagged RFC as I need to fix few XXX comments,
checking back the specs.

Since v1:
- rewrote mostly all patches to keep it simpler.

Regards,

Phil.

$ git backport-diff
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/25:[down] 'sdcard: reorder SDState struct members'
002/25:[0026] [FC] 'sdcard: replace DPRINTF() by trace events'
003/25:[----] [-C] 'sdcard: add more trace events'
004/25:[down] 'sdcard: define SDCARD_CMD_MAX instead of using the magic '64''
005/25:[down] 'sdcard: display command name when tracing CMD/ACMD'
006/25:[----] [-C] 'sdcard: let cmd_valid_while_locked() returns a bool'
007/25:[down] 'sdcard: rename sd_set_$REG() functions called once as sd_reset_$REG()'
008/25:[down] 'sdcard: use the registerfields API to access the OCR register'
009/25:[down] 'sdcard: use G_BYTE from cutils'
010/25:[down] 'sdcard: remove unreachable code'
011/25:[down] 'sdcard: replace switch(unique case) statements -> if(case)'
012/25:[down] 'sdcard: use a 16-bit integer for the 16-bit RCA register'
013/25:[down] 'sdcard: let function handling response codes returns the response size'
014/25:[down] 'sdcard: add missing command CMD55'
015/25:[down] 'sdcard: add missing CMD54 SDIO command'
016/25:[down] 'sdcard: add missing SPI legal commands'
017/25:[down] 'sdcard: fix SPI response length'
018/25:[down] 'sdcard: add an enum for the SD PHY Spec version'
019/25:[down] 'sdcard: check if the card capacity is supported'
020/25:[down] 'sdcard: Don't always set the high capacity bit'
021/25:[down] 'sdcard: add cmd_version_supported(), improve cmd_class_supported()'
022/25:[down] 'sdcard: remove unreachable SPI commands'
023/25:[down] 'sdcard: store the bus protocol in an enum'
024/25:[down] 'sdcard: introduce Spec v3.01 SD commands'
025/25:[down] 'sdcard: introduce Spec v4.51 & v5.1 MMC commands'

Based-on: 20180103183418.23730-26-f4bug@amsat.org
          (SDHCI: add qtests and fix few issues)

Philippe Mathieu-Daudé (25):
  sdcard: reorder SDState struct members
  sdcard: replace DPRINTF() by trace events
  sdcard: add more trace events
  sdcard: define SDCARD_CMD_MAX instead of using the magic '64'
  sdcard: display command name when tracing CMD/ACMD
  sdcard: let cmd_valid_while_locked() returns a bool
  sdcard: rename sd_set_$REG() functions called once as sd_reset_$REG()
  sdcard: use the registerfields API to access the OCR register
  sdcard: use G_BYTE from cutils
  sdcard: remove unreachable code
  sdcard: replace switch(unique case) statements -> if(case)
  sdcard: use a 16-bit integer for the 16-bit RCA register
  sdcard: let function handling response codes returns the response size
  sdcard: add missing command CMD55
  sdcard: add missing CMD54 SDIO command
  sdcard: add missing SPI legal commands
  sdcard: fix SPI response length
  sdcard: add an enum for the SD PHY Spec version
  sdcard: check if the card capacity is supported
  sdcard: Don't always set the high capacity bit
  sdcard: add cmd_version_supported(), improve cmd_class_supported()
  sdcard: remove unreachable SPI commands
  sdcard: store the bus protocol in an enum
  sdcard: introduce Spec v3.01 SD commands
  sdcard: introduce Spec v4.51 & v5.1 MMC commands

 include/hw/sd/sd.h            |   6 +
 include/hw/sd/sdcard_legacy.h |   2 +-
 hw/sd/sd.c                    | 872 +++++++++++++++++++++++++++++-------------
 hw/sd/trace-events            |  15 +
 4 files changed, 627 insertions(+), 268 deletions(-)

-- 
2.15.1

[Qemu-devel] [PATCH v2 01/25] sdcard: reorder SDState struct members

[Philippe Mathieu-Daudé]

place card registers first, this will ease further code movements.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 7755bedfa0..180c43e218 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -89,16 +89,15 @@ enum SDCardStates {
 struct SDState {
     DeviceState parent_obj;
 
-    uint32_t mode;    /* current card mode, one of SDCardModes */
-    int32_t state;    /* current card state, one of SDCardStates */
+    /* SD Memory Card Registers */
     uint32_t ocr;
-    QEMUTimer *ocr_power_timer;
     uint8_t scr[8];
     uint8_t cid[16];
     uint8_t csd[16];
     uint16_t rca;
     uint32_t card_status;
     uint8_t sd_status[64];
+
     uint32_t vhs;
     bool wp_switch;
     unsigned long *wp_groups;
@@ -113,6 +112,9 @@ struct SDState {
     uint8_t function_group[6];
 
     bool spi;
+
+    uint32_t mode;    /* current card mode, one of SDCardModes */
+    int32_t state;    /* current card state, one of SDCardStates */
     uint8_t current_cmd;
     /* True if we will handle the next command as an ACMD. Note that this does
      * *not* track the APP_CMD status bit!
@@ -125,6 +127,7 @@ struct SDState {
     qemu_irq readonly_cb;
     qemu_irq inserted_cb;
     BlockBackend *blk;
+    QEMUTimer *ocr_power_timer;
 
     bool enable;
 };
-- 
2.15.1

Re: [Qemu-devel] [PATCH v2 01/25] sdcard: reorder SDState struct members

[Alistair Francis]

On Wed, Jan 3, 2018 at 1:24 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> place card registers first, this will ease further code movements.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Alistair

> ---
>  hw/sd/sd.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 7755bedfa0..180c43e218 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -89,16 +89,15 @@ enum SDCardStates {
>  struct SDState {
>      DeviceState parent_obj;
>
> -    uint32_t mode;    /* current card mode, one of SDCardModes */
> -    int32_t state;    /* current card state, one of SDCardStates */
> +    /* SD Memory Card Registers */
>      uint32_t ocr;
> -    QEMUTimer *ocr_power_timer;
>      uint8_t scr[8];
>      uint8_t cid[16];
>      uint8_t csd[16];
>      uint16_t rca;
>      uint32_t card_status;
>      uint8_t sd_status[64];
> +
>      uint32_t vhs;
>      bool wp_switch;
>      unsigned long *wp_groups;
> @@ -113,6 +112,9 @@ struct SDState {
>      uint8_t function_group[6];
>
>      bool spi;
> +
> +    uint32_t mode;    /* current card mode, one of SDCardModes */
> +    int32_t state;    /* current card state, one of SDCardStates */
>      uint8_t current_cmd;
>      /* True if we will handle the next command as an ACMD. Note that this does
>       * *not* track the APP_CMD status bit!
> @@ -125,6 +127,7 @@ struct SDState {
>      qemu_irq readonly_cb;
>      qemu_irq inserted_cb;
>      BlockBackend *blk;
> +    QEMUTimer *ocr_power_timer;
>
>      bool enable;
>  };
> --
> 2.15.1
>
>

[Qemu-devel] [PATCH v2 02/25] sdcard: replace DPRINTF() by trace events

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c         | 31 +++++++++++++++++++++++++------
 hw/sd/trace-events |  6 ++++++
 2 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 180c43e218..df2e099151 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -41,6 +41,7 @@
 #include "qemu/error-report.h"
 #include "qemu/timer.h"
 #include "qemu/log.h"
+#include "trace.h"
 
 //#define DEBUG_SD 1
 
@@ -132,6 +133,25 @@ struct SDState {
     bool enable;
 };
 
+static const char *sd_state_name(enum SDCardStates state)
+{
+    static const char *state_name[] = {
+        [sd_idle_state]             = "idle",
+        [sd_ready_state]            = "ready",
+        [sd_identification_state]   = "identification",
+        [sd_standby_state]          = "standby",
+        [sd_transfer_state]         = "transfer",
+        [sd_sendingdata_state]      = "sendingdata",
+        [sd_receivingdata_state]    = "receivingdata",
+        [sd_programming_state]      = "programming",
+        [sd_disconnect_state]       = "disconnect",
+    };
+    if (state == sd_inactive_state) {
+        return "inactive";
+    }
+    return state_name[state];
+}
+
 static void sd_set_mode(SDState *sd)
 {
     switch (sd->state) {
@@ -752,6 +772,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
     uint32_t rca = 0x0000;
     uint64_t addr = (sd->ocr & (1 << 30)) ? (uint64_t) req.arg << 9 : req.arg;
 
+    trace_sdcard_normal_command(req.cmd, req.arg, sd_state_name(sd->state));
+
     /* Not interpreting this as an app command */
     sd->card_status &= ~APP_CMD;
 
@@ -766,7 +788,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
         sd->multi_blk_cnt = 0;
     }
 
-    DPRINTF("CMD%d 0x%08x state %d\n", req.cmd, req.arg, sd->state);
     switch (req.cmd) {
     /* Basic commands (Class 0 and Class 1) */
     case 0:	/* CMD0:   GO_IDLE_STATE */
@@ -1286,8 +1307,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
         return sd_r1;
 
     case 56:	/* CMD56:  GEN_CMD */
-        fprintf(stderr, "SD: GEN_CMD 0x%08x\n", req.arg);
-
         switch (sd->state) {
         case sd_transfer_state:
             sd->data_offset = 0;
@@ -1321,7 +1340,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
 static sd_rsp_type_t sd_app_command(SDState *sd,
                                     SDRequest req)
 {
-    DPRINTF("ACMD%d 0x%08x\n", req.cmd, req.arg);
+    trace_sdcard_app_command(req.cmd, req.arg);
     sd->card_status |= APP_CMD;
     switch (req.cmd) {
     case 6:	/* ACMD6:  SET_BUS_WIDTH */
@@ -1581,8 +1600,7 @@ send_response:
 
 static void sd_blk_read(SDState *sd, uint64_t addr, uint32_t len)
 {
-    DPRINTF("sd_blk_read: addr = 0x%08llx, len = %d\n",
-            (unsigned long long) addr, len);
+    trace_sdcard_read_block(addr, len);
     if (!sd->blk || blk_pread(sd->blk, addr, sd->data, len) < 0) {
         fprintf(stderr, "sd_blk_read: read error on host side\n");
     }
@@ -1590,6 +1608,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);
     if (!sd->blk || blk_pwrite(sd->blk, addr, sd->data, len, 0) < 0) {
         fprintf(stderr, "sd_blk_write: write error on host side\n");
     }
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index e9f0ea6044..86c1687572 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -15,6 +15,12 @@ sdhci_read_dataport(uint16_t data_count) "all %u bytes of data have been read fr
 sdhci_write_dataport(uint16_t data_count) "write buffer filled with %u bytes of data"
 sdhci_led(bool state) "LED: %u"
 
+# hw/sd/sd.c
+sdcard_normal_command(uint8_t cmd, uint32_t arg, const char *state) "CMD%d arg 0x%08x (state %s)"
+sdcard_app_command(uint8_t acmd, uint32_t arg) "ACMD%d arg 0x%08x"
+sdcard_read_block(uint64_t addr, uint32_t len) "addr 0x%" PRIx64 " size 0x%x"
+sdcard_write_block(uint64_t addr, uint32_t len) "addr 0x%" PRIx64 " size 0x%x"
+
 # hw/sd/milkymist-memcard.c
 milkymist_memcard_memory_read(uint32_t addr, uint32_t value) "addr 0x%08x value 0x%08x"
 milkymist_memcard_memory_write(uint32_t addr, uint32_t value) "addr 0x%08x value 0x%08x"
-- 
2.15.1

[Qemu-devel] [PATCH v2 03/25] sdcard: add more trace events

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c         | 12 ++++++++++++
 hw/sd/trace-events |  8 ++++++++
 2 files changed, 20 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index df2e099151..f0eaac4d42 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -440,6 +440,7 @@ static void sd_reset(DeviceState *dev)
     uint64_t size;
     uint64_t sect;
 
+    trace_sdcard_reset();
     if (sd->blk) {
         blk_get_geometry(sd->blk, &sect);
     } else {
@@ -491,7 +492,10 @@ static void sd_cardchange(void *opaque, bool load, Error **errp)
     bool readonly = sd_get_readonly(sd);
 
     if (inserted) {
+        trace_sdcard_inserted(readonly);
         sd_reset(dev);
+    } else {
+        trace_sdcard_ejected();
     }
 
     /* The IRQ notification is for legacy non-QOM SD controller devices;
@@ -623,6 +627,7 @@ static void sd_erase(SDState *sd)
     uint64_t erase_start = sd->erase_start;
     uint64_t erase_end = sd->erase_end;
 
+    trace_sdcard_erase();
     if (!sd->erase_start || !sd->erase_end) {
         sd->card_status |= ERASE_SEQ_ERROR;
         return;
@@ -712,6 +717,11 @@ static void sd_lock_command(SDState *sd)
     else
         pwd_len = 0;
 
+    if (lock) {
+        trace_sdcard_lock();
+    } else {
+        trace_sdcard_unlock();
+    }
     if (erase) {
         if (!(sd->card_status & CARD_IS_LOCKED) || sd->blk_len > 1 ||
                         set_pwd || clr_pwd || lock || sd->wp_switch ||
@@ -1635,6 +1645,7 @@ void sd_write_data(SDState *sd, uint8_t value)
     if (sd->card_status & (ADDRESS_ERROR | WP_VIOLATION))
         return;
 
+    trace_sdcard_write_data(value);
     switch (sd->current_cmd) {
     case 24:	/* CMD24:  WRITE_SINGLE_BLOCK */
         sd->data[sd->data_offset ++] = value;
@@ -1772,6 +1783,7 @@ uint8_t sd_read_data(SDState *sd)
 
     io_len = (sd->ocr & (1 << 30)) ? 512 : sd->blk_len;
 
+    trace_sdcard_read_data(io_len);
     switch (sd->current_cmd) {
     case 6:	/* CMD6:   SWITCH_FUNCTION */
         ret = sd->data[sd->data_offset ++];
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index 86c1687572..727d6fd904 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -18,8 +18,16 @@ sdhci_led(bool state) "LED: %u"
 # hw/sd/sd.c
 sdcard_normal_command(uint8_t cmd, uint32_t arg, const char *state) "CMD%d arg 0x%08x (state %s)"
 sdcard_app_command(uint8_t acmd, uint32_t arg) "ACMD%d arg 0x%08x"
+sdcard_reset(void) ""
+sdcard_inserted(bool readonly) "read_only: %d"
+sdcard_ejected(void) ""
+sdcard_erase(void) ""
+sdcard_lock(void) ""
+sdcard_unlock(void) ""
 sdcard_read_block(uint64_t addr, uint32_t len) "addr 0x%" PRIx64 " size 0x%x"
 sdcard_write_block(uint64_t addr, uint32_t len) "addr 0x%" PRIx64 " size 0x%x"
+sdcard_write_data(uint8_t value) "value 0x%02x"
+sdcard_read_data(int length) "len %d"
 
 # hw/sd/milkymist-memcard.c
 milkymist_memcard_memory_read(uint32_t addr, uint32_t value) "addr 0x%08x value 0x%08x"
-- 
2.15.1

[Qemu-devel] [PATCH v2 04/25] sdcard: define SDCARD_CMD_MAX instead of using the magic '64'

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index f0eaac4d42..489d13681d 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -56,6 +56,8 @@ do { fprintf(stderr, "SD: " fmt , ## __VA_ARGS__); } while (0)
 #define OCR_POWER_UP            0x80000000
 #define OCR_POWER_DELAY_NS      500000 /* 0.5ms */
 
+#define SDCARD_CMD_MAX 64
+
 typedef enum {
     sd_r0 = 0,    /* no response */
     sd_r1,        /* normal response command */
@@ -176,18 +178,21 @@ static void sd_set_mode(SDState *sd)
     }
 }
 
-static const sd_cmd_type_t sd_cmd_type[64] = {
+static const sd_cmd_type_t sd_cmd_type[SDCARD_CMD_MAX] = {
     sd_bc,   sd_none, sd_bcr,  sd_bcr,  sd_none, sd_none, sd_none, sd_ac,
     sd_bcr,  sd_ac,   sd_ac,   sd_adtc, sd_ac,   sd_ac,   sd_none, sd_ac,
+    /* 16 */
     sd_ac,   sd_adtc, sd_adtc, sd_none, sd_none, sd_none, sd_none, sd_none,
     sd_adtc, sd_adtc, sd_adtc, sd_adtc, sd_ac,   sd_ac,   sd_adtc, sd_none,
+    /* 32 */
     sd_ac,   sd_ac,   sd_none, sd_none, sd_none, sd_none, sd_ac,   sd_none,
     sd_none, sd_none, sd_bc,   sd_none, sd_none, sd_none, sd_none, sd_none,
+    /* 48 */
     sd_none, sd_none, sd_none, sd_none, sd_none, sd_none, sd_none, sd_ac,
     sd_adtc, sd_none, sd_none, sd_none, sd_none, sd_none, sd_none, sd_none,
 };
 
-static const int sd_cmd_class[64] = {
+static const int sd_cmd_class[SDCARD_CMD_MAX] = {
     0,  0,  0,  0,  0,  9, 10,  0,  0,  0,  0,  1,  0,  0,  0,  0,
     2,  2,  2,  2,  3,  3,  3,  3,  4,  4,  4,  4,  6,  6,  6,  6,
     5,  5, 10, 10, 10, 10,  5,  9,  9,  9,  7,  7,  7,  7,  7,  7,
@@ -787,8 +792,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
     /* Not interpreting this as an app command */
     sd->card_status &= ~APP_CMD;
 
-    if (sd_cmd_type[req.cmd & 0x3F] == sd_ac
-        || sd_cmd_type[req.cmd & 0x3F] == sd_adtc) {
+    if (sd_cmd_type[req.cmd] == sd_ac
+        || sd_cmd_type[req.cmd] == sd_adtc) {
         rca = req.arg >> 16;
     }
 
@@ -1495,8 +1500,8 @@ static int cmd_valid_while_locked(SDState *sd, SDRequest *req)
     if (req->cmd == 16 || req->cmd == 55) {
         return 1;
     }
-    return sd_cmd_class[req->cmd & 0x3F] == 0
-            || sd_cmd_class[req->cmd & 0x3F] == 7;
+    return sd_cmd_class[req->cmd] == 0
+            || sd_cmd_class[req->cmd] == 7;
 }
 
 int sd_do_command(SDState *sd, SDRequest *req,
@@ -1515,6 +1520,12 @@ int sd_do_command(SDState *sd, SDRequest *req,
         goto send_response;
     }
 
+    if (req->cmd >= SDCARD_CMD_MAX) {
+        qemu_log_mask(LOG_GUEST_ERROR, "SD: incorrect command 0x%02x\n",
+                      req->cmd);
+        req->cmd &= 0x3f;
+    }
+
     if (sd->card_status & CARD_IS_LOCKED) {
         if (!cmd_valid_while_locked(sd, req)) {
             sd->card_status |= ILLEGAL_COMMAND;
-- 
2.15.1

Re: [Qemu-devel] [PATCH v2 04/25] sdcard: define SDCARD_CMD_MAX instead of using the magic '64'

[Alistair Francis]

On Wed, Jan 3, 2018 at 1:24 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Alistair

> ---
>  hw/sd/sd.c | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index f0eaac4d42..489d13681d 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -56,6 +56,8 @@ do { fprintf(stderr, "SD: " fmt , ## __VA_ARGS__); } while (0)
>  #define OCR_POWER_UP            0x80000000
>  #define OCR_POWER_DELAY_NS      500000 /* 0.5ms */
>
> +#define SDCARD_CMD_MAX 64
> +
>  typedef enum {
>      sd_r0 = 0,    /* no response */
>      sd_r1,        /* normal response command */
> @@ -176,18 +178,21 @@ static void sd_set_mode(SDState *sd)
>      }
>  }
>
> -static const sd_cmd_type_t sd_cmd_type[64] = {
> +static const sd_cmd_type_t sd_cmd_type[SDCARD_CMD_MAX] = {
>      sd_bc,   sd_none, sd_bcr,  sd_bcr,  sd_none, sd_none, sd_none, sd_ac,
>      sd_bcr,  sd_ac,   sd_ac,   sd_adtc, sd_ac,   sd_ac,   sd_none, sd_ac,
> +    /* 16 */
>      sd_ac,   sd_adtc, sd_adtc, sd_none, sd_none, sd_none, sd_none, sd_none,
>      sd_adtc, sd_adtc, sd_adtc, sd_adtc, sd_ac,   sd_ac,   sd_adtc, sd_none,
> +    /* 32 */
>      sd_ac,   sd_ac,   sd_none, sd_none, sd_none, sd_none, sd_ac,   sd_none,
>      sd_none, sd_none, sd_bc,   sd_none, sd_none, sd_none, sd_none, sd_none,
> +    /* 48 */
>      sd_none, sd_none, sd_none, sd_none, sd_none, sd_none, sd_none, sd_ac,
>      sd_adtc, sd_none, sd_none, sd_none, sd_none, sd_none, sd_none, sd_none,
>  };
>
> -static const int sd_cmd_class[64] = {
> +static const int sd_cmd_class[SDCARD_CMD_MAX] = {
>      0,  0,  0,  0,  0,  9, 10,  0,  0,  0,  0,  1,  0,  0,  0,  0,
>      2,  2,  2,  2,  3,  3,  3,  3,  4,  4,  4,  4,  6,  6,  6,  6,
>      5,  5, 10, 10, 10, 10,  5,  9,  9,  9,  7,  7,  7,  7,  7,  7,
> @@ -787,8 +792,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
>      /* Not interpreting this as an app command */
>      sd->card_status &= ~APP_CMD;
>
> -    if (sd_cmd_type[req.cmd & 0x3F] == sd_ac
> -        || sd_cmd_type[req.cmd & 0x3F] == sd_adtc) {
> +    if (sd_cmd_type[req.cmd] == sd_ac
> +        || sd_cmd_type[req.cmd] == sd_adtc) {
>          rca = req.arg >> 16;
>      }
>
> @@ -1495,8 +1500,8 @@ static int cmd_valid_while_locked(SDState *sd, SDRequest *req)
>      if (req->cmd == 16 || req->cmd == 55) {
>          return 1;
>      }
> -    return sd_cmd_class[req->cmd & 0x3F] == 0
> -            || sd_cmd_class[req->cmd & 0x3F] == 7;
> +    return sd_cmd_class[req->cmd] == 0
> +            || sd_cmd_class[req->cmd] == 7;
>  }
>
>  int sd_do_command(SDState *sd, SDRequest *req,
> @@ -1515,6 +1520,12 @@ int sd_do_command(SDState *sd, SDRequest *req,
>          goto send_response;
>      }
>
> +    if (req->cmd >= SDCARD_CMD_MAX) {
> +        qemu_log_mask(LOG_GUEST_ERROR, "SD: incorrect command 0x%02x\n",
> +                      req->cmd);
> +        req->cmd &= 0x3f;
> +    }
> +
>      if (sd->card_status & CARD_IS_LOCKED) {
>          if (!cmd_valid_while_locked(sd, req)) {
>              sd->card_status |= ILLEGAL_COMMAND;
> --
> 2.15.1
>
>

[Qemu-devel] [PATCH v2 05/25] sdcard: display command name when tracing CMD/ACMD

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c         | 75 +++++++++++++++++++++++++++++++++++++++++++++++++++---
 hw/sd/trace-events |  4 +--
 2 files changed, 73 insertions(+), 6 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 489d13681d..7bb5c6fb5e 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -133,6 +133,7 @@ struct SDState {
     QEMUTimer *ocr_power_timer;
 
     bool enable;
+    const char *proto_name;
 };
 
 static const char *sd_state_name(enum SDCardStates state)
@@ -154,6 +155,66 @@ static const char *sd_state_name(enum SDCardStates state)
     return state_name[state];
 }
 
+static const char *sd_cmd_abbreviation(uint8_t cmd)
+{
+    static const char *cmd_abbrev[SDCARD_CMD_MAX] = {
+         [0]    = "GO_IDLE_STATE",
+         [2]    = "ALL_SEND_CID",            [3]    = "SEND_RELATIVE_ADDR",
+         [4]    = "SET_DSR",                 [5]    = "IO_SEND_OP_COND",
+         [6]    = "SWITCH_FUNC",             [7]    = "SELECT/DESELECT_CARD",
+         [8]    = "SEND_IF_COND",            [9]    = "SEND_CSD",
+        [10]    = "SEND_CID",               [11]    = "VOLTAGE_SWITCH",
+        [12]    = "STOP_TRANSMISSION",      [13]    = "SEND_STATUS",
+                                            [15]    = "GO_INACTIVE_STATE",
+        [16]    = "SET_BLOCKLEN",           [17]    = "READ_SINGLE_BLOCK",
+        [18]    = "READ_MULTIPLE_BLOCK",    [19]    = "SEND_TUNING_BLOCK",
+        [20]    = "SPEED_CLASS_CONTROL",    [21]    = "DPS_spec",
+                                            [23]    = "SET_BLOCK_COUNT",
+        [24]    = "WRITE_BLOCK",            [25]    = "WRITE_MULTIPLE_BLOCK",
+        [26]    = "MANUF_RSVD",             [27]    = "PROGRAM_CSD",
+        [28]    = "SET_WRITE_PROT",         [29]    = "CLR_WRITE_PROT",
+        [30]    = "SEND_WRITE_PROT",
+        [32]    = "ERASE_WR_BLK_START",     [33]    = "ERASE_WR_BLK_END",
+        [34]    = "SW_FUNC_RSVD",           [35]    = "SW_FUNC_RSVD",
+        [36]    = "SW_FUNC_RSVD",           [37]    = "SW_FUNC_RSVD",
+        [38]    = "ERASE",
+        [40]    = "DPS_spec",
+        [42]    = "LOCK_UNLOCK",            [43]    = "Q_MANAGEMENT",
+        [44]    = "Q_TASK_INFO_A",          [45]    = "Q_TASK_INFO_B",
+        [46]    = "Q_RD_TASK",              [47]    = "Q_WR_TASK",
+        [48]    = "READ_EXTR_SINGLE",       [49]    = "WRITE_EXTR_SINGLE",
+        [50]    = "SW_FUNC_RSVD", /* FIXME */
+        [52]    = "IO_RW_DIRECT",           [53]    = "IO_RW_EXTENDED",
+        [54]    = "SDIO_RSVD",              [55]    = "APP_CMD",
+        [56]    = "GEN_CMD",                [57]    = "SW_FUNC_RSVD",
+        [58]    = "READ_EXTR_MULTI",        [59]    = "WRITE_EXTR_MULTI",
+        [60]    = "MANUF_RSVD",             [61]    = "MANUF_RSVD",
+        [62]    = "MANUF_RSVD",             [63]    = "MANUF_RSVD",
+    };
+    return cmd_abbrev[cmd] ? cmd_abbrev[cmd] : "UNKNOWN_CMD";
+}
+
+static const char *sd_acmd_abbreviation(uint8_t cmd)
+{
+    static const char *acmd_abbrev[SDCARD_CMD_MAX] = {
+         [6] = "SET_BUS_WIDTH",
+        [13] = "SD_STATUS",
+        [14] = "DPS_spec",                  [15] = "DPS_spec",
+        [16] = "DPS_spec",
+        [18] = "SECU_spec",
+        [22] = "SEND_NUM_WR_BLOCKS",        [23] = "SET_WR_BLK_ERASE_COUNT",
+        [41] = "SD_SEND_OP_COND",
+        [42] = "SET_CLR_CARD_DETECT",
+        [51] = "SEND_SCR",
+        [52] = "SECU_spec",                 [53] = "SECU_spec",
+        [54] = "SECU_spec",
+        [56] = "SECU_spec",                 [57] = "SECU_spec",
+        [58] = "SECU_spec",                 [59] = "SECU_spec",
+    };
+
+    return acmd_abbrev[cmd] ? acmd_abbrev[cmd] : "UNKNOWN_ACMD";
+}
+
 static void sd_set_mode(SDState *sd)
 {
     switch (sd->state) {
@@ -781,13 +842,16 @@ static void sd_lock_command(SDState *sd)
         sd->card_status &= ~CARD_IS_LOCKED;
 }
 
-static sd_rsp_type_t sd_normal_command(SDState *sd,
-                                       SDRequest req)
+static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 {
     uint32_t rca = 0x0000;
     uint64_t addr = (sd->ocr & (1 << 30)) ? (uint64_t) req.arg << 9 : req.arg;
 
-    trace_sdcard_normal_command(req.cmd, req.arg, sd_state_name(sd->state));
+    if (req.cmd != 55 || sd->expecting_acmd) {
+        trace_sdcard_normal_command(sd->proto_name,
+                                    sd_cmd_abbreviation(req.cmd), req.cmd,
+                                    req.arg, sd_state_name(sd->state));
+    }
 
     /* Not interpreting this as an app command */
     sd->card_status &= ~APP_CMD;
@@ -1355,7 +1419,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
 static sd_rsp_type_t sd_app_command(SDState *sd,
                                     SDRequest req)
 {
-    trace_sdcard_app_command(req.cmd, req.arg);
+    trace_sdcard_app_command(sd->proto_name, sd_acmd_abbreviation(req.cmd),
+                             req.cmd, req.arg, sd_state_name(sd->state));
     sd->card_status |= APP_CMD;
     switch (req.cmd) {
     case 6:	/* ACMD6:  SET_BUS_WIDTH */
@@ -1935,6 +2000,8 @@ static void sd_realize(DeviceState *dev, Error **errp)
     SDState *sd = SD_CARD(dev);
     int ret;
 
+    sd->proto_name = sd->spi ? "SPI" : "SD";
+
     if (sd->blk && blk_is_read_only(sd->blk)) {
         error_setg(errp, "Cannot use read-only drive as SD card");
         return;
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index 727d6fd904..25ab339f97 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -16,8 +16,8 @@ sdhci_write_dataport(uint16_t data_count) "write buffer filled with %u bytes of
 sdhci_led(bool state) "LED: %u"
 
 # hw/sd/sd.c
-sdcard_normal_command(uint8_t cmd, uint32_t arg, const char *state) "CMD%d arg 0x%08x (state %s)"
-sdcard_app_command(uint8_t acmd, uint32_t arg) "ACMD%d arg 0x%08x"
+sdcard_normal_command(const char *proto, const char *cmd_desc, uint8_t cmd, uint32_t arg, const char *state) "%s %20s/ CMD%02d arg 0x%08x (state %s)"
+sdcard_app_command(const char *proto, const char *acmd_desc, uint8_t acmd, uint32_t arg, const char *state) "%s %23s/ACMD%02d arg 0x%08x (state %s)"
 sdcard_reset(void) ""
 sdcard_inserted(bool readonly) "read_only: %d"
 sdcard_ejected(void) ""
-- 
2.15.1

[Qemu-devel] [PATCH v2 06/25] sdcard: let cmd_valid_while_locked() returns a bool

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.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 7bb5c6fb5e..b0e54f69d8 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1549,7 +1549,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
     return sd_illegal;
 }
 
-static int cmd_valid_while_locked(SDState *sd, SDRequest *req)
+static bool cmd_valid_while_locked(SDState *sd, SDRequest *req)
 {
     /* Valid commands in locked state:
      * basic class (0)
@@ -1563,7 +1563,7 @@ static int cmd_valid_while_locked(SDState *sd, SDRequest *req)
         return req->cmd == 41 || req->cmd == 42;
     }
     if (req->cmd == 16 || req->cmd == 55) {
-        return 1;
+        return true;
     }
     return sd_cmd_class[req->cmd] == 0
             || sd_cmd_class[req->cmd] == 7;
-- 
2.15.1

[Qemu-devel] [PATCH v2 07/25] sdcard: rename sd_set_$REG() functions called once as sd_reset_$REG()

[Philippe Mathieu-Daudé]

All are only called once at reset.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index b0e54f69d8..7d43ab072b 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -293,7 +293,7 @@ static uint16_t sd_crc16(void *message, size_t width)
     return shift_reg;
 }
 
-static void sd_set_ocr(SDState *sd)
+static void sd_reset_ocr(SDState *sd)
 {
     /* All voltages OK, Standard Capacity SD Memory Card, not yet powered up */
     sd->ocr = 0x00ffff00;
@@ -308,7 +308,7 @@ static void sd_ocr_powerup(void *opaque)
     sd->ocr |= OCR_POWER_UP;
 }
 
-static void sd_set_scr(SDState *sd)
+static void sd_reset_scr(SDState *sd)
 {
     sd->scr[0] = 0x00;		/* SCR Structure */
     sd->scr[1] = 0x2f;		/* SD Security Support */
@@ -327,7 +327,7 @@ static void sd_set_scr(SDState *sd)
 #define MDT_YR	2006
 #define MDT_MON	2
 
-static void sd_set_cid(SDState *sd)
+static void sd_reset_cid(SDState *sd)
 {
     sd->cid[0] = MID;		/* Fake card manufacturer ID (MID) */
     sd->cid[1] = OID[0];	/* OEM/Application ID (OID) */
@@ -359,7 +359,7 @@ static const uint8_t sd_csd_rw_mask[16] = {
     0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0xfc, 0xfe,
 };
 
-static void sd_set_csd(SDState *sd, uint64_t size)
+static void sd_reset_csd(SDState *sd, uint64_t size)
 {
     uint32_t csize = (size >> (CMULT_SHIFT + HWBLOCK_SHIFT)) - 1;
     uint32_t sectsize = (1 << (SECTOR_SHIFT + 1)) - 1;
@@ -414,6 +414,11 @@ static void sd_set_csd(SDState *sd, uint64_t size)
     }
 }
 
+static void sd_reset_rca(SDState *sd)
+{
+    sd->rca = 0;
+}
+
 static void sd_set_rca(SDState *sd)
 {
     sd->rca += 0x4567;
@@ -428,12 +433,12 @@ static void sd_set_rca(SDState *sd)
 #define CARD_STATUS_B	0x00c01e00
 #define CARD_STATUS_C	0xfd39a028
 
-static void sd_set_cardstatus(SDState *sd)
+static void sd_reset_cardstatus(SDState *sd)
 {
     sd->card_status = 0x00000100;
 }
 
-static void sd_set_sdstatus(SDState *sd)
+static void sd_reset_sdstatus(SDState *sd)
 {
     memset(sd->sd_status, 0, 64);
 }
@@ -517,13 +522,13 @@ static void sd_reset(DeviceState *dev)
     sect = sd_addr_to_wpnum(size) + 1;
 
     sd->state = sd_idle_state;
-    sd->rca = 0x0000;
-    sd_set_ocr(sd);
-    sd_set_scr(sd);
-    sd_set_cid(sd);
-    sd_set_csd(sd, size);
-    sd_set_cardstatus(sd);
-    sd_set_sdstatus(sd);
+    sd_reset_rca(sd);
+    sd_reset_ocr(sd);
+    sd_reset_scr(sd);
+    sd_reset_cid(sd);
+    sd_reset_csd(sd, size);
+    sd_reset_cardstatus(sd);
+    sd_reset_sdstatus(sd);
 
     g_free(sd->wp_groups);
     sd->wp_switch = sd->blk ? blk_is_read_only(sd->blk) : false;
-- 
2.15.1

[Qemu-devel] [PATCH v2 08/25] sdcard: use the registerfields API to access the OCR register

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 7d43ab072b..1f209b8199 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -32,6 +32,7 @@
 #include "qemu/osdep.h"
 #include "hw/qdev.h"
 #include "hw/hw.h"
+#include "hw/registerfields.h"
 #include "sysemu/block-backend.h"
 #include "hw/sd/sd.h"
 #include "hw/sd/sdcard_legacy.h"
@@ -53,8 +54,6 @@ do { fprintf(stderr, "SD: " fmt , ## __VA_ARGS__); } while (0)
 #endif
 
 #define ACMD41_ENQUIRY_MASK     0x00ffffff
-#define OCR_POWER_UP            0x80000000
-#define OCR_POWER_DELAY_NS      500000 /* 0.5ms */
 
 #define SDCARD_CMD_MAX 64
 
@@ -293,6 +292,10 @@ static uint16_t sd_crc16(void *message, size_t width)
     return shift_reg;
 }
 
+FIELD(OCR, CARD_POWER_UP,              31, 1);
+
+#define OCR_POWER_DELAY_NS      500000 /* 0.5ms */
+
 static void sd_reset_ocr(SDState *sd)
 {
     /* All voltages OK, Standard Capacity SD Memory Card, not yet powered up */
@@ -303,9 +306,10 @@ static void sd_ocr_powerup(void *opaque)
 {
     SDState *sd = opaque;
 
-    /* Set powered up bit in OCR */
-    assert(!(sd->ocr & OCR_POWER_UP));
-    sd->ocr |= OCR_POWER_UP;
+    assert(!FIELD_EX32(sd->ocr, OCR, CARD_POWER_UP));
+
+    /* card power-up OK */
+    sd->ocr = FIELD_DP32(sd->ocr, OCR, CARD_POWER_UP, 1);
 }
 
 static void sd_reset_scr(SDState *sd)
@@ -594,7 +598,7 @@ static bool sd_ocr_vmstate_needed(void *opaque)
     SDState *sd = opaque;
 
     /* Include the OCR state (and timer) if it is not yet powered up */
-    return !(sd->ocr & OCR_POWER_UP);
+    return !FIELD_EX32(sd->ocr, OCR, CARD_POWER_UP);
 }
 
 static const VMStateDescription sd_ocr_vmstate = {
@@ -1494,7 +1498,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
              * UEFI, which sends an initial enquiry ACMD41, but
              * assumes that the card is in ready state as soon as it
              * sees the power up bit set. */
-            if (!(sd->ocr & OCR_POWER_UP)) {
+            if (!FIELD_EX32(sd->ocr, OCR, CARD_POWER_UP)) {
                 if ((req.arg & ACMD41_ENQUIRY_MASK) != 0) {
                     timer_del(sd->ocr_power_timer);
                     sd_ocr_powerup(sd);
-- 
2.15.1

[Qemu-devel] [PATCH v2 09/25] sdcard: use G_BYTE from cutils

[Philippe Mathieu-Daudé]

code is now easier to read.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 1f209b8199..d4aa7b4a92 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -38,6 +38,7 @@
 #include "hw/sd/sdcard_legacy.h"
 #include "qapi/error.h"
 #include "qemu/bitmap.h"
+#include "qemu/cutils.h"
 #include "hw/qdev-properties.h"
 #include "qemu/error-report.h"
 #include "qemu/timer.h"
@@ -369,7 +370,7 @@ static void sd_reset_csd(SDState *sd, uint64_t size)
     uint32_t sectsize = (1 << (SECTOR_SHIFT + 1)) - 1;
     uint32_t wpsize = (1 << (WPGROUP_SHIFT + 1)) - 1;
 
-    if (size <= 0x40000000) {	/* Standard Capacity SD */
+    if (size <= 1 * G_BYTE /* FIXME 2GB? */) { /* 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 */
-- 
2.15.1

Re: [Qemu-devel] [PATCH v2 09/25] sdcard: use G_BYTE from cutils

[Alistair Francis]

On Wed, Jan 3, 2018 at 1:24 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> code is now easier to read.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Alistair

> ---
>  hw/sd/sd.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/sd/sd.c b/hw/sd/sd.c
> index 1f209b8199..d4aa7b4a92 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -38,6 +38,7 @@
>  #include "hw/sd/sdcard_legacy.h"
>  #include "qapi/error.h"
>  #include "qemu/bitmap.h"
> +#include "qemu/cutils.h"
>  #include "hw/qdev-properties.h"
>  #include "qemu/error-report.h"
>  #include "qemu/timer.h"
> @@ -369,7 +370,7 @@ static void sd_reset_csd(SDState *sd, uint64_t size)
>      uint32_t sectsize = (1 << (SECTOR_SHIFT + 1)) - 1;
>      uint32_t wpsize = (1 << (WPGROUP_SHIFT + 1)) - 1;
>
> -    if (size <= 0x40000000) {  /* Standard Capacity SD */
> +    if (size <= 1 * G_BYTE /* FIXME 2GB? */) { /* 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 */
> --
> 2.15.1
>
>

[Qemu-devel] [PATCH v2 10/25] sdcard: remove unreachable code

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index d4aa7b4a92..c9fc6f5369 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1188,9 +1188,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
             goto unimplemented_cmd;
         switch (sd->state) {
         case sd_transfer_state:
-            /* Writing in SPI mode not implemented.  */
-            if (sd->spi)
-                break;
             sd->state = sd_receivingdata_state;
             sd->data_start = addr;
             sd->data_offset = 0;
@@ -1214,9 +1211,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
             goto unimplemented_cmd;
         switch (sd->state) {
         case sd_transfer_state:
-            /* Writing in SPI mode not implemented.  */
-            if (sd->spi)
-                break;
             sd->state = sd_receivingdata_state;
             sd->data_start = addr;
             sd->data_offset = 0;
-- 
2.15.1

[Qemu-devel] [PATCH v2 11/25] sdcard: replace switch(unique case) statements -> if(case)

[Philippe Mathieu-Daudé]

this shortens the sd_normal_command/sd_app_command() big functions

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 177 +++++++++++--------------------------------------------------
 1 file changed, 30 insertions(+), 147 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index c9fc6f5369..e0550cd174 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -880,34 +880,24 @@ 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:
+        if (sd->state != sd_inactive_state) {
             sd->state = sd_idle_state;
             sd_reset(DEVICE(sd));
-            return sd->spi ? sd_r1 : sd_r0;
         }
-        break;
+        return sd->spi ? sd_r1 : sd_r0;
 
     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:
+        if (sd->state == sd_ready_state) {
             sd->state = sd_identification_state;
             return sd_r2_i;
-
-        default:
-            break;
         }
         break;
 
@@ -944,16 +934,12 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
     case 6:	/* CMD6:   SWITCH_FUNCTION */
         if (sd->spi)
             goto bad_cmd;
-        switch (sd->mode) {
-        case sd_data_transfer_mode:
+        if (sd->mode == sd_data_transfer_mode) {
             sd_function_switch(sd, req.arg);
             sd->state = sd_sendingdata_state;
             sd->data_start = 0;
             sd->data_offset = 0;
             return sd_r1;
-
-        default:
-            break;
         }
         break;
 
@@ -1062,8 +1048,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
     case 11:	/* CMD11:  READ_DAT_UNTIL_STOP */
         if (sd->spi)
             goto bad_cmd;
-        switch (sd->state) {
-        case sd_transfer_state:
+        if (sd->state == sd_transfer_state) {
             sd->state = sd_sendingdata_state;
             sd->data_start = req.arg;
             sd->data_offset = 0;
@@ -1071,9 +1056,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
             if (sd->data_start + sd->blk_len > sd->size)
                 sd->card_status |= ADDRESS_ERROR;
             return sd_r0;
-
-        default:
-            break;
         }
         break;
 
@@ -1095,53 +1077,40 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         break;
 
     case 13:	/* CMD13:  SEND_STATUS */
-        switch (sd->mode) {
-        case sd_data_transfer_mode:
+        if (sd->mode == sd_data_transfer_mode) {
             if (sd->rca != rca)
                 return sd_r0;
 
             return sd_r1;
-
-        default:
-            break;
         }
         break;
 
     case 15:	/* CMD15:  GO_INACTIVE_STATE */
         if (sd->spi)
             goto bad_cmd;
-        switch (sd->mode) {
-        case sd_data_transfer_mode:
+        if (sd->mode == sd_data_transfer_mode) {
             if (sd->rca != rca)
                 return sd_r0;
 
             sd->state = sd_inactive_state;
             return sd_r0;
-
-        default:
-            break;
         }
         break;
 
     /* Block read commands (Classs 2) */
     case 16:	/* CMD16:  SET_BLOCKLEN */
-        switch (sd->state) {
-        case sd_transfer_state:
+        if (sd->state == sd_transfer_state) {
             if (req.arg > (1 << HWBLOCK_SHIFT))
                 sd->card_status |= BLOCK_LEN_ERROR;
             else
                 sd->blk_len = req.arg;
 
             return sd_r1;
-
-        default:
-            break;
         }
         break;
 
     case 17:	/* CMD17:  READ_SINGLE_BLOCK */
-        switch (sd->state) {
-        case sd_transfer_state:
+        if (sd->state == sd_transfer_state) {
             sd->state = sd_sendingdata_state;
             sd->data_start = addr;
             sd->data_offset = 0;
@@ -1149,15 +1118,11 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
             if (sd->data_start + sd->blk_len > sd->size)
                 sd->card_status |= ADDRESS_ERROR;
             return sd_r1;
-
-        default:
-            break;
         }
         break;
 
     case 18:	/* CMD18:  READ_MULTIPLE_BLOCK */
-        switch (sd->state) {
-        case sd_transfer_state:
+        if (sd->state == sd_transfer_state) {
             sd->state = sd_sendingdata_state;
             sd->data_start = addr;
             sd->data_offset = 0;
@@ -1165,20 +1130,13 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
             if (sd->data_start + sd->blk_len > sd->size)
                 sd->card_status |= ADDRESS_ERROR;
             return sd_r1;
-
-        default:
-            break;
         }
         break;
 
     case 23:    /* CMD23: SET_BLOCK_COUNT */
-        switch (sd->state) {
-        case sd_transfer_state:
+        if (sd->state == sd_transfer_state) {
             sd->multi_blk_cnt = req.arg;
             return sd_r1;
-
-        default:
-            break;
         }
         break;
 
@@ -1186,8 +1144,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
     case 24:	/* CMD24:  WRITE_SINGLE_BLOCK */
         if (sd->spi)
             goto unimplemented_cmd;
-        switch (sd->state) {
-        case sd_transfer_state:
+        if (sd->state == sd_transfer_state) {
             sd->state = sd_receivingdata_state;
             sd->data_start = addr;
             sd->data_offset = 0;
@@ -1200,17 +1157,13 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
             if (sd->csd[14] & 0x30)
                 sd->card_status |= WP_VIOLATION;
             return sd_r1;
-
-        default:
-            break;
         }
         break;
 
     case 25:	/* CMD25:  WRITE_MULTIPLE_BLOCK */
         if (sd->spi)
             goto unimplemented_cmd;
-        switch (sd->state) {
-        case sd_transfer_state:
+        if (sd->state == sd_transfer_state) {
             sd->state = sd_receivingdata_state;
             sd->data_start = addr;
             sd->data_offset = 0;
@@ -1223,46 +1176,34 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
             if (sd->csd[14] & 0x30)
                 sd->card_status |= WP_VIOLATION;
             return sd_r1;
-
-        default:
-            break;
         }
         break;
 
     case 26:	/* CMD26:  PROGRAM_CID */
         if (sd->spi)
             goto bad_cmd;
-        switch (sd->state) {
-        case sd_transfer_state:
+        if (sd->state == sd_transfer_state) {
             sd->state = sd_receivingdata_state;
             sd->data_start = 0;
             sd->data_offset = 0;
             return sd_r1;
-
-        default:
-            break;
         }
         break;
 
     case 27:	/* CMD27:  PROGRAM_CSD */
         if (sd->spi)
             goto unimplemented_cmd;
-        switch (sd->state) {
-        case sd_transfer_state:
+        if (sd->state == sd_transfer_state) {
             sd->state = sd_receivingdata_state;
             sd->data_start = 0;
             sd->data_offset = 0;
             return sd_r1;
-
-        default:
-            break;
         }
         break;
 
     /* Write protection (Class 6) */
     case 28:	/* CMD28:  SET_WRITE_PROT */
-        switch (sd->state) {
-        case sd_transfer_state:
+        if (sd->state == sd_transfer_state) {
             if (addr >= sd->size) {
                 sd->card_status |= ADDRESS_ERROR;
                 return sd_r1b;
@@ -1273,15 +1214,11 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
             /* Bzzzzzzztt .... Operation complete.  */
             sd->state = sd_transfer_state;
             return sd_r1b;
-
-        default:
-            break;
         }
         break;
 
     case 29:	/* CMD29:  CLR_WRITE_PROT */
-        switch (sd->state) {
-        case sd_transfer_state:
+        if (sd->state == sd_transfer_state) {
             if (addr >= sd->size) {
                 sd->card_status |= ADDRESS_ERROR;
                 return sd_r1b;
@@ -1292,52 +1229,36 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
             /* Bzzzzzzztt .... Operation complete.  */
             sd->state = sd_transfer_state;
             return sd_r1b;
-
-        default:
-            break;
         }
         break;
 
     case 30:	/* CMD30:  SEND_WRITE_PROT */
-        switch (sd->state) {
-        case sd_transfer_state:
+        if (sd->state == sd_transfer_state) {
             sd->state = sd_sendingdata_state;
             *(uint32_t *) sd->data = sd_wpbits(sd, req.arg);
             sd->data_start = addr;
             sd->data_offset = 0;
             return sd_r1b;
-
-        default:
-            break;
         }
         break;
 
     /* Erase commands (Class 5) */
     case 32:	/* CMD32:  ERASE_WR_BLK_START */
-        switch (sd->state) {
-        case sd_transfer_state:
+        if (sd->state == sd_transfer_state) {
             sd->erase_start = req.arg;
             return sd_r1;
-
-        default:
-            break;
         }
         break;
 
     case 33:	/* CMD33:  ERASE_WR_BLK_END */
-        switch (sd->state) {
-        case sd_transfer_state:
+        if (sd->state == sd_transfer_state) {
             sd->erase_end = req.arg;
             return sd_r1;
-
-        default:
-            break;
         }
         break;
 
     case 38:	/* CMD38:  ERASE */
-        switch (sd->state) {
-        case sd_transfer_state:
+        if (sd->state == sd_transfer_state) {
             if (sd->csd[14] & 0x30) {
                 sd->card_status |= WP_VIOLATION;
                 return sd_r1b;
@@ -1348,9 +1269,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
             /* Bzzzzzzztt .... Operation complete.  */
             sd->state = sd_transfer_state;
             return sd_r1b;
-
-        default:
-            break;
         }
         break;
 
@@ -1358,15 +1276,11 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
     case 42:	/* CMD42:  LOCK_UNLOCK */
         if (sd->spi)
             goto unimplemented_cmd;
-        switch (sd->state) {
-        case sd_transfer_state:
+        if (sd->state == sd_transfer_state) {
             sd->state = sd_receivingdata_state;
             sd->data_start = 0;
             sd->data_offset = 0;
             return sd_r1;
-
-        default:
-            break;
         }
         break;
 
@@ -1390,17 +1304,13 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         return sd_r1;
 
     case 56:	/* CMD56:  GEN_CMD */
-        switch (sd->state) {
-        case sd_transfer_state:
+        if (sd->state == sd_transfer_state) {
             sd->data_offset = 0;
             if (req.arg & 1)
                 sd->state = sd_sendingdata_state;
             else
                 sd->state = sd_receivingdata_state;
             return sd_r1;
-
-        default:
-            break;
         }
         break;
 
@@ -1428,52 +1338,37 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
     sd->card_status |= APP_CMD;
     switch (req.cmd) {
     case 6:	/* ACMD6:  SET_BUS_WIDTH */
-        switch (sd->state) {
-        case sd_transfer_state:
+        if (sd->state == sd_transfer_state) {
             sd->sd_status[0] &= 0x3f;
             sd->sd_status[0] |= (req.arg & 0x03) << 6;
             return sd_r1;
-
-        default:
-            break;
         }
         break;
 
     case 13:	/* ACMD13: SD_STATUS */
-        switch (sd->state) {
-        case sd_transfer_state:
+        if (sd->state == sd_transfer_state) {
             sd->state = sd_sendingdata_state;
             sd->data_start = 0;
             sd->data_offset = 0;
             return sd_r1;
 
-        default:
-            break;
         }
         break;
 
     case 22:	/* ACMD22: SEND_NUM_WR_BLOCKS */
-        switch (sd->state) {
-        case sd_transfer_state:
+        if (sd->state == sd_transfer_state) {
             *(uint32_t *) sd->data = sd->blk_written;
 
             sd->state = sd_sendingdata_state;
             sd->data_start = 0;
             sd->data_offset = 0;
             return sd_r1;
-
-        default:
-            break;
         }
         break;
 
     case 23:	/* ACMD23: SET_WR_BLK_ERASE_COUNT */
-        switch (sd->state) {
-        case sd_transfer_state:
+        if (sd->state == sd_transfer_state) {
             return sd_r1;
-
-        default:
-            break;
         }
         break;
 
@@ -1483,8 +1378,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
             sd->state = sd_transfer_state;
             return sd_r1;
         }
-        switch (sd->state) {
-        case sd_idle_state:
+        if (sd->state == sd_idle_state) {
             /* If it's the first ACMD41 since reset, we need to decide
              * whether to power up. If this is not an enquiry ACMD41,
              * we immediately report power on and proceed below to the
@@ -1514,33 +1408,22 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
             }
 
             return sd_r3;
-
-        default:
-            break;
         }
         break;
 
     case 42:	/* ACMD42: SET_CLR_CARD_DETECT */
-        switch (sd->state) {
-        case sd_transfer_state:
+        if (sd->state == sd_transfer_state) {
             /* Bringing in the 50KOhm pull-up resistor... Done.  */
             return sd_r1;
-
-        default:
-            break;
         }
         break;
 
     case 51:	/* ACMD51: SEND_SCR */
-        switch (sd->state) {
-        case sd_transfer_state:
+        if (sd->state == sd_transfer_state) {
             sd->state = sd_sendingdata_state;
             sd->data_start = 0;
             sd->data_offset = 0;
             return sd_r1;
-
-        default:
-            break;
         }
         break;
 
-- 
2.15.1

[Qemu-devel] [PATCH v2 12/25] sdcard: use a 16-bit integer for the 16-bit RCA register

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.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 e0550cd174..e6a6e0b2ad 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -854,7 +854,7 @@ static void sd_lock_command(SDState *sd)
 
 static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 {
-    uint32_t rca = 0x0000;
+    uint16_t rca = 0x0000;
     uint64_t addr = (sd->ocr & (1 << 30)) ? (uint64_t) req.arg << 9 : req.arg;
 
     if (req.cmd != 55 || sd->expecting_acmd) {
-- 
2.15.1

Re: [Qemu-devel] [PATCH v2 12/25] sdcard: use a 16-bit integer for the 16-bit RCA register

[Alistair Francis]

On Wed, Jan 3, 2018 at 1:24 PM, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Reviewed-by: Alistair Francis <alistair.francis@xilinx.com>

Alistair

> ---
>  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 e0550cd174..e6a6e0b2ad 100644
> --- a/hw/sd/sd.c
> +++ b/hw/sd/sd.c
> @@ -854,7 +854,7 @@ static void sd_lock_command(SDState *sd)
>
>  static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
>  {
> -    uint32_t rca = 0x0000;
> +    uint16_t rca = 0x0000;
>      uint64_t addr = (sd->ocr & (1 << 30)) ? (uint64_t) req.arg << 9 : req.arg;
>
>      if (req.cmd != 55 || sd->expecting_acmd) {
> --
> 2.15.1
>
>

[Qemu-devel] [PATCH v2 13/25] sdcard: let function handling response codes returns the response size

[Philippe Mathieu-Daudé]

this will ease cases when the response is different regarding the protocol
use (SPI, MMC).

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index e6a6e0b2ad..7e9d7366fe 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -460,7 +460,7 @@ static int sd_req_crc_validate(SDRequest *req)
     return sd_crc7(buffer, 5) != req->crc;	/* TODO */
 }
 
-static void sd_response_r1_make(SDState *sd, uint8_t *response)
+static size_t sd_response_r1_make(SDState *sd, uint8_t *response)
 {
     uint32_t status = sd->card_status;
     /* Clear the "clear on read" status bits */
@@ -470,14 +470,25 @@ static void sd_response_r1_make(SDState *sd, uint8_t *response)
     response[1] = (status >> 16) & 0xff;
     response[2] = (status >> 8) & 0xff;
     response[3] = (status >> 0) & 0xff;
+
+    return 4;
+}
+
+static size_t sd_response_r2s_make(SDState *sd, uint8_t *response)
+{
+    memcpy(response, sd->csd, sizeof(sd->csd));
+
+    return 16;
 }
 
-static void sd_response_r3_make(SDState *sd, uint8_t *response)
+static size_t sd_response_r3_make(SDState *sd, uint8_t *response)
 {
     response[0] = (sd->ocr >> 24) & 0xff;
     response[1] = (sd->ocr >> 16) & 0xff;
     response[2] = (sd->ocr >> 8) & 0xff;
     response[3] = (sd->ocr >> 0) & 0xff;
+
+    return 4;
 }
 
 static void sd_response_r6_make(SDState *sd, uint8_t *response)
@@ -1513,8 +1524,7 @@ send_response:
     switch (rtype) {
     case sd_r1:
     case sd_r1b:
-        sd_response_r1_make(sd, response);
-        rsplen = 4;
+        rsplen = sd_response_r1_make(sd, response);
         break;
 
     case sd_r2_i:
@@ -1523,13 +1533,11 @@ send_response:
         break;
 
     case sd_r2_s:
-        memcpy(response, sd->csd, sizeof(sd->csd));
-        rsplen = 16;
+        rsplen = sd_response_r2s_make(sd, response);
         break;
 
     case sd_r3:
-        sd_response_r3_make(sd, response);
-        rsplen = 4;
+        rsplen = sd_response_r3_make(sd, response);
         break;
 
     case sd_r6:
-- 
2.15.1

[Qemu-devel] [PATCH v2 14/25] sdcard: add missing command CMD55

[Philippe Mathieu-Daudé]

we don't implement it, but can see firmwares using it via UNIMP logs.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 7e9d7366fe..2b4254802f 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1438,6 +1438,9 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
         }
         break;
 
+    case 55:    /* Not exist */
+        break;
+
     default:
         /* Fall back to standard commands.  */
         return sd_normal_command(sd, req);
-- 
2.15.1

[Qemu-devel] [PATCH v2 15/25] sdcard: add missing CMD54 SDIO command

[Philippe Mathieu-Daudé]

we don't implement it, but can see firmwares using it via UNIMP logs.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 2b4254802f..d2a0baa0a7 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1295,9 +1295,8 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         }
         break;
 
-    case 52:
-    case 53:
-        /* CMD52, CMD53: reserved for SDIO cards
+    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
-- 
2.15.1

[Qemu-devel] [RFC PATCH v2 16/25] sdcard: add missing SPI legal commands

[Philippe Mathieu-Daudé]

we don't implement them, but when firmware use them we get a UNIMP log.
 CMD: 58, 59
 ACMD: 6, 18, 25, 26, 38, 43 - 49

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 46 insertions(+), 2 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index d2a0baa0a7..a8198ecce9 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1324,6 +1324,18 @@ 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;
+        }
+        goto unimplemented_cmd;
+
     default:
     bad_cmd:
         qemu_log_mask(LOG_GUEST_ERROR, "SD: Unknown CMD%i\n", req.cmd);
@@ -1340,14 +1352,16 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
     return sd_illegal;
 }
 
-static sd_rsp_type_t sd_app_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_abbreviation(req.cmd),
                              req.cmd, req.arg, sd_state_name(sd->state));
     sd->card_status |= APP_CMD;
     switch (req.cmd) {
     case 6:	/* ACMD6:  SET_BUS_WIDTH */
+        if (sd->spi) {
+            goto unimplemented_cmd;
+        }
         if (sd->state == sd_transfer_state) {
             sd->sd_status[0] &= 0x3f;
             sd->sd_status[0] |= (req.arg & 0x03) << 6;
@@ -1361,7 +1375,12 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
             sd->data_start = 0;
             sd->data_offset = 0;
             return sd_r1;
+        }
+        break;
 
+    case 18:
+        if (sd->spi) {
+            goto unimplemented_cmd;
         }
         break;
 
@@ -1382,6 +1401,19 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
         }
         break;
 
+    case 25:
+    case 26:
+        if (sd->spi) {
+            goto unimplemented_cmd;
+        }
+        break;
+
+    case 38:
+        if (sd->spi) {
+            goto unimplemented_cmd;
+        }
+        break;
+
     case 41:	/* ACMD41: SD_APP_OP_COND */
         if (sd->spi) {
             /* SEND_OP_CMD */
@@ -1428,6 +1460,12 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
         }
         break;
 
+    case 43 ... 49:
+        if (sd->spi) {
+            goto unimplemented_cmd;
+        }
+        break;
+
     case 51:	/* ACMD51: SEND_SCR */
         if (sd->state == sd_transfer_state) {
             sd->state = sd_sendingdata_state;
@@ -1443,6 +1481,12 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
     default:
         /* Fall back to standard commands.  */
         return sd_normal_command(sd, req);
+
+    unimplemented_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);
-- 
2.15.1

[Qemu-devel] [RFC PATCH v2 17/25] sdcard: fix SPI response length

[Philippe Mathieu-Daudé]

SPI response are often shorter than SD/MMC protocols.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++----------------
 1 file changed, 54 insertions(+), 19 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index a8198ecce9..024a9fa1df 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -466,29 +466,59 @@ static size_t sd_response_r1_make(SDState *sd, uint8_t *response)
     /* Clear the "clear on read" status bits */
     sd->card_status &= ~CARD_STATUS_C;
 
-    response[0] = (status >> 24) & 0xff;
-    response[1] = (status >> 16) & 0xff;
-    response[2] = (status >> 8) & 0xff;
-    response[3] = (status >> 0) & 0xff;
+    if (sd->spi) {
+        response[0] = 0xff; /* XXX */
+        return 1;
+    } else {
+        response[0] = (status >> 24) & 0xff;
+        response[1] = (status >> 16) & 0xff;
+        response[2] = (status >> 8) & 0xff;
+        response[3] = (status >> 0) & 0xff;
+        return 4;
+    }
+}
+
+static size_t sd_response_r1b_make(SDState *sd, uint8_t *response)
+{
+    /* This response token is identical to the R1 format with the
+     * optional addition of the busy signal. */
+    if (sd->spi) {
+        /* The busy signal token can be any number of bytes. A zero value
+         * indicates card is busy. A non-zero value indicates the card is
+         * ready for the next command. */
+        size_t sz = sd_response_r1_make(sd, response);
+
+        response[sz++] = 0x42;
 
-    return 4;
+        return sz;
+    }
+    return sd_response_r1_make(sd, response);
 }
 
 static size_t sd_response_r2s_make(SDState *sd, uint8_t *response)
 {
-    memcpy(response, sd->csd, sizeof(sd->csd));
-
-    return 16;
+    if (sd->spi) {
+        /* TODO */
+        return 2;
+    } else {
+        memcpy(response, sd->csd, sizeof(sd->csd));
+        return 16;
+    }
 }
 
 static size_t sd_response_r3_make(SDState *sd, uint8_t *response)
 {
-    response[0] = (sd->ocr >> 24) & 0xff;
-    response[1] = (sd->ocr >> 16) & 0xff;
-    response[2] = (sd->ocr >> 8) & 0xff;
-    response[3] = (sd->ocr >> 0) & 0xff;
+    int ofs = 0;
+
+    if (sd->spi) {
+        ofs += sd_response_r1_make(sd, response);
+    }
+    response[ofs++] = (sd->ocr >> 24) & 0xff;
+    response[ofs++] = (sd->ocr >> 16) & 0xff;
+    response[ofs++] = (sd->ocr >> 8) & 0xff;
+    response[ofs++] = (sd->ocr >> 0) & 0xff;
 
-    return 4;
+    return ofs;
 }
 
 static void sd_response_r6_make(SDState *sd, uint8_t *response)
@@ -1249,7 +1279,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
             *(uint32_t *) sd->data = sd_wpbits(sd, req.arg);
             sd->data_start = addr;
             sd->data_offset = 0;
-            return sd_r1b;
+            return sd->spi ? sd_r1 : sd_r1b;
         }
         break;
 
@@ -1306,9 +1336,11 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 
     /* Application specific commands (Class 8) */
     case 55:	/* CMD55:  APP_CMD */
-        if (sd->rca != rca)
-            return sd_r0;
-
+        if (!sd->spi) {
+            if (sd->rca != rca) {
+                return sd_r0;
+            }
+        }
         sd->expecting_acmd = true;
         sd->card_status |= APP_CMD;
         return sd_r1;
@@ -1374,7 +1406,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd, SDRequest req)
             sd->state = sd_sendingdata_state;
             sd->data_start = 0;
             sd->data_offset = 0;
-            return sd_r1;
+            return sd->spi ? sd_r2_s : sd_r1;
         }
         break;
 
@@ -1569,10 +1601,13 @@ int sd_do_command(SDState *sd, SDRequest *req,
 send_response:
     switch (rtype) {
     case sd_r1:
-    case sd_r1b:
         rsplen = sd_response_r1_make(sd, response);
         break;
 
+    case sd_r1b:
+        rsplen = sd_response_r1b_make(sd, response);
+        break;
+
     case sd_r2_i:
         memcpy(response, sd->cid, sizeof(sd->cid));
         rsplen = 16;
-- 
2.15.1

[Qemu-devel] [PATCH v2 18/25] sdcard: add an enum for the SD PHY Spec version

[Philippe Mathieu-Daudé]

So far this device intends to model the Spec v2.00

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 024a9fa1df..ed5874e4d6 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -58,6 +58,11 @@ do { fprintf(stderr, "SD: " fmt , ## __VA_ARGS__); } while (0)
 
 #define SDCARD_CMD_MAX 64
 
+typedef enum {
+    SD_PHY_SPEC_VER_2_00 = 200,
+    SD_PHY_SPEC_VER_3_01 = 301, /* not yet supported */
+} sd_phy_spec_ver_t;
+
 typedef enum {
     sd_r0 = 0,    /* no response */
     sd_r1,        /* normal response command */
@@ -114,6 +119,7 @@ struct SDState {
     uint32_t pwd_len;
     uint8_t function_group[6];
 
+    int spec_version;
     bool spi;
 
     uint32_t mode;    /* current card mode, one of SDCardModes */
@@ -1977,6 +1983,7 @@ static void sd_realize(DeviceState *dev, Error **errp)
     int ret;
 
     sd->proto_name = sd->spi ? "SPI" : "SD";
+    sd->spec_version = SD_PHY_SPEC_VER_2_00;
 
     if (sd->blk && blk_is_read_only(sd->blk)) {
         error_setg(errp, "Cannot use read-only drive as SD card");
-- 
2.15.1

[Qemu-devel] [PATCH v2 19/25] sdcard: check if the card capacity is supported

[Philippe Mathieu-Daudé]

The current Spec (v2.00) does not support SDXC cards (> 32GB).

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c         | 41 +++++++++++++++++++++++++++++++++++++++++
 hw/sd/trace-events |  1 +
 2 files changed, 42 insertions(+)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index ed5874e4d6..a7cede8da6 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -63,6 +63,13 @@ typedef enum {
     SD_PHY_SPEC_VER_3_01 = 301, /* not yet supported */
 } sd_phy_spec_ver_t;
 
+typedef enum {
+    sd_capacity_unknown,
+    sd_capacity_sdsc,           /* not well supported */
+    sd_capacity_sdhc,
+    sd_capacity_sdxc,           /* not yet supported */
+} sd_card_capacity_t;
+
 typedef enum {
     sd_r0 = 0,    /* no response */
     sd_r1,        /* normal response command */
@@ -121,6 +128,7 @@ struct SDState {
 
     int spec_version;
     bool spi;
+    sd_card_capacity_t capacity;
 
     uint32_t mode;    /* current card mode, one of SDCardModes */
     int32_t state;    /* current card state, one of SDCardStates */
@@ -161,6 +169,17 @@ static const char *sd_state_name(enum SDCardStates state)
     return state_name[state];
 }
 
+static const char *sd_capacity(sd_card_capacity_t capacity)
+{
+    static const char *capacity_name[] = {
+        [sd_capacity_unknown]   = "UNKN",
+        [sd_capacity_sdsc]      = "SDSC",
+        [sd_capacity_sdhc]      = "SDHC",
+        [sd_capacity_sdxc]      = "SDXC",
+    };
+    return capacity_name[capacity];
+}
+
 static const char *sd_cmd_abbreviation(uint8_t cmd)
 {
     static const char *cmd_abbrev[SDCARD_CMD_MAX] = {
@@ -1991,11 +2010,33 @@ static void sd_realize(DeviceState *dev, Error **errp)
     }
 
     if (sd->blk) {
+        int64_t size;
+
         ret = blk_set_perm(sd->blk, BLK_PERM_CONSISTENT_READ | BLK_PERM_WRITE,
                            BLK_PERM_ALL, errp);
         if (ret < 0) {
             return;
         }
+
+        size = blk_getlength(sd->blk);
+        if (size < 0 || size <= 2 * G_BYTE) {
+            sd->capacity = sd_capacity_sdsc;
+        } else if (size <= 32 * G_BYTE) {
+            sd->capacity = sd_capacity_sdhc;
+        } else if (size <= 2 * T_BYTE) {
+            sd->capacity = sd_capacity_sdxc;
+        } else {
+            error_setg(errp, "block size unsupported: %lld TB", size / T_BYTE);
+            return;
+        }
+        trace_sdcard_capacity(sd_capacity(sd->capacity), size);
+
+        if (sd->capacity == sd_capacity_sdxc
+                && sd->spec_version < SD_PHY_SPEC_VER_3_01) {
+            error_setg(errp, "capacity SDHC requires at least Spec v3.01");
+            return;
+        }
+
         blk_set_dev_ops(sd->blk, &sd_block_ops, sd);
     }
 }
diff --git a/hw/sd/trace-events b/hw/sd/trace-events
index 25ab339f97..215e0eec3a 100644
--- a/hw/sd/trace-events
+++ b/hw/sd/trace-events
@@ -28,6 +28,7 @@ sdcard_read_block(uint64_t addr, uint32_t len) "addr 0x%" PRIx64 " size 0x%x"
 sdcard_write_block(uint64_t addr, uint32_t len) "addr 0x%" PRIx64 " size 0x%x"
 sdcard_write_data(uint8_t value) "value 0x%02x"
 sdcard_read_data(int length) "len %d"
+sdcard_capacity(const char *capacity, int64_t size) "%s card (%ld B)"
 
 # hw/sd/milkymist-memcard.c
 milkymist_memcard_memory_read(uint32_t addr, uint32_t value) "addr 0x%08x value 0x%08x"
-- 
2.15.1

[Qemu-devel] [PATCH v2 20/25] sdcard: Don't always set the high capacity bit

[Philippe Mathieu-Daudé]

Don't set the high capacity bit by default as it will be set if required
in the sd_set_csd() function.

[based on a patch from Alistair Francis <alistair.francis@xilinx.com>
 and Peter Ogden <ogden@xilinx.com> from qemu/xilinx tag xilinx-v2015.4]
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index a7cede8da6..16a8a7411d 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -318,6 +318,7 @@ static uint16_t sd_crc16(void *message, size_t width)
     return shift_reg;
 }
 
+FIELD(OCR, CARD_CAPACITY,              30, 1); /* 0:SDSC, 1:SDHC/SDXC */
 FIELD(OCR, CARD_POWER_UP,              31, 1);
 
 #define OCR_POWER_DELAY_NS      500000 /* 0.5ms */
@@ -336,6 +337,10 @@ static void sd_ocr_powerup(void *opaque)
 
     /* card power-up OK */
     sd->ocr = FIELD_DP32(sd->ocr, OCR, CARD_POWER_UP, 1);
+
+    if (sd->capacity >= sd_capacity_sdhc) {
+        sd->ocr = FIELD_DP32(sd->ocr, OCR, CARD_CAPACITY, 1);
+    }
 }
 
 static void sd_reset_scr(SDState *sd)
@@ -440,7 +445,6 @@ static void sd_reset_csd(SDState *sd, uint64_t size)
         sd->csd[13] = 0x40;
         sd->csd[14] = 0x00;
         sd->csd[15] = 0x00;
-        sd->ocr |= 1 << 30;     /* High Capacity SD Memory Card */
     }
 }
 
-- 
2.15.1

[Qemu-devel] [PATCH v2 21/25] sdcard: add cmd_version_supported(), improve cmd_class_supported()

[Philippe Mathieu-Daudé]

Using the cmd_supported[] array we keep track of which Spec version introduced
a command, and which class it belongs to.

This array is initialized with SD Spec v2.00.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 128 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 121 insertions(+), 7 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 16a8a7411d..0e4871869f 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -278,13 +278,121 @@ static const sd_cmd_type_t sd_cmd_type[SDCARD_CMD_MAX] = {
     sd_adtc, sd_none, sd_none, sd_none, sd_none, sd_none, sd_none, sd_none,
 };
 
-static const int sd_cmd_class[SDCARD_CMD_MAX] = {
-    0,  0,  0,  0,  0,  9, 10,  0,  0,  0,  0,  1,  0,  0,  0,  0,
-    2,  2,  2,  2,  3,  3,  3,  3,  4,  4,  4,  4,  6,  6,  6,  6,
-    5,  5, 10, 10, 10, 10,  5,  9,  9,  9,  7,  7,  7,  7,  7,  7,
-    7,  7, 10,  7,  9,  9,  9,  8,  8, 10,  8,  8,  8,  8,  8,  8,
+#define BIT_2_4     (BIT(2) | BIT(4))
+#define BIT_2_4_7   (BIT(2) | BIT(4) | BIT(7))
+#define BIT_SECU    BIT(30) /* SD Specifications Part3 Security Specification */
+#define BIT_MANUF   BIT(31) /* reserved for manufacturer */
+
+typedef struct {
+    struct {
+        uint16_t version;
+        uint32_t ccc_mask;
+    } sd, spi;
+} sd_cmd_supported_t;
+
+static const sd_cmd_supported_t cmd_supported[SDCARD_CMD_MAX] = {
+     /*           SD                  SPI          */
+     [0] = {{200, BIT(0)},      {200, BIT(0)},      },
+     [1] = {{},                 {200, BIT(0)},      },
+     [2] = {{200, BIT(0)},      {},                 },
+     [3] = {{200, BIT(0)},      {},                 },
+     [4] = {{200, BIT(0)},      {},                 },
+     [5] = {{200, BIT(9)},      {200, BIT(9)},      },
+     [6] = {{200, BIT(10)},     {200, BIT(10)},     },
+     [7] = {{200, BIT(0)},      {},                 },
+     [8] = {{200, BIT(0)},      {200, BIT(0)},      },
+     [9] = {{200, BIT(0)},      {200, BIT(0)},      },
+    [10] = {{200, BIT(0)},      {200, BIT(0)},      },
+    [12] = {{200, BIT(0)},      {200, BIT(0)},      },
+    [13] = {{200, BIT(0)},      {200, BIT(0)},      },
+    [14] = {{200, BIT(0)},      {},                 },
+    [15] = {{200, BIT(0)},      {},                 },
+    [16] = {{200, BIT_2_4_7},   {200, BIT_2_4_7},   },
+    [17] = {{200, BIT(2)},      {200, BIT(2)},      },
+    [18] = {{200, BIT(2)},      {200, BIT(2)},      },
+    [24] = {{200, BIT(4)},      {200, BIT(4)},      },
+    [25] = {{200, BIT(4)},      {200, BIT(4)},      },
+    [26] = {{200, BIT_MANUF},   {/*?*/},            },
+    [27] = {{200, BIT(4)},      {200, BIT(4)},      },
+    [28] = {{200, BIT(6)},      {200, BIT(6)},      },
+    [29] = {{200, BIT(6)},      {200, BIT(6)},      },
+    [30] = {{200, BIT(6)},      {200, BIT(6)},      },
+    [32] = {{200, BIT(5)},      {200, BIT(5)},      },
+    [33] = {{200, BIT(5)},      {200, BIT(5)},      },
+    [34] = {{200, BIT(10)},     {200, BIT(10)},     },
+    [35] = {{200, BIT(10)},     {200, BIT(10)},     },
+    [36] = {{200, BIT(10)},     {200, BIT(10)},     },
+    [37] = {{200, BIT(10)},     {200, BIT(10)},     },
+    [38] = {{200, BIT(5)},      {200, BIT(5)},      },
+    [42] = {{200, BIT(7)},      {200, BIT(7)},      },
+    [50] = {{200, BIT(10)},     {200, BIT(10)},     },
+    [52] = {{200, BIT(9)},      {200, BIT(9)},      },
+    [53] = {{200, BIT(9)},      {200, BIT(9)},      },
+    [54] = {{/* 2.00 SDIO */},  {/* 2.00 SDIO */},  },
+    [55] = {{200, BIT(8)},      {200, BIT(8)},      },
+    [56] = {{200, BIT(8)},      {200, BIT(8)},      },
+    [57] = {{200, BIT(10)},     {200, BIT(10)},     },
+    [58] = {{},                 {200, BIT(0)},      },
+    [59] = {{},                 {200, BIT(0)},      },
+    [60] = {{200, BIT_MANUF},   {/*?*/},            },
+    [61] = {{200, BIT_MANUF},   {/*?*/},            },
+    [62] = {{200, BIT_MANUF},   {/*?*/},            },
+    [63] = {{200, BIT_MANUF},   {/*?*/},            },
+}, acmd_supported[SDCARD_CMD_MAX] = {
+     /*           SD                  SPI          */
+     [6] = {{200, BIT(8)},      {},                 },
+    [13] = {{200, BIT(8)},      {200, BIT(8)},      },
+    [18] = {{200, BIT_SECU},    {200, BIT_SECU},    },
+    [22] = {{200, BIT(8)},      {200, BIT(8)},      },
+    [23] = {{200, BIT(8)},      {200, BIT(8)},      },
+    [25] = {{200, BIT_SECU},    {200, BIT_SECU},    },
+    [26] = {{200, BIT_SECU},    {200, BIT_SECU},    },
+    [38] = {{200, BIT_SECU},    {200, BIT_SECU},    },
+    [41] = {{200, BIT(8)},      {200, BIT(8)},      },
+    [42] = {{200, BIT(8)},      {200, BIT(8)},      },
+    [43] = {{200, BIT_SECU},    {200, BIT_SECU},    },
+    [44] = {{200, BIT_SECU},    {200, BIT_SECU},    },
+    [45] = {{200, BIT_SECU},    {200, BIT_SECU},    },
+    [46] = {{200, BIT_SECU},    {200, BIT_SECU},    },
+    [47] = {{200, BIT_SECU},    {200, BIT_SECU},    },
+    [48] = {{200, BIT_SECU},    {200, BIT_SECU},    },
+    [49] = {{200, BIT_SECU},    {200, BIT_SECU},    },
+    [51] = {{200, BIT(8)},      {200, BIT(8)},      },
 };
 
+static bool cmd_version_supported(SDState *sd, uint8_t cmd, bool is_acmd)
+{
+    const sd_cmd_supported_t *cmdset = is_acmd ? acmd_supported : cmd_supported;
+    uint16_t cmd_version;
+
+    cmd_version = sd->spi ? cmdset[cmd].spi.version : cmdset[cmd].sd.version;
+    if (cmd_version) {
+        return true;
+    }
+    qemu_log_mask(LOG_GUEST_ERROR, "%s: Unsupported %s%02u\n",
+                  sd->proto_name, is_acmd ? "ACMD" : "CMD", cmd);
+
+    return false;
+}
+
+static bool cmd_class_supported(SDState *sd, uint8_t cmd, uint8_t class,
+                                bool is_acmd)
+{
+    const sd_cmd_supported_t *cmdset = is_acmd ? acmd_supported : cmd_supported;
+    uint32_t cmd_ccc_mask;
+
+    cmd_ccc_mask = sd->spi ? cmdset[cmd].spi.ccc_mask : cmdset[cmd].sd.ccc_mask;
+
+    /* class 1, 3 and 9 are not supported in SPI mode */
+    if (cmd_ccc_mask & BIT(class)) {
+        return true;
+    }
+    qemu_log_mask(LOG_GUEST_ERROR, "%s: Unsupported %s%02u (class %d)\n",
+                  sd->proto_name, is_acmd ? "ACMD" : "CMD", cmd, class);
+
+    return false;
+}
+
 static uint8_t sd_crc7(void *message, size_t width)
 {
     int i, bit;
@@ -1570,8 +1678,8 @@ static bool cmd_valid_while_locked(SDState *sd, SDRequest *req)
     if (req->cmd == 16 || req->cmd == 55) {
         return true;
     }
-    return sd_cmd_class[req->cmd] == 0
-            || sd_cmd_class[req->cmd] == 7;
+    return cmd_class_supported(sd, req->cmd, 0, false) ||
+            cmd_class_supported(sd, req->cmd, 7, false);
 }
 
 int sd_do_command(SDState *sd, SDRequest *req,
@@ -1596,6 +1704,12 @@ int sd_do_command(SDState *sd, SDRequest *req,
         req->cmd &= 0x3f;
     }
 
+    if (!cmd_version_supported(sd, req->cmd, sd->expecting_acmd)) {
+        sd->card_status |= ILLEGAL_COMMAND;
+        rtype = sd_illegal;
+        goto send_response;
+    }
+
     if (sd->card_status & CARD_IS_LOCKED) {
         if (!cmd_valid_while_locked(sd, req)) {
             sd->card_status |= ILLEGAL_COMMAND;
-- 
2.15.1

[Qemu-devel] [PATCH v2 22/25] sdcard: remove unreachable SPI commands

[Philippe Mathieu-Daudé]

we already checked in sd_do_command() with cmd_version_supported() if
the command is protocol supported (returning ILLEGAL if not).

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 0e4871869f..16199cd886 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -1071,8 +1071,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         return sd_r1;
 
     case 2:	/* CMD2:   ALL_SEND_CID */
-        if (sd->spi)
-            goto bad_cmd;
         if (sd->state == sd_ready_state) {
             sd->state = sd_identification_state;
             return sd_r2_i;
@@ -1080,8 +1078,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:
@@ -1095,8 +1091,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;
@@ -1110,8 +1104,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         return sd_illegal;
 
     case 6:	/* CMD6:   SWITCH_FUNCTION */
-        if (sd->spi)
-            goto bad_cmd;
         if (sd->mode == sd_data_transfer_mode) {
             sd_function_switch(sd, req.arg);
             sd->state = sd_sendingdata_state;
@@ -1122,8 +1114,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)
@@ -1224,8 +1214,6 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         break;
 
     case 11:	/* CMD11:  READ_DAT_UNTIL_STOP */
-        if (sd->spi)
-            goto bad_cmd;
         if (sd->state == sd_transfer_state) {
             sd->state = sd_sendingdata_state;
             sd->data_start = req.arg;
@@ -1264,8 +1252,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;
         if (sd->mode == sd_data_transfer_mode) {
             if (sd->rca != rca)
                 return sd_r0;
@@ -1358,8 +1344,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;
         if (sd->state == sd_transfer_state) {
             sd->state = sd_receivingdata_state;
             sd->data_start = 0;
@@ -1528,9 +1512,6 @@ static sd_rsp_type_t sd_app_command(SDState *sd, SDRequest req)
     sd->card_status |= APP_CMD;
     switch (req.cmd) {
     case 6:	/* ACMD6:  SET_BUS_WIDTH */
-        if (sd->spi) {
-            goto unimplemented_cmd;
-        }
         if (sd->state == sd_transfer_state) {
             sd->sd_status[0] &= 0x3f;
             sd->sd_status[0] |= (req.arg & 0x03) << 6;
-- 
2.15.1

[Qemu-devel] [RFC PATCH v2 23/25] sdcard: store the bus protocol in an enum

[Philippe Mathieu-Daudé]

One and only one protocol can be used at a time.
We use a bitmask 'bus_protocol' simplify to check with one variable which
protocols are supported (this will help when we start using the MMC mode).

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/sd/sd.h            |   5 ++
 include/hw/sd/sdcard_legacy.h |   2 +-
 hw/sd/sd.c                    | 141 ++++++++++++++++++++++++++++--------------
 3 files changed, 100 insertions(+), 48 deletions(-)

diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index a342e7bc3e..6874696ff7 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -55,6 +55,11 @@
 #define AKE_SEQ_ERROR		(1 << 3)
 #define OCR_CCS_BITN        30
 
+typedef enum {
+    PROTO_SD =  0,
+    PROTO_SPI = 1 << 1,
+} sd_bus_protocol_t;
+
 typedef enum {
     sd_none = -1,
     sd_bc = 0,	/* broadcast -- no response */
diff --git a/include/hw/sd/sdcard_legacy.h b/include/hw/sd/sdcard_legacy.h
index 882e13a8f1..08674797b5 100644
--- a/include/hw/sd/sdcard_legacy.h
+++ b/include/hw/sd/sdcard_legacy.h
@@ -32,7 +32,7 @@
 #include "hw/sd/sd.h"
 
 /* Legacy functions to be used only by non-qdevified callers */
-SDState *sd_init(BlockBackend *blk, bool is_spi);
+SDState *sd_init(BlockBackend *blk, bool is_spi); /* deprecated */
 int sd_do_command(SDState *card, SDRequest *request, uint8_t *response);
 void sd_write_data(SDState *card, uint8_t value);
 uint8_t sd_read_data(SDState *card);
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 16199cd886..d626445282 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -127,7 +127,7 @@ struct SDState {
     uint8_t function_group[6];
 
     int spec_version;
-    bool spi;
+    uint32_t bus_protocol;
     sd_card_capacity_t capacity;
 
     uint32_t mode;    /* current card mode, one of SDCardModes */
@@ -150,6 +150,18 @@ struct SDState {
     const char *proto_name;
 };
 
+static const char *sd_protocol_name(sd_bus_protocol_t protocol)
+{
+    switch (protocol) {
+    case PROTO_SD:
+        return "SD";
+    case PROTO_SPI:
+        return "SPI";
+    default:
+        g_assert_not_reached();
+    }
+}
+
 static const char *sd_state_name(enum SDCardStates state)
 {
     static const char *state_name[] = {
@@ -365,7 +377,16 @@ static bool cmd_version_supported(SDState *sd, uint8_t cmd, bool is_acmd)
     const sd_cmd_supported_t *cmdset = is_acmd ? acmd_supported : cmd_supported;
     uint16_t cmd_version;
 
-    cmd_version = sd->spi ? cmdset[cmd].spi.version : cmdset[cmd].sd.version;
+    switch (sd->bus_protocol) {
+    case PROTO_SD:
+        cmd_version = cmdset[cmd].sd.version;
+        break;
+    case PROTO_SPI:
+        cmd_version = cmdset[cmd].spi.version;
+        break;
+    default:
+        g_assert_not_reached();
+    }
     if (cmd_version) {
         return true;
     }
@@ -381,9 +402,17 @@ static bool cmd_class_supported(SDState *sd, uint8_t cmd, uint8_t class,
     const sd_cmd_supported_t *cmdset = is_acmd ? acmd_supported : cmd_supported;
     uint32_t cmd_ccc_mask;
 
-    cmd_ccc_mask = sd->spi ? cmdset[cmd].spi.ccc_mask : cmdset[cmd].sd.ccc_mask;
-
-    /* class 1, 3 and 9 are not supported in SPI mode */
+    switch (sd->bus_protocol) {
+    case PROTO_SD:
+        cmd_ccc_mask = cmdset[cmd].sd.ccc_mask;
+        break;
+    case PROTO_SPI:
+        /* class 1, 3 and 9 are not supported in SPI mode */
+        cmd_ccc_mask = cmdset[cmd].spi.ccc_mask;
+        break;
+    default:
+        g_assert_not_reached();
+    }
     if (cmd_ccc_mask & BIT(class)) {
         return true;
     }
@@ -603,7 +632,7 @@ static size_t sd_response_r1_make(SDState *sd, uint8_t *response)
     /* Clear the "clear on read" status bits */
     sd->card_status &= ~CARD_STATUS_C;
 
-    if (sd->spi) {
+    if (sd->bus_protocol == PROTO_SPI) {
         response[0] = 0xff; /* XXX */
         return 1;
     } else {
@@ -619,7 +648,7 @@ static size_t sd_response_r1b_make(SDState *sd, uint8_t *response)
 {
     /* This response token is identical to the R1 format with the
      * optional addition of the busy signal. */
-    if (sd->spi) {
+    if (sd->bus_protocol == PROTO_SPI) {
         /* The busy signal token can be any number of bytes. A zero value
          * indicates card is busy. A non-zero value indicates the card is
          * ready for the next command. */
@@ -634,7 +663,7 @@ static size_t sd_response_r1b_make(SDState *sd, uint8_t *response)
 
 static size_t sd_response_r2s_make(SDState *sd, uint8_t *response)
 {
-    if (sd->spi) {
+    if (sd->bus_protocol == PROTO_SPI) {
         /* TODO */
         return 2;
     } else {
@@ -647,7 +676,7 @@ static size_t sd_response_r3_make(SDState *sd, uint8_t *response)
 {
     int ofs = 0;
 
-    if (sd->spi) {
+    if (sd->bus_protocol == PROTO_SPI) {
         ofs += sd_response_r1_make(sd, response);
     }
     response[ofs++] = (sd->ocr >> 24) & 0xff;
@@ -663,6 +692,7 @@ static void sd_response_r6_make(SDState *sd, uint8_t *response)
     uint16_t arg;
     uint16_t status;
 
+    assert(sd->bus_protocol != PROTO_SPI);
     arg = sd->rca;
     status = ((sd->card_status >> 8) & 0xc000) |
              ((sd->card_status >> 6) & 0x2000) |
@@ -677,6 +707,7 @@ static void sd_response_r6_make(SDState *sd, uint8_t *response)
 
 static void sd_response_r7_make(SDState *sd, uint8_t *response)
 {
+    assert(sd->bus_protocol != PROTO_SPI);
     response[0] = (sd->vhs >> 24) & 0xff;
     response[1] = (sd->vhs >> 16) & 0xff;
     response[2] = (sd->vhs >>  8) & 0xff;
@@ -844,7 +875,7 @@ static const VMStateDescription sd_vmstate = {
 };
 
 /* Legacy initialization function for use by non-qdevified callers */
-SDState *sd_init(BlockBackend *blk, bool is_spi)
+static SDState *sdcard_init(BlockBackend *blk, sd_bus_protocol_t bus_protocol)
 {
     Object *obj;
     DeviceState *dev;
@@ -857,7 +888,13 @@ SDState *sd_init(BlockBackend *blk, bool is_spi)
         error_report("sd_init failed: %s", error_get_pretty(err));
         return NULL;
     }
-    qdev_prop_set_bit(dev, "spi", is_spi);
+    switch (bus_protocol) {
+    case PROTO_SPI:
+        qdev_prop_set_bit(dev, "spi", true);
+        break;
+    default:
+        break;
+    }
     object_property_set_bool(obj, true, "realized", &err);
     if (err) {
         error_report("sd_init failed: %s", error_get_pretty(err));
@@ -867,6 +904,11 @@ SDState *sd_init(BlockBackend *blk, bool is_spi)
     return SD_CARD(dev);
 }
 
+SDState *sd_init(BlockBackend *blk, bool is_spi)
+{
+    return sdcard_init(blk, is_spi ? PROTO_SPI : PROTO_SD);
+}
+
 void sd_set_cb(SDState *sd, qemu_irq readonly, qemu_irq insert)
 {
     sd->readonly_cb = readonly;
@@ -1062,13 +1104,14 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
             sd->state = sd_idle_state;
             sd_reset(DEVICE(sd));
         }
-        return sd->spi ? sd_r1 : sd_r0;
+        return sd->bus_protocol == PROTO_SPI ? sd_r1 : sd_r0;
 
     case 1:	/* CMD1:   SEND_OP_CMD */
-        if (!sd->spi)
-            goto bad_cmd;
-        sd->state = sd_transfer_state;
-        return sd_r1;
+        if (sd->bus_protocol == PROTO_SPI) {
+            sd->state = sd_transfer_state;
+            return sd_r1;
+        }
+        goto bad_cmd;
 
     case 2:	/* CMD2:   ALL_SEND_CID */
         if (sd->state == sd_ready_state) {
@@ -1157,7 +1200,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 
             /* No response if not exactly one VHS bit is set.  */
             if (!(req.arg >> 8) || (req.arg >> (ctz32(req.arg & ~0xff) + 1))) {
-                return sd->spi ? sd_r7 : sd_r0;
+                return sd->bus_protocol == PROTO_SPI ? sd_r7 : sd_r0; /* XXX */
             }
 
             /* Accept.  */
@@ -1178,13 +1221,13 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
             return sd_r2_s;
 
         case sd_transfer_state:
-            if (!sd->spi)
-                break;
-            sd->state = sd_sendingdata_state;
-            memcpy(sd->data, sd->csd, 16);
-            sd->data_start = addr;
-            sd->data_offset = 0;
-            return sd_r1;
+            if (sd->bus_protocol == PROTO_SPI) {
+                sd->state = sd_sendingdata_state;
+                memcpy(sd->data, sd->csd, 16);
+                sd->data_start = addr;
+                sd->data_offset = 0;
+                return sd_r1;
+            }
 
         default:
             break;
@@ -1200,13 +1243,13 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
             return sd_r2_i;
 
         case sd_transfer_state:
-            if (!sd->spi)
-                break;
-            sd->state = sd_sendingdata_state;
-            memcpy(sd->data, sd->cid, 16);
-            sd->data_start = addr;
-            sd->data_offset = 0;
-            return sd_r1;
+            if (sd->bus_protocol == PROTO_SPI) {
+                sd->state = sd_sendingdata_state;
+                memcpy(sd->data, sd->cid, 16);
+                sd->data_start = addr;
+                sd->data_offset = 0;
+                return sd_r1;
+            }
 
         default:
             break;
@@ -1306,8 +1349,9 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 
     /* Block write commands (Class 4) */
     case 24:	/* CMD24:  WRITE_SINGLE_BLOCK */
-        if (sd->spi)
+        if (sd->bus_protocol == PROTO_SPI) {
             goto unimplemented_cmd;
+        }
         if (sd->state == sd_transfer_state) {
             sd->state = sd_receivingdata_state;
             sd->data_start = addr;
@@ -1325,8 +1369,9 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         break;
 
     case 25:	/* CMD25:  WRITE_MULTIPLE_BLOCK */
-        if (sd->spi)
+        if (sd->bus_protocol == PROTO_SPI) {
             goto unimplemented_cmd;
+        }
         if (sd->state == sd_transfer_state) {
             sd->state = sd_receivingdata_state;
             sd->data_start = addr;
@@ -1353,8 +1398,9 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         break;
 
     case 27:	/* CMD27:  PROGRAM_CSD */
-        if (sd->spi)
+        if (sd->bus_protocol == PROTO_SPI) {
             goto unimplemented_cmd;
+        }
         if (sd->state == sd_transfer_state) {
             sd->state = sd_receivingdata_state;
             sd->data_start = 0;
@@ -1400,7 +1446,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
             *(uint32_t *) sd->data = sd_wpbits(sd, req.arg);
             sd->data_start = addr;
             sd->data_offset = 0;
-            return sd->spi ? sd_r1 : sd_r1b;
+            return sd->bus_protocol == PROTO_SPI ? sd_r1 : sd_r1b;
         }
         break;
 
@@ -1436,8 +1482,9 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 
     /* Lock card commands (Class 7) */
     case 42:	/* CMD42:  LOCK_UNLOCK */
-        if (sd->spi)
+        if (sd->bus_protocol == PROTO_SPI) {
             goto unimplemented_cmd;
+        }
         if (sd->state == sd_transfer_state) {
             sd->state = sd_receivingdata_state;
             sd->data_start = 0;
@@ -1457,7 +1504,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
 
     /* Application specific commands (Class 8) */
     case 55:	/* CMD55:  APP_CMD */
-        if (!sd->spi) {
+        if (sd->bus_protocol != PROTO_SPI) {
             if (sd->rca != rca) {
                 return sd_r0;
             }
@@ -1478,13 +1525,13 @@ static sd_rsp_type_t sd_normal_command(SDState *sd, SDRequest req)
         break;
 
     case 58:    /* CMD58:   READ_OCR (SPI) */
-        if (!sd->spi) {
+        if (sd->bus_protocol != PROTO_SPI) {
             goto bad_cmd;
         }
         return sd_r3;
 
     case 59:    /* CMD59:   CRC_ON_OFF (SPI) */
-        if (!sd->spi) {
+        if (sd->bus_protocol != PROTO_SPI) {
             goto bad_cmd;
         }
         goto unimplemented_cmd;
@@ -1524,12 +1571,12 @@ static sd_rsp_type_t sd_app_command(SDState *sd, SDRequest req)
             sd->state = sd_sendingdata_state;
             sd->data_start = 0;
             sd->data_offset = 0;
-            return sd->spi ? sd_r2_s : sd_r1;
+            return sd->bus_protocol == PROTO_SPI ? sd_r2_s : sd_r1;
         }
         break;
 
     case 18:
-        if (sd->spi) {
+        if (sd->bus_protocol == PROTO_SPI) {
             goto unimplemented_cmd;
         }
         break;
@@ -1553,19 +1600,19 @@ static sd_rsp_type_t sd_app_command(SDState *sd, SDRequest req)
 
     case 25:
     case 26:
-        if (sd->spi) {
+        if (sd->bus_protocol == PROTO_SPI) {
             goto unimplemented_cmd;
         }
         break;
 
     case 38:
-        if (sd->spi) {
+        if (sd->bus_protocol == PROTO_SPI) {
             goto unimplemented_cmd;
         }
         break;
 
     case 41:	/* ACMD41: SD_APP_OP_COND */
-        if (sd->spi) {
+        if (sd->bus_protocol == PROTO_SPI) {
             /* SEND_OP_CMD */
             sd->state = sd_transfer_state;
             return sd_r1;
@@ -1611,7 +1658,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd, SDRequest req)
         break;
 
     case 43 ... 49:
-        if (sd->spi) {
+        if (sd->bus_protocol == PROTO_SPI) {
             goto unimplemented_cmd;
         }
         break;
@@ -2100,7 +2147,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_name = sd_protocol_name(sd->bus_protocol);
     sd->spec_version = SD_PHY_SPEC_VER_2_00;
 
     if (sd->blk && blk_is_read_only(sd->blk)) {
@@ -2146,7 +2193,7 @@ static Property sd_properties[] = {
      * whether card should be in SSI or MMC/SD mode.  It is also up to the
      * board to ensure that ssi transfers only occur when the chip select
      * is asserted.  */
-    DEFINE_PROP_BOOL("spi", SDState, spi, false),
+    DEFINE_PROP_BIT("spi", SDState, bus_protocol, 1, false),
     DEFINE_PROP_END_OF_LIST()
 };
 
-- 
2.15.1

[Qemu-devel] [PATCH v2 24/25] sdcard: introduce Spec v3.01 SD commands

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/sd/sd.c | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index d626445282..6fbc640d2e 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -305,7 +305,7 @@ typedef struct {
 static const sd_cmd_supported_t cmd_supported[SDCARD_CMD_MAX] = {
      /*           SD                  SPI          */
      [0] = {{200, BIT(0)},      {200, BIT(0)},      },
-     [1] = {{},                 {200, BIT(0)},      },
+     [1] = {{301, BIT(0)},      {200, BIT(0)},      },
      [2] = {{200, BIT(0)},      {},                 },
      [3] = {{200, BIT(0)},      {},                 },
      [4] = {{200, BIT(0)},      {},                 },
@@ -322,6 +322,7 @@ static const sd_cmd_supported_t cmd_supported[SDCARD_CMD_MAX] = {
     [16] = {{200, BIT_2_4_7},   {200, BIT_2_4_7},   },
     [17] = {{200, BIT(2)},      {200, BIT(2)},      },
     [18] = {{200, BIT(2)},      {200, BIT(2)},      },
+    [23] = {{301, BIT_2_4},     {},                 },
     [24] = {{200, BIT(4)},      {200, BIT(4)},      },
     [25] = {{200, BIT(4)},      {200, BIT(4)},      },
     [26] = {{200, BIT_MANUF},   {/*?*/},            },
@@ -344,8 +345,8 @@ static const sd_cmd_supported_t cmd_supported[SDCARD_CMD_MAX] = {
     [55] = {{200, BIT(8)},      {200, BIT(8)},      },
     [56] = {{200, BIT(8)},      {200, BIT(8)},      },
     [57] = {{200, BIT(10)},     {200, BIT(10)},     },
-    [58] = {{},                 {200, BIT(0)},      },
-    [59] = {{},                 {200, BIT(0)},      },
+    [58] = {{301, BIT(0)},      {200, BIT(0)},      },
+    [59] = {{301, BIT(0)},      {200, BIT(0)},      },
     [60] = {{200, BIT_MANUF},   {/*?*/},            },
     [61] = {{200, BIT_MANUF},   {/*?*/},            },
     [62] = {{200, BIT_MANUF},   {/*?*/},            },
@@ -372,6 +373,18 @@ static const sd_cmd_supported_t cmd_supported[SDCARD_CMD_MAX] = {
     [51] = {{200, BIT(8)},      {200, BIT(8)},      },
 };
 
+static const char *spec_version_name(uint16_t spec_version)
+{
+    switch (spec_version) {
+    case SD_PHY_SPEC_VER_2_00:
+        return "v2.00";
+    case SD_PHY_SPEC_VER_3_01:
+        return "v3.01";
+    default:
+        g_assert_not_reached();
+    }
+}
+
 static bool cmd_version_supported(SDState *sd, uint8_t cmd, bool is_acmd)
 {
     const sd_cmd_supported_t *cmdset = is_acmd ? acmd_supported : cmd_supported;
@@ -387,11 +400,12 @@ static bool cmd_version_supported(SDState *sd, uint8_t cmd, bool is_acmd)
     default:
         g_assert_not_reached();
     }
-    if (cmd_version) {
+    if (cmd_version >= sd->spec_version) {
         return true;
     }
-    qemu_log_mask(LOG_GUEST_ERROR, "%s: Unsupported %s%02u\n",
-                  sd->proto_name, is_acmd ? "ACMD" : "CMD", cmd);
+    qemu_log_mask(LOG_GUEST_ERROR, "%s: Unsupported %s%02u (%s)\n",
+                  sd->proto_name, is_acmd ? "ACMD" : "CMD", cmd,
+                  spec_version_name(cmd_version));
 
     return false;
 }
@@ -416,8 +430,9 @@ static bool cmd_class_supported(SDState *sd, uint8_t cmd, uint8_t class,
     if (cmd_ccc_mask & BIT(class)) {
         return true;
     }
-    qemu_log_mask(LOG_GUEST_ERROR, "%s: Unsupported %s%02u (class %d)\n",
-                  sd->proto_name, is_acmd ? "ACMD" : "CMD", cmd, class);
+    qemu_log_mask(LOG_GUEST_ERROR, "%s: Unsupported %s%02u (class %d, %s)\n",
+                  sd->proto_name, is_acmd ? "ACMD" : "CMD", cmd, class,
+                  spec_version_name(sd->spec_version));
 
     return false;
 }
-- 
2.15.1

[Qemu-devel] [PATCH v2 25/25] sdcard: introduce Spec v4.51 & v5.1 MMC commands

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/sd/sd.h |   1 +
 hw/sd/sd.c         | 181 ++++++++++++++++++++++++++++++++---------------------
 2 files changed, 112 insertions(+), 70 deletions(-)

diff --git a/include/hw/sd/sd.h b/include/hw/sd/sd.h
index 6874696ff7..1f1ec2a042 100644
--- a/include/hw/sd/sd.h
+++ b/include/hw/sd/sd.h
@@ -58,6 +58,7 @@
 typedef enum {
     PROTO_SD =  0,
     PROTO_SPI = 1 << 1,
+    PROTO_MMC = 1 << 2,
 } sd_bus_protocol_t;
 
 typedef enum {
diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 6fbc640d2e..c4701f9c7c 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -63,6 +63,13 @@ typedef enum {
     SD_PHY_SPEC_VER_3_01 = 301, /* not yet supported */
 } sd_phy_spec_ver_t;
 
+typedef enum {
+    MMC_SPEC_VER_2_2    = 202,  /* not well supported */
+    /* e.MMC */
+    MMC_SPEC_VER_4_51   = 451,
+    MMC_SPEC_VER_5_1    = 501,  /* not supported */
+} mmc_phy_spec_ver_t;
+
 typedef enum {
     sd_capacity_unknown,
     sd_capacity_sdsc,           /* not well supported */
@@ -157,6 +164,8 @@ static const char *sd_protocol_name(sd_bus_protocol_t protocol)
         return "SD";
     case PROTO_SPI:
         return "SPI";
+    case PROTO_MMC:
+        return "MMC";
     default:
         g_assert_not_reached();
     }
@@ -299,78 +308,90 @@ typedef struct {
     struct {
         uint16_t version;
         uint32_t ccc_mask;
-    } sd, spi;
+    } sd, spi, mmc;
 } sd_cmd_supported_t;
 
 static const sd_cmd_supported_t cmd_supported[SDCARD_CMD_MAX] = {
-     /*           SD                  SPI          */
-     [0] = {{200, BIT(0)},      {200, BIT(0)},      },
-     [1] = {{301, BIT(0)},      {200, BIT(0)},      },
-     [2] = {{200, BIT(0)},      {},                 },
-     [3] = {{200, BIT(0)},      {},                 },
-     [4] = {{200, BIT(0)},      {},                 },
-     [5] = {{200, BIT(9)},      {200, BIT(9)},      },
-     [6] = {{200, BIT(10)},     {200, BIT(10)},     },
-     [7] = {{200, BIT(0)},      {},                 },
-     [8] = {{200, BIT(0)},      {200, BIT(0)},      },
-     [9] = {{200, BIT(0)},      {200, BIT(0)},      },
-    [10] = {{200, BIT(0)},      {200, BIT(0)},      },
-    [12] = {{200, BIT(0)},      {200, BIT(0)},      },
-    [13] = {{200, BIT(0)},      {200, BIT(0)},      },
-    [14] = {{200, BIT(0)},      {},                 },
-    [15] = {{200, BIT(0)},      {},                 },
-    [16] = {{200, BIT_2_4_7},   {200, BIT_2_4_7},   },
-    [17] = {{200, BIT(2)},      {200, BIT(2)},      },
-    [18] = {{200, BIT(2)},      {200, BIT(2)},      },
-    [23] = {{301, BIT_2_4},     {},                 },
-    [24] = {{200, BIT(4)},      {200, BIT(4)},      },
-    [25] = {{200, BIT(4)},      {200, BIT(4)},      },
-    [26] = {{200, BIT_MANUF},   {/*?*/},            },
-    [27] = {{200, BIT(4)},      {200, BIT(4)},      },
-    [28] = {{200, BIT(6)},      {200, BIT(6)},      },
-    [29] = {{200, BIT(6)},      {200, BIT(6)},      },
-    [30] = {{200, BIT(6)},      {200, BIT(6)},      },
-    [32] = {{200, BIT(5)},      {200, BIT(5)},      },
-    [33] = {{200, BIT(5)},      {200, BIT(5)},      },
-    [34] = {{200, BIT(10)},     {200, BIT(10)},     },
-    [35] = {{200, BIT(10)},     {200, BIT(10)},     },
-    [36] = {{200, BIT(10)},     {200, BIT(10)},     },
-    [37] = {{200, BIT(10)},     {200, BIT(10)},     },
-    [38] = {{200, BIT(5)},      {200, BIT(5)},      },
-    [42] = {{200, BIT(7)},      {200, BIT(7)},      },
-    [50] = {{200, BIT(10)},     {200, BIT(10)},     },
-    [52] = {{200, BIT(9)},      {200, BIT(9)},      },
-    [53] = {{200, BIT(9)},      {200, BIT(9)},      },
-    [54] = {{/* 2.00 SDIO */},  {/* 2.00 SDIO */},  },
-    [55] = {{200, BIT(8)},      {200, BIT(8)},      },
-    [56] = {{200, BIT(8)},      {200, BIT(8)},      },
-    [57] = {{200, BIT(10)},     {200, BIT(10)},     },
-    [58] = {{301, BIT(0)},      {200, BIT(0)},      },
-    [59] = {{301, BIT(0)},      {200, BIT(0)},      },
-    [60] = {{200, BIT_MANUF},   {/*?*/},            },
-    [61] = {{200, BIT_MANUF},   {/*?*/},            },
-    [62] = {{200, BIT_MANUF},   {/*?*/},            },
-    [63] = {{200, BIT_MANUF},   {/*?*/},            },
+     /*           SD                  SPI                 eMMC         */
+     [0] = {{200, BIT(0)},      {200, BIT(0)},      {451, BIT(0)}       },
+     [1] = {{301, BIT(0)},      {200, BIT(0)},      {451, BIT(0)}       },
+     [2] = {{200, BIT(0)},      {},                 {451, BIT(0)}       },
+     [3] = {{200, BIT(0)},      {},                 {451, BIT(0)}       },
+     [4] = {{200, BIT(0)},      {},                 {451, BIT(0)}       },
+     [5] = {{200, BIT(9)},      {200, BIT(9)},      {451, BIT(0)}       },
+     [6] = {{200, BIT(10)},     {200, BIT(10)},     {451, BIT(0)}       },
+     [7] = {{200, BIT(0)},      {},                 {451, BIT(0)}       },
+     [8] = {{200, BIT(0)},      {200, BIT(0)},      {451, BIT(0)}       },
+     [9] = {{200, BIT(0)},      {200, BIT(0)},      {451, BIT(0)}       },
+    [10] = {{200, BIT(0)},      {200, BIT(0)},      {451, BIT(0)}       },
+    [12] = {{200, BIT(0)},      {200, BIT(0)},      {451, BIT(0)}       },
+    [13] = {{200, BIT(0)},      {200, BIT(0)},      {451, BIT(0)}       },
+    [14] = {{200, BIT(0)},      {},                 {451, BIT(0)}       },
+    [15] = {{200, BIT(0)},      {},                 {451, BIT(0)}       },
+    [16] = {{200, BIT_2_4_7},   {200, BIT_2_4_7},   {451, BIT_2_4_7}    },
+    [17] = {{200, BIT(2)},      {200, BIT(2)},      {451, BIT(2)}       },
+    [18] = {{200, BIT(2)},      {200, BIT(2)},      {451, BIT(2)}       },
+    [19] = {{},                 {},                 {451, BIT(0)}       },
+    [21] = {{},                 {},                 {/* HS200 */}       },
+    [23] = {{301, BIT_2_4},     {},                 {/*BIT_2_4 ?*/}     },
+    [24] = {{200, BIT(4)},      {200, BIT(4)},      {451, BIT(4)}       },
+    [25] = {{200, BIT(4)},      {200, BIT(4)},      {451, BIT(4)}       },
+    [26] = {{200, BIT_MANUF},   {/*?*/},            {/*?*/}             },
+    [27] = {{200, BIT(4)},      {200, BIT(4)},      {451, BIT(4)}       },
+    [28] = {{200, BIT(6)},      {200, BIT(6)},      {451, BIT(6)}       },
+    [29] = {{200, BIT(6)},      {200, BIT(6)},      {451, BIT(6)}       },
+    [30] = {{200, BIT(6)},      {200, BIT(6)},      {451, BIT(6)}       },
+    [31] = {{},                 {},                 {451, BIT(6)}       },
+    [32] = {{200, BIT(5)},      {200, BIT(5)},      {}                  },
+    [33] = {{200, BIT(5)},      {200, BIT(5)},      {}                  },
+    [34] = {{200, BIT(10)},     {200, BIT(10)},     {}                  },
+    [35] = {{200, BIT(10)},     {200, BIT(10)},     {451, BIT(5)}       },
+    [36] = {{200, BIT(10)},     {200, BIT(10)},     {451, BIT(5)}       },
+    [37] = {{200, BIT(10)},     {200, BIT(10)},     {},                 },
+    [38] = {{200, BIT(5)},      {200, BIT(5)},      {451, BIT(5)}       },
+    [39] = {{},                 {},                 {451, BIT(9)}       },
+    [40] = {{},                 {},                 {451, BIT(9)}       },
+    [41] = {{},                 {},                 {/*451, BIT(7)*/}   },
+    [42] = {{200, BIT(7)},      {200, BIT(7)},      {451, BIT(4)}       },
+    [44] = {{},                 {},                 {501, BIT(10)}      },
+    [45] = {{},                 {},                 {501, BIT(10)}      },
+    [46] = {{},                 {},                 {501, BIT(10)}      },
+    [47] = {{},                 {},                 {501, BIT(10)}      },
+    [48] = {{},                 {},                 {501, BIT(10)}      },
+    [49] = {{},                 {},                 {451, BIT(4)}       },
+    [50] = {{200, BIT(10)},     {200, BIT(10)},     {}                  },
+    [52] = {{200, BIT(9)},      {200, BIT(9)},      {}                  },
+    [53] = {{200, BIT(9)},      {200, BIT(9)},      {451, BIT(10)}      },
+    [54] = {{/* 2.00 SDIO */},  {/* 2.00 SDIO */},  {451, BIT(10)}      },
+    [55] = {{200, BIT(8)},      {200, BIT(8)},      {451, BIT(8)}       },
+    [56] = {{200, BIT(8)},      {200, BIT(8)},      {451, BIT(8)}       },
+    [57] = {{200, BIT(10)},     {200, BIT(10)},     {}                  },
+    [58] = {{301, BIT(0)},      {200, BIT(0)},      {}                  },
+    [59] = {{301, BIT(0)},      {200, BIT(0)},      {}                  },
+    [60] = {{200, BIT_MANUF},   {/*?*/},            {/*?*/}             },
+    [61] = {{200, BIT_MANUF},   {/*?*/},            {/*?*/}             },
+    [62] = {{200, BIT_MANUF},   {/*?*/},            {/*?*/}             },
+    [63] = {{200, BIT_MANUF},   {/*?*/},            {/*?*/}             },
 }, acmd_supported[SDCARD_CMD_MAX] = {
-     /*           SD                  SPI          */
-     [6] = {{200, BIT(8)},      {},                 },
-    [13] = {{200, BIT(8)},      {200, BIT(8)},      },
-    [18] = {{200, BIT_SECU},    {200, BIT_SECU},    },
-    [22] = {{200, BIT(8)},      {200, BIT(8)},      },
-    [23] = {{200, BIT(8)},      {200, BIT(8)},      },
-    [25] = {{200, BIT_SECU},    {200, BIT_SECU},    },
-    [26] = {{200, BIT_SECU},    {200, BIT_SECU},    },
-    [38] = {{200, BIT_SECU},    {200, BIT_SECU},    },
-    [41] = {{200, BIT(8)},      {200, BIT(8)},      },
-    [42] = {{200, BIT(8)},      {200, BIT(8)},      },
-    [43] = {{200, BIT_SECU},    {200, BIT_SECU},    },
-    [44] = {{200, BIT_SECU},    {200, BIT_SECU},    },
-    [45] = {{200, BIT_SECU},    {200, BIT_SECU},    },
-    [46] = {{200, BIT_SECU},    {200, BIT_SECU},    },
-    [47] = {{200, BIT_SECU},    {200, BIT_SECU},    },
-    [48] = {{200, BIT_SECU},    {200, BIT_SECU},    },
-    [49] = {{200, BIT_SECU},    {200, BIT_SECU},    },
-    [51] = {{200, BIT(8)},      {200, BIT(8)},      },
+     /*           SD                  SPI                 eMMC         */
+     [6] = {{200, BIT(8)},      {},                 {451, BIT(0)}       },
+    [13] = {{200, BIT(8)},      {200, BIT(8)},      {451, BIT(0)}       },
+    [18] = {{200, BIT_SECU},    {200, BIT_SECU},    {451, BIT(0)}       },
+    [22] = {{200, BIT(8)},      {200, BIT(8)},      {451, BIT(4)}       },
+    [23] = {{200, BIT(8)},      {200, BIT(8)},      {451, BIT(4)}       },
+    [25] = {{200, BIT_SECU},    {200, BIT_SECU},    {451, BIT(0)}       },
+    [26] = {{200, BIT_SECU},    {200, BIT_SECU},    {451, BIT(0)}       },
+    [38] = {{200, BIT_SECU},    {200, BIT_SECU},    {451, BIT(0)}       },
+    [41] = {{200, BIT(8)},      {200, BIT(8)},      {451, BIT(8)}       },
+    [42] = {{200, BIT(8)},      {200, BIT(8)},      {451, BIT(0)}       },
+    [43] = {{200, BIT_SECU},    {200, BIT_SECU},    {451, BIT(0)}       },
+    [44] = {{200, BIT_SECU},    {200, BIT_SECU},    {451, BIT(0)}       },
+    [45] = {{200, BIT_SECU},    {200, BIT_SECU},    {451, BIT(0)}       },
+    [46] = {{200, BIT_SECU},    {200, BIT_SECU},    {451, BIT(0)}       },
+    [47] = {{200, BIT_SECU},    {200, BIT_SECU},    {451, BIT(0)}       },
+    [48] = {{200, BIT_SECU},    {200, BIT_SECU},    {451, BIT(0)}       },
+    [49] = {{200, BIT_SECU},    {200, BIT_SECU},    {451, BIT(0)}       },
+    [51] = {{200, BIT(8)},      {200, BIT(8)},      {451, BIT(8)}       },
 };
 
 static const char *spec_version_name(uint16_t spec_version)
@@ -378,8 +399,14 @@ static const char *spec_version_name(uint16_t spec_version)
     switch (spec_version) {
     case SD_PHY_SPEC_VER_2_00:
         return "v2.00";
+    case MMC_SPEC_VER_2_2:
+        return "v2.2";
     case SD_PHY_SPEC_VER_3_01:
         return "v3.01";
+    case MMC_SPEC_VER_4_51:
+        return "v4.51";
+    case MMC_SPEC_VER_5_1:
+        return "v5.1";
     default:
         g_assert_not_reached();
     }
@@ -397,6 +424,9 @@ static bool cmd_version_supported(SDState *sd, uint8_t cmd, bool is_acmd)
     case PROTO_SPI:
         cmd_version = cmdset[cmd].spi.version;
         break;
+    case PROTO_MMC:
+        cmd_version = cmdset[cmd].mmc.version;
+        break;
     default:
         g_assert_not_reached();
     }
@@ -424,6 +454,9 @@ static bool cmd_class_supported(SDState *sd, uint8_t cmd, uint8_t class,
         /* class 1, 3 and 9 are not supported in SPI mode */
         cmd_ccc_mask = cmdset[cmd].spi.ccc_mask;
         break;
+    case PROTO_MMC:
+        cmd_ccc_mask = cmdset[cmd].mmc.ccc_mask;
+        break;
     default:
         g_assert_not_reached();
     }
@@ -602,7 +635,7 @@ static void sd_reset_csd(SDState *sd, uint64_t size)
 
 static void sd_reset_rca(SDState *sd)
 {
-    sd->rca = 0;
+    sd->rca = sd->bus_protocol == PROTO_MMC;
 }
 
 static void sd_set_rca(SDState *sd)
@@ -907,6 +940,9 @@ static SDState *sdcard_init(BlockBackend *blk, sd_bus_protocol_t bus_protocol)
     case PROTO_SPI:
         qdev_prop_set_bit(dev, "spi", true);
         break;
+    case PROTO_MMC:
+        qdev_prop_set_bit(dev, "mmc", true);
+        break;
     default:
         break;
     }
@@ -2163,7 +2199,11 @@ static void sd_realize(DeviceState *dev, Error **errp)
     int ret;
 
     sd->proto_name = sd_protocol_name(sd->bus_protocol);
-    sd->spec_version = SD_PHY_SPEC_VER_2_00;
+    if (sd->bus_protocol == PROTO_MMC) {
+        sd->spec_version = MMC_SPEC_VER_4_51;
+    } else {
+        sd->spec_version = SD_PHY_SPEC_VER_2_00;
+    }
 
     if (sd->blk && blk_is_read_only(sd->blk)) {
         error_setg(errp, "Cannot use read-only drive as SD card");
@@ -2209,6 +2249,7 @@ static Property sd_properties[] = {
      * board to ensure that ssi transfers only occur when the chip select
      * is asserted.  */
     DEFINE_PROP_BIT("spi", SDState, bus_protocol, 1, false),
+    DEFINE_PROP_BIT("mmc", SDState, bus_protocol, 2, false),
     DEFINE_PROP_END_OF_LIST()
 };
 
-- 
2.15.1