From: Nicholas Piggin <npiggin@gmail.com>
To: linuxppc-dev@lists.ozlabs.org
Cc: Nicholas Piggin <npiggin@gmail.com>
Subject: [PATCH 2/2] powerpc/64s/interrupt: Check and fix srr_valid without crashing
Date: Tue, 22 Jun 2021 16:04:16 +1000 [thread overview]
Message-ID: <20210622060416.548187-3-npiggin@gmail.com> (raw)
In-Reply-To: <20210622060416.548187-1-npiggin@gmail.com>
The PPC_RFI_SRR_DEBUG check added by patch "powerpc/64s: avoid reloading
(H)SRR registers if they are still valid" has a few deficiencies. It
does not fix the actual problem, it's not enabled by default, and it
causes a program check interrupt which can cause more difficulties.
However there are a lot of paths which may clobber SRRs or change return
regs, and difficult to have a high confidence that all paths are covered
without wider testing.
Add a relatively low overhead always-enabled check that catches most
such cases, reports once, and fixes it so the kernel can continue.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
arch/powerpc/kernel/interrupt.c | 58 +++++++++++++++++++++++++++++++++
1 file changed, 58 insertions(+)
diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
index 05fa3ae56e25..5920a3e8d1d5 100644
--- a/arch/powerpc/kernel/interrupt.c
+++ b/arch/powerpc/kernel/interrupt.c
@@ -231,6 +231,56 @@ static notrace void booke_load_dbcr0(void)
#endif
}
+#include <linux/sched/debug.h> /* for show_regs */
+static void check_return_regs_valid(struct pt_regs *regs)
+{
+#ifdef CONFIG_PPC_BOOK3S_64
+ static bool warned = false;
+
+ if (regs->trap == 0x980 || regs->trap == 0xe00 || regs->trap == 0xe20 ||
+ regs->trap == 0xe40 || regs->trap == 0xe60 || regs->trap == 0xe80 ||
+ regs->trap == 0xea0 || regs->trap == 0xf80 || regs->trap == 0x1200 ||
+ regs->trap == 0x1500 || regs->trap == 0x1600 || regs->trap == 0x1800) {
+ if (local_paca->hsrr_valid) {
+ unsigned long hsrr0 = mfspr(SPRN_HSRR0);
+ unsigned long hsrr1 = mfspr(SPRN_HSRR1);
+
+ if (hsrr0 == regs->nip && hsrr1 == regs->msr)
+ return;
+
+ if (!warned) {
+ warned = true;
+ printk("HSRR0 was: %lx should be: %lx\n",
+ hsrr0, regs->nip);
+ printk("HSRR1 was: %lx should be: %lx\n",
+ hsrr1, regs->msr);
+ show_regs(regs);
+ }
+ local_paca->hsrr_valid = 0; /* fixup */
+ }
+
+ } else if (regs->trap != 0x3000) {
+ if (local_paca->srr_valid) {
+ unsigned long srr0 = mfspr(SPRN_SRR0);
+ unsigned long srr1 = mfspr(SPRN_SRR1);
+
+ if (srr0 == regs->nip && srr1 == regs->msr)
+ return;
+
+ if (!warned) {
+ warned = true;
+ printk("SRR0 was: %lx should be: %lx\n",
+ srr0, regs->nip);
+ printk("SRR1 was: %lx should be: %lx\n",
+ srr1, regs->msr);
+ show_regs(regs);
+ }
+ local_paca->srr_valid = 0; /* fixup */
+ }
+ }
+#endif
+}
+
/*
* This should be called after a syscall returns, with r3 the return value
* from the syscall. If this function returns non-zero, the system call
@@ -327,6 +377,8 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
}
}
+ check_return_regs_valid(regs);
+
user_enter_irqoff();
/* scv need not set RI=0 because SRRs are not used */
@@ -405,6 +457,8 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs)
}
}
+ check_return_regs_valid(regs);
+
user_enter_irqoff();
if (unlikely(!__prep_irq_for_enabled_exit(true))) {
@@ -469,9 +523,13 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs)
}
}
+ check_return_regs_valid(regs);
+
if (unlikely(!prep_irq_for_enabled_exit(true, !irqs_disabled_flags(flags))))
goto again;
} else {
+ check_return_regs_valid(regs);
+
/* Returning to a kernel context with local irqs disabled. */
__hard_EE_RI_disable();
#ifdef CONFIG_PPC64
--
2.23.0
next prev parent reply other threads:[~2021-06-22 6:05 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-22 6:04 [PATCH 0/2] fast interrupts fixes Nicholas Piggin
2021-06-22 6:04 ` [PATCH 1/2] powerpc/64s: Fix "avoid reloading (H)SRR registers if they are still valid" Nicholas Piggin
2021-06-22 6:04 ` Nicholas Piggin [this message]
2021-06-22 6:47 ` [PATCH 2/2] powerpc/64s/interrupt: Check and fix srr_valid without crashing Christophe Leroy
2021-06-22 8:54 ` Nicholas Piggin
2021-06-22 8:55 ` Christophe Leroy
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=20210622060416.548187-3-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.