* [PATCH 0/2] Fix some PMP implementation
@ 2020-07-20 8:39 Zong Li
2020-07-20 8:39 ` [PATCH 1/2] target/riscv: Fix the range of pmpcfg of CSR funcion table Zong Li
2020-07-20 8:39 ` [PATCH 2/2] target/riscv/pmp.c: Fix the index offset on RV64 Zong Li
0 siblings, 2 replies; 7+ messages in thread
From: Zong Li @ 2020-07-20 8:39 UTC (permalink / raw)
To: palmer, Alistair.Francis, 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.
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 | 9 +++++++++
2 files changed, 10 insertions(+), 1 deletion(-)
--
2.27.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] target/riscv: Fix the range of pmpcfg of CSR funcion table
2020-07-20 8:39 [PATCH 0/2] Fix some PMP implementation Zong Li
@ 2020-07-20 8:39 ` Zong Li
2020-07-20 23:12 ` Alistair Francis
2020-07-21 2:41 ` Bin Meng
2020-07-20 8:39 ` [PATCH 2/2] target/riscv/pmp.c: Fix the index offset on RV64 Zong Li
1 sibling, 2 replies; 7+ messages in thread
From: Zong Li @ 2020-07-20 8:39 UTC (permalink / raw)
To: palmer, Alistair.Francis, sagark, kbastian, qemu-riscv, qemu-devel
Cc: 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>
---
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] 7+ messages in thread
* [PATCH 2/2] target/riscv/pmp.c: Fix the index offset on RV64
2020-07-20 8:39 [PATCH 0/2] Fix some PMP implementation Zong Li
2020-07-20 8:39 ` [PATCH 1/2] target/riscv: Fix the range of pmpcfg of CSR funcion table Zong Li
@ 2020-07-20 8:39 ` Zong Li
2020-07-21 2:41 ` Bin Meng
1 sibling, 1 reply; 7+ messages in thread
From: Zong Li @ 2020-07-20 8:39 UTC (permalink / raw)
To: palmer, Alistair.Francis, 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.
Signed-off-by: Zong Li <zong.li@sifive.com>
---
target/riscv/pmp.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index 2a2b9f5363..adcdd411e6 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -320,8 +320,13 @@ void pmpcfg_csr_write(CPURISCVState *env, uint32_t reg_index,
for (i = 0; i < sizeof(target_ulong); i++) {
cfg_val = (val >> 8 * i) & 0xff;
+#if defined(TARGET_RISCV32)
pmp_write_cfg(env, (reg_index * sizeof(target_ulong)) + i,
cfg_val);
+#elif defined(TARGET_RISCV64)
+ pmp_write_cfg(env, ((reg_index >> 1) * sizeof(target_ulong)) + i,
+ cfg_val);
+#endif
}
}
@@ -336,7 +341,11 @@ target_ulong pmpcfg_csr_read(CPURISCVState *env, uint32_t reg_index)
target_ulong val = 0;
for (i = 0; i < sizeof(target_ulong); i++) {
+#if defined(TARGET_RISCV32)
val = pmp_read_cfg(env, (reg_index * sizeof(target_ulong)) + i);
+#elif defined(TARGET_RISCV64)
+ val = pmp_read_cfg(env, ((reg_index >> 1) * sizeof(target_ulong)) + i);
+#endif
cfg_val |= (val << (i * 8));
}
trace_pmpcfg_csr_read(env->mhartid, reg_index, cfg_val);
--
2.27.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] target/riscv: Fix the range of pmpcfg of CSR funcion table
2020-07-20 8:39 ` [PATCH 1/2] target/riscv: Fix the range of pmpcfg of CSR funcion table Zong Li
@ 2020-07-20 23:12 ` Alistair Francis
2020-07-21 2:41 ` Bin Meng
1 sibling, 0 replies; 7+ messages in thread
From: Alistair Francis @ 2020-07-20 23:12 UTC (permalink / raw)
To: Zong Li
Cc: open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
qemu-devel@nongnu.org Developers, Alistair Francis,
Palmer Dabbelt
On Mon, Jul 20, 2020 at 2:48 AM Zong Li <zong.li@sifive.com> wrote:
>
> 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>
Alistair
> ---
> 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 [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] target/riscv/pmp.c: Fix the index offset on RV64
2020-07-20 8:39 ` [PATCH 2/2] target/riscv/pmp.c: Fix the index offset on RV64 Zong Li
@ 2020-07-21 2:41 ` Bin Meng
2020-07-21 2:46 ` Zong Li
0 siblings, 1 reply; 7+ messages in thread
From: Bin Meng @ 2020-07-21 2:41 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 Mon, Jul 20, 2020 at 5:46 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.
>
> Signed-off-by: Zong Li <zong.li@sifive.com>
> ---
> target/riscv/pmp.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 2a2b9f5363..adcdd411e6 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -320,8 +320,13 @@ void pmpcfg_csr_write(CPURISCVState *env, uint32_t reg_index,
>
> for (i = 0; i < sizeof(target_ulong); i++) {
> cfg_val = (val >> 8 * i) & 0xff;
> +#if defined(TARGET_RISCV32)
> pmp_write_cfg(env, (reg_index * sizeof(target_ulong)) + i,
> cfg_val);
> +#elif defined(TARGET_RISCV64)
> + pmp_write_cfg(env, ((reg_index >> 1) * sizeof(target_ulong)) + i,
> + cfg_val);
> +#endif
Can you please simplify this by shifting reg_index outside the for
loop for RV64?
> }
> }
>
> @@ -336,7 +341,11 @@ target_ulong pmpcfg_csr_read(CPURISCVState *env, uint32_t reg_index)
> target_ulong val = 0;
>
> for (i = 0; i < sizeof(target_ulong); i++) {
> +#if defined(TARGET_RISCV32)
> val = pmp_read_cfg(env, (reg_index * sizeof(target_ulong)) + i);
> +#elif defined(TARGET_RISCV64)
> + val = pmp_read_cfg(env, ((reg_index >> 1) * sizeof(target_ulong)) + i);
> +#endif
> cfg_val |= (val << (i * 8));
> }
> trace_pmpcfg_csr_read(env->mhartid, reg_index, cfg_val);
Regards,
Bin
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] target/riscv: Fix the range of pmpcfg of CSR funcion table
2020-07-20 8:39 ` [PATCH 1/2] target/riscv: Fix the range of pmpcfg of CSR funcion table Zong Li
2020-07-20 23:12 ` Alistair Francis
@ 2020-07-21 2:41 ` Bin Meng
1 sibling, 0 replies; 7+ messages in thread
From: Bin Meng @ 2020-07-21 2:41 UTC (permalink / raw)
To: Zong Li
Cc: open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
qemu-devel@nongnu.org Developers, Alistair Francis,
Palmer Dabbelt
On Mon, Jul 20, 2020 at 5:48 PM Zong Li <zong.li@sifive.com> wrote:
>
> 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>
> ---
> target/riscv/csr.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Reviewed-by: Bin Meng <bin.meng@windriver.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] target/riscv/pmp.c: Fix the index offset on RV64
2020-07-21 2:41 ` Bin Meng
@ 2020-07-21 2:46 ` Zong Li
0 siblings, 0 replies; 7+ messages in thread
From: Zong Li @ 2020-07-21 2:46 UTC (permalink / raw)
To: Bin Meng
Cc: open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
qemu-devel@nongnu.org Developers, Alistair Francis,
Palmer Dabbelt
On Tue, Jul 21, 2020 at 10:41 AM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Zong,
>
> On Mon, Jul 20, 2020 at 5:46 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.
> >
> > Signed-off-by: Zong Li <zong.li@sifive.com>
> > ---
> > target/riscv/pmp.c | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> > index 2a2b9f5363..adcdd411e6 100644
> > --- a/target/riscv/pmp.c
> > +++ b/target/riscv/pmp.c
> > @@ -320,8 +320,13 @@ void pmpcfg_csr_write(CPURISCVState *env, uint32_t reg_index,
> >
> > for (i = 0; i < sizeof(target_ulong); i++) {
> > cfg_val = (val >> 8 * i) & 0xff;
> > +#if defined(TARGET_RISCV32)
> > pmp_write_cfg(env, (reg_index * sizeof(target_ulong)) + i,
> > cfg_val);
> > +#elif defined(TARGET_RISCV64)
> > + pmp_write_cfg(env, ((reg_index >> 1) * sizeof(target_ulong)) + i,
> > + cfg_val);
> > +#endif
>
> Can you please simplify this by shifting reg_index outside the for
> loop for RV64?
>
OK, that would be great. Change it in the next version, thanks.
> > }
> > }
> >
> > @@ -336,7 +341,11 @@ target_ulong pmpcfg_csr_read(CPURISCVState *env, uint32_t reg_index)
> > target_ulong val = 0;
> >
> > for (i = 0; i < sizeof(target_ulong); i++) {
> > +#if defined(TARGET_RISCV32)
> > val = pmp_read_cfg(env, (reg_index * sizeof(target_ulong)) + i);
> > +#elif defined(TARGET_RISCV64)
> > + val = pmp_read_cfg(env, ((reg_index >> 1) * sizeof(target_ulong)) + i);
> > +#endif
> > cfg_val |= (val << (i * 8));
> > }
> > trace_pmpcfg_csr_read(env->mhartid, reg_index, cfg_val);
>
> Regards,
> Bin
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-07-21 2:47 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-20 8:39 [PATCH 0/2] Fix some PMP implementation Zong Li
2020-07-20 8:39 ` [PATCH 1/2] target/riscv: Fix the range of pmpcfg of CSR funcion table Zong Li
2020-07-20 23:12 ` Alistair Francis
2020-07-21 2:41 ` Bin Meng
2020-07-20 8:39 ` [PATCH 2/2] target/riscv/pmp.c: Fix the index offset on RV64 Zong Li
2020-07-21 2:41 ` Bin Meng
2020-07-21 2:46 ` Zong Li
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).