Ibex integration #979
Merged
Ibex integration #979
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
chick
reviewed
Sep 13, 2021
abejgonzalez
requested changes
Sep 13, 2021
There was a problem hiding this comment.
Couple of comments:
- Have you tried a multi-core Ibex configuration? If it doesn't work then we should prevent there being multiple cores from being instantiated.
- Have you tried a heterogeneous setup? Ibex + Rocket... etc. Similar to above, then we should prevent this using something like
requires
docs/Generators/Ibex.rst
Outdated
|
|
||
| .. Warning:: The Ibex mtvec register is 256 byte aligned. When writing/running tests, ensure that the trap vector is also 256 byte aligned. | ||
|
|
||
| .. Warning:: The Ibex reset vector is located at 0x80. |
There was a problem hiding this comment.
Isn't the more appropriate thing to say is that the reset vector is located at ADDRESS+OFFSET where OFFSET is 0x80.
There was a problem hiding this comment.
Yeah, that sounds more correct. I'll fix it.
abejgonzalez
requested changes
Sep 13, 2021
schwarz-em
force-pushed
the
ibex-integration
branch
from
079d70f
to
0c16243
Compare
|
Looks like the CI is still failing, can you double check that this config builds in Verilator properly? |
schwarz-em
force-pushed
the
ibex-integration
branch
from
4e498c8
to
91a6133
Compare
schwarz-em
force-pushed
the
ibex-integration
branch
from
3563b34
to
6ed99ff
Compare
abejgonzalez
requested changes
Nov 18, 2021
There was a problem hiding this comment.
Since there is no Ibex PR I'll put the comments here:
- Can you improve the Ibex README to be more like the https://github.com/ucb-bar/nvdla-wrapper and https://github.com/ucb-bar/cva6-wrapper
- Can you add a comment in the Makefile for why you need the
sedfix/hack.
| @@ -252,6 +252,13 @@ jobs: | |||
| group-key: "group-cores" | |||
| project-key: "chipyard-sodor" | |||
| timeout: "30m" | |||
| chipyard-ibex-run-tests: | |||
There was a problem hiding this comment.
What's going on with all the .circleci files being deleted?
.circleci/run-tests.sh
Outdated
| @@ -100,6 +100,9 @@ case $1 in | |||
| chipyard-sodor) | |||
| run_asm ${mapping[$1]} | |||
| ;; | |||
| chipyard-ibex) | |||
| run_bmark ${mapping[$1]} | |||
There was a problem hiding this comment.
Wait... why does this pass? Shouldn't the bmarks be RV64... not RV32 (and thus should break)?
.github/README.md
Outdated
| @@ -0,0 +1,137 @@ | |||
| Chipyard Continuous Integration (CI) | |||
There was a problem hiding this comment.
This file shouldn't exist (was renamed in the most recent dev work).
- As similar as possible to the circle ci code. - The `.github/README.md` file has a fair amount of documentation for this. - Creates a worfklow file - re-uses most of the circleci/scipts unchanged - defines a number of *Composite Actions* which are like YML subroutines - Removes the circle-ci code - Points the CI badge in the top level README to use the GA test result
Address PR comments
schwarz-em
force-pushed
the
ibex-integration
branch
from
6ed99ff
to
ab520e4
Compare
abejgonzalez
approved these changes
Nov 23, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Type of change: new feature
Impact: software change
Release Notes
Integrate the SystemVerilog embedded core Ibex into Chipyard. This change adds the necessary Chisel wrapper module and adds Ibex to the Chipyard configurations. Documentation and CI updates are also included.
There was a also a change to the testchipip bootrom that I had to make in order to have Ibex boot correctly within Chipyard. See the pull request here: ucb-bar/testchipip#136