Add Spike-specific extension option in cva6.py for issue #3249#3280
Add Spike-specific extension option in cva6.py for issue #3249#3280sjo99-kr wants to merge 5 commits into
Conversation
cainria
left a comment
There was a problem hiding this comment.
Hi, thanks for your contribution!
Here are a few formatting suggestions (spacing and diff reduction).
Maybe @valentinThomazic or @zchamski could review the changes in more depth?
Co-authored-by: Côme <come.allart@inria.fr>
Co-authored-by: Côme <come.allart@inria.fr>
Co-authored-by: Côme <come.allart@inria.fr>
Co-authored-by: Côme <come.allart@inria.fr>
|
Hi @sjo99-kr thank you for your contribution! Could you explain a bit more on this part? |
|
Thank you for the detailed explanation 👍 !! As I mentioned above, I am still not very familiar with the CVA6 verification framework, so my testing was based on my custom This is the baseline Makefile (in ~/verif/sim) code for Spike simulation with The However, it is different when you use -- In cva6.py based executionI think the important difference appears when For example, when I run The actual execute command is like this. As you can see, the -- Makefile-based executionHowever, as you mentioned, when I run Spike directly through the Makefile like this: It works as same as your example! -- ConclusionFrom the examples above, I think the key point is that So I believe there are two possible ways to solve this problem:
This is the codes of line 849~850 in Sorry for my imperfect English, and thank you for your patience. Thanks :) |
|
Hi @sjo99-kr, I now understand what you want to achieve. The You should be able to pass I think it would be better to fix the current flow instead of adding another redundant functionality to |
|
Sorry to bother you, @valentinThomazic . When I ran this, the commnd line shows It seems that whatever I pass through spike_params gets automatically converted into the form Please let me know if I am misunderstanding the intended usage, or if you would prefer that I investigate the current handling and propose a fix. Thank you :) |
|
Hi @sjo99-kr, For these kind of parameters, giving |
|
BTW the plan is to replace cva6.py by a more convenient script within the next weeks... Saying that, does this PR is useful ? |
|
@JeanRochCoulon the code would need to change but the feature in itself could still be discussed. |
|
Hi, @valentinThomazic . Thank you for the clarification! I now understand that However, I tried running but in the generated command it still becomes: As I mentioned above response, this still does not enable Svadu in Spike in my test. I attached two figures for comparison: one using the current PR code, and one using
In this case, the return-code error comes from my directed assembly test, which raises an error when hardware-based page-table updates are not supported.
With the PR code, the same directed test passes.
If there are any missing points, please feel free to tell me. Thank you. |
|
Hi, taking a look at the spike log should be helpful to investigate the issue |




This PR adds support for Spike-specific extension options in
cva6.py, based on Issue #3249.Current state
In
cva6.py, theisaargument is currently shared across all ISS configurations and is also passed to the riscv-compiler (risc-gcc).Because of this, when I try to compare traces between Spike and Verilator while working on Svadu, the current argument handling causes problems.
--isa_extensionapplies the extension to both Spike and GCC, which leads to errors because the ISA string is shared.--spike_paramsis not sufficient either, because it does not update the Spike ISA/variant.In my opinion, for robust verification of extensions to RVA23/RVB23, it is useful to configure Spike-specific ISA extensions independently from the compiler ISA.
--
What this PR changes
This PR updates the simulation framework in
cva6.pyby:--spike_extension--
Why PR?
I'm working on Svadu extension to the CVA6 core in my local environment, but it is hard to verify my changes with this
cva6.pyframework for using Spike as a golden reference against Verilator.It keeps triggering errors by sharing ISA between the compiler and Spike, making executing each step independently.
(It can be my fault, which I'm not familar with this CVA6 verification framework, but I tried my best....)
So I believe this makes it easier to use Spike as a golden reference for verification without affecting the GCC ISA configuration or other ISS settings.
And it is helpful to separate ISA for compiler (i.e., toolchain) and ISA for Spike for future RVA23/RVB23 extensions.
--
This modification may not be necessary for extensions that mainly affect the core pipeline, but it can be useful for extensions that affect architectural/MMU behavior than normal compiler-visible core extensions.
Thank you for reviewing this PR. If any concerns about the approach or the naming, please feel free to let me know :)