Re: [PATCH] riscv: Don't use va_pa_offset on kdump

From: Palmer Dabbelt
Date: Sat Oct 23 2021 - 16:14:16 EST


On Sat, 09 Oct 2021 06:18:48 PDT (-0700), mick@xxxxxxxxxxxx wrote:
Στις 2021-10-06 14:13, Alexandre Ghiti έγραψε:
+
+ /* This will trigger a jump to CSR_STVEC anyway */
jalr zero, a2, 0

The last jump to a2 can be removed since the fault will be triggered
before even reaching this instruction.


Just switching SATP to zero doesn't generate a trap unless mstatus.TVM
is set (for visualization purposes). The hart will try and execute the
next instruction but it's not clear in the spec what happens in case the
code is cached, I don't want to rely solely on STVEC. I prefer having
this instruction there, note that some earlier QEMU versions also had
this behavior (the original kdump patch didn't set STVEC and it worked
fine after setting SATP to zero).

IIRC this came down to some very specific wording in the spec. Something along the lines of the 0 in SATP meaning "no translation", SFENCE.VMA ordering translations, and the general "if the spec doesn't mention it then it has to work" logic. I thought I opened a spec issue about this for clarification, but I can't find it.

That said, I'm perfectly fine taking the safe approach here as it's not like the performance matters here. Warrants a comment, though.



This patch fixes a regression introduced when moving the kernel to the
end of the address space, so we should add:
Fixes: 2bfc6cd81bd1 ("riscv: Move kernel mapping outside of linear
mapping")

And it should be backported to 5.13 and 5.14. It seems that the
following tags should be enough:

Cc: <stable@xxxxxxxxxxxxxxx> # 5.13
Cc: <stable@xxxxxxxxxxxxxxx> # 5.14

And finally, you can add:

Reviewed-by: Alexandre Ghiti <alex@xxxxxxxx>


ACK, thanks ! I'll resend the patch with the tags you mentioned.

I don't have a v2 in my inbox, did I miss something? Also, if it's just the tags then it's generally not necessary to re-send something. The comment does, though.

LMK if you want me to deal with this, or if there's going to be a v2.

Thanks!