target/riscv: fix counter-enable checks in ctr()

[Qemu-devel] [PATCH] target/riscv: fix counter-enable checks in ctr()

Xi Wang posted 1 patch 6 years, 9 months ago

Download mbox

Test docker-mingw@fedora passed

Test asan passed

Test checkpatch passed

Test docker-clang@ubuntu failed

Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20190126230256.10983-1-xi.wang@gmail.com

Maintainers: Palmer Dabbelt <palmer@sifive.com>, Michael Clark <mjc@sifive.com>, Bastian Koppelmann <kbastian@mail.uni-paderborn.de>, Alistair Francis <Alistair.Francis@wdc.com>, Sagar Karandikar <sagark@eecs.berkeley.edu>

target/riscv/csr.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)

Expand all Fold all

[Qemu-devel] [PATCH] target/riscv: fix counter-enable checks in ctr()

Posted by Xi Wang 6 years, 9 months ago

Access to a counter in U-mode is permitted only if the corresponding
bit is set in both mcounteren and scounteren.  The current code
ignores mcounteren and checks scounteren only for U-mode access.

Signed-off-by: Xi Wang <xi.wang@gmail.com>
---
 target/riscv/csr.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 5e7e7d16b8..133f2ff7e2 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -56,9 +56,15 @@ static int fs(CPURISCVState *env, int csrno)
 static int ctr(CPURISCVState *env, int csrno)
 {
 #if !defined(CONFIG_USER_ONLY)
-    target_ulong ctr_en = env->priv == PRV_U ? env->scounteren :
-                          env->priv == PRV_S ? env->mcounteren : -1U;
-    if (!(ctr_en & (1 << (csrno & 31)))) {
+    uint32_t ctr_en = ~0u;
+
+    if (env->priv < PRV_M) {
+        ctr_en &= env->mcounteren;
+    }
+    if (env->priv < PRV_S) {
+        ctr_en &= env->scounteren;
+    }
+    if (!(ctr_en & (1u << (csrno & 31)))) {
         return -1;
     }
 #endif
-- 
2.19.1

Re: [Qemu-devel] [PATCH] target/riscv: fix counter-enable checks in ctr()

Posted by Palmer Dabbelt 6 years, 9 months ago

On Sat, 26 Jan 2019 15:02:56 PST (-0800), xi.wang@gmail.com wrote:
> Access to a counter in U-mode is permitted only if the corresponding
> bit is set in both mcounteren and scounteren.  The current code
> ignores mcounteren and checks scounteren only for U-mode access.
>
> Signed-off-by: Xi Wang <xi.wang@gmail.com>
> ---
>  target/riscv/csr.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 5e7e7d16b8..133f2ff7e2 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -56,9 +56,15 @@ static int fs(CPURISCVState *env, int csrno)
>  static int ctr(CPURISCVState *env, int csrno)
>  {
>  #if !defined(CONFIG_USER_ONLY)
> -    target_ulong ctr_en = env->priv == PRV_U ? env->scounteren :
> -                          env->priv == PRV_S ? env->mcounteren : -1U;
> -    if (!(ctr_en & (1 << (csrno & 31)))) {
> +    uint32_t ctr_en = ~0u;
> +
> +    if (env->priv < PRV_M) {
> +        ctr_en &= env->mcounteren;
> +    }
> +    if (env->priv < PRV_S) {
> +        ctr_en &= env->scounteren;
> +    }
> +    if (!(ctr_en & (1u << (csrno & 31)))) {
>          return -1;
>      }
>  #endif

Reviewed-by: Palmer Dabbelt <palmer@sifive.com>

I'll add this to my next PR.