[PATCH 0/3] x86: unify crash_32/64.c

[PATCH 0/3] x86: unify crash_32/64.c

[Hiroshi Shimamoto]

Hi,

I made patches to unify crash_32/64.c.
There are three patches;
1. add lapic_shutdown for x86_64
2. add safe_smp_processor_id for x86_64
3. unify crash_32/64.c

I'm not sure that it's good to split to these patches.

I've compiled on both of 32bit and 64bit, and tested
kdump on 64bit.

Thanks
Hiroshi Shimamoto

Re: [PATCH 1/3] x86: add lapic_shutdown for x86_64

[Hiroshi Shimamoto]

From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>

Signed-off-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
---
 arch/x86/kernel/apic_64.c |   14 ++++++++++++++
 include/asm-x86/apic_64.h |    1 +
 2 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/apic_64.c b/arch/x86/kernel/apic_64.c
index f47bc49..f28ccb5 100644
--- a/arch/x86/kernel/apic_64.c
+++ b/arch/x86/kernel/apic_64.c
@@ -287,6 +287,20 @@ void disable_local_APIC(void)
 	apic_write(APIC_SPIV, value);
 }
 
+void lapic_shutdown(void)
+{
+	unsigned long flags;
+
+	if (!cpu_has_apic)
+		return;
+
+	local_irq_save(flags);
+
+	disable_local_APIC();
+
+	local_irq_restore(flags);
+}
+
 /*
  * This is to verify that we're looking at a real local APIC.
  * Check these against your board if the CPUs aren't getting
diff --git a/include/asm-x86/apic_64.h b/include/asm-x86/apic_64.h
index 3c8f21e..2747a11 100644
--- a/include/asm-x86/apic_64.h
+++ b/include/asm-x86/apic_64.h
@@ -69,6 +69,7 @@ extern void clear_local_APIC (void);
 extern void connect_bsp_APIC (void);
 extern void disconnect_bsp_APIC (int virt_wire_setup);
 extern void disable_local_APIC (void);
+extern void lapic_shutdown (void);
 extern int verify_local_APIC (void);
 extern void cache_APIC_registers (void);
 extern void sync_Arb_IDs (void);
-- 
1.5.2.3

Re: [PATCH 1/3] x86: add lapic_shutdown for x86_64

[Vivek Goyal]

On Fri, Oct 19, 2007 at 06:21:11PM -0700, Hiroshi Shimamoto wrote:
> From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
> 
> Signed-off-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
> ---
>  arch/x86/kernel/apic_64.c |   14 ++++++++++++++
>  include/asm-x86/apic_64.h |    1 +
>  2 files changed, 15 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/x86/kernel/apic_64.c b/arch/x86/kernel/apic_64.c
> index f47bc49..f28ccb5 100644
> --- a/arch/x86/kernel/apic_64.c
> +++ b/arch/x86/kernel/apic_64.c
> @@ -287,6 +287,20 @@ void disable_local_APIC(void)
>  	apic_write(APIC_SPIV, value);
>  }
> 
> +void lapic_shutdown(void)
> +{
> +	unsigned long flags;
> +
> +	if (!cpu_has_apic)
> +		return;
> +
> +	local_irq_save(flags);
> +
> +	disable_local_APIC();
> +
> +	local_irq_restore(flags);
> +}
> +
>  /*

Do we really have to introduce this function for 64bit? I remember some
issues were faced on i386 w.r.t kernel enabling the LAPIC against the
wishes of BIOS hence kernel was disabling it while shutting down. No
such problems were reported for x86_64 hence this function existed only
for i386.

If that is the case, probably we don't have to introduce lapic_shutdown()
for x86_64. Instead call lapic_shutdown() for X86_32, and disble_local_APIC()
otherwise?

Thanks
Vivek

Re: [PATCH 1/3] x86: add lapic_shutdown for x86_64

[Hiroshi Shimamoto]

Vivek Goyal wrote:
> On Fri, Oct 19, 2007 at 06:21:11PM -0700, Hiroshi Shimamoto wrote:
>> From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
>>
>> Signed-off-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
>> ---
>>  arch/x86/kernel/apic_64.c |   14 ++++++++++++++
>>  include/asm-x86/apic_64.h |    1 +
>>  2 files changed, 15 insertions(+), 0 deletions(-)
>>
>> diff --git a/arch/x86/kernel/apic_64.c b/arch/x86/kernel/apic_64.c
>> index f47bc49..f28ccb5 100644
>> --- a/arch/x86/kernel/apic_64.c
>> +++ b/arch/x86/kernel/apic_64.c
>> @@ -287,6 +287,20 @@ void disable_local_APIC(void)
>>  	apic_write(APIC_SPIV, value);
>>  }
>>
>> +void lapic_shutdown(void)
>> +{
>> +	unsigned long flags;
>> +
>> +	if (!cpu_has_apic)
>> +		return;
>> +
>> +	local_irq_save(flags);
>> +
>> +	disable_local_APIC();
>> +
>> +	local_irq_restore(flags);
>> +}
>> +
>>  /*
> 
> Do we really have to introduce this function for 64bit? I remember some
> issues were faced on i386 w.r.t kernel enabling the LAPIC against the
> wishes of BIOS hence kernel was disabling it while shutting down. No
> such problems were reported for x86_64 hence this function existed only
> for i386.

Thanks for the comment. I didn't know the issues, so I'd simply added
this function for unification.

> If that is the case, probably we don't have to introduce lapic_shutdown()
> for x86_64. Instead call lapic_shutdown() for X86_32, and disble_local_APIC()
> otherwise?

I will do that. I was thinking which is good when posting these patches.

Thanks
Hiroshi Shimamoto

Re: [PATCH 1/3] x86: add lapic_shutdown for x86_64

[Eric W. Biederman]

Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com> writes:

>> Do we really have to introduce this function for 64bit? I remember some
>> issues were faced on i386 w.r.t kernel enabling the LAPIC against the
>> wishes of BIOS hence kernel was disabling it while shutting down. No
>> such problems were reported for x86_64 hence this function existed only
>> for i386.
>
> Thanks for the comment. I didn't know the issues, so I'd simply added
> this function for unification.
>
>> If that is the case, probably we don't have to introduce lapic_shutdown()
>> for x86_64. Instead call lapic_shutdown() for X86_32, and disble_local_APIC()
>> otherwise?
>
> I will do that. I was thinking which is good when posting these patches.

I'm a little concerned here.  This sounds like forced unification.
If we can't clean up the infrastructure so things are obviously better
and cleanly factored for both architectures we should not unify the files.

As a general principle I would rather have two crudy files side by
side the one super crudy file.

So for unification I suggest finally fixing this right and taking the
apics completely out of the kexec on panic path.

Eric

Re: [PATCH 1/3] x86: add lapic_shutdown for x86_64

[Hiroshi Shimamoto]

Eric W. Biederman wrote:
> Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com> writes:
> 
>>> Do we really have to introduce this function for 64bit? I remember some
>>> issues were faced on i386 w.r.t kernel enabling the LAPIC against the
>>> wishes of BIOS hence kernel was disabling it while shutting down. No
>>> such problems were reported for x86_64 hence this function existed only
>>> for i386.
>> Thanks for the comment. I didn't know the issues, so I'd simply added
>> this function for unification.
>>
>>> If that is the case, probably we don't have to introduce lapic_shutdown()
>>> for x86_64. Instead call lapic_shutdown() for X86_32, and disble_local_APIC()
>>> otherwise?
>> I will do that. I was thinking which is good when posting these patches.
> 
> I'm a little concerned here.  This sounds like forced unification.
> If we can't clean up the infrastructure so things are obviously better
> and cleanly factored for both architectures we should not unify the files.
> 
> As a general principle I would rather have two crudy files side by
> side the one super crudy file.
> 
> So for unification I suggest finally fixing this right and taking the
> apics completely out of the kexec on panic path.

Thanks for the suggestion.
But it's hard for me to imagine.
I'll try to consider about it.

Thanks
Hiroshi Shimamoto

[PATCH] Revert x86: add lapic_shutdown for x86_64

[Hiroshi Shimamoto]

lapic_shutdown is useless on x86_64.

Signed-off-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
---
 arch/x86/kernel/apic_64.c |   14 --------------
 arch/x86/kernel/crash.c   |    5 +++++
 include/asm-x86/apic_64.h |    1 -
 3 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/arch/x86/kernel/apic_64.c b/arch/x86/kernel/apic_64.c
index f28ccb5..f47bc49 100644
--- a/arch/x86/kernel/apic_64.c
+++ b/arch/x86/kernel/apic_64.c
@@ -287,20 +287,6 @@ void disable_local_APIC(void)
 	apic_write(APIC_SPIV, value);
 }
 
-void lapic_shutdown(void)
-{
-	unsigned long flags;
-
-	if (!cpu_has_apic)
-		return;
-
-	local_irq_save(flags);
-
-	disable_local_APIC();
-
-	local_irq_restore(flags);
-}
-
 /*
  * This is to verify that we're looking at a real local APIC.
  * Check these against your board if the CPUs aren't getting
diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
index 8bb482f..79a5a25 100644
--- a/arch/x86/kernel/crash.c
+++ b/arch/x86/kernel/crash.c
@@ -136,7 +136,12 @@ void machine_crash_shutdown(struct pt_regs *regs)
 	/* Make a note of crashing cpu. Will be used in NMI callback.*/
 	crashing_cpu = safe_smp_processor_id();
 	nmi_shootdown_cpus();
+#ifdef CONFIG_X86_32
 	lapic_shutdown();
+#else
+	if (cpu_has_apic)
+		disable_local_APIC();
+#endif
 #if defined(CONFIG_X86_IO_APIC)
 	disable_IO_APIC();
 #endif
diff --git a/include/asm-x86/apic_64.h b/include/asm-x86/apic_64.h
index 2747a11..3c8f21e 100644
--- a/include/asm-x86/apic_64.h
+++ b/include/asm-x86/apic_64.h
@@ -69,7 +69,6 @@ extern void clear_local_APIC (void);
 extern void connect_bsp_APIC (void);
 extern void disconnect_bsp_APIC (int virt_wire_setup);
 extern void disable_local_APIC (void);
-extern void lapic_shutdown (void);
 extern int verify_local_APIC (void);
 extern void cache_APIC_registers (void);
 extern void sync_Arb_IDs (void);
-- 
1.5.3.4

Re: [PATCH] Revert x86: add lapic_shutdown for x86_64

[Arjan van de Ven]

On Mon, 29 Oct 2007 15:39:46 -0700
Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com> wrote:

> lapic_shutdown is useless on x86_64.
> 

.... but since the goal is to get apic_32.c and apic_64.c to be more
converging (to the point of becoming the same file)... isn't your patch
going in the opposite direction?

-- 
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [PATCH] Revert x86: add lapic_shutdown for x86_64

[Hiroshi Shimamoto]

Arjan van de Ven wrote:
> On Mon, 29 Oct 2007 15:39:46 -0700
> Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com> wrote:
> 
>> lapic_shutdown is useless on x86_64.
>>
> 
> .... but since the goal is to get apic_32.c and apic_64.c to be more
> converging (to the point of becoming the same file)... isn't your patch
> going in the opposite direction?
> 
Hmm, I'm not sure that this revert affects x86 unification.
Vivek said that probably we don't have to introduce lapic_shutdown() for 64bit.
So I submitted this patch which reverts my previous post, it was applied before
the comment.

Thanks
Hiroshi Shimamoto

Re: [PATCH] Revert x86: add lapic_shutdown for x86_64

[Thomas Gleixner]

On Mon, 29 Oct 2007, Hiroshi Shimamoto wrote:

> Arjan van de Ven wrote:
> > On Mon, 29 Oct 2007 15:39:46 -0700
> > Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com> wrote:
> > 
> >> lapic_shutdown is useless on x86_64.
> >>
> > 
> > .... but since the goal is to get apic_32.c and apic_64.c to be more
> > converging (to the point of becoming the same file)... isn't your patch
> > going in the opposite direction?
> > 
> Hmm, I'm not sure that this revert affects x86 unification.
> Vivek said that probably we don't have to introduce lapic_shutdown() for 64bit.
> So I submitted this patch which reverts my previous post, it was applied before
> the comment.

Don't worry, we sort this out. There is some inevitable friction loss
for now, but we are getting closer. I gave the apic code some care to
get a readable diff out of it. Result is in:

git://git.kernel.org/pub/scm/linux/kernel/git/x86/linux-2.6-x86.git cleanup

Thanks,

	tglx

[PATCH 2/3] x86: add safe_smp_processor_id for x86_64

[Hiroshi Shimamoto]

From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>

Signed-off-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
---
 include/asm-x86/smp_64.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/include/asm-x86/smp_64.h b/include/asm-x86/smp_64.h
index 6f0e027..ab612b0 100644
--- a/include/asm-x86/smp_64.h
+++ b/include/asm-x86/smp_64.h
@@ -76,6 +76,8 @@ extern unsigned __cpuinitdata disabled_cpus;
 
 #endif /* CONFIG_SMP */
 
+#define safe_smp_processor_id()		smp_processor_id()
+
 static inline int hard_smp_processor_id(void)
 {
 	/* we don't want to mark this access volatile - bad code generation */
-- 
1.5.2.3

Re: [PATCH 2/3] x86: add safe_smp_processor_id for x86_64

[Vivek Goyal]

On Fri, Oct 19, 2007 at 06:23:02PM -0700, Hiroshi Shimamoto wrote:
> From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
> 
> Signed-off-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
> ---
>  include/asm-x86/smp_64.h |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/include/asm-x86/smp_64.h b/include/asm-x86/smp_64.h
> index 6f0e027..ab612b0 100644
> --- a/include/asm-x86/smp_64.h
> +++ b/include/asm-x86/smp_64.h
> @@ -76,6 +76,8 @@ extern unsigned __cpuinitdata disabled_cpus;
> 
>  #endif /* CONFIG_SMP */
> 
> +#define safe_smp_processor_id()		smp_processor_id()
> +

Can you please implement a patch for safe_smp_processor_id() instead of
using smp_processor_id(). safe_smp_processor_id() was introduced to make
sure that we are not dependent on the stack of threads after kernel has
crashed instead read the apic id and convert it to cpu id with other
data structures. This helped in stack overflow case.

Hardcoding it to smp_processor_id() will give the false impression.

Thanks
Vivek

Re: [PATCH 2/3] x86: add safe_smp_processor_id for x86_64

[Vivek Goyal]

On Wed, Oct 24, 2007 at 12:01:41PM +0530, Vivek Goyal wrote:
> On Fri, Oct 19, 2007 at 06:23:02PM -0700, Hiroshi Shimamoto wrote:
> > From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
> > 
> > Signed-off-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
> > ---
> >  include/asm-x86/smp_64.h |    2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> > 
> > diff --git a/include/asm-x86/smp_64.h b/include/asm-x86/smp_64.h
> > index 6f0e027..ab612b0 100644
> > --- a/include/asm-x86/smp_64.h
> > +++ b/include/asm-x86/smp_64.h
> > @@ -76,6 +76,8 @@ extern unsigned __cpuinitdata disabled_cpus;
> > 
> >  #endif /* CONFIG_SMP */
> > 
> > +#define safe_smp_processor_id()		smp_processor_id()
> > +
> 
> Can you please implement a patch for safe_smp_processor_id() instead of
> using smp_processor_id(). safe_smp_processor_id() was introduced to make
> sure that we are not dependent on the stack of threads after kernel has
> crashed instead read the apic id and convert it to cpu id with other
> data structures. This helped in stack overflow case.
> 
> Hardcoding it to smp_processor_id() will give the false impression.
> 

Just now Aneesh pointed that x86_64 using pda for retrieving processor id
and not kernel stack.

I think it is fine then.

Thanks
Vivek

[PATCH 3/3] x86: unify crash_32/64.c

[Hiroshi Shimamoto]

From: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>

Most of contents in crash are same.

Signed-off-by: Hiroshi Shimamoto <h-shimamoto@ct.jp.nec.com>
---
 arch/x86/kernel/Makefile_32 |    2 +-
 arch/x86/kernel/Makefile_64 |    2 +-
 arch/x86/kernel/crash.c     |  144 +++++++++++++++++++++++++++++++++++++++++++
 arch/x86/kernel/crash_32.c  |  137 ----------------------------------------
 arch/x86/kernel/crash_64.c  |  135 ----------------------------------------
 5 files changed, 146 insertions(+), 274 deletions(-)
 create mode 100644 arch/x86/kernel/crash.c
 delete mode 100644 arch/x86/kernel/crash_32.c
 delete mode 100644 arch/x86/kernel/crash_64.c

diff --git a/arch/x86/kernel/Makefile_32 b/arch/x86/kernel/Makefile_32
index ccea590..b9d6798 100644
--- a/arch/x86/kernel/Makefile_32
+++ b/arch/x86/kernel/Makefile_32
@@ -26,7 +26,7 @@ obj-$(CONFIG_X86_MPPARSE)	+= mpparse_32.o
 obj-$(CONFIG_X86_LOCAL_APIC)	+= apic_32.o nmi_32.o
 obj-$(CONFIG_X86_IO_APIC)	+= io_apic_32.o
 obj-$(CONFIG_X86_REBOOTFIXUPS)	+= reboot_fixups_32.o
-obj-$(CONFIG_KEXEC)		+= machine_kexec_32.o relocate_kernel_32.o crash_32.o
+obj-$(CONFIG_KEXEC)		+= machine_kexec_32.o relocate_kernel_32.o crash.o
 obj-$(CONFIG_CRASH_DUMP)	+= crash_dump_32.o
 obj-$(CONFIG_X86_NUMAQ)		+= numaq_32.o
 obj-$(CONFIG_X86_SUMMIT_NUMA)	+= summit_32.o
diff --git a/arch/x86/kernel/Makefile_64 b/arch/x86/kernel/Makefile_64
index dec06e7..7b917d4 100644
--- a/arch/x86/kernel/Makefile_64
+++ b/arch/x86/kernel/Makefile_64
@@ -23,7 +23,7 @@ obj-$(CONFIG_X86_CPUID)		+= cpuid.o
 obj-$(CONFIG_SMP)		+= smp_64.o smpboot_64.o trampoline_64.o tsc_sync.o
 obj-y				+= apic_64.o  nmi_64.o
 obj-y				+= io_apic_64.o mpparse_64.o genapic_64.o genapic_flat_64.o
-obj-$(CONFIG_KEXEC)		+= machine_kexec_64.o relocate_kernel_64.o crash_64.o
+obj-$(CONFIG_KEXEC)		+= machine_kexec_64.o relocate_kernel_64.o crash.o
 obj-$(CONFIG_CRASH_DUMP)	+= crash_dump_64.o
 obj-$(CONFIG_PM)		+= suspend_64.o
 obj-$(CONFIG_HIBERNATION)	+= suspend_asm_64.o
diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
new file mode 100644
index 0000000..af0253f
--- /dev/null
+++ b/arch/x86/kernel/crash.c
@@ -0,0 +1,144 @@
+/*
+ * Architecture specific (i386/x86_64) functions for kexec based crash dumps.
+ *
+ * Created by: Hariprasad Nellitheertha (hari@in.ibm.com)
+ *
+ * Copyright (C) IBM Corporation, 2004. All rights reserved.
+ *
+ */
+
+#include <linux/init.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/smp.h>
+#include <linux/reboot.h>
+#include <linux/kexec.h>
+#include <linux/delay.h>
+#include <linux/elf.h>
+#include <linux/elfcore.h>
+
+#include <asm/processor.h>
+#include <asm/hardirq.h>
+#include <asm/nmi.h>
+#include <asm/hw_irq.h>
+#include <asm/apic.h>
+#include <linux/kdebug.h>
+#include <asm/smp.h>
+
+#ifdef X86_32
+#include <mach_ipi.h>
+#else
+#include <asm/mach_apic.h>
+#endif
+
+/* This keeps a track of which one is crashing cpu. */
+static int crashing_cpu;
+
+#if defined(CONFIG_SMP) && defined(CONFIG_X86_LOCAL_APIC)
+static atomic_t waiting_for_crash_ipi;
+
+static int crash_nmi_callback(struct notifier_block *self,
+			unsigned long val, void *data)
+{
+	struct pt_regs *regs;
+#ifdef X86_32
+	struct pt_regs fixed_regs;
+#endif
+	int cpu;
+
+	if (val != DIE_NMI_IPI)
+		return NOTIFY_OK;
+
+	regs = ((struct die_args *)data)->regs;
+	cpu = raw_smp_processor_id();
+
+	/* Don't do anything if this handler is invoked on crashing cpu.
+	 * Otherwise, system will completely hang. Crashing cpu can get
+	 * an NMI if system was initially booted with nmi_watchdog parameter.
+	 */
+	if (cpu == crashing_cpu)
+		return NOTIFY_STOP;
+	local_irq_disable();
+
+#ifdef X86_32
+	if (!user_mode_vm(regs)) {
+		crash_fixup_ss_esp(&fixed_regs, regs);
+		regs = &fixed_regs;
+	}
+#endif
+	crash_save_cpu(regs, cpu);
+	disable_local_APIC();
+	atomic_dec(&waiting_for_crash_ipi);
+	/* Assume hlt works */
+	halt();
+	for (;;)
+		cpu_relax();
+
+	return 1;
+}
+
+static void smp_send_nmi_allbutself(void)
+{
+	cpumask_t mask = cpu_online_map;
+	cpu_clear(safe_smp_processor_id(), mask);
+	if (!cpus_empty(mask))
+		send_IPI_mask(mask, NMI_VECTOR);
+}
+
+static struct notifier_block crash_nmi_nb = {
+	.notifier_call = crash_nmi_callback,
+};
+
+static void nmi_shootdown_cpus(void)
+{
+	unsigned long msecs;
+
+	atomic_set(&waiting_for_crash_ipi, num_online_cpus() - 1);
+	/* Would it be better to replace the trap vector here? */
+	if (register_die_notifier(&crash_nmi_nb))
+		return;		/* return what? */
+	/* Ensure the new callback function is set before sending
+	 * out the NMI
+	 */
+	wmb();
+
+	smp_send_nmi_allbutself();
+
+	msecs = 1000; /* Wait at most a second for the other cpus to stop */
+	while ((atomic_read(&waiting_for_crash_ipi) > 0) && msecs) {
+		mdelay(1);
+		msecs--;
+	}
+
+	/* Leave the nmi callback set */
+	disable_local_APIC();
+}
+#else
+static void nmi_shootdown_cpus(void)
+{
+	/* There are no cpus to shootdown */
+}
+#endif
+
+void machine_crash_shutdown(struct pt_regs *regs)
+{
+	/* This function is only called after the system
+	 * has panicked or is otherwise in a critical state.
+	 * The minimum amount of code to allow a kexec'd kernel
+	 * to run successfully needs to happen here.
+	 *
+	 * In practice this means shooting down the other cpus in
+	 * an SMP system.
+	 */
+	/* The kernel is broken so disable interrupts */
+	local_irq_disable();
+
+	/* Make a note of crashing cpu. Will be used in NMI callback.*/
+	crashing_cpu = safe_smp_processor_id();
+	nmi_shootdown_cpus();
+	lapic_shutdown();
+#if defined(CONFIG_X86_IO_APIC)
+	disable_IO_APIC();
+#endif
+	crash_save_cpu(regs, safe_smp_processor_id());
+}
diff --git a/arch/x86/kernel/crash_32.c b/arch/x86/kernel/crash_32.c
deleted file mode 100644
index 53589d1..0000000
--- a/arch/x86/kernel/crash_32.c
+++ /dev/null
@@ -1,137 +0,0 @@
-/*
- * Architecture specific (i386) functions for kexec based crash dumps.
- *
- * Created by: Hariprasad Nellitheertha (hari@in.ibm.com)
- *
- * Copyright (C) IBM Corporation, 2004. All rights reserved.
- *
- */
-
-#include <linux/init.h>
-#include <linux/types.h>
-#include <linux/kernel.h>
-#include <linux/smp.h>
-#include <linux/reboot.h>
-#include <linux/kexec.h>
-#include <linux/delay.h>
-#include <linux/elf.h>
-#include <linux/elfcore.h>
-
-#include <asm/processor.h>
-#include <asm/hardirq.h>
-#include <asm/nmi.h>
-#include <asm/hw_irq.h>
-#include <asm/apic.h>
-#include <linux/kdebug.h>
-#include <asm/smp.h>
-
-#include <mach_ipi.h>
-
-
-/* This keeps a track of which one is crashing cpu. */
-static int crashing_cpu;
-
-#if defined(CONFIG_SMP) && defined(CONFIG_X86_LOCAL_APIC)
-static atomic_t waiting_for_crash_ipi;
-
-static int crash_nmi_callback(struct notifier_block *self,
-			unsigned long val, void *data)
-{
-	struct pt_regs *regs;
-	struct pt_regs fixed_regs;
-	int cpu;
-
-	if (val != DIE_NMI_IPI)
-		return NOTIFY_OK;
-
-	regs = ((struct die_args *)data)->regs;
-	cpu = raw_smp_processor_id();
-
-	/* Don't do anything if this handler is invoked on crashing cpu.
-	 * Otherwise, system will completely hang. Crashing cpu can get
-	 * an NMI if system was initially booted with nmi_watchdog parameter.
-	 */
-	if (cpu == crashing_cpu)
-		return NOTIFY_STOP;
-	local_irq_disable();
-
-	if (!user_mode_vm(regs)) {
-		crash_fixup_ss_esp(&fixed_regs, regs);
-		regs = &fixed_regs;
-	}
-	crash_save_cpu(regs, cpu);
-	disable_local_APIC();
-	atomic_dec(&waiting_for_crash_ipi);
-	/* Assume hlt works */
-	halt();
-	for (;;)
-		cpu_relax();
-
-	return 1;
-}
-
-static void smp_send_nmi_allbutself(void)
-{
-	cpumask_t mask = cpu_online_map;
-	cpu_clear(safe_smp_processor_id(), mask);
-	if (!cpus_empty(mask))
-		send_IPI_mask(mask, NMI_VECTOR);
-}
-
-static struct notifier_block crash_nmi_nb = {
-	.notifier_call = crash_nmi_callback,
-};
-
-static void nmi_shootdown_cpus(void)
-{
-	unsigned long msecs;
-
-	atomic_set(&waiting_for_crash_ipi, num_online_cpus() - 1);
-	/* Would it be better to replace the trap vector here? */
-	if (register_die_notifier(&crash_nmi_nb))
-		return;		/* return what? */
-	/* Ensure the new callback function is set before sending
-	 * out the NMI
-	 */
-	wmb();
-
-	smp_send_nmi_allbutself();
-
-	msecs = 1000; /* Wait at most a second for the other cpus to stop */
-	while ((atomic_read(&waiting_for_crash_ipi) > 0) && msecs) {
-		mdelay(1);
-		msecs--;
-	}
-
-	/* Leave the nmi callback set */
-	disable_local_APIC();
-}
-#else
-static void nmi_shootdown_cpus(void)
-{
-	/* There are no cpus to shootdown */
-}
-#endif
-
-void machine_crash_shutdown(struct pt_regs *regs)
-{
-	/* This function is only called after the system
-	 * has panicked or is otherwise in a critical state.
-	 * The minimum amount of code to allow a kexec'd kernel
-	 * to run successfully needs to happen here.
-	 *
-	 * In practice this means shooting down the other cpus in
-	 * an SMP system.
-	 */
-	/* The kernel is broken so disable interrupts */
-	local_irq_disable();
-
-	/* Make a note of crashing cpu. Will be used in NMI callback.*/
-	crashing_cpu = safe_smp_processor_id();
-	nmi_shootdown_cpus();
-	lapic_shutdown();
-#if defined(CONFIG_X86_IO_APIC)
-	disable_IO_APIC();
-#endif
-	crash_save_cpu(regs, safe_smp_processor_id());
-}
diff --git a/arch/x86/kernel/crash_64.c b/arch/x86/kernel/crash_64.c
deleted file mode 100644
index 13432a1..0000000
--- a/arch/x86/kernel/crash_64.c
+++ /dev/null
@@ -1,135 +0,0 @@
-/*
- * Architecture specific (x86_64) functions for kexec based crash dumps.
- *
- * Created by: Hariprasad Nellitheertha (hari@in.ibm.com)
- *
- * Copyright (C) IBM Corporation, 2004. All rights reserved.
- *
- */
-
-#include <linux/init.h>
-#include <linux/types.h>
-#include <linux/kernel.h>
-#include <linux/smp.h>
-#include <linux/irq.h>
-#include <linux/reboot.h>
-#include <linux/kexec.h>
-#include <linux/delay.h>
-#include <linux/elf.h>
-#include <linux/elfcore.h>
-#include <linux/kdebug.h>
-
-#include <asm/processor.h>
-#include <asm/hardirq.h>
-#include <asm/nmi.h>
-#include <asm/hw_irq.h>
-#include <asm/mach_apic.h>
-
-/* This keeps a track of which one is crashing cpu. */
-static int crashing_cpu;
-
-#ifdef CONFIG_SMP
-static atomic_t waiting_for_crash_ipi;
-
-static int crash_nmi_callback(struct notifier_block *self,
-				unsigned long val, void *data)
-{
-	struct pt_regs *regs;
-	int cpu;
-
-	if (val != DIE_NMI_IPI)
-		return NOTIFY_OK;
-
-	regs = ((struct die_args *)data)->regs;
-	cpu = raw_smp_processor_id();
-
-	/*
-	 * Don't do anything if this handler is invoked on crashing cpu.
-	 * Otherwise, system will completely hang. Crashing cpu can get
-	 * an NMI if system was initially booted with nmi_watchdog parameter.
-	 */
-	if (cpu == crashing_cpu)
-		return NOTIFY_STOP;
-	local_irq_disable();
-
-	crash_save_cpu(regs, cpu);
-	disable_local_APIC();
-	atomic_dec(&waiting_for_crash_ipi);
-	/* Assume hlt works */
-	for(;;)
-		halt();
-
-	return 1;
-}
-
-static void smp_send_nmi_allbutself(void)
-{
-	send_IPI_allbutself(NMI_VECTOR);
-}
-
-/*
- * This code is a best effort heuristic to get the
- * other cpus to stop executing. So races with
- * cpu hotplug shouldn't matter.
- */
-
-static struct notifier_block crash_nmi_nb = {
-	.notifier_call = crash_nmi_callback,
-};
-
-static void nmi_shootdown_cpus(void)
-{
-	unsigned long msecs;
-
-	atomic_set(&waiting_for_crash_ipi, num_online_cpus() - 1);
-	if (register_die_notifier(&crash_nmi_nb))
-		return;         /* return what? */
-
-	/*
-	 * Ensure the new callback function is set before sending
-	 * out the NMI
-	 */
-	wmb();
-
-	smp_send_nmi_allbutself();
-
-	msecs = 1000; /* Wait at most a second for the other cpus to stop */
-	while ((atomic_read(&waiting_for_crash_ipi) > 0) && msecs) {
-		mdelay(1);
-		msecs--;
-	}
-	/* Leave the nmi callback set */
-	disable_local_APIC();
-}
-#else
-static void nmi_shootdown_cpus(void)
-{
-	/* There are no cpus to shootdown */
-}
-#endif
-
-void machine_crash_shutdown(struct pt_regs *regs)
-{
-	/*
-	 * This function is only called after the system
-	 * has panicked or is otherwise in a critical state.
-	 * The minimum amount of code to allow a kexec'd kernel
-	 * to run successfully needs to happen here.
-	 *
-	 * In practice this means shooting down the other cpus in
-	 * an SMP system.
-	 */
-	/* The kernel is broken so disable interrupts */
-	local_irq_disable();
-
-	/* Make a note of crashing cpu. Will be used in NMI callback.*/
-	crashing_cpu = smp_processor_id();
-	nmi_shootdown_cpus();
-
-	if(cpu_has_apic)
-		 disable_local_APIC();
-
-	disable_IO_APIC();
-
-	crash_save_cpu(regs, smp_processor_id());
-}
-- 
1.5.2.3

Re: [PATCH 0/3] x86: unify crash_32/64.c

[Thomas Gleixner]

On Fri, 19 Oct 2007, Hiroshi Shimamoto wrote:

> Hi,
> 
> I made patches to unify crash_32/64.c.
> There are three patches;
> 1. add lapic_shutdown for x86_64
> 2. add safe_smp_processor_id for x86_64
> 3. unify crash_32/64.c
> 
> I'm not sure that it's good to split to these patches.

It's fine. So the steps leading to unification are clear.

Applied. Thanks,

	 tglx


	

Re: [PATCH 0/3] x86: unify crash_32/64.c

[Vivek Goyal]

On Fri, Oct 19, 2007 at 06:18:27PM -0700, Hiroshi Shimamoto wrote:
> Hi,
> 
> I made patches to unify crash_32/64.c.
> There are three patches;
> 1. add lapic_shutdown for x86_64
> 2. add safe_smp_processor_id for x86_64
> 3. unify crash_32/64.c
> 
> I'm not sure that it's good to split to these patches.
> 
> I've compiled on both of 32bit and 64bit, and tested
> kdump on 64bit.
> 

Hi Hiroshi,

Thanks for the patches. Can you please also test it on 32bit to make
sure nothing is broken.

Thanks
Vivek

Re: [PATCH 0/3] x86: unify crash_32/64.c

[Hiroshi Shimamoto]

Vivek Goyal wrote:
> On Fri, Oct 19, 2007 at 06:18:27PM -0700, Hiroshi Shimamoto wrote:
>> Hi,
>>
>> I made patches to unify crash_32/64.c.
>> There are three patches;
>> 1. add lapic_shutdown for x86_64
>> 2. add safe_smp_processor_id for x86_64
>> 3. unify crash_32/64.c
>>
>> I'm not sure that it's good to split to these patches.
>>
>> I've compiled on both of 32bit and 64bit, and tested
>> kdump on 64bit.
>>
> 
> Hi Hiroshi,
> 
> Thanks for the patches. Can you please also test it on 32bit to make
> sure nothing is broken.

Okay, I'll test it on 32bit.
A build problem already has been found on 32bit.

Thanks,
Hiroshi

Re: [PATCH 0/3] x86: unify crash_32/64.c

[Hiroshi Shimamoto]

Hiroshi Shimamoto wrote:
> Vivek Goyal wrote:
>> On Fri, Oct 19, 2007 at 06:18:27PM -0700, Hiroshi Shimamoto wrote:
>>> Hi,
>>>
>>> I made patches to unify crash_32/64.c.
>>> There are three patches;
>>> 1. add lapic_shutdown for x86_64
>>> 2. add safe_smp_processor_id for x86_64
>>> 3. unify crash_32/64.c
>>>
>>> I'm not sure that it's good to split to these patches.
>>>
>>> I've compiled on both of 32bit and 64bit, and tested
>>> kdump on 64bit.
>>>
>> Hi Hiroshi,
>>
>> Thanks for the patches. Can you please also test it on 32bit to make
>> sure nothing is broken.
> 
> Okay, I'll test it on 32bit.
> A build problem already has been found on 32bit.
> 
I'm now testing crash on 32bit, but there is an issue before
applying the patches. My machine stopped at checking 'hlt'
after kexec, showing below message.

CPU: Intel(R) Xeon(TM) CPU 3.80GHz stepping 0a
Checking 'hlt' instruction...

v2.6.23.1 works fine for 1st kernel.
I'm investigating it..

Thanks
Hiroshi Shimamoto

Re: [PATCH 0/3] x86: unify crash_32/64.c

[Hiroshi Shimamoto]

> I'm now testing crash on 32bit, but there is an issue before
> applying the patches. My machine stopped at checking 'hlt'
> after kexec, showing below message.
> 
> CPU: Intel(R) Xeon(TM) CPU 3.80GHz stepping 0a
> Checking 'hlt' instruction...
> 
> v2.6.23.1 works fine for 1st kernel.
> I'm investigating it..

I found that the following patch makes my machine stopped.
bfe0c1cc6456bba1f4e3cc1fe29c0ea578ac763a
x86: HPET force enable for ICH5

It means that after applied this patch, HPET is enabled
automatically on 1st kernel and after crash/kexec the 2nd
kernel stopped at checking 'hlt'.

I also tested the latest kernel(2.6.24-rc1-gec3b67c1).
Boot parameter "nohpet" resolves this issue and kdump
works well on 32bit.
So I guess HPET affects this.
But I don't know why 64bit kernel with HPET is OK.

Thanks
Hiroshi Shimamoto

Re: [PATCH 0/3] x86: unify crash_32/64.c

[Thomas Gleixner]

On Fri, 26 Oct 2007, Hiroshi Shimamoto wrote:

Added Venki to CC

> > I'm now testing crash on 32bit, but there is an issue before
> > applying the patches. My machine stopped at checking 'hlt'
> > after kexec, showing below message.
> > 
> > CPU: Intel(R) Xeon(TM) CPU 3.80GHz stepping 0a
> > Checking 'hlt' instruction...
> > 
> v2.6.23.1 works fine for 1st kernel.
> > I'm investigating it..
> 
> I found that the following patch makes my machine stopped.
> bfe0c1cc6456bba1f4e3cc1fe29c0ea578ac763a
> x86: HPET force enable for ICH5
> 
> It means that after applied this patch, HPET is enabled
> automatically on 1st kernel and after crash/kexec the 2nd
> kernel stopped at checking 'hlt'.
> 
> I also tested the latest kernel(2.6.24-rc1-gec3b67c1).
> Boot parameter "nohpet" resolves this issue and kdump
> works well on 32bit.
> So I guess HPET affects this.
> But I don't know why 64bit kernel with HPET is OK.

Hmm. Does the 64 bit code shutdown HPET and restore the IRQ routing to
PIT and 32 bit is missing this ?

    tglx

Re: [PATCH 0/3] x86: unify crash_32/64.c

[Hiroshi Shimamoto]

Thomas Gleixner wrote:
> On Fri, 26 Oct 2007, Hiroshi Shimamoto wrote:
> 
> Added Venki to CC
> 
>>> I'm now testing crash on 32bit, but there is an issue before
>>> applying the patches. My machine stopped at checking 'hlt'
>>> after kexec, showing below message.
>>>
>>> CPU: Intel(R) Xeon(TM) CPU 3.80GHz stepping 0a
>>> Checking 'hlt' instruction...
>>>
>> v2.6.23.1 works fine for 1st kernel.
>>> I'm investigating it..
>> I found that the following patch makes my machine stopped.
>> bfe0c1cc6456bba1f4e3cc1fe29c0ea578ac763a
>> x86: HPET force enable for ICH5
>>
>> It means that after applied this patch, HPET is enabled
>> automatically on 1st kernel and after crash/kexec the 2nd
>> kernel stopped at checking 'hlt'.
>>
>> I also tested the latest kernel(2.6.24-rc1-gec3b67c1).
>> Boot parameter "nohpet" resolves this issue and kdump
>> works well on 32bit.
>> So I guess HPET affects this.
>> But I don't know why 64bit kernel with HPET is OK.
> 
> Hmm. Does the 64 bit code shutdown HPET and restore the IRQ routing to
> PIT and 32 bit is missing this ?

Sorry, I'm not sure how I can get these informations.
Can you please tell me what I should do?
I'll continue to dig the issue.

I also have the following message.
..MP-BIOS bug: 8254 timer not connected to IO-APIC
Kernel panic - not syncing: IO-APIC + timer doesn't work! Try using the 'noapic' kernel parameter

It appeared only on 64bit and the 2nd kernel without
boot parameter noapic.


Thanks
Hiroshi Shimamoto

Re: [PATCH 0/3] x86: unify crash_32/64.c

[Hiroshi Shimamoto]

[-- Attachment #1: Type: text/plain, Size: 1590 bytes --]

Hiroshi Shimamoto wrote:
> Thomas Gleixner wrote:
>> On Fri, 26 Oct 2007, Hiroshi Shimamoto wrote:
>>
>> Added Venki to CC
>>
>>>> I'm now testing crash on 32bit, but there is an issue before
>>>> applying the patches. My machine stopped at checking 'hlt'
>>>> after kexec, showing below message.
>>>>
>>>> CPU: Intel(R) Xeon(TM) CPU 3.80GHz stepping 0a
>>>> Checking 'hlt' instruction...
>>>>
>>> v2.6.23.1 works fine for 1st kernel.
>>>> I'm investigating it..
>>> I found that the following patch makes my machine stopped.
>>> bfe0c1cc6456bba1f4e3cc1fe29c0ea578ac763a
>>> x86: HPET force enable for ICH5
>>>
>>> It means that after applied this patch, HPET is enabled
>>> automatically on 1st kernel and after crash/kexec the 2nd
>>> kernel stopped at checking 'hlt'.
>>>
>>> I also tested the latest kernel(2.6.24-rc1-gec3b67c1).
>>> Boot parameter "nohpet" resolves this issue and kdump
>>> works well on 32bit.
>>> So I guess HPET affects this.
>>> But I don't know why 64bit kernel with HPET is OK.
>> Hmm. Does the 64 bit code shutdown HPET and restore the IRQ routing to
>> PIT and 32 bit is missing this ?
> 
> Sorry, I'm not sure how I can get these informations.
> Can you please tell me what I should do?
> I'll continue to dig the issue.

I attached the .config files and console logs.
config32/64 are for 1st kernel, and cap32/64 are for 2nd capture kernel.
kdump1.log is boot with nohpet on 32bit.
kdump2.log is boot without nohpet on 32bit and the 2nd kernel hangs.
kdump3.log is on 64bit. And the first kdump is failed because of
without noapic.

Thanks
Hiroshi Shimamoto

[-- Attachment #2: configs.tar.bz2 --]
[-- Type: application/octet-stream, Size: 11849 bytes --]

[-- Attachment #3: consolelog.tar.bz2 --]
[-- Type: application/octet-stream, Size: 13358 bytes --]