Add RV64 PMP with bug fixes and firmware test suite#50
Conversation
| self.requires("zlib/1.3.1") | ||
| self.requires("asmjit/cci.20240531") | ||
| if "WITH_LLVM" in os.environ: | ||
| if "WITH_LLVM" in os.environ and "WITH_SYSTEM_LLVM" not in os.environ: |
There was a problem hiding this comment.
Since WITH_SYSTEM_LLVM is a CMake cached variable there is no nee to check it in conan.
There was a problem hiding this comment.
The WITH_SYSTEM_LLVM check has been removed
There was a problem hiding this comment.
SInce vm_rv64gc.cpp is generated from a template, additions will be overwritten whenever the file is re-generated due to changes in the template or generation process. Pls. move the additions to a separate file.
There was a problem hiding this comment.
Done. I moved to src/vm/interp/vm_rv64gc_mp.cpp. vm_rv64gc.cpp is back to its generated state.
…to 15min - Add WITH_SYSTEM_LLVM cmake option; set LLVM_DIR directly and use BYPASS_PROVIDER to skip conan_provider's version-check quirk - Pin CI runner to ubuntu-24.04 for reproducibility - Remove redundant apt-get update (package lists are fresh in image) - Add workflow_dispatch trigger for manual CI runs
- ci.yml: split into two jobs. cpp-compliance (matrix C++17/20) handles pure build compliance and uploads the C++17 binary as an artifact. pmp-tests downloads that binary, compiles firmware with only gcc-riscv64-unknown-elf, and runs the test directly. Trigger paths extended to include **.S for firmware changes. - contrib/fw/pmp-csr-test/pmp_csr_test.S: firmware that writes 0xDEAD to pmpaddr0, reads it back, and exits 0 on match (j .) or non-zero via semihosting SYS_EXIT on mismatch.
vm_rv64gc_mp.cpp: rv64gc_mp_hart is a derived class that rewires the memory chain in its constructor body. The base constructor sets memory = default_mem via memories.append, but virtual dispatch during base construction bypasses any derived override of set_next. The constructor body runs after all members are initialized and explicitly sets pmp_obj downstream to default_mem and memory to pmp_obj.get_mem_if(). Kept in a dedicated file separate from the generated vm_rv64gc.cpp so the implementation survives template re-generation. register_cores.cpp: register rv64gc_mp for the SystemC backend. pmp.h: read_pmpcfg and write_pmpcfg both operated on pmpaddr[] instead of pmpcfg[], so pmpcfg[] was never written and any_active was never set. Fixed both to use pmpcfg[]. pmp_enforce_test.S: firmware test for entry 0 (pmpaddr0 + pmpcfg0 byte 0 = NA4|L). Passes when PMP denies a load to the protected address; fails via semihosting SYS_EXIT if the load succeeds or any CSR write traps.
pmp_shift_test.S: firmware test for entry 1 (pmpaddr1 + pmpcfg0 byte 1 = 0x9000). Passes when PMP denies a load to the protected address; fails via semihosting SYS_EXIT if load succeeds. pmp.h: the any_active loop in write_pmpcfg and the cfg extraction in pmp_check both used (i % cfg_reg_size) as the shift amount, treating the index as a bit offset rather than a byte index. Changed both to (i % cfg_reg_size) * 8 so that each PMP entry's config byte is correctly extracted from the packed register.
On RV64 pmpcfg0 is a 64-bit register holding 8 config bytes (entries 0-7). The hard-coded 32-bit mask 0x9f9f9f9f silently zeroed bytes 4-7 on every write, making PMP entries 4-7 permanently unconfigurable. Add pmp-upper-cfg-test firmware and CI step to guard the fix.
On RV64 only even-numbered pmpcfg CSRs exist (pmpcfg0 at 0x3A0, pmpcfg2 at 0x3A2). The constructor loop incremented by 1, registering pmpcfg0+pmpcfg1 instead of pmpcfg0+pmpcfg2, making entries 8-15 unreachable and pmpcfg1 incorrectly writable. Add pmpcfg_stride (cfg_reg_size/4: 1 for RV32, 2 for RV64), step the registration loop by it, and divide the CSR-address-to-array index by it in read_pmpcfg/write_pmpcfg. Add pmp-cfg2-test firmware and CI step to guard the fix.
The per-sector TOR match used `cur_addr + len - 1` for the lower-bound test, where `len` is the total access length. This made the check too loose: a sector lying entirely below `base` would pass as long as the full access reached into the TOR region, causing `all_match=true` for a partial-overlap access and incorrectly allowing it. Replace with `base <= cur_addr`: TOR boundaries are 4-byte aligned so the sector start alone determines whether the sector is inside the range. Add pmp-tor-test firmware and CI step to guard the fix.
cphurley82
changed the title
|
@cphurley82 While you are on it, could you extend the number of pmpcfg/pmpaddr CSRs to match Privileged Architecture Spec Version 20260120? It says:
|
@eyck , I'll see if I can knock this out real quick. The RV64 core I'm modeling only uses 8 PMP entries so I'll make it configurable. I'm curious about the history of PMP on dbt-rise-riscv, was it used much that you know of or might we be the first ones to try? |
|
We used it to verify our RTL implementation. But this is based on the RISC-V Privileged Architectures V20211203 and only for 32bit. |
2 participants
This PR enables Physical Memory Protection (PMP) on the RV64 target by introducing a new
rv64gc_mpISA variant, fixing bugs in the PMP implementation, adding a firmware test suite that validates each fix, and improving CI to reduce build time from ~1.5 hours to ~15 minutes.Firmware test suite (
contrib/fw/)Six assembly test programs exercise the fixed code paths:
pmp-csr-testpmpaddr0CSR accessible without illegal-instruction trappmp-enforce-testpmp-shift-testpmp-upper-cfg-testpmpcfg0preserved across writepmp-cfg2-testpmpcfg2accessible and readable on RV64 (entries 8–15)pmp-tor-testAll tests use a consistent semihosting exit pattern: an infinite
j .loop signals success (exit 0);SYS_EXITwith code 2 signals failure.CI changes (
.github/workflows/ci.yml)Replaced
ci-compile.ymlwith a two-job workflow:apt(eliminates Conan building LLVM, drops build time from ~1.5 h to ~15 min), uploads binaries as artifacts.