[usbdev] OUT side STALL response can leave device unable to capture next SETUP/OUT packet

[alees24]

Description

An OUT packet that elicits a 'STALL' response is neither acknowledged nor rolled back; in usb_fs_nb_out_pe neither 'out_ep_acked_o' nor 'rollback_data' (= out_ep_rollback_o) is asserted.

This means that if the packet is 62 or more bytes in length, the byte counting logic in usbdev_usbif is not reset in preparation for the next OUT/SETUP packet to be received and the packet size recorded into the RX FIFO is incorrect; the true size of the packet is unknowable.

The STALLing mechanism is used within current test software to declare 'protocol stall' in response to the DEVICE_QUALIFIER requests, but this does not break anything because only the IN side receives a STALL response for that Control Transfer, I think.

To reproduce, fetch draft PR sequence #23805 and run with the following seed:

util/dvsim/dvsim.py hw/ip/usbdev/dv/usbdev_sim_cfg.hjson -i usbdev_setup_priority_over_stall_response --fi 71923730856705723741826963384972440023047469689158980899448095559948831305837 --wa shm --tool xcelium -v m

If that somehow passes, try specifying '.randomize_length(0), .num_of_bytes(64)' to the task enable 'send_prnd_out_packet(..)'

This issues needs further consideration and clarification of what else may be impacted.
To be clear, this is with ES and prod RTL; it does not relate to the draft proposed changes.

@a-will, @vogelpi, @andreaskurth

[a-will]

With VCS, the seed 6059828404840696268329756370818438228836193083784097444560659638759758565984 revealed the same problem.

[a-will]

Without a fix, it seems there would be problems for any subsequent OUT / SETUP transaction. There is just the one counter for accumulated data, and a count that is carried over can lead to data loss (by way of the gated std_write_d/q in usbdev_usbif) and incorrect packet sizes, even for an unrelated endpoint.

Note that for usbdev, there are use cases for functional stall as well, and the effect on unrelated endpoints is especially problematic.

[alees24]

As per the linked PR, if a link reset occurs during transmission of the final part of an OUT packet of >= 62 bytes the same failure can be triggered and the first packet after the Bus Reset signaling will not be received correctly. This would likely result in a failure to reconfigure the device.

It is presently not possible to inject a Bus Reset at just the 'right' moment to trigger this fault in DV because the Bus Reset signaling and the transmission of OUT packets are both performed by the usb20_driver module. However, with just the change in PR #23812 (and not that in #23807) cherry-picked onto the modified draft PR #23805 DV sequence the test now passes[1] since the 'link_reset' signal resets the packet reception state as intended.

1. xcelium seed -1396724950 causes a Bus Reset stimulus to be generated in the modified sequence.

[andreaskurth]

Now that #23812 is merged, can we close this again?

[alees24]

The merged RTL changes are believed to be comprehensive in avoiding this fault.