qemu-riscv
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 2/2] target/riscv/pmp.c: Fix the index offset on RV64


From: Bin Meng
Subject: Re: [PATCH v2 2/2] target/riscv/pmp.c: Fix the index offset on RV64
Date: Tue, 21 Jul 2020 14:11:43 +0800

Hi Zong,

On Tue, Jul 21, 2020 at 2:03 PM Zong Li <zong.li@sifive.com> wrote:
>
> On RV64, the reg_index is 2 (pmpcfg2 CSR) after the seventh pmp
> entry, it is not 1 (pmpcfg1 CSR) like RV32. In the original
> implementation, the second parameter of pmp_write_cfg is
> "reg_index * sizeof(target_ulong)", and we get the the result
> which is started from 16 if reg_index is 2, but we expect that
> it should be started from 8. Separate the implementation for
> RV32 and RV64 respectively.
>
> Changed in v2:
>  - Move out the shifting operation from loop. Suggested by Bin Meng

The changelog should go after --- below

>
> Signed-off-by: Zong Li <zong.li@sifive.com>
> ---
>  target/riscv/pmp.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 2a2b9f5363..3de6535fbd 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -309,6 +309,7 @@ void pmpcfg_csr_write(CPURISCVState *env, uint32_t 
> reg_index,
>  {
>      int i;
>      uint8_t cfg_val;
> +    uint32_t pmp_entry_base;
>
>      trace_pmpcfg_csr_write(env->mhartid, reg_index, val);
>
> @@ -318,10 +319,15 @@ void pmpcfg_csr_write(CPURISCVState *env, uint32_t 
> reg_index,
>          return;
>      }
>
> +#if defined(TARGET_RISCV32)
> +    pmp_entry_base = (reg_index * sizeof(target_ulong));
> +#elif defined(TARGET_RISCV64)
> +    pmp_entry_base = (reg_index >> 1) * sizeof(target_ulong);
> +#endif

This is not necessary. You can simply do:

#if defined(TARGET_RISCV64)
    reg_index >>= 1;
#endif

> +
>      for (i = 0; i < sizeof(target_ulong); i++) {
>          cfg_val = (val >> 8 * i)  & 0xff;
> -        pmp_write_cfg(env, (reg_index * sizeof(target_ulong)) + i,
> -            cfg_val);
> +        pmp_write_cfg(env, pmp_entry_base + i, cfg_val);
>      }
>  }
>
> @@ -332,11 +338,18 @@ void pmpcfg_csr_write(CPURISCVState *env, uint32_t 
> reg_index,
>  target_ulong pmpcfg_csr_read(CPURISCVState *env, uint32_t reg_index)
>  {
>      int i;
> +    uint32_t pmp_entry_base;
>      target_ulong cfg_val = 0;
>      target_ulong val = 0;
>
> +#if defined(TARGET_RISCV32)
> +    pmp_entry_base = (reg_index * sizeof(target_ulong));
> +#elif defined(TARGET_RISCV64)
> +    pmp_entry_base = (reg_index >> 1) * sizeof(target_ulong);
> +#endif
> +
>      for (i = 0; i < sizeof(target_ulong); i++) {
> -        val = pmp_read_cfg(env, (reg_index * sizeof(target_ulong)) + i);
> +        val = pmp_read_cfg(env, pmp_entry_base + i);
>          cfg_val |= (val << (i * 8));
>      }
>      trace_pmpcfg_csr_read(env->mhartid, reg_index, cfg_val);

Regards,
Bin



reply via email to

[Prev in Thread] Current Thread [Next in Thread]