diff mbox series

target/riscv: Smepmp: Return error when access permission not allowed in PMP

Message ID 20230605075150.367555-1-hchauhan@ventanamicro.com
State New
Headers show
Series target/riscv: Smepmp: Return error when access permission not allowed in PMP | expand

Commit Message

Himanshu Chauhan June 5, 2023, 7:51 a.m. UTC 
On an address match, skip checking for default permissions and return error
based on access defined in PMP configuration.

Fixes: 90b1fafce06 ("target/riscv: Smepmp: Skip applying default rules when address matches")
Signed-off-by: Himanshu Chauhan <hchauhan@ventanamicro.com>
---
 target/riscv/pmp.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Weiwei Li June 5, 2023, 9:16 a.m. UTC | #1 
On 2023/6/5 15:51, Himanshu Chauhan wrote:
> On an address match, skip checking for default permissions and return error
> based on access defined in PMP configuration.
>
> Fixes: 90b1fafce06 ("target/riscv: Smepmp: Skip applying default rules when address matches")
> Signed-off-by: Himanshu Chauhan <hchauhan@ventanamicro.com>
> ---
>   target/riscv/pmp.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 418738afd8..6238528282 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -435,8 +435,8 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
>                * defined with PMP must be used. We shouldn't fallback on
>                * finding default privileges.
>                */
> -            ret = true;
> -            break;
> +            ret = ((privs & *allowed_privs) == privs ? true : false);
> +            goto _address_matched;

goto seems unnecessary. We can directly return (privs & *allowed_privs) 
== privs here.

And then we can directly return pmp_hart_has_privs_default(env, privs, 
allowed_privs, mode) after this loop.

Regards,

Weiwei Li

>           }
>       }
>   
> @@ -445,6 +445,7 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
>           ret = pmp_hart_has_privs_default(env, privs, allowed_privs, mode);

>       }
>   
> + _address_matched:
>       return ret;
>   }
>
Richard Henderson June 5, 2023, 3:12 p.m. UTC | #2 
On 6/5/23 00:51, Himanshu Chauhan wrote:
> +            ret = ((privs & *allowed_privs) == privs ? true : false);

Never convert bool to bool in this way.


r~
diff mbox series

Patch

diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index 418738afd8..6238528282 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -435,8 +435,8 @@  bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
              * defined with PMP must be used. We shouldn't fallback on
              * finding default privileges.
              */
-            ret = true;
-            break;
+            ret = ((privs & *allowed_privs) == privs ? true : false);
+            goto _address_matched;
         }
     }
 
@@ -445,6 +445,7 @@  bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
         ret = pmp_hart_has_privs_default(env, privs, allowed_privs, mode);
     }
 
+ _address_matched:
     return ret;
 }