[doc] Update documentation to align with recent RTL changes by SamuelRiedel · Pull Request #2484 · lowRISC/ibex

SamuelRiedel · 2026-08-28T10:08:55Z

Some new changes to Ibex were not reflected in the documentation. This PR updates it:

add new ibex_top parameters
add cross-refs throughout the parameter list to explain what they do without duplication
add missing ports and their description
udpate instantiation snippet updated to match current RTL
add missing CSRs
add pipeline details for pipeline flush on CSR writes
expand explanation on Zcmp behavior concerning interrupts

The CHERIoT documentation is deferred to a separate PR

nasahlpa

LGTM apart from one question.

nasahlpa · 2026-08-30T11:37:07Z

+|                                  |                     |                         | (only relevant when ICacheScramble == 1)                                                     |
+----------------------------------+---------------------+-------------------------+----------------------------------------------------------------------------------------------+
+| ``ICacheTweakInfection``         | bit                 | SecureIbex              | Enable :ref:`icache-tweak-infection`                                                         |
+|                                  |                     |                         | (only relevant when ICacheScramble == 1)                                                     |


Is that correct? As far as I remember, it is only useful when having SecureIbex enabled (because then ECC errors are flagged by a wrong un-XOR operation) and when we have an icache available.

You're right. That was a copy-paste error.

Signed-off-by: Samuel Riedel <sriedel@lowrisc.org>

We check all incoming valid responses, not just load data Fixes lowRISC#2200 Signed-off-by: Samuel Riedel <sriedel@lowrisc.org>

andreaskurth

Thanks @SamuelRiedel for updating the Ibex documentation with many recent RTL changes (not just CHERIoT)!

This mostly LGTM, but I have see a few points that should be changed: one factual mix-up on ICacheScrNumPrinceRoundsHalf, a few over- or under-stated qualifiers, and some stale text sitting at the far end of the new cross-references.

andreaskurth · 2026-08-31T12:42:59Z

+----------------------------------+---------------------+-------------------------+----------------------------------------------------------------------------------------------+
+| ``PMPNumRegions``                | int (1..16)         | 4                       | Number implemented PMP regions (ignored if PMPEnable == 0)                                   |
+----------------------------------+---------------------+-------------------------+----------------------------------------------------------------------------------------------+
+| ``MHPMCounterNum``               | int (0..10)         | 0                       | Number of :ref:`performance-counters`                                                        |


Following this new :ref: lands on performance_counters.rst:85-90, which uses WidthMHPMCounters/NumMHPMCounters (RTL names them MHPMCounterWidth/MHPMCounterNum) and caps the useful count at 8 (mhpmcounter3 - mhpmcounter10). The RTL defines events up to mhpmcounter12 (ibex_cs_registers.sv:1583-1597), i.e. 10, which matches this row and that page's own event table. Worth fixing there too while we're at it?

andreaskurth · 2026-08-31T12:42:59Z

+----------------------------------+---------------------+-------------------------+----------------------------------------------------------------------------------------------+
+| ``ICacheECC``                    | bit                 | 0                       | Enable :ref:`icache-ecc` (if ICache == 1)                                                    |
+----------------------------------+---------------------+-------------------------+----------------------------------------------------------------------------------------------+
+| ``BranchPredictor``              | bit                 | 0                       | *EXPERIMENTAL* Enable :ref:`branch-prediction`                                               |


The :ref: target still calls the parameter BranchPrediction (instruction_fetch.rst:34). Rename it there too so the cross-reference doesn't contradict the name in this row?

andreaskurth · 2026-08-31T12:42:59Z

+| ``SecureIbex``                   | bit                 | 0                       | Enable various :ref:`security`. Note: SecureIbex == 1'b1 and                                 |
+|                                  |                     |                         | RV32M == ibex_pkg::RV32MNone is an illegal combination.                                      |
+----------------------------------+---------------------+-------------------------+----------------------------------------------------------------------------------------------+
+| ``LockstepOffset``               | int (>= 1)          | 1                       | Number of cycles to delay the :ref:`lockstep` shadow core                                    |


Add "(only relevant when SecureIbex == 1)" here, like the neighbouring rows carry? Lockstep = SecureIbex, so the offset has no effect otherwise.

andreaskurth · 2026-08-31T12:42:59Z

+----------------------------------+---------------------+-------------------------+----------------------------------------------------------------------------------------------+
+| ``LockstepOffset``               | int (>= 1)          | 1                       | Number of cycles to delay the :ref:`lockstep` shadow core                                    |
+----------------------------------+---------------------+-------------------------+----------------------------------------------------------------------------------------------+
+| ``MemECC``                       | bit                 | SecureIbex              | Enable SECDED ECC for :ref:`bus-integrity`                                                   |


Following this new :ref: lands on security.rst:93, which still says "When the SecureIbex parameter is set, incoming data will be checked ..." (same in load_store_unit.rst:68). Now that MemECC is documented as an independent parameter, should those two name MemECC instead?

andreaskurth · 2026-08-31T12:42:59Z

+----------------------------------+---------------------+-------------------------+----------------------------------------------------------------------------------------------+
+| ``MemECC``                       | bit                 | SecureIbex              | Enable SECDED ECC for :ref:`bus-integrity`                                                   |
+----------------------------------+---------------------+-------------------------+----------------------------------------------------------------------------------------------+
+| ``MemDataWidth``                 | int (>= 32)         | MemECC ? 32 + 7 : 32    | Data width of memory interface including integrity bits                                      |


int (>= 32) reads as if this were freely choosable, but it is derived from MemECC and effectively fixed at 32 or 39: ibex_top.sv:864 slices data_wdata_core[MemDataWidth-1:32] into a prim_buf #(.Width(7)), so any other value breaks elaboration. Say "derived" or "32 or 39" in the range column?

andreaskurth · 2026-08-31T12:42:59Z

+| 0      | RW   | **CY:** Enable U-mode access to ``cycle`` / ``cycleh``.                             |
+--------+------+-------------------------------------------------------------------------------------+
+
+Writes to ``mcounteren`` are only accepted when the ``mcounteren_writable_i`` input is set to ``IbexMuBiOn``.


performance_counters.rst:77 describes the same mechanism but calls the signal mcounteren_writeable. Fix the name there and cross-reference here, so the behaviour is only spelled out once?

andreaskurth · 2026-08-31T12:42:59Z

 +-----------------------+--------------------------------------+-------------------------------------------------------------+
 | CSR Access            | 0                                    | CSR Access Instruction are defined in 'Zicsr' of the        |
-|                       |                                      | RISC-V specification.                                       |
+|                       |                                      | RISC-V specification. Note that any CSR *write* flushes the |


"any CSR write" is slightly too strong: mscratch and mepc are exempt via no_flush_csr_addr (ibex_id_stage.sv:593), and the RTL comment says "on most CSR modification". Mention the two exceptions?

andreaskurth · 2026-08-31T12:42:59Z

+
+.. note::
+
+   **Zcmp interrupt and exception behaviour**


Could the note also cover debug-mode entry? It is blocked over a wider window than interrupts: INSTR_EXPANDED and INSTR_EXPANDED_COMMIT (ibex_controller.sv:474-477), versus INSTR_EXPANDED_COMMIT only for handle_irq (ibex_controller.sv:499).

andreaskurth · 2026-08-31T12:42:59Z

+
+   **Zcmp interrupt and exception behaviour**
+
+   * **Interrupt masking:** Interrupts are suppressed for the duration of a Zcmp push/pop commit sequence.


cm.push never reaches INSTR_EXPANDED_COMMIT (it goes INSTR_EXPANDED -> INSTR_EXPANDED_LAST), so interrupts are not suppressed during it. The sentence two lines down already says as much, so dropping "push/" here would remove the contradiction.

andreaskurth · 2026-08-31T12:43:00Z

+   * **minstret counting:** An entire Zcmp push/pop sequence (regardless of how many register load/store micro-operations it expands to) is counted as a single retired instruction in ``minstret``.
+
+   * **Exception/interrupt during expansion:** If an exception or interrupt is signalled before the commit sequence has started, the Zcmp state machine is flushed and the partial sequence is abandoned.
+     The core takes the trap as if the Zcmp instruction had not yet started, so software can safely restart it on return from the handler.


"as if the Zcmp instruction had not yet started" seems a bit strong: a partial cm.push has already written stack memory (negative offsets from sp, see cm_push_store_reg) and a partial cm.pop has already overwritten registers. The reason restarting is safe is that sp is adjusted last, so the micro-ops are idempotent. Say that instead?

SamuelRiedel marked this pull request as ready for review August 28, 2026 14:39

SamuelRiedel requested review from andreaskurth, gautschimi and nasahlpa August 28, 2026 14:39

SamuelRiedel mentioned this pull request Aug 28, 2026

[doc] Add CHERIoT documentation #2486

Draft

nasahlpa reviewed Aug 30, 2026

View reviewed changes

SamuelRiedel added 2 commits August 30, 2026 17:09

[doc] Catch up integration.rst to current RTL state

f36f2ac

Signed-off-by: Samuel Riedel <sriedel@lowrisc.org>

[doc] Clarify bus integrity checking

7ea6405

We check all incoming valid responses, not just load data Fixes lowRISC#2200 Signed-off-by: Samuel Riedel <sriedel@lowrisc.org>

SamuelRiedel force-pushed the doc branch from ee01352 to 7ea6405 Compare August 30, 2026 15:09

andreaskurth changed the title ~~[doc] Update documentation to align with recent RTL chagnes~~ [doc] Update documentation to align with recent RTL changes Aug 31, 2026

andreaskurth reviewed Aug 31, 2026

View reviewed changes


		Zcmp interrupt and exception behaviour

		* Interrupt masking: Interrupts are suppressed for the duration of a Zcmp push/pop commit sequence.

Uh oh!

Conversation

SamuelRiedel commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nasahlpa 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!

andreaskurth 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

SamuelRiedel commented Aug 28, 2026 •

edited

Loading