Re: [PATCH] hw/sd/sdhci: complete non-interrupt ADMA descriptor chains in one pass
Wadim Mueller <[email protected]>
| Newsgroups | gmane.comp.emulators.qemu.block,gmane.comp.emulators.qemu |
|---|---|
| Message-ID | <[email protected]> |
Hi Bin,
Thanks for the 513930 pointer -- that was exactly the missing piece, and
it changes my earlier result, so let me correct the record first.
With "hw/sd: sdhci: Fix SDMA boundary bug" (513930) applied first, series
515264 builds on my tree (I applied 513930 patch 1 + 515264 patches
07-12; I dropped 13/26 "keep high-capacity memory blocks at 512" -- it
didn't apply on my older hw/sd/sd.c and is orthogonal to the ADMA path).
Re-running the same AM64x reproducer, 15 hands-off boots each:
baseline (before either change): 7/15 time out
your series (513930 + 515264 07-12): 0/15
my patch (complete non-INT chains in a pass): 0/15
So your series *does* fix the AM64x case -- my earlier "10/15" number was
from my crude hand-approximation of patch 12 alone on the old base, which
(as I feared) simply wasn't representative. Apologies for the noise; the
real series resolves it here, same as my patch.
As promised, the reproducer -- I kept it free of any proprietary bits:
- The machine is public in my QEMU fork:
https://github.com/wafgo/qemu branch cmblu/corenode
(an "am64-virt" TI AM64x board). Build qemu-system-aarch64 from it;
to observe the original failure, revert my fix commit cad8b499
("fix(sdhci): complete non-int adma chains in one pass").
- A small image-free qtest on that branch,
tests/qtest/am64-adma-pacing-test.c, drives an ADMA2 read on am64-virt
and counts how many SDHC_TRANSFER_DELAY virtual-clock steps the chain
takes to complete: 0 with either fix in place, >0 (the sliced,
timer-paced transfer) without. It reproduces the *mechanism*
deterministically, no guest image needed.
Since your series fixes this, my standalone patch is effectively
superseded -- please feel free to drop it in favour of 515264, I don't
think a separate fix is needed.
The one thing possibly worth keeping is the pacing regression test. As
written it targets my fork's am64-virt, so it isn't directly
mainline-able; if a test that catches this ADMA virtual-clock pacing
would be useful upstream, I'd be glad to port it to an in-tree SDHCI
machine -- e.g. the Icicle Kit where you hit the same "Timeout for
status update", or the xilinx-zynq SDHCI you just added a test for in
513930. Would that be worthwhile, and which board would you prefer?
Thanks again for the help,
Wadim