Re: [PATCH for-9.0] target/riscv: prioritize pmp errors in raise_mmu_exception()

[Peter Maydell]

On Tue, 9 Apr 2024 at 18:53, Daniel Henrique Barboza
<dbarboza@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 <jchan@ventanamicro.com>
> Fixes: 82d53adfbb ("target/riscv/cpu_helper.c: Invalid exception on MMU 
> translation stage")
> Signed-off-by: Daniel Henrique Barboza <dbarboza@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.

thanks
-- PMM