lowRISC / lowRISC/opentitan

[dma/rtl] CHUNK_DATA_SIZE needs to be a multiple of TRANSFER_WIDTH

Open
#31,191 0 comments 0 reactions 0 assignees View on GitHub
IP:dma
Dominant language
SystemVerilog
Stars
3.6k
Forks
1.1k
Avg merge
2d 22h
Merged PRs (30d)
141

Description

In DMA, we have three different sizes:
- TOTAL_DATA_SIZE: number of total bytes transferred.
- CHUNK_DATA_SIZE: a chunk is a slice of that transfer job.
- TRANSFER_WIDTH: these are the number of bytes per transaction.

This RTL correctly computes the remaining bytes even when the CHUNK_DATA_SIZE is not a multiple of the TRANSFER_WIDTH:
https://github.com/lowRISC/opentitan/blob/8fcd27c579b1793d21a1542d6370291cf17af146/hw/ip/dma/rtl/dma.sv#L1150-L1153

However, this RTL:
https://github.com/lowRISC/opentitan/blob/8fcd27c579b1793d21a1542d6370291cf17af146/hw/ip/dma/rtl/dma.sv#L957-L959
https://github.com/lowRISC/opentitan/blob/8fcd27c579b1793d21a1542d6370291cf17af146/hw/ip/dma/rtl/dma.sv#L763-L764
https://github.com/lowRISC/opentitan/blob/8fcd27c579b1793d21a1542d6370291cf17af146/hw/ip/dma/rtl/dma.sv#L776
always uses the TRANSFER_WIDTH size to advance. So a transaction where the CHUNK_DATA_SIZE is not a multiple of the TRANSFER_WIDTH, bytes can get lost.

In an example:
TOTAL_DATA_SIZE=12, CHUNK_DATA_SIZE=6, TRANSFER_WIDTH=4:
Chunk0, txn0: remaining_bytes=6, bytes written=4 (0...3), counter advances +4
Chunk0, txn1: remaining_bytes=2, bytes_written=2 (4...5), counter advances +4 <- wrong
Chunk1, txn0: remaining bytes=4, bytes_written=4 (8...11), counter advances +4 <- done
So bytes 6 and 7 are never written.

This was not detected in DV as DV constrained the chunk data and transwer width accordingly:
https://github.com/lowRISC/opentitan/blob/8fcd27c579b1793d21a1542d6370291cf17af146/hw/ip/dma/dv/env/dma_seq_item.sv#L340-L345

To fix this in RTL, the counting logic should be adapted.
To fix this in SW, make sure that CHUNK_DATA_SIZE is a multiple of TRANSFER_WIDTH.

Thanks to Vishal Bhogade (@vishalbb-git) for reporting this!

Contributor guide

Open the contributing guide

Research direction

Start in hw/ip/dma/rtl/dma.sv at lines 763-764, 776, 957-959, and 1150-1153 to trace how counters advance for partial transactions. Then inspect hw/ip/dma/dv/env/dma_seq_item.sv at lines 340-345 and run or extend DMA DV with non-multiple CHUNK_DATA_SIZE and TRANSFER_WIDTH values. Done means the bytes in the example are all transferred, with the RTL or software constraint behavior made consistent.

Written by the indexing model from the issue text.

Assessment

Domain
embedded-iot
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.