From: Daniel Henrique Barboza <dbarb...@ventanamicro.com> raise_mmu_exception(), as is today, is prioritizing guest page faults by checking first if virt_enabled && !first_stage, and then considering the regular inst/load/store faults.
There's no mention in the spec about guest page fault being a higher priority that PMP faults. In fact, privileged spec section 3.7.1 says: "Attempting to fetch an instruction from a PMP region that does not have execute permissions raises an instruction access-fault exception. Attempting to execute a load or load-reserved instruction which accesses a physical address within a PMP region without read permissions raises a load access-fault exception. Attempting to execute a store, store-conditional, or AMO instruction which accesses a physical address within a PMP region without write permissions raises a store access-fault exception." So, in fact, we're doing it wrong - PMP faults should always be thrown, regardless of also being a first or second stage fault. The way riscv_cpu_tlb_fill() and get_physical_address() work is adequate: a TRANSLATE_PMP_FAIL error is immediately reported and reflected in the 'pmp_violation' flag. What we need is to change raise_mmu_exception() to prioritize it. Reported-by: Joseph Chan <jc...@ventanamicro.com> Fixes: 82d53adfbb ("target/riscv/cpu_helper.c: Invalid exception on MMU translation stage") Signed-off-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com> Reviewed-by: Alistair Francis <alistair.fran...@wdc.com> Message-ID: <20240413105929.7030-1-alexei.filip...@syntacore.com> Cc: qemu-stable <qemu-sta...@nongnu.org> Signed-off-by: Alistair Francis <alistair.fran...@wdc.com> (cherry picked from commit 68e7c86927afa240fa450578cb3a4f18926153e4) Signed-off-by: Michael Tokarev <m...@tls.msk.ru> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c index da1a153abf..cf97c782dd 100644 --- a/target/riscv/cpu_helper.c +++ b/target/riscv/cpu_helper.c @@ -1162,28 +1162,30 @@ static void raise_mmu_exception(CPURISCVState *env, target_ulong address, switch (access_type) { case MMU_INST_FETCH: - if (env->virt_enabled && !first_stage) { + if (pmp_violation) { + cs->exception_index = RISCV_EXCP_INST_ACCESS_FAULT; + } else if (env->virt_enabled && !first_stage) { cs->exception_index = RISCV_EXCP_INST_GUEST_PAGE_FAULT; } else { - cs->exception_index = pmp_violation ? - RISCV_EXCP_INST_ACCESS_FAULT : RISCV_EXCP_INST_PAGE_FAULT; + cs->exception_index = RISCV_EXCP_INST_PAGE_FAULT; } break; case MMU_DATA_LOAD: - if (two_stage && !first_stage) { + if (pmp_violation) { + cs->exception_index = RISCV_EXCP_LOAD_ACCESS_FAULT; + } else if (two_stage && !first_stage) { cs->exception_index = RISCV_EXCP_LOAD_GUEST_ACCESS_FAULT; } else { - cs->exception_index = pmp_violation ? - RISCV_EXCP_LOAD_ACCESS_FAULT : RISCV_EXCP_LOAD_PAGE_FAULT; + cs->exception_index = RISCV_EXCP_LOAD_PAGE_FAULT; } break; case MMU_DATA_STORE: - if (two_stage && !first_stage) { + if (pmp_violation) { + cs->exception_index = RISCV_EXCP_STORE_AMO_ACCESS_FAULT; + } else if (two_stage && !first_stage) { cs->exception_index = RISCV_EXCP_STORE_GUEST_AMO_ACCESS_FAULT; } else { - cs->exception_index = pmp_violation ? - RISCV_EXCP_STORE_AMO_ACCESS_FAULT : - RISCV_EXCP_STORE_PAGE_FAULT; + cs->exception_index = RISCV_EXCP_STORE_PAGE_FAULT; } break; default: -- 2.39.2