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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.