[PATCH] hw/sd/sdhci: let i.MX (u)SDHC issue commands without SDCLK_EN

[PATCH] hw/sd/sdhci: let i.MX (u)SDHC issue commands without SDCLK_EN

[Kyle Fox]

The i.MX (u)SDHC gates the SD card clock in hardware and has no
software-visible "SD Clock Enable" bit, so the Linux sdhci-esdhc-imx
driver programs only the divider and the internal-clock enable and never
sets SDHC_CLOCK_SDCLK_EN. The generic command-dispatch gate,
sdhci_can_issue_command() -> SDHC_CLOCK_IS_ON(), requires that bit, so
under Linux every command issued to such a controller is silently
dropped: no command-complete or command-timeout interrupt is raised, and
the guest mmc layer falls back to a 10s host-side timeout per command,
indefinitely. (The existing VENDORSPEC->clkcon mirror only covered the
U-Boot fsl_esdhc_imx clock path, which does toggle the VENDORSPEC enable
bits; the upstream Linux driver does not.)

On the i.MX 95 machine this wedges shutdown: with no backing storage the
empty eMMC slot (non-removable, always probed) and SD slot both spin on
these 10s timeouts, so device_shutdown() never returns and the guest
never reaches PSCI SYSTEM_OFF -- a guest poweroff fails to terminate
QEMU, and the controllers spew sdhci-esdhc-imx debug-status dumps the
whole time.

Add SDHCI_QUIRK_SDCLK_AUTO_GATE: when set, the controller is considered
ready to issue commands once the internal clock is enabled and stable,
without also requiring SDHC_CLOCK_SDCLK_EN. Set it on the three i.MX
variants (imx-usdhc, fsl-esdhc-be, fsl-esdhc-le). With the clock gate
relaxed, an absent card is reported promptly as a command timeout, card
init fails in milliseconds instead of 10s, device_shutdown() completes,
and guest poweroff terminates the VM cleanly.

The change is strictly more permissive than the previous gate (every
state that satisfied SDHC_CLOCK_IS_ON still dispatches), so controllers
without the quirk are unaffected; the sdhci qtests (xlnx-zcu102 and
sdhci-pci, neither of which sets the quirk) still pass.

Signed-off-by: Kyle Fox <kylefoxaustin.github@gmail.com>

---
 hw/sd/sdhci.c         | 25 +++++++++++++++++++++----
 include/hw/sd/sdhci.h | 11 +++++++++++
 2 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 4ad96a8760..d52335424b 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -1013,9 +1013,26 @@ static void sdhci_data_transfer(void *opaque)
     }
 }
 
+static bool sdhci_clock_is_on(SDHCIState *s)
+{
+    /*
+     * The i.MX (u)SDHC has no software SD-clock-enable bit (the card clock
+     * is auto-gated by hardware), so its driver never sets
+     * SDHC_CLOCK_SDCLK_EN. For such a controller, treat the clock as
+     * running once the internal clock is enabled and stable; otherwise
+     * every command would be silently dropped (no completion/timeout IRQ),
+     * stalling the guest on 10s host-side timeouts.
+     */
+    if (s->quirks & SDHCI_QUIRK_SDCLK_AUTO_GATE) {
+        return (s->clkcon & (SDHC_CLOCK_INT_EN | SDHC_CLOCK_INT_STABLE)) ==
+               (SDHC_CLOCK_INT_EN | SDHC_CLOCK_INT_STABLE);
+    }
+    return SDHC_CLOCK_IS_ON(s->clkcon);
+}
+
 static bool sdhci_can_issue_command(SDHCIState *s)
 {
-    if (!SDHC_CLOCK_IS_ON(s->clkcon) ||
+    if (!sdhci_clock_is_on(s) ||
         (((s->prnsts & SDHC_DATA_INHIBIT) || s->stopped_state) &&
         ((s->cmdreg & SDHC_CMD_DATA_PRESENT) ||
         ((s->cmdreg & SDHC_CMD_RESPONSE) == SDHC_CMD_RSP_WITH_BUSY &&
@@ -1905,7 +1922,7 @@ static void fsl_esdhc_be_init(Object *obj)
     DeviceState *dev = DEVICE(obj);
 
     s->io_ops = &esdhc_mmio_be_ops;
-    s->quirks = SDHCI_QUIRK_NO_BUSY_IRQ;
+    s->quirks = SDHCI_QUIRK_NO_BUSY_IRQ | SDHCI_QUIRK_SDCLK_AUTO_GATE;
     qdev_prop_set_uint8(dev, "sd-spec-version", 2);
 }
 
@@ -1930,7 +1947,7 @@ static void fsl_esdhc_le_init(Object *obj)
     DeviceState *dev = DEVICE(obj);
 
     s->io_ops = &esdhc_mmio_le_ops;
-    s->quirks = SDHCI_QUIRK_NO_BUSY_IRQ;
+    s->quirks = SDHCI_QUIRK_NO_BUSY_IRQ | SDHCI_QUIRK_SDCLK_AUTO_GATE;
     qdev_prop_set_uint8(dev, "sd-spec-version", 2);
 }
 
@@ -1951,7 +1968,7 @@ static void imx_usdhc_init(Object *obj)
     DeviceState *dev = DEVICE(obj);
 
     s->io_ops = &usdhc_mmio_ops;
-    s->quirks = SDHCI_QUIRK_NO_BUSY_IRQ;
+    s->quirks = SDHCI_QUIRK_NO_BUSY_IRQ | SDHCI_QUIRK_SDCLK_AUTO_GATE;
     qdev_prop_set_uint8(dev, "sd-spec-version", 3);
 }
 
diff --git a/include/hw/sd/sdhci.h b/include/hw/sd/sdhci.h
index a9da6203fc..d96f9c1dfa 100644
--- a/include/hw/sd/sdhci.h
+++ b/include/hw/sd/sdhci.h
@@ -115,6 +115,17 @@ typedef struct SDHCIState SDHCIState;
  */
 #define SDHCI_QUIRK_NO_BUSY_IRQ    BIT(14)
 
+/*
+ * The i.MX (u)SDHC gates the SD card clock automatically and has no
+ * software-visible "SD Clock Enable" bit: its driver programs only the
+ * divider and the internal-clock enable, never SDHC_CLOCK_SDCLK_EN. With
+ * this quirk the controller is considered ready to issue commands once the
+ * internal clock is enabled and stable, instead of also requiring
+ * SDHC_CLOCK_SDCLK_EN (which such a driver never sets). QEMU-internal bit,
+ * not mirrored from Linux.
+ */
+#define SDHCI_QUIRK_SDCLK_AUTO_GATE    BIT(16)
+
 #define TYPE_PCI_SDHCI "sdhci-pci"
 DECLARE_INSTANCE_CHECKER(SDHCIState, PCI_SDHCI,
                          TYPE_PCI_SDHCI)
-- 
2.34.1