ibex
ibex copied to clipboard
[dv, fcov] PMP related coverage holes
@ctopal assigned to all of these
The following PMP coverpoints are not yet being hit:
- [x] A higher priority entry allowing an access a lower priority entry denies -
cp_pmp_iside_region_override
/cp_pmp_iside2_region_override
/cp_pmp_dside_region_override
- [ ] NA4 and NAPOT matching modes and all privileges other than RWX for region 0 -
cp_region_mode
/cp_region_priv_bits
Assigned to: @marnovandermaas - [x] All locked privilege settings for all regions -
cp_region_priv_bits
- [x] Attempting to edit a locked region -
cp_edit_locked_pmpcfg
/cp_edit_locked_pmpaddr
I've ignored ePMP related functionality for now as it's known that's not being stimulated and initial work to rectify this is underway.
Original estimate 8 Estimate before 2022-08-02: 16
estimate 20 remaining 2022-07-28 8 remaining 2022-08-02 12 remaining 2022-08-02 8 remaining 2022-08-09 6
I haven't looked at the first one yet but here are my overall findings:
For the second one, I am not exactly sure what region 90 means but I can also see R/RW/X/XR
privileges and every matching modes in every region except region 0 when I run these tests: riscv_pmp_basic_test, riscv_pmp_disable_all_regions_test, riscv_pmp_out_of_bounds_test
with iteration count of 5. (please bare in mind those tests probably fail in our nightly regressions because of #1712)
Locked privilege settings are indeed not getting set even with these tests, which probably tells it has something to do with some riscv-dv knob and require further investigation. That problem also explains item 3 and 4.
After having a more detailed look at the reports and testing out Marno's PR for ePMP this one looks like it's going to be a bigger task than I first thought.
1 - Region override: This one is only possible when we are in NA4
mode in PMP (which is not our default mode and it's currently not possible to only randomize mode
field of pmpcfg's.) so we need to either define a directed test to hit this or refactor how we handle PMPCFG
randomization in riscv-dv.
2 - Similar as above, if we are not explicitly setting NA4 mode we are highly unlikely to see it due to not having a field specific randomization in PMPCFG
registers. Also as far as I understand, we are not even supposed to see NAPOT
with our current Ibex configuration of opentitan
. The reason is the parameter of PMPGranularity
is 0 in opentitan
config (which only allows for TOR
and NA4
according to the spec.)
3 - This will only be 100% when we have non-failing riscv_pmp_full_random_test
with a lot of iterations
4 - This one is easy and gets done with Marno's PR (#1695).
Region override: This one is only possible when we are in NA4 mode in PMP
Why is this the case? A smaller region inside a larger region should result in an override for instance. Overlapping regions using TOR could also exhibit overrides.
Also as far as I understand, we are not even supposed to see NAPOT with our current Ibex configuration of opentitan. The reason is the parameter of PMPGranularity is 0 in opentitan config (which only allows for TOR and NA4 according to the spec.)
PMPGranularity
of 0 gives you the maximum possible options, allowing NA4 regions along with NAPOT regions of sizes 8 bytes and up.
I think we definitely need to improve the PMP randomization here to hit the above two cases.
3 - This will only be 100% when we have non-failing riscv_pmp_full_random_test with a lot of iterations
From what I saw of the coverage we didn't get any locked regions at all. Have you observed locked regions anywhere in RISC-V DV?
Region override: This one is only possible when we are in NA4 mode in PMP
Why is this the case? A smaller region inside a larger region should result in an override for instance. Overlapping regions using TOR could also exhibit overrides.
As I explained in the next part, I mistakenly looked at the code and spec together to decide we can't do it but it's probably a code bug. In these constraints: https://github.com/lowRISC/ibex/blob/6518cb6db63ef56f428331cf534794336a5c7af9/vendor/google_riscv-dv/src/riscv_pmp_cfg.sv#L108-L124
we are effectively disabling a possibility of override in TOR
mode
Also as far as I understand, we are not even supposed to see NAPOT with our current Ibex configuration of opentitan. The reason is the parameter of PMPGranularity is 0 in opentitan config (which only allows for TOR and NA4 according to the spec.)
PMPGranularity
of 0 gives you the maximum possible options, allowing NA4 regions along with NAPOT regions of sizes 8 bytes and up.I think we definitely need to improve the PMP randomization here to hit the above two cases.
Okay I think I made the mistake of looking at riscv-dv source code and spec together, thinking it was bug-free :) In riscv-dv we have this constraint: https://github.com/lowRISC/ibex/blob/6518cb6db63ef56f428331cf534794336a5c7af9/vendor/google_riscv-dv/src/riscv_pmp_cfg.sv#L89-L94
that guarantees if the granularity is set to 0, we wouldn't get NAPOT
mode.
I definitely agree we need to improve PMP randomization.
3 - This will only be 100% when we have non-failing riscv_pmp_full_random_test with a lot of iterations
From what I saw of the coverage we didn't get any locked regions at all. Have you observed locked regions anywhere in RISC-V DV?
I tried to run a regression on top of Marno's changes in PR that touches PMP tests in #1695 . In that we were setting locked field (it's not getting set because in most of our current PMP tests locked
field of pmpcfgN
is not set -leaves it at default because of how riscv-dv handles things.)
Opened up https://github.com/google/riscv-dv/pull/882 for fixing up broken randomization constraints in riscv-dv that I mentioned in the previous comment.
First item is getting hit with the latest work around pmp randomization.
Second item will need further investigation.
Third item will be complete after ePMP integration is done. Already hitting every option with #1734 (- epmp related ones) in every region except region 0.
Fourth item will be complete after ePMP integration is done. It was getting hit with Marno's PR.
Second item can be solved by adding a "if we are enabling pmp_randomize
, do not owerride pmp_cfg0
" clause to the place where we do PMP related instruction generation in riscv-dv. However, when I do that -in almost half of the seeds- I encounter issue https://github.com/lowRISC/ibex/issues/1720.
Second item can be solved by adding a "if we are enabling
pmp_randomize
, do not owerridepmp_cfg0
" clause to the place where we do PMP related instruction generation in riscv-dv. However, when I do that -in almost half of the seeds- I encounter issue #1720.
As discussed yesterday, the problem is we do not want full random PMP when MML and/or MMWP are enabled. This is because it will never reach useful instructions unless the instructions before and after main are set as executable. As a solution, we discussed that in the case that PMP randomization is enabled and MML/MMWP are set, we do not hardcode region 0 (instruction before main) and region 1 (instructions after main). Instead, we select a random value i
between 1
and n
(where n
is the highest PMP region), we then set region i
to executable, TOR and with the address <main> + 0x2666
to. Finally we set region i-1
address to 0x80000000
.
I created a PR for the second point: https://github.com/google/riscv-dv/pull/885
spent 4
I adjusted https://github.com/google/riscv-dv/pull/885 to initialize the PMP config entries as disabled in the case that MML or MMWP are enabled.
spent 2
I spent quite some time debugging https://github.com/google/riscv-dv/pull/885 and solved a number of problems:
- Full random test now generates random addresses instead of offsets from main.
- Stop CSR write tests being generated when full random is enabled.
- When MML mode is enabled fix the region 0 CSR permission for the PMP config write test.
- Generate a PMP region to be read/write for the kernel stack.
- Open up an issue on an observed divergence: https://github.com/lowRISC/ibex/issues/1774
Since this task seems to be growing larger I created another issue for this: https://github.com/lowRISC/ibex/issues/1777
spent 6
I've run riscv_pmp_full_random_test
12 times with coverage enabled, looks like we are hitting the last point of this task which was NA4 and NAPOT matching modes and all privileges other than RWX for region 0
. Thanks @marnovandermaas for your work regarding PMP in general!