* [Qemu-devel] [PATCH] RISC-V: Fix pmpcfg register indexing
@ 2019-01-30 21:03 Luke Nelson
2019-02-08 18:57 ` Alistair Francis
0 siblings, 1 reply; 4+ messages in thread
From: Luke Nelson @ 2019-01-30 21:03 UTC (permalink / raw)
To: qemu-riscv
Cc: lukenels, Luke Nelson, Xi Wang, Michael Clark, Palmer Dabbelt,
Alistair Francis, Sagar Karandikar, Bastian Koppelmann,
open list:All patches CC here
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
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH] RISC-V: Fix pmpcfg register indexing
2019-01-30 21:03 [Qemu-devel] [PATCH] RISC-V: Fix pmpcfg register indexing Luke Nelson
@ 2019-02-08 18:57 ` Alistair Francis
2019-02-13 18:12 ` Palmer Dabbelt
0 siblings, 1 reply; 4+ messages in thread
From: Alistair Francis @ 2019-02-08 18:57 UTC (permalink / raw)
To: Luke Nelson
Cc: open list:RISC-V, Sagar Karandikar, Bastian Koppelmann,
Palmer Dabbelt, open list:All patches CC here, lukenels,
Michael Clark, Alistair Francis, Xi Wang
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
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH] RISC-V: Fix pmpcfg register indexing
2019-02-08 18:57 ` Alistair Francis
@ 2019-02-13 18:12 ` Palmer Dabbelt
2019-06-29 19:44 ` Luke Nelson
0 siblings, 1 reply; 4+ messages in thread
From: Palmer Dabbelt @ 2019-02-13 18:12 UTC (permalink / raw)
To: alistair23
Cc: luke.r.nels, qemu-riscv, sagark, Bastian Koppelmann, qemu-devel,
lukenels, mjc, Alistair Francis, xi.wang
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
>>
>>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH] RISC-V: Fix pmpcfg register indexing
2019-02-13 18:12 ` Palmer Dabbelt
@ 2019-06-29 19:44 ` Luke Nelson
0 siblings, 0 replies; 4+ messages in thread
From: Luke Nelson @ 2019-06-29 19:44 UTC (permalink / raw)
To: Palmer Dabbelt
Cc: qemu-riscv, Sagar Karandikar, Bastian Koppelmann,
open list:All patches CC here, lukenels, Michael Clark,
Alistair Francis, alistair23, Xi Wang
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-06-29 19:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-01-30 21:03 [Qemu-devel] [PATCH] RISC-V: Fix pmpcfg register indexing Luke Nelson
2019-02-08 18:57 ` Alistair Francis
2019-02-13 18:12 ` Palmer Dabbelt
2019-06-29 19:44 ` Luke Nelson
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).