lowRISC / lowRISC/opentitan

[usbdev] Increase robustness of packet reception when SYNC signal is corrupted.

Open
#23,770 7 comments 0 reactions 1 assignee View on GitHub

@alees24 is already working on this.

Since Jun 21, 2024.

Component:RTL IP:usbdev Type:FutureRelease
Dominant language
SystemVerilog
Stars
3.6k
Forks
1.1k
Avg merge
2d 22h
Merged PRs (30d)
141

Description

Description (copied over from issue #23719)

The packet reception within USBDEV does not cope well with a corrupted/invalid SYNC signal at the start of the packet, leading to a failure of the OUT side state machine. Invalid/corrupted SYNC signals do not occur in normal operation but there is a risk of them being received through interference, signal integrity issues or substantial host-device frequency mismatch leading to sampling issues.

The observed behavior in block level DV is that the 'pkt_start_o' signal is asserted by 'usb_fs_rx' leading to the OUT side state machine (usb_fs_nb_out_pe) advancing into StRecvdDataStart in anticipation of receiving a complete, valid data packet. The signal 'valid_packet_o' from the _rx module is not asserted for that packet and with the present logic this means that 'pkt_end_o' is also not asserted. The state machine relies critically upon 'pkt_end_o' in order to leave the StRecvdDataStart at the end of the packet. It thus wait until the end of a subsequent packet, if any, at which point communications failure is likely to occur through - for example - a Control Transfer having been retired unsuccessfully.

An additional problem is that the packet reception logic will start up again if the KJKJKK sequence is spotted within the packet - where it may quite legitimately occur - following detection of a bit stuffing violation within a packet.

PR #23717 proposes a change to the RTL to improve resilience/recovery and permits the block level DV tests (usbdev_invalid_sync and usbdev_stress_usb_traffic) to run to completion.

Description (copied over from PR #23717)

Commit 1:

Modify end of packet detection so that it does not depend upon the validity of the SYNC signal. End of packet must always be signaled to the packet state machines even if the packet has been declared invalid, because otherwise the OUT side state machine will wait indefinitely for the end of a DATA packet.

This does not occur in normal use, but may occur in the presence of interference, signal integrity issues and/or improper sampling.

Background: fault injection testing in block level DV transmitted a valid OUT packet which was then followed by a DATA packet with a corrupted SYNC signal. The result is that 'packet_valid_q' does not become set for the DATA packet, although 'pkt_start_o' is asserted, and there is thus no 'pkt_end_o' assertion for the DATA packet. The OUT side state machine remains in the StRcvdDataStart state, from which there is no exit until the end of a subsequent packet.

The RTL change proposed in this draft PR permits the block level test to run to completion. (The test in question is generating invalid traffic whilst a streaming + checking process is running and keeping the DUT busy; the invalid traffic is expected to produce no response from the DUT but because of the above issue caused interference with the streaming operation.)

Update: There is a further deficiency in the handling of 'packet_valid_q' because of the way that bit stuffing errors and preamble cause the signal to be deasserted. After a bit stuffing error the logic can wake up again mid-packet in response to a bit sequence that looks like a SYNC signal. There is nothing to stop a KJKJKK sequence occurring within the ensuing bits. This PR now remembers the fact that the packet is being ignored and prevents 'packet_valid_d|q' becoming reasserted prematurely.

DV sequence usbdev_bitstuff_err has been modified in PR #24234 to sidestep the problems with detection of bit stuffing violations, pending inclusion of RTL refinements in a later revision.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.