* [PATCH v2 0/2] Fix some PMP implementation @ 2020-07-21 6:03 Zong Li 2020-07-21 6:03 ` Zong Li 2020-07-21 6:03 ` [PATCH v2 2/2] target/riscv/pmp.c: Fix the index offset on RV64 Zong Li 0 siblings, 2 replies; 6+ messages in thread From: Zong Li @ 2020-07-21 6:03 UTC (permalink / raw) To: palmer, Alistair.Francis, bmeng.cn, sagark, kbastian, qemu-riscv, qemu-devel Cc: Zong Li This patch set contains the fixes for wrong index of pmpcfg CSR on rv64, and the pmp range in CSR function table. Changed in v2: - Move out the shifting operation from loop. Suggested by Bin Meng. Zong Li (2): target/riscv: Fix the range of pmpcfg of CSR funcion table target/riscv/pmp.c: Fix the index offset on RV64 target/riscv/csr.c | 2 +- target/riscv/pmp.c | 19 ++++++++++++++++--- 2 files changed, 17 insertions(+), 4 deletions(-) -- 2.27.0 ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] target/riscv: Fix the range of pmpcfg of CSR funcion table 2020-07-21 6:03 [PATCH v2 0/2] Fix some PMP implementation Zong Li @ 2020-07-21 6:03 ` Zong Li 2020-07-21 6:03 ` [PATCH v2 2/2] target/riscv/pmp.c: Fix the index offset on RV64 Zong Li 1 sibling, 0 replies; 6+ messages in thread From: Zong Li @ 2020-07-21 6:03 UTC (permalink / raw) To: palmer, Alistair.Francis, bmeng.cn, sagark, kbastian, qemu-riscv, qemu-devel Cc: Bin Meng, Alistair Francis, Zong Li The range of Physical Memory Protection should be from CSR_PMPCFG0 to CSR_PMPCFG3, not to CSR_PMPADDR9. Signed-off-by: Zong Li <zong.li@sifive.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Reviewed-by: Bin Meng <bin.meng@windriver.com> --- target/riscv/csr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/riscv/csr.c b/target/riscv/csr.c index ac01c835e1..6a96a01b1c 100644 --- a/target/riscv/csr.c +++ b/target/riscv/csr.c @@ -1353,7 +1353,7 @@ static riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = { [CSR_MTINST] = { hmode, read_mtinst, write_mtinst }, /* Physical Memory Protection */ - [CSR_PMPCFG0 ... CSR_PMPADDR9] = { pmp, read_pmpcfg, write_pmpcfg }, + [CSR_PMPCFG0 ... CSR_PMPCFG3] = { pmp, read_pmpcfg, write_pmpcfg }, [CSR_PMPADDR0 ... CSR_PMPADDR15] = { pmp, read_pmpaddr, write_pmpaddr }, /* Performance Counters */ -- 2.27.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 1/2] target/riscv: Fix the range of pmpcfg of CSR funcion table @ 2020-07-21 6:03 ` Zong Li 0 siblings, 0 replies; 6+ messages in thread From: Zong Li @ 2020-07-21 6:03 UTC (permalink / raw) To: palmer, Alistair.Francis, bmeng.cn, sagark, kbastian, qemu-riscv, qemu-devel Cc: Zong Li, Alistair Francis, Bin Meng The range of Physical Memory Protection should be from CSR_PMPCFG0 to CSR_PMPCFG3, not to CSR_PMPADDR9. Signed-off-by: Zong Li <zong.li@sifive.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Reviewed-by: Bin Meng <bin.meng@windriver.com> --- target/riscv/csr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/riscv/csr.c b/target/riscv/csr.c index ac01c835e1..6a96a01b1c 100644 --- a/target/riscv/csr.c +++ b/target/riscv/csr.c @@ -1353,7 +1353,7 @@ static riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = { [CSR_MTINST] = { hmode, read_mtinst, write_mtinst }, /* Physical Memory Protection */ - [CSR_PMPCFG0 ... CSR_PMPADDR9] = { pmp, read_pmpcfg, write_pmpcfg }, + [CSR_PMPCFG0 ... CSR_PMPCFG3] = { pmp, read_pmpcfg, write_pmpcfg }, [CSR_PMPADDR0 ... CSR_PMPADDR15] = { pmp, read_pmpaddr, write_pmpaddr }, /* Performance Counters */ -- 2.27.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] target/riscv/pmp.c: Fix the index offset on RV64 2020-07-21 6:03 [PATCH v2 0/2] Fix some PMP implementation Zong Li 2020-07-21 6:03 ` Zong Li @ 2020-07-21 6:03 ` Zong Li 2020-07-21 6:11 ` Bin Meng 1 sibling, 1 reply; 6+ messages in thread From: Zong Li @ 2020-07-21 6:03 UTC (permalink / raw) To: palmer, Alistair.Francis, bmeng.cn, sagark, kbastian, qemu-riscv, qemu-devel Cc: Zong Li 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 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 + 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); -- 2.27.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] target/riscv/pmp.c: Fix the index offset on RV64 2020-07-21 6:03 ` [PATCH v2 2/2] target/riscv/pmp.c: Fix the index offset on RV64 Zong Li @ 2020-07-21 6:11 ` Bin Meng 0 siblings, 0 replies; 6+ messages in thread From: Bin Meng @ 2020-07-21 6:11 UTC (permalink / raw) To: Zong Li Cc: open list:RISC-V, Sagar Karandikar, Bastian Koppelmann, qemu-devel@nongnu.org Developers, Alistair Francis, Palmer Dabbelt 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] target/riscv/pmp.c: Fix the index offset on RV64 @ 2020-07-21 6:11 ` Bin Meng 0 siblings, 0 replies; 6+ messages in thread From: Bin Meng @ 2020-07-21 6:11 UTC (permalink / raw) To: Zong Li Cc: Palmer Dabbelt, Alistair Francis, Sagar Karandikar, Bastian Koppelmann, open list:RISC-V, qemu-devel@nongnu.org Developers 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-07-21 6:12 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-21 6:03 [PATCH v2 0/2] Fix some PMP implementation Zong Li 2020-07-21 6:03 ` [PATCH v2 1/2] target/riscv: Fix the range of pmpcfg of CSR funcion table Zong Li 2020-07-21 6:03 ` Zong Li 2020-07-21 6:03 ` [PATCH v2 2/2] target/riscv/pmp.c: Fix the index offset on RV64 Zong Li 2020-07-21 6:11 ` Bin Meng 2020-07-21 6:11 ` Bin Meng
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.