Skip to content

Conversation

@JingJerYen
Copy link
Contributor

Commit 4a6c2aa introduced an issue in AXI4 write strobe handling.
This patch:

  • Restores the original, correct strobe implementation
  • Disables byte_enable_len when unnecessary, improving efficiency

Root Cause

The transfer size was incorrectly derived from simply counting the number of 0xFF strobes.
This works only when the valid bytes are aligned at the lower address boundary, but fails otherwise.

For example:

✅ Correct case (transfer size = 2 bytes, last two bytes masked off):

byte 0 : 0xFF
byte 1 : 0xFF
byte 2 : 0x00
byte 3 : 0x00

❌ Incorrect case (transfer size incorrectly computed as 2 bytes, but we actually need 4 bytes to cover byte 2 and byte 3):

byte 0 : 0x00
byte 1 : 0x00
byte 2 : 0xFF
byte 3 : 0xFF

The correct behavior should instead use the byte enable mask (as in the pre-4a6c2aa implementation), ensuring the full valid byte range is included.

My Bug Example

When a verilated CPU writes 128 bytes starting from address 0x60000004, the SCC AXI4 target produces the following 3 write transactions. Note that the first transaction is incorrect:

[debug] transaction: write, addr=0x60000000, len=60, byte_enable_len=60
[debug] transaction: write, addr=0x60000040, len=64, byte_enable_len=64
[debug] transaction: write, addr=0x60000080, len=4,  byte_enable_len=4

With the original implementation (before commit 4a6c2aa), the result is correct:

[debug] transaction: write, addr=0x60000000, len=64, byte_enable_len=64
[debug] transaction: write, addr=0x60000040, len=64, byte_enable_len=64
[debug] transaction: write, addr=0x60000080, len=32, byte_enable_len=32

@eyck
Copy link
Contributor

eyck commented Sep 29, 2025

Many THX for the PR! I will have a look into this.
One question regarding your 128 bytes example: what address is sent by the cpu? 0x60000000 or 0x60000004?

@JingJerYen
Copy link
Contributor Author

JingJerYen commented Sep 29, 2025

螢幕擷取畫面 2025-09-29 145327 My cpu sends 0x60000000 with strobe signals to mask off the first 4 bytes

@eyck eyck self-assigned this Sep 30, 2025
@eyck eyck changed the base branch from main to develop September 30, 2025 07:59
@eyck eyck merged commit 95f6f72 into Minres:develop Sep 30, 2025
@eyck
Copy link
Contributor

eyck commented Sep 30, 2025

Actually the PR works well for aligned AXI accesses. I had to extend it to also cover unaligned accesses.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants