2005-10-04 15:06:27

by Eric W. Biederman

[permalink] [raw]
Subject: [PATCH] i386: move apic init in init_IRQs


All kinds of ugliness exists because we don't initialize
the apics during init_IRQs.
- We calibrate jiffies in non apic mode even when we are using apics.
- We have to have special code to initialize the apics when non-smp.
- The legacy i8259 must exist and be setup correctly, even
when we won't use it past initialization.
- The kexec on panic code must restore the state of the io_apics.
- init/main.c needs a special case for !smp smp_init on x86

In addition to pure code movement I needed a couple
of non-obvious changes:
- Move setup_boot_APIC_clock into APIC_late_time_init for
simplicity.
- Use cpu_khz to generate a better approximation of loops_per_jiffies
so I can verify the timer interrupt is working.
- Call setup_apic_nmi_watchdog again after cpu_khz is initialized on
the boot cpu.

Signed-off-by: Eric W. Biederman <[email protected]>


---

arch/i386/kernel/apic.c | 83 ++++++++++++++++++++-----
arch/i386/kernel/i8259.c | 4 +
arch/i386/kernel/io_apic.c | 6 ++
arch/i386/kernel/smpboot.c | 68 +++++---------------
arch/i386/kernel/time.c | 14 ++++
include/asm-i386/apic.h | 3 +
include/asm-i386/hw_irq.h | 1
include/asm-i386/mach-default/smpboot_hooks.h | 15 -----
include/asm-i386/mach-visws/smpboot_hooks.h | 7 --
init/main.c | 11 ---
10 files changed, 106 insertions(+), 106 deletions(-)

a3b2016a27cf9d583dce93f2375af7277049000f
diff --git a/arch/i386/kernel/apic.c b/arch/i386/kernel/apic.c
--- a/arch/i386/kernel/apic.c
+++ b/arch/i386/kernel/apic.c
@@ -803,6 +803,7 @@ no_apic:

void __init init_apic_mappings(void)
{
+ unsigned int orig_apicid;
unsigned long apic_phys;

/*
@@ -824,8 +825,11 @@ void __init init_apic_mappings(void)
* Fetch the APIC ID of the BSP in case we have a
* default configuration (or the MP table is broken).
*/
- if (boot_cpu_physical_apicid == -1U)
- boot_cpu_physical_apicid = GET_APIC_ID(apic_read(APIC_ID));
+ orig_apicid = boot_cpu_physical_apicid;
+ boot_cpu_physical_apicid = GET_APIC_ID(apic_read(APIC_ID));
+ if ((orig_apicid != -1U) && (orig_apicid != boot_cpu_physical_apicid))
+ printk(KERN_WARNING "Boot APIC ID in local APIC unexpected (%d vs %d)",
+ orig_apicid, boot_cpu_physical_apicid);

#ifdef CONFIG_X86_IO_APIC
{
@@ -1046,9 +1050,11 @@ static unsigned int calibration_result;

void __init setup_boot_APIC_clock(void)
{
+ unsigned long flags;
apic_printk(APIC_VERBOSE, "Using local APIC timer interrupts.\n");
using_apic_timer = 1;

+ local_irq_save(flags);
local_irq_disable();

calibration_result = calibrate_APIC_clock();
@@ -1057,7 +1063,7 @@ void __init setup_boot_APIC_clock(void)
*/
setup_APIC_timer(calibration_result);

- local_irq_enable();
+ local_irq_restore(flags);
}

void __devinit setup_secondary_APIC_clock(void)
@@ -1254,40 +1260,81 @@ fastcall void smp_error_interrupt(struct
}

/*
- * This initializes the IO-APIC and APIC hardware if this is
- * a UP kernel.
+ * This initializes the IO-APIC and APIC hardware.
*/
-int __init APIC_init_uniprocessor (void)
+int __init APIC_init(void)
{
- if (enable_local_apic < 0)
- clear_bit(X86_FEATURE_APIC, boot_cpu_data.x86_capability);
-
- if (!smp_found_config && !cpu_has_apic)
+ if (enable_local_apic < 0) {
+ printk(KERN_INFO "Apic disabled\n");
+ return -1;
+ }
+
+ /* See if we have a SMP configuration or have forced enabled
+ * the local apic.
+ */
+ if (!smp_found_config && !acpi_lapic && !cpu_has_apic) {
+ enable_local_apic = -1;
return -1;
+ }

/*
- * Complain if the BIOS pretends there is one.
+ * Complain if the BIOS pretends there is an apic.
+ * Then get out because we don't have an a local apic.
*/
if (!cpu_has_apic && APIC_INTEGRATED(apic_version[boot_cpu_physical_apicid])) {
printk(KERN_ERR "BIOS bug, local APIC #%d not detected!...\n",
boot_cpu_physical_apicid);
+ printk(KERN_ERR "... forcing use of dummy APIC emulation. (tell your hw vendor)\n");
+ enable_local_apic = -1;
return -1;
}

verify_local_APIC();

+ /*
+ * Should not be necessary because the MP table should list the boot
+ * CPU too, but we do it for the sake of robustness anyway.
+ * Makes no sense to do this check in clustered apic mode, so skip it
+ */
+ if (!check_phys_apicid_present(boot_cpu_physical_apicid)) {
+ printk("weird, boot CPU (#%d) not listed by the BIOS.\n",
+ boot_cpu_physical_apicid);
+ physid_set(hard_smp_processor_id(), phys_cpu_present_map);
+ }
+
+ /*
+ * Switch from PIC to APIC mode.
+ */
connect_bsp_APIC();
+ setup_local_APIC();

- phys_cpu_present_map = physid_mask_of_physid(boot_cpu_physical_apicid);
+#ifdef CONFIG_X86_IO_APIC
+ /*
+ * Now start the IO-APICs
+ */
+ if (smp_found_config && !skip_ioapic_setup && nr_ioapics)
+ setup_IO_APIC();
+#endif
+ return 0;
+}

- setup_local_APIC();
+void __init APIC_late_time_init(void)
+{
+ /* Improve our loops per jiffy estimate */
+ loops_per_jiffy = ((1000 + HZ - 1)/HZ)*cpu_khz;
+ boot_cpu_data.loops_per_jiffy = loops_per_jiffy;
+ cpu_data[0].loops_per_jiffy = loops_per_jiffy;
+
+ /* setup_apic_nmi_watchdog doesn't work properly before cpu_khz is
+ * initialized. So redo it here to ensure the boot cpu is setup
+ * properly.
+ */
+ if (nmi_watchdog == NMI_LOCAL_APIC)
+ setup_apic_nmi_watchdog();

#ifdef CONFIG_X86_IO_APIC
- if (smp_found_config)
- if (!skip_ioapic_setup && nr_ioapics)
- setup_IO_APIC();
+ if (smp_found_config && !skip_ioapic_setup && nr_ioapics)
+ IO_APIC_late_time_init();
#endif
setup_boot_APIC_clock();
-
- return 0;
}
diff --git a/arch/i386/kernel/i8259.c b/arch/i386/kernel/i8259.c
--- a/arch/i386/kernel/i8259.c
+++ b/arch/i386/kernel/i8259.c
@@ -435,4 +435,8 @@ void __init init_IRQ(void)
setup_irq(FPU_IRQ, &fpu_irq);

irq_ctx_init(smp_processor_id());
+
+#ifdef CONFIG_X86_LOCAL_APIC
+ APIC_init();
+#endif
}
diff --git a/arch/i386/kernel/io_apic.c b/arch/i386/kernel/io_apic.c
--- a/arch/i386/kernel/io_apic.c
+++ b/arch/i386/kernel/io_apic.c
@@ -2310,11 +2310,15 @@ void __init setup_IO_APIC(void)
sync_Arb_IDs();
setup_IO_APIC_irqs();
init_IO_APIC_traps();
- check_timer();
if (!acpi_ioapic)
print_IO_APIC();
}

+void __init IO_APIC_late_time_init(void)
+{
+ check_timer();
+}
+
/*
* Called after all the initialization is done. If we didnt find any
* APIC bugs then we can allow the modify fast path
diff --git a/arch/i386/kernel/smpboot.c b/arch/i386/kernel/smpboot.c
--- a/arch/i386/kernel/smpboot.c
+++ b/arch/i386/kernel/smpboot.c
@@ -1074,6 +1074,16 @@ void *xquad_portio;
EXPORT_SYMBOL(xquad_portio);
#endif

+/*
+ * Fall back to non SMP mode after errors.
+ *
+ */
+static __init void disable_smp(void)
+{
+ cpu_set(0, cpu_sibling_map[0]);
+ cpu_set(0, cpu_core_map[0]);
+}
+
static void __init smp_boot_cpus(unsigned int max_cpus)
{
int apicid, cpu, bit, kicked;
@@ -1086,7 +1096,6 @@ static void __init smp_boot_cpus(unsigne
printk("CPU%d: ", 0);
print_cpu_info(&cpu_data[0]);

- boot_cpu_physical_apicid = GET_APIC_ID(apic_read(APIC_ID));
boot_cpu_logical_apicid = logical_smp_processor_id();
x86_cpu_to_apicid[0] = boot_cpu_physical_apicid;

@@ -1098,68 +1107,27 @@ static void __init smp_boot_cpus(unsigne
cpus_clear(cpu_core_map[0]);
cpu_set(0, cpu_core_map[0]);

+ map_cpu_to_logical_apicid();
+
/*
* If we couldn't find an SMP configuration at boot time,
* get out of here now!
*/
if (!smp_found_config && !acpi_lapic) {
printk(KERN_NOTICE "SMP motherboard not detected.\n");
- smpboot_clear_io_apic_irqs();
- phys_cpu_present_map = physid_mask_of_physid(0);
- if (APIC_init_uniprocessor())
- printk(KERN_NOTICE "Local APIC not detected."
- " Using dummy APIC emulation.\n");
- map_cpu_to_logical_apicid();
- cpu_set(0, cpu_sibling_map[0]);
- cpu_set(0, cpu_core_map[0]);
+ disable_smp();
return;
}

/*
- * Should not be necessary because the MP table should list the boot
- * CPU too, but we do it for the sake of robustness anyway.
- * Makes no sense to do this check in clustered apic mode, so skip it
- */
- if (!check_phys_apicid_present(boot_cpu_physical_apicid)) {
- printk("weird, boot CPU (#%d) not listed by the BIOS.\n",
- boot_cpu_physical_apicid);
- physid_set(hard_smp_processor_id(), phys_cpu_present_map);
- }
-
- /*
- * If we couldn't find a local APIC, then get out of here now!
- */
- if (APIC_INTEGRATED(apic_version[boot_cpu_physical_apicid]) && !cpu_has_apic) {
- printk(KERN_ERR "BIOS bug, local APIC #%d not detected!...\n",
- boot_cpu_physical_apicid);
- printk(KERN_ERR "... forcing use of dummy APIC emulation. (tell your hw vendor)\n");
- smpboot_clear_io_apic_irqs();
- phys_cpu_present_map = physid_mask_of_physid(0);
- cpu_set(0, cpu_sibling_map[0]);
- cpu_set(0, cpu_core_map[0]);
- return;
- }
-
- verify_local_APIC();
-
- /*
* If SMP should be disabled, then really disable it!
*/
- if (!max_cpus) {
- smp_found_config = 0;
- printk(KERN_INFO "SMP mode deactivated, forcing use of dummy APIC emulation.\n");
- smpboot_clear_io_apic_irqs();
- phys_cpu_present_map = physid_mask_of_physid(0);
- cpu_set(0, cpu_sibling_map[0]);
- cpu_set(0, cpu_core_map[0]);
+ if (!max_cpus || (enable_local_apic < 0)) {
+ printk(KERN_INFO "SMP mode deactivated.\n");
+ disable_smp();
return;
}

- connect_bsp_APIC();
- setup_local_APIC();
- map_cpu_to_logical_apicid();
-
-
setup_portio_remap();

/*
@@ -1240,10 +1208,6 @@ static void __init smp_boot_cpus(unsigne
cpu_set(0, cpu_sibling_map[0]);
cpu_set(0, cpu_core_map[0]);

- smpboot_setup_io_apic();
-
- setup_boot_APIC_clock();
-
/*
* Synchronize the TSC with the AP
*/
diff --git a/arch/i386/kernel/time.c b/arch/i386/kernel/time.c
--- a/arch/i386/kernel/time.c
+++ b/arch/i386/kernel/time.c
@@ -444,8 +444,8 @@ static int time_init_device(void)

device_initcall(time_init_device);

-#ifdef CONFIG_HPET_TIMER
extern void (*late_time_init)(void);
+#ifdef CONFIG_HPET_TIMER
/* Duplicate of time_init() below, with hpet_enable part added */
static void __init hpet_time_init(void)
{
@@ -462,6 +462,12 @@ static void __init hpet_time_init(void)
printk(KERN_INFO "Using %s for high-res timesource\n",cur_timer->name);

time_init_hook();
+
+#if CONFIG_X86_LOCAL_APIC
+ if (enable_local_apic >= 0) {
+ APIC_late_time_init();
+ }
+#endif
}
#endif

@@ -486,4 +492,10 @@ void __init time_init(void)
printk(KERN_INFO "Using %s for high-res timesource\n",cur_timer->name);

time_init_hook();
+
+#if CONFIG_X86_LOCAL_APIC
+ if (enable_local_apic >= 0) {
+ late_time_init = APIC_late_time_init;
+ }
+#endif
}
diff --git a/include/asm-i386/apic.h b/include/asm-i386/apic.h
--- a/include/asm-i386/apic.h
+++ b/include/asm-i386/apic.h
@@ -118,7 +118,8 @@ extern void release_lapic_nmi(void);
extern void disable_timer_nmi_watchdog(void);
extern void enable_timer_nmi_watchdog(void);
extern void nmi_watchdog_tick (struct pt_regs * regs);
-extern int APIC_init_uniprocessor (void);
+extern int APIC_init(void);
+extern void APIC_late_time_init(void);
extern void disable_APIC_timer(void);
extern void enable_APIC_timer(void);

diff --git a/include/asm-i386/hw_irq.h b/include/asm-i386/hw_irq.h
--- a/include/asm-i386/hw_irq.h
+++ b/include/asm-i386/hw_irq.h
@@ -55,6 +55,7 @@ void init_8259A(int aeoi);
void FASTCALL(send_IPI_self(int vector));
void init_VISWS_APIC_irqs(void);
void setup_IO_APIC(void);
+void IO_APIC_late_time_init(void);
void disable_IO_APIC(void);
void print_IO_APIC(void);
int IO_APIC_get_PCI_irq_vector(int bus, int slot, int fn);
diff --git a/include/asm-i386/mach-default/smpboot_hooks.h b/include/asm-i386/mach-default/smpboot_hooks.h
--- a/include/asm-i386/mach-default/smpboot_hooks.h
+++ b/include/asm-i386/mach-default/smpboot_hooks.h
@@ -1,11 +1,6 @@
/* two abstractions specific to kernel/smpboot.c, mainly to cater to visws
* which needs to alter them. */

-static inline void smpboot_clear_io_apic_irqs(void)
-{
- io_apic_irqs = 0;
-}
-
static inline void smpboot_setup_warm_reset_vector(unsigned long start_eip)
{
CMOS_WRITE(0xa, 0xf);
@@ -32,13 +27,3 @@ static inline void smpboot_restore_warm_

*((volatile long *) phys_to_virt(0x467)) = 0;
}
-
-static inline void smpboot_setup_io_apic(void)
-{
- /*
- * Here we can be sure that there is an IO-APIC in the system. Let's
- * go and set it up:
- */
- if (!skip_ioapic_setup && nr_ioapics)
- setup_IO_APIC();
-}
diff --git a/include/asm-i386/mach-visws/smpboot_hooks.h b/include/asm-i386/mach-visws/smpboot_hooks.h
--- a/include/asm-i386/mach-visws/smpboot_hooks.h
+++ b/include/asm-i386/mach-visws/smpboot_hooks.h
@@ -11,14 +11,7 @@ static inline void smpboot_setup_warm_re

/* for visws do nothing for any of these */

-static inline void smpboot_clear_io_apic_irqs(void)
-{
-}
-
static inline void smpboot_restore_warm_reset_vector(void)
{
}

-static inline void smpboot_setup_io_apic(void)
-{
-}
diff --git a/init/main.c b/init/main.c
--- a/init/main.c
+++ b/init/main.c
@@ -64,10 +64,6 @@
#endif
#endif

-#ifdef CONFIG_X86_LOCAL_APIC
-#include <asm/smp.h>
-#endif
-
/*
* Versions of gcc older than that listed below may actually compile
* and link okay, but the end product can have subtle run time bugs.
@@ -314,14 +310,7 @@ extern void setup_arch(char **);

#ifndef CONFIG_SMP

-#ifdef CONFIG_X86_LOCAL_APIC
-static void __init smp_init(void)
-{
- APIC_init_uniprocessor();
-}
-#else
#define smp_init() do { } while (0)
-#endif

static inline void setup_per_cpu_areas(void) { }
static inline void smp_prepare_cpus(unsigned int maxcpus) { }


2005-10-04 15:34:29

by Maciej W. Rozycki

[permalink] [raw]
Subject: Re: [PATCH] i386: move apic init in init_IRQs

On Tue, 4 Oct 2005, Eric W. Biederman wrote:

> - if (enable_local_apic < 0)
> - clear_bit(X86_FEATURE_APIC, boot_cpu_data.x86_capability);

I think this should stay.

> + if (enable_local_apic < 0) {
> + printk(KERN_INFO "Apic disabled\n");

Capitalisation. ;-)

Otherwise it seems reasonable -- provided it works for you. ;-)

Maciej

2005-10-04 15:49:33

by Eric W. Biederman

[permalink] [raw]
Subject: Re: [PATCH] i386: move apic init in init_IRQs

"Maciej W. Rozycki" <[email protected]> writes:

> On Tue, 4 Oct 2005, Eric W. Biederman wrote:
>
>> - if (enable_local_apic < 0)
>> - clear_bit(X86_FEATURE_APIC, boot_cpu_data.x86_capability);
>
> I think this should stay.

lapic_disable() already does this. I am just testing the results.

>> + if (enable_local_apic < 0) {
>> + printk(KERN_INFO "Apic disabled\n");
>
> Capitalisation. ;-)
>
> Otherwise it seems reasonable -- provided it works for you. ;-)

So what should the capitalization be? "APIC disabled\n" ?

Sorry.

Eric


2005-10-04 17:16:29

by Maciej W. Rozycki

[permalink] [raw]
Subject: Re: [PATCH] i386: move apic init in init_IRQs

On Tue, 4 Oct 2005, Eric W. Biederman wrote:

> lapic_disable() already does this. I am just testing the results.

I see -- this just proves how big a mess the current code is. ;-)

> So what should the capitalization be? "APIC disabled\n" ?

Obviously. Thanks for your tidy-up!

Maciej

2005-10-05 18:31:58

by Eric W. Biederman

[permalink] [raw]
Subject: [PATCH] i386 apic: Fix mispelling of APIC

"Maciej W. Rozycki" <[email protected]> writes:


>> So what should the capitalization be? "APIC disabled\n" ?
>
> Obviously. Thanks for your tidy-up!

Welcome.

Looking a little deeper I just copied the mispelling from x86_64.
Here is the incremental patch that fixes the i386 version.

Signed-off-by: Eric W. Biederman <[email protected]>


---

arch/i386/kernel/apic.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

7edc564590555e94268fb73ddf97431b4b9df945
diff --git a/arch/i386/kernel/apic.c b/arch/i386/kernel/apic.c
--- a/arch/i386/kernel/apic.c
+++ b/arch/i386/kernel/apic.c
@@ -1265,7 +1265,7 @@ fastcall void smp_error_interrupt(struct
int __init APIC_init(void)
{
if (enable_local_apic < 0) {
- printk(KERN_INFO "Apic disabled\n");
+ printk(KERN_INFO "APIC disabled\n");
return -1;
}


2005-10-21 13:33:16

by Vivek Goyal

[permalink] [raw]
Subject: Re: [Fastboot] [PATCH] i386: move apic init in init_IRQs

Hi Eric,

I had a couple of observations.

[..]
> #ifdef CONFIG_X86_IO_APIC
> {
> @@ -1046,9 +1050,11 @@ static unsigned int calibration_result;
>
> void __init setup_boot_APIC_clock(void)
> {
> + unsigned long flags;
> apic_printk(APIC_VERBOSE, "Using local APIC timer interrupts.\n");
> using_apic_timer = 1;
>
> + local_irq_save(flags);
> local_irq_disable();
>

Should the local_irq_disable() call go away onece local_irq_save() got
introduced.


> calibration_result = calibrate_APIC_clock();
> @@ -1057,7 +1063,7 @@ void __init setup_boot_APIC_clock(void)
> */
> setup_APIC_timer(calibration_result);
>
> - local_irq_enable();
> + local_irq_restore(flags);
> }
>

[..]
>
> verify_local_APIC();
>
> + /*
> + * Should not be necessary because the MP table should list the boot
> + * CPU too, but we do it for the sake of robustness anyway.
> + * Makes no sense to do this check in clustered apic mode, so skip it
> + */
> + if (!check_phys_apicid_present(boot_cpu_physical_apicid)) {
> + printk("weird, boot CPU (#%d) not listed by the BIOS.\n",
> + boot_cpu_physical_apicid);


I am testing kdump on i386 and I am hitting this message while second kernel
is booting. I am doing testing with 2.6.14-rc4-mm1. Logs are pasted below.

Also kdump testing fails almost 50% of the time on my machine with
2.6.14-rc4-mm1. It works fine with 2.6.14-rc4 though.

Second kernel is unable to come up. earlyprintk on serial console showed
a kernel BUG in setup_local_APIC(). Details are included in the logs below.

Second kernel boot log.

# SysRq : Trigger a crashdump
I'm in purgatory
Linux version 2.6.14-rc4-mm1-16M ([email protected]) (gcc version 3.4.3 20041212 (Red Hat 3.4.3-9.EL4)) #1 PREEMPT Wed Oct 19 13:55:24 IST 2005
BIOS-provided physical RAM map:
BIOS-e820: 0000000000000100 - 000000000009d000 (usable)
BIOS-e820: 000000000009d000 - 00000000000a0000 (reserved)
BIOS-e820: 0000000000100000 - 000000002fffa480 (usable)
BIOS-e820: 000000002fffa480 - 0000000030000000 (ACPI data)
BIOS-e820: 00000000fec00000 - 0000000100000000 (reserved)
user-defined physical RAM map:
user: 0000000000000000 - 00000000000a0000 (usable)
user: 0000000001000000 - 000000000142d000 (usable)
user: 00000000014cd400 - 0000000004000000 (usable)
0MB HIGHMEM available.
64MB LOWMEM available.
found SMP MP-table at 0009e140
early console enabled
DMI 2.1 present.
ACPI: LAPIC (acpi_id[0x00] lapic_id[0x03] enabled)
Processor #3 6:10 APIC version 17
ACPI: LAPIC (acpi_id[0x01] lapic_id[0x00] enabled)
Processor #0 6:10 APIC version 17
WARNING: NR_CPUS limit of 1 reached. Processor ignored.
ACPI: LAPIC (acpi_id[0x02] lapic_id[0x01] enabled)
Processor #1 6:10 APIC version 17
WARNING: NR_CPUS limit of 1 reached. Processor ignored.
ACPI: LAPIC (acpi_id[0x03] lapic_id[0x02] enabled)
Processor #2 6:10 APIC version 17
WARNING: NR_CPUS limit of 1 reached. Processor ignored.
ACPI: IOAPIC (id[0x0e] address[0xfec00000] gsi_base[0])
IOAPIC[0]: apic_id 14, version 17, address 0xfec00000, GSI 0-15
ACPI: IOAPIC (id[0x0d] address[0xfec01000] gsi_base[16])
IOAPIC[1]: apic_id 13, version 17, address 0xfec01000, GSI 16-31
ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 dfl dfl)
Enabling APIC mode: Flat. Using 2 I/O APICs
Using ACPI (MADT) for SMP configuration information
Allocating PCI resources starting at 10000000 (gap: 04000000:fc000000)
Built 1 zonelists
Initializing CPU#0
Kernel command line: ro root=/dev/sda7 rhgb console=ttyS0,38400 irqpoll init 3 earlyprintk=ttyS0,38400 memmap=exactmap memmap=640K@0K memmap=4276K@16384K memmap=44235K@21301K elfcorehdr=21300K
Misrouted IRQ fixup and polling support enabled
This may significantly impact system performance
weird, boot CPU (#1) not listed by the BIOS.
------------[ cut here ]------------
kernel BUG at ????:1479!
invalid operand: 0000 [#1]
PREEMPT
last sysfs file:
Modules linked in:
CPU: 0
EIP: 0060:[<c1012b17>] Not tainted VLI
EFLAGS: 00010046 (2.6.14-rc4-mm1-16M)
EIP is at setup_local_APIC+0x26/0x18c
eax: 00000000 ebx: 00040011 ecx: 00000c06 edx: 00000000
esi: 00000011 edi: c13a9800 ebp: 01445007 esp: c13b5fbc
ds: 007b es: 007b ss: 0068
Process swapper (pid: 0, threadinfo=c13b4000 task=c133faa0)
Stack: c13a9800 01445007 c101ac30 00000000 01429900 c13c1c49 c12e8a4c 00000001
00000003 c13b66cf c12e5d5d c13eddc0 c133b9fc 00000078 c13b6342 c13eddc0
c1000199
Call Trace:
[<c101ac30>] printk+0x17/0x1b
[<c13c1c49>] APIC_init+0x5a/0xf6
[<c13b66cf>] start_kernel+0xb3/0x1cd
[<c13b6342>] unknown_bootoption+0x0/0x1b6
Code: e4 f7 0f 30 c3 56 53 83 ec 0c 8b 1d 30 d0 ff ff a1 20 d0 ff ff c1 e8 18 0f b6 f3 83 e0 0f 0f a3 05 e0 03 3f c1 19 c0 85 c0 75 02 <0f> 0b c7 05 e0 d0 ff ff ff ff ff ff 8b 0d c4 03 3f c1 a1 d0 d0
<0>Kernel panic - not syncing: Attempted to kill the idle task!


Thanks
Vivek

2005-10-21 14:45:39

by Eric W. Biederman

[permalink] [raw]
Subject: Re: [Fastboot] [PATCH] i386: move apic init in init_IRQs

Vivek Goyal <[email protected]> writes:

> Hi Eric,
>
> I had a couple of observations.
>
> [..]
>> #ifdef CONFIG_X86_IO_APIC
>> {
>> @@ -1046,9 +1050,11 @@ static unsigned int calibration_result;
>>
>> void __init setup_boot_APIC_clock(void)
>> {
>> + unsigned long flags;
>> apic_printk(APIC_VERBOSE, "Using local APIC timer interrupts.\n");
>> using_apic_timer = 1;
>>
>> + local_irq_save(flags);
>> local_irq_disable();
>>
>
> Should the local_irq_disable() call go away onece local_irq_save() got
> introduced.

Nope. The irqs need to be disabled. The save just allows this
to be called in a context where irqs start out disabled. It is
just a save.

>> + /*
>> + * Should not be necessary because the MP table should list the boot
>> + * CPU too, but we do it for the sake of robustness anyway.
>> + * Makes no sense to do this check in clustered apic mode, so skip it
>> + */
>> + if (!check_phys_apicid_present(boot_cpu_physical_apicid)) {
>> + printk("weird, boot CPU (#%d) not listed by the BIOS.\n",
>> + boot_cpu_physical_apicid);
>
>
> I am testing kdump on i386 and I am hitting this message while second kernel
> is booting. I am doing testing with 2.6.14-rc4-mm1. Logs are pasted below.

The check has been there for a while. All it is saying is that
our boot cpu has apicid #1. So I suspect you are either on
an Opteron system or a hyperthreaded Xeon system.

> Also kdump testing fails almost 50% of the time on my machine with
> 2.6.14-rc4-mm1. It works fine with 2.6.14-rc4 though.

Is the failure that happens 50% represented by the bootlog below?

The problem bootlog appears to be a glitch in the handling
of apicids on the boot cpu that the BIOS does not report to the
kernel.

> Second kernel is unable to come up. earlyprintk on serial console showed
> a kernel BUG in setup_local_APIC(). Details are included in the logs below.

> Second kernel boot log.

The BUG is weird. I don't think apic.c even goes to line 1479.
Unless the BUG is inline in one of the other functions called
by setup_local_APIC() .

/*
* Double-check whether this APIC is really registered.
*/
if (!apic_id_registered())
BUG();


apic_id_registered expands to:
static inline int apic_id_registered(void)
{
return physid_isset(GET_APIC_ID(apic_read(APIC_ID)), phys_cpu_present_map);
}

Which indicates to me that the code that, there is something
wrong in the logic of:
if (!check_phys_apicid_present(boot_cpu_physical_apicid)) {
printk("weird, boot CPU (#%d) not listed by the BIOS.\n",
boot_cpu_physical_apicid);
physid_set(hard_smp_processor_id(), phys_cpu_present_map);
}

Currently we are refering to the boot cpus apicid with 3 different expressions
one of them appears to be wrong.

That is as far as I can get at the moment.

Eric


>
> # SysRq : Trigger a crashdump
> I'm in purgatory
> Linux version 2.6.14-rc4-mm1-16M ([email protected]) (gcc version 3.4.3
> 20041212 (Red Hat 3.4.3-9.EL4)) #1 PREEMPT Wed Oct 19 13:55:24 IST 2005
> BIOS-provided physical RAM map:
> BIOS-e820: 0000000000000100 - 000000000009d000 (usable)
> BIOS-e820: 000000000009d000 - 00000000000a0000 (reserved)
> BIOS-e820: 0000000000100000 - 000000002fffa480 (usable)
> BIOS-e820: 000000002fffa480 - 0000000030000000 (ACPI data)
> BIOS-e820: 00000000fec00000 - 0000000100000000 (reserved)
> user-defined physical RAM map:
> user: 0000000000000000 - 00000000000a0000 (usable)
> user: 0000000001000000 - 000000000142d000 (usable)
> user: 00000000014cd400 - 0000000004000000 (usable)
> 0MB HIGHMEM available.
> 64MB LOWMEM available.
> found SMP MP-table at 0009e140
> early console enabled
> DMI 2.1 present.
> ACPI: LAPIC (acpi_id[0x00] lapic_id[0x03] enabled)
> Processor #3 6:10 APIC version 17
> ACPI: LAPIC (acpi_id[0x01] lapic_id[0x00] enabled)
> Processor #0 6:10 APIC version 17
> WARNING: NR_CPUS limit of 1 reached. Processor ignored.
> ACPI: LAPIC (acpi_id[0x02] lapic_id[0x01] enabled)
> Processor #1 6:10 APIC version 17
> WARNING: NR_CPUS limit of 1 reached. Processor ignored.
> ACPI: LAPIC (acpi_id[0x03] lapic_id[0x02] enabled)
> Processor #2 6:10 APIC version 17
> WARNING: NR_CPUS limit of 1 reached. Processor ignored.
> ACPI: IOAPIC (id[0x0e] address[0xfec00000] gsi_base[0])
> IOAPIC[0]: apic_id 14, version 17, address 0xfec00000, GSI 0-15
> ACPI: IOAPIC (id[0x0d] address[0xfec01000] gsi_base[16])
> IOAPIC[1]: apic_id 13, version 17, address 0xfec01000, GSI 16-31
> ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 dfl dfl)
> Enabling APIC mode: Flat. Using 2 I/O APICs
> Using ACPI (MADT) for SMP configuration information
> Allocating PCI resources starting at 10000000 (gap: 04000000:fc000000)
> Built 1 zonelists
> Initializing CPU#0
> Kernel command line: ro root=/dev/sda7 rhgb console=ttyS0,38400 irqpoll init 3
> earlyprintk=ttyS0,38400 memmap=exactmap memmap=640K@0K memmap=4276K@16384K
> memmap=44235K@21301K elfcorehdr=21300K
> Misrouted IRQ fixup and polling support enabled
> This may significantly impact system performance
> weird, boot CPU (#1) not listed by the BIOS.
> ------------[ cut here ]------------
> kernel BUG at ????:1479!
> invalid operand: 0000 [#1]
> PREEMPT
> last sysfs file:
> Modules linked in:
> CPU: 0
> EIP: 0060:[<c1012b17>] Not tainted VLI
> EFLAGS: 00010046 (2.6.14-rc4-mm1-16M)
> EIP is at setup_local_APIC+0x26/0x18c
> eax: 00000000 ebx: 00040011 ecx: 00000c06 edx: 00000000
> esi: 00000011 edi: c13a9800 ebp: 01445007 esp: c13b5fbc
> ds: 007b es: 007b ss: 0068
> Process swapper (pid: 0, threadinfo=c13b4000 task=c133faa0)
> Stack: c13a9800 01445007 c101ac30 00000000 01429900 c13c1c49 c12e8a4c 00000001
> 00000003 c13b66cf c12e5d5d c13eddc0 c133b9fc 00000078 c13b6342 c13eddc0
> c1000199
> Call Trace:
> [<c101ac30>] printk+0x17/0x1b
> [<c13c1c49>] APIC_init+0x5a/0xf6
> [<c13b66cf>] start_kernel+0xb3/0x1cd
> [<c13b6342>] unknown_bootoption+0x0/0x1b6
> Code: e4 f7 0f 30 c3 56 53 83 ec 0c 8b 1d 30 d0 ff ff a1 20 d0 ff ff c1 e8 18 0f
> b6 f3 83 e0 0f 0f a3 05 e0 03 3f c1 19 c0 85 c0 75 02 <0f> 0b c7 05 e0 d0 ff ff
> ff ff ff ff 8b 0d c4 03 3f c1 a1 d0 d0
> <0>Kernel panic - not syncing: Attempted to kill the idle task!
>
>
> Thanks
> Vivek

2005-10-21 16:54:00

by Albert Herranz

[permalink] [raw]
Subject: Re: [Fastboot] [PATCH] i386: move apic init in init_IRQs

> > Should the local_irq_disable() call go away onece
> local_irq_save() got
> > introduced.
>
> Nope. The irqs need to be disabled. The save just
> allows this
> to be called in a context where irqs start out
> disabled. It is
> just a save.

local_irq_save() also disables interrupts.

Cheers,
Albert





______________________________________________
Renovamos el Correo Yahoo!
Nuevos servicios, m?s seguridad
http://correo.yahoo.es

2005-10-21 18:02:14

by Eric W. Biederman

[permalink] [raw]
Subject: Re: [Fastboot] [PATCH] i386: move apic init in init_IRQs

Albert Herranz <[email protected]> writes:

>> > Should the local_irq_disable() call go away onece
>> local_irq_save() got
>> > introduced.
>>
>> Nope. The irqs need to be disabled. The save just
>> allows this
>> to be called in a context where irqs start out
>> disabled. It is
>> just a save.
>
> local_irq_save() also disables interrupts.

Bah. I was thinking and reading local_save_flags()....

So yes that does make the local_irq_disable redundant.

I still used to seeing:
save_flags();
cli();
...
restore_flags();

Eric


2005-10-22 14:52:16

by Vivek Goyal

[permalink] [raw]
Subject: Re: [Fastboot] [PATCH] i386: move apic init in init_IRQs

On Fri, Oct 21, 2005 at 08:45:12AM -0600, Eric W. Biederman wrote:
> Vivek Goyal <[email protected]> writes:
>

[..]

> >> + /*
> >> + * Should not be necessary because the MP table should list the boot
> >> + * CPU too, but we do it for the sake of robustness anyway.
> >> + * Makes no sense to do this check in clustered apic mode, so skip it
> >> + */
> >> + if (!check_phys_apicid_present(boot_cpu_physical_apicid)) {
> >> + printk("weird, boot CPU (#%d) not listed by the BIOS.\n",
> >> + boot_cpu_physical_apicid);
> >
> >
> > I am testing kdump on i386 and I am hitting this message while second kernel
> > is booting. I am doing testing with 2.6.14-rc4-mm1. Logs are pasted below.
>
> The check has been there for a while. All it is saying is that
> our boot cpu has apicid #1. So I suspect you are either on
> an Opteron system or a hyperthreaded Xeon system.
>

I am using Pentium. No hyperthreading.

> > Also kdump testing fails almost 50% of the time on my machine with
> > 2.6.14-rc4-mm1. It works fine with 2.6.14-rc4 though.
>
> Is the failure that happens 50% represented by the bootlog below?
>

Yes. But this problem is not happening all the time. Now in 4 trials
I got it once again. The message in all the failures remains the same.


> The problem bootlog appears to be a glitch in the handling
> of apicids on the boot cpu that the BIOS does not report to the
> kernel.
>
> > Second kernel is unable to come up. earlyprintk on serial console showed
> > a kernel BUG in setup_local_APIC(). Details are included in the logs below.
>
> > Second kernel boot log.
>
> The BUG is weird. I don't think apic.c even goes to line 1479.
> Unless the BUG is inline in one of the other functions called
> by setup_local_APIC() .
>
> /*
> * Double-check whether this APIC is really registered.
> */
> if (!apic_id_registered())
> BUG();
>
>
> apic_id_registered expands to:
> static inline int apic_id_registered(void)
> {
> return physid_isset(GET_APIC_ID(apic_read(APIC_ID)), phys_cpu_present_map);
> }
>
> Which indicates to me that the code that, there is something
> wrong in the logic of:
> if (!check_phys_apicid_present(boot_cpu_physical_apicid)) {
> printk("weird, boot CPU (#%d) not listed by the BIOS.\n",
> boot_cpu_physical_apicid);
> physid_set(hard_smp_processor_id(), phys_cpu_present_map);
> }
>
> Currently we are refering to the boot cpus apicid with 3 different expressions
> one of them appears to be wrong.
>

Looks like apic_id_registered() is failing. I had put two debug printk()
statements and to my surprise hard_smp_processor_id() is returning different
value then GET_APIC_ID(apic_read(APIC_ID)).

source code of hard_smp_processor_id() shows that it is also reading APIC_ID
register only. Then how can two values be different. (Until and unless
somebody modified the value in between two reads).

I am pasting another failure log with my debug messages(prefixed with "Debug:").
My debug patch is also attached with the mail.

Second kernel boot log
---------------------

I'm in purgatory
Linux version 2.6.14-rc4-mm1-16M ([email protected]) (gcc version 3.4.3 20041212 (Red Hat 3.4.3-9.EL4)) #2 PREEMPT Sat Oct 22 18:44:25 IST 2005
BIOS-provided physical RAM map:
BIOS-e820: 0000000000000100 - 000000000009d000 (usable)
BIOS-e820: 000000000009d000 - 00000000000a0000 (reserved)
BIOS-e820: 0000000000100000 - 000000002fffa480 (usable)
BIOS-e820: 000000002fffa480 - 0000000030000000 (ACPI data)
BIOS-e820: 00000000fec00000 - 0000000100000000 (reserved)
user-defined physical RAM map:
user: 0000000000000000 - 00000000000a0000 (usable)
user: 0000000001000000 - 000000000142d000 (usable)
user: 00000000014cd400 - 0000000005000000 (usable)
0MB HIGHMEM available.
80MB LOWMEM available.
found SMP MP-table at 0009e140
early console enabled
DMI 2.1 present.
ACPI: LAPIC (acpi_id[0x00] lapic_id[0x03] enabled)
Processor #3 6:10 APIC version 17
ACPI: LAPIC (acpi_id[0x01] lapic_id[0x00] enabled)
Processor #0 6:10 APIC version 17
WARNING: NR_CPUS limit of 1 reached. Processor ignored.
ACPI: LAPIC (acpi_id[0x02] lapic_id[0x01] enabled)
Processor #1 6:10 APIC version 17
WARNING: NR_CPUS limit of 1 reached. Processor ignored.
ACPI: LAPIC (acpi_id[0x03] lapic_id[0x02] enabled)
Processor #2 6:10 APIC version 17
WARNING: NR_CPUS limit of 1 reached. Processor ignored.
ACPI: IOAPIC (id[0x0e] address[0xfec00000] gsi_base[0])
IOAPIC[0]: apic_id 14, version 17, address 0xfec00000, GSI 0-15
ACPI: IOAPIC (id[0x0d] address[0xfec01000] gsi_base[16])
IOAPIC[1]: apic_id 13, version 17, address 0xfec01000, GSI 16-31
ACPI: INT_SRC_OVR (bus 0 bus_irq 0 global_irq 2 dfl dfl)
Enabling APIC mode: Flat. Using 2 I/O APICs
Using ACPI (MADT) for SMP configuration information
Allocating PCI resources starting at 10000000 (gap: 05000000:fb000000)
Built 1 zonelists
Initializing CPU#0
Kernel command line: ro root=/dev/sda7 rhgb console=ttyS0,38400 irqpoll init 3 earlyprintk=ttyS0,38400 memmap=exactmap memmap=640K@0K memmap=4276K@16384K memmap=60619K@21301K elfcorehdr=21300K
Misrouted IRQ fixup and polling support enabled
This may significantly impact system performance
weird, boot CPU (#1) not listed by the BIOS.
Debug:Harsetting cpu apic id 0 to be present
Debug: APIC id being queried is 1
------------[ cut here ]------------
kernel BUG at ????:1479!
invalid operand: 0000 [#1]
PREEMPT
last sysfs file:
Modules linked in:
CPU: 0
EIP: 0060:[<c1012b32>] Not tainted VLI
EFLAGS: 00010046 (2.6.14-rc4-mm1-16M)
EIP is at setup_local_APIC+0x41/0x1a7
eax: 00000000 ebx: 00040011 ecx: 00000c5b edx: c1344201
esi: 00000011 edi: c13a9800 ebp: 01445007 esp: c13b5fbc
ds: 007b es: 007b ss: 0068
Process swapper (pid: 0, threadinfo=c13b4000 task=c133faa0)
Stack: c12e8774 00000001 c101ac40 00000000 01429900 c13c1c49 c12e8ac0 00000000
00000003 c13b66cf c12e5d7d c13eddc0 c133ba5c 00000078 c13b6342 c13eddc0
c1000199
Call Trace:
[<c101ac40>] printk+0x17/0x1b
[<c13c1c49>] APIC_init+0x5a/0x10a
[<c13b66cf>] start_kernel+0xb3/0x1cd
[<c13b6342>] unknown_bootoption+0x0/0x1b6
Code: c1 c1 e8 18 0f b6 f3 83 e0 0f 89 44 24 04 e8 0f 81 00 00 a1 20 d0 ff ff c1 e8 18 83 e0 0f 0f a3 05 e0 03 3f c1 19 c0 85 c0 75 02 <0f> 0b c7 05 e0 d0 ff ff ff ff ff ff 8b 0d c4 03 3f c1 a1 d0 d0
<0>Kernel panic - not syncing: Attempted to kill the idle task!


Debug Patch
----------


linux-2.6.14-rc4-mm1-16M-root/arch/i386/kernel/apic.c | 2 ++
linux-2.6.14-rc4-mm1-16M-root/include/asm-i386/mach-default/mach_apic.h | 1 +
2 files changed, 3 insertions(+)

diff -puN arch/i386/kernel/apic.c~apic-debug arch/i386/kernel/apic.c
--- linux-2.6.14-rc4-mm1-16M/arch/i386/kernel/apic.c~apic-debug 2005-10-22 18:37:28.000000000 +0530
+++ linux-2.6.14-rc4-mm1-16M-root/arch/i386/kernel/apic.c 2005-10-22 18:42:50.000000000 +0530
@@ -1299,6 +1299,8 @@ int __init APIC_init(void)
if (!check_phys_apicid_present(boot_cpu_physical_apicid)) {
printk("weird, boot CPU (#%d) not listed by the BIOS.\n",
boot_cpu_physical_apicid);
+ printk("Debug:Harsetting cpu apic id %d to be present\n",
+ hard_smp_processor_id());
physid_set(hard_smp_processor_id(), phys_cpu_present_map);
}

diff -puN include/asm-i386/mach-default/mach_apic.h~apic-debug include/asm-i386/mach-default/mach_apic.h
--- linux-2.6.14-rc4-mm1-16M/include/asm-i386/mach-default/mach_apic.h~apic-debug 2005-10-22 18:38:42.000000000 +0530
+++ linux-2.6.14-rc4-mm1-16M-root/include/asm-i386/mach-default/mach_apic.h 2005-10-22 18:44:10.000000000 +0530
@@ -111,6 +111,7 @@ static inline int check_phys_apicid_pres

static inline int apic_id_registered(void)
{
+ printk("Debug: APIC id being queried is %d\n", GET_APIC_ID(apic_read(APIC_ID)));
return physid_isset(GET_APIC_ID(apic_read(APIC_ID)), phys_cpu_present_map);
}

_

/proc/cpuinfo output
--------------------

[root@llm01 ~]# cat /proc/cpuinfo
processor : 0
vendor_id : GenuineIntel
cpu family : 6
model : 10
model name : Pentium III (Cascades)
stepping : 1
cpu MHz : 699.365
cache size : 1024 KB
fdiv_bug : no
hlt_bug : no
f00f_bug : no
coma_bug : no
fpu : yes
fpu_exception : yes
cpuid level : 2
wp : yes
flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 mmx fxsr sse
bogomips : 1400.68

processor : 1
vendor_id : GenuineIntel
cpu family : 6
model : 10
model name : Pentium III (Cascades)
stepping : 1
cpu MHz : 699.365
cache size : 1024 KB
fdiv_bug : no
hlt_bug : no
f00f_bug : no
coma_bug : no
fpu : yes
fpu_exception : yes
cpuid level : 2
wp : yes
flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 mmx fxsr sse
bogomips : 1398.47

processor : 2
vendor_id : GenuineIntel
cpu family : 6
model : 10
model name : Pentium III (Cascades)
stepping : 1
cpu MHz : 699.365
cache size : 1024 KB
fdiv_bug : no
hlt_bug : no
f00f_bug : no
coma_bug : no
fpu : yes
fpu_exception : yes
cpuid level : 2
wp : yes
flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 mmx fxsr sse
bogomips : 1398.47

processor : 3
vendor_id : GenuineIntel
cpu family : 6
model : 10
model name : Pentium III (Cascades)
stepping : 1
cpu MHz : 699.365
cache size : 1024 KB
fdiv_bug : no
hlt_bug : no
f00f_bug : no
coma_bug : no
fpu : yes
fpu_exception : yes
cpuid level : 2
wp : yes
flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 mmx fxsr sse
bogomips : 1398.48


Thanks
Vivek

2005-10-22 15:23:58

by Eric W. Biederman

[permalink] [raw]
Subject: Re: [Fastboot] [PATCH] i386: move apic init in init_IRQs

Vivek Goyal <[email protected]> writes:

> On Fri, Oct 21, 2005 at 08:45:12AM -0600, Eric W. Biederman wrote:
>> Vivek Goyal <[email protected]> writes:
>>
>
> [..]
>
>> >> + /*
>> >> + * Should not be necessary because the MP table should list the boot
>> >> + * CPU too, but we do it for the sake of robustness anyway.
>> >> + * Makes no sense to do this check in clustered apic mode, so skip it
>> >> + */
>> >> + if (!check_phys_apicid_present(boot_cpu_physical_apicid)) {
>> >> + printk("weird, boot CPU (#%d) not listed by the BIOS.\n",
>> >> + boot_cpu_physical_apicid);
>> >
>> >
>> > I am testing kdump on i386 and I am hitting this message while second kernel
>> > is booting. I am doing testing with 2.6.14-rc4-mm1. Logs are pasted below.
>>
>> The check has been there for a while. All it is saying is that
>> our boot cpu has apicid #1. So I suspect you are either on
>> an Opteron system or a hyperthreaded Xeon system.
>>
>
> I am using Pentium. No hyperthreading.

Weird. I would have that the BIOS would have listed the second cpu...
It might be worth tracking down later why the message appears but
the real problem is that the map is not getting filled in.

>> > Also kdump testing fails almost 50% of the time on my machine with
>> > 2.6.14-rc4-mm1. It works fine with 2.6.14-rc4 though.
>>
>> Is the failure that happens 50% represented by the bootlog below?
>>
>
> Yes. But this problem is not happening all the time. Now in 4 trials
> I got it once again. The message in all the failures remains the same.

It seems to only happen when this all starts on the second cpu,
with apic id #1. Which explains the randomness.


>> apic_id_registered expands to:
>> static inline int apic_id_registered(void)
>> {
>> return physid_isset(GET_APIC_ID(apic_read(APIC_ID)), phys_cpu_present_map);
>> }
>>
>> Which indicates to me that the code that, there is something
>> wrong in the logic of:
>> if (!check_phys_apicid_present(boot_cpu_physical_apicid)) {
>> printk("weird, boot CPU (#%d) not listed by the BIOS.\n",
>> boot_cpu_physical_apicid);
>> physid_set(hard_smp_processor_id(), phys_cpu_present_map);
>> }
>>
>> Currently we are refering to the boot cpus apicid with 3 different expressions
>> one of them appears to be wrong.
>>
>
> Looks like apic_id_registered() is failing. I had put two debug printk()
> statements and to my surprise hard_smp_processor_id() is returning different
> value then GET_APIC_ID(apic_read(APIC_ID)).
>
> source code of hard_smp_processor_id() shows that it is also reading APIC_ID
> register only. Then how can two values be different. (Until and unless
> somebody modified the value in between two reads).

It appears the buggy expression is hard_smp_processor_id. Quite
possibly because it doesn't call apic_read() and instead open codes
it.

boot_cpu_physical_apicid also returns apicid #1, before we have
a problem.

So either we want to change hard_smp_processor_id to use apic_read()
or we can just use boot_cpu_physical_apicid when fixing the apicid present
bitmap.

> I am pasting another failure log with my debug messages(prefixed with "Debug:").
> My debug patch is also attached with the mail.

See above but I am pretty certain we know enough to get farther. For
testing you may want to hard code your first kernel to use the second
cpu.

The fact that hard_smp_processor_id gets the wrong value makes me wonder
if your kernel will boot all of the way once we get past this problem.

I suspect if you disassemble the code for hard_smp_processor_id we
will see the compiler doing the wrong thing.

Eric

2005-10-24 13:03:24

by Vivek Goyal

[permalink] [raw]
Subject: Re: [Fastboot] [PATCH] i386: move apic init in init_IRQs

On Sat, Oct 22, 2005 at 09:23:34AM -0600, Eric W. Biederman wrote:
> Vivek Goyal <[email protected]> writes:

[..]

>
>
> >> apic_id_registered expands to:
> >> static inline int apic_id_registered(void)
> >> {
> >> return physid_isset(GET_APIC_ID(apic_read(APIC_ID)), phys_cpu_present_map);
> >> }
> >>
> >> Which indicates to me that the code that, there is something
> >> wrong in the logic of:
> >> if (!check_phys_apicid_present(boot_cpu_physical_apicid)) {
> >> printk("weird, boot CPU (#%d) not listed by the BIOS.\n",
> >> boot_cpu_physical_apicid);
> >> physid_set(hard_smp_processor_id(), phys_cpu_present_map);
> >> }
> >>
> >> Currently we are refering to the boot cpus apicid with 3 different expressions
> >> one of them appears to be wrong.
> >>
> >
> > Looks like apic_id_registered() is failing. I had put two debug printk()
> > statements and to my surprise hard_smp_processor_id() is returning different
> > value then GET_APIC_ID(apic_read(APIC_ID)).
> >
> > source code of hard_smp_processor_id() shows that it is also reading APIC_ID
> > register only. Then how can two values be different. (Until and unless
> > somebody modified the value in between two reads).
>
> It appears the buggy expression is hard_smp_processor_id. Quite
> possibly because it doesn't call apic_read() and instead open codes
> it.
>
> boot_cpu_physical_apicid also returns apicid #1, before we have
> a problem.
>
> So either we want to change hard_smp_processor_id to use apic_read()
> or we can just use boot_cpu_physical_apicid when fixing the apicid present
> bitmap.
>
> > I am pasting another failure log with my debug messages(prefixed with "Debug:").
> > My debug patch is also attached with the mail.
>
> See above but I am pretty certain we know enough to get farther. For
> testing you may want to hard code your first kernel to use the second
> cpu.
>
> The fact that hard_smp_processor_id gets the wrong value makes me wonder
> if your kernel will boot all of the way once we get past this problem.
>
> I suspect if you disassemble the code for hard_smp_processor_id we
> will see the compiler doing the wrong thing.


You are right. hard_smp_processor_id() is hard-coded to zero in case of a
non SMP kernel (include/linux/smp.h) and that's why the problem is happening.
I am booting a non-SMP capture kernel. In case of kexec on panic, we can very
well boot on a cpu whose id is not zero.

I have attached a patch with the mail which is now using
boot_cpu_physical_apicid to hard set presence of boot cpu instead of
hard_smp_processor_id(). But the interesting questoin remains why BIOS is
not reporting the boot cpu.

Thanks
Vivek


o Removes the unnecessary call to local_irq_disable().

o Kdump was failing while second kernel was coming up. Check for presence
of boot cpu apic id was failing in (apic_id_registered), hence hitting
BUG().

o This should not have failed because before calling setup_local_APIC(), it is
ensured that even if BIOS has not reported boot cpu, then hard set the
prence of it. Problem happens because of usage of hard_smp_processor_id()
which is hardcoded to zero in case of non SMP kernel. In kdump case second
kernel can boot on a cpu whose boot cpu id is not zero.

o Using boot_cpu_physical_apicid instead to hard set the presence of boot cpu.

Signed-off-by: Vivek Goyal <[email protected]>
---

linux-2.6.14-rc4-mm1-16M-root/arch/i386/kernel/apic.c | 3 +--
1 files changed, 1 insertion(+), 2 deletions(-)

diff -puN arch/i386/kernel/apic.c~kdump-i386-apic-verification-failure-fix arch/i386/kernel/apic.c
--- linux-2.6.14-rc4-mm1-16M/arch/i386/kernel/apic.c~kdump-i386-apic-verification-failure-fix 2005-10-24 17:40:08.000000000 +0530
+++ linux-2.6.14-rc4-mm1-16M-root/arch/i386/kernel/apic.c 2005-10-24 18:19:53.000000000 +0530
@@ -1055,7 +1055,6 @@ void __init setup_boot_APIC_clock(void)
using_apic_timer = 1;

local_irq_save(flags);
- local_irq_disable();

calibration_result = calibrate_APIC_clock();
/*
@@ -1299,7 +1298,7 @@ int __init APIC_init(void)
if (!check_phys_apicid_present(boot_cpu_physical_apicid)) {
printk("weird, boot CPU (#%d) not listed by the BIOS.\n",
boot_cpu_physical_apicid);
- physid_set(hard_smp_processor_id(), phys_cpu_present_map);
+ physid_set(boot_cpu_physical_apicid, phys_cpu_present_map);
}

/*
_

2005-10-24 15:37:24

by Eric W. Biederman

[permalink] [raw]
Subject: Re: [Fastboot] [PATCH] i386: move apic init in init_IRQs

Vivek Goyal <[email protected]> writes:

> You are right. hard_smp_processor_id() is hard-coded to zero in case of a
> non SMP kernel (include/linux/smp.h) and that's why the problem is happening.
> I am booting a non-SMP capture kernel. In case of kexec on panic, we can very
> well boot on a cpu whose id is not zero.
>
> I have attached a patch with the mail which is now using
> boot_cpu_physical_apicid to hard set presence of boot cpu instead of
> hard_smp_processor_id(). But the interesting questoin remains why BIOS is
> not reporting the boot cpu.

Ok this looks good. But it raises a couple of followup questions.
- Are there other places that use hard_smp_processor_id in
in a uniprocessor kernel?
- Does x86_64 have this same problem?

Anyway it looks like we have this working which is a big step forward
in having a reliable kdump mechanism.

Eric

2005-10-25 07:17:28

by Eric W. Biederman

[permalink] [raw]
Subject: Re: [Fastboot] [PATCH] i386: move apic init in init_IRQs

Vivek Goyal <[email protected]> writes:
> I have attached a patch with the mail which is now using
> boot_cpu_physical_apicid to hard set presence of boot cpu instead of
> hard_smp_processor_id(). But the interesting questoin remains why BIOS is
> not reporting the boot cpu.


Ok. I don't know if we care but I do know why we were not seeing
the report from the bios about your boot processor. We record
information about cpus for up to NR_CPUS, and since you had
a UP kernel NR_CPUS was one.

>From your earlier boot log.

> ACPI: LAPIC (acpi_id[0x00] lapic_id[0x03] enabled)
> Processor #3 6:10 APIC version 17
> ACPI: LAPIC (acpi_id[0x01] lapic_id[0x00] enabled)
> Processor #0 6:10 APIC version 17
> WARNING: NR_CPUS limit of 1 reached. Processor ignored.
> ACPI: LAPIC (acpi_id[0x02] lapic_id[0x01] enabled)
> Processor #1 6:10 APIC version 17
> WARNING: NR_CPUS limit of 1 reached. Processor ignored.
> ACPI: LAPIC (acpi_id[0x03] lapic_id[0x02] enabled)
> Processor #2 6:10 APIC version 17
> WARNING: NR_CPUS limit of 1 reached. Processor ignored.

So it looks like we have this problem completely fixed.

I don't see a good way to ensure that we always record our boot
apicid when we boot a multiple processor system and only use one
processor.

Eric



2005-10-25 07:47:31

by Eric W. Biederman

[permalink] [raw]
Subject: [PATCH] i386 mpparse: Only ignore lapic information we can't store


After staring at mpparse.c for a little longer I noticed that
when we hit our limit of num_processors we are filtering out
information about other processors that we can still store.

This patch just reorders the code so we store everything we
can.

This should avoid the incorrect warning about our boot CPU
not being listed by the BIOS that we are now getting in
the kexec on panic case, and it should allow us to detect
all apicid conflicts even when our physical number of
cpus exceeds maxcpus.

Signed-off-by: Eric W. Biederman <[email protected]>


---

arch/i386/kernel/mpparse.c | 35 +++++++++++++++++++----------------
1 files changed, 19 insertions(+), 16 deletions(-)

applies-to: cf16f96fe9347e42dd2fc6b305005a52783195d4
192f11c9442be11c6535b38d371aa3771fd9513e
diff --git a/arch/i386/kernel/mpparse.c b/arch/i386/kernel/mpparse.c
index 27aabfc..07555a4 100644
--- a/arch/i386/kernel/mpparse.c
+++ b/arch/i386/kernel/mpparse.c
@@ -182,17 +182,6 @@ static void __init MP_processor_info (st
boot_cpu_physical_apicid = m->mpc_apicid;
}

- if (num_processors >= NR_CPUS) {
- printk(KERN_WARNING "WARNING: NR_CPUS limit of %i reached."
- " Processor ignored.\n", NR_CPUS);
- return;
- }
-
- if (num_processors >= maxcpus) {
- printk(KERN_WARNING "WARNING: maxcpus limit of %i reached."
- " Processor ignored.\n", maxcpus);
- return;
- }
ver = m->mpc_apicver;

if (!MP_valid_apicid(apicid, ver)) {
@@ -201,11 +190,6 @@ static void __init MP_processor_info (st
return;
}

- cpu_set(num_processors, cpu_possible_map);
- num_processors++;
- phys_cpu = apicid_to_cpu_present(apicid);
- physids_or(phys_cpu_present_map, phys_cpu_present_map, phys_cpu);
-
/*
* Validate version
*/
@@ -216,6 +200,25 @@ static void __init MP_processor_info (st
ver = 0x10;
}
apic_version[m->mpc_apicid] = ver;
+
+ phys_cpu = apicid_to_cpu_present(apicid);
+ physids_or(phys_cpu_present_map, phys_cpu_present_map, phys_cpu);
+
+ if (num_processors >= NR_CPUS) {
+ printk(KERN_WARNING "WARNING: NR_CPUS limit of %i reached."
+ " Processor ignored.\n", NR_CPUS);
+ return;
+ }
+
+ if (num_processors >= maxcpus) {
+ printk(KERN_WARNING "WARNING: maxcpus limit of %i reached."
+ " Processor ignored.\n", maxcpus);
+ return;
+ }
+
+ cpu_set(num_processors, cpu_possible_map);
+ num_processors++;
+
if ((num_processors > 8) &&
APIC_XAPIC(ver) &&
(boot_cpu_data.x86_vendor == X86_VENDOR_INTEL))

2005-10-25 09:42:50

by Vivek Goyal

[permalink] [raw]
Subject: Re: [PATCH] i386 mpparse: Only ignore lapic information we can't store

On Tue, Oct 25, 2005 at 01:47:08AM -0600, Eric W. Biederman wrote:
>
> After staring at mpparse.c for a little longer I noticed that
> when we hit our limit of num_processors we are filtering out
> information about other processors that we can still store.
>
> This patch just reorders the code so we store everything we
> can.
>
> This should avoid the incorrect warning about our boot CPU
> not being listed by the BIOS that we are now getting in
> the kexec on panic case, and it should allow us to detect
> all apicid conflicts even when our physical number of
> cpus exceeds maxcpus.
>
> Signed-off-by: Eric W. Biederman <[email protected]>
>
>

I justed tested the patch. It looks good. BIOS not reporting CPU message
is gone.

Thanks
Vivek

> ---
>
> arch/i386/kernel/mpparse.c | 35 +++++++++++++++++++----------------
> 1 files changed, 19 insertions(+), 16 deletions(-)
>
> applies-to: cf16f96fe9347e42dd2fc6b305005a52783195d4
> 192f11c9442be11c6535b38d371aa3771fd9513e
> diff --git a/arch/i386/kernel/mpparse.c b/arch/i386/kernel/mpparse.c
> index 27aabfc..07555a4 100644
> --- a/arch/i386/kernel/mpparse.c
> +++ b/arch/i386/kernel/mpparse.c
> @@ -182,17 +182,6 @@ static void __init MP_processor_info (st
> boot_cpu_physical_apicid = m->mpc_apicid;
> }
>
> - if (num_processors >= NR_CPUS) {
> - printk(KERN_WARNING "WARNING: NR_CPUS limit of %i reached."
> - " Processor ignored.\n", NR_CPUS);
> - return;
> - }
> -
> - if (num_processors >= maxcpus) {
> - printk(KERN_WARNING "WARNING: maxcpus limit of %i reached."
> - " Processor ignored.\n", maxcpus);
> - return;
> - }
> ver = m->mpc_apicver;
>
> if (!MP_valid_apicid(apicid, ver)) {
> @@ -201,11 +190,6 @@ static void __init MP_processor_info (st
> return;
> }
>
> - cpu_set(num_processors, cpu_possible_map);
> - num_processors++;
> - phys_cpu = apicid_to_cpu_present(apicid);
> - physids_or(phys_cpu_present_map, phys_cpu_present_map, phys_cpu);
> -
> /*
> * Validate version
> */
> @@ -216,6 +200,25 @@ static void __init MP_processor_info (st
> ver = 0x10;
> }
> apic_version[m->mpc_apicid] = ver;
> +
> + phys_cpu = apicid_to_cpu_present(apicid);
> + physids_or(phys_cpu_present_map, phys_cpu_present_map, phys_cpu);
> +
> + if (num_processors >= NR_CPUS) {
> + printk(KERN_WARNING "WARNING: NR_CPUS limit of %i reached."
> + " Processor ignored.\n", NR_CPUS);
> + return;
> + }
> +
> + if (num_processors >= maxcpus) {
> + printk(KERN_WARNING "WARNING: maxcpus limit of %i reached."
> + " Processor ignored.\n", maxcpus);
> + return;
> + }
> +
> + cpu_set(num_processors, cpu_possible_map);
> + num_processors++;
> +
> if ((num_processors > 8) &&
> APIC_XAPIC(ver) &&
> (boot_cpu_data.x86_vendor == X86_VENDOR_INTEL))