qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] target/riscv: Smepmp: Return error when access permission not allowed in PMP
@ 2023-06-05  7:51 Himanshu Chauhan
  2023-06-05  9:16 ` Weiwei Li
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Himanshu Chauhan @ 2023-06-05  7:51 UTC (permalink / raw)
  To: qemu-riscv, qemu-devel; +Cc: Himanshu Chauhan

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;
         }
     }
 
@@ -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;
 }
 
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] target/riscv: Smepmp: Return error when access permission not allowed in PMP
  2023-06-05  7:51 [PATCH] target/riscv: Smepmp: Return error when access permission not allowed in PMP Himanshu Chauhan
@ 2023-06-05  9:16 ` Weiwei Li
  2023-06-05 11:49 ` [PATCH v2] " Himanshu Chauhan
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Weiwei Li @ 2023-06-05  9:16 UTC (permalink / raw)
  To: Himanshu Chauhan, qemu-riscv, qemu-devel; +Cc: liweiwei


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;
>   }
>   



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] target/riscv: Smepmp: Return error when access permission not allowed in PMP
  2023-06-05  7:51 [PATCH] target/riscv: Smepmp: Return error when access permission not allowed in PMP Himanshu Chauhan
  2023-06-05  9:16 ` Weiwei Li
@ 2023-06-05 11:49 ` Himanshu Chauhan
  2023-06-05 12:17   ` Weiwei Li
  2023-06-05 15:12 ` [PATCH] " Richard Henderson
  2023-06-05 16:45 ` [PATCH v3] " Himanshu Chauhan
  3 siblings, 1 reply; 9+ messages in thread
From: Himanshu Chauhan @ 2023-06-05 11:49 UTC (permalink / raw)
  To: qemu-riscv, qemu-devel; +Cc: Himanshu Chauhan

On an address match, skip checking for default permissions and return error
based on access defined in PMP configuration.

v2 Changes:
o Removed goto to return in place when address matches
o Call pmp_hart_has_privs_default at the end of the loop

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

diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index 418738afd8..976b199156 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -291,7 +291,6 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
                         pmp_priv_t *allowed_privs, target_ulong mode)
 {
     int i = 0;
-    bool ret = false;
     int pmp_size = 0;
     target_ulong s = 0;
     target_ulong e = 0;
@@ -435,17 +434,12 @@ 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;
+            return (privs & *allowed_privs) == privs ? true : false;
         }
     }
 
     /* No rule matched */
-    if (!ret) {
-        ret = pmp_hart_has_privs_default(env, privs, allowed_privs, mode);
-    }
-
-    return ret;
+    return pmp_hart_has_privs_default(env, privs, allowed_privs, mode);
 }
 
 /*
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] target/riscv: Smepmp: Return error when access permission not allowed in PMP
  2023-06-05 11:49 ` [PATCH v2] " Himanshu Chauhan
@ 2023-06-05 12:17   ` Weiwei Li
  0 siblings, 0 replies; 9+ messages in thread
From: Weiwei Li @ 2023-06-05 12:17 UTC (permalink / raw)
  To: Himanshu Chauhan, qemu-riscv, qemu-devel; +Cc: liweiwei


On 2023/6/5 19:49, Himanshu Chauhan wrote:
> On an address match, skip checking for default permissions and return error
> based on access defined in PMP configuration.
>
> v2 Changes:
> o Removed goto to return in place when address matches
> o Call pmp_hart_has_privs_default at the end of the loop
>
> Fixes: 90b1fafce06 ("target/riscv: Smepmp: Skip applying default rules when address matches")
> Signed-off-by: Himanshu Chauhan <hchauhan@ventanamicro.com>
> ---
>   target/riscv/pmp.c | 10 ++--------
>   1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 418738afd8..976b199156 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -291,7 +291,6 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
>                           pmp_priv_t *allowed_privs, target_ulong mode)
>   {
>       int i = 0;
> -    bool ret = false;
>       int pmp_size = 0;
>       target_ulong s = 0;
>       target_ulong e = 0;
> @@ -435,17 +434,12 @@ 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;
> +            return (privs & *allowed_privs) == privs ? true : false;

This conditional assignment is unnecessary.

+                                  return  (privs & *allowed_privs) == 
privs;

Otherwise, Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>

Weiwei Li

>           }
>       }
>   
>       /* No rule matched */
> -    if (!ret) {
> -        ret = pmp_hart_has_privs_default(env, privs, allowed_privs, mode);
> -    }
> -
> -    return ret;
> +    return pmp_hart_has_privs_default(env, privs, allowed_privs, mode);
>   }
>   
>   /*



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] target/riscv: Smepmp: Return error when access permission not allowed in PMP
  2023-06-05  7:51 [PATCH] target/riscv: Smepmp: Return error when access permission not allowed in PMP Himanshu Chauhan
  2023-06-05  9:16 ` Weiwei Li
  2023-06-05 11:49 ` [PATCH v2] " Himanshu Chauhan
@ 2023-06-05 15:12 ` Richard Henderson
  2023-06-05 16:45 ` [PATCH v3] " Himanshu Chauhan
  3 siblings, 0 replies; 9+ messages in thread
From: Richard Henderson @ 2023-06-05 15:12 UTC (permalink / raw)
  To: Himanshu Chauhan, qemu-riscv, qemu-devel

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~


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3] target/riscv: Smepmp: Return error when access permission not allowed in PMP
  2023-06-05  7:51 [PATCH] target/riscv: Smepmp: Return error when access permission not allowed in PMP Himanshu Chauhan
                   ` (2 preceding siblings ...)
  2023-06-05 15:12 ` [PATCH] " Richard Henderson
@ 2023-06-05 16:45 ` Himanshu Chauhan
  2023-06-06  1:01   ` Weiwei Li
                     ` (2 more replies)
  3 siblings, 3 replies; 9+ messages in thread
From: Himanshu Chauhan @ 2023-06-05 16:45 UTC (permalink / raw)
  To: qemu-riscv, qemu-devel; +Cc: Himanshu Chauhan

On an address match, skip checking for default permissions and return error
based on access defined in PMP configuration.

v3 Changes:
o Removed explicit return of boolean value from comparision
  of priv/allowed_priv

v2 Changes:
o Removed goto to return in place when address matches
o Call pmp_hart_has_privs_default at the end of the loop

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

diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index 418738afd8..9d8db493e6 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -291,7 +291,6 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
                         pmp_priv_t *allowed_privs, target_ulong mode)
 {
     int i = 0;
-    bool ret = false;
     int pmp_size = 0;
     target_ulong s = 0;
     target_ulong e = 0;
@@ -435,17 +434,12 @@ 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;
+            return (privs & *allowed_privs) == privs;
         }
     }
 
     /* No rule matched */
-    if (!ret) {
-        ret = pmp_hart_has_privs_default(env, privs, allowed_privs, mode);
-    }
-
-    return ret;
+    return pmp_hart_has_privs_default(env, privs, allowed_privs, mode);
 }
 
 /*
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] target/riscv: Smepmp: Return error when access permission not allowed in PMP
  2023-06-05 16:45 ` [PATCH v3] " Himanshu Chauhan
@ 2023-06-06  1:01   ` Weiwei Li
  2023-06-06 12:53   ` Daniel Henrique Barboza
  2023-06-12  3:50   ` Alistair Francis
  2 siblings, 0 replies; 9+ messages in thread
From: Weiwei Li @ 2023-06-06  1:01 UTC (permalink / raw)
  To: Himanshu Chauhan, qemu-riscv, qemu-devel; +Cc: liweiwei


On 2023/6/6 00:45, Himanshu Chauhan wrote:
> On an address match, skip checking for default permissions and return error
> based on access defined in PMP configuration.
>
> v3 Changes:
> o Removed explicit return of boolean value from comparision
>    of priv/allowed_priv
>
> v2 Changes:
> o Removed goto to return in place when address matches
> o Call pmp_hart_has_privs_default at the end of the loop
>
> Fixes: 90b1fafce06 ("target/riscv: Smepmp: Skip applying default rules when address matches")
> Signed-off-by: Himanshu Chauhan <hchauhan@ventanamicro.com>
> ---

Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>

Weiwei Li
>   target/riscv/pmp.c | 10 ++--------
>   1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 418738afd8..9d8db493e6 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -291,7 +291,6 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
>                           pmp_priv_t *allowed_privs, target_ulong mode)
>   {
>       int i = 0;
> -    bool ret = false;
>       int pmp_size = 0;
>       target_ulong s = 0;
>       target_ulong e = 0;
> @@ -435,17 +434,12 @@ 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;
> +            return (privs & *allowed_privs) == privs;
>           }
>       }
>   
>       /* No rule matched */
> -    if (!ret) {
> -        ret = pmp_hart_has_privs_default(env, privs, allowed_privs, mode);
> -    }
> -
> -    return ret;
> +    return pmp_hart_has_privs_default(env, privs, allowed_privs, mode);
>   }
>   
>   /*



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] target/riscv: Smepmp: Return error when access permission not allowed in PMP
  2023-06-05 16:45 ` [PATCH v3] " Himanshu Chauhan
  2023-06-06  1:01   ` Weiwei Li
@ 2023-06-06 12:53   ` Daniel Henrique Barboza
  2023-06-12  3:50   ` Alistair Francis
  2 siblings, 0 replies; 9+ messages in thread
From: Daniel Henrique Barboza @ 2023-06-06 12:53 UTC (permalink / raw)
  To: Himanshu Chauhan, qemu-riscv, qemu-devel



On 6/5/23 13:45, Himanshu Chauhan wrote:
> On an address match, skip checking for default permissions and return error
> based on access defined in PMP configuration.
> 
> v3 Changes:
> o Removed explicit return of boolean value from comparision
>    of priv/allowed_priv
> 
> v2 Changes:
> o Removed goto to return in place when address matches
> o Call pmp_hart_has_privs_default at the end of the loop
> 
> Fixes: 90b1fafce06 ("target/riscv: Smepmp: Skip applying default rules when address matches")
> Signed-off-by: Himanshu Chauhan <hchauhan@ventanamicro.com>
> ---

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

>   target/riscv/pmp.c | 10 ++--------
>   1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 418738afd8..9d8db493e6 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -291,7 +291,6 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
>                           pmp_priv_t *allowed_privs, target_ulong mode)
>   {
>       int i = 0;
> -    bool ret = false;
>       int pmp_size = 0;
>       target_ulong s = 0;
>       target_ulong e = 0;
> @@ -435,17 +434,12 @@ 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;
> +            return (privs & *allowed_privs) == privs;
>           }
>       }
>   
>       /* No rule matched */
> -    if (!ret) {
> -        ret = pmp_hart_has_privs_default(env, privs, allowed_privs, mode);
> -    }
> -
> -    return ret;
> +    return pmp_hart_has_privs_default(env, privs, allowed_privs, mode);
>   }
>   
>   /*


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] target/riscv: Smepmp: Return error when access permission not allowed in PMP
  2023-06-05 16:45 ` [PATCH v3] " Himanshu Chauhan
  2023-06-06  1:01   ` Weiwei Li
  2023-06-06 12:53   ` Daniel Henrique Barboza
@ 2023-06-12  3:50   ` Alistair Francis
  2 siblings, 0 replies; 9+ messages in thread
From: Alistair Francis @ 2023-06-12  3:50 UTC (permalink / raw)
  To: Himanshu Chauhan; +Cc: qemu-riscv, qemu-devel

On Tue, Jun 6, 2023 at 2:47 AM Himanshu Chauhan
<hchauhan@ventanamicro.com> wrote:
>
> On an address match, skip checking for default permissions and return error
> based on access defined in PMP configuration.
>
> v3 Changes:
> o Removed explicit return of boolean value from comparision
>   of priv/allowed_priv
>
> v2 Changes:
> o Removed goto to return in place when address matches
> o Call pmp_hart_has_privs_default at the end of the loop
>
> Fixes: 90b1fafce06 ("target/riscv: Smepmp: Skip applying default rules when address matches")
> Signed-off-by: Himanshu Chauhan <hchauhan@ventanamicro.com>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  target/riscv/pmp.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 418738afd8..9d8db493e6 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -291,7 +291,6 @@ bool pmp_hart_has_privs(CPURISCVState *env, target_ulong addr,
>                          pmp_priv_t *allowed_privs, target_ulong mode)
>  {
>      int i = 0;
> -    bool ret = false;
>      int pmp_size = 0;
>      target_ulong s = 0;
>      target_ulong e = 0;
> @@ -435,17 +434,12 @@ 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;
> +            return (privs & *allowed_privs) == privs;
>          }
>      }
>
>      /* No rule matched */
> -    if (!ret) {
> -        ret = pmp_hart_has_privs_default(env, privs, allowed_privs, mode);
> -    }
> -
> -    return ret;
> +    return pmp_hart_has_privs_default(env, privs, allowed_privs, mode);
>  }
>
>  /*
> --
> 2.34.1
>
>


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2023-06-12  3:51 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-05  7:51 [PATCH] target/riscv: Smepmp: Return error when access permission not allowed in PMP Himanshu Chauhan
2023-06-05  9:16 ` Weiwei Li
2023-06-05 11:49 ` [PATCH v2] " Himanshu Chauhan
2023-06-05 12:17   ` Weiwei Li
2023-06-05 15:12 ` [PATCH] " Richard Henderson
2023-06-05 16:45 ` [PATCH v3] " Himanshu Chauhan
2023-06-06  1:01   ` Weiwei Li
2023-06-06 12:53   ` Daniel Henrique Barboza
2023-06-12  3:50   ` Alistair Francis

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).