diff mbox series

[v1] target/riscv: fix pmp implementation

Message ID 20200706084550.24117-1-amergnat@baylibre.com
State New
Headers show
Series [v1] target/riscv: fix pmp implementation | expand

Commit Message

Alexandre Mergnat July 6, 2020, 8:45 a.m. UTC
The end address calculation for NA4 mode is wrong because the address
used isn't shifted.

That imply all NA4 setup are not applied by the PMP.

The solution is to use the shifted address calculated for start address
variable.

Modifications are tested on Zephyr OS userspace test suite which works
for other RISC-V boards (E31 and E34 core).

Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com>
---
 target/riscv/pmp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Alistair Francis July 10, 2020, 8:25 p.m. UTC | #1
On Mon, Jul 6, 2020 at 2:45 AM Alexandre Mergnat <amergnat@baylibre.com> wrote:
>
> The end address calculation for NA4 mode is wrong because the address
> used isn't shifted.
>
> That imply all NA4 setup are not applied by the PMP.

I'm not sure what you mean here, can you clarify this?

>
> The solution is to use the shifted address calculated for start address
> variable.
>
> Modifications are tested on Zephyr OS userspace test suite which works
> for other RISC-V boards (E31 and E34 core).
>
> Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com>

Otherwise:

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/pmp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index 9418660f1b..2a2b9f5363 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -171,7 +171,7 @@ static void pmp_update_rule(CPURISCVState *env, uint32_t pmp_index)
>
>      case PMP_AMATCH_NA4:
>          sa = this_addr << 2; /* shift up from [xx:0] to [xx+2:2] */
> -        ea = (this_addr + 4u) - 1u;
> +        ea = (sa + 4u) - 1u;
>          break;
>
>      case PMP_AMATCH_NAPOT:
> --
> 2.17.1
>
>
Alexandre Mergnat July 13, 2020, 10:10 a.m. UTC | #2
Le ven. 10 juil. 2020 à 22:35, Alistair Francis <alistair23@gmail.com> a écrit :
>
> On Mon, Jul 6, 2020 at 2:45 AM Alexandre Mergnat <amergnat@baylibre.com> wrote:
> >
> > The end address calculation for NA4 mode is wrong because the address
> > used isn't shifted.
> >
> > That imply all NA4 setup are not applied by the PMP.
>
> I'm not sure what you mean here, can you clarify this?

I'm just saying NA4 configuration doesn't work properly on QEMU (It
doesn't watch 4byte but a huge range)
because the end address calculation is wrong.

>
> >
> > The solution is to use the shifted address calculated for start address
> > variable.
> >
> > Modifications are tested on Zephyr OS userspace test suite which works
> > for other RISC-V boards (E31 and E34 core).
> >
> > Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com>
>
> Otherwise:
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>
> Alistair
>
> > ---
> >  target/riscv/pmp.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> > index 9418660f1b..2a2b9f5363 100644
> > --- a/target/riscv/pmp.c
> > +++ b/target/riscv/pmp.c
> > @@ -171,7 +171,7 @@ static void pmp_update_rule(CPURISCVState *env, uint32_t pmp_index)
> >
> >      case PMP_AMATCH_NA4:
> >          sa = this_addr << 2; /* shift up from [xx:0] to [xx+2:2] */
> > -        ea = (this_addr + 4u) - 1u;
> > +        ea = (sa + 4u) - 1u;
> >          break;
> >
> >      case PMP_AMATCH_NAPOT:
> > --
> > 2.17.1
> >
> >
Alistair Francis July 13, 2020, 7:34 p.m. UTC | #3
On Mon, Jul 13, 2020 at 3:10 AM Alexandre Mergnat <amergnat@baylibre.com> wrote:
>
> Le ven. 10 juil. 2020 à 22:35, Alistair Francis <alistair23@gmail.com> a écrit :
> >
> > On Mon, Jul 6, 2020 at 2:45 AM Alexandre Mergnat <amergnat@baylibre.com> wrote:
> > >
> > > The end address calculation for NA4 mode is wrong because the address
> > > used isn't shifted.
> > >
> > > That imply all NA4 setup are not applied by the PMP.
> >
> > I'm not sure what you mean here, can you clarify this?
>
> I'm just saying NA4 configuration doesn't work properly on QEMU (It
> doesn't watch 4byte but a huge range)
> because the end address calculation is wrong.

Ok, I replaced the original sentence with:

It doesn't watch 4 bytes but a huge range because the end address
calculation is wrong.

and changed the title to: target/riscv: Fix pmp NA4 implementation

and applied it to the RISC-V tree.

Alistair

>
> >
> > >
> > > The solution is to use the shifted address calculated for start address
> > > variable.
> > >
> > > Modifications are tested on Zephyr OS userspace test suite which works
> > > for other RISC-V boards (E31 and E34 core).
> > >
> > > Signed-off-by: Alexandre Mergnat <amergnat@baylibre.com>
> >
> > Otherwise:
> >
> > Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> >
> > Alistair
> >
> > > ---
> > >  target/riscv/pmp.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> > > index 9418660f1b..2a2b9f5363 100644
> > > --- a/target/riscv/pmp.c
> > > +++ b/target/riscv/pmp.c
> > > @@ -171,7 +171,7 @@ static void pmp_update_rule(CPURISCVState *env, uint32_t pmp_index)
> > >
> > >      case PMP_AMATCH_NA4:
> > >          sa = this_addr << 2; /* shift up from [xx:0] to [xx+2:2] */
> > > -        ea = (this_addr + 4u) - 1u;
> > > +        ea = (sa + 4u) - 1u;
> > >          break;
> > >
> > >      case PMP_AMATCH_NAPOT:
> > > --
> > > 2.17.1
> > >
> > >
diff mbox series

Patch

diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
index 9418660f1b..2a2b9f5363 100644
--- a/target/riscv/pmp.c
+++ b/target/riscv/pmp.c
@@ -171,7 +171,7 @@  static void pmp_update_rule(CPURISCVState *env, uint32_t pmp_index)
 
     case PMP_AMATCH_NA4:
         sa = this_addr << 2; /* shift up from [xx:0] to [xx+2:2] */
-        ea = (this_addr + 4u) - 1u;
+        ea = (sa + 4u) - 1u;
         break;
 
     case PMP_AMATCH_NAPOT: