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

FireSim CI #409

Merged
merged 127 commits into from
May 27, 2020
Merged

FireSim CI #409

merged 127 commits into from
May 27, 2020

Conversation

abejgonzalez
Copy link
Contributor

@abejgonzalez abejgonzalez commented Nov 5, 2019

Related issue: Fixes #364

Type of change: new feature

Impact: no rtl change

Development Phase: implementation

Release Notes
This PR adds FireSim (https://fires.im/) FPGA integration to CI to check both Buildroot and Fedora Linux boot on LargeBoom. This uses Chipyard to initialize and setup FireSim. FireSim runs are started at 0:00am on Thursdays (weekly) only on the master branch. In order to start the FPGA image build one of the BOOM developers must click the "aws-approval" button in the CircleCI UI.

This PR also changes the CI significantly by using CircleCI "commands" to create job "functions" which can be reused and pipeline parameters that can determine how to manage the workflow.

@abejgonzalez
Copy link
Contributor Author

abejgonzalez commented Nov 5, 2019

Stopping this from getting merged to figure out why copying things from the Fedora instance is not working (BOOM works with Fedora... just copying the uartlog is breaking (this happens on both BOOM and Rocket))

@abejgonzalez
Copy link
Contributor Author

abejgonzalez commented Nov 13, 2019

The copying is a FireSim bug that is being tracked here: firesim/firesim#329. Until then, this PR works and things are tested properly.

.circleci/test.sh Outdated Show resolved Hide resolved
@@ -1,2 +1 @@
0f34a22c5383284d62fdd594e882e321c4ef9cfa

6f1a53a2f3e43c8444a308e002a611fcfb995bcc
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This CI hash is on pointing to the firesim-bump branch of Chipyard, but will be updated once the release happens.


If the workloads fail to run AND the manager instance stops, you can still run tests again:
- Re-boot the instance (note: the IP address has changed)
- Add to the CircleCI UI `$AWS_IP_ADDR_OVERRIDE` with the new IP address
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the use case for this? When would someone want to rerun this way after a failure

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was used to debug why the manager was failing and instead of spawning a new manager (which takes O(30min)) you could just reuse the previous manager.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also what does this mean "Add to the CircleCI UI". Are you referring to the config file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ill clarify this a bit. I mean within the CircleCI Web UI enter "Project Settings" and under the "Environment Variables" tab add the environment variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ill update once CI is done so that the tests can run to completion.

Copy link
Contributor

Choose a reason for hiding this comment

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

Wow TIL that feature

Copy link
Contributor

Choose a reason for hiding this comment

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

Is that a project-wide global setting, or can you add it per-run?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its for the entire project.

@jerryz123
Copy link
Contributor

Can you squash some of the more trivial commits here, and also prefix them all with [ci]? Otherwise looks fine. I assume the green check means it works

@jerryz123 jerryz123 self-requested a review May 27, 2020 17:25
@abejgonzalez abejgonzalez merged commit 4508d68 into master May 27, 2020
@abejgonzalez abejgonzalez deleted the firesim-ci branch May 27, 2020 18:00
jerryz123 pushed a commit that referenced this pull request Mar 11, 2021
Don't hardcode version in docs conf.py, let readthedocs fill it in
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FireSim based CI
2 participants