target/riscv/pmp.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
pmpcfg_csr_{read,write} do not correctly handle accesses to PMP
configurations 8 through 15 (CSR pmpcfg2) on RV64.
The current code computes the pmpcfg index using:
(reg_index * sizeof(target_ulong))
This is incorrect on RV64. For example, when reg_index is 2 (i.e.,
pmpcfg2), the computed configuration index will be 16-23, which
should be 8-15.
A correct way is to use (reg_index * 4) instead, which works for
both RV32 and RV64.
Cc: Xi Wang <xi.wang@gmail.com>
Signed-off-by: Luke Nelson <luke.r.nels@gmail.com>
---
target/riscv/pmp.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index 15a5366616..a1bee56c86 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -311,9 +311,8 @@ void pmpcfg_csr_write(CPURISCVState *env, uint32_t reg_index,
}
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);
+ cfg_val = (val >> (i * 8)) & 0xff;
+ pmp_write_cfg(env, (reg_index * 4) + i, cfg_val);
}
}
@@ -328,7 +327,7 @@ target_ulong pmpcfg_csr_read(CPURISCVState *env, uint32_t reg_index)
target_ulong val = 0;
for (i = 0; i < sizeof(target_ulong); i++) {
- val = pmp_read_cfg(env, (reg_index * sizeof(target_ulong)) + i);
+ val = pmp_read_cfg(env, (reg_index * 4) + i);
cfg_val |= (val << (i * 8));
}
--
2.19.1
On Wed, Jan 30, 2019 at 2:20 PM Luke Nelson <luke.r.nels@gmail.com> wrote: > > pmpcfg_csr_{read,write} do not correctly handle accesses to PMP > configurations 8 through 15 (CSR pmpcfg2) on RV64. > > The current code computes the pmpcfg index using: > > (reg_index * sizeof(target_ulong)) > > This is incorrect on RV64. For example, when reg_index is 2 (i.e., > pmpcfg2), the computed configuration index will be 16-23, which > should be 8-15. > > A correct way is to use (reg_index * 4) instead, which works for > both RV32 and RV64. > > Cc: Xi Wang <xi.wang@gmail.com> > Signed-off-by: Luke Nelson <luke.r.nels@gmail.com> Good catch! Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > target/riscv/pmp.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c > index 15a5366616..a1bee56c86 100644 > --- a/target/riscv/pmp.c > +++ b/target/riscv/pmp.c > @@ -311,9 +311,8 @@ void pmpcfg_csr_write(CPURISCVState *env, uint32_t reg_index, > } > > 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); > + cfg_val = (val >> (i * 8)) & 0xff; > + pmp_write_cfg(env, (reg_index * 4) + i, cfg_val); > } > } > > @@ -328,7 +327,7 @@ target_ulong pmpcfg_csr_read(CPURISCVState *env, uint32_t reg_index) > target_ulong val = 0; > > for (i = 0; i < sizeof(target_ulong); i++) { > - val = pmp_read_cfg(env, (reg_index * sizeof(target_ulong)) + i); > + val = pmp_read_cfg(env, (reg_index * 4) + i); > cfg_val |= (val << (i * 8)); > } > > -- > 2.19.1 > >
On Fri, 08 Feb 2019 10:57:17 PST (-0800), alistair23@gmail.com wrote: > On Wed, Jan 30, 2019 at 2:20 PM Luke Nelson <luke.r.nels@gmail.com> wrote: >> >> pmpcfg_csr_{read,write} do not correctly handle accesses to PMP >> configurations 8 through 15 (CSR pmpcfg2) on RV64. >> >> The current code computes the pmpcfg index using: >> >> (reg_index * sizeof(target_ulong)) >> >> This is incorrect on RV64. For example, when reg_index is 2 (i.e., >> pmpcfg2), the computed configuration index will be 16-23, which >> should be 8-15. >> >> A correct way is to use (reg_index * 4) instead, which works for >> both RV32 and RV64. >> >> Cc: Xi Wang <xi.wang@gmail.com> >> Signed-off-by: Luke Nelson <luke.r.nels@gmail.com> > > Good catch! > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Ya, thanks -- that's a somewhat embarrassing bug, as someone else just fixed one on the line below :). I'll target this for my next PR. > > Alistair > >> --- >> target/riscv/pmp.c | 7 +++---- >> 1 file changed, 3 insertions(+), 4 deletions(-) >> >> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c >> index 15a5366616..a1bee56c86 100644 >> --- a/target/riscv/pmp.c >> +++ b/target/riscv/pmp.c >> @@ -311,9 +311,8 @@ void pmpcfg_csr_write(CPURISCVState *env, uint32_t reg_index, >> } >> >> 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); >> + cfg_val = (val >> (i * 8)) & 0xff; >> + pmp_write_cfg(env, (reg_index * 4) + i, cfg_val); >> } >> } >> >> @@ -328,7 +327,7 @@ target_ulong pmpcfg_csr_read(CPURISCVState *env, uint32_t reg_index) >> target_ulong val = 0; >> >> for (i = 0; i < sizeof(target_ulong); i++) { >> - val = pmp_read_cfg(env, (reg_index * sizeof(target_ulong)) + i); >> + val = pmp_read_cfg(env, (reg_index * 4) + i); >> cfg_val |= (val << (i * 8)); >> } >> >> -- >> 2.19.1 >> >>
On Wed, Feb 13, 2019 at 10:12 AM Palmer Dabbelt <palmer@sifive.com> wrote: > > On Fri, 08 Feb 2019 10:57:17 PST (-0800), alistair23@gmail.com wrote: > > > > Good catch! > > > > Reviewed-by: Alistair Francis <alistair.francis@wdc.com> > > Ya, thanks -- that's a somewhat embarrassing bug, as someone else just fixed > one on the line below :). I'll target this for my next PR. > Is there any chance this patch could make it in the next PR? Thanks, - Luke
© 2016 - 2024 Red Hat, Inc.