When running in TOR mode (Top of Range) the next PMP entry controls
whether the entry is locked. However simply checking if the PMP_LOCK bit
is set is not sufficient with the Smepmp extension which now provides a
bit (mseccfg.RLB (Rule Lock Bypass)) to disregard the lock bits. In
order to respect this bit use the convenience pmp_is_locked() function
rather than directly checking PMP_LOCK since this function checks
mseccfg.RLB.

Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
---
 target/riscv/pmp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index a185c246d6..85ab270dad 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -524,7 +524,7 @@ void pmpaddr_csr_write(CPURISCVState *env, uint32_t addr_index,
             uint8_t pmp_cfg = env->pmp_state.pmp[addr_index + 1].cfg_reg;
             is_next_cfg_tor = PMP_AMATCH_TOR == pmp_get_a_field(pmp_cfg);
 
-            if (pmp_cfg & PMP_LOCK && is_next_cfg_tor) {
+            if (pmp_is_locked(env, addr_index + 1) && is_next_cfg_tor) {
                 qemu_log_mask(LOG_GUEST_ERROR,
                               "ignoring pmpaddr write - pmpcfg + 1 locked\n");
                 return;
-- 
2.48.1

On Tue, Feb 11, 2025 at 1:38 AM Rob Bradford <rbradford@rivosinc.com> wrote:
>
> When running in TOR mode (Top of Range) the next PMP entry controls
> whether the entry is locked. However simply checking if the PMP_LOCK bit
> is set is not sufficient with the Smepmp extension which now provides a
> bit (mseccfg.RLB (Rule Lock Bypass)) to disregard the lock bits. In
> order to respect this bit use the convenience pmp_is_locked() function
> rather than directly checking PMP_LOCK since this function checks
> mseccfg.RLB.
>
> Signed-off-by: Rob Bradford <rbradford@rivosinc.com>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  target/riscv/pmp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index a185c246d6..85ab270dad 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -524,7 +524,7 @@ void pmpaddr_csr_write(CPURISCVState *env, uint32_t addr_index,
>              uint8_t pmp_cfg = env->pmp_state.pmp[addr_index + 1].cfg_reg;
>              is_next_cfg_tor = PMP_AMATCH_TOR == pmp_get_a_field(pmp_cfg);
>
> -            if (pmp_cfg & PMP_LOCK && is_next_cfg_tor) {
> +            if (pmp_is_locked(env, addr_index + 1) && is_next_cfg_tor) {
>                  qemu_log_mask(LOG_GUEST_ERROR,
>                                "ignoring pmpaddr write - pmpcfg + 1 locked\n");
>                  return;
> --
> 2.48.1
>
>

On Tue, Feb 11, 2025 at 1:38 AM Rob Bradford <rbradford@rivosinc.com> wrote:
>
> When running in TOR mode (Top of Range) the next PMP entry controls
> whether the entry is locked. However simply checking if the PMP_LOCK bit
> is set is not sufficient with the Smepmp extension which now provides a
> bit (mseccfg.RLB (Rule Lock Bypass)) to disregard the lock bits. In
> order to respect this bit use the convenience pmp_is_locked() function
> rather than directly checking PMP_LOCK since this function checks
> mseccfg.RLB.
>
> Signed-off-by: Rob Bradford <rbradford@rivosinc.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/pmp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index a185c246d6..85ab270dad 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -524,7 +524,7 @@ void pmpaddr_csr_write(CPURISCVState *env, uint32_t addr_index,
>              uint8_t pmp_cfg = env->pmp_state.pmp[addr_index + 1].cfg_reg;
>              is_next_cfg_tor = PMP_AMATCH_TOR == pmp_get_a_field(pmp_cfg);
>
> -            if (pmp_cfg & PMP_LOCK && is_next_cfg_tor) {
> +            if (pmp_is_locked(env, addr_index + 1) && is_next_cfg_tor) {
>                  qemu_log_mask(LOG_GUEST_ERROR,
>                                "ignoring pmpaddr write - pmpcfg + 1 locked\n");
>                  return;
> --
> 2.48.1
>
>

On 2/10/25 12:37 PM, Rob Bradford wrote:
> When running in TOR mode (Top of Range) the next PMP entry controls
> whether the entry is locked. However simply checking if the PMP_LOCK bit
> is set is not sufficient with the Smepmp extension which now provides a
> bit (mseccfg.RLB (Rule Lock Bypass)) to disregard the lock bits. In
> order to respect this bit use the convenience pmp_is_locked() function
> rather than directly checking PMP_LOCK since this function checks
> mseccfg.RLB.
> 
> Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
> ---

Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>


>   target/riscv/pmp.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index a185c246d6..85ab270dad 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -524,7 +524,7 @@ void pmpaddr_csr_write(CPURISCVState *env, uint32_t addr_index,
>               uint8_t pmp_cfg = env->pmp_state.pmp[addr_index + 1].cfg_reg;
>               is_next_cfg_tor = PMP_AMATCH_TOR == pmp_get_a_field(pmp_cfg);
>   
> -            if (pmp_cfg & PMP_LOCK && is_next_cfg_tor) {
> +            if (pmp_is_locked(env, addr_index + 1) && is_next_cfg_tor) {
>                   qemu_log_mask(LOG_GUEST_ERROR,
>                                 "ignoring pmpaddr write - pmpcfg + 1 locked\n");
>                   return;