Re: [PATCH] x86/mce: Keep quiet in case of broadcasted mce after system panic

2017-02-21 Thread Xunlei Pang 
On 02/22/2017 at 02:20 AM, Luck, Tony wrote:
>> It's from my understanding, I didn't get the explicit description from the 
>> intel SDM on this point.
>> If a broadcast SRAO comes on real hardware, will MSR_IA32_MCG_STATUS of each 
>> cpu have MCG_STATUS_RIPV bit set?
> MCG_STATUS is a per-thread MSR and will contain the status appropriate for 
> that thread when #MC is delivered.
> So the RIPV bit will be set if, and only if, the thread saved a valid return 
> address for this exception. The net result
> is that it is almost always set for "innocent bystander" CPUs that were 
> dragged into the exception handler because
> of a broadcast #MC. We make the test because if it isn't set, then the 
> do_machine_check() had better not return
> because we have no idea where it will return to - since there is not a valid 
> return IP.
>

Got it, thanks for the details.

Regards,
Xunlei

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v3] x86/mce: Don't participate in rendezvous process once nmi_shootdown_cpus() was made

2017-02-21 Thread Xunlei Pang 
We met an issue for kdump: after kdump kernel boots up,
and there comes a broadcasted mce in first kernel, the
other cpus remaining in first kernel will enter the old
mce handler of first kernel, then timeout and panic due
to MCE synchronization, finally reset the kdump cpus.

This patch lets cpus stay quiet after nmi_shootdown_cpus(),
so after kdump boots, cpus remaining in 1st kernel should 
not do anything except clearing MCG_STATUS. This is useful
for kdump to let vmcore dumping perform as hard as it can.

Previous efforts:
https://patchwork.kernel.org/patch/6167631/
https://lists.gt.net/linux/kernel/2146557

Cc: Naoya Horiguchi 
Suggested-by: Borislav Petkov 
Signed-off-by: Xunlei Pang 
---
v1->v2:
Using crashing_cpu according to Borislav's suggestion.

v2->v3:
- Used crashing_cpu in mce.c explicitly, not skip crashing_cpu.
- Added some comments.

 arch/x86/include/asm/reboot.h|  1 +
 arch/x86/kernel/cpu/mcheck/mce.c | 12 ++--
 arch/x86/kernel/reboot.c |  5 +++--
 3 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/reboot.h b/arch/x86/include/asm/reboot.h
index 2cb1cc2..fc62ba8 100644
--- a/arch/x86/include/asm/reboot.h
+++ b/arch/x86/include/asm/reboot.h
@@ -15,6 +15,7 @@ struct machine_ops {
 };
 
 extern struct machine_ops machine_ops;
+extern int crashing_cpu;
 
 void native_machine_crash_shutdown(struct pt_regs *regs);
 void native_machine_shutdown(void);
diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 8e9725c..1493222 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -49,6 +49,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "mce-internal.h"
 
@@ -1127,9 +1128,16 @@ void do_machine_check(struct pt_regs *regs, long 
error_code)
 * on Intel.
 */
int lmce = 1;
+   int cpu = smp_processor_id();
 
-   /* If this CPU is offline, just bail out. */
-   if (cpu_is_offline(smp_processor_id())) {
+   /*
+* Cases to bail out to avoid rendezvous process timeout:
+* 1)If this CPU is offline.
+* 2)If crashing_cpu was set, e.g. entering kdump,
+*   we need to skip cpus remaining in 1st kernel.
+*/
+   if (cpu_is_offline(cpu) ||
+   (crashing_cpu != -1 && crashing_cpu != cpu)) {
u64 mcgstatus;
 
mcgstatus = mce_rdmsrl(MSR_IA32_MCG_STATUS);
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index e244c19..92ecf4b 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -749,10 +749,11 @@ void machine_crash_shutdown(struct pt_regs *regs)
 #endif
 
 
+/* This keeps a track of which one is crashing cpu. */
+int crashing_cpu = -1;
+
 #if defined(CONFIG_SMP)
 
-/* This keeps a track of which one is crashing cpu. */
-static int crashing_cpu;
 static nmi_shootdown_cb shootdown_callback;
 
 static atomic_t waiting_for_crash_ipi;
-- 
1.8.3.1


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v2] gitignore: add two generated files in purgatory

2017-02-21 Thread Eric DeVolder 


On 02/21/2017 01:48 PM, Daniel Kiper wrote:

On Tue, Feb 21, 2017 at 10:18:24AM -0600, Eric DeVolder wrote:

This patch adds the two generated files below to .gitignore,
so that 'git status' does not complain about them.


This is not a problem. Do you have issues during 'git checkout'
or so? If yes then I think it is worth considering.

Daniel


The contents of .gitignore are as follows. There are other generated 
files in the file list. It's not that there is a problem, but rather I 
added these two files for the likely same reason the other generated 
files are in the list, to cease the complaints from git about those files.


eric

=== contents of .gitignore ===

#
# Normal rules
#
*~
*.a
*.d
*.o
*.ro

# generated files
/Makefile
/autom4te.cache/
/bin/
/build/
/config.log
/config.status
/configure
/include/config.h.in
/include/config.h


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v2] gitignore: add two generated files in purgatory

2017-02-21 Thread Daniel Kiper 
On Tue, Feb 21, 2017 at 10:18:24AM -0600, Eric DeVolder wrote:
> This patch adds the two generated files below to .gitignore,
> so that 'git status' does not complain about them.

This is not a problem. Do you have issues during 'git checkout'
or so? If yes then I think it is worth considering.

Daniel

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

RE: [PATCH] x86/mce: Keep quiet in case of broadcasted mce after system panic

2017-02-21 Thread Luck, Tony 
> It's from my understanding, I didn't get the explicit description from the 
> intel SDM on this point.
> If a broadcast SRAO comes on real hardware, will MSR_IA32_MCG_STATUS of each 
> cpu have MCG_STATUS_RIPV bit set?

MCG_STATUS is a per-thread MSR and will contain the status appropriate for that 
thread when #MC is delivered.
So the RIPV bit will be set if, and only if, the thread saved a valid return 
address for this exception. The net result
is that it is almost always set for "innocent bystander" CPUs that were dragged 
into the exception handler because
of a broadcast #MC. We make the test because if it isn't set, then the 
do_machine_check() had better not return
because we have no idea where it will return to - since there is not a valid 
return IP.

-Tony

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v2] x86/mce: Don't participate in rendezvous process once nmi_shootdown_cpus() was made

2017-02-21 Thread Borislav Petkov 
On Tue, Feb 21, 2017 at 08:37:21PM +0800, Xunlei Pang wrote:
> -/* If this CPU is offline, just bail out. */
> -if (cpu_is_offline(smp_processor_id())) {
> +/*
> + * Cases to bail out to avoid rendezvous process timeout:
> + * 1)If crashing_cpu was set, e.g. entering kdump,
> + *   we need to skip cpus remaining in 1st kernel.
> + * 2)If this CPU is offline.
> + */
> +if (crashing_cpu != -1 ||
> +cpu_is_offline(smp_processor_id())) {

You're still not letting the crashing_cpu enter the #MC handler. You
need to handle the MCE no matter how short the window is.

-- 
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v2] gitignore: add two generated files in purgatory

2017-02-21 Thread Eric DeVolder 
This patch adds the two generated files below to .gitignore,
so that 'git status' does not complain about them.

purgatory/purgatory.map
purgatory/purgatory.ro.sym

Signed-off-by: Eric DeVolder 
---
v2: Incorporated feedback
- A bit more specific why these files added to .gitignore
v1: Posted to kexec-tools mailing list
---
 .gitignore | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/.gitignore b/.gitignore
index 81e03ab..1ab52d9 100644
--- a/.gitignore
+++ b/.gitignore
@@ -17,3 +17,5 @@
 /configure
 /include/config.h.in
 /include/config.h
+/purgatory/purgatory.map
+/purgatory/purgatory.ro.sym
-- 
2.7.4


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v2] x86/mce: Don't participate in rendezvous process once nmi_shootdown_cpus() was made

2017-02-21 Thread Xunlei Pang 
On 02/20/2017 at 07:09 PM, Borislav Petkov wrote:
> On Mon, Feb 20, 2017 at 02:10:37PM +0800, Xunlei Pang wrote:
>> @@ -1128,8 +1129,9 @@ void do_machine_check(struct pt_regs *regs, long 
>> error_code)
>>   */
>>  int lmce = 1;
>>  
>> -/* If this CPU is offline, just bail out. */
>> -if (cpu_is_offline(smp_processor_id())) {
>> +/* If nmi shootdown happened or this CPU is offline, just bail out. */
>> +if (cpus_shotdown() ||
> I don't like "cpus_shotdown" - it doesn't hint at all that this is
> special-handling crash/kdump.
>
> And more importantly, I want it to be obvious that we do let the
> crashing CPU into the MCE handler.

Hi Boris,

I made some improvements, what do you think the following one?
If you think it is fine, I can send out v3. Thanks for your time!

---
 arch/x86/include/asm/reboot.h|  1 +
 arch/x86/kernel/cpu/mcheck/mce.c | 11 +--
 arch/x86/kernel/reboot.c |  5 +++--
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/reboot.h b/arch/x86/include/asm/reboot.h
index 2cb1cc2..fc62ba8 100644
--- a/arch/x86/include/asm/reboot.h
+++ b/arch/x86/include/asm/reboot.h
@@ -15,6 +15,7 @@ struct machine_ops {
 };
 
 extern struct machine_ops machine_ops;
+extern int crashing_cpu;
 
 void native_machine_crash_shutdown(struct pt_regs *regs);
 void native_machine_shutdown(void);
diff --git a/arch/x86/kernel/cpu/mcheck/mce.c b/arch/x86/kernel/cpu/mcheck/mce.c
index 8e9725c..7f53145 100644
--- a/arch/x86/kernel/cpu/mcheck/mce.c
+++ b/arch/x86/kernel/cpu/mcheck/mce.c
@@ -49,6 +49,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "mce-internal.h"
 
@@ -1128,8 +1129,14 @@ void do_machine_check(struct pt_regs *regs, long 
error_code)
  */
 int lmce = 1;
 
-/* If this CPU is offline, just bail out. */
-if (cpu_is_offline(smp_processor_id())) {
+/*
+ * Cases to bail out to avoid rendezvous process timeout:
+ * 1)If crashing_cpu was set, e.g. entering kdump,
+ *   we need to skip cpus remaining in 1st kernel.
+ * 2)If this CPU is offline.
+ */
+if (crashing_cpu != -1 ||
+cpu_is_offline(smp_processor_id())) {
 u64 mcgstatus;
 
 mcgstatus = mce_rdmsrl(MSR_IA32_MCG_STATUS);
diff --git a/arch/x86/kernel/reboot.c b/arch/x86/kernel/reboot.c
index e244c19..92ecf4b 100644
--- a/arch/x86/kernel/reboot.c
+++ b/arch/x86/kernel/reboot.c
@@ -749,10 +749,11 @@ void machine_crash_shutdown(struct pt_regs *regs)
 #endif
 
 
+/* This keeps a track of which one is crashing cpu. */
+int crashing_cpu = -1;
+
 #if defined(CONFIG_SMP)
 
-/* This keeps a track of which one is crashing cpu. */
-static int crashing_cpu;
 static nmi_shootdown_cb shootdown_callback;
 
 static atomic_t waiting_for_crash_ipi;
-- 
1.8.3.1

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v2] x86/mce: Don't participate in rendezvous process once nmi_shootdown_cpus() was made

2017-02-21 Thread Borislav Petkov 
On Tue, Feb 21, 2017 at 09:28:20AM +0800, Xunlei Pang wrote:
> Not kdump kernel starts dumping, just during nmi_shootdown_cpus(), if some
> MCE comes after crashing_cpu was set and we don't skip crashing_cpu, then
> the crashing cpu will enter mce handler and trigger the synchronization issue.

Ok, I went and looked at __crash_kexec() - so we do nmi_shootdown_cpus()
as part of machine_crash_shutdown(). The next thing we do is kexec the
new kernel in machine_kexec(kexec_crash_image). The picture is clearer
now.

-- 
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec