Skip to content

hw/ssi/esp32_spi: fix RX bounds check and add dummy cycle handling (QEMU-282)#144

Open
Ahmad-myenergi wants to merge 2 commits into
espressif:esp-developfrom
Ahmad-myenergi:fix-esp32-spi-nvs
Open

hw/ssi/esp32_spi: fix RX bounds check and add dummy cycle handling (QEMU-282)#144
Ahmad-myenergi wants to merge 2 commits into
espressif:esp-developfrom
Ahmad-myenergi:fix-esp32-spi-nvs

Conversation

@Ahmad-myenergi

@Ahmad-myenergi Ahmad-myenergi commented Feb 28, 2026

Copy link
Copy Markdown

Description

hw/ssi/esp32_spi, esp32c3_spi, esp32s3_spi: fix txrx buffer bounds check and add flash read dummy cycles

Problem 1 — Incorrect bounds check in txrx_buffer (all three variants)

All three ESP32 SPI controller models (esp32_spi.c, esp32c3_spi.c, esp32s3_spi.c) contain the same bug in their txrx_buffer function:

// Before (buggy):
if (byte < tx_bytes) { ... }  // 'byte' is the data value, not the loop index
if (byte < rx_bytes) { ... }

The local variable byte holds the data being transferred, not the loop position. Using it as a bounds guard makes TX/RX behaviour depend on data values rather than byte positions. In the worst case, a received byte with a value ≥ rx_bytes is silently dropped. This is a latent correctness bug in all three variants.

Fix: Replace byte with the loop index i in both conditions.


Problem 2 — Missing dummy cycles for flash fast-read commands (esp32_spi.c only)

SPI flash fast-read commands (DOR 0x3B, Fast Read 0x0B, QOR 0x6B, DIOR 0xBB, QIOR 0xEB and their 4-byte address variants) require one dummy byte to be clocked between the address phase and the data phase. The flash chip uses those extra clock cycles to prepare its output buffer. Without them, the m25p80 flash model consumes the first real data byte as the dummy, shifting all read data by one byte.

The esp32c3_spi.c and esp32s3_spi.c variants already handle dummy cycles correctly via esp32c3_spi_dummy_cycles() / esp32s3_spi_dummy_cycles(). The original esp32_spi.c had no dummy phase at all.

This caused NVS operations under ESP-IDF v5.5.3 to fail: every page read returned corrupted data, resulting in ESP_ERR_NVS_NOT_FOUND for all stored keys.

Fix: Add esp32_spi_dummy_cycles() helper (matching the pattern in the C3/S3 variants) and call it from esp32_spi_transaction(). In the USR command path, set dummy_bytes = 1 when SPI_USER.DUMMY is enabled and the opcode is a known flash fast-read command. PSRAM commands (which also use SPI_USER.DUMMY) are excluded as they handle dummy cycles internally in their peripheral models.

Related

Testing


Checklist

Before submitting a Pull Request, please ensure the following:

  • 🚨 This PR does not introduce breaking changes.
  • All CI checks (GH Actions) pass.
  • Documentation is updated as needed.
  • Tests are updated or added as necessary.
  • Code is well-commented, especially in complex areas.
  • Git history is clean — commits are squashed to the minimum necessary.

The SPI TX/RX loop incorrectly used the transmitted byte value
to guard buffer access instead of the loop index. This caused
received bytes to be dropped when the payload value exceeded
rx_bytes, breaking flash read operations.

This resulted in NVS reads returning ESP_ERR_NVS_NOT_FOUND
under ESP-IDF v5.5.3 when running on QEMU.

Additionally, inject dummy cycles between address and data
phases when SPI_USER.DUMMY is enabled to match flash
fast-read behaviour expected by ESP-IDF.

Reproducer:
- ESP-IDF v5.5.3
- NVS set/commit/get sequence under QEMU
- Fails before patch, succeeds after patch

Signed-off-by: Ahmad Alhmmori <ahmad.alhmmori@myenergi.com>
Signed-off-by: Ahmad Alhmmori <ahmad.alhmmori@myenergi.com>
@github-actions github-actions Bot changed the title hw/ssi/esp32_spi: fix RX bounds check and add dummy cycle handling hw/ssi/esp32_spi: fix RX bounds check and add dummy cycle handling (QEMU-282) Feb 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants