target/riscv: Fix pmp range wraparound on zero

[PATCH] target/riscv: Fix pmp range wraparound on zero

Posted by Vac Chen 5 months, 1 week ago

pmp_is_in_range() prefers to match addresses within the interval
[start, end]. To archieve this, pmpaddrX is decremented during the end
address update.

In TOR mode, a rule is ignored if its start address is greater than or
equal to its end address.

However, if pmpaddrX is set to 0, this decrement operation causes the
calulated end address to wrap around to UINT_MAX. In this scenario, the
address guard for this PMP entry would become ineffective.

This patch addresses the issue by moving the guard check earlier,
preventing the problematic wraparound when pmpaddrX is zero.

Signed-off-by: Vac Chen <vacantron@gmail.com>
---
 target/riscv/pmp.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index 3540327c9a..72f1372a49 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -211,11 +211,12 @@ void pmp_update_rule_addr(CPURISCVState *env, uint32_t pmp_index)
         break;
 
     case PMP_AMATCH_TOR:
-        sa = prev_addr << 2; /* shift up from [xx:0] to [xx+2:2] */
-        ea = (this_addr << 2) - 1u;
-        if (sa > ea) {
+        if (prev_addr >= this_addr) {
             sa = ea = 0u;
+            break;
         }
+        sa = prev_addr << 2; /* shift up from [xx:0] to [xx+2:2] */
+        ea = (this_addr << 2) - 1u;
         break;
 
     case PMP_AMATCH_NA4:
-- 
2.50.0

Re: [PATCH] target/riscv: Fix pmp range wraparound on zero

Posted by Alistair Francis 5 months, 1 week ago

On Sun, Jul 6, 2025 at 11:04 PM Vac Chen <vacantron@gmail.com> wrote:
>
> pmp_is_in_range() prefers to match addresses within the interval
> [start, end]. To archieve this, pmpaddrX is decremented during the end
> address update.
>
> In TOR mode, a rule is ignored if its start address is greater than or
> equal to its end address.
>
> However, if pmpaddrX is set to 0, this decrement operation causes the
> calulated end address to wrap around to UINT_MAX. In this scenario, the
> address guard for this PMP entry would become ineffective.
>
> This patch addresses the issue by moving the guard check earlier,
> preventing the problematic wraparound when pmpaddrX is zero.
>
> Signed-off-by: Vac Chen <vacantron@gmail.com>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  target/riscv/pmp.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 3540327c9a..72f1372a49 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -211,11 +211,12 @@ void pmp_update_rule_addr(CPURISCVState *env, uint32_t pmp_index)
>          break;
>
>      case PMP_AMATCH_TOR:
> -        sa = prev_addr << 2; /* shift up from [xx:0] to [xx+2:2] */
> -        ea = (this_addr << 2) - 1u;
> -        if (sa > ea) {
> +        if (prev_addr >= this_addr) {
>              sa = ea = 0u;
> +            break;
>          }
> +        sa = prev_addr << 2; /* shift up from [xx:0] to [xx+2:2] */
> +        ea = (this_addr << 2) - 1u;
>          break;
>
>      case PMP_AMATCH_NA4:
> --
> 2.50.0
>
>

Re: [PATCH] target/riscv: Fix pmp range wraparound on zero

Posted by Alistair Francis 5 months, 1 week ago

On Sun, Jul 6, 2025 at 11:04 PM Vac Chen <vacantron@gmail.com> wrote:
>
> pmp_is_in_range() prefers to match addresses within the interval
> [start, end]. To archieve this, pmpaddrX is decremented during the end
> address update.
>
> In TOR mode, a rule is ignored if its start address is greater than or
> equal to its end address.
>
> However, if pmpaddrX is set to 0, this decrement operation causes the
> calulated end address to wrap around to UINT_MAX. In this scenario, the
> address guard for this PMP entry would become ineffective.
>
> This patch addresses the issue by moving the guard check earlier,
> preventing the problematic wraparound when pmpaddrX is zero.
>
> Signed-off-by: Vac Chen <vacantron@gmail.com>

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

Alistair

> ---
>  target/riscv/pmp.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 3540327c9a..72f1372a49 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -211,11 +211,12 @@ void pmp_update_rule_addr(CPURISCVState *env, uint32_t pmp_index)
>          break;
>
>      case PMP_AMATCH_TOR:
> -        sa = prev_addr << 2; /* shift up from [xx:0] to [xx+2:2] */
> -        ea = (this_addr << 2) - 1u;
> -        if (sa > ea) {
> +        if (prev_addr >= this_addr) {
>              sa = ea = 0u;
> +            break;
>          }
> +        sa = prev_addr << 2; /* shift up from [xx:0] to [xx+2:2] */
> +        ea = (this_addr << 2) - 1u;
>          break;
>
>      case PMP_AMATCH_NA4:
> --
> 2.50.0
>
>