[Qemu-devel] [PATCH 2/2] hw/sd: model a power-up delay, as a workaround for an EDK2 bug

[Andrew Baumann <Andrew.Baumann@microsoft.com>]

The SD spec for ACMD41 says that a zero argument is an "inquiry"
ACMD41, which does not start initialisation and is used only for
retrieving the OCR. However, Tianocore EDK2 (UEFI) has a bug [1]: it
first sends an inquiry (zero) ACMD41. If that first request returns an
OCR value with the power up bit (0x80000000) set, it assumes the card
is ready and continues, leaving the card in the wrong state. (My
assumption is that this works on hardware, because no real card is
immediately powered up upon reset.)

This change implements a simple workaround: the first time an ACMD41
is issued, the OCR is returned without the power up bit set. Upon
subsequent reads, the OCR reports power up.

[1] https://github.com/tianocore/edk2/blob/master/EmbeddedPkg/Universal/MmcDxe/MmcIdentification.c#L279
(This is the loop starting with "We need to wait for the MMC or SD
card is ready")

Signed-off-by: Andrew Baumann <Andrew.Baumann@microsoft.com>
---
Obviously this is a bug that should be fixed in EDK2. However, this
initialisation appears to have been around for quite a while in EDK2
(in various forms), and the fact that it has obviously worked with so
many real SD/MMC cards makes me think that it would be pragmatic to
have the workaround in QEMU as well.

 hw/sd/sd.c | 30 ++++++++++++++++++++++++++----
 1 file changed, 26 insertions(+), 4 deletions(-)

diff --git a/hw/sd/sd.c b/hw/sd/sd.c
index 98af8bc..31a25bc 100644
--- a/hw/sd/sd.c
+++ b/hw/sd/sd.c
@@ -80,6 +80,7 @@ struct SDState {
     uint32_t mode;    /* current card mode, one of SDCardModes */
     int32_t state;    /* current card state, one of SDCardStates */
     uint32_t ocr;
+    bool ocr_initdelay;
     uint8_t scr[8];
     uint8_t cid[16];
     uint8_t csd[16];
@@ -195,8 +196,21 @@ static uint16_t sd_crc16(void *message, size_t width)
 
 static void sd_set_ocr(SDState *sd)
 {
-    /* All voltages OK, card power-up OK, Standard Capacity SD Memory Card */
-    sd->ocr = 0x80ffff00;
+    /* All voltages OK, Standard Capacity SD Memory Card, not yet powered up */
+    sd->ocr = 0x00ffff00;
+    sd->ocr_initdelay = false;
+}
+
+static bool sd_model_ocr_init_delay(SDState *sd)
+{
+    if (!sd->ocr_initdelay) {
+        sd->ocr_initdelay = true;
+        return false;
+    } else {
+        /* Set powered up bit in OCR */
+        sd->ocr |= 0x80000000;
+        return true;
+    }
 }
 
 static void sd_set_scr(SDState *sd)
@@ -451,6 +465,7 @@ static const VMStateDescription sd_vmstate = {
     .fields = (VMStateField[]) {
         VMSTATE_UINT32(mode, SDState),
         VMSTATE_INT32(state, SDState),
+        VMSTATE_BOOL(ocr_initdelay, SDState),
         VMSTATE_UINT8_ARRAY(cid, SDState, 16),
         VMSTATE_UINT8_ARRAY(csd, SDState, 16),
         VMSTATE_UINT16(rca, SDState),
@@ -1300,10 +1315,17 @@ static sd_rsp_type_t sd_app_command(SDState *sd,
         case sd_idle_state:
             /* We accept any voltage.  10000 V is nothing.
              *
-             * We don't model init delay so just advance straight to ready state
+             * We model an init "delay" of one polling cycle, as a
+             * workaround for around a bug in TianoCore EDK2 which
+             * sends an initial zero ACMD41, but nevertheless assumes
+             * that the card is in ready state as soon as it sees the
+             * power up bit set.
+             *
+             * Once we've delayed, we advance straight to ready state
              * unless it's an enquiry ACMD41 (bits 23:0 == 0).
              */
-            if (req.arg & ACMD41_ENQUIRY_MASK) {
+            if (sd_model_ocr_init_delay(sd)
+                && (req.arg & ACMD41_ENQUIRY_MASK)) {
                 sd->state = sd_ready_state;
             }
 
-- 
2.5.3