On Wed, 2026-10-21 at 22:02 -0600, Sebastian Alba Vives wrote:
> pmp_unlock_entries() bounds its loop with pmp_get_num_rules(), which
> returns the number of entries whose A field is not OFF rather than
> the
> number of implemented entries. With a sparse configuration the loop
> stops early: with entries 0 and 1 OFF and entries 2 and 3 locked,
> num_rules is 2, the loop covers i = 0 and i = 1, and entries 2 and 3
> keep their lock and address-match bits across reset.
>
> riscv_cpu_reset_hold() clears mseccfg, and with it RLB, before
> calling
> this, so pmp_is_readonly() then silently discards every subsequent
> pmpcfg write to those entries. The stale rules survive for the
> lifetime of the machine.
>
> Bound the loop by the number of implemented regions instead.
>
> Fixes: 4bf501dc0118 ("target/riscv: pmp: Clear pmp/smepmp bits on
> reset")
> Signed-off-by: Sebastian Alba Vives <[email protected]>
Reviewed-by: Alistair Francis <[email protected]>
Alistair
> ---
> target/riscv/tcg/pmp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/tcg/pmp.c b/target/riscv/tcg/pmp.c
> index 41b55519a8..061f0e078d 100644
> --- a/target/riscv/tcg/pmp.c
> +++ b/target/riscv/tcg/pmp.c
> @@ -201,7 +201,7 @@ 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);
> + uint32_t pmp_num = riscv_cpu_cfg(env)->pmp_regions;
> int i;
>
> for (i = 0; i < pmp_num; i++) {
