Our check was extra cautious, we've audited crash_send_ipi
and it sends an IPI only to online CPU's. Removal of this
check should have not functional impact on crash kdump.
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---
arch/powerpc/kernel/crash.c | 3 ---
1 file changed, 3 deletions(-)
In irq_set_pending_from_srr1() we were missing 0x2 as system
reset identified from SRR1 caused by back to back system
resets or when interrupts are caused by SCOM when the thread
is not in power saving mode.
This helps us get to NMI handling in both the case where NMI
is caused when in power-saving and not in power-saving mode.
The actual exploitation is expected when we are doing a kdump
and an offline CPU might not be in power-saving mode due to
an already spurious IPI or any other reason.
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---
arch/powerpc/kernel/irq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Certain HMI's such as malfunction error propagate through
all threads/core on the system. If a thread was offline
prior to us crashing the system and jumping to the kdump
kernel, bad things happen when it wakes up due to an HMI
in the kdump kernel.
There are several possible ways to solve this problem
1. Put the offline cores in a state such that they are
not woken up for machine check and HMI errors. This
does not work, since we might need to wake up offline
threads to handle TB errors
2. Ignore HMI errors, setup HMEER to mask HMI errors,
but this still leads the window open for any MCEs
and masking them for the duration of the dump might
be a concern
3. Wake up offline CPUs, as in send them to
crash_ipi_callback (not wake them up as in mark them
online as seen by the hotplug). kexec does a
wake_online_cpus() call, this patch does something
similar, but instead sends an IPI and forces them to
crash_ipi_callback()
This patch takes approach #3.
Care is taken to enable this only for powenv platforms
via crash_wake_offline (a global value set at setup
time). The crash code sends out IPI's to all CPU's
which then move to crash_ipi_callback and kexec_smp_wait().
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---
Changelog v3
- Use SRR1's reason to wake up to drive replay_system_reset()
as a means of getting to kdump() as opposed to calling
crash_ipi_callback based on comments from Nick Piggin.
arch/powerpc/include/asm/kexec.h | 2 ++
arch/powerpc/kernel/crash.c | 13 ++++++++++++-
arch/powerpc/kernel/smp.c | 18 ++++++++++++++++++
arch/powerpc/platforms/powernv/smp.c | 12 ++++++++++++
4 files changed, 44 insertions(+), 1 deletion(-)
From: Nicholas Piggin <npiggin@gmail.com> Date: 2017-12-15 02:45:11
On Fri, 15 Dec 2017 12:27:39 +1100
Balbir Singh [off-list ref] wrote:
In irq_set_pending_from_srr1() we were missing 0x2 as system
reset identified from SRR1 caused by back to back system
resets or when interrupts are caused by SCOM when the thread
is not in power saving mode.
This helps us get to NMI handling in both the case where NMI
is caused when in power-saving and not in power-saving mode.
The actual exploitation is expected when we are doing a kdump
and an offline CPU might not be in power-saving mode due to
an already spurious IPI or any other reason.
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
When not in power saving mode, we don't look at SRR1 at all, so
we don't need this. You should never be getting it returned as the
result of your idle instruction (except on DD1 which has a bug,
but firmware doesn't implement the NMI IPI).
It's possible we could pay more attention to the reason, for
example in the powernv system reset handle we might only call
the NMI IPI handler if it was a scom sreset... but that would
be a regs->msr test.
On Fri, Dec 15, 2017 at 1:44 PM, Nicholas Piggin [off-list ref] wrote:
On Fri, 15 Dec 2017 12:27:39 +1100
Balbir Singh [off-list ref] wrote:
quoted
In irq_set_pending_from_srr1() we were missing 0x2 as system
reset identified from SRR1 caused by back to back system
resets or when interrupts are caused by SCOM when the thread
is not in power saving mode.
This helps us get to NMI handling in both the case where NMI
is caused when in power-saving and not in power-saving mode.
The actual exploitation is expected when we are doing a kdump
and an offline CPU might not be in power-saving mode due to
an already spurious IPI or any other reason.
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
When not in power saving mode, we don't look at SRR1 at all, so
we don't need this. You should never be getting it returned as the
result of your idle instruction (except on DD1 which has a bug,
but firmware doesn't implement the NMI IPI).
I added this for the next patch. We call irq_set_pending_from_srr1
while coming out of CPU idle, but if for any reason the IPI was spurious
and then we see an NMI during kdump, I did want to detect that
and callback into the NMI handler.
It's possible we could pay more attention to the reason, for
example in the powernv system reset handle we might only call
the NMI IPI handler if it was a scom sreset... but that would
be a regs->msr test.
regs->msr is the same as SRR1 in my kdump case, but your
right in general we want to look at regs->msr
Balbir Singh
From: Nicholas Piggin <npiggin@gmail.com> Date: 2017-12-15 02:59:00
On Fri, 15 Dec 2017 13:54:18 +1100
Balbir Singh [off-list ref] wrote:
On Fri, Dec 15, 2017 at 1:44 PM, Nicholas Piggin [off-list ref] wrote:
quoted
On Fri, 15 Dec 2017 12:27:39 +1100
Balbir Singh [off-list ref] wrote:
quoted
In irq_set_pending_from_srr1() we were missing 0x2 as system
reset identified from SRR1 caused by back to back system
resets or when interrupts are caused by SCOM when the thread
is not in power saving mode.
This helps us get to NMI handling in both the case where NMI
is caused when in power-saving and not in power-saving mode.
The actual exploitation is expected when we are doing a kdump
and an offline CPU might not be in power-saving mode due to
an already spurious IPI or any other reason.
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
When not in power saving mode, we don't look at SRR1 at all, so
we don't need this. You should never be getting it returned as the
result of your idle instruction (except on DD1 which has a bug,
but firmware doesn't implement the NMI IPI).
I added this for the next patch. We call irq_set_pending_from_srr1
while coming out of CPU idle, but if for any reason the IPI was spurious
and then we see an NMI during kdump, I did want to detect that
and callback into the NMI handler.
You'll never see it. The SRR1 value you get is the idle wakeup code.
So any scom-when-not-idle bit should never be set.
Thanks,
Nick
From: Nicholas Piggin <npiggin@gmail.com> Date: 2017-12-15 03:11:06
On Fri, 15 Dec 2017 12:27:40 +1100
Balbir Singh [off-list ref] wrote:
quoted hunk
Certain HMI's such as malfunction error propagate through
all threads/core on the system. If a thread was offline
prior to us crashing the system and jumping to the kdump
kernel, bad things happen when it wakes up due to an HMI
in the kdump kernel.
There are several possible ways to solve this problem
1. Put the offline cores in a state such that they are
not woken up for machine check and HMI errors. This
does not work, since we might need to wake up offline
threads to handle TB errors
2. Ignore HMI errors, setup HMEER to mask HMI errors,
but this still leads the window open for any MCEs
and masking them for the duration of the dump might
be a concern
3. Wake up offline CPUs, as in send them to
crash_ipi_callback (not wake them up as in mark them
online as seen by the hotplug). kexec does a
wake_online_cpus() call, this patch does something
similar, but instead sends an IPI and forces them to
crash_ipi_callback()
This patch takes approach #3.
Care is taken to enable this only for powenv platforms
via crash_wake_offline (a global value set at setup
time). The crash code sends out IPI's to all CPU's
which then move to crash_ipi_callback and kexec_smp_wait().
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---
Changelog v3
- Use SRR1's reason to wake up to drive replay_system_reset()
as a means of getting to kdump() as opposed to calling
crash_ipi_callback based on comments from Nick Piggin.
arch/powerpc/include/asm/kexec.h | 2 ++
arch/powerpc/kernel/crash.c | 13 ++++++++++++-
arch/powerpc/kernel/smp.c | 18 ++++++++++++++++++
arch/powerpc/platforms/powernv/smp.c | 12 ++++++++++++
4 files changed, 44 insertions(+), 1 deletion(-)
I wonder if we want to special-case it for system reset only? Everything
is all going down for kdump anyway I guess, but theoretically we don't
want to put other interrupts in our pending mask.
We might as well just do it unconditionally as well, so we can easily test
and make sure we do something sane if we happen to take a stray sreset
here for some reason (e.g., pdbg).
I would do this:
} else if ((srr1 & wmask) == SRR1_WAKERESET) {
/* kdump or debug tools can sreset offline CPUs */
irq_set_pending_from_srr1(srr1);
}
But then you still need an explicit kdump check for non-sreset wakeups
because the platform may not implement sreset, or it may fall back to
normal IPI if the sreset scom fails. So you then still need your
if (kdump) crash_ipi_callback
Thanks,
Nick
On Fri, Dec 15, 2017 at 2:10 PM, Nicholas Piggin [off-list ref] wrote:
On Fri, 15 Dec 2017 12:27:40 +1100
Balbir Singh [off-list ref] wrote:
quoted
Certain HMI's such as malfunction error propagate through
all threads/core on the system. If a thread was offline
prior to us crashing the system and jumping to the kdump
kernel, bad things happen when it wakes up due to an HMI
in the kdump kernel.
There are several possible ways to solve this problem
1. Put the offline cores in a state such that they are
not woken up for machine check and HMI errors. This
does not work, since we might need to wake up offline
threads to handle TB errors
2. Ignore HMI errors, setup HMEER to mask HMI errors,
but this still leads the window open for any MCEs
and masking them for the duration of the dump might
be a concern
3. Wake up offline CPUs, as in send them to
crash_ipi_callback (not wake them up as in mark them
online as seen by the hotplug). kexec does a
wake_online_cpus() call, this patch does something
similar, but instead sends an IPI and forces them to
crash_ipi_callback()
This patch takes approach #3.
Care is taken to enable this only for powenv platforms
via crash_wake_offline (a global value set at setup
time). The crash code sends out IPI's to all CPU's
which then move to crash_ipi_callback and kexec_smp_wait().
Signed-off-by: Balbir Singh <bsingharora@gmail.com>
---
Changelog v3
- Use SRR1's reason to wake up to drive replay_system_reset()
as a means of getting to kdump() as opposed to calling
crash_ipi_callback based on comments from Nick Piggin.
arch/powerpc/include/asm/kexec.h | 2 ++
arch/powerpc/kernel/crash.c | 13 ++++++++++++-
arch/powerpc/kernel/smp.c | 18 ++++++++++++++++++
arch/powerpc/platforms/powernv/smp.c | 12 ++++++++++++
4 files changed, 44 insertions(+), 1 deletion(-)
I wonder if we want to special-case it for system reset only? Everything
is all going down for kdump anyway I guess, but theoretically we don't
want to put other interrupts in our pending mask.
We might as well just do it unconditionally as well, so we can easily test
and make sure we do something sane if we happen to take a stray sreset
here for some reason (e.g., pdbg).
I would do this:
} else if ((srr1 & wmask) == SRR1_WAKERESET) {
/* kdump or debug tools can sreset offline CPUs */
irq_set_pending_from_srr1(srr1);
}
But then you still need an explicit kdump check for non-sreset wakeups
because the platform may not implement sreset, or it may fall back to
normal IPI if the sreset scom fails. So you then still need your
if (kdump) crash_ipi_callback
Yep, its required for the the non NMI case.
How does this look -- based on your comment
+ } else if ((srr1 & wmask) == SRR1_WAKESRESET) {
+ irq_set_pending_from_srr1(srr1);
+ /* Does not return */
}
+
smp_mb();
/*
* For kdump kernels, we process the ipi and jump to
- * handling the system reset exception.
+ * crash_ipi_callback
*/
- if (kdump_in_progress())
- irq_set_pending_from_srr1(srr1);
+ if (kdump_in_progress()) {
+ /*
+ * If we got to this point, we've not used
+ * NMI's, otherwise we would have gone
+ * via the SRR1_WAKESRESET path. We are
+ * using regular IPI's for waking up offline
+ * threads.
+ */
+ struct pt_regs regs;
+
+ ppc_save_regs(®s);
+ crash_ipi_callback(regs);
+ /* Does not return */
+ }
Balbir Singh
From: Nicholas Piggin <npiggin@gmail.com> Date: 2017-12-15 04:47:46
On Fri, 15 Dec 2017 14:34:03 +1100
Balbir Singh [off-list ref] wrote:
On Fri, Dec 15, 2017 at 2:10 PM, Nicholas Piggin [off-list ref] wrote:
quoted
On Fri, 15 Dec 2017 12:27:40 +1100
Balbir Singh [off-list ref] wrote:
quoted
But then you still need an explicit kdump check for non-sreset wakeups
because the platform may not implement sreset, or it may fall back to
normal IPI if the sreset scom fails. So you then still need your
if (kdump) crash_ipi_callback
Yep, its required for the the non NMI case.
How does this look -- based on your comment
+ } else if ((srr1 & wmask) == SRR1_WAKESRESET) {
+ irq_set_pending_from_srr1(srr1);
+ /* Does not return */
}
+
smp_mb();
/*
* For kdump kernels, we process the ipi and jump to
- * handling the system reset exception.
+ * crash_ipi_callback
*/
- if (kdump_in_progress())
- irq_set_pending_from_srr1(srr1);
+ if (kdump_in_progress()) {
+ /*
+ * If we got to this point, we've not used
+ * NMI's, otherwise we would have gone
+ * via the SRR1_WAKESRESET path. We are
+ * using regular IPI's for waking up offline
+ * threads.
+ */
+ struct pt_regs regs;
+
+ ppc_save_regs(®s);
+ crash_ipi_callback(regs);
+ /* Does not return */
+ }
That looks like it should do the trick.
Thanks,
Nick