On 7/7/26 10:34, Bin Meng wrote:
>
> This series fixes dummy-cycle accounting for SPI NOR fast-read
> operations across the m25p80 flash model and several SSI controller
> models.
>
> The root problem is that the affected code paths mixed two different
> units. Flash datasheets and controller registers often describe
> fast-read dummy phases in clock cycles, while the SSI bus interface used
> by these models transfers byte-sized words to the flash. As a result,
> some paths treated cycle counts as byte counts directly, and other paths
> compensated in controller-specific ways that duplicated flash command
> semantics.
>
> The first four patches fix the flash side. They make m25p80 keep
> `needed_bytes` as a byte count and convert manufacturer-specific
> dummy-cycle requirements to dummy byte transfers for Winbond,
> Numonyx/Micron, Macronix, and Spansion flashes. The Numonyx/Micron
> change also handles the standard-mode case where the dummy phase follows
> the address bus width for output fast-read commands.
>
> The next patches fix controller-generated dummy phases. NPCM7xx FIU
> and Xilinx ZynqMP GQSPI now convert programmed dummy-cycle fields to
> SSI byte transfers using the bus width selected for the dummy phase.
> The ASPEED SMC direct-read path is updated the same way. The two ASPEED
> reverts then remove older workarounds that either hard-coded FAST_READ_4
> dummy behavior or snooped manual user-mode SPI streams to inject extra
> dummy cycles. With m25p80 accounting fixed, user-mode byte streams
> should be forwarded as supplied by the guest.
>
> The final patch documents the intended ownership boundary: flash models
> own command semantics and dummy-byte consumption, while controller
> models own hardware-generated dummy phases and cycle-to-byte conversion.
>
> Some background:
>
> While recently working on some device modeling, I hit this issue
> again. I then realized that I had sent a fix series for it more than
> five years ago, but it never made it upstream.
>
> This series picks up that old work, refreshes it against the current
> tree, and fixes several issues in the SPI controller models. The original
> series is available at [1]. I have tried to verify all the SPI controller
> changes by booting real-world guest software to ensure the change does
> not break anything.
>
> [1] https://lore.kernel.org/qemu-devel/20210114150902.11515-1-bmeng.cn@gmail.com/
>
> Changes in v2:
> - correct the typo in the commit message: instrunction => instruction
> - correct the typo in the commit message: genenic => generic
> - change numonyx_extract_cfg_num_dummies() to numonyx_extract_cfg_dummy_bytes()
> to avoid confusion
> - change macronix_extract_cfg_num_dummies() to macronix_extract_cfg_dummy_bytes()
> to avoid confusion
> - change spansion_extract_cfg_num_dummies() to spansion_extract_cfg_dummy_bytes()
> to avoid confusion
> - use assert() when the dummy bit count is not byte-aligned
>
> Bin Meng (10):
> hw/block: m25p80: Fix dummy byte handling for Winbond flash
> hw/block: m25p80: Fix dummy byte handling for Numonyx/Micron flash
> hw/block: m25p80: Fix dummy byte handling for Macronix flash
> hw/block: m25p80: Fix dummy byte handling for Spansion flash
> hw/ssi: npcm7xx_fiu: Correct the dummy cycle emulation logic
> hw/ssi: xilinx_spips: Fix dummy phase handling
> hw/ssi: aspeed_smc: Fix direct-read dummy bytes
> Revert "aspeed/smc: Fix number of dummy cycles for FAST_READ_4
> command"
> Revert "aspeed/smc: snoop SPI transfers to fake dummy cycles"
> docs/devel: Document SSI dummy-cycle ownership
>
> docs/devel/index-internals.rst | 1 +
> docs/devel/ssi.rst | 132 ++++++++++++++++++++++++++++
> hw/block/m25p80.c | 154 +++++++++++++++++++++------------
> hw/ssi/aspeed_smc.c | 135 ++++-------------------------
> hw/ssi/npcm7xx_fiu.c | 25 ++----
> hw/ssi/trace-events | 1 -
> hw/ssi/xilinx_spips.c | 122 +++++++++++++++++++-------
> include/hw/ssi/aspeed_smc.h | 2 -
> include/hw/ssi/xilinx_spips.h | 2 +-
> 9 files changed, 348 insertions(+), 226 deletions(-)
> create mode 100644 docs/devel/ssi.rst
>
> ---
> base-commit: 94826ec1370328375c3b6d1e80fdc94c8f46c348
> branch: m25p80-v2
With some help of claude, here is a summary of the functional test coverage :
┌───────┬───────────────────┬──────────────────────────────────────────────────────────────────┐
│ Patch │ Component │ Functional Test Coverage │
├───────┼───────────────────┼──────────────────────────────────────────────────────────────────┤
│ 0001 │ m25p80 (Winbond) │ COVERED — ast2600-evb (6), bletchley-bmc, catalina-bmc, │
│ │ │ ast2700a1-evb (3), ast2700a2-evb (3), ast2700fc (2), anacapa-bmc │
├───────┼───────────────────┼──────────────────────────────────────────────────────────────────┤
│ 0002 │ m25p80 (Numonyx) │ PARTIAL — palmetto-bmc, romulus-bmc │
│ │ │ (default config only, no QIO/DIO mode test) │
├───────┼───────────────────┼──────────────────────────────────────────────────────────────────┤
│ 0003 │ m25p80 (Macronix) │ PARTIAL — ast2500-evb (3), witherspoon-bmc, anacapa-bmc, │
│ │ │ fby4-bmc, gb200nvl-bmc, quanta-gsj (default DC only) │
├───────┼───────────────────┼──────────────────────────────────────────────────────────────────┤
│ 0004 │ m25p80 (Spansion) │ NOT COVERED — no machine uses Spansion flash │
├───────┼───────────────────┼──────────────────────────────────────────────────────────────────┤
│ 0005 │ npcm7xx_fiu │ PARTIAL — quanta-gsj (direct-read path only) │
├───────┼───────────────────┼──────────────────────────────────────────────────────────────────┤
│ 0006 │ xilinx_spips │ NOT COVERED — no ZynqMP GQSPI machine in the suite │
├───────┼───────────────────┼──────────────────────────────────────────────────────────────────┤
│ 0007 │ aspeed_smc │ COVERED — palmetto-bmc, ast2500-evb (3), romulus-bmc, │
│ │ direct-read │ witherspoon-bmc, ast2600-evb (6), anacapa-bmc, bletchley-bmc, │
│ │ │ catalina-bmc, gb200nvl-bmc, fby4-bmc, ast2700a1-evb (3), │
│ │ │ ast2700a2-evb (3), ast2700fc (2) │
├───────┼───────────────────┼──────────────────────────────────────────────────────────────────┤
│ 0008 │ aspeed_smc │ COVERED — superseded by 0009 │
│ │ snoop table │ │
├───────┼───────────────────┼──────────────────────────────────────────────────────────────────┤
│ 0009 │ aspeed_smc │ COVERED — all Aspeed Linux boot tests: ast2500-evb (3), │
│ │ snoop removal │ romulus-bmc, witherspoon-bmc, ast2600-evb (6), anacapa-bmc, │
│ │ │ bletchley-bmc, catalina-bmc, gb200nvl-bmc, fby4-bmc, │
│ │ │ ast2700a1-evb (3), ast2700a2-evb (3), ast2700fc (2) │
├───────┼───────────────────┼──────────────────────────────────────────────────────────────────┤
│ 0010 │ docs only │ N/A │
└───────┴───────────────────┴──────────────────────────────────────────────────────────────────┘
Coverage Gaps
1. Spansion flash — No functional test at all. Need a machine with
s25fl*/s25fs* flash or a dedicated unit test.
2. Xilinx ZynqMP GQSPI — No functional test. The xlnx-zcu102 or
xlnx-versal-virt machines would exercise this, but no test with
SPI flash operations exists in the suite.
3. Non-default volatile configuration — All tests boot with default
flash config register values. The complex dummy-cycle scaling
tables (Macronix DC[1:0], Numonyx volatile_cfg, Spansion CR2V)
are only tested at their default settings.
4. QIO/DIO command families — The U-Boot and Linux drivers used in
tests typically issue standard or dual/quad-output fast
reads. The quad-I/O and dual-I/O read commands (EBh, BBh) that
have the most different dummy handling may not be exercised.
5. NPCM UMA path — Only the direct-read FIU path is confirmed
exercised; the UMA transaction path in npcm7xx_fiu_uma_transaction()
may or may not be hit during kernel boot.
To be cross checked.
Thanks,
C.