diff mbox series

[09/18] target/riscv: Avoid reporting odd-numbered pmpcfgX in the CSR XML for RV64

Message ID 20230213180215.1524938-10-bmeng@tinylab.org
State New
Headers show
Series target/riscv: Various fixes to gdbstub and CSR access | expand

Commit Message

Bin Meng Feb. 13, 2023, 6:02 p.m. UTC
At present the odd-numbered PMP configuration registers for RV64 are
reported in the CSR XML by QEMU gdbstub. However these registers do
not exist on RV64 so trying to access them from gdb results in 'E14'.

Move the pmpcfgX index check from the actual read/write routine to
the PMP CSR predicate() routine, so that non-existent pmpcfgX won't
be reported in the CSR XML for RV64.

Signed-off-by: Bin Meng <bmeng@tinylab.org>
---

 target/riscv/csr.c | 23 ++++++++---------------
 1 file changed, 8 insertions(+), 15 deletions(-)

Comments

Weiwei Li Feb. 14, 2023, 8:56 a.m. UTC | #1
On 2023/2/14 02:02, Bin Meng wrote:
> At present the odd-numbered PMP configuration registers for RV64 are
> reported in the CSR XML by QEMU gdbstub. However these registers do
> not exist on RV64 so trying to access them from gdb results in 'E14'.
>
> Move the pmpcfgX index check from the actual read/write routine to
> the PMP CSR predicate() routine, so that non-existent pmpcfgX won't
> be reported in the CSR XML for RV64.
>
> Signed-off-by: Bin Meng<bmeng@tinylab.org>
Reviewed-by: Weiwei Li <liweiwei@iscas.ac.cn>

Regards,
Weiwei Li
> ---
>
>   target/riscv/csr.c | 23 ++++++++---------------
>   1 file changed, 8 insertions(+), 15 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 0a3f2bef6f..749d0ef83e 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -412,6 +412,14 @@ static int aia_hmode32(CPURISCVState *env, int csrno)
>   static RISCVException pmp(CPURISCVState *env, int csrno)
>   {
>       if (riscv_feature(env, RISCV_FEATURE_PMP)) {
> +        if (csrno <= CSR_PMPCFG3) {
> +            uint32_t reg_index = csrno - CSR_PMPCFG0;
> +
> +            if ((reg_index & 1) && (riscv_cpu_mxl(env) == MXL_RV64)) {
> +                return RISCV_EXCP_ILLEGAL_INST;
> +            }
> +        }
> +
>           return RISCV_EXCP_NONE;
>       }
>   
> @@ -3334,23 +3342,11 @@ static RISCVException write_mseccfg(CPURISCVState *env, int csrno,
>       return RISCV_EXCP_NONE;
>   }
>   
> -static bool check_pmp_reg_index(CPURISCVState *env, uint32_t reg_index)
> -{
> -    /* TODO: RV128 restriction check */
> -    if ((reg_index & 1) && (riscv_cpu_mxl(env) == MXL_RV64)) {
> -        return false;
> -    }
> -    return true;
> -}
> -
>   static RISCVException read_pmpcfg(CPURISCVState *env, int csrno,
>                                     target_ulong *val)
>   {
>       uint32_t reg_index = csrno - CSR_PMPCFG0;
>   
> -    if (!check_pmp_reg_index(env, reg_index)) {
> -        return RISCV_EXCP_ILLEGAL_INST;
> -    }
>       *val = pmpcfg_csr_read(env, reg_index);
>       return RISCV_EXCP_NONE;
>   }
> @@ -3360,9 +3356,6 @@ static RISCVException write_pmpcfg(CPURISCVState *env, int csrno,
>   {
>       uint32_t reg_index = csrno - CSR_PMPCFG0;
>   
> -    if (!check_pmp_reg_index(env, reg_index)) {
> -        return RISCV_EXCP_ILLEGAL_INST;
> -    }
>       pmpcfg_csr_write(env, reg_index, val);
>       return RISCV_EXCP_NONE;
>   }
LIU Zhiwei Feb. 17, 2023, 2:36 a.m. UTC | #2
On 2023/2/14 2:02, Bin Meng wrote:
> At present the odd-numbered PMP configuration registers for RV64 are
> reported in the CSR XML by QEMU gdbstub. However these registers do
> not exist on RV64 so trying to access them from gdb results in 'E14'.
>
> Move the pmpcfgX index check from the actual read/write routine to
> the PMP CSR predicate() routine, so that non-existent pmpcfgX won't
> be reported in the CSR XML for RV64.
>
> Signed-off-by: Bin Meng <bmeng@tinylab.org>
> ---
>
>   target/riscv/csr.c | 23 ++++++++---------------
>   1 file changed, 8 insertions(+), 15 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 0a3f2bef6f..749d0ef83e 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -412,6 +412,14 @@ static int aia_hmode32(CPURISCVState *env, int csrno)
>   static RISCVException pmp(CPURISCVState *env, int csrno)
>   {
>       if (riscv_feature(env, RISCV_FEATURE_PMP)) {
> +        if (csrno <= CSR_PMPCFG3) {
> +            uint32_t reg_index = csrno - CSR_PMPCFG0;
> +
> +            if ((reg_index & 1) && (riscv_cpu_mxl(env) == MXL_RV64)) {
> +                return RISCV_EXCP_ILLEGAL_INST;
> +            }
> +        }
> +
>           return RISCV_EXCP_NONE;
>       }
>   
> @@ -3334,23 +3342,11 @@ static RISCVException write_mseccfg(CPURISCVState *env, int csrno,
>       return RISCV_EXCP_NONE;
>   }
>   
> -static bool check_pmp_reg_index(CPURISCVState *env, uint32_t reg_index)
> -{
> -    /* TODO: RV128 restriction check */

Should keep this comment. Otherwise,

Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>

Zhiwei

> -    if ((reg_index & 1) && (riscv_cpu_mxl(env) == MXL_RV64)) {
> -        return false;
> -    }
> -    return true;
> -}
> -
>   static RISCVException read_pmpcfg(CPURISCVState *env, int csrno,
>                                     target_ulong *val)
>   {
>       uint32_t reg_index = csrno - CSR_PMPCFG0;
>   
> -    if (!check_pmp_reg_index(env, reg_index)) {
> -        return RISCV_EXCP_ILLEGAL_INST;
> -    }
>       *val = pmpcfg_csr_read(env, reg_index);
>       return RISCV_EXCP_NONE;
>   }
> @@ -3360,9 +3356,6 @@ static RISCVException write_pmpcfg(CPURISCVState *env, int csrno,
>   {
>       uint32_t reg_index = csrno - CSR_PMPCFG0;
>   
> -    if (!check_pmp_reg_index(env, reg_index)) {
> -        return RISCV_EXCP_ILLEGAL_INST;
> -    }
>       pmpcfg_csr_write(env, reg_index, val);
>       return RISCV_EXCP_NONE;
>   }
diff mbox series

Patch

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 0a3f2bef6f..749d0ef83e 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -412,6 +412,14 @@  static int aia_hmode32(CPURISCVState *env, int csrno)
 static RISCVException pmp(CPURISCVState *env, int csrno)
 {
     if (riscv_feature(env, RISCV_FEATURE_PMP)) {
+        if (csrno <= CSR_PMPCFG3) {
+            uint32_t reg_index = csrno - CSR_PMPCFG0;
+
+            if ((reg_index & 1) && (riscv_cpu_mxl(env) == MXL_RV64)) {
+                return RISCV_EXCP_ILLEGAL_INST;
+            }
+        }
+
         return RISCV_EXCP_NONE;
     }
 
@@ -3334,23 +3342,11 @@  static RISCVException write_mseccfg(CPURISCVState *env, int csrno,
     return RISCV_EXCP_NONE;
 }
 
-static bool check_pmp_reg_index(CPURISCVState *env, uint32_t reg_index)
-{
-    /* TODO: RV128 restriction check */
-    if ((reg_index & 1) && (riscv_cpu_mxl(env) == MXL_RV64)) {
-        return false;
-    }
-    return true;
-}
-
 static RISCVException read_pmpcfg(CPURISCVState *env, int csrno,
                                   target_ulong *val)
 {
     uint32_t reg_index = csrno - CSR_PMPCFG0;
 
-    if (!check_pmp_reg_index(env, reg_index)) {
-        return RISCV_EXCP_ILLEGAL_INST;
-    }
     *val = pmpcfg_csr_read(env, reg_index);
     return RISCV_EXCP_NONE;
 }
@@ -3360,9 +3356,6 @@  static RISCVException write_pmpcfg(CPURISCVState *env, int csrno,
 {
     uint32_t reg_index = csrno - CSR_PMPCFG0;
 
-    if (!check_pmp_reg_index(env, reg_index)) {
-        return RISCV_EXCP_ILLEGAL_INST;
-    }
     pmpcfg_csr_write(env, reg_index, val);
     return RISCV_EXCP_NONE;
 }