From 0872cc2857c0157c49200484340e3c20e400f6b6 Mon Sep 17 00:00:00 2001 From: "cphurley82@gmail.com" Date: Tue, 12 May 2026 09:11:58 -0700 Subject: [PATCH 1/7] build: use apt llvm-19-dev instead of Conan to cut CI time from 1.5h 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 --- .github/workflows/ci-compile.yml | 10 +++++----- CMakeLists.txt | 17 +++++++++++++++++ 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci-compile.yml b/.github/workflows/ci-compile.yml index 97c9fe6..179ac90 100644 --- a/.github/workflows/ci-compile.yml +++ b/.github/workflows/ci-compile.yml @@ -1,6 +1,7 @@ name: CI C++ Std compliance on: + workflow_dispatch: push: paths: - '**.hpp' @@ -22,7 +23,7 @@ on: jobs: build-and-test: - runs-on: ubuntu-latest + runs-on: ubuntu-24.04 strategy: matrix: cpp_std: [17, 20] @@ -40,16 +41,15 @@ jobs: - name: Install dependencies run: | - sudo apt-get update - sudo apt-get install -y g++ python3-pip cmake + sudo apt-get install -y g++ python3-pip cmake llvm-19-dev gcc-riscv64-unknown-elf pip3 install conan cmake --version - name: Configure - run: WITH_LLVM=1 cmake --preset Release -B build -DCMAKE_CXX_STANDARD=${{ matrix.cpp_std }} -DWITH_LLVM=ON + run: cmake --preset Release -B build -DCMAKE_CXX_STANDARD=${{ matrix.cpp_std }} -DWITH_LLVM=ON -DWITH_SYSTEM_LLVM=ON -DBUILD_TESTING=ON - name: Build - run: WITH_LLVM=1 cmake --build build -j + run: cmake --build build -j - name: Run smoke test run: ./build/riscv-sim -h diff --git a/CMakeLists.txt b/CMakeLists.txt index bd14ca5..5c5188a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -22,6 +22,23 @@ if(OPTIMIZE_FOR_NATIVE AND COMPILER_SUPPORTS_MARCH_NATIVE) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -march=native") endif() +option(WITH_SYSTEM_LLVM "Use system-installed LLVM (llvm-dev from apt) instead of Conan-built" OFF) +set(LLVM_MAJOR_VERSION "19" CACHE STRING "System LLVM major version when WITH_SYSTEM_LLVM=ON") +if(WITH_SYSTEM_LLVM) + set(LLVM_DIR "/usr/lib/llvm-${LLVM_MAJOR_VERSION}/lib/cmake/llvm" CACHE PATH "" FORCE) + find_package(LLVM REQUIRED CONFIG BYPASS_PROVIDER) + message(STATUS "Found system LLVM ${LLVM_VERSION}") + if(NOT TARGET llvm-core::llvm-core) + add_library(llvm-core::llvm-core INTERFACE IMPORTED GLOBAL) + target_include_directories(llvm-core::llvm-core INTERFACE ${LLVM_INCLUDE_DIRS}) + target_compile_definitions(llvm-core::llvm-core INTERFACE ${LLVM_DEFINITIONS}) + target_link_libraries(llvm-core::llvm-core INTERFACE LLVM-${LLVM_VERSION_MAJOR}) + endif() + if(NOT WITH_LLVM) + set(WITH_LLVM ON CACHE BOOL "Build LLVM backend" FORCE) + endif() +endif() + include(FetchContent) if(NOT TARGET dbt-rise-core) FetchContent_Declare( From a7d0b7183d3f88c471e45f95c33cba1089f5c345 Mon Sep 17 00:00:00 2001 From: "cphurley82@gmail.com" Date: Tue, 12 May 2026 11:36:47 -0700 Subject: [PATCH 2/7] test: add PMP CSR ctest and restructure CI - 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. --- .github/workflows/ci-compile.yml | 55 -------------- .github/workflows/ci.yml | 101 +++++++++++++++++++++++++ contrib/fw/pmp-csr-test/pmp_csr_test.S | 32 ++++++++ 3 files changed, 133 insertions(+), 55 deletions(-) delete mode 100644 .github/workflows/ci-compile.yml create mode 100644 .github/workflows/ci.yml create mode 100644 contrib/fw/pmp-csr-test/pmp_csr_test.S diff --git a/.github/workflows/ci-compile.yml b/.github/workflows/ci-compile.yml deleted file mode 100644 index 179ac90..0000000 --- a/.github/workflows/ci-compile.yml +++ /dev/null @@ -1,55 +0,0 @@ -name: CI C++ Std compliance - -on: - workflow_dispatch: - push: - paths: - - '**.hpp' - - '**.cpp' - - '**.h' - - '**.c' - - '**CMakeLists.txt' - - '.github/workflows/**' - - 'conanfile.py' - pull_request: - paths: - - '**.hpp' - - '**.cpp' - - '**.h' - - '**.c' - - '**CMakeLists.txt' - - '.github/workflows/**' - - 'conanfile.py' - -jobs: - build-and-test: - runs-on: ubuntu-24.04 - strategy: - matrix: - cpp_std: [17, 20] - steps: - - uses: actions/checkout@v4 - - - name: Update submodules - run: git submodule update --init --recursive - - - name: Cache Conan - uses: actions/cache@v4 - with: - path: ~/.conan2 - key: conan-${{ runner.os }}-unit-cpp${{ matrix.cpp_std }}-${{ hashFiles('conanfile.py') }} - - - name: Install dependencies - run: | - sudo apt-get install -y g++ python3-pip cmake llvm-19-dev gcc-riscv64-unknown-elf - pip3 install conan - cmake --version - - - name: Configure - run: cmake --preset Release -B build -DCMAKE_CXX_STANDARD=${{ matrix.cpp_std }} -DWITH_LLVM=ON -DWITH_SYSTEM_LLVM=ON -DBUILD_TESTING=ON - - - name: Build - run: cmake --build build -j - - - name: Run smoke test - run: ./build/riscv-sim -h diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..8cc74d1 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,101 @@ +name: CI + +on: + workflow_dispatch: + push: + paths: + - '**.hpp' + - '**.cpp' + - '**.h' + - '**.c' + - '**.S' + - '**CMakeLists.txt' + - '.github/workflows/**' + - 'conanfile.py' + pull_request: + paths: + - '**.hpp' + - '**.cpp' + - '**.h' + - '**.c' + - '**.S' + - '**CMakeLists.txt' + - '.github/workflows/**' + - 'conanfile.py' + +jobs: + cpp-compliance: + name: C++ Std Compliance (C++${{ matrix.cpp_std }}) + runs-on: ubuntu-24.04 + strategy: + matrix: + cpp_std: [17, 20] + steps: + - uses: actions/checkout@v4 + + - name: Update submodules + run: git submodule update --init --recursive + + - name: Cache Conan + uses: actions/cache@v4 + with: + path: ~/.conan2 + key: conan-${{ runner.os }}-unit-cpp${{ matrix.cpp_std }}-${{ hashFiles('conanfile.py') }} + + - name: Install dependencies + run: | + sudo apt-get install -y g++ python3-pip cmake llvm-19-dev + pip3 install conan + cmake --version + + - name: Configure + run: cmake --preset Release -B build -DCMAKE_CXX_STANDARD=${{ matrix.cpp_std }} -DWITH_LLVM=ON -DWITH_SYSTEM_LLVM=ON + + - name: Build + run: cmake --build build -j + + - name: Smoke test + run: ./build/riscv-sim -h + + # NOTE: .so filename is tied to VERSION in top-level CMakeLists.txt - update both together + - name: Upload binary + if: matrix.cpp_std == 20 + uses: actions/upload-artifact@v4 + with: + name: riscv-sim + path: | + build/riscv-sim + build/libdbt-rise-riscv.so.2.1.0 + + pmp-tests: + name: PMP Functional Tests + runs-on: ubuntu-24.04 + needs: cpp-compliance + steps: + - uses: actions/checkout@v4 + + - name: Install RISC-V toolchain + run: sudo apt-get install -y gcc-riscv64-unknown-elf + + - name: Download riscv-sim binary + uses: actions/download-artifact@v4 + with: + name: riscv-sim + + - name: Make binary executable + run: chmod +x riscv-sim + + - name: Build PMP CSR test firmware + run: | + riscv64-unknown-elf-gcc -nostdlib -march=rv64gc -mabi=lp64 \ + -Wl,-Ttext=0x10000,--no-dynamic-linker \ + -o pmp_csr_test \ + contrib/fw/pmp-csr-test/pmp_csr_test.S + + - name: rv64gc_m CSR test - interp (no PMP, expect exit 2) + run: | + LD_LIBRARY_PATH=. ./riscv-sim -f pmp_csr_test --isa rv64gc_m --backend interp || rc=$? + [ "${rc:-0}" -eq 2 ] + + - name: rv64gc_mp CSR test - interp (with PMP) + run: LD_LIBRARY_PATH=. ./riscv-sim -f pmp_csr_test --isa rv64gc_mp --backend interp diff --git a/contrib/fw/pmp-csr-test/pmp_csr_test.S b/contrib/fw/pmp-csr-test/pmp_csr_test.S new file mode 100644 index 0000000..e363642 --- /dev/null +++ b/contrib/fw/pmp-csr-test/pmp_csr_test.S @@ -0,0 +1,32 @@ +// Verify pmpaddr0 is accessible as a CSR (no illegal-instruction trap on read/write). +// Pass: j . detected as JUMP_TO_SELF by ISS, exits 0. +// Fail: semihosting SYS_EXIT sequence, ISS exits non-zero. + +.section .text +.globl _start +_start: + // Install fail handler: any unexpected trap before the ecall path means the CSR + // is not available or behaves incorrectly + la t0, fail + csrw mtvec, t0 + + // Write a distinctive value to pmpaddr0 and verify the readback matches + li t0, 0xDEAD + csrw pmpaddr0, t0 + csrr t1, pmpaddr0 + bne t0, t1, fail + + j . // pass: ISS detects j . and exits 0 + +fail: + // Semihosting SYS_EXIT: the ISS checks for slli+ebreak+srai and intercepts + // before dispatching to mtvec. Must use 4-byte ebreak (not c.ebreak) so the + // check lands at the correct offsets (-4 and +4 from the ebreak address). + li a0, 0x18 // SYS_EXIT + .option push + .option norvc // force 4-byte ebreak (0x00100073), not 2-byte c.ebreak + slli zero, zero, 0x1f + ebreak + srai zero, zero, 7 + .option pop + j . // fallback if semihosting is not configured From b9a241674a43a62f61bd489da599a05ddf7d14ce Mon Sep 17 00:00:00 2001 From: "cphurley82@gmail.com" Date: Tue, 12 May 2026 15:12:58 -0700 Subject: [PATCH 3/7] feat(pmp): register rv64gc_mp:interp with PMP enforcement 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. --- .github/workflows/ci.yml | 10 ++++ CMakeLists.txt | 1 + .../fw/pmp-enforce-test/pmp_enforce_test.S | 52 +++++++++++++++++++ src/iss/mem/pmp.h | 4 +- src/sysc/register_cores.cpp | 9 +++- src/vm/interp/vm_rv64gc_mp.cpp | 42 +++++++++++++++ 6 files changed, 115 insertions(+), 3 deletions(-) create mode 100644 contrib/fw/pmp-enforce-test/pmp_enforce_test.S create mode 100644 src/vm/interp/vm_rv64gc_mp.cpp diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8cc74d1..b65b27f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -99,3 +99,13 @@ jobs: - name: rv64gc_mp CSR test - interp (with PMP) run: LD_LIBRARY_PATH=. ./riscv-sim -f pmp_csr_test --isa rv64gc_mp --backend interp + + - name: Build PMP enforcement test firmware + run: | + riscv64-unknown-elf-gcc -nostdlib -march=rv64gc -mabi=lp64 \ + -Wl,-Ttext=0x10000,--no-dynamic-linker \ + -o pmp_enforce_test \ + contrib/fw/pmp-enforce-test/pmp_enforce_test.S + + - name: rv64gc_mp enforcement test - interp + run: LD_LIBRARY_PATH=. ./riscv-sim -f pmp_enforce_test --isa rv64gc_mp --backend interp diff --git a/CMakeLists.txt b/CMakeLists.txt index 5c5188a..ff081ae 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -87,6 +87,7 @@ set(LIB_SOURCES src/vm/interp/vm_rv64i.cpp src/vm/interp/vm_rv64imac.cpp src/vm/interp/vm_rv64gc.cpp + src/vm/interp/vm_rv64gc_mp.cpp src/vm/interp/vm_rv64gcv.cpp src/iss/debugger/csr_names.cpp src/iss/semihosting/semihosting.cpp diff --git a/contrib/fw/pmp-enforce-test/pmp_enforce_test.S b/contrib/fw/pmp-enforce-test/pmp_enforce_test.S new file mode 100644 index 0000000..549f133 --- /dev/null +++ b/contrib/fw/pmp-enforce-test/pmp_enforce_test.S @@ -0,0 +1,52 @@ +// PMP enforcement test - entry 0 (pmpaddr0 + pmpcfg0 byte 0). +// Tests that pmpcfg naming bugs 1+2 in pmp.h are fixed: +// Bug 1: read_pmpcfg reads pmpaddr[] instead of pmpcfg[] +// Bug 2: write_pmpcfg writes pmpaddr[] instead of pmpcfg[] +// +// Pass: PMP denies load -> load fault fires -> j . (exit 0) +// Fail: any CSR trap, OR load succeeds without fault -> semihosting SYS_EXIT (exit 2) + +.macro semihosting_fail + li a0, 0x18 + .option push + .option norvc + slli zero, zero, 0x1f + ebreak + srai zero, zero, 7 + .option pop + j . +.endm + +.section .text +.globl _start +_start: + // phase1_trap: CSR write trapped - PMP not available + la t0, phase1_trap + csrw mtvec, t0 + + // Configure pmpaddr0 = test_word >> 2 (NA4: covers exactly 4 bytes at test_word) + la t5, test_word + srli t1, t5, 2 + csrw pmpaddr0, t1 + + // pmpcfg0 byte 0 = 0x90 = NA4 (0x10) | L (0x80), no R/W/X permissions + li t1, 0x90 + csrw pmpcfg0, t1 + + // phase2_trap: load fault fired - PMP enforcement active = PASS + la t0, phase2_trap + csrw mtvec, t0 + + lw t1, 0(t5) // should fault: PMP denies read + + semihosting_fail // no fault: enforcement did not fire -> FAIL + +phase1_trap: + semihosting_fail // CSR trapped: PMP unavailable -> FAIL + +phase2_trap: + j . // load denied as expected -> PASS + + .align 2 +test_word: + .word 0xCAFECAFE diff --git a/src/iss/mem/pmp.h b/src/iss/mem/pmp.h index b5d75a9..83526e4 100644 --- a/src/iss/mem/pmp.h +++ b/src/iss/mem/pmp.h @@ -124,14 +124,14 @@ template struct pmp : public memory_elem { iss::status read_pmpcfg(unsigned addr, reg_t& val) { if(addr >= arch::pmpcfg0 && addr < (arch::pmpcfg0 + 16 / sizeof(reg_t))) { - val = pmpaddr[addr - arch::pmpcfg0]; + val = pmpcfg[addr - arch::pmpcfg0]; return iss::Ok; } return iss::Err; } iss::status write_pmpcfg(unsigned addr, reg_t val) { if(addr >= arch::pmpcfg0 && addr < (arch::pmpcfg0 + 16 / sizeof(reg_t))) { - pmpaddr[addr - arch::pmpcfg0] = val & 0x9f9f9f9f; + pmpcfg[addr - arch::pmpcfg0] = val & 0x9f9f9f9f; any_active = false; for(size_t i = 0; i < 16; i++) { auto cfg = pmpcfg[i / cfg_reg_size] >> (i % cfg_reg_size); diff --git a/src/sysc/register_cores.cpp b/src/sysc/register_cores.cpp index 928da11..236d176 100644 --- a/src/sysc/register_cores.cpp +++ b/src/sysc/register_cores.cpp @@ -50,7 +50,7 @@ namespace iss { namespace interp { using namespace sysc; -__attribute__((used)) volatile std::array riscv_init = { +__attribute__((used)) volatile std::array riscv_init = { iss_factory::instance().register_creator("rv32i_m:interp", [](unsigned gdb_port, sysc::riscv::core_complex_if* cc) -> iss_factory::base_t { auto* cpu = new core2sc_adapter>(cc); @@ -125,6 +125,13 @@ __attribute__((used)) volatile std::array riscv_init = { auto* cpu = new core2sc_adapter>(cc); return {sysc::core_ptr{cpu}, vm_ptr{create(static_cast(cpu), gdb_port)}}; }), + iss_factory::instance().register_creator("rv64gc_mp:interp", + [](unsigned gdb_port, sysc::riscv::core_complex_if* cc) -> iss_factory::base_t { + auto* cpu = new core2sc_adapter>(cc); + cpu->memories.insert_before_last( + std::make_unique>(cpu->get_priv_if())); + return {sysc::core_ptr{cpu}, vm_ptr{create(static_cast(cpu), gdb_port)}}; + }), iss_factory::instance().register_creator("rv64gc_mu:interp", [](unsigned gdb_port, sysc::riscv::core_complex_if* cc) -> iss_factory::base_t { auto* cpu = new core2sc_adapter>(cc); diff --git a/src/vm/interp/vm_rv64gc_mp.cpp b/src/vm/interp/vm_rv64gc_mp.cpp new file mode 100644 index 0000000..badb673 --- /dev/null +++ b/src/vm/interp/vm_rv64gc_mp.cpp @@ -0,0 +1,42 @@ +// rv64gc_mp:interp — rv64gc with PMP enforcement. +// Kept separate from the generated vm_rv64gc.cpp to survive template re-generation. +#include +#include +#include +#include +#include +#include +#include + +namespace iss { + +// rv64gc_mp_hart inserts pmp between the hart and default_mem. +// The base constructor calls set_next(default_mem) via memories.append(), but +// virtual dispatch during base construction routes to the base implementation, +// so the rewiring must be done explicitly in the derived constructor body once +// all members (including pmp_obj) are fully initialized. +struct rv64gc_mp_hart : public arch::riscv_hart_m_p { + mem::pmp pmp_obj{this->get_priv_if()}; + + rv64gc_mp_hart() { + pmp_obj.set_next(this->default_mem.get_mem_if()); + memory = pmp_obj.get_mem_if(); + } +}; + +namespace { + +volatile std::array rv64gc_mp_dummy = { + core_factory::instance().register_creator("rv64gc_mp:interp", + [](unsigned port, void* init_data) -> std::tuple { + auto* cpu = new rv64gc_mp_hart(); + if (init_data) { + auto* cb = reinterpret_cast::reg_t>*>(init_data); + cpu->set_semihosting_callback(*cb); + } + return {cpu_ptr{cpu}, vm_ptr{iss::interp::create(cpu, port, false)}}; + }) +}; + +} // namespace +} // namespace iss From 0771ec0d8c48ed6544b05d30c3ec8a24c3310d78 Mon Sep 17 00:00:00 2001 From: "cphurley82@gmail.com" Date: Tue, 12 May 2026 17:17:41 -0700 Subject: [PATCH 4/7] fix: pmpcfg byte-shift in pmp.h 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. --- .github/workflows/ci.yml | 10 ++++ contrib/fw/pmp-shift-test/pmp_shift_test.S | 54 ++++++++++++++++++++++ src/iss/mem/pmp.h | 4 +- 3 files changed, 66 insertions(+), 2 deletions(-) create mode 100644 contrib/fw/pmp-shift-test/pmp_shift_test.S diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b65b27f..2c929ca 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -109,3 +109,13 @@ jobs: - name: rv64gc_mp enforcement test - interp run: LD_LIBRARY_PATH=. ./riscv-sim -f pmp_enforce_test --isa rv64gc_mp --backend interp + + - name: Build PMP shift test firmware + run: | + riscv64-unknown-elf-gcc -nostdlib -march=rv64gc -mabi=lp64 \ + -Wl,-Ttext=0x10000,--no-dynamic-linker \ + -o pmp_shift_test \ + contrib/fw/pmp-shift-test/pmp_shift_test.S + + - name: rv64gc_mp shift test - interp + run: LD_LIBRARY_PATH=. ./riscv-sim -f pmp_shift_test --isa rv64gc_mp --backend interp diff --git a/contrib/fw/pmp-shift-test/pmp_shift_test.S b/contrib/fw/pmp-shift-test/pmp_shift_test.S new file mode 100644 index 0000000..bbb15bb --- /dev/null +++ b/contrib/fw/pmp-shift-test/pmp_shift_test.S @@ -0,0 +1,54 @@ +// PMP shift test - entry 1 (pmpaddr1 + pmpcfg0 byte 1). +// Verifies that pmpcfg byte extraction uses the correct shift: (i % cfg_reg_size) * 8. +// +// For entry 1, byte 1 of pmpcfg0 = 0x90 (NA4|L), written as 0x9000 to pmpcfg0. +// A wrong shift of 1 bit instead of 8 bits reads 0x4800, which has no PMP_A bits +// set, so any_active stays false and enforcement is skipped. +// +// Pass: PMP denies load -> load fault fires -> j . (exit 0) +// Fail: any CSR trap, OR load succeeds without fault -> semihosting SYS_EXIT (exit 2) + +.macro semihosting_fail + li a0, 0x18 + .option push + .option norvc + slli zero, zero, 0x1f + ebreak + srai zero, zero, 7 + .option pop + j . +.endm + +.section .text +.globl _start +_start: + // phase1_trap: CSR write trapped - PMP not available + la t0, phase1_trap + csrw mtvec, t0 + + // Configure pmpaddr1 = test_word2 >> 2 (NA4) + la t5, test_word2 + srli t1, t5, 2 + csrw pmpaddr1, t1 + + // pmpcfg0 byte 1 = 0x90 -> write 0x9000 to pmpcfg0 + li t1, 0x9000 + csrw pmpcfg0, t1 + + // phase2_trap: load fault fired - PMP enforcement active = PASS + la t0, phase2_trap + csrw mtvec, t0 + + lw t1, 0(t5) // should fault: PMP denies read + + semihosting_fail // no fault: enforcement did not fire -> FAIL + +phase1_trap: + semihosting_fail // CSR trapped: PMP unavailable -> FAIL + +phase2_trap: + j . // load denied as expected -> PASS + + .align 2 +test_word2: + .word 0xCAFECAFE diff --git a/src/iss/mem/pmp.h b/src/iss/mem/pmp.h index 83526e4..30291ed 100644 --- a/src/iss/mem/pmp.h +++ b/src/iss/mem/pmp.h @@ -134,7 +134,7 @@ template struct pmp : public memory_elem { pmpcfg[addr - arch::pmpcfg0] = val & 0x9f9f9f9f; any_active = false; for(size_t i = 0; i < 16; i++) { - auto cfg = pmpcfg[i / cfg_reg_size] >> (i % cfg_reg_size); + auto cfg = pmpcfg[i / cfg_reg_size] >> ((i % cfg_reg_size) * 8); any_active |= cfg & PMP_A; } return iss::Ok; @@ -156,7 +156,7 @@ template bool pmp::pmp_check(access_type type, uint64_t ad reg_t base = 0; for(size_t i = 0; i < 16; i++) { reg_t tor = pmpaddr[i] << PMP_SHIFT; - reg_t cfg = pmpcfg[i / cfg_reg_size] >> (i % cfg_reg_size); + reg_t cfg = pmpcfg[i / cfg_reg_size] >> ((i % cfg_reg_size) * 8); if(cfg & PMP_A) { auto pmp_a = (cfg & PMP_A) >> 3; auto is_tor = pmp_a == PMP_TOR; From e288d80e938970a265c9d884529874c045275bc3 Mon Sep 17 00:00:00 2001 From: "cphurley82@gmail.com" Date: Wed, 13 May 2026 10:58:38 -0700 Subject: [PATCH 5/7] fix(pmp): extend write_pmpcfg valid-bit mask to reg_t width for RV64 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. --- .github/workflows/ci.yml | 10 +++++++ .../pmp-upper-cfg-test/pmp_upper_cfg_test.S | 27 +++++++++++++++++++ src/iss/mem/pmp.h | 3 ++- 3 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 contrib/fw/pmp-upper-cfg-test/pmp_upper_cfg_test.S diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2c929ca..5bdc8a4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -119,3 +119,13 @@ jobs: - name: rv64gc_mp shift test - interp run: LD_LIBRARY_PATH=. ./riscv-sim -f pmp_shift_test --isa rv64gc_mp --backend interp + + - name: Build PMP upper-cfg test firmware + run: | + riscv64-unknown-elf-gcc -nostdlib -march=rv64gc -mabi=lp64 \ + -Wl,-Ttext=0x10000,--no-dynamic-linker \ + -o pmp_upper_cfg_test \ + contrib/fw/pmp-upper-cfg-test/pmp_upper_cfg_test.S + + - name: rv64gc_mp upper-cfg test - interp + run: LD_LIBRARY_PATH=. ./riscv-sim -f pmp_upper_cfg_test --isa rv64gc_mp --backend interp diff --git a/contrib/fw/pmp-upper-cfg-test/pmp_upper_cfg_test.S b/contrib/fw/pmp-upper-cfg-test/pmp_upper_cfg_test.S new file mode 100644 index 0000000..7c7e524 --- /dev/null +++ b/contrib/fw/pmp-upper-cfg-test/pmp_upper_cfg_test.S @@ -0,0 +1,27 @@ +// Verify that RV64 pmpcfg0 upper 32 bits (cfg bytes 4-7) are preserved on write. +// On RV64 a pmpcfg register is 64 bits and holds 8 config bytes (entries 0-7). +// A write mask narrower than reg_t silently zeroes bytes 4-7. +// +// Pass: readback == written value -> j . (exit 0) +// Fail: readback != written value -> semihosting SYS_EXIT (exit 2) + +.section .text +.globl _start +_start: + // 0x18 per byte = NAPOT mode, no RWX, no lock; survives 0x9f byte mask + li t0, 0x1818181818181818 + csrw pmpcfg0, t0 + csrr t1, pmpcfg0 + bne t0, t1, fail + + j . // pass: ISS detects j . and exits 0 + +fail: + li a0, 0x18 + .option push + .option norvc + slli zero, zero, 0x1f + ebreak + srai zero, zero, 7 + .option pop + j . diff --git a/src/iss/mem/pmp.h b/src/iss/mem/pmp.h index 30291ed..8c0bd81 100644 --- a/src/iss/mem/pmp.h +++ b/src/iss/mem/pmp.h @@ -48,6 +48,7 @@ template struct pmp : public memory_elem { using this_class = pmp; using reg_t = typename PLAT::reg_t; static constexpr auto cfg_reg_size = sizeof(reg_t); + static constexpr reg_t cfg_valid_mask = sizeof(reg_t) == 8 ? reg_t(0x9f9f9f9f9f9f9f9fULL) : reg_t(0x9f9f9f9fU); static constexpr auto PMP_SHIFT = 2U; static constexpr auto PMP_R = 0x1U; static constexpr auto PMP_W = 0x2U; @@ -131,7 +132,7 @@ template struct pmp : public memory_elem { } iss::status write_pmpcfg(unsigned addr, reg_t val) { if(addr >= arch::pmpcfg0 && addr < (arch::pmpcfg0 + 16 / sizeof(reg_t))) { - pmpcfg[addr - arch::pmpcfg0] = val & 0x9f9f9f9f; + pmpcfg[addr - arch::pmpcfg0] = val & cfg_valid_mask; any_active = false; for(size_t i = 0; i < 16; i++) { auto cfg = pmpcfg[i / cfg_reg_size] >> ((i % cfg_reg_size) * 8); From da67215bae349a7a61895ca397eacdcbbb61e6ec Mon Sep 17 00:00:00 2001 From: "cphurley82@gmail.com" Date: Wed, 13 May 2026 11:17:00 -0700 Subject: [PATCH 6/7] fix(pmp): register pmpcfg0+pmpcfg2 on RV64, fix CSR index arithmetic 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. --- .github/workflows/ci.yml | 10 ++++++++ contrib/fw/pmp-cfg2-test/pmp_cfg2_test.S | 29 ++++++++++++++++++++++++ src/iss/mem/pmp.h | 11 +++++---- 3 files changed, 45 insertions(+), 5 deletions(-) create mode 100644 contrib/fw/pmp-cfg2-test/pmp_cfg2_test.S diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5bdc8a4..a0127ed 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -129,3 +129,13 @@ jobs: - name: rv64gc_mp upper-cfg test - interp run: LD_LIBRARY_PATH=. ./riscv-sim -f pmp_upper_cfg_test --isa rv64gc_mp --backend interp + + - name: Build PMP cfg2 test firmware + run: | + riscv64-unknown-elf-gcc -nostdlib -march=rv64gc -mabi=lp64 \ + -Wl,-Ttext=0x10000,--no-dynamic-linker \ + -o pmp_cfg2_test \ + contrib/fw/pmp-cfg2-test/pmp_cfg2_test.S + + - name: rv64gc_mp cfg2 test - interp + run: LD_LIBRARY_PATH=. ./riscv-sim -f pmp_cfg2_test --isa rv64gc_mp --backend interp diff --git a/contrib/fw/pmp-cfg2-test/pmp_cfg2_test.S b/contrib/fw/pmp-cfg2-test/pmp_cfg2_test.S new file mode 100644 index 0000000..7a125a0 --- /dev/null +++ b/contrib/fw/pmp-cfg2-test/pmp_cfg2_test.S @@ -0,0 +1,29 @@ +// Verify pmpcfg2 (entries 8-15) is accessible as a CSR on RV64. +// On RV64 valid pmpcfg CSRs are pmpcfg0 (entries 0-7) and pmpcfg2 (entries 8-15). +// +// Pass: pmpcfg2 readback == written value -> j . (exit 0) +// Fail: pmpcfg2 write traps OR readback wrong -> semihosting SYS_EXIT (exit 2) + +.section .text +.globl _start +_start: + la t0, fail + csrw mtvec, t0 + + // 0x18 per byte = NAPOT mode, no RWX, no lock; survives 0x9f byte mask + li t0, 0x1818181818181818 + csrw pmpcfg2, t0 + csrr t1, pmpcfg2 + bne t0, t1, fail + + j . // pass: ISS detects j . and exits 0 + +fail: + li a0, 0x18 + .option push + .option norvc + slli zero, zero, 0x1f + ebreak + srai zero, zero, 7 + .option pop + j . diff --git a/src/iss/mem/pmp.h b/src/iss/mem/pmp.h index 8c0bd81..d593601 100644 --- a/src/iss/mem/pmp.h +++ b/src/iss/mem/pmp.h @@ -49,6 +49,7 @@ template struct pmp : public memory_elem { using reg_t = typename PLAT::reg_t; static constexpr auto cfg_reg_size = sizeof(reg_t); static constexpr reg_t cfg_valid_mask = sizeof(reg_t) == 8 ? reg_t(0x9f9f9f9f9f9f9f9fULL) : reg_t(0x9f9f9f9fU); + static constexpr size_t pmpcfg_stride = cfg_reg_size / 4; // 1 for RV32, 2 for RV64 static constexpr auto PMP_SHIFT = 2U; static constexpr auto PMP_R = 0x1U; static constexpr auto PMP_W = 0x2U; @@ -65,7 +66,7 @@ template struct pmp : public memory_elem { hart_if.csr_rd_cb[i] = MK_CSR_RD_CB(read_pmpaddr); hart_if.csr_wr_cb[i] = MK_CSR_WR_CB(write_pmpaddr); } - for(size_t i = arch::pmpcfg0; i < arch::pmpcfg0 + 16 / sizeof(reg_t); ++i) { + for(size_t i = arch::pmpcfg0; i < arch::pmpcfg0 + 4; i += pmpcfg_stride) { hart_if.csr_rd_cb[i] = MK_CSR_RD_CB(read_pmpcfg); hart_if.csr_wr_cb[i] = MK_CSR_WR_CB(write_pmpcfg); } @@ -124,15 +125,15 @@ template struct pmp : public memory_elem { } iss::status read_pmpcfg(unsigned addr, reg_t& val) { - if(addr >= arch::pmpcfg0 && addr < (arch::pmpcfg0 + 16 / sizeof(reg_t))) { - val = pmpcfg[addr - arch::pmpcfg0]; + if(addr >= arch::pmpcfg0 && addr < arch::pmpcfg0 + 4) { + val = pmpcfg[(addr - arch::pmpcfg0) / pmpcfg_stride]; return iss::Ok; } return iss::Err; } iss::status write_pmpcfg(unsigned addr, reg_t val) { - if(addr >= arch::pmpcfg0 && addr < (arch::pmpcfg0 + 16 / sizeof(reg_t))) { - pmpcfg[addr - arch::pmpcfg0] = val & cfg_valid_mask; + if(addr >= arch::pmpcfg0 && addr < arch::pmpcfg0 + 4) { + pmpcfg[(addr - arch::pmpcfg0) / pmpcfg_stride] = val & cfg_valid_mask; any_active = false; for(size_t i = 0; i < 16; i++) { auto cfg = pmpcfg[i / cfg_reg_size] >> ((i % cfg_reg_size) * 8); From 203d3b4d1e4d61eb253082a034461f88ddaf4091 Mon Sep 17 00:00:00 2001 From: "cphurley82@gmail.com" Date: Wed, 13 May 2026 13:36:17 -0700 Subject: [PATCH 7/7] fix(pmp): use sector start for TOR lower-bound check in pmp_check 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. --- .github/workflows/ci.yml | 10 +++++ contrib/fw/pmp-tor-test/pmp_tor_test.S | 56 ++++++++++++++++++++++++++ src/iss/mem/pmp.h | 2 +- 3 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 contrib/fw/pmp-tor-test/pmp_tor_test.S diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a0127ed..58c0ee0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -139,3 +139,13 @@ jobs: - name: rv64gc_mp cfg2 test - interp run: LD_LIBRARY_PATH=. ./riscv-sim -f pmp_cfg2_test --isa rv64gc_mp --backend interp + + - name: Build PMP TOR test firmware + run: | + riscv64-unknown-elf-gcc -nostdlib -march=rv64gc -mabi=lp64 \ + -Wl,-Ttext=0x10000,--no-dynamic-linker \ + -o pmp_tor_test \ + contrib/fw/pmp-tor-test/pmp_tor_test.S + + - name: rv64gc_mp TOR test - interp + run: LD_LIBRARY_PATH=. ./riscv-sim -f pmp_tor_test --isa rv64gc_mp --backend interp diff --git a/contrib/fw/pmp-tor-test/pmp_tor_test.S b/contrib/fw/pmp-tor-test/pmp_tor_test.S new file mode 100644 index 0000000..b9001a9 --- /dev/null +++ b/contrib/fw/pmp-tor-test/pmp_tor_test.S @@ -0,0 +1,56 @@ +// Verify that a partial-overlap load (sectors spanning a TOR region boundary) is denied. +// pmpaddr0 sets the TOR base; pmpaddr1/pmpcfg0[1] set a locked read-only TOR region. +// An 8-byte load at test_region covers two 4-byte sectors: +// sector 0: test_region+0 (below TOR base -> outside range) +// sector 1: test_region+4 (inside TOR range) +// Correct behaviour: partial overlap -> PMP denies -> load fault fires. +// +// Pass: load fault fires -> j . (exit 0) +// Fail: load succeeds without fault -> semihosting SYS_EXIT (exit 2) + +.section .text +.globl _start +_start: + la t0, fail + csrw mtvec, t0 + + // pmpaddr0 = (test_region+4) >> 2 (TOR base for entry 1) + la t5, test_region + addi t1, t5, 4 + srli t1, t1, 2 + csrw pmpaddr0, t1 + + // pmpaddr1 = (test_region+12) >> 2 (TOR top for entry 1) + addi t1, t5, 12 + srli t1, t1, 2 + csrw pmpaddr1, t1 + + // pmpcfg0: byte 1 = 0x89 (TOR|L|R) -> locked read-only TOR [test_region+4, test_region+12) + li t1, 0x8900 + csrw pmpcfg0, t1 + + la t0, pmp_fault + csrw mtvec, t0 + + // 8-byte load at test_region: sector 0 is outside range, sector 1 is inside. + // PMP must deny (partial overlap). + ld t2, 0(t5) + + // No fault fired -> bug present +fail: + li a0, 0x18 + .option push + .option norvc + slli zero, zero, 0x1f + ebreak + srai zero, zero, 7 + .option pop + j . + +pmp_fault: + j . // load denied as expected -> PASS + + .align 3 // 8-byte align for ld +test_region: + .quad 0x1234567890ABCDEF + .quad 0xC001CAFEDEADBEEF diff --git a/src/iss/mem/pmp.h b/src/iss/mem/pmp.h index d593601..19ff488 100644 --- a/src/iss/mem/pmp.h +++ b/src/iss/mem/pmp.h @@ -171,7 +171,7 @@ template bool pmp::pmp_check(access_type type, uint64_t ad for(reg_t offset = 0; offset < len; offset += 1 << PMP_SHIFT) { reg_t cur_addr = addr + offset; auto napot_match = ((cur_addr ^ tor) & mask) == 0; - auto tor_match = base <= (cur_addr + len - 1) && cur_addr < tor; + auto tor_match = base <= cur_addr && cur_addr < tor; auto match = is_tor ? tor_match : napot_match; any_match |= match; all_match &= match;