On 4/12/24 14:12, Peter Maydell wrote:
On Tue, 9 Apr 2024 at 18:53, Daniel Henrique Barboza <dbarb...@ventanamicro.com> wrote: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>I guess from the Fixes: git commit hash that this isn't a regression since 8.2 ? That would make it too late for 9.0 at this point in the release cycle.
I don't think it's critical enough to push it for 9.0 now. We'll settle for 9.1 and then Michael can pick it for 9.0-stable. Thanks, Daniel
thanks -- PMM
