There is an overflow with the current code where a pmpaddr value of
0x1fffffff is decoded as sa=0 and ea=0 whereas it should be sa=0 and
ea=0xffffffff.
Fix that by simplifying the computation. There is in fact no need for
ctz64() nor special case for -1 to achieve proper results.
Signed-off-by: Nicolas Pitre <nico@fluxnic.net>
---
This is in fact the same patch I posted yesterday but turns out its
scope is far more important than I initially thought.
diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index 81b61bb65c..151da3fa08 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -141,17 +141,9 @@ static void pmp_decode_napot(target_ulong a, target_ulong *sa, target_ulong *ea)
0111...1111 2^(XLEN+2)-byte NAPOT range
1111...1111 Reserved
*/
- if (a == -1) {
- *sa = 0u;
- *ea = -1;
- return;
- } else {
- target_ulong t1 = ctz64(~a);
- target_ulong base = (a & ~(((target_ulong)1 << t1) - 1)) << 2;
- target_ulong range = ((target_ulong)1 << (t1 + 3)) - 1;
- *sa = base;
- *ea = base + range;
- }
+ a = (a << 2) | 0x3;
+ *sa = a & (a + 1);
+ *ea = a | (a + 1);
}
void pmp_update_rule_addr(CPURISCVState *env, uint32_t pmp_index)
On Sat, Apr 9, 2022 at 2:25 AM Nicolas Pitre <nico@fluxnic.net> wrote: > > There is an overflow with the current code where a pmpaddr value of > 0x1fffffff is decoded as sa=0 and ea=0 whereas it should be sa=0 and > ea=0xffffffff. > > Fix that by simplifying the computation. There is in fact no need for > ctz64() nor special case for -1 to achieve proper results. > > Signed-off-by: Nicolas Pitre <nico@fluxnic.net> Thanks! Applied to riscv-to-apply.next Alistair > --- > > This is in fact the same patch I posted yesterday but turns out its > scope is far more important than I initially thought. > > diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c > index 81b61bb65c..151da3fa08 100644 > --- a/target/riscv/pmp.c > +++ b/target/riscv/pmp.c > @@ -141,17 +141,9 @@ static void pmp_decode_napot(target_ulong a, target_ulong *sa, target_ulong *ea) > 0111...1111 2^(XLEN+2)-byte NAPOT range > 1111...1111 Reserved > */ > - if (a == -1) { > - *sa = 0u; > - *ea = -1; > - return; > - } else { > - target_ulong t1 = ctz64(~a); > - target_ulong base = (a & ~(((target_ulong)1 << t1) - 1)) << 2; > - target_ulong range = ((target_ulong)1 << (t1 + 3)) - 1; > - *sa = base; > - *ea = base + range; > - } > + a = (a << 2) | 0x3; > + *sa = a & (a + 1); > + *ea = a | (a + 1); > } > > void pmp_update_rule_addr(CPURISCVState *env, uint32_t pmp_index) >
On Sat, Apr 9, 2022 at 2:25 AM Nicolas Pitre <nico@fluxnic.net> wrote: > > There is an overflow with the current code where a pmpaddr value of > 0x1fffffff is decoded as sa=0 and ea=0 whereas it should be sa=0 and > ea=0xffffffff. > > Fix that by simplifying the computation. There is in fact no need for > ctz64() nor special case for -1 to achieve proper results. > > Signed-off-by: Nicolas Pitre <nico@fluxnic.net> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > > This is in fact the same patch I posted yesterday but turns out its > scope is far more important than I initially thought. > > diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c > index 81b61bb65c..151da3fa08 100644 > --- a/target/riscv/pmp.c > +++ b/target/riscv/pmp.c > @@ -141,17 +141,9 @@ static void pmp_decode_napot(target_ulong a, target_ulong *sa, target_ulong *ea) > 0111...1111 2^(XLEN+2)-byte NAPOT range > 1111...1111 Reserved > */ > - if (a == -1) { > - *sa = 0u; > - *ea = -1; > - return; > - } else { > - target_ulong t1 = ctz64(~a); > - target_ulong base = (a & ~(((target_ulong)1 << t1) - 1)) << 2; > - target_ulong range = ((target_ulong)1 << (t1 + 3)) - 1; > - *sa = base; > - *ea = base + range; > - } > + a = (a << 2) | 0x3; > + *sa = a & (a + 1); > + *ea = a | (a + 1); > } > > void pmp_update_rule_addr(CPURISCVState *env, uint32_t pmp_index) >
© 2016 - 2024 Red Hat, Inc.