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

[Daniel Henrique Barboza]

On 4/12/24 13:52, Aleksei Filippov wrote:
> On 12.04.2024 19:00, Daniel Henrique Barboza wrote:
>
> > Thanks for giving it a go. You're right, this patch alone is not enough and 
> > we'll
> > need your patch too.
> >
> > But note that, with what you've said in mind, your patch will also end up 
> > setting
> > mtval2 and env->guest_phys_fault_addr in case a PMP fault occurs during the
> > get_physical_address() right at the start of second stage:
> >
> >          if (ret == TRANSLATE_SUCCESS) {
> >              /* Second stage lookup */
> >              im_address = pa;
> >
> >              ret = get_physical_address(env, &pa, &prot2, im_address, NULL,
> >                                         access_type, MMUIdx_U, false, true,
> >                                         false);
> >
> >
> > I think your patch needs to also prevent env->guest_phys_fault_addr to be set 
> > when
> > ret == TRANSLATE_PMP_FAIL.
> >
> > With these changes in your patch, and this patch, we're free to set 
> > "first_stage_error = false;"
> > at the start of second stage lookup, keeping consistency, because 
> > raise_mmu_exception is now
> > able to deal with it. I can amend this change in this patch. This patch would 
> > prioritize
> > PMP errors, your patch will fix the problem with mtval2.
> >
> > Let me know what do you think. If you agree I can re-send both patches together.
> >
> >
> > Thanks,
> >
> >
> > Daniel
>
> Oh, I actually missed that, thx, you are right. I could prepare patch to fix
> this, do you want it in this thread or in previous with only my patch in?

If you don't mind, please re-send it together in a new thread with this patch 
too.

Include this one as is, then include yours as a complement that fixes the 
problem
with mtval2 that my patch missed. You can use the explanation you gave me as the
new commit msg for your patch. E.g:

"target/riscv: do not set mtval2 for non guest-page faults

Previous patch fixed the PMP priority in raise_mmu_exception() but we're still
setting mtval2 incorrectly. In riscv_cpu_tlb_fill(), after pmp check in 2 stage
translation part, mtval2 will be set in case of successes 2 stage translation 
but
failed pmp check.

In this case we gonna set mtval2 via env->guest_phys_fault_addr in context of
riscv_cpu_tlb_fill(), as this was a guest-page-fault, but it didn't and mtval2
should be zero, according to RISCV privileged spec sect. 9.4.4: When a guest
page-fault is taken into M-mode, mtval2 is written with either zero or guest
physical address that faulted, shifted by 2 bits. *For other traps, mtval2
is set to zero...* "


Thanks,

Daniel

>