All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nicholas Piggin <npiggin@gmail.com>
To: linuxppc-dev@lists.ozlabs.org
Cc: Nicholas Piggin <npiggin@gmail.com>
Subject: [PATCH v4 01/16] powerpc/64s/exception: Fix machine check no-loss idle wakeup
Date: Fri,  8 May 2020 14:33:53 +1000	[thread overview]
Message-ID: <20200508043408.886394-2-npiggin@gmail.com> (raw)
In-Reply-To: <20200508043408.886394-1-npiggin@gmail.com>

The architecture allows for machine check exceptions to cause idle
wakeups which resume at the 0x200 address which has to return via
the idle wakeup code, but the early machine check handler is run
first.

The case of a no state-loss sleep is broken because the early
handler uses non-volatile register r1 , which is needed for the wakeup
protocol, but it is not restored.

Fix this by loading r1 from the MCE exception frame before returning
to the idle wakeup code. Also update the comment which has become
stale since the idle rewrite in C.

Fixes: 10d91611f426d ("powerpc/64s: Reimplement book3s idle code in C")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

This crash was found and fix confirmed with a machine check injection
test in qemu powernv model (which is not upstream in qemu yet).
---
 arch/powerpc/kernel/exceptions-64s.S | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
index 728ccb0f560c..bbf3109c5cba 100644
--- a/arch/powerpc/kernel/exceptions-64s.S
+++ b/arch/powerpc/kernel/exceptions-64s.S
@@ -1224,17 +1224,19 @@ EXC_COMMON_BEGIN(machine_check_idle_common)
 	bl	machine_check_queue_event
 
 	/*
-	 * We have not used any non-volatile GPRs here, and as a rule
-	 * most exception code including machine check does not.
-	 * Therefore PACA_NAPSTATELOST does not need to be set. Idle
-	 * wakeup will restore volatile registers.
+	 * GPR-loss wakeups are relatively straightforward, because the
+	 * idle sleep code has saved all non-volatile registers on its
+	 * own stack, and r1 in PACAR1.
 	 *
-	 * Load the original SRR1 into r3 for pnv_powersave_wakeup_mce.
+	 * For no-loss wakeups the r1 and lr registers used by the
+	 * early machine check handler have to be restored first. r2 is
+	 * the kernel TOC, so no need to restore it.
 	 *
 	 * Then decrement MCE nesting after finishing with the stack.
 	 */
 	ld	r3,_MSR(r1)
 	ld	r4,_LINK(r1)
+	ld	r1,GPR1(r1)
 
 	lhz	r11,PACA_IN_MCE(r13)
 	subi	r11,r11,1
@@ -1243,7 +1245,7 @@ EXC_COMMON_BEGIN(machine_check_idle_common)
 	mtlr	r4
 	rlwinm	r10,r3,47-31,30,31
 	cmpwi	cr1,r10,2
-	bltlr	cr1	/* no state loss, return to idle caller */
+	bltlr	cr1	/* no state loss, return to idle caller with r3=SRR1 */
 	b	idle_return_gpr_loss
 #endif
 
-- 
2.23.0


  reply	other threads:[~2020-05-08  4:38 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-08  4:33 [PATCH v4 00/16] powerpc: machine check and system reset fixes Nicholas Piggin
2020-05-08  4:33 ` Nicholas Piggin [this message]
2020-05-08  4:33 ` [PATCH v4 02/16] powerpc/64s/exceptions: Fix in_mce accounting in unrecoverable path Nicholas Piggin
2020-05-08  4:33 ` [PATCH v4 03/16] powerpc/64s/exceptions: Change irq reconcile for NMIs from reusing _DAR to RESULT Nicholas Piggin
2020-05-08  4:33 ` [PATCH v4 04/16] powerpc/64s/exceptions: machine check reconcile irq state Nicholas Piggin
2020-05-08 13:39   ` Michael Ellerman
2020-05-09  7:48     ` Nicholas Piggin
2020-05-08  4:33 ` [PATCH v4 05/16] powerpc/pseries/ras: avoid calling rtas_token in NMI paths Nicholas Piggin
2020-05-08  4:33 ` [PATCH v4 06/16] powerpc/pseries/ras: FWNMI_VALID off by one Nicholas Piggin
2020-05-08  4:33 ` [PATCH v4 07/16] powerpc/pseries/ras: fwnmi avoid modifying r3 in error case Nicholas Piggin
2020-05-08  4:34 ` [PATCH v4 08/16] powerpc/pseries/ras: fwnmi sreset should not interlock Nicholas Piggin
2020-05-08  4:34 ` [PATCH v4 09/16] powerpc/pseries: limit machine check stack to 4GB Nicholas Piggin
2020-05-08  4:34 ` [PATCH v4 10/16] powerpc/pseries: machine check use rtas_call_unlocked with args on stack Nicholas Piggin
2020-05-08  4:34 ` [PATCH v4 11/16] powerpc/64s: machine check interrupt update NMI accounting Nicholas Piggin
2020-05-09  3:13   ` kbuild test robot
2020-05-09  3:13     ` kbuild test robot
2020-05-09  7:50     ` Nicholas Piggin
2020-05-11  9:50       ` Michael Ellerman
2020-05-11  9:50         ` Michael Ellerman
2020-05-08  4:34 ` [PATCH v4 12/16] powerpc: implement ftrace_enabled helper Nicholas Piggin
2020-05-08  4:34 ` [PATCH v4 13/16] powerpc/64s: machine check do not trace real-mode handler Nicholas Piggin
2020-05-08  4:34 ` [PATCH v4 14/16] powerpc/traps: system reset do not trace Nicholas Piggin
2020-05-08  4:34 ` [PATCH v4 15/16] powerpc/traps: make unrecoverable NMIs die instead of panic Nicholas Piggin
2020-05-08  4:34 ` [PATCH v4 16/16] powerpc/traps: Machine check fix RI=0 recoverability check Nicholas Piggin
2020-05-20 11:00 ` [PATCH v4 00/16] powerpc: machine check and system reset fixes Michael Ellerman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200508043408.886394-2-npiggin@gmail.com \
    --to=npiggin@gmail.com \
    --cc=linuxppc-dev@lists.ozlabs.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.