Skip to content

reg_to_tlul drives a partial a_mask on a Get → mask-honoring TL-UL slaves corrupt read data #65

Description

@siju-felsite

Module: src/reg_to_tlul.sv — sibling of the A9 one-outstanding fix (PR #63), same module.
Affected: live master HEAD d6e1d4c (Release v0.4.7) — the buggy line is present at HEAD: git show d6e1d4c:src/reg_to_tlul.svassign tl_o.a_mask = reg_req_i.wstrb;.

Summary

reg_to_tlul forwards the reg-bus write strobe reg_req_i.wstrb as the TL-UL a_mask for every transaction, including Gets (reads):

assign tl_o.a_opcode = reg_req_i.write ? PutFullData : Get;
assign tl_o.a_size   = 'h2;                       // 4-byte access
assign tl_o.a_mask   = reg_req_i.wstrb;           // <-- BUG: used for Gets too

On a Get, wstrb is the reader's byte-strobe (a partial / don't-care value from the upstream bus), but a TL-UL Get must request a full mask for a_size ('h2 ⇒ 4 bytes ⇒ a_mask should be 4'hF). A mask-honoring slave AND-masks the returned data by a_mask, so the un-requested byte lanes come back zeroed → silently corrupted sub-word / SRAM-backed reads.

Upstream vs integration — upstream

The defect is in upstream reg_to_tlul.sv's own combinational mapping, not in downstream glue: the buggy assignment exists verbatim at HEAD d6e1d4c, and the companion masking it relies on (tlul_adapter_sram AND-masking read data by a_mask) is the correct, spec-compliant TL-UL behaviour. So the fix belongs here, exactly like the A9 one-outstanding fix (PR #63) in the same module.

Honest caveat: the bug is latent for CSR-only deployments. tlul_adapter_reg (CSR targets) does not mask read data by a_mask, so its reads are already correct regardless of this line — which is why it has gone unnoticed. It manifests only when a Get reaches a mask-honoring TL-UL target (e.g. tlul_adapter_sram / any SRAM-backed window). It is nonetheless a genuine TL-UL-correctness bug: a Get must present a full byte mask for the bytes it reads.

Reproduce (cycle-level)

A 64-bit read of an SRAM-window slave (an OpenTitan spi_device WrFIFO DPSRAM) after the DPSRAM is written with a known payload: the raw DPSRAM holds the data correctly, but the read returns corrupt data. Traced at the device tl_i:

a_valid=1 a_opcode=Get a_mask=0101   d_data=0x00000000   (bank1: bytes 1,3 dropped)
a_valid=1 a_opcode=Get a_mask=0101   d_data=0x00000080   (bank0: byte 0x01 dropped)

a_mask = 0101 (0x5 = bytes 0,2); the device returns the SRAM word AND-masked by a_mask, dropping bytes 1,3.

Minimal upstream repro (no SoC): put reg_to_tlul in front of a tlul_adapter_sram, preload the SRAM, issue a reg read whose wstrb is not all-ones, and observe the returned word AND-masked to the strobe.

Fix (one line, src/reg_to_tlul.sv)

// A Get must request all bytes; only a write carries a meaningful byte-strobe.
assign tl_o.a_mask = reg_req_i.write ? reg_req_i.wstrb : '1;

a_size is hard-coded 'h2, so '1 = 4'hF covers the 4-byte access. The Put (write) path is unchanged.

Verify

CSR reads stay byte-identical (tlul_adapter_reg does not mask reads by a_mask, so forcing Gets to '1 changes nothing for CSR targets). Writes unchanged (PutFullData keeps wstrb). The SRAM-window read is fixed: the lower lane returns the correct word and both 64-bit beats are correct.

A ready one-line patch (clean git apply --check at HEAD d6e1d4c) is available — happy to open a PR if preferred.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions