[Qemu-devel] [PATCH 00/10] hw/sd.c: Fix various status related bugs

[Qemu-devel] [PATCH 00/10] hw/sd.c: Fix various status related bugs

[Peter Maydell]

This patchset fixes a number of bugs in our SD card emulation, mostly
in the status bit handling. In particular, it fixes the issues raised
in https://bugs.launchpad.net/qemu/+bug/597641 . The others are things
I noticed while I was poking around in the code.

Patches 01-04, 07 are pretty straightforward. 05, 06 are refactoring for
the benefit of later patches. 08 and 09 are more interesting. 10 makes
sense to me although the spec is rather vague on the point.

Peter Maydell (10):
  hw/sd.c: Fix the set of commands which are failed when card is locked
  hw/sd.c: Add comment regarding CARD_STATUS_* defines
  hw/sd.c: On CRC error, set CRC error status bit rather than clearing it
  hw/sd.c: When setting ADDRESS_ERROR bit, don't clear everything else
  hw/sd.c: Handle illegal commands in sd_do_command
  hw/sd.c: Handle CRC and locked-card errors in normal code path
  hw/sd.c: Set ILLEGAL_COMMAND for ACMDs in invalid state
  hw/sd.c: Correct handling of type B SD status bits
  hw/sd.c: Correct handling of APP_CMD status bit
  hw/sd.c: Clear status bits when read via response r6

 hw/sd.c |  131 ++++++++++++++++++++++++++++++++++++++++----------------------
 1 files changed, 84 insertions(+), 47 deletions(-)

-- 
1.7.5.4

[Qemu-devel] [PATCH 01/10] hw/sd.c: Fix the set of commands which are failed when card is locked

[Peter Maydell]

Fix bugs in the code determining whether to accept a card when the
SD card is locked. Most notably, we had the condition completely
reversed, so we would accept all the commands we should refuse and
refuse all the commands we should accept. Correct this by refactoring
the enormous if () clause into a separate function.
We had also missed ACMD42 off the list of commands which are accepted
in locked state: add it.

This is one of the two problems reported in LP:597641.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/sd.c |   29 ++++++++++++++++++++++-------
 1 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/hw/sd.c b/hw/sd.c
index 10e26ad..a1c98c0 100644
--- a/hw/sd.c
+++ b/hw/sd.c
@@ -1265,6 +1265,25 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
     return sd_r0;
 }
 
+static int cmd_valid_while_locked(SDState *sd, SDRequest *req)
+{
+    /* Valid commands in locked state:
+     * basic class (0)
+     * lock card class (7)
+     * CMD16
+     * implicitly, the ACMD prefix CMD55
+     * ACMD41 and ACMD42
+     * Anything else provokes an "illegal command" response.
+     */
+    if (sd->card_status & APP_CMD) {
+        return req->cmd == 41 || req->cmd == 42;
+    }
+    if (req->cmd == 16 || req->cmd == 55) {
+        return 1;
+    }
+    return sd_cmd_class[req->cmd] == 0 || sd_cmd_class[req->cmd] == 7;
+}
+
 int sd_do_command(SDState *sd, SDRequest *req,
                   uint8_t *response) {
     uint32_t last_status = sd->card_status;
@@ -1283,17 +1302,13 @@ int sd_do_command(SDState *sd, SDRequest *req,
     sd->card_status &= ~CARD_STATUS_B;
     sd_set_status(sd);
 
-    if (last_status & CARD_IS_LOCKED)
-        if (((last_status & APP_CMD) &&
-                                 req->cmd == 41) ||
-                        (!(last_status & APP_CMD) &&
-                         (sd_cmd_class[req->cmd] == 0 ||
-                          sd_cmd_class[req->cmd] == 7 ||
-                          req->cmd == 16 || req->cmd == 55))) {
+    if (last_status & CARD_IS_LOCKED) {
+        if (!cmd_valid_while_locked(sd, req)) {
             sd->card_status |= ILLEGAL_COMMAND;
             fprintf(stderr, "SD: Card is locked\n");
             return 0;
         }
+    }
 
     if (last_status & APP_CMD) {
         rtype = sd_app_command(sd, *req);
-- 
1.7.5.4

[Qemu-devel] [PATCH 02/10] hw/sd.c: Add comment regarding CARD_STATUS_* defines

[Peter Maydell]

Add a clarifying comment about what the CARD_STATUS_[ABC]
macros are defining.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/sd.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/hw/sd.c b/hw/sd.c
index a1c98c0..245b6c3 100644
--- a/hw/sd.c
+++ b/hw/sd.c
@@ -309,6 +309,11 @@ static void sd_set_rca(SDState *sd)
     sd->rca += 0x4567;
 }
 
+/* Card status bits, split by clear condition:
+ * A : According to the card current state
+ * B : Always related to the previous command
+ * C : Cleared by read
+ */
 #define CARD_STATUS_A	0x02004100
 #define CARD_STATUS_B	0x00c01e00
 #define CARD_STATUS_C	0xfd39a028
-- 
1.7.5.4

[Qemu-devel] [PATCH 03/10] hw/sd.c: On CRC error, set CRC error status bit rather than clearing it

[Peter Maydell]

If we fail to validate the CRC for an SD command we should be setting
COM_CRC_ERROR, not clearing it. (This bug actually has no effect currently
because sd_req_crc_validate() always returns success.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/sd.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/sd.c b/hw/sd.c
index 245b6c3..e57852e 100644
--- a/hw/sd.c
+++ b/hw/sd.c
@@ -1300,7 +1300,7 @@ int sd_do_command(SDState *sd, SDRequest *req,
     }
 
     if (sd_req_crc_validate(req)) {
-        sd->card_status &= ~COM_CRC_ERROR;
+        sd->card_status |= COM_CRC_ERROR;
         return 0;
     }
 
-- 
1.7.5.4

[Qemu-devel] [PATCH 04/10] hw/sd.c: When setting ADDRESS_ERROR bit, don't clear everything else

[Peter Maydell]

Fix a typo that meant that ADDRESS_ERRORs setting or clearing write
protection would clear every other bit in the status register.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/sd.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/sd.c b/hw/sd.c
index e57852e..dd28061 100644
--- a/hw/sd.c
+++ b/hw/sd.c
@@ -999,7 +999,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
         switch (sd->state) {
         case sd_transfer_state:
             if (addr >= sd->size) {
-                sd->card_status = ADDRESS_ERROR;
+                sd->card_status |= ADDRESS_ERROR;
                 return sd_r1b;
             }
 
@@ -1019,7 +1019,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
         switch (sd->state) {
         case sd_transfer_state:
             if (addr >= sd->size) {
-                sd->card_status = ADDRESS_ERROR;
+                sd->card_status |= ADDRESS_ERROR;
                 return sd_r1b;
             }
 
-- 
1.7.5.4

[Qemu-devel] [PATCH 05/10] hw/sd.c: Handle illegal commands in sd_do_command

[Peter Maydell]

Add an extra sd_illegal value to the sd_rsp_type_t enum so that
sd_app_command() and sd_normal_command() can tell sd_do_command()
that the command was illegal. This is needed so we can do things
like reset certain status bits only on receipt of a valid command.
For the moment, just use it to pull out the setting of the
ILLEGAL_COMMAND status bit into sd_do_command().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/sd.c |   25 +++++++++++--------------
 1 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/hw/sd.c b/hw/sd.c
index dd28061..57925af 100644
--- a/hw/sd.c
+++ b/hw/sd.c
@@ -51,6 +51,7 @@ typedef enum {
     sd_r6 = 6,    /* Published RCA response */
     sd_r7,        /* Operating voltage */
     sd_r1b = -1,
+    sd_illegal = -2,
 } sd_rsp_type_t;
 
 struct SDState {
@@ -679,8 +680,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
         break;
 
     case 5: /* CMD5: reserved for SDIO cards */
-        sd->card_status |= ILLEGAL_COMMAND;
-        return sd_r0;
+        return sd_illegal;
 
     case 6:	/* CMD6:   SWITCH_FUNCTION */
         if (sd->spi)
@@ -1115,8 +1115,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
          * on stderr, as some OSes may use these in their
          * probing for presence of an SDIO card.
          */
-        sd->card_status |= ILLEGAL_COMMAND;
-        return sd_r0;
+        return sd_illegal;
 
     /* Application specific commands (Class 8) */
     case 55:	/* CMD55:  APP_CMD */
@@ -1145,21 +1144,17 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
 
     default:
     bad_cmd:
-        sd->card_status |= ILLEGAL_COMMAND;
-
         fprintf(stderr, "SD: Unknown CMD%i\n", req.cmd);
-        return sd_r0;
+        return sd_illegal;
 
     unimplemented_cmd:
         /* Commands that are recognised but not yet implemented in SPI mode.  */
-        sd->card_status |= ILLEGAL_COMMAND;
         fprintf(stderr, "SD: CMD%i not implemented in SPI mode\n", req.cmd);
-        return sd_r0;
+        return sd_illegal;
     }
 
-    sd->card_status |= ILLEGAL_COMMAND;
     fprintf(stderr, "SD: CMD%i in a wrong state\n", req.cmd);
-    return sd_r0;
+    return sd_illegal;
 }
 
 static sd_rsp_type_t sd_app_command(SDState *sd,
@@ -1321,6 +1316,10 @@ int sd_do_command(SDState *sd, SDRequest *req,
     } else
         rtype = sd_normal_command(sd, *req);
 
+    if (rtype == sd_illegal) {
+        sd->card_status |= ILLEGAL_COMMAND;
+    }
+
     sd->current_cmd = req->cmd;
 
     switch (rtype) {
@@ -1356,14 +1355,12 @@ int sd_do_command(SDState *sd, SDRequest *req,
         break;
 
     case sd_r0:
+    case sd_illegal:
     default:
         rsplen = 0;
         break;
     }
 
-    if (sd->card_status & ILLEGAL_COMMAND)
-        rsplen = 0;
-
 #ifdef DEBUG_SD
     if (rsplen) {
         int i;
-- 
1.7.5.4

[Qemu-devel] [PATCH 06/10] hw/sd.c: Handle CRC and locked-card errors in normal code path

[Peter Maydell]

Handle returning CRC and locked-card errors in the same code path
we use for other responses. This makes no difference in behaviour
but means that these error responses will be printed by the debug
logging code.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/sd.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/sd.c b/hw/sd.c
index 57925af..9116f67 100644
--- a/hw/sd.c
+++ b/hw/sd.c
@@ -1296,7 +1296,8 @@ int sd_do_command(SDState *sd, SDRequest *req,
 
     if (sd_req_crc_validate(req)) {
         sd->card_status |= COM_CRC_ERROR;
-        return 0;
+        rtype = sd_illegal;
+        goto send_response;
     }
 
     sd->card_status &= ~CARD_STATUS_B;
@@ -1306,7 +1307,8 @@ int sd_do_command(SDState *sd, SDRequest *req,
         if (!cmd_valid_while_locked(sd, req)) {
             sd->card_status |= ILLEGAL_COMMAND;
             fprintf(stderr, "SD: Card is locked\n");
-            return 0;
+            rtype = sd_illegal;
+            goto send_response;
         }
     }
 
@@ -1322,6 +1324,7 @@ int sd_do_command(SDState *sd, SDRequest *req,
 
     sd->current_cmd = req->cmd;
 
+send_response:
     switch (rtype) {
     case sd_r1:
     case sd_r1b:
-- 
1.7.5.4

[Qemu-devel] [PATCH 07/10] hw/sd.c: Set ILLEGAL_COMMAND for ACMDs in invalid state

[Peter Maydell]

App commands in an invalid state should set ILLEGAL_COMMAND, not
merely return a zero response.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/sd.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/sd.c b/hw/sd.c
index 9116f67..9eebfac 100644
--- a/hw/sd.c
+++ b/hw/sd.c
@@ -1262,7 +1262,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
     }
 
     fprintf(stderr, "SD: ACMD%i in a wrong state\n", req.cmd);
-    return sd_r0;
+    return sd_illegal;
 }
 
 static int cmd_valid_while_locked(SDState *sd, SDRequest *req)
-- 
1.7.5.4

[Qemu-devel] [PATCH 08/10] hw/sd.c: Correct handling of type B SD status bits

[Peter Maydell]

Correct how we handle the type B ("cleared on valid command")
status bits. In particular, the CURRENT_STATE bits in a response
should be the state of the card when it received that command,
not the state when it received the preceding command. (This is
one of the issues noted in LP:597641.)

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/sd.c |   44 +++++++++++++++++++++++++-------------------
 1 files changed, 25 insertions(+), 19 deletions(-)

diff --git a/hw/sd.c b/hw/sd.c
index 9eebfac..e1565b6 100644
--- a/hw/sd.c
+++ b/hw/sd.c
@@ -104,7 +104,7 @@ struct SDState {
     int enable;
 };
 
-static void sd_set_status(SDState *sd)
+static void sd_set_mode(SDState *sd)
 {
     switch (sd->state) {
     case sd_inactive_state:
@@ -126,9 +126,6 @@ static void sd_set_status(SDState *sd)
         sd->mode = sd_data_transfer_mode;
         break;
     }
-
-    sd->card_status &= ~CURRENT_STATE;
-    sd->card_status |= sd->state << 9;
 }
 
 static const sd_cmd_type_t sd_cmd_type[64] = {
@@ -341,13 +338,10 @@ 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, uint32_t last_status)
+static void sd_response_r1_make(SDState *sd, uint8_t *response)
 {
-    uint32_t mask = CARD_STATUS_B ^ ILLEGAL_COMMAND;
-    uint32_t status;
-
-    status = (sd->card_status & ~mask) | (last_status & mask);
+    uint32_t status = sd->card_status;
+    /* Clear the "clear on read" status bits (except APP_CMD) */
     sd->card_status &= ~CARD_STATUS_C | APP_CMD;
 
     response[0] = (status >> 24) & 0xff;
@@ -1286,7 +1280,7 @@ static int cmd_valid_while_locked(SDState *sd, SDRequest *req)
 
 int sd_do_command(SDState *sd, SDRequest *req,
                   uint8_t *response) {
-    uint32_t last_status = sd->card_status;
+    int last_state;
     sd_rsp_type_t rtype;
     int rsplen;
 
@@ -1300,10 +1294,7 @@ int sd_do_command(SDState *sd, SDRequest *req,
         goto send_response;
     }
 
-    sd->card_status &= ~CARD_STATUS_B;
-    sd_set_status(sd);
-
-    if (last_status & CARD_IS_LOCKED) {
+    if (sd->card_status & CARD_IS_LOCKED) {
         if (!cmd_valid_while_locked(sd, req)) {
             sd->card_status |= ILLEGAL_COMMAND;
             fprintf(stderr, "SD: Card is locked\n");
@@ -1312,7 +1303,10 @@ int sd_do_command(SDState *sd, SDRequest *req,
         }
     }
 
-    if (last_status & APP_CMD) {
+    last_state = sd->state;
+    sd_set_mode(sd);
+
+    if (sd->card_status & APP_CMD) {
         rtype = sd_app_command(sd, *req);
         sd->card_status &= ~APP_CMD;
     } else
@@ -1320,15 +1314,20 @@ int sd_do_command(SDState *sd, SDRequest *req,
 
     if (rtype == sd_illegal) {
         sd->card_status |= ILLEGAL_COMMAND;
+    } else {
+        /* Valid command, we can update the 'state before command' bits.
+         * (Do this now so they appear in r1 responses.)
+         */
+        sd->current_cmd = req->cmd;
+        sd->card_status &= ~CURRENT_STATE;
+        sd->card_status |= (last_state << 9);
     }
 
-    sd->current_cmd = req->cmd;
-
 send_response:
     switch (rtype) {
     case sd_r1:
     case sd_r1b:
-        sd_response_r1_make(sd, response, last_status);
+        sd_response_r1_make(sd, response);
         rsplen = 4;
         break;
 
@@ -1364,6 +1363,13 @@ send_response:
         break;
     }
 
+    if (rtype != sd_illegal) {
+        /* Clear the "clear on valid command" status bits now we've
+         * sent any response
+         */
+        sd->card_status &= ~CARD_STATUS_B;
+    }
+
 #ifdef DEBUG_SD
     if (rsplen) {
         int i;
-- 
1.7.5.4

[Qemu-devel] [PATCH 09/10] hw/sd.c: Correct handling of APP_CMD status bit

[Peter Maydell]

Fix some bugs in our implementation of the APP_CMD status bit:
 * the response to an ACMD should have APP_CMD set, not cleared
 * if an illegal ACMD is sent then the next command should be
   handled as a normal command

This requires that we split "card is expecting an ACMD" from
the state of the APP_CMD status bit (the latter indicates
both "expecting ACMD" and "that was an ACMD").

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/sd.c |   24 +++++++++++++++++-------
 1 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/hw/sd.c b/hw/sd.c
index e1565b6..6614cbf 100644
--- a/hw/sd.c
+++ b/hw/sd.c
@@ -92,6 +92,10 @@ struct SDState {
 
     int spi;
     int current_cmd;
+    /* True if we will handle the next command as an ACMD. Note that this does
+     * *not* track the APP_CMD status bit!
+     */
+    int expecting_acmd;
     int blk_written;
     uint64_t data_start;
     uint32_t data_offset;
@@ -341,8 +345,8 @@ static int sd_req_crc_validate(SDRequest *req)
 static void sd_response_r1_make(SDState *sd, uint8_t *response)
 {
     uint32_t status = sd->card_status;
-    /* Clear the "clear on read" status bits (except APP_CMD) */
-    sd->card_status &= ~CARD_STATUS_C | APP_CMD;
+    /* Clear the "clear on read" status bits */
+    sd->card_status &= ~CARD_STATUS_C;
 
     response[0] = (status >> 24) & 0xff;
     response[1] = (status >> 16) & 0xff;
@@ -608,6 +612,9 @@ 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;
 
+    /* Not interpreting this as an app command */
+    sd->card_status &= ~APP_CMD;
+
     if (sd_cmd_type[req.cmd] == sd_ac || sd_cmd_type[req.cmd] == sd_adtc)
         rca = req.arg >> 16;
 
@@ -1116,6 +1123,7 @@ static sd_rsp_type_t sd_normal_command(SDState *sd,
         if (sd->rca != rca)
             return sd_r0;
 
+        sd->expecting_acmd = 1;
         sd->card_status |= APP_CMD;
         return sd_r1;
 
@@ -1155,6 +1163,7 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
                                     SDRequest req)
 {
     DPRINTF("ACMD%d 0x%08x\n", req.cmd, req.arg);
+    sd->card_status |= APP_CMD;
     switch (req.cmd) {
     case 6:	/* ACMD6:  SET_BUS_WIDTH */
         switch (sd->state) {
@@ -1251,7 +1260,6 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
 
     default:
         /* Fall back to standard commands.  */
-        sd->card_status &= ~APP_CMD;
         return sd_normal_command(sd, req);
     }
 
@@ -1269,7 +1277,7 @@ static int cmd_valid_while_locked(SDState *sd, SDRequest *req)
      * ACMD41 and ACMD42
      * Anything else provokes an "illegal command" response.
      */
-    if (sd->card_status & APP_CMD) {
+    if (sd->expecting_acmd) {
         return req->cmd == 41 || req->cmd == 42;
     }
     if (req->cmd == 16 || req->cmd == 55) {
@@ -1297,6 +1305,7 @@ int sd_do_command(SDState *sd, SDRequest *req,
     if (sd->card_status & CARD_IS_LOCKED) {
         if (!cmd_valid_while_locked(sd, req)) {
             sd->card_status |= ILLEGAL_COMMAND;
+            sd->expecting_acmd = 0;
             fprintf(stderr, "SD: Card is locked\n");
             rtype = sd_illegal;
             goto send_response;
@@ -1306,11 +1315,12 @@ int sd_do_command(SDState *sd, SDRequest *req,
     last_state = sd->state;
     sd_set_mode(sd);
 
-    if (sd->card_status & APP_CMD) {
+    if (sd->expecting_acmd) {
+        sd->expecting_acmd = 0;
         rtype = sd_app_command(sd, *req);
-        sd->card_status &= ~APP_CMD;
-    } else
+    } else {
         rtype = sd_normal_command(sd, *req);
+    }
 
     if (rtype == sd_illegal) {
         sd->card_status |= ILLEGAL_COMMAND;
-- 
1.7.5.4

[Qemu-devel] [PATCH 10/10] hw/sd.c: Clear status bits when read via response r6

[Peter Maydell]

Response format r6 includes a subset of the status bits;
clear the clear-on-read bits which are read by an r6 response.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 hw/sd.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/hw/sd.c b/hw/sd.c
index 6614cbf..2b8ebe4 100644
--- a/hw/sd.c
+++ b/hw/sd.c
@@ -371,6 +371,7 @@ static void sd_response_r6_make(SDState *sd, uint8_t *response)
     status = ((sd->card_status >> 8) & 0xc000) |
              ((sd->card_status >> 6) & 0x2000) |
               (sd->card_status & 0x1fff);
+    sd->card_status &= ~(CARD_STATUS_C & 0xc81fff);
 
     response[0] = (arg >> 8) & 0xff;
     response[1] = arg & 0xff;
-- 
1.7.5.4

Re: [Qemu-devel] [PATCH 00/10] hw/sd.c: Fix various status related bugs

[andrzej zaborowski]

Hi Peter,

On 18 December 2011 21:37, Peter Maydell <peter.maydell@linaro.org> wrote:
> This patchset fixes a number of bugs in our SD card emulation, mostly
> in the status bit handling. In particular, it fixes the issues raised
> in https://bugs.launchpad.net/qemu/+bug/597641 . The others are things
> I noticed while I was poking around in the code.
>
> Patches 01-04, 07 are pretty straightforward. 05, 06 are refactoring for
> the benefit of later patches. 08 and 09 are more interesting. 10 makes
> sense to me although the spec is rather vague on the point.
>

Thanks, I pushed the series.  Some good catches here.  Also thanks to
bug reporter.

> Peter Maydell (10):
>  hw/sd.c: Fix the set of commands which are failed when card is locked

I replaced "card" with "command" in the commit message.

>  hw/sd.c: Add comment regarding CARD_STATUS_* defines
>  hw/sd.c: On CRC error, set CRC error status bit rather than clearing it
>  hw/sd.c: When setting ADDRESS_ERROR bit, don't clear everything else
>  hw/sd.c: Handle illegal commands in sd_do_command
>  hw/sd.c: Handle CRC and locked-card errors in normal code path
>  hw/sd.c: Set ILLEGAL_COMMAND for ACMDs in invalid state
>  hw/sd.c: Correct handling of type B SD status bits
>  hw/sd.c: Correct handling of APP_CMD status bit

I added resetting of .expecting_acmd in a separate patch.

>  hw/sd.c: Clear status bits when read via response r6

I thought it might be possible to test what bits real cards reset in
those cases, but then it would be problematic getting the card to set
each error bit.

Cheers

Re: [Qemu-devel] [PATCH 00/10] hw/sd.c: Fix various status related bugs

[Peter Maydell]

On 21 December 2011 03:54, andrzej zaborowski <balrogg@gmail.com> wrote:
> On 18 December 2011 21:37, Peter Maydell <peter.maydell@linaro.org> wrote:
>> This patchset fixes a number of bugs in our SD card emulation

> Thanks, I pushed the series.  Some good catches here.  Also thanks to
> bug reporter.
>
> I replaced "card" with "command" in the commit message.
> I added resetting of .expecting_acmd in a separate patch.

Thanks for catching these mistakes.

-- PMM