target/riscv: Align pmp size to pmp-granularity

[PATCH] target/riscv: Align pmp size to pmp-granularity

Posted by Jay Chang 1 month, 2 weeks ago

When configuring pmpcfg (TOR, NA4, or NAPOT) and pmpaddr, if the
value is smaller than the PMP granularity, it automatically aligned
to the PMP granularity.

Signed-off-by: Jay Chang <jay.chang@sifive.com>
Reviewed-by: Frank Chang <frank.chang@sifive.com>
---
 target/riscv/pmp.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index 3ef62d26ad..01b337f529 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -167,11 +167,12 @@ static bool pmp_write_cfg(CPURISCVState *env, uint32_t pmp_index, uint8_t val)
             uint8_t a_field = pmp_get_a_field(val);
             /*
              * When granularity g >= 1 (i.e., granularity > 4 bytes),
-             * the NA4 (Naturally Aligned 4-byte) mode is not selectable
+             * the NA4 (Naturally Aligned 4-byte) mode is not selectable.
+             * In this case, an NA4 setting is reinterpreted as a NAPOT mode.
              */
             if ((riscv_cpu_cfg(env)->pmp_granularity >
                 MIN_RISCV_PMP_GRANULARITY) && (a_field == PMP_AMATCH_NA4)) {
-                    return false;
+                    val |= PMP_AMATCH;
             }
             env->pmp_state.pmp[pmp_index].cfg_reg = val;
             pmp_update_rule_addr(env, pmp_index);
@@ -251,6 +252,11 @@ void pmp_update_rule_addr(CPURISCVState *env, uint32_t pmp_index)
         break;
 
     case PMP_AMATCH_NAPOT:
+        /* Align to pmp_granularity */
+        if (g >= 2) {
+            this_addr |= ((1ULL << (g - 1ULL)) - 1ULL);
+        }
+
         pmp_decode_napot(this_addr, &sa, &ea);
         break;
 
-- 
2.48.1

Re: [PATCH] target/riscv: Align pmp size to pmp-granularity

Posted by Michael Tokarev 1 month, 1 week ago

On 12/23/25 13:25, Jay Chang wrote:
> When configuring pmpcfg (TOR, NA4, or NAPOT) and pmpaddr, if the
> value is smaller than the PMP granularity, it automatically aligned
> to the PMP granularity.

There's a verb missing here. It "should be"? "needs to be" aligned?

Is it a qemu-stable material?

/mjt

Re: [PATCH] target/riscv: Align pmp size to pmp-granularity

Posted by Jay Chang 1 month, 1 week ago

Hi Michael

Thank you for the review. You're right, the sentence needs a verb.

It should read:
" it needs to be aligned to the PMP granularity."

Yes, this is qemu-stable material. The PMP alignment implements WARL
from the RISC-V specification.

On Sun, Dec 28, 2025 at 4:16 PM Michael Tokarev <mjt@tls.msk.ru> wrote:

> On 12/23/25 13:25, Jay Chang wrote:
> > When configuring pmpcfg (TOR, NA4, or NAPOT) and pmpaddr, if the
> > value is smaller than the PMP granularity, it automatically aligned
> > to the PMP granularity.
>
> There's a verb missing here. It "should be"? "needs to be" aligned?
>
> Is it a qemu-stable material?
>
> /mjt
>

Re: [PATCH] target/riscv: Align pmp size to pmp-granularity

Posted by Philippe Mathieu-Daudé 1 month, 1 week ago

Hi,

On 23/12/25 11:25, Jay Chang wrote:
> When configuring pmpcfg (TOR, NA4, or NAPOT) and pmpaddr, if the
> value is smaller than the PMP granularity, it automatically aligned
> to the PMP granularity.
> 
> Signed-off-by: Jay Chang <jay.chang@sifive.com>
> Reviewed-by: Frank Chang <frank.chang@sifive.com>
> ---
>   target/riscv/pmp.c | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 3ef62d26ad..01b337f529 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -167,11 +167,12 @@ static bool pmp_write_cfg(CPURISCVState *env, uint32_t pmp_index, uint8_t val)
>               uint8_t a_field = pmp_get_a_field(val);
>               /*
>                * When granularity g >= 1 (i.e., granularity > 4 bytes),
> -             * the NA4 (Naturally Aligned 4-byte) mode is not selectable
> +             * the NA4 (Naturally Aligned 4-byte) mode is not selectable.
> +             * In this case, an NA4 setting is reinterpreted as a NAPOT mode.
>                */
>               if ((riscv_cpu_cfg(env)->pmp_granularity >
>                   MIN_RISCV_PMP_GRANULARITY) && (a_field == PMP_AMATCH_NA4)) {
> -                    return false;
> +                    val |= PMP_AMATCH;
>               }
>               env->pmp_state.pmp[pmp_index].cfg_reg = val;
>               pmp_update_rule_addr(env, pmp_index);
> @@ -251,6 +252,11 @@ void pmp_update_rule_addr(CPURISCVState *env, uint32_t pmp_index)
>           break;
>   
>       case PMP_AMATCH_NAPOT:
> +        /* Align to pmp_granularity */
> +        if (g >= 2) {
> +            this_addr |= ((1ULL << (g - 1ULL)) - 1ULL);

Could we use QEMU_ALIGN_UP() here?

> +        }
> +
>           pmp_decode_napot(this_addr, &sa, &ea);
>           break;
>

Re: [PATCH] target/riscv: Align pmp size to pmp-granularity

Posted by Jay Chang 1 month, 1 week ago

Hi Philippe,

Thank you for the suggestion. QEMU_ALIGN_UP() is not suitable for
NAPOT because the logic is different:

- QEMU_ALIGN_UP() performs alignment (rounds up to the nearest multiple)
- NAPOT requires setting lower bits to 1 (not rounding up)

For example, with g=3 and this_addr=0x1005:
- Current code: 0x1005 | 0x3 = 0x1007 (sets lower 2 bits)
- QEMU_ALIGN_UP: QEMU_ALIGN_UP(0x1005, 4) = 0x1008 (rounds up)

However, you're right that the TOR case can be improved using
ROUND_DOWN() since it clears lower bits (which is alignment)

I'll update the patch with this improvement.

On Sun, Dec 28, 2025 at 1:03 AM Philippe Mathieu-Daudé <philmd@linaro.org>
wrote:

> Hi,
>
> On 23/12/25 11:25, Jay Chang wrote:
> > When configuring pmpcfg (TOR, NA4, or NAPOT) and pmpaddr, if the
> > value is smaller than the PMP granularity, it automatically aligned
> > to the PMP granularity.
> >
> > Signed-off-by: Jay Chang <jay.chang@sifive.com>
> > Reviewed-by: Frank Chang <frank.chang@sifive.com>
> > ---
> >   target/riscv/pmp.c | 10 ++++++++--
> >   1 file changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> > index 3ef62d26ad..01b337f529 100644
> > --- a/target/riscv/pmp.c
> > +++ b/target/riscv/pmp.c
> > @@ -167,11 +167,12 @@ static bool pmp_write_cfg(CPURISCVState *env,
> uint32_t pmp_index, uint8_t val)
> >               uint8_t a_field = pmp_get_a_field(val);
> >               /*
> >                * When granularity g >= 1 (i.e., granularity > 4 bytes),
> > -             * the NA4 (Naturally Aligned 4-byte) mode is not selectable
> > +             * the NA4 (Naturally Aligned 4-byte) mode is not
> selectable.
> > +             * In this case, an NA4 setting is reinterpreted as a NAPOT
> mode.
> >                */
> >               if ((riscv_cpu_cfg(env)->pmp_granularity >
> >                   MIN_RISCV_PMP_GRANULARITY) && (a_field ==
> PMP_AMATCH_NA4)) {
> > -                    return false;
> > +                    val |= PMP_AMATCH;
> >               }
> >               env->pmp_state.pmp[pmp_index].cfg_reg = val;
> >               pmp_update_rule_addr(env, pmp_index);
> > @@ -251,6 +252,11 @@ void pmp_update_rule_addr(CPURISCVState *env,
> uint32_t pmp_index)
> >           break;
> >
> >       case PMP_AMATCH_NAPOT:
> > +        /* Align to pmp_granularity */
> > +        if (g >= 2) {
> > +            this_addr |= ((1ULL << (g - 1ULL)) - 1ULL);
>
> Could we use QEMU_ALIGN_UP() here?
>
> > +        }
> > +
> >           pmp_decode_napot(this_addr, &sa, &ea);
> >           break;
> >
>
>

Re: [PATCH] target/riscv: Align pmp size to pmp-granularity

Posted by Daniel Henrique Barboza 1 month, 1 week ago


On 12/23/25 7:25 AM, Jay Chang wrote:
> When configuring pmpcfg (TOR, NA4, or NAPOT) and pmpaddr, if the
> value is smaller than the PMP granularity, it automatically aligned
> to the PMP granularity.
> 
> Signed-off-by: Jay Chang <jay.chang@sifive.com>
> Reviewed-by: Frank Chang <frank.chang@sifive.com>
> ---

Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>

>   target/riscv/pmp.c | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 3ef62d26ad..01b337f529 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -167,11 +167,12 @@ static bool pmp_write_cfg(CPURISCVState *env, uint32_t pmp_index, uint8_t val)
>               uint8_t a_field = pmp_get_a_field(val);
>               /*
>                * When granularity g >= 1 (i.e., granularity > 4 bytes),
> -             * the NA4 (Naturally Aligned 4-byte) mode is not selectable
> +             * the NA4 (Naturally Aligned 4-byte) mode is not selectable.
> +             * In this case, an NA4 setting is reinterpreted as a NAPOT mode.
>                */
>               if ((riscv_cpu_cfg(env)->pmp_granularity >
>                   MIN_RISCV_PMP_GRANULARITY) && (a_field == PMP_AMATCH_NA4)) {
> -                    return false;
> +                    val |= PMP_AMATCH;
>               }
>               env->pmp_state.pmp[pmp_index].cfg_reg = val;
>               pmp_update_rule_addr(env, pmp_index);
> @@ -251,6 +252,11 @@ void pmp_update_rule_addr(CPURISCVState *env, uint32_t pmp_index)
>           break;
>   
>       case PMP_AMATCH_NAPOT:
> +        /* Align to pmp_granularity */
> +        if (g >= 2) {
> +            this_addr |= ((1ULL << (g - 1ULL)) - 1ULL);
> +        }
> +
>           pmp_decode_napot(this_addr, &sa, &ea);
>           break;
>