On Tue, 2018-09-25 at 22:00 +1000, Michael Ellerman wrote:

Michael Neuling [off-list ref] writes:

quoted

Current we store the userspace r1 to PACATMSCRATCH before finally
saving it to the thread struct.
=20
In theory an exception could be taken here (like a machine check or
SLB miss) that could write PACATMSCRATCH and hence corrupt the
userspace r1. The SLB fault currently doesn't touch PACATMSCRATCH, but
others do.
=20
We've never actually seen this happen but it's theoretically
possible. Either way, the code is fragile as it is.
=20
This patch saves r1 to the kernel stack (which can't fault) before we
turn MSR[RI] back on. PACATMSCRATCH is still used but only with
MSR[RI] off. We then copy r1 from the kernel stack to the thread
struct once we have MSR[RI] back on.
=20
Suggested-by: Breno Leitao <leitao@debian.org>
Signed-off-by: Michael Neuling <redacted>
---
 arch/powerpc/kernel/tm.S | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
=20

diff --git a/arch/powerpc/kernel/tm.S b/arch/powerpc/kernel/tm.S
index 701b0f5b09..8207816a1e 100644
--- a/arch/powerpc/kernel/tm.S
+++ b/arch/powerpc/kernel/tm.S

@@ -178,6 +178,12 @@ _GLOBAL(tm_reclaim)

=20
 	std	r11, GPR11(r1)			/* Temporary stash */
=20
+	/* Move r1 to kernel stack in case PACATMSCRATCH is used once
+	 * we turn on RI
+	 */

=20
I see we still need to send you to Comment Formatting Re-Education Camp.

The rest of that file has they style, so I'm just keeping with that.  I can
submit a patch later to fix them all up.

I rewrote it a bit too, to hopefully be clearer?
=20
	/*
	 * Move the saved user r1 to the kernel stack in case PACATMSCRATCH is
	 * clobbered by an exception once we turn on MSR_RI below.
	 */
	ld	r11, PACATMSCRATCH(r13)
	std	r11, GPR1(r1)

Yeah, that's clearer... thanks.

Mikey