The latest version of `prim_count` from OpenTitan introduces a
`commit_i` input. To retain the behaviour of the previous `prim_count`
this should be set to a constant 1.
The `cnt_next_o` output has been renamed to `cnt_after_commit_o`.
Modify tracer to use the appropriate read/write masks when logging
load/store traffic from the Load Store Unit.
Signed-off-by: Adrian Lees <a.lees@lowrisc.org>
Currently, the dual-core lockstep FI mitigation is enabled/disabled
using a single bit.
For transient bit-flips, this is not problematic, as one bit-flip
into this signal and one bit into the Ibex is required to threaten
the security of the system.
However, a permanent stuck-at-0 fault could disable the lockstep
completely by targeting this signal. Then, only a single, additional
fault (transient or permanent) is required.
This PR enhances the FI resilience of the Ibex lockstep by encoding
this single bit into a ibex_mubi_t signal, i.e., a 4-bit multi-bit
signal.
Signed-off-by: Pascal Nasahl <nasahlpa@lowrisc.org>
As described in #20715, a single fault-induced bit-flip inside the
register file could change which of the register file value is
provided to Ibex.
This PR fixes this issue by (i) encoding raddr_a/b to one-hot
encoded signals, (ii) checking these signals for faults, and
(iii) using an one-hot encoded MUX to select which register file
value is forwarded to rdata_a/b.
Area increases by ~1% (Yosys + Nangate45 synthesis).
I conducted a formal fault injection verification at the Yosys
netlist to ensure that the issue really is fixed.
Signed-off-by: Pascal Nasahl <nasahlpa@lowrisc.org>
Recent versions of Verilator complain about the code that was there
because the csr_pmp_cfg argument clashes with a name in ibex_core.sv.
What's more, they mean different things! In ibex_core.sv, it was the
PMP configuration for the entire core. In the functions, it's the PMP
configuration for a single region. This patch adds a "region_" prefix
to the names, which fixes both the Verilator warning and my confusion!
The existing code wanted to open file_handle as a trace file if
necessary and then use it on that clock cycle. So it (sensibly) used a
blocking assignment.
Verilator now warns about blocking assignments to globals in
"sequential logic processes" (the always_ff that is driving
everything). This is sort of easy to fix: just use an "always" block!
This commit looks slightly more involved because I've changed things
to pass the file handle to printbuffer_dumpline as an argument. It
makes the state update (where we open the file handle) a little easier
to follow.
Previously any changes in interrupt state or debug requests were
strictly associated with retired instructions. This causes cosim
mismatches where a lower priority interrupt occurs in time before a
higher priority interrupt or debug request but between instruction
fetches/retirements so both the low and high priority interrupts are
signalled with the instruction retirement.
This introduces a way for the RVFI to signal an interrupt has occurred
that isn't associated with an instruction retirement to allow the cosim
to see the seperation in time between different interrupts and debug
requests and hence model behaviour correctly.
- rvfi_trap now correctly handled for writeback
- issue created to track coverpoint for pmpcfg reserved bits writes.
- flush pipe on debug CSR writes is reasonable
This commit adds two assertions in `ibex_top` to ensure that the
scramble key is correctly applied to the icache scrambled memory
primitives. Those assertions previously existed in the module that
instantiated Ibex in OpenTitan, but the reference into the generate
loops was problematic for some EDA tools; see lowRISC/opentitan#17155.
Additionally, the assertions previously used the input scramble key
(`scramble_key_i`) even though they tolerated a delay after which the
input scramble key was not necessarily valid anymore (i.e.,
`scramble_key_valid_i` could go low and `scramble_key_i` could take any
value). This mistake has been corrected by sampling the input scramble
key for the assertions when it is valid and using the sampled value in
the comparison of the assertions. This problem surfaced in the DV
environment of Ibex (but not in OpenTitan), where multiple tests
(including `riscv_rand_instr_test`, `riscv_mem_error_test`, and
`riscv_multiple_interrupt_test`) failed; these tests now pass.
Signed-off-by: Andreas Kurth <adk@lowrisc.org>
Previously, it was possible to glitch data_rvalid_i at the interconnect
level and if the data integrity bits happened to be valid, Ibex would
write the current data_rdata_i into the register file even if it wasn't
doing a load. Since the glitch is inserted at the interconnect level,
both the main and the shadow core are affected equally.
This commit changes the WB stage to only forward the LSU write enable,
which is generated from data_rvalid_i, when Ibex is actually waiting for
an interconnect response for a load instruction. This substantially
narrows down the window for attacks at the interconnect level.
Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>
Xprop is a simulation feature that improves the SV semantics when
conditions contain 'X values. Change RTL or DV code to enable more xprop
instrumentation.
This addresses lowRISC/opentitan#16791 and some of
lowRISC/opentitan#16723.
Signed-off-by: Guillermo Maturana <maturana@google.com>
This refactors the code to avoid a -1 index access that caused no issues
in functional verification but caused lint errors and is problematic for
formal tools.
Fixes#1799
When Ibex does a load that receives data with bad integrity it
suppresses the write to the destination register. The implements
matching functionality for cosim.
This commit is mainly an extension to cosim environment to drive the newly
introduced state variable `nmi_int` in Spike.
This commit
- Extends RVFI interface by a single bit (ext_nmi_int)
- Configures cosim to set nmi_int inside Spike
Signed-off-by: Canberk Topal <ctopal@lowrisc.org>
Previously there was a single dummy_instr_id_o signal from ibex_core
which the register file used to determine if it could write to the zero
register (which reads as zero always for real instructions). However a
write occurs in the writeback stage so this signal was not asserted
correctly.
This adds a dummy_instr_wb_o signal to control the write to zero
register. dummy_instr_id_o remains as it's still employed for register
reads for dummy instructions.
Previously it was asserted when an instruction in ID would cause an
exception but an earlier instruction in WB also causes an exception
which takes priority.
This didn't cause a functional bug as the `id_exception_o` signal was
used in a single place ORed with `wb_exception_o`. However it was
confusing behaviour and could cause killed instructions to appear on the
RVFI causing false cosim mismatches.
It was triggered only on the debug wakeup actually occurring, so in
particular would never capture debug activity around entering sleep. Now
it just considers if there's something that would trigger debug wakeup.
This cross wasn't much use as many of the transitions it was crossing
with instruction types only occur when the pipeline is empty (so there's
no instruction type to check).
The remaining interesting cases are already covered by other crosses
(e.g. `debug_if_entry_instr_cross` and `pipe_flush_instr_cross`).
Also adds an assertion to check the pipe is empty when we transition to
IRQ_TAKEN (we need this condition to hold to ensure we don't need extra
coverage for instruction types on this transition).
When in the FLUSH state we cannot have `csr_pipe_flush` set as it
depends upon `instr_executing` being set (within `ibex_id_stage`) and
that is only set in the DECODE stage.
We should only indicate an ebreak debug cause if an ebreak leads to a
debug entry (otherwise when single stepping over an ebreak that traps to
an exception we incorrectly enter debug mode with an ebreak cause).
This commit protects the core_busy_o signal using a multi-bit encoding
to reduce the chances of an adversary for glitching this signal to low,
thereby putting the core to sleep and e.g. not handling an alert.
Without this commit, the glitch would only be detected once both the
main core and the shadow core wake up again and the comparison of the
core_busy_o signals continues.
This resolveslowRISC/Ibex#1827.
Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>
Previously if a dummy instruction entered the pipeline whilst it
wouldn't make RVFI stage 0 valid, it would make RVFI stage 1 valid.
Now stage 1 can only become valid if stage 0 was valid.
Previously `fetch_enable_i` only controlled the request going into the
instruction fetch stage. Due to buffering in the prefetch queue and
icache when this request is dropped it's possible for multiple
instructions to still be available for the ID/EX stage to consume. So
when `fetch_enable_i` was set to off you would get a 'soft stop'. Some
finite number of instructions may still execute and Ibex would come to
an eventual halt.
Now `fetch_enable_i` also gates the instruction moving between the fetch
stage and the ID/EX stage. This gives a 'hard stop' where once fetch is
disabled Ibex comes to an immediate halt.
This commit changes when we cath the debug causes. Since debug_cause_o
only gets latched when `csr_save_cause_o` is high, it would work if
we change the cause with a mux that is connected to the input signals.
Resolves#1772
Signed-off-by: Canberk Topal <ctopal@lowrisc.org>
Previously Ibex signalled a major alert on an integrity error (where
incoming read data doesn't match its integrity bits) for both read and
write responses. This was removed as the data part of a response to a
write is ignored.
This brings it back in a more measured way. This provides a little extra
fault injection hardening as an attacker glitching the memory bus will
generate an alert on both read and write responses.
Observing the spec change:
RISC-V Debug Support Version 1.0.0-STABLE
1.2.1.4 New Features from 0.13 to 1.0
> 8. Move scontext, renaming original to mscontext, and create hcontext. #535
MSCONTEXT is a backwards-compatible alias to SCONTEXT
In Ibex, SCONTEXT is a read-only zero register. Hence MSCONTEXT has the same behaviour.
