-
Notifications
You must be signed in to change notification settings - Fork 747
Enable RISC-V bitmanip extensions for OpenTitan #4145
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
Conversation
This looks good, but the QEMU CI tests and invocations in the board's Makefile should be adjusted to enable those extensions (I'm assuming they're supported?). |
The OpenTitan machine in QEMU uses a vendor CPU, so we can't just enable the extensions from the command line. I have sent a patch to QEMU to enable the extensions. We just have to wait until the QEMU merge window opens again. |
I think that sounds like a good plan! I do want to mention that -- should this be critical to OpenTitan or take a very long time to merge in QEMU -- we should probably not delay such changes that work on real hardware, but not in simulation. However, this seems like an easy enough patch to simply either way a bit and maintain downstream, or work around in CI. |
Considering that this was merged into OT in 2021 and we have only just been notified, I think we can wait a few more weeks :) |
FYI we have a fork of |
@jwnrt did you mean a fork of qemu? |
Sorry, yes :) |
I think that we'd generally be happy to accept an additional CI workflow that runs on that fork, if that is of interest. In particular, if upstream QEMU doesn't support features or is less faithful of emulating a given HW revision, I'd say that this would be an acceptable vehicle to run tests on, in addition to the couple we run on upstream QEMU. |
It looks like this was accepted upstream: https://gitlab.com/qemu-project/qemu/-/commit/06fb3bda6aadeb190be09a1513f1f0d31d119d16 but didn't make it into 9.1.0. Are we waiting for this to be released in 9.1.1? (presumably in a few days) |
19aaf3e
to
9fccd60
Compare
Rebased on top of #4214 to include @alistair23's patch. |
This doesn't seem to work (yet) unfortunately:
Can you check whether this works locally for you? |
It doesn't work locally (which I should have checked first). I'll try to debug. |
@jwnrt What's the status of this? |
Had a look at this today, QEMU is faulting on a |
The fix isn't in QEMU 9.1.1, which is why it isn't working. It's probably worth just updating to the current master branch |
Ibex in OpenTitan supports the bitmanip extensions in addition to IMC. The ratified set of these are supported upstream in LLVM, but not exposed in any of the default Rust targets. We can manually enable them by adding this flag.
9fccd60
to
23013e1
Compare
EDIT: it's working in CI so I must have forgotten to re-clone QEMU. Thanks @alistair23. Sorry for the great big delay on this, it's very busy here at the moment. |
@jwnrt all good, thanks for pushing this forward! |
Pull Request Overview
This pull request enables RISC-V bitmanip extension support when compiling OpenTitan.
Ibex in OpenTitan supports all the bitmanip extensions in addition to the IMC already enabled. The ratified subset of these extensions is supported upstream in LLVM but not exposed in any of the default Rust targets. We can manually enable them via this flag.
I'm not sure how important these extensions are for Tock but the
.text
size did reduce by 1024 bytes.Testing Strategy
This pull request was tested by compiling for OpenTitan. I haven't run any proper tests.
Documentation Updated
/docs
, or no updates are required.Formatting
make prepush
.