lowRISC / sonata-system

A full micro-controller system utilizing the CHERIoT Ibex core, part of the Sunburst project funded by UKRI
Apache License 2.0
29 stars 17 forks source link

Create basic CHERI USB device test from existing check #193

Closed AlexJones0 closed 1 month ago

AlexJones0 commented 2 months ago

This PR incorporates USB device tests into the existing Sonata CHERI test runner, and for now just adds a simple smoke/configuration test based on the existing USB device check, which simply attempts to connect and set up a basic configuration.

This also makes a minor refactoring change to the hyperram test to avoid name clashes between tests.

This has been tested on FPGA with a USB Type-A to Type-C connector in the User USB port, and has been tested in a Verilator build using the DPI. This has also been tested running for 100 iterations on FPGA and it passes consistently.

elliotb-lowrisc commented 1 month ago

I've tested this on FPGA using a v0.4.1 bitstream and confirmed it worked with User USB connected to a USB Type-A port of a USB host using the following command:

util/test_runner.py -t 30 fpga /dev/ttyUSB2 --elf-file sw/cheri/build/tests/test_runner --tcl-file util/sonata-openocd-cfg.tcl

The test failed when run connected to a USB Type-C port with no USB Type-A / USB 2.0 conversion. I believe this is a known issues and is due to USB Type-C complexities beyond my understanding.

Nit-pick: the iterations printout is zero-based, which didn't make much sense to me at first glance.

elliotb-lowrisc commented 1 month ago

Oh, a bigger issue for me is the lack of instruction to connect the User USB port. Anyone running test_runner.py may be rather confused as to why it was failing, as I was, without that detail. Perhaps the test could at least print a message mentioning that it is testing using the User USB port, and ideally mention that it should be connected to a USB Type-A port.

AlexJones0 commented 1 month ago

Thank you for testing this!

Nit-pick: the iterations printout is zero-based, which didn't make much sense to me at first glance.

I agree - also, the iterations is actually printing hex and not decimal numbers. I've kept the printout this way to match with the rest of our existing testing. If we want to clean up the output of these tests, I think that it would be more appropriate to do it later in a refactoring/cleanup PR addressing all of the tests, rather than spending the time just updating this test and then dealing with inconsistent output between tests.

Perhaps the test could at least print a message mentioning that it is testing using the User USB port, and ideally mention that it should be connected to a USB Type-A port.

I mostly made these tests under the assumption they would just be running in CI. I'll have a think about how best to document this connection requirement - perhaps through a printed message on the test (e.g. "requires USB Type-A connection") with an optional compile flag to disable the test (albeit there are no other USB tests at the moment).

AlexJones0 commented 1 month ago

@elliotb-lowrisc I've now added the following short message that prints alongside the USB Device test when you run it:

  running usbdev_test: 00\00
+ (needs User USB connected to a Type-A USB host port)
    Running USBDEV configure test... PASS!

Do you think that this is sufficient to address your concerns?

elliotb-lowrisc commented 1 month ago

Do you think that this is sufficient to address your concerns?

Sure

alees24 commented 1 month ago

The USB Type-A requirement comes from the need to negotiate power delivery and identify the Sonata FPGA as a USB device not a host controller. USB-C connectors are used for both USB host controllers and USB devices, and the USRUSB port on the Sonata board can operate in either role - determined by the logic in the FPGA - but is neither by default and so we do not request power from the other end of the cable. The adapter that connects us to a Type-A port does the necessary negotiation because only USB devices may be plugged into a Type-A port.