[RFC PATCH v2 03/14] hw/sd/sdhci: complete non-interrupt ADMA descriptor chains in one pass
Wadim Mueller <[email protected]>
| Newsgroups | org.nongnu.qemu-arm,org.nongnu.qemu-devel |
|---|---|
| Message-ID | <[email protected]> |
sdhci_do_adma() returns to the main loop after every descriptor and relies on a timer re-entry to pick up the next one. For a descriptor chain whose entries do not request an interrupt this makes the transfer rate depend from the virtual clock rather than on the guest's programming, which significantly slows down large transfers and makes guest-visible timing depend on host timer behaviour. Keep processing the chain in the same invocation while no descriptor asks for an interrupt and the transfer has not finished, and only fall back to the deferred path when the guest actually requested a notification. A qtest reproducer is added later in this series. Signed-off-by: Wadim Mueller <[email protected]> --- This patch can be dropped once Bin Meng's SDHCI series https://patchwork.ozlabs.org/project/qemu-devel/list/?series=515264 (which needs series=513930 applied first) is merged - it covers the same AM64x failure, see https://lore.kernel.org/qemu-devel/[email protected]/ It is included here only so that the series works on actual master. 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 4aeed120bf..e6ce12617e 100644 --- a/hw/sd/sdhci-internal.h +++ b/hw/sd/sdhci-internal.h @@ -277,6 +277,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 e58a610397..9a5dd1d93f 100644 --- a/hw/sd/sdhci.c +++ b/hw/sd/sdhci.c @@ -780,7 +780,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 */ @@ -788,7 +787,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