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

Verilator v5 #102

Merged
merged 7 commits into from
May 27, 2024
Merged

Verilator v5 #102

merged 7 commits into from
May 27, 2024

Conversation

GregAC
Copy link
Contributor

@GregAC GregAC commented May 27, 2024

I'd like this working for the hackathon so if you've got more major changes you'd like following review I'd appreciate it if we could just record it in an issue for now and get this merged.

@HU90m
Copy link
Member

HU90m commented May 27, 2024

This looks brilliant, thanks Greg!

I'll quickly change the GitHub actions to do simulator builds in the nix sandbox, so we don't have to keep updating the dependencies.

@GregAC
Copy link
Contributor Author

GregAC commented May 27, 2024

This looks brilliant, thanks Greg!

I'll quickly change the GitHub actions to do simulator builds in the nix sandbox, so we don't have to keep updating the dependencies.

Thanks I've just added a commit to this PR to do just that but as you know nix better than me let's see what you propose as well!

.github/workflows/ci.yml Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@GregAC
Copy link
Contributor Author

GregAC commented May 27, 2024

Thanks @HU90m, any thoughts on how to make it use the lowRISC cache? It spends the first few minutes building python dependencies

@HU90m
Copy link
Member

HU90m commented May 27, 2024

This should work for now:

nix build --accept-flake-config .#sonata-simulator

It would be nice to get the sonata-system CI pushing to the cache and running on pomona, especially once #98 is merged. @nbdd0121 (tagging for visibility, no immediate action required.)


See #102 (comment) for a better solution.

sw/cheri/sim_boot_stub/makefile Outdated Show resolved Hide resolved
@@ -0,0 +1,9 @@
CFLAGS=-target riscv32-unknown-unknown -mcpu=cheriot -mabi=cheriot \
Copy link
Contributor

Choose a reason for hiding this comment

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

It'll be good that this uses cmake similar to other binaries that we have, but we can save that to a later PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It uses make intentionally so you can build it in environments where you don't have cmake just to minimize dependencies required to build it

flake.nix Outdated
buildPhase = ''
HOME=$TMPDIR fusesoc --cores-root=. run \
--target=sim --tool=verilator --setup \
--build lowrisc:sonata:system \
--verilator_options="+define+RVFI -j $NIX_BUILD_CORES" \
--make_options="-j $NIX_BUILD_CORES"
--make_options="-j $NIX_BUILD_CORES" &&
make -C sw/cheri/sim_boot_stub/
Copy link
Contributor

Choose a reason for hiding this comment

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

We probably should put the sim stub building in another derivation.

Copy link
Member

Choose a reason for hiding this comment

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

I had a similar thought, but I'm happy for this to go in for now. In a follow up PR, we can transition to CMake and create derivations for the software builds.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did actually do it that way initially, then switched it over to this as it felt like it's something you should distribute with the simulator

Copy link
Contributor

Choose a reason for hiding this comment

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

What could be done is to build it in a separate derivation, and then copy it to $out/share for the simulator.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Turns out we need to do this to get the osx build to work. CHERIoT LLVM clashes with the system LLVM and the verilated C++ fails to build so will make that change.

sudo apt install verilator libelf-dev
pip install -r python-requirements.txt
- name: Install Nix
uses: cachix/install-nix-action@v24
Copy link
Contributor

Choose a reason for hiding this comment

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

To enable cache globally for all nix commands, you can do something similar to https://github.com/lowRISC/lowrisc-nix/blob/34f1d472d6066a42c684416f5e9e755d25ec6270/.github/workflows/ci.yml#L52-L54 here.

flake.nix Outdated Show resolved Hide resolved
Copy link
Contributor

@marnovandermaas marnovandermaas left a comment

Choose a reason for hiding this comment

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

Works for me. Happy to merge this once CI passes.

lint_off -rule UNOPTFLAT -file "*/rtl/prim_subreg_ext.sv"
lint_off -rule BLKSEQ -file "*/rtl/ibex_tracer.sv"

// Bug seem in Verilator v5.020 where trace chandles produces a C++ compliation
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: handles, compilation

GregAC added 7 commits May 27, 2024 13:18
With this fixed direct elf loading works properly
Previously C++11 was requested in the verilator arguments. Pinning to a
specific C++ version is no longer required and verilator v5 generated
code requires C++14 features.
The stub just immediately jumps to the start address for programs loaded
via the flash bootloader. It allows programs built to be started via the
flash bootloader to run unmodified on the simulator.
@GregAC
Copy link
Contributor Author

GregAC commented May 27, 2024

@HU90m happy to approve this now?

@HU90m HU90m merged commit 8bc7c50 into lowRISC:main May 27, 2024
2 checks passed
@GregAC GregAC deleted the verilator_v5 branch May 27, 2024 13:04
@marnovandermaas
Copy link
Contributor

You force pushed over my documentation change, I've made a follow up PR here: #105

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.

4 participants