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

Initial RTL ingestion #14

Merged
merged 20 commits into from
Jan 18, 2024
Merged

Conversation

marnovandermaas
Copy link
Contributor

This PR does the following:

  • Take a snapshot of the Ibex demo system.
  • Replace Ibex with CHERIoT Ibex.
  • Replace the old bus with a TileLink Uncached Lightweight bus.
  • Add memory test.
  • Clean up file formatting.
  • Add an initial getting started guide.

This commit pulls in part of the Ibex demo system repository. It is
pulled in at revision 30404dd and you
can find this at this URL:
https://github.com/lowRISC/ibex-demo-system/tree/30404dd0d2a5565ecff2671187539e937d56ce0a

Main things that are included:
- Initial RTL
- Verilator simulation environment
- Core build files
- Demo software in C
- Vendored IP including Ibex

There are some exclusions:
- `container/` I'm not sure if we need this since we want to use Nix.
- `doc/` Sonata has a different set of documentation from the demo
system.
- `dependencies/` The Nix environment will be set up later.
- `sw/rust/` There is currently no CHERI Rust toolchain.
- `rtl/fpga/top_cw*.sv` These boards are not currently being tested.
This includes the core files, the RTL and the software. This commit does
not have any functional changes but does touch quite a few files due to
renaming.
This commit also includes three patch files:
- 0002: Build changes to CHERI-specific files
- 0003: Verilator lint fixes to build the simulation
- 0004: An upstream fix from Ibex for reading memory files:
        lowRISC/opentitan@0deeaa9
Update code from upstream repository
https://github.com/microsoft/cheriot-ibex.git to revision
1b27d74

Signed-off-by: Marno van der Maas <[email protected]>
The older version of the `prim_fifo_sync` that is included in CHERIoT
Ibex does not have an error output. This commit removes these from our
UART and SPI blocks.
This is so that we can test the bitstreams without having to separately
program the SRAM through JTAG and the debug module.
This uses the module `ibexc_top` instead of the old `ibex_top`
Copy link
Member

@HU90m HU90m left a comment

Choose a reason for hiding this comment

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

I'm happy for this to go in.

Vendoring is very messy, but the descriptive commit messages made the process easy to follow, thank you

It may be worth making an issue for the following TODO from a commit message:

This commit adds a tlul_fifo_sync to latch tl_ibex_lsu_h2d, this
needs to be investigated further and resolved for performance reasons.

Copy link
Contributor

@alees24 alees24 left a comment

Choose a reason for hiding this comment

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

Thanks and congratulations on all this work. I've left a couple of comments on the RTL changes, but I think the main thing is to address the point about ensuring the the FPGA image contains the demo code.
I've tried the FPGA build/download and the Verilator sim without other issues, thanks.

Comment on lines +117 to +123
assign core_instr_req_filtered =
core_instr_req & ((core_instr_addr & ~(tl_main_pkg::ADDR_MASK_SRAM)) == tl_main_pkg::ADDR_SPACE_SRAM);
Copy link
Contributor

Choose a reason for hiding this comment

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

Q: What happens if an Ibex in FPGA attempts to jump to an address outside of the SRAM, through coding errors/memory corruption etc?
I'm a bit concerned that we could waste development time investigating such issues; is there a way that we can make them visible, please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, I've mentioned this comment in the existing TL-UL watchdog issue: #7
For now, debugging in simulation and looking at waveforms should be enough, although could be time consuming as you say.

@marnovandermaas
Copy link
Contributor Author

I'm happy for this to go in.

Vendoring is very messy, but the descriptive commit messages made the process easy to follow, thank you

It may be worth making an issue for the following TODO from a commit message:

This commit adds a tlul_fifo_sync to latch tl_ibex_lsu_h2d, this
needs to be investigated further and resolved for performance reasons.

I've opened up an issue as you suggested: #16

Currently this does not include the debug module, simulation control and
the instruction interface.
This includes the RTL, the core files and the top package.
This is done by patching the core file that is vendored in.
- Make TileLink host adapter only depend on the assert primitive as not all
primitives are buildable in Sonata.
- Remove `err_o` from `prim_fifo_sync` instances in TL-UL files. This is
because the version of the primitives vendored into CHERIoT Ibex is
lagging behind the ones available in OpenTitan.
This makes the generated core file for the crossbar to be more portable.
Namely it removes the dependency on the life-cycle controller and
removes the `rtl/autogen` prefix.
Update code from upstream repository
https://github.com/lowRISC/opentitan to revision
b2d9b8356aa72164fb828e60ca8118495df6669d

Signed-off-by: Marno van der Maas <[email protected]>
This involves:
- Patching ram2p in Ibex to remove address offset and changing the
`rvalid` behavior.
- Instantiating the crossbar and adapters in the Sonata System.
- Adding the dependencies to FuseSoC.
- This commit also removes the debug module and simulation control,
which need to be added back in at a later point.

This commit adds a `tlul_fifo_sync` to latch `tl_ibex_lsu_h2d`, this
needs to be investigated further and resolved for performance reasons.
Update code from upstream repository
https://github.com/microsoft/cheriot-ibex.git to revision
1b27d74

Signed-off-by: Marno van der Maas <[email protected]>
This test is useful when making changes to the memory sub-system. It
first tests writing data to RAM and reading it back. Then it writes to a
peripheral register and checks the value that is read back. At the
moment it passes and fails using an infinite while loop so that you can
check the result using a wave output from simulation.
These fixes include:
- Indentation of parameter lists.
- Indentation of input/output lists.
- Indentation of variable declaration.
- Making comments full sentences ending in period.
- Adding address and data width parameters.
- Turn Verilator lints back on and resolve width and unused warnings.
- Use ANSI style of parameter declarations.
Git should ignore fst files generated by Verilator and also the
`pcount.csv` file which outputs CPU statistics from the simulation.
@marnovandermaas marnovandermaas merged commit f640161 into lowRISC:main Jan 18, 2024
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.

3 participants