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

Unify htif implementation with firesim #683

Merged
merged 3 commits into from
Oct 12, 2020
Merged

Unify htif implementation with firesim #683

merged 3 commits into from
Oct 12, 2020

Conversation

jerryz123
Copy link
Contributor

@jerryz123 jerryz123 commented Oct 2, 2020

The original inheritance structure for our various htif_t implementations was

- htif_t (upstream riscv-tools)
  - firesim_fesvr_t
  - tsi_t (forked with custom mods in esp-isa-sim)

The new inheritance structure is

- htif_t (upstream riscv-tools)
  - tsi_t (upstream riscv-tools)
    - testchip_fesvr_t (adds loadmem support)
      - firesim_fesvr_t

Users no longer have to install esp-tools to get loadmem support in CY SW sims.
Additionally lots of duplicated code between upstream tsi_t and firesim_fesvr_t is removed.

Future new features that are too controversial for upstream tsi_t can be added to testchip_fesvr_t.

Type of change: other enhancement

Impact: software change

Release Notes

Copy link
Contributor

@colinschmidt colinschmidt left a comment

Choose a reason for hiding this comment

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

LGTM. I assume you tested:

  • Verilator with and without loadmem
  • VCS with and without loadmem
  • Firesim with and without loadmem

@jerryz123
Copy link
Contributor Author

Yep those all work. For firesim I tested FireSim target SW sim, which should test the htif functionality sufficiently.

@jerryz123 jerryz123 changed the base branch from clocking-features to dev October 8, 2020 21:40
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.

2 participants