Re: [RFC PATCH 18/18] target/riscv: Pass first/last to pmp_hart_has_privs

On Sat, 2026-08-15 at 13:37 -0700, Richard Henderson wrote:
> This eliminates an ambiguity with size == 0 and also
> aligns with the underlying hw representation.
> 
> Signed-off-by: Richard Henderson <[email protected]>

Reviewed-by: Alistair Francis <[email protected]>

Alistair

> ---
>  target/riscv/tcg/pmp.h        |  5 ++---
>  target/riscv/tcg/cpu_helper.c |  2 +-
>  target/riscv/tcg/pmp.c        | 24 ++++--------------------
>  3 files changed, 7 insertions(+), 24 deletions(-)
> 
> diff --git a/target/riscv/tcg/pmp.h b/target/riscv/tcg/pmp.h
> index 4c95c2767a..eacb5b5349 100644
> --- a/target/riscv/tcg/pmp.h
> +++ b/target/riscv/tcg/pmp.h
> @@ -70,9 +70,8 @@ typedef struct {
>  
>  typedef struct CPUArchState CPURISCVState;
>  
> -bool pmp_hart_has_privs(CPURISCVState *env, hwaddr addr,
> -                        int size, pmp_priv_t privs,
> -                        pmp_priv_t *allowed_privs,
> +bool pmp_hart_has_privs(CPURISCVState *env, hwaddr first, hwaddr
> last,
> +                        pmp_priv_t privs, pmp_priv_t *allowed_privs,
>                          privilege_mode_t mode);
>  uint64_t pmp_get_tlb_size(CPURISCVState *env, hwaddr addr);
>  void pmp_update_rule_addr(CPURISCVState *env, uint32_t pmp_index);
> diff --git a/target/riscv/tcg/cpu_helper.c
> b/target/riscv/tcg/cpu_helper.c
> index fb1250a4b8..7ef5ab5277 100644
> --- a/target/riscv/tcg/cpu_helper.c
> +++ b/target/riscv/tcg/cpu_helper.c
> @@ -916,7 +916,7 @@ static int get_physical_address_pmp(CPURISCVState
> *env, int *prot,
>          return TRANSLATE_SUCCESS;
>      }
>  
> -    pmp_has_privs = pmp_hart_has_privs(env, first, last - first + 1,
> +    pmp_has_privs = pmp_hart_has_privs(env, first, last,
>                                         1 << access_type, &pmp_priv,
> mode);
>      if (!pmp_has_privs) {
>          *prot = 0;
> diff --git a/target/riscv/tcg/pmp.c b/target/riscv/tcg/pmp.c
> index fc035c95cf..d3cbb2cb2e 100644
> --- a/target/riscv/tcg/pmp.c
> +++ b/target/riscv/tcg/pmp.c
> @@ -372,12 +372,10 @@ static bool
> pmp_hart_has_privs_default(CPURISCVState *env, pmp_priv_t privs,
>   * pmpcfg but is not acted upon during access checks. Cache
> attributes
>   * have no functional impact in QEMU emulation.
>   */
> -bool pmp_hart_has_privs(CPURISCVState *env, hwaddr addr,
> -                        int size, pmp_priv_t privs,
> -                        pmp_priv_t *allowed_privs,
> +bool pmp_hart_has_privs(CPURISCVState *env, hwaddr first, hwaddr
> last,
> +                        pmp_priv_t privs, pmp_priv_t *allowed_privs,
>                          privilege_mode_t mode)
>  {
> -    int pmp_size = 0;
>      uint8_t pmp_regions = riscv_cpu_cfg(env)->pmp_regions;
>  
>      /* Short cut if no rules */
> @@ -385,20 +383,6 @@ bool pmp_hart_has_privs(CPURISCVState *env,
> hwaddr addr,
>          return pmp_hart_has_privs_default(env, privs, allowed_privs,
> mode);
>      }
>  
> -    if (size == 0) {
> -        if (riscv_cpu_cfg(env)->mmu) {
> -            /*
> -             * If size is unknown (0), assume that all bytes
> -             * from addr to the end of the page will be accessed.
> -             */
> -            pmp_size = -(addr | TARGET_PAGE_MASK);
> -        } else {
> -            pmp_size = 2 << riscv_cpu_mxl(env);
> -        }
> -    } else {
> -        pmp_size = size;
> -    }
> -
>      /*
>       * 1.10 draft priv spec states there is an implicit order
>       * from low to high
> @@ -410,8 +394,8 @@ bool pmp_hart_has_privs(CPURISCVState *env,
> hwaddr addr,
>              continue;
>          }
>  
> -        s = pmp_is_in_range(env, i, addr);
> -        e = pmp_is_in_range(env, i, addr + pmp_size - 1);
> +        s = pmp_is_in_range(env, i, first);
> +        e = pmp_is_in_range(env, i, last);
>  
>          /* fully inside */
>          if (s && e) {