Currently the Ibex run scripts return 0 no matter what the test result
is.
To get Ibex sims correctly integrated into CI, the Makefile needs to
return 1 upon seeing a log comparison failure to indicate an error.
Signed-off-by: Udi Jonnalagadda <udij@google.com>
- This change should have no functional impact on the design
- Adding the separate module will allow easy parameterization
of security hardening for individual CSRs in the future
- As a side benefit, clock gating is added for CSRs that didn't
previously have it
- Note that this change makes the cpuctrl register always present,
rather than individual bits being added depending on parameterized
features. This is not ideal, but the parameterization becomes rather
messy otherwise.
Signed-off-by: Tom Roberts <tomroberts@lowrisc.org>
Some sequences run in "no invalidate" mode. Unfortunately, sometimes
we *have* to invalidate, either because we have a stale seed, or
because we're at the start of a sequence.
It turns out that "no invalidate" is a soft requirement, so we can
weaken it here. This patch also renames the flag to
"avoid_invalidation" to make it clear that this is allowed to be
violated occasionally.
Closes#1101.
PRJ_DIR is presumably meant to point to the root of the git repository;
instead, it was pointing one directory higher up in the hierarchy. This
worked as long as the repository was checked out into a directory called
"ibex" -- but not otherwise, as in CI.
This commit replaces the previous combination of `RV32M` bit parameter
used to en/disable the M extension and the `MultiplierImplementation`
used to select the multiplier implementation by a single enum parameter.
Signed-off-by: Pirmin Vogel <vogelpi@lowrisc.org>
The shortlog from the vendor tool's automated patch is reproduced at
the bottom of this commit message.
The automated commit is squashed with one to update how we depend on
bus parameters. Before, we had to provide an "Ibex top package". This
behaved the same as OpenTitan's "lowrisc:constants:top_pkg", but
avoided having to vendor in that file.
On the OpenTitan side, this has been tidied up with commit
d266c68 ("[dv] Update dv_utils sources to use bus_params_pkg"). This
changes the dependency of dv_utils to
"lowrisc:opentitan:bus_params_pkg". We still have to provide our
own (now called "lowrisc:ibex:bus_params_pkg") and need to patch the
dv_utils dependency, but this is a bit cleaner because dv_utils is
less likely to accidentally include dependencies on OpenTitan
internals.
On our side, we have to update the vendoring patch for dv_utils (and
change its name). We also need an equivalent patch for dv_lib. Then we
rename our hacky "Ibex top package" to "bus_params_pkg". The ICache DV
environment also needs patching to use the bus parameters properly.
Phew!
* [dv] Update prim_present cov opt (Srikrishna Iyer)
* [dv] Align VCS and Xcelium cov var names (Srikrishna Iyer)
* [dv] Split coverage for functional and auto tests (Srikrishna Iyer)
* [dvsim] Do builds smartly (Srikrishna Iyer)
* [syn] Carry over synthesis flow updates from bronze (Michael
Schaffner)
* [dvsim] Lint cleanup (Srikrishna Iyer)
* [dvsim] Allow testplan to be omitted (Srikrishna Iyer)
* [dvsim] Address lowRISC/opentitan#3071 comments (Srikrishna Iyer)
* [dvsim] lint cleanup (Srikrishna Iyer)
* [dvsim] Add support for second-level indirection (Srikrishna Iyer)
* [dvsim] Change cores-root to avoid conflicts with autogen'd core
files (Michael Schaffner)
* [dvsim] Update `tests` key behavior in regressions (Srikrishna Iyer)
* [lint] Minor update of ERROR patterns in parser script (Michael
Schaffner)
* [packer] Revise the implementation (Eunchan Kim)
* [flow] Remove lint makefile (Timothy Chen)
* [flows] Various updates to tools and documents to suppose top/ip
select (Timothy Chen)
* [dv/shadow_reg] shadow_reg update error (Cindy Chen)
* [rtl/alert] change the naming from _en_i to _req_i (Cindy Chen)
* [dvsim] Tidy up config file loading in FlowCfg.py (Rupert Swarbrick)
* [dvsim] Make it simpler to derive from FlowCfg (Rupert Swarbrick)
* [lint] Update warning/error exclusions in parser scripts (Michael
Schaffner)
* [dvsim] Fix for `--tool` override (Srikrishna Iyer)
* [dvsim] Bug fix in LintCfg.py (Srikrishna Iyer)
* [prim/dv] Integrate LFSR TB with dvsim (Udi Jonnalagadda)
* [uvmdvgen] Update template to reflect bind reorg (Srikrishna Iyer)
* [dv] remove prim_lfsr_bind (Srikrishna Iyer)
* [dv] Cleanup lint warnings in csr_utils_pkg (Srikrishna Iyer)
* [dv] Cleanup lint warnings in clk_rst_if (Srikrishna Iyer)
* [dvsim] Fix coverage dashboard link (Srikrishna Iyer)
* [prim] Rename prim_util_memload.sv to svh (Philipp Wagner)
* [lint/doc] Update linting readme to reflect recent updates (Michael
Schaffner)
* [lint] Remove legacy Makefile flow for linting tools (Michael
Schaffner)
* [dvsim/lint] Enable Verilator lint in Dvsim (Michael Schaffner)
* [prim_arbiter_fixed/fpv] Add generated FPV testbench (Michael
Schaffner)
* [prim_arbiter_fixed] This adds a fixed priority arbiter (Michael
Schaffner)
* [prim] Domain-Oriented Masking AND logic (Eunchan Kim)
* [dv] Update dv_utils sources to use bus_params_pkg (Srikrishna Iyer)
* [dv] Update mem_model to use bus-params_pkg (Srikrishna Iyer)
* [dv] Update dv_lib sources to use bus_params_pkg (Srikrishna Iyer)
* [uvmdvgen] Support for setting vendor name in VLNV (Srikrishna Iyer)
This PR adds a riscv-dv option `enable_write_pmp_csr=1` to each of the
tests in the existing PMP test suite.
This will enable the core to execute a directed test sequence to write
random values to each `pmpaddr` and `pmpcfg` CSR in order to test write
accessibility and spec compliance.
The original values stored in these CSRs are restored after this random
write.
Signed-off-by: Udi <udij@google.com>
This PR updates the `implemented_csrs` list in `riscv_core_setting.sv`
and adds the two custom CSRs `cpuctrl` and `secureseed` to the
`custom_csrs` list. Both are for use by the riscv-dv generator.
Signed-off-by: Udi <udij@google.com>
This removes the manually copied version at dv/uvm/core_ibex/common
and vendors things properly now that the vendor tool supports such
things (this picks up the same OpenTitan version as the previous
commit: lowRISC/opentitan@067272a2).
This fixes a test failure that I was seeing when following a "many
errors" test by something different. To reproduce,
make -C dv/uvm/icache/dv \
SEED=1465832714 \
TESTS=ibex_icache_stress_all_with_reset
There are actually two different ways this can come unstuck:
(1) Memory request goes out and gets put into the response queue.
req_i goes low. Sequence changes. req_i goes high and we get the
response from the previous request (but mem_err_shift has changed
in the meantime).
To fix this, we pair up the memory seed and its associated
mem_err_shift in the scoreboard queue, rather than retrieving
mem_err_shift from the config object when the response comes in.
(2) Memory request goes out. Sequence changes. Memory request is
handled (with new mem_err_shift). Scoreboard sees the result. New
sequence generates its first item.
In this case, the scoreboard will expect the old mem_err_shift and
see the new one. To fix this, we add an extra entry to the list of
valid states in the scoreboard if needed so that we also check the
mem_err_shift currently in the config object.
You might worry about what happens if we have two back-to-back
sequence changes that change mem_err_shift without ever changing seed:
what happens if we have a situation like (1), but for the "middle"
sequence. To avoid this problem, we actually add the extra entry in
the fix for (2), so it will look like a new seed arrived as part of
the middle sequence, so long as we have read at least one
result (always true in the core sequence).
This now allows you to specify how many seeds to run. Sadly, you can't
say "give me 10 seeds, starting at 1234" because dvsim doesn't support
that at the moment. But it does at least avoid quite such long command
lines.
Instead of:
../../../../vendor/lowrisc_ip/dvsim/dvsim.py \
ibex_icache_sim_cfg.hjson \
--scratch-root ../../../../build \
--reseed 5 \
-c
you can run:
make RESEED=5 COVERAGE=1
We now have a clock/reset interface and the dv_utils stuff vendored
from OpenTitan so can delete the duplicates and point the file list at
the vendored files.
The only difficulty is that the clock interfaces are slightly
different, so there are a couple of minor changes to the core_ibex
test lib, renaming "clk_if" to "clk_rst_if" and changing how we apply
resets.
Note that the testbench (core_ibex_tb_top.sv) starts the clock and
resets the DUT at the start of time. This is different from how other
OpenTitan VIP does things (where the reset happens in the sequence),
but this is the smallest change I could make from how it worked
before (where the reset happened in the clock interface itself).
The idea is that this can supply top_pkg.sv, a top-level thing in
OpenTitan, for DV code we vendor from there. It's probably better to
do this than to directly vendor in OpenTitan's top_pkg, because the
latter has information about e.g. flash memory layout, which doesn't
really have any meaning for Ibex.
cov_report_page is used by dvsim's SimCfg.py to print a message to the
console with the path to the dashboard HTML page. Most of these
messages have the full path (useful for copy-pasting), but this one
didn't.
This is essentially a duplicate of OpenTitan PR 2934[1] (because we're
not able to vendor these files properly yet).
[1] https://github.com/lowRISC/opentitan/pull/2934
As pointed out by @tomroberts-lowrisc in #983, the current
implementation of riscv_debug_single_step_test cannot handle
single-stepping over instructions that change the PC.
This PR aims to introduce this functionality, utilizing the
new instr_monitor_if.
Now, if the core single-steps onto a branch/jump instruction, the
testbench will log the new target PC and compare it against the actual
target address stored in `dpc`.
"Normal" instructions are checked as usual by incrementing the
instruction's PC by either 2 or 4 (depending whether it is compressed)
and comparing that against `dpc`.
This PR modifies the Ibex DV environment to use request/response
terminology instead of the current outdated naming scheme.
These changes are purely aesthetic.
- Add SECDED ECC checking to the register file when SecureIbex is
enabled
- No correction is attempted, but an alert is raised for the system to
intervene
Signed-off-by: Tom Roberts <tomroberts@lowrisc.org>
- Add a major and minor alert output which can be used by the system to
react to fault injection attacks
Signed-off-by: Tom Roberts <tomroberts@lowrisc.org>
But if you are using precompiled UVM it may be compiled with other timescale depending on compilation option used when it was compiled or tools default timescale value (uvm does not set timescale int the code).
In this case for us precompiled UVM timescale is 1ps/1ps - so UVM gets 1000000000 in set timeout but interprets it as ps. As a result timeout is 1000 times smaller that you expect. That is why we are getting timeouts.
It is hard to find perfect solution. One of them is to recompile the UVM with -timescale 1ns/ps (or whatever you will use for your design).
As a result of lowRISC/opentitan#2405 and lowRISC/ibex#928 (reporting
that interrupts that came in while a load instruction was in the ID
stage caused some incorrect behavior in Ibex), this PR adds some new
directed interrupt and debug tests to check that the core behaves
properly during execution of each supported instruction when some
external irq/debug stimulus comes in.
To do this, we use the two new functions `decode_instr(...)` and
`decode_compressed_instr(...)` in `core_ibex_test_list.sv` to "decode"
every instruction that the `core_ibex_instr_monitor_if` sees in the ID
stage of the pipeline. Once the testbench decodes an instruction that
we have not seen before, it can then drive interrupt or debug stimulus
into the core.
Once any given instruction has been detected by the testbench (and
stimulus driven), it will no longer drive stimulus if this instruction
is seen in the decode pipeline (e.g. if we have previously detected a
`c.addi` instruction in the ID stage and have driven irq/debug stimulus,
we will no longer drive stimulus if we see another `c.addi` instruction,
no matter the operands). This is to avoid driving irq/debug stimulus
after every single instruction as this will add a huge unwanted amount
of simulation latency.
A few notes:
- We drive irq/debug stimulus into the core every time we see a
`wfi` instruction, as otherwise we will timeout as the core waits
infinitely for some stimulus from the outside world.
- We ignore some system-level instructions (ebreak/mret/dret) and
illegal instructionsfor now, as driving stimulus during these
instructions will result in a nested trap, which requires special
handling.
- The interrupt agent was modified slightly to drive stimulus by
default on the falling edge of the clock, so this way we can "catch"
instructions that are in the ID pipeline for only a single cycle.
- The duration for which the testbench raises `debug_req_i` for the core
is also increased to avoid edge cases where we lower the debug line
too early (e.g. while long multicycle instructions like `div` are
executing in the ID stage).
- The "PINCONNECTEMPTY" waiver is part of our normal waiver file, no need
to add it to the tool invocation.
- Recent versions of Verilator choose good defaults for MAKE_OPTS,
passing it explicitly overrides the settings.
- All Verilator code is now lint clean, we can remove `-Wno-fatal`.
- FST traces are not much slower then VCD traces any more in recent
Verilator versions, remove the respective comment.
- Align comment about the compile/sim time for tracing with other files
and OpenTitan.
This commit contains some final optimizations regarding the bit
manipulation extension as well as the parametrization into a balanced
version and a full performance version.
Balanced Version:
* Supports ZBB, ZBS, ZBF and ZBT extensions
* Dual cycle instructions:
ror[i], rol, cmov, cmix fsl, fsr[i]
* Everything else completes in a single cycle.
Full Version:
* Supports all 32b sub extensions.
* Dual cycle instructions:
ror[i], rol, cmov, cmix fsl, fsr[i], crc32[c], bext, bdep
* Everything else completes in a single cycle.
Notable Changes:
* bext/bdep are now multi-cycle: Sharing additional register
with multiplier module
* grev/gorc instructions are implemented in separate structures
rather than sharing the shifter or butterfly network.
* Speed up decision on using rs1 or rs3 for alu_operand_a by
introducing single-bit register, to identify ternary
instructions in their first cycle.
* Introduce enumerated parameter to chose bit manipulation
implementation
Signed-off-by: ganoam <gnoam@live.com>
This PR adds clocking blocks to all major Ibex interfaces and updates
all corresponding interface accesses to use these clocking blocks.
A few notes:
- `ibex_mem_intf` has two driver clocking blocks, one for host side and
one for device side.
This is because our Ibex testbench currently provides both host and
device agents for both I/D interfaces (of course we only use the
reactive device agents in the main testbench).
- `csr_if` and `dut_if` only have one clocking block each, as all
signals in each will only be either sampled or driven, never both.
- Some utility tasks have been added to some interfaces to wait for a
specified number of clock cycles.
This is like the stress_all test, picking other sequences at random
and running them back-to-back. The difference is in the reset
behaviour, where we randomly pull the reset line at unexpected times
to try to trigger any strange glitches this might cause.
This requires slight changes to the core and memory drivers, which
need to learn to stop and return early from the current item when they
see a reset.
When we chain sequences together, we are careful to pass seeds between
neighbouring sequences. However, I didn't think to check
mem_err_shift. Before this patch, you see problems if you have a
"caching" sequence followed by a "many_errors" sequence with no reset
and no change of seed and they both happen to pick the same address
range.
The problem is that if the data at address A is cached in the first
sequence, the icache will merrily return it when address A comes up in
the second. However, the change to mem_err_shift might mean that this
would cause a memory error if it hadn't been cached, causing the
scoreboard to get upset.
This patch ensures that we always start a sequence with an
invalidation if there was a previous sequence with a different value
of mem_err_shift.
To do this cleanly, the patch also moves some of the "grab the guts of
the old sequence and put it in the new one" logic from
ibex_icache_combo_vseq and into the underlying sequence classes. The
trick is that a sequence now has a handle to the previous sequence (if
there was one), and can use that to extract whatever information it
needs.
This fixes several problems. Firstly, the window_reset function was
switching off tracking until it next saw busy_o go low, which is
correct at the start of time, but not what we want after we've
started. This patch splits that behaviour into a new tracking_reset
function (which calls window_reset). This is called on reset or
invalidate.
Secondly, this check was occasionally failing where we'd have an ECC
sequence (which should disable the check) immediately followed by a
caching sequence with similar addresses. If the window ended in the
caching sequence, we'd see a high fetch ratio and conclude that
something had gone wrong.
Now we clear the window completely whenever we fetch an instruction
when the check is disabled, which should avoid the problem (at worst,
you might get 1 instruction overlap, which is unlikely to matter).
Finally, we move the call to tracking_reset up to the end of the reset
sequence. It doesn't usually matter, but if there's a pending item
from the core monitor with busy = 0, we need to make sure that item
comes in before we set not_invalidating = 1. Otherwise, the scoreboard
incorrectly thinks it's seen the end of the invalidation
sequence (before it's even started) and starts tracking fetch ratios
too early.
This runs sequences back-to-back, occasionally resetting between
sequences.
Because our virtual sequences are composed of several smaller
sequences, we have to stop them when the core sequence finishes (see
the calls to kill() in ibex_icache_base_vseq). We also have to make
sure that we don't drop items in the memory sequence, which can be
pre-empted as part of sending a response (see the peek/get code
there).
Finally, the memory sequence also has a current seed and a list of
pending grants: this patch has to copy those across between sequences
to make everything work correctly.
This virtual sequence controls what sequence we use in the core agent
with a factory override. We need to make sure that we "tidy up" after
starting it, otherwise every sequence afterwards will use the wrong
core sequence.
This will have no effect for now: we just move the "pick a number in
the range 800..1000" logic to the virtual sequence.
The reason to do this is for tests that combine sequences: we want to
be able to shorten each component sequence so that the combined test
isn't way longer than the original ones were.
As with the ECC sequence, it turns out that you don't actually need
the separate test class for this, so this commit gets rid of it. The
advantage of doing this is that we can now chain this vseq with
others.
