[PATCH] hw/sd/sdhci: complete non-interrupt ADMA descriptor chains in one pass

Wadim Mueller <[email protected]> Tue, 4 Aug 2026 10:14:59 +0200
Newsgroups gmane.comp.emulators.qemu,gmane.comp.emulators.qemu.block
Message-ID <[email protected]>
sdhci_do_adma() processes at most SDHC_ADMA_DESCS_PER_DELAY descriptors
per invocation and then reschedules itself SDHC_TRANSFER_DELAY ns later
on QEMU_CLOCK_VIRTUAL.  For a large bulk transfer this spreads the DMA
across hundreds of virtual-clock round-trips, advancing the guest's
virtual time between descriptor batches.

A guest that bounds the transfer with its own data/status timeout
(counted in guest time) can then see that timeout expire mid-transfer.
Concretely, U-Boot on a TI AM64x reading a ~28 MiB image with a single
CMD18 multi-block ADMA2 read intermittently aborts with "Timeout for
status update" and falls back to (failing) distro boot.  The transfer
itself is correct; only the artificial per-batch pacing triggers the
guest timeout.

Run a descriptor chain that carries no SDHC_ADMA_ATTR_INT attribute to
completion within a single call.  Chains that do request a DMA-boundary
interrupt still deliver it and reschedule exactly as before, so a guest
relying on that pacing is unaffected.

Now that the per-call bound is gone, cap the number of descriptors
processed per call so that a malformed or circular descriptor list (a
self-referencing link, or a persistently faulting non-END/non-INT
descriptor) cannot spin the calling thread forever; on overflow, break
to the existing reschedule path so the main loop stays responsive.

Signed-off-by: Wadim Mueller <[email protected]>
---
 hw/sd/sdhci-internal.h |  9 +++++++++
 hw/sd/sdhci.c          | 23 +++++++++++++++++++++--
 2 files changed, 30 insertions(+), 2 deletions(-)

diff --git a/hw/sd/sdhci-internal.h b/hw/sd/sdhci-internal.h
index 9f768c418e..bb586a9666 100644
--- a/hw/sd/sdhci-internal.h
+++ b/hw/sd/sdhci-internal.h
@@ -278,6 +278,15 @@ FIELD(SDHC_MAXCURR, V18_VDD2,         32, 8); /* since v4.20 */
 #define SDHC_INSERTION_DELAY            (NANOSECONDS_PER_SECOND)
 #define SDHC_TRANSFER_DELAY             100
 #define SDHC_ADMA_DESCS_PER_DELAY       5
+/*
+ * Upper bound on ADMA2 descriptors handled in a single sdhci_do_adma()
+ * call, as a safety valve against a malformed or circular descriptor
+ * list. A well-formed transfer terminates far below this via END or
+ * blkcnt == 0 (even a 4 GiB transfer built from 64 KiB TRAN descriptors
+ * is only ~64K descriptors); the bound merely guarantees the loop makes
+ * a decision instead of spinning forever.
+ */
+#define SDHC_ADMA_MAX_DESCRIPTORS       (1 << 20)
 #define SDHC_CMD_RESPONSE               (3 << 0)
 
 enum {
diff --git a/hw/sd/sdhci.c b/hw/sd/sdhci.c
index 89b595ce4a..e8e8928b4f 100644
--- a/hw/sd/sdhci.c
+++ b/hw/sd/sdhci.c
@@ -779,7 +779,6 @@ static void sdhci_do_adma(SDHCIState *s)
     const MemTxAttrs attrs = { .memory = true };
     ADMADescr dscr = {};
     MemTxResult res = MEMTX_ERROR;
-    int i;
 
     if (s->trnmod & SDHC_TRNS_BLK_CNT_EN && !s->blkcnt) {
         /* Stop Multiple Transfer */
@@ -787,7 +786,27 @@ static void sdhci_do_adma(SDHCIState *s)
         return;
     }
 
-    for (i = 0; i < SDHC_ADMA_DESCS_PER_DELAY; ++i) {
+    /*
+     * Process the descriptor chain to completion (END or blkcnt == 0),
+     * yielding to the guest only for a descriptor carrying the INT
+     * attribute (a DMA-boundary interrupt, handled at the end of the loop).
+     *
+     * Historically at most SDHC_ADMA_DESCS_PER_DELAY descriptors were
+     * handled per call before rescheduling SDHC_TRANSFER_DELAY ns later on
+     * QEMU_CLOCK_VIRTUAL. That pacing is only needed so a guest can observe
+     * the intermediate DMA-interrupt state; a bulk transfer that requests
+     * no interrupt does not need slicing, and throttling it across many
+     * virtual-clock round-trips can make it race a guest-side transfer
+     * timeout. Run such chains to completion in one call instead.
+     *
+     * SDHC_ADMA_MAX_DESCRIPTORS bounds the loop so a malformed or circular
+     * chain cannot spin here forever; on overflow, break to the reschedule
+     * path so the main loop stays responsive.
+     */
+    for (unsigned int adma_descs = 0; ; adma_descs++) {
+        if (adma_descs >= SDHC_ADMA_MAX_DESCRIPTORS) {
+            break;
+        }
         s->admaerr &= ~SDHC_ADMAERR_LENGTH_MISMATCH;
 
         get_adma_description(s, &dscr);
-- 
2.43.0