[cheriot,rtl] Integrate TRVK filter into Ibex Top#2456
Conversation
The `stream_fork` and `stream_join` modules are used in some CHERIoT memory IPs. Signed-off-by: Thomas Benz <tbenz@lowrisc.org>
Signed-off-by: Thomas Benz <tbenz@lowrisc.org>
9a2ee2d to
748b234
Compare
This commit updates the vendored ibex to `thommythomaso/ibex-tbenz-pub`. This has to be updated to point to `lowrisc/ibex` as soon as lowRISC/ibex#2455 and lowRISC/ibex#2456 are merged. Signed-off-by: Thomas Benz <tbenz@lowrisc.org>
f8eaaff to
8348e15
Compare
This commit updates the vendored ibex to `thommythomaso/ibex-tbenz-pub`. This has to be updated to point to `lowrisc/ibex` as soon as lowRISC/ibex#2455 and lowRISC/ibex#2456 are merged. Signed-off-by: Thomas Benz <tbenz@lowrisc.org>
Signed-off-by: Thomas Benz <tbenz@lowrisc.org>
8348e15 to
c2bd352
Compare
andreaskurth
left a comment
There was a problem hiding this comment.
Mostly LGTM, thanks @thommythomaso!
A few comments/suggestions below
| trvk_revbm_data_intg_error | | ||
| trvk_revbm_device_error; |
There was a problem hiding this comment.
Shouldn't these feed into alert_major_bus_o? Potentially worth discussing with Pascal and/or Pirmin
| .trvk_revbm_addr_o ( ), | ||
| .trvk_revbm_rdata_i ('b0 ), | ||
| .trvk_revbm_rdata_intg_i ('b0 ), | ||
| .trvk_revbm_err_i ( ), |
There was a problem hiding this comment.
Suggest tying off (only the last line)
| trvk_revbm_rdata_i, | ||
| trvk_revbm_rdata_intg_i, | ||
| trvk_revbm_err_i, | ||
| trvk_rtag, |
There was a problem hiding this comment.
suggest tying off in this branch (only the last line) - otherwise it's both undriven and unused, which may cause lint problems
| @@ -21,6 +22,8 @@ module ibex_top import ibex_pkg::*; #( | |||
| parameter ibex_pkg::pmp_cfg_t PMPRstCfg[PMP_MAX_REGIONS] = ibex_pkg::PmpCfgRst, | |||
| parameter logic [PMP_ADDR_MSB:0] PMPRstAddr[PMP_MAX_REGIONS] = ibex_pkg::PmpAddrRst, | |||
| parameter ibex_pkg::pmp_mseccfg_t PMPRstMsecCfg = ibex_pkg::PmpMseccfgRst, | |||
| parameter int unsigned CheriotRevBitmapAddrWidth = 32'd9, | |||
| parameter int unsigned CheriotRevBitmapBaseAddr = 32'h0, | |||
There was a problem hiding this comment.
Suggest documenting the new parameters in integration.rst. Not a blocker - if you prefer to do this later, please create an issue.
Similarly, I suggest describing the internal workings of this PR and #2455 in the reference guide - also not blocking but should be tracked in an issue if defered.
This PR depends on #2455.