Re: [Qemu-devel] [Qemu-riscv] [PATCH 1/2] RISC-V: Raise access fault exceptions on PMP violations

[Jonathan Behrens <fintelia@gmail.com>]

This patch assumes that translation failure should always raise a paging
fault, but it should be possible for it to raise an access fault as well
(since according to the spec "PMP  checks  are  also  applied  to
page-table  accesses  for  virtual-address translation, for which the
effective privilege mode is S."). I think the code to actually do the PMP
checking during page table walks is currently unimplemented though...

Jonathan

On Sat, May 18, 2019 at 3:14 PM Hesham Almatary <
Hesham.Almatary@cl.cam.ac.uk> wrote:

> Section 3.6 in RISC-V v1.10 privilege specification states that PMP
> violations
> report "access exceptions." The current PMP implementation has
> a bug which wrongly reports "page exceptions" on PMP violations.
>
> This patch fixes this bug by reporting the correct PMP access exceptions
> trap values.
>
> Signed-off-by: Hesham Almatary <hesham.almatary@cl.cam.ac.uk>
> ---
>  target/riscv/cpu_helper.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 41d6db41c3..b48de36114 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -318,12 +318,13 @@ restart:
>  }
>
>  static void raise_mmu_exception(CPURISCVState *env, target_ulong address,
> -                                MMUAccessType access_type)
> +                                MMUAccessType access_type, bool
> pmp_violation)
>  {
>      CPUState *cs = CPU(riscv_env_get_cpu(env));
>      int page_fault_exceptions =
>          (env->priv_ver >= PRIV_VERSION_1_10_0) &&
> -        get_field(env->satp, SATP_MODE) != VM_1_10_MBARE;
> +        get_field(env->satp, SATP_MODE) != VM_1_10_MBARE &&
> +        !pmp_violation;
>      switch (access_type) {
>      case MMU_INST_FETCH:
>          cs->exception_index = page_fault_exceptions ?
> @@ -389,6 +390,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address,
> int size,
>      CPURISCVState *env = &cpu->env;
>      hwaddr pa = 0;
>      int prot;
> +    bool pmp_violation = false;
>      int ret = TRANSLATE_FAIL;
>
>      qemu_log_mask(CPU_LOG_MMU, "%s ad %" VADDR_PRIx " rw %d mmu_idx %d\n",
> @@ -402,6 +404,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address,
> int size,
>
>      if (riscv_feature(env, RISCV_FEATURE_PMP) &&
>          !pmp_hart_has_privs(env, pa, TARGET_PAGE_SIZE, 1 << access_type))
> {
> +        pmp_violation = true;
>          ret = TRANSLATE_FAIL;
>      }
>      if (ret == TRANSLATE_SUCCESS) {
> @@ -411,7 +414,7 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address,
> int size,
>      } else if (probe) {
>          return false;
>      } else {
> -        raise_mmu_exception(env, address, access_type);
> +        raise_mmu_exception(env, address, access_type, pmp_violation);
>          riscv_raise_exception(env, cs->exception_index, retaddr);
>      }
>  #else
> --
> 2.17.1
>
>
>