Re: [Qemu-devel] [PATCHv4 4/6] RISC-V: Check PMP during Page Table Walks

[Alistair Francis <alistair23@gmail.com>]

On Thu, May 30, 2019 at 6:52 AM Hesham Almatary
<Hesham.Almatary@cl.cam.ac.uk> wrote:
>
> The PMP should be checked when doing a page table walk, and report access
> fault exception if the to-be-read PTE failed the PMP check.
>
> Suggested-by: Jonathan Behrens <fintelia@gmail.com>
> Signed-off-by: Hesham Almatary <Hesham.Almatary@cl.cam.ac.uk>
> ---
>  target/riscv/cpu.h        |  1 +
>  target/riscv/cpu_helper.c | 10 +++++++++-
>  2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index c17184f4e4..ab3ba3f15a 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -94,6 +94,7 @@ enum {
>  #define PRIV_VERSION_1_09_1 0x00010901
>  #define PRIV_VERSION_1_10_0 0x00011000
>
> +#define TRANSLATE_PMP_FAIL 2
>  #define TRANSLATE_FAIL 1
>  #define TRANSLATE_SUCCESS 0
>  #define NB_MMU_MODES 4
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 5a1cd7cf96..00bc4f1712 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -211,6 +211,12 @@ restart:
>
>          /* check that physical address of PTE is legal */
>          target_ulong pte_addr = base + idx * ptesize;
> +
> +        if (riscv_feature(env, RISCV_FEATURE_PMP) &&
> +            !pmp_hart_has_privs(env, pte_addr, sizeof(target_ulong),
> +            1 << MMU_DATA_LOAD, PRV_S)) {

Shouldn't we be passing mode in here?

Alistair

> +            return TRANSLATE_PMP_FAIL;
> +        }
>  #if defined(TARGET_RISCV32)
>          target_ulong pte = ldl_phys(cs->as, pte_addr);
>  #elif defined(TARGET_RISCV64)
> @@ -413,8 +419,10 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
>          (ret == TRANSLATE_SUCCESS) &&
>          !pmp_hart_has_privs(env, pa, TARGET_PAGE_SIZE, 1 << access_type,
>          mode)) {
> +        ret = TRANSLATE_PMP_FAIL;
> +    }
> +    if (ret == TRANSLATE_PMP_FAIL) {
>          pmp_violation = true;
> -        ret = TRANSLATE_FAIL;
>      }
>      if (ret == TRANSLATE_SUCCESS) {
>          tlb_set_page(cs, address & TARGET_PAGE_MASK, pa & TARGET_PAGE_MASK,
> --
> 2.17.1
>
>