On Tue, May 21, 2019 at 3:47 AM Hesham Almatary
<hesham.almat...@cl.cam.ac.uk> wrote:
>
> The current implementation returns 1 (PMP check success) if the address is in
> range even if the PMP entry is off. This is a bug.
>
> For example, if there is a PMP check in S-Mode which is in range, but its PMP
> entry is off, this will succeed, which it should not.
>
> The patch fixes this bug by only checking the PMP permissions if the address
> is
> in range and its corresponding PMP entry it not off. Otherwise, it will keep
> the ret = -1 which will be checked and handled correctly at the end of the
> function.
>
> Signed-off-by: Hesham Almatary <hesham.almat...@cl.cam.ac.uk>
Reviewed-by: Alistair Francis <alistair.fran...@wdc.com>
Alistair
> ---
> target/riscv/pmp.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index b11c4ae22f..8668f0dd7c 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -259,11 +259,12 @@ bool pmp_hart_has_privs(CPURISCVState *env,
> target_ulong addr,
> /* fully inside */
> const uint8_t a_field =
> pmp_get_a_field(env->pmp_state.pmp[i].cfg_reg);
> - if ((s + e) == 2) {
> - if (PMP_AMATCH_OFF == a_field) {
> - return 1;
> - }
>
> + /*
> + * If the PMP entry is not off and the address is in range, do the
> priv
> + * check
> + */
> + if (((s + e) == 2) && (PMP_AMATCH_OFF != a_field)) {
> allowed_privs = PMP_READ | PMP_WRITE | PMP_EXEC;
> if ((env->priv != PRV_M) || pmp_is_locked(env, i)) {
> allowed_privs &= env->pmp_state.pmp[i].cfg_reg;
> --
> 2.17.1
>
>
