qemu-riscv
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH for-9.0] target/riscv: prioritize pmp errors in raise_mmu_exc


From: Daniel Henrique Barboza
Subject: Re: [PATCH for-9.0] target/riscv: prioritize pmp errors in raise_mmu_exception()
Date: Fri, 12 Apr 2024 14:19:36 -0300
User-agent: Mozilla Thunderbird



On 4/12/24 14:12, Peter Maydell wrote:
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.

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]