Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sonata CI #364

Merged
merged 8 commits into from
Dec 12, 2024
Merged

Sonata CI #364

merged 8 commits into from
Dec 12, 2024

Conversation

marnovandermaas
Copy link
Contributor

@marnovandermaas marnovandermaas commented Dec 3, 2024

This PR makes a few fixes to the Sonata board file and adds a script to run programs on the Sonata simulator.

Finally this PR is adding it to the CI, where all examples are run on the Sonata simulator and the UART output is checked against the model output.

This PR builds on top of adding the Sonata simulator and simulation boot stub to the development container: CHERIoT-Platform/devcontainer#19 and CHERIoT-Platform/devcontainer#21

@marnovandermaas marnovandermaas force-pushed the sonata_ci branch 10 times, most recently from 608f9ac to 12248ca Compare December 3, 2024 16:23
@nwf
Copy link
Member

nwf commented Dec 3, 2024

Does the Sonata TB...

  • understand enough of HTIF to gracefully exit,
  • speak the Ibex "0x80 on the uart to exit" protocol,
  • expect someone to drive the lowRISC simulation_ctrl verilog, or
  • something else?

@marnovandermaas
Copy link
Contributor Author

Unfortunately none of those, our current CI monitors the uart log file and based on the output there closes the simulation. I probably should change the script to do that instead of having a fixed timeout.

@davidchisnall
Copy link
Collaborator

It would be nice to reuse one of the many existing mechanisms, rather than invent a new one.

@marnovandermaas
Copy link
Contributor Author

It would be nice to reuse one of the many existing mechanisms, rather than invent a new one.

The only existing solution I know of is our Sonata test runner script: https://github.com/lowRISC/sonata-system/blob/c4b717946aa64a06593feb95487fa498d457f920/util/test_runner.py#L176
Would you prefer me to use that script as opposed to the bash version I made? It essentially does the same thing, monitoring the UART script for a specific value.

@marnovandermaas marnovandermaas force-pushed the sonata_ci branch 5 times, most recently from d60b096 to b15ef2b Compare December 5, 2024 16:02
@marnovandermaas marnovandermaas marked this pull request as ready for review December 5, 2024 16:07
@marnovandermaas
Copy link
Contributor Author

Ok, I think I'm relatively happy with the state of this PR now. It runs all the examples on the Sonata simulator. It monitors the UART log and kills the simulator if it sees the model output match. There's also a timeout built in so the simulation doesn't take forever. The main question is whether you are happy with this mechanism I built here or whether you feel strongly enough to block this in favour of making changes to the simulator to add simulation control.

@davidchisnall
Copy link
Collaborator

I don't like cluttering the tree with model output in directories that users will explore. If we're going to do that, we need to make sure that all of the READMEs tell people not to look at those files.

Is there really no way of exiting the Sonata simulator from in the simulation? That seems like a core feature for a simulator or emulator. Is it very hard to add one? The Ibex-SAFE one is hacked in a little bit in the verilator wrapper for the UART: it just reads the top bit and exits if you write something to the UART with the top bit set. Not very clean, but it does work (until you forget about it and write binary data over the UART).

I also don't like the custom logic for running Sonata. We probably should have a sonata-sim board description that uses a different run script, so that we can just xmake run.

@marnovandermaas
Copy link
Contributor Author

marnovandermaas commented Dec 6, 2024

Thanks David, I can move the model output to this folder scripts/model_output/examples/. Would that be better or is there a better place?

For exiting the simulator, I don't think it's particularly hard but it would mean blocking this PR until I make that change in Sonata system.

I was trying to get xmake run to work. Is there a way to make the sonata simulator board description be the same as the main Sonata one but then with the following fields changed?
https://github.com/CHERIoT-Platform/cheriot-rtos/blob/main/sdk/boards/sonata-prerelease.json#L73-L74
It would be annoying having to update the simulator and the FPGA configuration separately.

@davidchisnall
Copy link
Collaborator

Thanks David, I can move the model output to this folder scripts/model_output/examples/. Would that be better or is there a better place?

I think that's fine. I want to restructure the scripts directory at some point and split the scripts that are part of the SDK from the ancillary ones, so anything in there is up for grabs.

For exiting the simulator, I don't think it's particularly hard but it would mean blocking this PR until I make that change in Sonata system.

Happy to merge this as a temporary fix, if that can come later.

I was trying to get xmake run to work. Is there a way to make the sonata simulator board description be the same as the main Sonata one but then with the following fields changed?

Not easily other than copy and paste currently. My plan at some point is to add something like json patch support in the build system. I'm pondering a simpler solution of look for a .jq file if there isn't a .json file and have that run and consume the output. That would let us have a file that is a small jq script that took a json file in the same directory and made some small changes. Unfortunately, we use some small JSON5 extensions (hex literals) and jq doesn't currently support them.

In particular, there's a bunch of stuff that's Ibex-specific that we should be able to share between a few places. Renode currently uses the sail JSON, but it should be possible to make it a 'sail + these bits' one.

sdk/boards/sonata-prerelease.json Show resolved Hide resolved
scripts/model_output/examples/04.temporal_safety.txt Outdated Show resolved Hide resolved
scripts/run-sonata-sim.sh Outdated Show resolved Hide resolved
scripts/run-sonata-sim.sh Outdated Show resolved Hide resolved
scripts/run-sonata-sim.sh Show resolved Hide resolved
@marnovandermaas marnovandermaas force-pushed the sonata_ci branch 3 times, most recently from 0836079 to bf6907c Compare December 10, 2024 11:44
@marnovandermaas
Copy link
Contributor Author

Ok, I'm now happy with the state of this PR again. I've done the following things:

  • Copied the Sonata prerelease board file into the simulator one and changed the simulator variables so that xmake run can use it.
  • I added an argument to xmake config to enable model output checking with "testing-model-output".
  • Those two things made it so I could remove the extra logic in the CI script and run Sonata like all the other boards.
  • I moved the model output into the script directory and made them board dependent.

Some open things, which I don't think need to be dealt with before the merging:

  • Using "expect" (suggestion from Wes) instead of the custom logic I built in simulator script.
  • Sonata simulator exiting cleanly, I've opened an issue upstream for this: Simulation exit procedure lowRISC/sonata-system#365 Once that has been implemented we can update the script here.
  • Reducing the duplication of the board files once the .jq method David mentioned is in.

@marnovandermaas marnovandermaas marked this pull request as ready for review December 10, 2024 11:56
These fixes were made in the Sonata prerelease board file.
This is useful in CI to see if examples are running correctly. The
output is added to the scripts directory so that it is not confusing to
users that are not necessarily familiar with CI. The names of the
examples do not include the number because at the moment I've not found
a clean way to get the number in xmake because the firmware image has
the number removed. Also, output may vary per board so the model output
is first divided up per board.
It uses the SONATA_SIMULATOR, SONATA_SIMULATOR_BOOT_STUB and
SONATA_SIMULATOR_UART_LOG environment variables. It has two modes, one
where the simulator runs in the foreground indefinitely and needs the user
to manually stop the script. The second mode runs the simulator in the
background and the UART log is monitored for a value given as a
parameter to the script. When running in the monitoring mode, there is a
60 second timeout just so the script does not run forever.

Example command, which returns 0 on success and 1 on failure:
scripts/run-sonata-sim.sh examples/01.hello_world/build/cheriot/cheriot/release/hello_world scripts/model_output/sonata-simulator/examples/01.hello_world.txt
This is to stay consistent with the rest of the examples
It add Sonata pre-release to the matrix of boards. Xmake run tries to
use the Sonata FPGA which does not exist in this CI.
The test suite doesn't run on the simulator at the moment because
HyperRAM is not supported in the RTOS right now. The
Sonata smoke test runs each example and checks the content of the UART
log against the model output.

Debug executables are currently not running on Sonata because the UART
log gets too full and causes the UART log to become unavailable inside
the GitHub runners.
This is a copy of the Sonata prerelease board file with the simulator
and simulation fields changed.
Instead of having a separate section to run examples in CI, this is now
done in xmake instead using the newly created sonata-simulator board
file.
Copy link
Member

@nwf nwf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@davidchisnall davidchisnall merged commit 6ee20e4 into CHERIoT-Platform:main Dec 12, 2024
7 checks passed
@davidchisnall
Copy link
Collaborator

Thanks!

@marnovandermaas
Copy link
Contributor Author

Thanks for all your reviews, it's much better now than my first attempt :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants