Re: instruction storage exception handling

Excerpts from Jacques de Laval's message of October 27, 2021 10:03 pm:

On Wednesday, October 27th, 2021 at 9:52 AM, Christophe Leroy [off-list ref] wrote:

quoted

Le 27/10/2021 à 09:47, Nicholas Piggin a écrit :

quoted

You're right. In that case it shouldn't change anything unless there

was a BO fault. I'm not sure what the problem is then. Guessing based

on the NIP and instructions, it looks like it's probably got the correct

user address that it's storing into vmf on the stack, so it has got past

the access checks so my theory would be wrong anyway.

Must be something simple but I can't see it yet.

Anyway, I think it is still worth doing the check with setting 0 in

_ESR(r11), maybe the Reference Manual is wrong.

So Jacques, please do the test anyway if you can.

Thanks

Christophe

I tested with the last patch from Nicholas, and with that I can not
reproduce the issue, so it seems like that solves it for my case and setup
at least.

Big thanks Christophe and Nicholas for looking in to this!

Thanks for reporting and testing. We can certainly send this patch 
upstream to fix the regression, but I'm still not exactly sure what is 
going on. If it is an errata or part of specification we missed that 
could explain it but it would be good to understand and comment it.

If you have time to test again with only the following patch applied,
it might give a better clue. This patch should keep running but it
would print some dmesg output.

Thanks,
Nick

diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
index a8d0ce85d39a..cf56f23ff90a 100644
--- a/arch/powerpc/mm/fault.c
+++ b/arch/powerpc/mm/fault.c

@@ -548,6 +548,12 @@ static __always_inline void __do_page_fault(struct pt_regs *regs)
 
 DEFINE_INTERRUPT_HANDLER(do_page_fault)
 {
+	if (TRAP(regs) == INTERRUPT_INST_STORAGE) {
+		if (regs->dsisr != 0) {
+			printk("ISI pc:%lx msr:%lx dsisr:%lx ESR:%lx\n", regs->nip, regs->msr, regs->dsisr, mfspr(SPRN_ESR));
+			regs->dsisr = 0; // fix?
+		}
+	}
 	__do_page_fault(regs);
 }