From cbab282d66b859042a230979959d6cf1afc87bd0 Mon Sep 17 00:00:00 2001 From: "cphurley82@gmail.com" Date: Thu, 14 May 2026 14:33:43 -0700 Subject: [PATCH] feat(pmp): extend to 64 entries with configurable NUM_ENTRIES Add a `size_t NUM_ENTRIES = 64` non-type template parameter to `pmp` so the number of PMP entries is set at instantiation time with no runtime overhead. The default of 64 matches the RISC-V Privileged Architecture Spec v20260120. Passing a smaller value (e.g. 8) models a core that implements fewer hardware PMP entries. New rv64gc_mp_8 ISA variant (8 entries): - vm_rv64gc_mp.cpp: add rv64gc_mp_8_hart struct and register "rv64gc_mp_8:interp" with core_factory (standalone riscv-sim) - register_cores.cpp: register "rv64gc_mp_8:interp" with iss_factory (SystemC VP / core_complex) Firmware tests (contrib/fw/): - pmp-64entry-addr-test: write/read pmpaddr32 (CSR 0x3D0); confirms entries 16-63 are accessible - pmp-64entry-cfg-test: write/read/clear pmpcfg4 (CSR 0x3A4, entries 32-39 on RV64); confirms persistence - pmp-8entry-guard-test: configure entry 0 deny region and expect load-access fault using rv64gc_mp_8; confirms enforcement still works with the 8-entry window --- .github/workflows/ci.yml | 30 +++++++++++ .../pmp_64entry_addr_test.S | 26 ++++++++++ .../pmp_64entry_cfg_test.S | 34 +++++++++++++ .../pmp_8entry_guard_test.S | 50 +++++++++++++++++++ src/iss/mem/pmp.h | 29 ++++++----- src/sysc/register_cores.cpp | 9 +++- src/vm/interp/vm_rv64gc_mp.cpp | 21 +++++++- 7 files changed, 184 insertions(+), 15 deletions(-) create mode 100644 contrib/fw/pmp-64entry-addr-test/pmp_64entry_addr_test.S create mode 100644 contrib/fw/pmp-64entry-cfg-test/pmp_64entry_cfg_test.S create mode 100644 contrib/fw/pmp-8entry-guard-test/pmp_8entry_guard_test.S diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 58c0ee0..35022c9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -149,3 +149,33 @@ jobs: - name: rv64gc_mp TOR test - interp run: LD_LIBRARY_PATH=. ./riscv-sim -f pmp_tor_test --isa rv64gc_mp --backend interp + + - name: Build PMP 64-entry pmpaddr test firmware + run: | + riscv64-unknown-elf-gcc -nostdlib -march=rv64gc -mabi=lp64 \ + -Wl,-Ttext=0x10000,--no-dynamic-linker \ + -o pmp_64entry_addr_test \ + contrib/fw/pmp-64entry-addr-test/pmp_64entry_addr_test.S + + - name: rv64gc_mp 64-entry pmpaddr test - interp + run: LD_LIBRARY_PATH=. ./riscv-sim -f pmp_64entry_addr_test --isa rv64gc_mp --backend interp + + - name: Build PMP 64-entry pmpcfg test firmware + run: | + riscv64-unknown-elf-gcc -nostdlib -march=rv64gc -mabi=lp64 \ + -Wl,-Ttext=0x10000,--no-dynamic-linker \ + -o pmp_64entry_cfg_test \ + contrib/fw/pmp-64entry-cfg-test/pmp_64entry_cfg_test.S + + - name: rv64gc_mp 64-entry pmpcfg test - interp + run: LD_LIBRARY_PATH=. ./riscv-sim -f pmp_64entry_cfg_test --isa rv64gc_mp --backend interp + + - name: Build PMP 8-entry guard test firmware (VP/S5 model) + run: | + riscv64-unknown-elf-gcc -nostdlib -march=rv64gc -mabi=lp64 \ + -Wl,-Ttext=0x10000,--no-dynamic-linker \ + -o pmp_8entry_guard_test \ + contrib/fw/pmp-8entry-guard-test/pmp_8entry_guard_test.S + + - name: rv64gc_mp_8 8-entry enforcement test - interp (VP/S5 model) + run: LD_LIBRARY_PATH=. ./riscv-sim -f pmp_8entry_guard_test --isa rv64gc_mp_8 --backend interp diff --git a/contrib/fw/pmp-64entry-addr-test/pmp_64entry_addr_test.S b/contrib/fw/pmp-64entry-addr-test/pmp_64entry_addr_test.S new file mode 100644 index 0000000..64fe595 --- /dev/null +++ b/contrib/fw/pmp-64entry-addr-test/pmp_64entry_addr_test.S @@ -0,0 +1,26 @@ +// Verify pmpaddr32 (CSR 0x3D0) is accessible - tests 64-entry pmpaddr support. +// Pass: readback matches written sentinel -> j . (exit 0) +// Fail: any trap (unregistered CSR) or readback mismatch -> semihosting SYS_EXIT (exit 2) + +.section .text +.globl _start +_start: + la t0, fail + csrw mtvec, t0 + + li t0, 0xDEAD + csrw 0x3D0, t0 // pmpaddr32 + csrr t1, 0x3D0 + bne t0, t1, fail + + j . // pass + +fail: + li a0, 0x18 + .option push + .option norvc + slli zero, zero, 0x1f + ebreak + srai zero, zero, 7 + .option pop + j . diff --git a/contrib/fw/pmp-64entry-cfg-test/pmp_64entry_cfg_test.S b/contrib/fw/pmp-64entry-cfg-test/pmp_64entry_cfg_test.S new file mode 100644 index 0000000..0336554 --- /dev/null +++ b/contrib/fw/pmp-64entry-cfg-test/pmp_64entry_cfg_test.S @@ -0,0 +1,34 @@ +// Verify pmpcfg4 (CSR 0x3A4, holds entries 32-39 on RV64) is accessible and persistent. +// Tests 64-entry pmpcfg registration. +// Pass: write 0x9F -> read back 0x9F -> write 0 -> read back 0 -> j . (exit 0) +// Fail: any trap or readback mismatch -> semihosting SYS_EXIT (exit 2) + +.section .text +.globl _start +_start: + la t0, fail + csrw mtvec, t0 + + // Write and verify a non-zero value + li t0, 0x9F + csrw 0x3A4, t0 // pmpcfg4 write + csrr t1, 0x3A4 // pmpcfg4 read + bne t0, t1, fail // must match + + // Clear and verify zero + li t0, 0 + csrw 0x3A4, t0 + csrr t1, 0x3A4 + bne t0, t1, fail + + j . // pass + +fail: + li a0, 0x18 + .option push + .option norvc + slli zero, zero, 0x1f + ebreak + srai zero, zero, 7 + .option pop + j . diff --git a/contrib/fw/pmp-8entry-guard-test/pmp_8entry_guard_test.S b/contrib/fw/pmp-8entry-guard-test/pmp_8entry_guard_test.S new file mode 100644 index 0000000..a739801 --- /dev/null +++ b/contrib/fw/pmp-8entry-guard-test/pmp_8entry_guard_test.S @@ -0,0 +1,50 @@ +// VP guard test: verify PMP enforcement works with only 8 entries (S5 hardware limit). +// Configures entry 0 to deny load from test_word; expects load-access fault. +// Also verifies pmpaddr8+ traps (outside the 8-entry window). +// +// Pass: phase2_trap fires (load denied) -> j . (exit 0) +// Fail: CSR trap, load succeeds, or unexpected behavior -> 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: any CSR trap = PMP not available + la t0, phase1_trap + csrw mtvec, t0 + + // Configure entry 0: NA4, no R/W/X, locked + la t5, test_word + srli t1, t5, 2 + csrw pmpaddr0, t1 + + li t1, 0x90 // NA4(0x10) | L(0x80) + csrw pmpcfg0, t1 + + // phase2_trap: load fault = enforcement active = PASS + la t0, phase2_trap + csrw mtvec, t0 + + lw t1, 0(t5) // should fault: entry 0 denies read + + semihosting_fail // no fault -> FAIL + +phase1_trap: + semihosting_fail // CSR trap -> 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 19ff488..6ffd654 100644 --- a/src/iss/mem/pmp.h +++ b/src/iss/mem/pmp.h @@ -30,6 +30,7 @@ * * Contributors: * eyck@minres.com - initial implementation + * cphurley82@gmail.com - bug fixes and improvements ******************************************************************************/ #ifndef ISS_MEM_PMP_H @@ -44,8 +45,10 @@ namespace iss { namespace mem { -template struct pmp : public memory_elem { - using this_class = pmp; +template struct pmp : public memory_elem { + static_assert(NUM_ENTRIES > 0 && NUM_ENTRIES <= 64 && (NUM_ENTRIES % sizeof(typename PLAT::reg_t)) == 0, + "NUM_ENTRIES must be a multiple of sizeof(reg_t) and at most 64"); + 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); @@ -62,11 +65,11 @@ template struct pmp : public memory_elem { pmp(arch::priv_if hart_if) : hart_if(hart_if) { - for(size_t i = arch::pmpaddr0; i <= arch::pmpaddr15; ++i) { + for(size_t i = arch::pmpaddr0; i < arch::pmpaddr0 + NUM_ENTRIES; ++i) { 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 + 4; i += pmpcfg_stride) { + for(size_t i = arch::pmpcfg0; i < arch::pmpcfg0 + (NUM_ENTRIES / cfg_reg_size) * pmpcfg_stride; 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); } @@ -82,8 +85,8 @@ template struct pmp : public memory_elem { void set_next(memory_if mem) override { down_stream_mem = mem; } private: - std::array pmpaddr{0}; - std::array pmpcfg{0}; + std::array pmpaddr{0}; + std::array pmpcfg{0}; iss::status read_mem(const addr_t& addr, unsigned length, uint8_t* data) { assert((addr.type == iss::address_type::PHYSICAL || is_debug(addr.access)) && "Only physical addresses are expected in pmp"); @@ -109,7 +112,7 @@ template struct pmp : public memory_elem { } iss::status read_pmpaddr(unsigned addr, reg_t& val) { - if(addr >= arch::pmpaddr0 && addr <= arch::pmpaddr15) { + if(addr >= arch::pmpaddr0 && addr < arch::pmpaddr0 + NUM_ENTRIES) { val = pmpaddr[addr - arch::pmpaddr0]; return iss::Ok; } @@ -117,7 +120,7 @@ template struct pmp : public memory_elem { } iss::status write_pmpaddr(unsigned addr, reg_t const& val) { - if(addr >= arch::pmpaddr0 && addr <= arch::pmpaddr15) { + if(addr >= arch::pmpaddr0 && addr < arch::pmpaddr0 + NUM_ENTRIES) { pmpaddr[addr - arch::pmpaddr0] = val; return iss::Ok; } @@ -125,17 +128,17 @@ template struct pmp : public memory_elem { } iss::status read_pmpcfg(unsigned addr, reg_t& val) { - if(addr >= arch::pmpcfg0 && addr < arch::pmpcfg0 + 4) { + if(addr >= arch::pmpcfg0 && addr < arch::pmpcfg0 + (NUM_ENTRIES / cfg_reg_size) * pmpcfg_stride) { 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 + 4) { + if(addr >= arch::pmpcfg0 && addr < arch::pmpcfg0 + (NUM_ENTRIES / cfg_reg_size) * pmpcfg_stride) { pmpcfg[(addr - arch::pmpcfg0) / pmpcfg_stride] = val & cfg_valid_mask; any_active = false; - for(size_t i = 0; i < 16; i++) { + for(size_t i = 0; i < NUM_ENTRIES; i++) { auto cfg = pmpcfg[i / cfg_reg_size] >> ((i % cfg_reg_size) * 8); any_active |= cfg & PMP_A; } @@ -152,11 +155,11 @@ template struct pmp : public memory_elem { memory_if down_stream_mem; }; -template bool pmp::pmp_check(access_type type, uint64_t addr, unsigned len) { +template bool pmp::pmp_check(access_type type, uint64_t addr, unsigned len) { if(!any_active) return true; reg_t base = 0; - for(size_t i = 0; i < 16; i++) { + for(size_t i = 0; i < NUM_ENTRIES; i++) { reg_t tor = pmpaddr[i] << PMP_SHIFT; reg_t cfg = pmpcfg[i / cfg_reg_size] >> ((i % cfg_reg_size) * 8); if(cfg & PMP_A) { diff --git a/src/sysc/register_cores.cpp b/src/sysc/register_cores.cpp index 236d176..7b45614 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); @@ -132,6 +132,13 @@ __attribute__((used)) volatile std::array riscv_init = { 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_mp_8: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 index badb673..265e2f8 100644 --- a/src/vm/interp/vm_rv64gc_mp.cpp +++ b/src/vm/interp/vm_rv64gc_mp.cpp @@ -24,9 +24,19 @@ struct rv64gc_mp_hart : public arch::riscv_hart_m_p { } }; +// rv64gc_mp_8_hart: same as rv64gc_mp but with only 8 PMP entries (models SiFive S5). +struct rv64gc_mp_8_hart : public arch::riscv_hart_m_p { + mem::pmp pmp_obj{this->get_priv_if()}; + + rv64gc_mp_8_hart() { + pmp_obj.set_next(this->default_mem.get_mem_if()); + memory = pmp_obj.get_mem_if(); + } +}; + namespace { -volatile std::array rv64gc_mp_dummy = { +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(); @@ -35,6 +45,15 @@ volatile std::array rv64gc_mp_dummy = { cpu->set_semihosting_callback(*cb); } return {cpu_ptr{cpu}, vm_ptr{iss::interp::create(cpu, port, false)}}; + }), + core_factory::instance().register_creator("rv64gc_mp_8:interp", + [](unsigned port, void* init_data) -> std::tuple { + auto* cpu = new rv64gc_mp_8_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)}}; }) };