[PATCH 00/10] hw/{block,ssi}: Fix spi-nor flash dummy byte handling

Bin Meng posted 10 patches 3 weeks, 4 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260630135729.466264-1-bin.meng@processmission.com
Maintainers: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com>, Alistair Francis <alistair@alistair23.me>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, "Cédric Le Goater" <clg@kaod.org>, Peter Maydell <peter.maydell@linaro.org>, Steven Lee <steven_lee@aspeedtech.com>, Troy Lee <leetroy@gmail.com>, Jamin Lin <jamin_lin@aspeedtech.com>, Kane Chen <kane_chen@aspeedtech.com>, Andrew Jeffery <andrew@codeconstruct.com.au>, Joel Stanley <joel@jms.id.au>, Tyrone Ting <kfting@nuvoton.com>, Hao Wu <wuhaotsh@google.com>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
docs/devel/index-internals.rst |   1 +
docs/devel/ssi.rst             | 132 +++++++++++++++++++++++++++++
include/hw/ssi/aspeed_smc.h    |   2 -
include/hw/ssi/xilinx_spips.h  |   2 +-
hw/block/m25p80.c              | 146 ++++++++++++++++++++++-----------
hw/ssi/aspeed_smc.c            | 135 ++++--------------------------
hw/ssi/npcm7xx_fiu.c           |  25 ++----
hw/ssi/xilinx_spips.c          | 120 +++++++++++++++++++--------
hw/ssi/trace-events            |   1 -
9 files changed, 345 insertions(+), 219 deletions(-)
create mode 100644 docs/devel/ssi.rst
[PATCH 00/10] hw/{block,ssi}: Fix spi-nor flash dummy byte handling
Posted by Bin Meng 3 weeks, 4 days ago
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/


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 +++++++++++++++++++++++++++++
 include/hw/ssi/aspeed_smc.h    |   2 -
 include/hw/ssi/xilinx_spips.h  |   2 +-
 hw/block/m25p80.c              | 146 ++++++++++++++++++++++-----------
 hw/ssi/aspeed_smc.c            | 135 ++++--------------------------
 hw/ssi/npcm7xx_fiu.c           |  25 ++----
 hw/ssi/xilinx_spips.c          | 120 +++++++++++++++++++--------
 hw/ssi/trace-events            |   1 -
 9 files changed, 345 insertions(+), 219 deletions(-)
 create mode 100644 docs/devel/ssi.rst

-- 
2.34.1
Re: [PATCH 00/10] hw/{block,ssi}: Fix spi-nor flash dummy byte handling
Posted by Cédric Le Goater 3 weeks, 4 days ago
+ Joel

On 6/30/26 15:57, 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/
> 
> 
> 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 +++++++++++++++++++++++++++++
>   include/hw/ssi/aspeed_smc.h    |   2 -
>   include/hw/ssi/xilinx_spips.h  |   2 +-
>   hw/block/m25p80.c              | 146 ++++++++++++++++++++++-----------
>   hw/ssi/aspeed_smc.c            | 135 ++++--------------------------
>   hw/ssi/npcm7xx_fiu.c           |  25 ++----
>   hw/ssi/xilinx_spips.c          | 120 +++++++++++++++++++--------
>   hw/ssi/trace-events            |   1 -
>   9 files changed, 345 insertions(+), 219 deletions(-)
>   create mode 100644 docs/devel/ssi.rst
> 

Thanks for the cleanup, Bin.

Tested-by: Cédric Le Goater <clg@redhat.com>

C.


Re: [PATCH 00/10] hw/{block,ssi}: Fix spi-nor flash dummy byte handling
Posted by Philippe Mathieu-Daudé 2 weeks, 6 days ago
Hi Bin,

On 30/6/26 15:57, 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 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.

I'd blame the API. I'd feel safer having the block/ layer use bit
cycles, and a core layer converting bits to bytes when calling the
SSI layer. Maybe too late to improve. The documentation you added
is helpful, but I'm pretty sure we'll introduce more regressions.
Anyway, thanks for your work :)

> 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/

(Could we add this as a functional test?)
Re: [PATCH 00/10] hw/{block,ssi}: Fix spi-nor flash dummy byte handling
Posted by Cédric Le Goater 2 weeks, 6 days ago
On 7/6/26 10:19, Philippe Mathieu-Daudé wrote:
> Hi Bin,
> 
> On 30/6/26 15:57, 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 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.
> 
> I'd blame the API. I'd feel safer having the block/ layer use bit
> cycles, and a core layer converting bits to bytes when calling the
> SSI layer. Maybe too late to improve. The documentation you added
> is helpful, but I'm pretty sure we'll introduce more regressions.
> Anyway, thanks for your work :)
> 
>> 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/
> 
> (Could we add this as a functional test?)

Nearly all aspeed machines will test these changes when booting.

Thanks,

C.


Re: [PATCH 00/10] hw/{block,ssi}: Fix spi-nor flash dummy byte handling
Posted by Philippe Mathieu-Daudé 2 weeks, 6 days ago
On 6/7/26 10:40, Cédric Le Goater wrote:
> On 7/6/26 10:19, Philippe Mathieu-Daudé wrote:
>> Hi Bin,
>>
>> On 30/6/26 15:57, 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 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.
>>
>> I'd blame the API. I'd feel safer having the block/ layer use bit
>> cycles, and a core layer converting bits to bytes when calling the
>> SSI layer. Maybe too late to improve. The documentation you added
>> is helpful, but I'm pretty sure we'll introduce more regressions.
>> Anyway, thanks for your work :)
>>
>>> 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/
>>
>> (Could we add this as a functional test?)
> 
> Nearly all aspeed machines will test these changes when booting.

Are you saying that currently the Aspeed tests pass without this
series?


Re: [PATCH 00/10] hw/{block,ssi}: Fix spi-nor flash dummy byte handling
Posted by Cédric Le Goater 2 weeks, 6 days ago
On 7/6/26 11:58, Philippe Mathieu-Daudé wrote:
> On 6/7/26 10:40, Cédric Le Goater wrote:
>> On 7/6/26 10:19, Philippe Mathieu-Daudé wrote:
>>> Hi Bin,
>>>
>>> On 30/6/26 15:57, 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 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.
>>>
>>> I'd blame the API. I'd feel safer having the block/ layer use bit
>>> cycles, and a core layer converting bits to bytes when calling the
>>> SSI layer. Maybe too late to improve. The documentation you added
>>> is helpful, but I'm pretty sure we'll introduce more regressions.
>>> Anyway, thanks for your work :)
>>>
>>>> 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/
>>>
>>> (Could we add this as a functional test?)
>>
>> Nearly all aspeed machines will test these changes when booting.
> 
> Are you saying that currently the Aspeed tests pass without this
> series?
> 

They do, and they also pass with the series.

The series fixes the dummy-cycle emulation and removes the ugly
workarounds currently used by several SPI controller models,
which rely on snooping transfers. This issue has been reported
a few times in the past, with tentative fixes, but this is the
first proposal that addresses it cleanly.

Thanks,

C.

Re: [PATCH 00/10] hw/{block,ssi}: Fix spi-nor flash dummy byte handling
Posted by Cédric Le Goater 2 weeks, 6 days ago
Hello,

On 6/30/26 15:57, 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/
> 
> 
> 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 +++++++++++++++++++++++++++++
>   include/hw/ssi/aspeed_smc.h    |   2 -
>   include/hw/ssi/xilinx_spips.h  |   2 +-
>   hw/block/m25p80.c              | 146 ++++++++++++++++++++++-----------
>   hw/ssi/aspeed_smc.c            | 135 ++++--------------------------
>   hw/ssi/npcm7xx_fiu.c           |  25 ++----
>   hw/ssi/xilinx_spips.c          | 120 +++++++++++++++++++--------
>   hw/ssi/trace-events            |   1 -
>   9 files changed, 345 insertions(+), 219 deletions(-)
>   create mode 100644 docs/devel/ssi.rst
> 

Has anyone planned to merge this series ? I think it would be a good
addition for QEMU 11.1. If not, I can take it through the aspeed queue.


Thanks,

C.