[cheriot,rtl] Add TRVK filter to Ibex sources by thommythomaso · Pull Request #2455 · lowRISC/ibex

thommythomaso · 2026-06-30T06:56:07Z

No description provided.

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>

github-actions · 2026-07-29T07:08:07Z

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

thommythomaso · 2026-07-29T07:09:23Z

I have read the CLA Document. By submitting this pull request comment, I am hereby confirming my acceptance of the terms of the CLA Document and my agreement to be legally bound by its terms.

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>

marnovandermaas

I've left a few comments on this, mainly to make the code more clear. Do we have any tests for this load filter? We should include at least a smoke test before merging this.

marnovandermaas · 2026-07-29T08:54:53Z

+  parameter int unsigned CPERMS_W  = 6;
+
+
+  // Obtain 32-bit representation of top


marnovandermaas · 2026-07-29T08:55:25Z

+  // Obtain 32-bit representation of top
+  function automatic logic[32:0] get_bound33(logic [TOP_W-1:0] top, logic [1:0]  cor,
+                                             logic [EXP_W-1:0] exponent, logic [31:0] addr);
+    logic [32:0] t1, t2, mask, cor_val;


t1 and t2 should have more descriptive names.

Agreed, but we should align ourself with the nomenclature used in the core. @SamuelRiedel, will you rename these signals?

marnovandermaas · 2026-07-29T08:56:08Z

+  function automatic logic [2:0] update_temp_fields(logic [TOP_W-1:0] top, logic [BOT_W-1:0] base,
+                                                    logic [BOT_W-1:0] addrmi);
+    logic top_hi, addr_hi;
+    logic [2:0] res3;


This should probably be called something like "correction_bits"

I would keep this aligned with the name originally used by the reference implementation of Microsoft. The plan is to merge the current ibex_cheriot_pkg.sv with the one that will be introduced with the core.

marnovandermaas · 2026-07-29T09:01:12Z

+    .oup_ready_i(1'b1)
+  );
+
+  // Forward OBI payload between host and device


You use host and device terminology here but downstream and upstream terminology for the signals. Does it make sense to align these?

marnovandermaas · 2026-07-29T09:01:37Z

+  assign upstream_rdata_intg_o   = downstream_rsp_out.intg;
+  assign upstream_err_o          = downstream_rsp_out.err;
+
+  // Forward host to device CHERIoT tag w/o changes


Nit, probably best to spell out "without" here.

marnovandermaas · 2026-07-29T09:22:07Z

+  // We have loaded valid capability pointer, now we see valid metadata, not a sealing cap,
+  // and are pointing into the revocation bitmap
+  assign revbm_req_required = !is_sealing_cap          && // Not sealing cap
+                              ptr_store_valid_q        && // The base pointer stored is valid


This one really confused me, it would be much clearer if this signal was named "cap_load_valid_q"

marnovandermaas · 2026-07-29T09:30:00Z

+// Licensed under the Apache License, Version 2.0, see LICENSE for details.
+// SPDX-License-Identifier: Apache-2.0
+
+module ibex_trvk #(


I would prefer to rename this. One idea I had is "ibex_cap_load_filter".

I would keep this aligned with the nomenclature introduced by Microsoft.

marnovandermaas · 2026-07-29T09:32:06Z

+  ///////////////////
+
+  // Pointer buffer is filled iff tag valid & 64-bit aligned
+  assign ptr_store_enable = downstream_rsp_out.tag && !align_out && align_out_valid &&


I would prefer to rename this "ptr_load_enable". I was very confused by this code thinking that this was about storing a pointer not loading one.

Also we should probably rename "ptr" to "cap".

marnovandermaas · 2026-07-29T09:34:29Z

+  assign revbm_req_required = !is_sealing_cap          && // Not sealing cap
+                              ptr_store_valid_q        && // The base pointer stored is valid
+                              downstream_rsp_out.tag   && // We are looking at a capability
+                              downstream_rsp_out_valid && // The stored response is valid


Maybe the comment should say "The latched load response is valid"

marnovandermaas · 2026-07-29T09:35:01Z

+                              downstream_rsp_out.tag   && // We are looking at a capability
+                              downstream_rsp_out_valid && // The stored response is valid
+                              align_out                && // We are on the second word of the cap
+                              align_out_valid          && // The alignment store is valid


Maybe this comment should say "The latched alignment bits are valid"

thommythomaso · 2026-07-29T09:51:26Z

I've left a few comments on this, mainly to make the code more clear. Do we have any tests for this load filter? We should include at least a smoke test before merging this.

I have a smoke test in OpenTitan of the entire CHERIoT subsystem. I would suggest to not add a dedicated test for just the filter here, especially as we are planning to merge the core soon which then will use and verify the filter.

thommythomaso self-assigned this Jun 30, 2026

thommythomaso mentioned this pull request Jun 30, 2026

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

Open

thommythomaso mentioned this pull request Jun 30, 2026

[cheriot,rtl] Integrate TRVK into rv_core_ibex lowRISC/opentitan#30538

Draft

1 task

thommythomaso mentioned this pull request Jul 1, 2026

[CHERIoT] RTL implementation of the CHERIoT memory subsystem lowRISC/opentitan#30562

Open

20 tasks

andreaskurth reviewed Jul 3, 2026

View reviewed changes

Comment thread ibex_top.core

thommythomaso marked this pull request as ready for review July 27, 2026 09:23

thommythomaso force-pushed the tbenz/cheriot-ibex branch from 6e28f03 to 588c2d9 Compare July 29, 2026 07:07

thommythomaso force-pushed the tbenz/cheriot-ibex branch from 588c2d9 to 49af40a Compare July 29, 2026 07:09

thommythomaso added 2 commits July 29, 2026 09:12

[vendor] Vendor pulp-platform/common_cells

f989116

The `stream_fork` and `stream_join` modules are used in some CHERIoT memory IPs. Signed-off-by: Thomas Benz <tbenz@lowrisc.org>

[cheriot,rtl] Add TRVK filter to Ibex sources

c4ad698

Signed-off-by: Thomas Benz <tbenz@lowrisc.org>

thommythomaso force-pushed the tbenz/cheriot-ibex branch from 49af40a to c4ad698 Compare July 29, 2026 07:13

thommythomaso mentioned this pull request Jul 29, 2026

[doc] Update outdated parts of the documentation #2464

Open

marnovandermaas reviewed Jul 29, 2026

View reviewed changes

		parameter int unsigned CPERMS_W = 6;


		// Obtain 32-bit representation of top

Uh oh!

Conversation

thommythomaso commented Jun 30, 2026

Uh oh!

Uh oh!

github-actions Bot commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thommythomaso commented Jul 29, 2026

Uh oh!

marnovandermaas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thommythomaso commented Jul 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Jul 29, 2026 •

edited

Loading