pmp_unlock_entries() bounds its loop with pmp_get_num_rules(), which is
the count of entries whose A field is not OFF, not the number of
implemented PMP entries. When the active rules are not contiguous from
index 0, every entry at an index greater than or equal to num_rules
keeps its lock bit across a CPU reset.

Those entries stay locked after the reset has cleared mseccfg, so
mseccfg.RLB is no longer set and pmp_is_readonly() silently discards
subsequent pmpcfg writes. Firmware started after the reset finds PMP
entries it did not configure and cannot clear.

Reproduced on -machine virt -cpu rv64,smepmp=true with a bare-metal
payload that sets pmpcfg0 = 0x99990000 (entries 2 and 3 as
L | NAPOT | R, entries 0 and 1 left OFF) and then resets through the
sifive_test device. On the second boot pmpcfg0 reads back 0x99990000
and writing 0 to it leaves the value unchanged. With this patch the
same payload reads 0x01010000 on the second boot and clearing pmpcfg0
succeeds.

Iterate over the implemented entries instead.

Fixes: 4bf501dc0118 ("target/riscv: pmp: Clear pmp/smepmp bits on reset")
Signed-off-by: Sebastian Alba Vives <[email protected]>
---
 target/riscv/tcg/pmp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/riscv/tcg/pmp.c b/target/riscv/tcg/pmp.c
index 41b55519a8..5286c6aa90 100644
--- a/target/riscv/tcg/pmp.c
+++ b/target/riscv/tcg/pmp.c
@@ -201,10 +201,10 @@ static bool pmp_write_cfg(CPURISCVState *env, uint32_t 
pmp_index, uint8_t val)
 
 void pmp_unlock_entries(CPURISCVState *env)
 {
-    uint32_t pmp_num = pmp_get_num_rules(env);
+    uint8_t pmp_regions = riscv_cpu_cfg(env)->pmp_regions;
     int i;
 
-    for (i = 0; i < pmp_num; i++) {
+    for (i = 0; i < pmp_regions; i++) {
         env->pmp_state.pmp[i].cfg_reg &= ~(PMP_LOCK | PMP_AMATCH);
     }
 }
-- 
2.43.0


Reply via email to