From 949b731e06ccba228904bbf339969f62cd4c125c Mon Sep 17 00:00:00 2001 From: "cphurley82@gmail.com" Date: Wed, 30 Sep 2026 18:51:54 -0700 Subject: [PATCH 1/2] ci: bound PMP functional test runs with a timeout The pmp-tests job set no timeout anywhere, so a firmware test that hangs the simulator would hold the runner until GitHub's six-hour default. A PMP check that never terminates produces exactly that: no trap, no log, no exit. Wrap every riscv-sim invocation in `timeout 60` so a hang fails its own step with exit 124, and cap the job at ten minutes as a backstop for the steps around it. Each test runs in under 0.1s locally and the whole job has taken 20 to 40 seconds on recent runs, so neither bound is close. The no-PMP CSR step, which expects exit 2, still fails on a timeout because 124 is not 2. --- .github/workflows/ci.yml | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0e9d5d7..fff686d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -71,6 +71,7 @@ jobs: name: PMP Functional Tests runs-on: ubuntu-24.04 needs: cpp-compliance + timeout-minutes: 10 steps: - uses: actions/checkout@v4 @@ -94,11 +95,11 @@ jobs: - 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=$? + LD_LIBRARY_PATH=. timeout 60 ./riscv-sim -f pmp_csr_test --isa rv64gc_m --backend interp || rc=$? [ "${rc:-0}" -eq 2 ] - name: rv64gc_mp_64 CSR test - interp (with PMP) - run: LD_LIBRARY_PATH=. ./riscv-sim -f pmp_csr_test --isa rv64gc_mp_64 --backend interp + run: LD_LIBRARY_PATH=. timeout 60 ./riscv-sim -f pmp_csr_test --isa rv64gc_mp_64 --backend interp - name: Build PMP enforcement test firmware run: | @@ -108,7 +109,7 @@ jobs: contrib/fw/pmp-enforce-test/pmp_enforce_test.S - name: rv64gc_mp_64 enforcement test - interp - run: LD_LIBRARY_PATH=. ./riscv-sim -f pmp_enforce_test --isa rv64gc_mp_64 --backend interp + run: LD_LIBRARY_PATH=. timeout 60 ./riscv-sim -f pmp_enforce_test --isa rv64gc_mp_64 --backend interp - name: Build PMP shift test firmware run: | @@ -118,7 +119,7 @@ jobs: contrib/fw/pmp-shift-test/pmp_shift_test.S - name: rv64gc_mp_64 shift test - interp - run: LD_LIBRARY_PATH=. ./riscv-sim -f pmp_shift_test --isa rv64gc_mp_64 --backend interp + run: LD_LIBRARY_PATH=. timeout 60 ./riscv-sim -f pmp_shift_test --isa rv64gc_mp_64 --backend interp - name: Build PMP upper-cfg test firmware run: | @@ -128,7 +129,7 @@ jobs: contrib/fw/pmp-upper-cfg-test/pmp_upper_cfg_test.S - name: rv64gc_mp_64 upper-cfg test - interp - run: LD_LIBRARY_PATH=. ./riscv-sim -f pmp_upper_cfg_test --isa rv64gc_mp_64 --backend interp + run: LD_LIBRARY_PATH=. timeout 60 ./riscv-sim -f pmp_upper_cfg_test --isa rv64gc_mp_64 --backend interp - name: Build PMP cfg2 test firmware run: | @@ -138,7 +139,7 @@ jobs: contrib/fw/pmp-cfg2-test/pmp_cfg2_test.S - name: rv64gc_mp_64 cfg2 test - interp - run: LD_LIBRARY_PATH=. ./riscv-sim -f pmp_cfg2_test --isa rv64gc_mp_64 --backend interp + run: LD_LIBRARY_PATH=. timeout 60 ./riscv-sim -f pmp_cfg2_test --isa rv64gc_mp_64 --backend interp - name: Build PMP TOR test firmware run: | @@ -148,7 +149,7 @@ jobs: contrib/fw/pmp-tor-test/pmp_tor_test.S - name: rv64gc_mp_64 TOR test - interp - run: LD_LIBRARY_PATH=. ./riscv-sim -f pmp_tor_test --isa rv64gc_mp_64 --backend interp + run: LD_LIBRARY_PATH=. timeout 60 ./riscv-sim -f pmp_tor_test --isa rv64gc_mp_64 --backend interp - name: Build PMP 64-entry pmpaddr test firmware run: | @@ -158,7 +159,7 @@ jobs: contrib/fw/pmp-64entry-addr-test/pmp_64entry_addr_test.S - name: rv64gc_mp_64 64-entry pmpaddr test - interp - run: LD_LIBRARY_PATH=. ./riscv-sim -f pmp_64entry_addr_test --isa rv64gc_mp_64 --backend interp + run: LD_LIBRARY_PATH=. timeout 60 ./riscv-sim -f pmp_64entry_addr_test --isa rv64gc_mp_64 --backend interp - name: Build PMP 64-entry pmpcfg test firmware run: | @@ -168,7 +169,7 @@ jobs: contrib/fw/pmp-64entry-cfg-test/pmp_64entry_cfg_test.S - name: rv64gc_mp_64 64-entry pmpcfg test - interp - run: LD_LIBRARY_PATH=. ./riscv-sim -f pmp_64entry_cfg_test --isa rv64gc_mp_64 --backend interp + run: LD_LIBRARY_PATH=. timeout 60 ./riscv-sim -f pmp_64entry_cfg_test --isa rv64gc_mp_64 --backend interp - name: Build PMP 8-entry guard test firmware (VP/S5 model) run: | @@ -178,7 +179,7 @@ jobs: 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 + run: LD_LIBRARY_PATH=. timeout 60 ./riscv-sim -f pmp_8entry_guard_test --isa rv64gc_mp_8 --backend interp - name: Build PMP fetch deny test firmware run: | @@ -188,7 +189,7 @@ jobs: contrib/fw/pmp-fetch-deny-test/pmp_fetch_deny_test.S - name: rv64gc_mp_64 fetch execute-permission test - interp - run: LD_LIBRARY_PATH=. ./riscv-sim -f pmp_fetch_deny_test --isa rv64gc_mp_64 --backend interp + run: LD_LIBRARY_PATH=. timeout 60 ./riscv-sim -f pmp_fetch_deny_test --isa rv64gc_mp_64 --backend interp - name: Build PMP fetch straddle test firmware run: | @@ -198,7 +199,7 @@ jobs: contrib/fw/pmp-fetch-straddle-test/pmp_fetch_straddle_test.S - name: rv64gc_mp_64 straddling-fetch sector test - interp - run: LD_LIBRARY_PATH=. ./riscv-sim -f pmp_fetch_straddle_test --isa rv64gc_mp_64 --backend interp + run: LD_LIBRARY_PATH=. timeout 60 ./riscv-sim -f pmp_fetch_straddle_test --isa rv64gc_mp_64 --backend interp - name: Build PMP lock test firmware run: | @@ -208,7 +209,7 @@ jobs: contrib/fw/pmp-lock-test/pmp_lock_test.S - name: rv64gc_mp_64 lock-bit test - interp - run: LD_LIBRARY_PATH=. ./riscv-sim -f pmp_lock_test --isa rv64gc_mp_64 --backend interp + run: LD_LIBRARY_PATH=. timeout 60 ./riscv-sim -f pmp_lock_test --isa rv64gc_mp_64 --backend interp - name: Build PMP no-entry U-mode test firmware run: | @@ -218,4 +219,4 @@ jobs: contrib/fw/pmp-noentry-umode-test/pmp_noentry_umode_test.S - name: rv64gc_mup no-entry U-mode denial test - interp - run: LD_LIBRARY_PATH=. ./riscv-sim -f pmp_noentry_umode_test --isa rv64gc_mup --backend interp + run: LD_LIBRARY_PATH=. timeout 60 ./riscv-sim -f pmp_noentry_umode_test --isa rv64gc_mup --backend interp From be590808a1f109ccdfcd032e42d64c05d0fbd90e Mon Sep 17 00:00:00 2001 From: "cphurley82@gmail.com" Date: Wed, 30 Sep 2026 19:45:13 -0700 Subject: [PATCH 2/2] fix(pmp): terminate the sector walk at the top of the address space pmp_check bounded its sector walk with cur_addr <= last_sector. When the access touches the top sector, the step past it wraps to 0 and the loop never ends, hanging the simulation with no trap or log. A zero-length access at address 0 hangs the same way. Bound the walk on a 64-bit byte count instead, which cannot wrap. Counting from the start of the first sector keeps misaligned accesses covering their last sector. pmp-top-sector-test loads from the top sector in M mode with one entry enabled elsewhere. It hung before this change and passes now. --- .github/workflows/ci.yml | 10 ++++ .../pmp-top-sector-test/pmp_top_sector_test.S | 53 +++++++++++++++++++ src/iss/mem/pmp.h | 15 ++++-- 3 files changed, 73 insertions(+), 5 deletions(-) create mode 100644 contrib/fw/pmp-top-sector-test/pmp_top_sector_test.S diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fff686d..a68f23d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -201,6 +201,16 @@ jobs: - name: rv64gc_mp_64 straddling-fetch sector test - interp run: LD_LIBRARY_PATH=. timeout 60 ./riscv-sim -f pmp_fetch_straddle_test --isa rv64gc_mp_64 --backend interp + - name: Build PMP top-sector test firmware + run: | + riscv64-unknown-elf-gcc -nostdlib -march=rv64gc -mabi=lp64 -I contrib/fw \ + -Wl,-Ttext=0x10000,--no-dynamic-linker \ + -o pmp_top_sector_test \ + contrib/fw/pmp-top-sector-test/pmp_top_sector_test.S + + - name: rv64gc_mp_64 M-mode load from top 4-byte sector completes - interp + run: LD_LIBRARY_PATH=. timeout 60 ./riscv-sim -f pmp_top_sector_test --isa rv64gc_mp_64 --backend interp + - name: Build PMP lock test firmware run: | riscv64-unknown-elf-gcc -nostdlib -march=rv64gc -mabi=lp64 -I contrib/fw \ diff --git a/contrib/fw/pmp-top-sector-test/pmp_top_sector_test.S b/contrib/fw/pmp-top-sector-test/pmp_top_sector_test.S new file mode 100644 index 0000000..b253b5d --- /dev/null +++ b/contrib/fw/pmp-top-sector-test/pmp_top_sector_test.S @@ -0,0 +1,53 @@ +// PMP sector iteration must terminate when an access touches the top sector of +// the address space. +// +// pmp_check walked sectors with an inclusive address bound: +// for(cur_addr = first; cur_addr <= last_sector; cur_addr += 4) +// When last_sector is the top 4-byte sector, the increment past it wraps +// cur_addr to 0, which is again <= last_sector, so the loop never ends. The +// simulator hangs with no trap and no log. +// +// The walk only runs for entries that are enabled, so entry 0 is programmed as +// an NA4 region over guard_word, well away from the top. It is unlocked, and +// guard_word lives in .data so no instruction fetch can overlap it; the entry +// exists only to make the walk run. The load from the top sector then matches +// no entry and must fall through to the M-mode default, which allows it. The +// load then reaches the simulator's default memory, which returns arbitrary +// data for unpopulated pages without faulting, so the address does not need to +// be backed. The loaded value is not checked. +// +// Buggy: the sector walk for entry 0 never terminates -> hang (CI timeout). +// Fixed: no entry matches, the M-mode load completes -> PASS. +// +// Pass: the load completes without a trap -> j . (exit 0) +// Fail: any trap, CSR or load -> semihosting SYS_EXIT (exit 2) + +#include "pmp_test_common.h" + +.section .text +.globl _start +_start: + // Any trap means either PMP is unavailable or the load faulted -> FAIL + la t0, unexpected_trap + csrw mtvec, t0 + + // pmpaddr0 = guard_word >> 2: NA4 over one word nowhere near the top + la t1, guard_word + srli t1, t1, 2 + csrw pmpaddr0, t1 + + // pmpcfg0 byte 0 = 0x10 = NA4, unlocked, no R/W/X + li t1, 0x10 + csrw pmpcfg0, t1 + + li t0, -4 // 0xFFFF_FFFF_FFFF_FFFC, the top 4-byte sector + lw t1, 0(t0) + j . // the load completed -> PASS + +trap_entry unexpected_trap + semihosting_fail // CSR or load trapped -> FAIL + +.section .data + .align 2 +guard_word: + .word 0 diff --git a/src/iss/mem/pmp.h b/src/iss/mem/pmp.h index 680b9ba..aa465c0 100644 --- a/src/iss/mem/pmp.h +++ b/src/iss/mem/pmp.h @@ -181,17 +181,22 @@ template bool pmp::pmp_ch auto is_na4 = pmp_a == PMP_NA4; reg_t mask = (pmpaddr[i] << 1) | (!is_na4); mask = ~(mask & ~(mask + 1)) << PMP_SHIFT; - // Check every 4-byte sector the access touches. Counting offsets up to len - // skips the last sector whenever addr is not sector aligned, which fetches can - // be: fetch_ins always asks for 4 bytes and the fetch alignment is 2 on a + // Check every 4-byte sector the access touches: span counts bytes from the start of + // the first sector, so a misaligned access also covers its last sector. Fetches can + // be misaligned: fetch_ins always asks for 4 bytes and the fetch alignment is 2 on a // compressed ISA, so an instruction at addr%4==2 spans two sectors. Note this // also inspects the 2 bytes the ISS over-reads past a compressed instruction, // so a fetch at the very end of an executable region is denied conservatively. + // The bound is a byte count, not an address: an address bound cannot terminate + // when the last sector is the top of the address space, as the step past it wraps. + // The count is 64 bits wide so that it cannot wrap either, whatever len is. auto any_match = false; auto all_match = true; constexpr reg_t sector_size = 1 << PMP_SHIFT; - reg_t last_sector = (addr + len - 1) & ~(sector_size - 1); - for(reg_t cur_addr = addr & ~(sector_size - 1); cur_addr <= last_sector; cur_addr += sector_size) { + reg_t first_sector = addr & ~(sector_size - 1); + uint64_t span = uint64_t(len) + (addr & (sector_size - 1)); + for(uint64_t off = 0; off < span; off += sector_size) { + reg_t cur_addr = first_sector + off; // wraps to 0 past the top; off, not cur_addr, ends the loop auto napot_match = ((cur_addr ^ tor) & mask) == 0; auto tor_match = base <= cur_addr && cur_addr < tor; auto match = is_tor ? tor_match : napot_match;