Skip to content

[cheriot,rtl] Integrate TRVK filter into Ibex Top - #2456

Merged
andreaskurth merged 1 commit into
lowRISC:masterfrom
thommythomaso:tbenz/cheriot-ibex-int
Aug 11, 2026
Merged

[cheriot,rtl] Integrate TRVK filter into Ibex Top#2456
andreaskurth merged 1 commit into
lowRISC:masterfrom
thommythomaso:tbenz/cheriot-ibex-int

Conversation

@thommythomaso

@thommythomaso thommythomaso commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

This PR depends on #2455 and it integrates the TRVK filter into Ibex.

Currently the TRVK filter is not operated in lockstep, an issue is opened here: #2467.
The TRVK filter is not yet verified in the Ibex repo, see #2466.

@thommythomaso thommythomaso self-assigned this Jun 30, 2026
@thommythomaso
thommythomaso force-pushed the tbenz/cheriot-ibex-int branch 4 times, most recently from 9a2ee2d to 748b234 Compare June 30, 2026 09:35
thommythomaso added a commit to thommythomaso/opentitan-tbenz-pub that referenced this pull request Jun 30, 2026
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>
@thommythomaso
thommythomaso force-pushed the tbenz/cheriot-ibex-int branch 2 times, most recently from f8eaaff to 8348e15 Compare June 30, 2026 13:24
thommythomaso added a commit to thommythomaso/opentitan-tbenz-pub that referenced this pull request Jun 30, 2026
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>
@thommythomaso
thommythomaso force-pushed the tbenz/cheriot-ibex-int branch from 8348e15 to c2bd352 Compare July 1, 2026 06:24

@andreaskurth andreaskurth left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Mostly LGTM, thanks @thommythomaso!

A few comments/suggestions below

Comment thread rtl/ibex_top.sv Outdated
Comment thread dv/uvm/core_ibex/tb/core_ibex_tb_top.sv Outdated
Comment thread rtl/ibex_top.sv
Comment thread rtl/ibex_top.sv Outdated
@thommythomaso
thommythomaso marked this pull request as ready for review July 27, 2026 09:23
@thommythomaso
thommythomaso force-pushed the tbenz/cheriot-ibex-int branch 2 times, most recently from 6f6b73e to 0eaeb4d Compare July 29, 2026 07:56

@SamuelRiedel SamuelRiedel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I rebased my work on top and things worked fine. I only had one minor issue.

Comment thread rtl/ibex_top.sv
Comment thread rtl/ibex_top.sv

@andreaskurth andreaskurth left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @thommythomaso! I have a couple more comments :-) Please see below.

One comment that doesn't attach inline: on rtl/ibex_top.sv:1523: The MajorAlertOnDMemIntegrityErr assertion pairs the downstream trigger data_rvalid_i with the core-side data data_rdata_core. When the TRVK response FIFO already holds an entry those are different words, so the assertion decodes the wrong one, no? Independently, the ##[0:5] window could be exceeded while the TRVK stalls a response for a bitmap lookup (although unlikely in our current systems). Would triggering on trvk_rvalid be the right fix?

Comment thread rtl/ibex_top.sv Outdated
Comment thread rtl/ibex_top.sv
Comment thread rtl/ibex_top.sv
Comment thread rtl/ibex_top.sv
Comment thread rtl/ibex_top.sv Outdated
Comment thread dv/uvm/core_ibex/tb/core_ibex_tb_top.sv
Comment thread ibex_core.core Outdated
@thommythomaso
thommythomaso force-pushed the tbenz/cheriot-ibex-int branch 7 times, most recently from 228a6a2 to adc11b5 Compare August 10, 2026 05:52
Comment thread rtl/ibex_trvk.sv

@andreaskurth andreaskurth left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks @thommythomaso, this addresses everything from the last round. Just one thing left, below.

Comment thread rtl/ibex_top.sv Outdated
@thommythomaso
thommythomaso force-pushed the tbenz/cheriot-ibex-int branch from adc11b5 to 5e2c7c8 Compare August 10, 2026 20:07
@thommythomaso
thommythomaso force-pushed the tbenz/cheriot-ibex-int branch from 5e2c7c8 to c148089 Compare August 10, 2026 20:22
Signed-off-by: Thomas Benz <tbenz@lowrisc.org>
@thommythomaso
thommythomaso force-pushed the tbenz/cheriot-ibex-int branch from c148089 to d7d679d Compare August 11, 2026 11:06

@andreaskurth andreaskurth left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, thanks @thommythomaso!

@andreaskurth
andreaskurth added this pull request to the merge queue Aug 11, 2026
@andreaskurth
andreaskurth removed this pull request from the merge queue due to the queue being cleared Aug 11, 2026
@andreaskurth
andreaskurth merged commit 2910e53 into lowRISC:master Aug 11, 2026
13 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 11, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants