Hi Zong, On Fri, Jul 24, 2020 at 5:08 PM Zong Li <zong...@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. > > Signed-off-by: Zong Li <zong...@sifive.com> > --- > target/riscv/pmp.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c > index 2a2b9f5363..e0161d6aab 100644 > --- a/target/riscv/pmp.c > +++ b/target/riscv/pmp.c > @@ -310,6 +310,10 @@ void pmpcfg_csr_write(CPURISCVState *env, uint32_t > reg_index, > int i; > uint8_t cfg_val; > > +#if defined(TARGET_RISCV64) > + reg_index >>= 1; > +#endif > + > trace_pmpcfg_csr_write(env->mhartid, reg_index, val); > > if ((reg_index & 1) && (sizeof(target_ulong) == 8)) { > @@ -335,6 +339,10 @@ target_ulong pmpcfg_csr_read(CPURISCVState *env, > uint32_t reg_index) > target_ulong cfg_val = 0; > target_ulong val = 0; > > +#if defined(TARGET_RISCV64) > + reg_index >>= 1; > +#endif > + > for (i = 0; i < sizeof(target_ulong); i++) { > val = pmp_read_cfg(env, (reg_index * sizeof(target_ulong)) + i); > cfg_val |= (val << (i * 8)); > --
It seems you missed to address my review comments in v3? reg_index should be shifted after we call the trace function. Regards, Bin