Skip to contentSkip to conversation

x86-64 UP_IOAPIC panic caused by cpumask_t conversion

BUG New Andi Kleen 3 participants 4 messages Aug 19, 2003 – Aug 20, 2003 1 message not in the archive

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

Andi Kleen

author
#1 lore raw
Quoted 12 lines · Mikael Pettersson
include/linux/cpumask.h renamed from include/linux/cpumask.h.~1~ +1 -1
@@ -21,7 +21,7 @@
 typedef unsigned long cpumask_t;
 #endif
 
-#ifdef CONFIG_SMP
+#if defined(CONFIG_SMP) || defined(CONFIG_X86_IO_APIC)
 #if NR_CPUS > BITS_PER_LONG
 #include <asm-generic/cpumask_array.h>
 #else
Quoted 3 lines

Since NR_CPUS==1 this makes UP_IOAPIC use cpumask_arith.h, which is what the code used before the cpumask_t conversion. With this change, the box boots Ok again.

Nasty.

But why does i386/UP work?

Quoted 3 lines

(I believe this is the correct thing to do, except having CONFIG_X86_IO_APIC in generic code isn't quite right.)

Better would be to undo the cpumask_t changes in io_apic.c and go back to unsigned long masks there again.

Obviously a cpu mask is not the right data structure to manage APICs

Another way would be to do whatever i386 does to avoid the problem. The IO-APIC code is unfortunately quite out of date/unsynced compared to i386, maybe it just needs some bug fix ported over. I will check that later.

-Andi

#2 lore raw
Quoted 2 lines · Mikael Pettersson

Nasty.
But why does i386/UP work?

Quoted 8 lines · Andi Kleen
Quoted 2 lines

(I believe this is the correct thing to do, except having CONFIG_X86_IO_APIC in generic code isn't quite right.)

Better would be to undo the cpumask_t changes in io_apic.c
and go back to unsigned long masks there again.
Obviously a cpu mask is not the right data structure to manage APICs Another way would be to do whatever i386 does to avoid the problem. The IO-APIC code is unfortunately quite out of date/unsynced compared to i386, maybe it just needs some bug fix ported over. I will check that later.

Odd; I have a UP IO-APIC ia32 box here and it appears to do okay; there is a question of sparse APIC ID's and APIC ID space needing to be independent of NR_CPUS handled in the ia32 code that isn't handled in the x86_64 code. It was handled for ia32 by using a bitmap of size MAX_APICS (physid_mask_t) instead of cpumask_t for the things, which appears to eliminate various special cases for xAPIC's too.

-- wli

#3 lore raw
Quoted 6 lines

Odd; I have a UP IO-APIC ia32 box here and it appears to do okay; there is a question of sparse APIC ID's and APIC ID space needing to be independent of NR_CPUS handled in the ia32 code that isn't handled in the x86_64 code. It was handled for ia32 by using a bitmap of size MAX_APICS (physid_mask_t) instead of cpumask_t for the things, which appears to eliminate various special cases for xAPIC's too.

Ok, then the physid_mask_t code needs to be ported over to x86-64.

-Andi

#4 lore raw

PATCH 1/1 physid_maskt for x86-64 was R[BUG][2.6.0-test3-bk7] x86-64 UP_IOAPIC panic caused by cpumask_t conversion

This patch fixes the UP boot problem in 2.6.0test3-bk7 for me. I just ported over the physid_mask_t code from i386 for the IO-APIC ids. This avoids the problems with the boolean cpumask on UP kernels.

Caveat: I only tested it with a UP kernel on a UP machine. SMP is untested.

-Andi

arch/x86_64/kernel/apic.c renamed from arch/x86_64/kernel/apic.c-o +2 -2
diff -u linux-2.5-amd64/arch/x86_64/kernel/apic.c-o linux-2.5-amd64/arch/x86_64/kernel/apic.c
@@ -299,7 +299,7 @@
 	 * This is meaningless in clustered apic mode, so we skip it.
 	 */
 	if (!clustered_apic_mode &&
-		!cpu_isset(GET_APIC_ID(apic_read(APIC_ID)), phys_cpu_present_map))
+		!physid_isset(GET_APIC_ID(apic_read(APIC_ID)), phys_cpu_present_map))
 		BUG();
 
 	/*

@@ -993,7 +993,7 @@
 
 	connect_bsp_APIC();
 
-	phys_cpu_present_map = cpumask_of_cpu(0);
+	phys_cpu_present_map = physid_mask_of_physid(0);
 	apic_write_around(APIC_ID, boot_cpu_id);
 
 	setup_local_APIC();
arch/x86_64/kernel/smpboot.c renamed from arch/x86_64/kernel/smpboot.c-o +8 -8
diff -u linux-2.5-amd64/arch/x86_64/kernel/smpboot.c-o linux-2.5-amd64/arch/x86_64/kernel/smpboot.c
@@ -734,10 +734,10 @@
 	current_thread_info()->cpu = 0;
 	smp_tune_scheduling();
 
-	if (!cpu_isset(hard_smp_processor_id(), phys_cpu_present_map)) {
+	if (!physid_isset(hard_smp_processor_id(), phys_cpu_present_map)) {
 		printk("weird, boot CPU (#%d) not listed by the BIOS.\n",
 		       hard_smp_processor_id());
-		cpu_set(hard_smp_processor_id(), phys_cpu_present_map);
+		physid_set(hard_smp_processor_id(), phys_cpu_present_map);
 	}
 
 	/*

@@ -748,7 +748,7 @@
 		printk(KERN_NOTICE "SMP motherboard not detected.\n");
 		io_apic_irqs = 0;
 		cpu_online_map = cpumask_of_cpu(0);
-		phys_cpu_present_map = cpumask_of_cpu(0);
+		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");

@@ -759,10 +759,10 @@
 	 * Should not be necessary because the MP table should list the boot
 	 * CPU too, but we do it for the sake of robustness anyway.
 	 */
-	if (!cpu_isset(boot_cpu_id, phys_cpu_present_map)) {
+	if (!physid_isset(boot_cpu_id, phys_cpu_present_map)) {
 		printk(KERN_NOTICE "weird, boot CPU (#%d) not listed by the BIOS.\n",
 								 boot_cpu_id);
-		cpu_set(hard_smp_processor_id(), phys_cpu_present_map);
+		physid_set(hard_smp_processor_id(), phys_cpu_present_map);
 	}
 
 	/*

@@ -774,7 +774,7 @@
 		printk(KERN_ERR "... forcing use of dummy APIC emulation. (tell your hw vendor)\n");
 		io_apic_irqs = 0;
 		cpu_online_map = cpumask_of_cpu(0);
-		phys_cpu_present_map = cpumask_of_cpu(0);
+		phys_cpu_present_map = physid_mask_of_physid(0);
 		disable_apic = 1;
 		goto smp_done;
 	}

@@ -789,7 +789,7 @@
 		printk(KERN_INFO "SMP mode deactivated, forcing use of dummy APIC emulation.\n");
 		io_apic_irqs = 0;
 		cpu_online_map = cpumask_of_cpu(0);
-		phys_cpu_present_map = cpumask_of_cpu(0);
+		phys_cpu_present_map = physid_mask_of_physid(0);
 		disable_apic = 1;
 		goto smp_done;
 	}

@@ -803,7 +803,7 @@
 	/*
 	 * Now scan the CPU present map and fire up the other CPUs.
 	 */
-	Dprintk("CPU present map: %lx\n", phys_cpu_present_map);
+	Dprintk("CPU present map: %lx\n", physids_coerce(phys_cpu_present_map));
 
 	for (apicid = 0; apicid < NR_CPUS; apicid++) {
 		/*
arch/x86_64/kernel/mpparse.c renamed from arch/x86_64/kernel/mpparse.c-o +2 -2
diff -u linux-2.5-amd64/arch/x86_64/kernel/mpparse.c-o linux-2.5-amd64/arch/x86_64/kernel/mpparse.c
@@ -67,7 +67,7 @@
 static unsigned int num_processors = 0;
 
 /* Bitmask of physically existing CPUs */
-cpumask_t phys_cpu_present_map = CPU_MASK_NONE;
+physid_mask_t phys_cpu_present_map = PHYSID_MASK_NONE;
 
 /* ACPI MADT entry parsing functions */
 #ifdef CONFIG_ACPI_BOOT

@@ -126,7 +126,7 @@
 	}
 	ver = m->mpc_apicver;
 
-	cpu_set(m->mpc_apicid, phys_cpu_present_map);
+	physid_set(m->mpc_apicid, phys_cpu_present_map);
 	/*
 	 * Validate version
 	 */
arch/x86_64/kernel/io_apic.c renamed from arch/x86_64/kernel/io_apic.c-o +10 -10
diff -u linux-2.5-amd64/arch/x86_64/kernel/io_apic.c-o linux-2.5-amd64/arch/x86_64/kernel/io_apic.c
@@ -1014,7 +1014,7 @@
 static void __init setup_ioapic_ids_from_mpc (void)
 {
 	union IO_APIC_reg_00 reg_00;
-	cpumask_t phys_id_present_map = phys_cpu_present_map;
+	physid_mask_t phys_id_present_map = phys_cpu_present_map;
 	int apic;
 	int i;
 	unsigned char old_id;

@@ -1047,22 +1047,22 @@
 		 * system must have a unique ID or we get lots of nice
 		 * 'stuck on smp_invalidate_needed IPI wait' messages.
 	 	 */
-		if (cpu_isset(mp_ioapics[apic].mpc_apicid, phys_id_present_map)) {
+		if (physid_isset(mp_ioapics[apic].mpc_apicid, phys_id_present_map)) {
 			printk(KERN_ERR "BIOS bug, IO-APIC#%d ID %d is already used!...\n",
 				apic, mp_ioapics[apic].mpc_apicid);
 			for (i = 0; i < 0xf; i++)
-				if (!cpu_isset(i, phys_id_present_map))
+				if (!physid_isset(i, phys_id_present_map))
 					break;
 			if (i >= 0xf)
 				panic("Max APIC ID exceeded!\n");
 			printk(KERN_ERR "... fixing up to %d. (tell your hw vendor)\n",
 				i);
-			cpu_set(i, phys_id_present_map);
+			physid_set(i, phys_id_present_map);
 			mp_ioapics[apic].mpc_apicid = i;
 		} else {
 			printk(KERN_INFO 
 			       "Using IO-APIC %d\n", mp_ioapics[apic].mpc_apicid);
-			cpu_set(mp_ioapics[apic].mpc_apicid, phys_id_present_map);
+			physid_set(mp_ioapics[apic].mpc_apicid, phys_id_present_map);
 		}
 
 

@@ -1642,7 +1642,7 @@
 int __init io_apic_get_unique_id (int ioapic, int apic_id)
 {
 	union IO_APIC_reg_00 reg_00;
-	static cpumask_t apic_id_map;
+	static physid_mask_t apic_id_map;
 	unsigned long flags;
 	int i = 0;
 

@@ -1655,7 +1655,7 @@
 	 *      advantage of new APIC bus architecture.
 	 */
 
-	if (!cpus_empty(apic_id_map))
+	if (!physids_empty(apic_id_map))
 		apic_id_map = phys_cpu_present_map;
 
 	spin_lock_irqsave(&ioapic_lock, flags);

@@ -1672,10 +1672,10 @@
 	 * Every APIC in a system must have a unique ID or we get lots of nice 
 	 * 'stuck on smp_invalidate_needed IPI wait' messages.
 	 */
-	if (cpu_isset(apic_id, apic_id_map)) {
+	if (physid_isset(apic_id, apic_id_map)) {
 
 		for (i = 0; i < IO_APIC_MAX_ID; i++) {
-			if (!cpu_isset(i, apic_id_map))
+			if (!physid_isset(i, apic_id_map))
 				break;
 		}
 

@@ -1688,7 +1688,7 @@
 		apic_id = i;
 	} 
 
-	cpu_set(apic_id, apic_id_map);
+	physid_set(apic_id, apic_id_map);
 
 	if (reg_00.bits.ID != apic_id) {
 		reg_00.bits.ID = apic_id;
include/asm-x86_64/mpspec.h renamed from include/asm-x86_64/mpspec.h-o +44 -1

56 lines not on this page · raw message