Otherwise an alert is raised if a core has a different
configuration than zero. There, the primary core would
return the non-zero value but the lockstep core zero.
This difference causes an alert.
This PR fixes this bug and passes the mvendirid and mimpid
parameters also to the lockstep core so that they are in sync
Signed-off-by: Robert Schilling <rschilling@rivosinc.com>
Instead of using default values from a package, create a top-level
parameter to define these and pass them down. This allows integrators
to specify them on a per-instance basis.
Signed-off-by: Robert Schilling <rschilling@rivosinc.com>
The "else" part of the if/else check here wasn't possible because the
surrounding else branch (starting at line 406) is already in the case
where instr[26] is zero.
When encountering certain illegal compressed instructions, incorrect instruction information was displayed. Now, illegal instructions can be printed correctly.
This removes several assertions from `ibex_controller`. They aimed to
ensure that controller behaviour was correct on exception behaviour
(e.g. ensuring that a pending interrupt will actually trigger an
interrupt). However they've proved to be flaky and hard to maintain with
multiple edge cases needing to be accounted for.
The co-simulation checking in functional verification will catch the
same issues these assertions catch. The assertions (when working
correctly) would cause a failure directly when the bug happens which
makes debugging easier. However they've added significant effort in
regression triage due to their many false failures so it's not worth the
maintenance burden.
Within formal they don't really add any value now we have the full
end-to-end formal flow.
This fixes#2193, an issue that meant bit clears in PMP related CSRs
didn't immediately apply to an instruction already in the fetch stage
due to a lack of a pipeline flush.
With this change the pipeline will flush in that scenario, fixing the
issue. It now flushes the pipeline on all CSR modifications as this
makes the pipeline more resliant against similar issues in the future
(where the list of CSRs to flush on should have been updated but
wasn't).
Previously the ibex_cs_registers module received the CSR address via the
operand muxes. This has been observed to cause timing issues in some
cases. The CSR address is always read from the same bits of the
instruction so there's no need to go via the operand muxes. With this
change the relevant instruction bits are fed straight out of the decoder
and into the ibex_cs_registers module.
This assertion wasn't quite correct if SecureIbex is false because it
was checking for the magic IbexMuBiOn value instead of just looking at
the bottom bit.
Fixes#2249.
These were noticed by someone responding to issue #2230. I think the
author's original logic was to point out that there's a path from e.g.
raddr_a_i to rdata_a_o which doesn't depend on any clock, so is
"asynchronous".
But that's the same in the other modes and also for the other register
file implementations, which don't have analogous comments.
Drop these ones.
The RISC-V Debug Specification (current release 1.0.0-rc4) in Section
A.2 states that the PMP must not disallow accesses to addresses of the
Debug Module when the hart is in debug mode, regardless of how the PMP
is configured. This commit changes the PMP accordingly.
Signed-off-by: Andreas Kurth <adk@lowrisc.org>
When targeting Xilinx FPGAs, we utilize a DSP for counters
with a width of less than 49-bit. In this case, a sync. reset
is needed. However, currently, there is a bug in the RTL
where also a sync. reset is used for the non-DSP counters
on the FPGA.
Signed-off-by: Pascal Nasahl <nasahlpa@lowrisc.org>
If the counter width is >= 49, we do not use a DSP on the FPGA.
Then, we should use an asynchronous reset to initialize the counter.
This bug was detected when enabling the lockstep for the CW340. A
lockstep mismatch happend as the mcycle counters of the main and
shadow core did not match due to this bug.
Signed-off-by: Pascal Nasahl <nasahlpa@lowrisc.org>
We should use `WordZeroVal` instead of `0` for reads from register `x0` in the
FPGA register file.
This bug was discovered when enabling the `RegFileECC` parameter. When this is
enabled, the core performs ECC checks, expecting that `WordZeroVal` is returned
for `x0`. Else, we get a major alert.
FixeslowRISC/opentitan#25146
Signed-off-by: Pascal Nasahl <nasahlpa@lowrisc.org>
By using top-level straps for the PMP reset configuration its
easier to implement different reset configurations if there are
multiple Ibex cores in the system.
Signed-off-by: Robert Schilling <rschilling@rivosinc.com>
Prior to this commit an ECC failure on the incoming data memory response
factored directly into the outputs for the instruction memory
interfaces. This existed due to a desire to take an NMI on an ECC
failure as soon as possible but causes timing issues so it has been
altered.
Now rather than directly raise the NMI the same cycle the assertion of
'irq_nm_int' is delayed by a cycle which breaks the feedthrough path.
The 'pre_val' MIP addresses the scenario where MIP changes as an
instruction is excuting, this means a CSR instruction can observe a
different MIP from the one that decides whether or not that instruction
will be interrupted.
With this change all memory responses are only acted on if Ibex is
expecting them for all secure configurations. Previously an error
response that was injected onto the bus would trigger an exception that
shouldn't occur (in particular breaking the functioning of the multiply
state machine). In addition for configurations without the writeback
stage an injected load data response could trigger an incorrect write to
the register file.
This is only applied to the secure configurations, non-secure
configurations assume correct adherence to the bus protocol meaning a
response will only be seen if a request is outstanding.
This parameter allows integrators controlling the number of PRINCE
half rounds in the scrambled ICache SRAM primitives, e.g., to balance
timing impact and security guarantees.
Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>
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>
