2003-07-04 08:35:25

by Helge Hafting

[permalink] [raw]
Subject: Re: 2.5.74-mm1 fails to boot due to APIC trouble, 2.5.73mm3 works.

2.5.74-mm1 dies very early during bootup due to some APIC trouble:
(written down by hand)
Posix conformance testing by UNIFIX
enabled Extint on cpu #0
ESR before enabling vector 00000000
ESR after enabling vector 00000000
Enabling IP-APIC IRQs
BIOS bug, IO-APIC #0 ID2 is already used!...
kernel panic: Max APIC ID exceeded!



Here is the corresponding log for 2.5.73mm3
(pasted from dmesg)
POSIX conformance testing by UNIFIX
enabled ExtINT on CPU#0
ESR value before enabling vector: 00000000
ESR value after enabling vector: 00000000
ENABLING IO-APIC IRQs
Setting 2 in the phys_id_present_map
...changing IO-APIC physical APIC ID to 2 ... ok.
init IO_APIC IRQs
IO-APIC (apicid-pin) 2-0, 2-5, 2-9, 2-11, 2-17, 2-19 not connected.
..TIMER: vector=0x31 pin1=2 pin2=0
number of MP IRQ sources: 22.
number of IO-APIC #2 registers: 24.
testing the IO APIC.......................
IO APIC #2......
.... register #00: 02000000
....... : physical APIC id: 02
....... : Delivery Type: 0
....... : LTS : 0
.... register #01: 00178014
....... : max redirection entries: 0017
....... : PRQ implemented: 1
....... : IO APIC version: 0014
.... register #02: 02000000
....... : arbitration: 02
.... IRQ redirection table:
NR Log Phy Mask Trig IRR Pol Stat Dest Deli Vect:
00 000 00 1 0 0 0 0 0 0 00
01 001 01 0 0 0 0 0 1 1 39
02 001 01 0 0 0 0 0 1 1 31
03 001 01 0 0 0 0 0 1 1 41
04 001 01 0 0 0 0 0 1 1 49
05 000 00 1 0 0 0 0 0 0 00
06 001 01 0 0 0 0 0 1 1 51
07 001 01 0 0 0 0 0 1 1 59
08 001 01 0 0 0 0 0 1 1 61
09 000 00 1 0 0 0 0 0 0 00
0a 001 01 0 0 0 0 0 1 1 69
0b 000 00 1 0 0 0 0 0 0 00
0c 001 01 0 0 0 0 0 1 1 71
0d 001 01 0 0 0 0 0 1 1 79
0e 001 01 0 0 0 0 0 1 1 81
0f 001 01 0 0 0 0 0 1 1 89
10 001 01 1 1 0 1 0 1 1 91
11 000 00 1 0 0 0 0 0 0 00
12 001 01 1 1 0 1 0 1 1 99
13 000 00 1 0 0 0 0 0 0 00
14 001 01 1 1 0 1 0 1 1 A1
15 001 01 1 1 0 1 0 1 1 A9
16 001 01 1 1 0 1 0 1 1 B1
17 001 01 1 1 0 1 0 1 1 B9

IRQ to pin mappings:
IRQ0 -> 0:2
IRQ1 -> 0:1
IRQ3 -> 0:3
IRQ4 -> 0:4
IRQ6 -> 0:6
IRQ7 -> 0:7
IRQ8 -> 0:8
IRQ10 -> 0:10
IRQ12 -> 0:12
IRQ13 -> 0:13
IRQ14 -> 0:14
IRQ15 -> 0:15
IRQ16 -> 0:16
IRQ18 -> 0:18
IRQ20 -> 0:20
IRQ21 -> 0:21
IRQ22 -> 0:22
IRQ23 -> 0:23
.................................... done.
Using local APIC timer interrupts.
calibrating APIC timer ...
..... CPU clock speed is 2390.3168 MHz.
..... host bus clock speed is 132.7952 MHz.
mtrr: v2.0 (20020519)


Seems the kernel wants to use APIC ID2, but
2.5.74 claims the BIOS used it up?



Some more APIC related info from 2.5.73mm3 dmesg:
511MB LOWMEM available.
found SMP MP-table at 000f5670
hm, page 000f5000 reserved twice.
hm, page 000f6000 reserved twice.
hm, page 000f1000 reserved twice.
hm, page 000f2000 reserved twice.
On node 0 totalpages: 131056
DMA zone: 4096 pages, LIFO batch:1
Normal zone: 126960 pages, LIFO batch:16
HighMem zone: 0 pages, LIFO batch:1
Intel MultiProcessor Specification v1.4
Virtual Wire compatibility mode.
OEM ID: OEM00000 Product ID: PROD00000000 APIC at: 0xFEE00000
Processor #0 15:2 APIC version 17
I/O APIC #2 Version 17 at 0xFEC00000.
Enabling APIC mode: Flat. Using 1 I/O APICs
Processors: 1


lspci output, in case it matters:
00:00.0 Host bridge: Silicon Integrated Systems [SiS] SiS645DX Host &
Memory & AGP Controller
00:01.0 PCI bridge: Silicon Integrated Systems [SiS] SiS 530 Virtual
PCI-to-PCI bridge (AGP)
00:02.0 ISA bridge: Silicon Integrated Systems [SiS] 85C503/5513 (rev 04)
00:02.1 SMBus: Silicon Integrated Systems [SiS]: Unknown device 0016
00:02.5 IDE interface: Silicon Integrated Systems [SiS] 5513 [IDE]
00:02.7 Multimedia audio controller: Silicon Integrated Systems [SiS]
SiS7012 PCI Audio Accelerator (rev a0)
00:03.0 USB Controller: Silicon Integrated Systems [SiS] SiS7001 USB
Controller (rev 0f)
00:03.1 USB Controller: Silicon Integrated Systems [SiS] SiS7001 USB
Controller (rev 0f)
00:03.2 USB Controller: Silicon Integrated Systems [SiS] SiS7001 USB
Controller (rev 0f)
00:03.3 USB Controller: Silicon Integrated Systems [SiS] SiS7002 USB 2.0
00:0b.0 Ethernet controller: 3Com Corporation 3c905C-TX/TX-M [Tornado]
(rev 78)
00:0c.0 Ethernet controller: Realtek Semiconductor Co., Ltd.
RTL-8139/8139C/8139C+ (rev 10)
01:00.0 VGA compatible controller: ATI Technologies Inc Radeon RV100 QY
[Radeon 7000/VE]

This is a desktop P4 with 512M. The kernel is a UP kernel.

Helge Hafting


2003-07-04 08:40:06

by William Lee Irwin III

[permalink] [raw]
Subject: Re: 2.5.74-mm1 fails to boot due to APIC trouble, 2.5.73mm3 works.

On Fri, Jul 04, 2003 at 10:55:37AM +0200, Helge Hafting wrote:
> 2.5.74-mm1 dies very early during bootup due to some APIC trouble:
> (written down by hand)
> Posix conformance testing by UNIFIX
> enabled Extint on cpu #0
> ESR before enabling vector 00000000
> ESR after enabling vector 00000000
> Enabling IP-APIC IRQs
> BIOS bug, IO-APIC #0 ID2 is already used!...
> kernel panic: Max APIC ID exceeded!

Okay, fixing.


-- wli

2003-07-04 09:22:18

by William Lee Irwin III

[permalink] [raw]
Subject: Re: 2.5.74-mm1 fails to boot due to APIC trouble, 2.5.73mm3 works.

On Fri, Jul 04, 2003 at 10:55:37AM +0200, Helge Hafting wrote:
> 2.5.74-mm1 dies very early during bootup due to some APIC trouble:
> (written down by hand)
> Posix conformance testing by UNIFIX
> enabled Extint on cpu #0
> ESR before enabling vector 00000000
> ESR after enabling vector 00000000
> Enabling IP-APIC IRQs
> BIOS bug, IO-APIC #0 ID2 is already used!...
> kernel panic: Max APIC ID exceeded!

Okay, now for the "final solution" wrt. sparse physical APIC ID's
in addition to what I hope is a fix for your bug. This uses a separate
bitmap type (of a NR_CPUS -independent width MAX_APICS) for physical
APIC ID bitmaps.

\begin{cross-fingers}


-- wli


diff -prauN virgin_cpu-2.5.74-1/arch/i386/kernel/apic.c virgin_cpu-2.5.74-2/arch/i386/kernel/apic.c
--- virgin_cpu-2.5.74-1/arch/i386/kernel/apic.c 2003-07-04 02:27:26.000000000 -0700
+++ virgin_cpu-2.5.74-2/arch/i386/kernel/apic.c 2003-07-04 02:29:01.000000000 -0700
@@ -1137,7 +1137,7 @@ int __init APIC_init_uniprocessor (void)

connect_bsp_APIC();

- phys_cpu_present_map = cpumask_of_cpu(boot_cpu_physical_apicid);
+ phys_cpu_present_map = physid_mask_of_physid(boot_cpu_physical_apicid);

setup_local_APIC();

diff -prauN virgin_cpu-2.5.74-1/arch/i386/kernel/io_apic.c virgin_cpu-2.5.74-2/arch/i386/kernel/io_apic.c
--- virgin_cpu-2.5.74-1/arch/i386/kernel/io_apic.c 2003-07-04 02:27:26.000000000 -0700
+++ virgin_cpu-2.5.74-2/arch/i386/kernel/io_apic.c 2003-07-04 02:29:01.000000000 -0700
@@ -1600,7 +1600,7 @@ void disable_IO_APIC(void)
static void __init setup_ioapic_ids_from_mpc(void)
{
union IO_APIC_reg_00 reg_00;
- cpumask_t phys_id_present_map;
+ physid_mask_t phys_id_present_map;
int apic;
int i;
unsigned char old_id;
@@ -1614,8 +1614,7 @@ static void __init setup_ioapic_ids_from
* This is broken; anything with a real cpu count has to
* circumvent this idiocy regardless.
*/
- phys_id_present_map =
- ioapic_phys_id_map(mk_cpumask_const(phys_cpu_present_map));
+ phys_id_present_map = ioapic_phys_id_map(phys_cpu_present_map);

/*
* Set the IOAPIC ID to the value stored in the MPC table.
@@ -1646,20 +1645,20 @@ static void __init setup_ioapic_ids_from
mp_ioapics[apic].mpc_apicid)) {
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))
+ for (i = 0; i < APIC_BROADCAST_ID; i++)
+ if (!physid_isset(i, phys_id_present_map))
break;
- if (i >= 0xf)
+ if (i >= APIC_BROADCAST_ID)
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 {
- cpumask_t tmp;
+ physid_mask_t tmp;
tmp = apicid_to_cpu_present(mp_ioapics[apic].mpc_apicid);
printk("Setting %d in the phys_id_present_map\n", mp_ioapics[apic].mpc_apicid);
- cpus_or(phys_id_present_map, phys_id_present_map, tmp);
+ physids_or(phys_id_present_map, phys_id_present_map, tmp);
}


@@ -2230,8 +2229,8 @@ late_initcall(io_apic_bug_finalize);
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 = CPU_MASK_NONE;
- cpumask_t tmp;
+ static physid_mask_t apic_id_map = CPU_MASK_NONE;
+ physid_mask_t tmp;
unsigned long flags;
int i = 0;

@@ -2244,8 +2243,8 @@ int __init io_apic_get_unique_id (int io
* advantage of new APIC bus architecture.
*/

- if (cpus_empty(apic_id_map))
- apic_id_map = ioapic_phys_id_map(mk_cpumask_const(phys_cpu_present_map));
+ if (physids_empty(apic_id_map))
+ apic_id_map = ioapic_phys_id_map(phys_cpu_present_map);

spin_lock_irqsave(&ioapic_lock, flags);
reg_00.raw = io_apic_read(ioapic, 0);
@@ -2278,7 +2277,7 @@ int __init io_apic_get_unique_id (int io
}

tmp = apicid_to_cpu_present(apic_id);
- cpus_or(apic_id_map, apic_id_map, tmp);
+ physids_or(apic_id_map, apic_id_map, tmp);

if (reg_00.bits.ID != apic_id) {
reg_00.bits.ID = apic_id;
diff -prauN virgin_cpu-2.5.74-1/arch/i386/kernel/mpparse.c virgin_cpu-2.5.74-2/arch/i386/kernel/mpparse.c
--- virgin_cpu-2.5.74-1/arch/i386/kernel/mpparse.c 2003-07-04 02:27:26.000000000 -0700
+++ virgin_cpu-2.5.74-2/arch/i386/kernel/mpparse.c 2003-07-04 02:29:01.000000000 -0700
@@ -71,7 +71,7 @@ unsigned int boot_cpu_logical_apicid = -
static unsigned int __initdata num_processors;

/* Bitmask of physically existing CPUs */
-cpumask_t phys_cpu_present_map;
+physid_mask_t phys_cpu_present_map;

u8 bios_cpu_apicid[NR_CPUS] = { [0 ... NR_CPUS-1] = BAD_APICID };

@@ -106,7 +106,7 @@ static struct mpc_config_translation *tr
void __init MP_processor_info (struct mpc_config_processor *m)
{
int ver, apicid;
- cpumask_t tmp;
+ physid_mask_t tmp;

if (!(m->mpc_cpuflag & CPU_ENABLED))
return;
@@ -178,7 +178,7 @@ void __init MP_processor_info (struct mp
ver = m->mpc_apicver;

tmp = apicid_to_cpu_present(apicid);
- cpus_or(phys_cpu_present_map, phys_cpu_present_map, tmp);
+ physids_or(phys_cpu_present_map, phys_cpu_present_map, tmp);

/*
* Validate version
diff -prauN virgin_cpu-2.5.74-1/arch/i386/kernel/smpboot.c virgin_cpu-2.5.74-2/arch/i386/kernel/smpboot.c
--- virgin_cpu-2.5.74-1/arch/i386/kernel/smpboot.c 2003-07-04 02:27:26.000000000 -0700
+++ virgin_cpu-2.5.74-2/arch/i386/kernel/smpboot.c 2003-07-04 02:31:37.000000000 -0700
@@ -957,7 +957,7 @@ static void __init smp_boot_cpus(unsigne
if (!smp_found_config) {
printk(KERN_NOTICE "SMP motherboard not detected.\n");
smpboot_clear_io_apic_irqs();
- 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");
@@ -984,7 +984,7 @@ static void __init smp_boot_cpus(unsigne
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 = cpumask_of_cpu(0);
+ phys_cpu_present_map = physid_mask_of_physid(0);
return;
}

@@ -997,7 +997,7 @@ static void __init smp_boot_cpus(unsigne
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 = cpumask_of_cpu(0);
+ phys_cpu_present_map = physid_mask_of_physid(0);
return;
}

@@ -1020,7 +1020,7 @@ static void __init smp_boot_cpus(unsigne
Dprintk("CPU present map: %lx\n", cpus_coerce(phys_cpu_present_map));

kicked = 1;
- for (bit = 0; kicked < NR_CPUS && bit < 8*sizeof(cpumask_t); bit++) {
+ for (bit = 0; kicked < NR_CPUS && bit < MAX_APICS; bit++) {
apicid = cpu_present_to_apicid(bit);
/*
* Don't even attempt to start the boot CPU!
diff -prauN virgin_cpu-2.5.74-1/include/asm-i386/genapic.h virgin_cpu-2.5.74-2/include/asm-i386/genapic.h
--- virgin_cpu-2.5.74-1/include/asm-i386/genapic.h 2003-07-04 02:27:26.000000000 -0700
+++ virgin_cpu-2.5.74-2/include/asm-i386/genapic.h 2003-07-04 02:29:01.000000000 -0700
@@ -27,18 +27,18 @@ struct genapic {
int int_dest_mode;
int apic_broadcast_id;
int esr_disable;
- unsigned long (*check_apicid_used)(cpumask_const_t bitmap, int apicid);
+ unsigned long (*check_apicid_used)(physid_mask_t bitmap, int apicid);
unsigned long (*check_apicid_present)(int apicid);
int no_balance_irq;
void (*init_apic_ldr)(void);
- cpumask_t (*ioapic_phys_id_map)(cpumask_const_t map);
+ physid_mask_t (*ioapic_phys_id_map)(cpumask_const_t map);

void (*clustered_apic_check)(void);
int (*multi_timer_check)(int apic, int irq);
int (*apicid_to_node)(int logical_apicid);
int (*cpu_to_logical_apicid)(int cpu);
int (*cpu_present_to_apicid)(int mps_cpu);
- cpumask_t (*apicid_to_cpu_present)(int phys_apicid);
+ physid_mask_t (*apicid_to_cpu_present)(int phys_apicid);
int (*mpc_apic_id)(struct mpc_config_processor *m,
struct mpc_config_translation *t);
void (*setup_portio_remap)(void);
diff -prauN virgin_cpu-2.5.74-1/include/asm-i386/mach-bigsmp/mach_apic.h virgin_cpu-2.5.74-2/include/asm-i386/mach-bigsmp/mach_apic.h
--- virgin_cpu-2.5.74-1/include/asm-i386/mach-bigsmp/mach_apic.h 2003-07-04 02:27:26.000000000 -0700
+++ virgin_cpu-2.5.74-2/include/asm-i386/mach-bigsmp/mach_apic.h 2003-07-04 02:29:01.000000000 -0700
@@ -29,15 +29,15 @@ static inline cpumask_t target_cpus(void
#define INT_DELIVERY_MODE dest_LowestPrio
#define INT_DEST_MODE 1 /* logical delivery broadcast to all procs */

-#define APIC_BROADCAST_ID (0x0f)
-static inline unsigned long check_apicid_used(cpumask_const_t bitmap, int apicid)
+#define APIC_BROADCAST_ID (0xff)
+static inline unsigned long check_apicid_used(physid_mask_t bitmap, int apicid)
{
return 0;
}

static inline unsigned long check_apicid_present(int bit)
{
- return cpu_isset(bit, phys_cpu_present_map);
+ return physid_isset(bit, phys_cpu_present_map);
}

#define apicid_cluster(apicid) (apicid & 0xF0)
@@ -89,9 +89,9 @@ static inline int cpu_present_to_apicid(
return (int) bios_cpu_apicid[mps_cpu];
}

-static inline cpumask_t apicid_to_cpu_present(int phys_apicid)
+static inline physid_mask_t apicid_to_cpu_present(int phys_apicid)
{
- return cpumask_of_cpu(phys_apicid);
+ return physid_mask_of_physid(phys_apicid);
}

extern volatile u8 cpu_2_logical_apicid[];
@@ -112,10 +112,10 @@ static inline int mpc_apic_id(struct mpc
return m->mpc_apicid;
}

-static inline cpumask_t ioapic_phys_id_map(cpumask_const_t phys_map)
+static inline physid_mask_t ioapic_phys_id_map(physid_mask_t phys_map)
{
/* For clustered we don't have a good way to do this yet - hack */
- return cpus_promote(0xFUL);
+ return physids_promote(0xFUL);
}

#define WAKE_SECONDARY_VIA_INIT
diff -prauN virgin_cpu-2.5.74-1/include/asm-i386/mach-default/mach_apic.h virgin_cpu-2.5.74-2/include/asm-i386/mach-default/mach_apic.h
--- virgin_cpu-2.5.74-1/include/asm-i386/mach-default/mach_apic.h 2003-07-04 02:27:26.000000000 -0700
+++ virgin_cpu-2.5.74-2/include/asm-i386/mach-default/mach_apic.h 2003-07-04 02:29:01.000000000 -0700
@@ -21,16 +21,20 @@ static inline cpumask_t target_cpus(void
#define INT_DELIVERY_MODE dest_LowestPrio
#define INT_DEST_MODE 1 /* logical delivery broadcast to all procs */

+/*
+ * this isn't really broadcast, just a (potentially inaccurate) upper
+ * bound for valid physical APIC id's
+ */
#define APIC_BROADCAST_ID 0x0F

-static inline unsigned long check_apicid_used(cpumask_const_t bitmap, int apicid)
+static inline unsigned long check_apicid_used(physid_mask_t bitmap, int apicid)
{
- return cpu_isset_const(apicid, bitmap);
+ return physid_isset(apicid, bitmap);
}

static inline unsigned long check_apicid_present(int bit)
{
- return cpu_isset(bit, phys_cpu_present_map);
+ return physid_isset(bit, phys_cpu_present_map);
}

/*
@@ -50,11 +54,9 @@ static inline void init_apic_ldr(void)
apic_write_around(APIC_LDR, val);
}

-static inline cpumask_t ioapic_phys_id_map(cpumask_const_t phys_map)
+static inline physid_mask_t ioapic_phys_id_map(physid_mask_t phys_map)
{
- cpumask_t ret;
- cpus_copy_const(ret, phys_map);
- return ret;
+ return phys_map;
}

static inline void clustered_apic_check(void)
@@ -84,9 +86,9 @@ static inline int cpu_present_to_apicid(
return mps_cpu;
}

-static inline cpumask_t apicid_to_cpu_present(int phys_apicid)
+static inline physid_mask_t apicid_to_cpu_present(int phys_apicid)
{
- return cpumask_of_cpu(phys_apicid);
+ return physid_mask_of_physid(phys_apicid);
}

static inline int mpc_apic_id(struct mpc_config_processor *m,
@@ -106,12 +108,12 @@ static inline void setup_portio_remap(vo

static inline int check_phys_apicid_present(int boot_cpu_physical_apicid)
{
- return cpu_isset(boot_cpu_physical_apicid, phys_cpu_present_map);
+ return physid_isset(boot_cpu_physical_apicid, phys_cpu_present_map);
}

static inline int apic_id_registered(void)
{
- return cpu_isset(GET_APIC_ID(apic_read(APIC_ID)), phys_cpu_present_map);
+ return physid_isset(GET_APIC_ID(apic_read(APIC_ID)), phys_cpu_present_map);
}

static inline unsigned int cpu_mask_to_apicid(cpumask_const_t cpumask)
diff -prauN virgin_cpu-2.5.74-1/include/asm-i386/mach-es7000/mach_apic.h virgin_cpu-2.5.74-2/include/asm-i386/mach-es7000/mach_apic.h
--- virgin_cpu-2.5.74-1/include/asm-i386/mach-es7000/mach_apic.h 2003-07-04 02:27:26.000000000 -0700
+++ virgin_cpu-2.5.74-2/include/asm-i386/mach-es7000/mach_apic.h 2003-07-04 02:29:01.000000000 -0700
@@ -40,13 +40,13 @@ static inline cpumask_t target_cpus(void

#define APIC_BROADCAST_ID (0xff)

-static inline unsigned long check_apicid_used(cpumask_const_t bitmap, int apicid)
+static inline unsigned long check_apicid_used(physid_mask_t bitmap, int apicid)
{
return 0;
}
static inline unsigned long check_apicid_present(int bit)
{
- return cpu_isset(bit, phys_cpu_present_map);
+ return physid_isset(bit, phys_cpu_present_map);
}

#define apicid_cluster(apicid) (apicid & 0xF0)
@@ -110,12 +110,12 @@ static inline int cpu_present_to_apicid(
return (int) bios_cpu_apicid[mps_cpu];
}

-static inline cpumask_t apicid_to_cpu_present(int phys_apicid)
+static inline physid_mask_t apicid_to_cpu_present(int phys_apicid)
{
- static int cpu = 0;
- cpumask_t mask;
- mask = cpumask_of_cpu(cpu);
- ++cpu;
+ static int id = 0;
+ physid_mask_t mask;
+ mask = physid_mask_of_physid(id);
+ ++id;
return mask;
}

@@ -136,10 +136,10 @@ static inline int mpc_apic_id(struct mpc
return (m->mpc_apicid);
}

-static inline cpumask_t ioapic_phys_id_map(cpumask_const_t phys_map)
+static inline physid_mask_t ioapic_phys_id_map(physid_mask_t phys_map)
{
/* For clustered we don't have a good way to do this yet - hack */
- return cpus_promote(0xff);
+ return physids_promote(0xff);
}


diff -prauN virgin_cpu-2.5.74-1/include/asm-i386/mach-numaq/mach_apic.h virgin_cpu-2.5.74-2/include/asm-i386/mach-numaq/mach_apic.h
--- virgin_cpu-2.5.74-1/include/asm-i386/mach-numaq/mach_apic.h 2003-07-04 02:27:26.000000000 -0700
+++ virgin_cpu-2.5.74-2/include/asm-i386/mach-numaq/mach_apic.h 2003-07-04 02:29:01.000000000 -0700
@@ -21,8 +21,8 @@ static inline cpumask_t target_cpus(void
#define INT_DEST_MODE 0 /* physical delivery on LOCAL quad */

#define APIC_BROADCAST_ID 0x0F
-#define check_apicid_used(bitmap, apicid) cpu_isset_const(apicid, bitmap)
-#define check_apicid_present(bit) cpu_isset(bit, phys_cpu_present_map)
+#define check_apicid_used(bitmap, apicid) physid_isset(apicid, bitmap)
+#define check_apicid_present(bit) physid_isset(bit, phys_cpu_present_map)
#define apicid_cluster(apicid) (apicid & 0xF0)

static inline int apic_id_registered(void)
@@ -50,10 +50,10 @@ static inline int multi_timer_check(int
return apic != 0 && irq == 0;
}

-static inline cpumask_t ioapic_phys_id_map(cpumask_const_t phys_map)
+static inline physid_mask_t ioapic_phys_id_map(physid_mask_t phys_map)
{
/* We don't have a good way to do this yet - hack */
- return cpus_promote(0xFUL);
+ return physids_promote(0xFUL);
}

/* Mapping from cpu number to logical apicid */
@@ -78,12 +78,12 @@ static inline int apicid_to_node(int log
return logical_apicid >> 4;
}

-static inline cpumask_t apicid_to_cpu_present(int logical_apicid)
+static inline physid_mask_t apicid_to_cpu_present(int logical_apicid)
{
int node = apicid_to_node(logical_apicid);
int cpu = __ffs(logical_apicid & 0xf);

- return cpumask_of_cpu(cpu + 4*node);
+ return physid_mask_of_physid(cpu + 4*node);
}

static inline int mpc_apic_id(struct mpc_config_processor *m,
diff -prauN virgin_cpu-2.5.74-1/include/asm-i386/mach-summit/mach_apic.h virgin_cpu-2.5.74-2/include/asm-i386/mach-summit/mach_apic.h
--- virgin_cpu-2.5.74-1/include/asm-i386/mach-summit/mach_apic.h 2003-07-04 02:27:26.000000000 -0700
+++ virgin_cpu-2.5.74-2/include/asm-i386/mach-summit/mach_apic.h 2003-07-04 02:29:01.000000000 -0700
@@ -28,8 +28,8 @@ static inline cpumask_t target_cpus(void
#define INT_DELIVERY_MODE (dest_Fixed)
#define INT_DEST_MODE 1 /* logical delivery broadcast to all procs */

-#define APIC_BROADCAST_ID (0x0F)
-static inline unsigned long check_apicid_used(cpumask_const_t bitmap, int apicid)
+#define APIC_BROADCAST_ID (0xFF)
+static inline unsigned long check_apicid_used(physid_mask_t bitmap, int apicid)
{
return 0;
}
@@ -88,15 +88,15 @@ static inline int cpu_present_to_apicid(
return (int) bios_cpu_apicid[mps_cpu];
}

-static inline cpumask_t ioapic_phys_id_map(cpumask_const_t phys_id_map)
+static inline physid_mask_t ioapic_phys_id_map(physid_mask_t phys_id_map)
{
/* For clustered we don't have a good way to do this yet - hack */
- return cpus_promote(0x0F);
+ return physids_promote(0x0F);
}

-static inline cpumask_t apicid_to_cpu_present(int apicid)
+static inline physid_mask_t apicid_to_cpu_present(int apicid)
{
- return cpumask_of_cpu(0);
+ return physid_mask_of_physid(0);
}

static inline int mpc_apic_id(struct mpc_config_processor *m,
diff -prauN virgin_cpu-2.5.74-1/include/asm-i386/mach-visws/mach_apic.h virgin_cpu-2.5.74-2/include/asm-i386/mach-visws/mach_apic.h
--- virgin_cpu-2.5.74-1/include/asm-i386/mach-visws/mach_apic.h 2003-07-04 02:27:26.000000000 -0700
+++ virgin_cpu-2.5.74-2/include/asm-i386/mach-visws/mach_apic.h 2003-07-04 02:29:01.000000000 -0700
@@ -16,12 +16,12 @@
#endif

#define APIC_BROADCAST_ID 0x0F
-#define check_apicid_used(bitmap, apicid) cpu_isset_const(apicid, bitmap)
-#define check_apicid_present(bit) cpu_isset(bit, phys_cpu_present_map)
+#define check_apicid_used(bitmap, apicid) physid_isset(apicid, bitmap)
+#define check_apicid_present(bit) physid_isset(bit, phys_cpu_present_map)

static inline int apic_id_registered(void)
{
- return cpu_isset(GET_APIC_ID(apic_read(APIC_ID)), phys_cpu_present_map);
+ return physid_isset(GET_APIC_ID(apic_read(APIC_ID)), phys_cpu_present_map);
}

/*
@@ -60,9 +60,9 @@ static inline int cpu_present_to_apicid(
return mps_cpu;
}

-static inline cpumask_t apicid_to_cpu_present(int apicid)
+static inline physid_mask_t apicid_to_cpu_present(int apicid)
{
- return cpumask_of_cpu(apicid);
+ return physid_mask_of_physid(apicid);
}

#define WAKE_SECONDARY_VIA_INIT
@@ -77,7 +77,7 @@ static inline void enable_apic_mode(void

static inline int check_phys_apicid_present(int boot_cpu_physical_apicid)
{
- return cpu_isset(boot_cpu_physical_apicid, phys_cpu_present_map);
+ return physid_isset(boot_cpu_physical_apicid, phys_cpu_present_map);
}

static inline unsigned int cpu_mask_to_apicid(cpumask_const_t cpumask)
diff -prauN virgin_cpu-2.5.74-1/include/asm-i386/mpspec.h virgin_cpu-2.5.74-2/include/asm-i386/mpspec.h
--- virgin_cpu-2.5.74-1/include/asm-i386/mpspec.h 2003-07-04 02:27:26.000000000 -0700
+++ virgin_cpu-2.5.74-2/include/asm-i386/mpspec.h 2003-07-04 02:29:01.000000000 -0700
@@ -12,7 +12,6 @@ extern int quad_local_to_mp_bus_id [NR_C
extern int mp_bus_id_to_pci_bus [MAX_MP_BUSSES];

extern unsigned int boot_cpu_physical_apicid;
-extern cpumask_t phys_cpu_present_map;
extern int smp_found_config;
extern void find_smp_config (void);
extern void get_smp_config (void);
@@ -42,5 +41,49 @@ extern void mp_config_ioapic_for_sci(int
extern void mp_parse_prt (void);
#endif /*CONFIG_ACPI_BOOT*/

+#define PHYSID_ARRAY_SIZE BITS_TO_LONGS(MAX_APICS)
+
+struct physid_mask
+{
+ unsigned long mask[PHYSID_ARRAY_SIZE];
+};
+
+typedef struct physid_mask physid_mask_t;
+
+#define physid_set(physid, map) set_bit(physid, (map).mask)
+#define physid_clear(physid, map) clear_bit(physid, (map).mask)
+#define physid_isset(physid, map) test_bit(physid, (map).mask)
+#define physid_test_and_set(physid, map) test_and_set_bit(physid, (map).mask)
+
+#define physids_and(dst, src1, src2) bitmap_and((dst).mask, (src1).mask, (src2).mask, MAX_APICS)
+#define physids_or(dst, src1, src2) bitmap_or((dst).mask, (src1).mask, (src2).mask, MAX_APICS)
+#define physids_clear(map) bitmap_clear((map).mask, MAX_APICS)
+#define physids_complement(map) bitmap_complement((map).mask, MAX_APICS)
+#define physids_empty(map) bitmap_empty((map).mask, MAX_APICS)
+#define physids_equal(map1, map2) bitmap_equal((map1).mask, (map2).mask, MAX_APICS)
+#define physids_weight(map) bitmap_weight((map).mask, MAX_APICS)
+#define physids_shift_right(d, s, n) bitmap_shift_right((d).mask, (s).mask, n, MAX_APICS)
+#define physids_shift_left(d, s, n) bitmap_shift_left((d).mask, (s).mask, n, MAX_APICS)
+#define physids_coerce(map) ((map).mask[0])
+
+#define physids_promote(physids) \
+ ({ \
+ physid_mask_t __physid_mask = PHYSID_MASK_NONE; \
+ __physid_mask.mask[0] = physids; \
+ __physid_mask; \
+ })
+
+#define physid_mask_of_physid(physid) \
+ ({ \
+ physid_mask_t __physid_mask = PHYSID_MASK_NONE; \
+ physid_set(physid, __physid_mask); \
+ __physid_mask; \
+ })
+
+#define PHYSID_MASK_ALL { {[0 ... PHYSID_ARRAY_SIZE-1] = ~0UL} }
+#define PHYSID_MASK_NONE { {[0 ... PHYSID_ARRAY_SIZE-1] = 0UL} }
+
+extern physid_mask_t phys_cpu_present_map;
+
#endif

diff -prauN virgin_cpu-2.5.74-1/include/asm-i386/smp.h virgin_cpu-2.5.74-2/include/asm-i386/smp.h
--- virgin_cpu-2.5.74-1/include/asm-i386/smp.h 2003-07-04 02:27:26.000000000 -0700
+++ virgin_cpu-2.5.74-2/include/asm-i386/smp.h 2003-07-04 02:29:01.000000000 -0700
@@ -32,7 +32,7 @@
*/

extern void smp_alloc_memory(void);
-extern cpumask_t phys_cpu_present_map;
+extern physid_mask_t phys_cpu_present_map;
extern int pic_mode;
extern int smp_num_siblings;
extern int cpu_sibling_map[];

2003-07-04 09:38:14

by William Lee Irwin III

[permalink] [raw]
Subject: Re: 2.5.74-mm1 fails to boot due to APIC trouble, 2.5.73mm3 works.

On Fri, Jul 04, 2003 at 02:35:31AM -0700, William Lee Irwin III wrote:
> Okay, now for the "final solution" wrt. sparse physical APIC ID's
> in addition to what I hope is a fix for your bug. This uses a separate
> bitmap type (of a NR_CPUS -independent width MAX_APICS) for physical
> APIC ID bitmaps.
> \begin{cross-fingers}

This time diffed against the right tree:


diff -prauN mm1-2.5.74-1/arch/i386/kernel/apic.c physid-2.5.74-1/arch/i386/kernel/apic.c
--- mm1-2.5.74-1/arch/i386/kernel/apic.c 2003-07-03 12:23:55.000000000 -0700
+++ physid-2.5.74-1/arch/i386/kernel/apic.c 2003-07-04 02:45:17.000000000 -0700
@@ -1137,7 +1137,7 @@ int __init APIC_init_uniprocessor (void)

connect_bsp_APIC();

- phys_cpu_present_map = cpumask_of_cpu(boot_cpu_physical_apicid);
+ phys_cpu_present_map = physid_mask_of_physid(boot_cpu_physical_apicid);

setup_local_APIC();

diff -prauN mm1-2.5.74-1/arch/i386/kernel/io_apic.c physid-2.5.74-1/arch/i386/kernel/io_apic.c
--- mm1-2.5.74-1/arch/i386/kernel/io_apic.c 2003-07-03 12:23:55.000000000 -0700
+++ physid-2.5.74-1/arch/i386/kernel/io_apic.c 2003-07-04 02:45:17.000000000 -0700
@@ -1601,7 +1601,7 @@ void disable_IO_APIC(void)
static void __init setup_ioapic_ids_from_mpc(void)
{
union IO_APIC_reg_00 reg_00;
- cpumask_t phys_id_present_map;
+ physid_mask_t phys_id_present_map;
int apic;
int i;
unsigned char old_id;
@@ -1615,8 +1615,7 @@ static void __init setup_ioapic_ids_from
* This is broken; anything with a real cpu count has to
* circumvent this idiocy regardless.
*/
- phys_id_present_map =
- ioapic_phys_id_map(mk_cpumask_const(phys_cpu_present_map));
+ phys_id_present_map = ioapic_phys_id_map(phys_cpu_present_map);

/*
* Set the IOAPIC ID to the value stored in the MPC table.
@@ -1647,20 +1646,20 @@ static void __init setup_ioapic_ids_from
mp_ioapics[apic].mpc_apicid)) {
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))
+ for (i = 0; i < APIC_BROADCAST_ID; i++)
+ if (!physid_isset(i, phys_id_present_map))
break;
- if (i >= 0xf)
+ if (i >= APIC_BROADCAST_ID)
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 {
- cpumask_t tmp;
+ physid_mask_t tmp;
tmp = apicid_to_cpu_present(mp_ioapics[apic].mpc_apicid);
printk("Setting %d in the phys_id_present_map\n", mp_ioapics[apic].mpc_apicid);
- cpus_or(phys_id_present_map, phys_id_present_map, tmp);
+ physids_or(phys_id_present_map, phys_id_present_map, tmp);
}


@@ -2230,8 +2229,8 @@ late_initcall(io_apic_bug_finalize);
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 = CPU_MASK_NONE;
- cpumask_t tmp;
+ static physid_mask_t apic_id_map = CPU_MASK_NONE;
+ physid_mask_t tmp;
unsigned long flags;
int i = 0;

@@ -2244,8 +2243,8 @@ int __init io_apic_get_unique_id (int io
* advantage of new APIC bus architecture.
*/

- if (cpus_empty(apic_id_map))
- apic_id_map = ioapic_phys_id_map(mk_cpumask_const(phys_cpu_present_map));
+ if (physids_empty(apic_id_map))
+ apic_id_map = ioapic_phys_id_map(phys_cpu_present_map);

spin_lock_irqsave(&ioapic_lock, flags);
reg_00.raw = io_apic_read(ioapic, 0);
@@ -2278,7 +2277,7 @@ int __init io_apic_get_unique_id (int io
}

tmp = apicid_to_cpu_present(apic_id);
- cpus_or(apic_id_map, apic_id_map, tmp);
+ physids_or(apic_id_map, apic_id_map, tmp);

if (reg_00.bits.ID != apic_id) {
reg_00.bits.ID = apic_id;
diff -prauN mm1-2.5.74-1/arch/i386/kernel/mpparse.c physid-2.5.74-1/arch/i386/kernel/mpparse.c
--- mm1-2.5.74-1/arch/i386/kernel/mpparse.c 2003-07-03 12:23:55.000000000 -0700
+++ physid-2.5.74-1/arch/i386/kernel/mpparse.c 2003-07-04 02:45:17.000000000 -0700
@@ -71,7 +71,7 @@ unsigned int boot_cpu_logical_apicid = -
static unsigned int __initdata num_processors;

/* Bitmask of physically existing CPUs */
-cpumask_t phys_cpu_present_map;
+physid_mask_t phys_cpu_present_map;

u8 bios_cpu_apicid[NR_CPUS] = { [0 ... NR_CPUS-1] = BAD_APICID };

@@ -106,7 +106,7 @@ static struct mpc_config_translation *tr
void __init MP_processor_info (struct mpc_config_processor *m)
{
int ver, apicid;
- cpumask_t tmp;
+ physid_mask_t tmp;

if (!(m->mpc_cpuflag & CPU_ENABLED))
return;
@@ -178,7 +178,7 @@ void __init MP_processor_info (struct mp
ver = m->mpc_apicver;

tmp = apicid_to_cpu_present(apicid);
- cpus_or(phys_cpu_present_map, phys_cpu_present_map, tmp);
+ physids_or(phys_cpu_present_map, phys_cpu_present_map, tmp);

/*
* Validate version
diff -prauN mm1-2.5.74-1/arch/i386/kernel/smpboot.c physid-2.5.74-1/arch/i386/kernel/smpboot.c
--- mm1-2.5.74-1/arch/i386/kernel/smpboot.c 2003-07-03 12:23:55.000000000 -0700
+++ physid-2.5.74-1/arch/i386/kernel/smpboot.c 2003-07-04 02:45:17.000000000 -0700
@@ -957,7 +957,7 @@ static void __init smp_boot_cpus(unsigne
if (!smp_found_config) {
printk(KERN_NOTICE "SMP motherboard not detected.\n");
smpboot_clear_io_apic_irqs();
- 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");
@@ -984,7 +984,7 @@ static void __init smp_boot_cpus(unsigne
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 = cpumask_of_cpu(0);
+ phys_cpu_present_map = physid_mask_of_physid(0);
return;
}

@@ -997,7 +997,7 @@ static void __init smp_boot_cpus(unsigne
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 = cpumask_of_cpu(0);
+ phys_cpu_present_map = physid_mask_of_physid(0);
return;
}

@@ -1020,7 +1020,7 @@ static void __init smp_boot_cpus(unsigne
Dprintk("CPU present map: %lx\n", cpus_coerce(phys_cpu_present_map));

kicked = 1;
- for (bit = 0; kicked < NR_CPUS && bit < 8*sizeof(cpumask_t); bit++) {
+ for (bit = 0; kicked < NR_CPUS && bit < MAX_APICS; bit++) {
apicid = cpu_present_to_apicid(bit);
/*
* Don't even attempt to start the boot CPU!
diff -prauN mm1-2.5.74-1/include/asm-i386/genapic.h physid-2.5.74-1/include/asm-i386/genapic.h
--- mm1-2.5.74-1/include/asm-i386/genapic.h 2003-07-03 12:23:56.000000000 -0700
+++ physid-2.5.74-1/include/asm-i386/genapic.h 2003-07-04 02:48:52.000000000 -0700
@@ -27,18 +27,18 @@ struct genapic {
int int_dest_mode;
int apic_broadcast_id;
int esr_disable;
- unsigned long (*check_apicid_used)(cpumask_const_t bitmap, int apicid);
+ unsigned long (*check_apicid_used)(physid_mask_t bitmap, int apicid);
unsigned long (*check_apicid_present)(int apicid);
int no_balance_irq;
void (*init_apic_ldr)(void);
- cpumask_t (*ioapic_phys_id_map)(cpumask_const_t map);
+ physid_mask_t (*ioapic_phys_id_map)(physid_mask_t map);

void (*clustered_apic_check)(void);
int (*multi_timer_check)(int apic, int irq);
int (*apicid_to_node)(int logical_apicid);
int (*cpu_to_logical_apicid)(int cpu);
int (*cpu_present_to_apicid)(int mps_cpu);
- cpumask_t (*apicid_to_cpu_present)(int phys_apicid);
+ physid_mask_t (*apicid_to_cpu_present)(int phys_apicid);
int (*mpc_apic_id)(struct mpc_config_processor *m,
struct mpc_config_translation *t);
void (*setup_portio_remap)(void);
diff -prauN mm1-2.5.74-1/include/asm-i386/mach-bigsmp/mach_apic.h physid-2.5.74-1/include/asm-i386/mach-bigsmp/mach_apic.h
--- mm1-2.5.74-1/include/asm-i386/mach-bigsmp/mach_apic.h 2003-07-03 12:23:56.000000000 -0700
+++ physid-2.5.74-1/include/asm-i386/mach-bigsmp/mach_apic.h 2003-07-04 02:47:45.000000000 -0700
@@ -29,15 +29,15 @@ static inline cpumask_t target_cpus(void
#define INT_DELIVERY_MODE dest_LowestPrio
#define INT_DEST_MODE 1 /* logical delivery broadcast to all procs */

-#define APIC_BROADCAST_ID (0x0f)
-static inline unsigned long check_apicid_used(cpumask_const_t bitmap, int apicid)
+#define APIC_BROADCAST_ID (0xff)
+static inline unsigned long check_apicid_used(physid_mask_t bitmap, int apicid)
{
return 0;
}

static inline unsigned long check_apicid_present(int bit)
{
- return cpu_isset(bit, phys_cpu_present_map);
+ return physid_isset(bit, phys_cpu_present_map);
}

#define apicid_cluster(apicid) (apicid & 0xF0)
@@ -89,9 +89,9 @@ static inline int cpu_present_to_apicid(
return (int) bios_cpu_apicid[mps_cpu];
}

-static inline cpumask_t apicid_to_cpu_present(int phys_apicid)
+static inline physid_mask_t apicid_to_cpu_present(int phys_apicid)
{
- return cpumask_of_cpu(phys_apicid);
+ return physid_mask_of_physid(phys_apicid);
}

extern volatile u8 cpu_2_logical_apicid[];
@@ -112,10 +112,10 @@ static inline int mpc_apic_id(struct mpc
return m->mpc_apicid;
}

-static inline cpumask_t ioapic_phys_id_map(cpumask_const_t phys_map)
+static inline physid_mask_t ioapic_phys_id_map(physid_mask_t phys_map)
{
/* For clustered we don't have a good way to do this yet - hack */
- return cpus_promote(0xFUL);
+ return physids_promote(0xFUL);
}

#define WAKE_SECONDARY_VIA_INIT
diff -prauN mm1-2.5.74-1/include/asm-i386/mach-default/mach_apic.h physid-2.5.74-1/include/asm-i386/mach-default/mach_apic.h
--- mm1-2.5.74-1/include/asm-i386/mach-default/mach_apic.h 2003-07-03 12:23:56.000000000 -0700
+++ physid-2.5.74-1/include/asm-i386/mach-default/mach_apic.h 2003-07-04 02:45:17.000000000 -0700
@@ -21,16 +21,20 @@ static inline cpumask_t target_cpus(void
#define INT_DELIVERY_MODE dest_LowestPrio
#define INT_DEST_MODE 1 /* logical delivery broadcast to all procs */

+/*
+ * this isn't really broadcast, just a (potentially inaccurate) upper
+ * bound for valid physical APIC id's
+ */
#define APIC_BROADCAST_ID 0x0F

-static inline unsigned long check_apicid_used(cpumask_const_t bitmap, int apicid)
+static inline unsigned long check_apicid_used(physid_mask_t bitmap, int apicid)
{
- return cpu_isset_const(apicid, bitmap);
+ return physid_isset(apicid, bitmap);
}

static inline unsigned long check_apicid_present(int bit)
{
- return cpu_isset(bit, phys_cpu_present_map);
+ return physid_isset(bit, phys_cpu_present_map);
}

/*
@@ -50,11 +54,9 @@ static inline void init_apic_ldr(void)
apic_write_around(APIC_LDR, val);
}

-static inline cpumask_t ioapic_phys_id_map(cpumask_const_t phys_map)
+static inline physid_mask_t ioapic_phys_id_map(physid_mask_t phys_map)
{
- cpumask_t ret;
- cpus_copy_const(ret, phys_map);
- return ret;
+ return phys_map;
}

static inline void clustered_apic_check(void)
@@ -84,9 +86,9 @@ static inline int cpu_present_to_apicid(
return mps_cpu;
}

-static inline cpumask_t apicid_to_cpu_present(int phys_apicid)
+static inline physid_mask_t apicid_to_cpu_present(int phys_apicid)
{
- return cpumask_of_cpu(phys_apicid);
+ return physid_mask_of_physid(phys_apicid);
}

static inline int mpc_apic_id(struct mpc_config_processor *m,
@@ -106,12 +108,12 @@ static inline void setup_portio_remap(vo

static inline int check_phys_apicid_present(int boot_cpu_physical_apicid)
{
- return cpu_isset(boot_cpu_physical_apicid, phys_cpu_present_map);
+ return physid_isset(boot_cpu_physical_apicid, phys_cpu_present_map);
}

static inline int apic_id_registered(void)
{
- return cpu_isset(GET_APIC_ID(apic_read(APIC_ID)), phys_cpu_present_map);
+ return physid_isset(GET_APIC_ID(apic_read(APIC_ID)), phys_cpu_present_map);
}

static inline unsigned int cpu_mask_to_apicid(cpumask_const_t cpumask)
diff -prauN mm1-2.5.74-1/include/asm-i386/mach-es7000/mach_apic.h physid-2.5.74-1/include/asm-i386/mach-es7000/mach_apic.h
--- mm1-2.5.74-1/include/asm-i386/mach-es7000/mach_apic.h 2003-07-03 12:23:56.000000000 -0700
+++ physid-2.5.74-1/include/asm-i386/mach-es7000/mach_apic.h 2003-07-04 02:46:36.000000000 -0700
@@ -40,13 +40,13 @@ static inline cpumask_t target_cpus(void

#define APIC_BROADCAST_ID (0xff)

-static inline unsigned long check_apicid_used(cpumask_const_t bitmap, int apicid)
+static inline unsigned long check_apicid_used(physid_mask_t bitmap, int apicid)
{
return 0;
}
static inline unsigned long check_apicid_present(int bit)
{
- return cpu_isset(bit, phys_cpu_present_map);
+ return physid_isset(bit, phys_cpu_present_map);
}

#define apicid_cluster(apicid) (apicid & 0xF0)
@@ -110,12 +110,12 @@ static inline int cpu_present_to_apicid(
return (int) bios_cpu_apicid[mps_cpu];
}

-static inline cpumask_t apicid_to_cpu_present(int phys_apicid)
+static inline physid_mask_t apicid_to_cpu_present(int phys_apicid)
{
- static int cpu = 0;
- cpumask_t mask;
- mask = cpumask_of_cpu(cpu);
- ++cpu;
+ static int id = 0;
+ physid_mask_t mask;
+ mask = physid_mask_of_physid(id);
+ ++id;
return mask;
}

@@ -136,10 +136,10 @@ static inline int mpc_apic_id(struct mpc
return (m->mpc_apicid);
}

-static inline cpumask_t ioapic_phys_id_map(cpumask_const_t phys_map)
+static inline physid_mask_t ioapic_phys_id_map(physid_mask_t phys_map)
{
/* For clustered we don't have a good way to do this yet - hack */
- return cpus_promote(0xff);
+ return physids_promote(0xff);
}


diff -prauN mm1-2.5.74-1/include/asm-i386/mach-numaq/mach_apic.h physid-2.5.74-1/include/asm-i386/mach-numaq/mach_apic.h
--- mm1-2.5.74-1/include/asm-i386/mach-numaq/mach_apic.h 2003-07-03 12:23:56.000000000 -0700
+++ physid-2.5.74-1/include/asm-i386/mach-numaq/mach_apic.h 2003-07-04 02:45:17.000000000 -0700
@@ -21,8 +21,8 @@ static inline cpumask_t target_cpus(void
#define INT_DEST_MODE 0 /* physical delivery on LOCAL quad */

#define APIC_BROADCAST_ID 0x0F
-#define check_apicid_used(bitmap, apicid) cpu_isset_const(apicid, bitmap)
-#define check_apicid_present(bit) cpu_isset(bit, phys_cpu_present_map)
+#define check_apicid_used(bitmap, apicid) physid_isset(apicid, bitmap)
+#define check_apicid_present(bit) physid_isset(bit, phys_cpu_present_map)
#define apicid_cluster(apicid) (apicid & 0xF0)

static inline int apic_id_registered(void)
@@ -50,10 +50,10 @@ static inline int multi_timer_check(int
return apic != 0 && irq == 0;
}

-static inline cpumask_t ioapic_phys_id_map(cpumask_const_t phys_map)
+static inline physid_mask_t ioapic_phys_id_map(physid_mask_t phys_map)
{
/* We don't have a good way to do this yet - hack */
- return cpus_promote(0xFUL);
+ return physids_promote(0xFUL);
}

/* Mapping from cpu number to logical apicid */
@@ -78,12 +78,12 @@ static inline int apicid_to_node(int log
return logical_apicid >> 4;
}

-static inline cpumask_t apicid_to_cpu_present(int logical_apicid)
+static inline physid_mask_t apicid_to_cpu_present(int logical_apicid)
{
int node = apicid_to_node(logical_apicid);
int cpu = __ffs(logical_apicid & 0xf);

- return cpumask_of_cpu(cpu + 4*node);
+ return physid_mask_of_physid(cpu + 4*node);
}

static inline int mpc_apic_id(struct mpc_config_processor *m,
diff -prauN mm1-2.5.74-1/include/asm-i386/mach-summit/mach_apic.h physid-2.5.74-1/include/asm-i386/mach-summit/mach_apic.h
--- mm1-2.5.74-1/include/asm-i386/mach-summit/mach_apic.h 2003-07-03 12:23:56.000000000 -0700
+++ physid-2.5.74-1/include/asm-i386/mach-summit/mach_apic.h 2003-07-04 02:47:00.000000000 -0700
@@ -28,8 +28,8 @@ static inline cpumask_t target_cpus(void
#define INT_DELIVERY_MODE (dest_Fixed)
#define INT_DEST_MODE 1 /* logical delivery broadcast to all procs */

-#define APIC_BROADCAST_ID (0x0F)
-static inline unsigned long check_apicid_used(cpumask_const_t bitmap, int apicid)
+#define APIC_BROADCAST_ID (0xFF)
+static inline unsigned long check_apicid_used(physid_mask_t bitmap, int apicid)
{
return 0;
}
@@ -88,15 +88,15 @@ static inline int cpu_present_to_apicid(
return (int) bios_cpu_apicid[mps_cpu];
}

-static inline cpumask_t ioapic_phys_id_map(cpumask_const_t phys_id_map)
+static inline physid_mask_t ioapic_phys_id_map(physid_mask_t phys_id_map)
{
/* For clustered we don't have a good way to do this yet - hack */
- return cpus_promote(0x0F);
+ return physids_promote(0x0F);
}

-static inline cpumask_t apicid_to_cpu_present(int apicid)
+static inline physid_mask_t apicid_to_cpu_present(int apicid)
{
- return cpumask_of_cpu(0);
+ return physid_mask_of_physid(0);
}

static inline int mpc_apic_id(struct mpc_config_processor *m,
diff -prauN mm1-2.5.74-1/include/asm-i386/mach-visws/mach_apic.h physid-2.5.74-1/include/asm-i386/mach-visws/mach_apic.h
--- mm1-2.5.74-1/include/asm-i386/mach-visws/mach_apic.h 2003-07-03 12:23:56.000000000 -0700
+++ physid-2.5.74-1/include/asm-i386/mach-visws/mach_apic.h 2003-07-04 02:45:17.000000000 -0700
@@ -16,12 +16,12 @@
#endif

#define APIC_BROADCAST_ID 0x0F
-#define check_apicid_used(bitmap, apicid) cpu_isset_const(apicid, bitmap)
-#define check_apicid_present(bit) cpu_isset(bit, phys_cpu_present_map)
+#define check_apicid_used(bitmap, apicid) physid_isset(apicid, bitmap)
+#define check_apicid_present(bit) physid_isset(bit, phys_cpu_present_map)

static inline int apic_id_registered(void)
{
- return cpu_isset(GET_APIC_ID(apic_read(APIC_ID)), phys_cpu_present_map);
+ return physid_isset(GET_APIC_ID(apic_read(APIC_ID)), phys_cpu_present_map);
}

/*
@@ -60,9 +60,9 @@ static inline int cpu_present_to_apicid(
return mps_cpu;
}

-static inline cpumask_t apicid_to_cpu_present(int apicid)
+static inline physid_mask_t apicid_to_cpu_present(int apicid)
{
- return cpumask_of_cpu(apicid);
+ return physid_mask_of_physid(apicid);
}

#define WAKE_SECONDARY_VIA_INIT
@@ -77,7 +77,7 @@ static inline void enable_apic_mode(void

static inline int check_phys_apicid_present(int boot_cpu_physical_apicid)
{
- return cpu_isset(boot_cpu_physical_apicid, phys_cpu_present_map);
+ return physid_isset(boot_cpu_physical_apicid, phys_cpu_present_map);
}

static inline unsigned int cpu_mask_to_apicid(cpumask_const_t cpumask)
diff -prauN mm1-2.5.74-1/include/asm-i386/mpspec.h physid-2.5.74-1/include/asm-i386/mpspec.h
--- mm1-2.5.74-1/include/asm-i386/mpspec.h 2003-07-03 12:23:56.000000000 -0700
+++ physid-2.5.74-1/include/asm-i386/mpspec.h 2003-07-04 02:45:17.000000000 -0700
@@ -12,7 +12,6 @@ extern int quad_local_to_mp_bus_id [NR_C
extern int mp_bus_id_to_pci_bus [MAX_MP_BUSSES];

extern unsigned int boot_cpu_physical_apicid;
-extern cpumask_t phys_cpu_present_map;
extern int smp_found_config;
extern void find_smp_config (void);
extern void get_smp_config (void);
@@ -42,5 +41,49 @@ extern void mp_config_ioapic_for_sci(int
extern void mp_parse_prt (void);
#endif /*CONFIG_ACPI_BOOT*/

+#define PHYSID_ARRAY_SIZE BITS_TO_LONGS(MAX_APICS)
+
+struct physid_mask
+{
+ unsigned long mask[PHYSID_ARRAY_SIZE];
+};
+
+typedef struct physid_mask physid_mask_t;
+
+#define physid_set(physid, map) set_bit(physid, (map).mask)
+#define physid_clear(physid, map) clear_bit(physid, (map).mask)
+#define physid_isset(physid, map) test_bit(physid, (map).mask)
+#define physid_test_and_set(physid, map) test_and_set_bit(physid, (map).mask)
+
+#define physids_and(dst, src1, src2) bitmap_and((dst).mask, (src1).mask, (src2).mask, MAX_APICS)
+#define physids_or(dst, src1, src2) bitmap_or((dst).mask, (src1).mask, (src2).mask, MAX_APICS)
+#define physids_clear(map) bitmap_clear((map).mask, MAX_APICS)
+#define physids_complement(map) bitmap_complement((map).mask, MAX_APICS)
+#define physids_empty(map) bitmap_empty((map).mask, MAX_APICS)
+#define physids_equal(map1, map2) bitmap_equal((map1).mask, (map2).mask, MAX_APICS)
+#define physids_weight(map) bitmap_weight((map).mask, MAX_APICS)
+#define physids_shift_right(d, s, n) bitmap_shift_right((d).mask, (s).mask, n, MAX_APICS)
+#define physids_shift_left(d, s, n) bitmap_shift_left((d).mask, (s).mask, n, MAX_APICS)
+#define physids_coerce(map) ((map).mask[0])
+
+#define physids_promote(physids) \
+ ({ \
+ physid_mask_t __physid_mask = PHYSID_MASK_NONE; \
+ __physid_mask.mask[0] = physids; \
+ __physid_mask; \
+ })
+
+#define physid_mask_of_physid(physid) \
+ ({ \
+ physid_mask_t __physid_mask = PHYSID_MASK_NONE; \
+ physid_set(physid, __physid_mask); \
+ __physid_mask; \
+ })
+
+#define PHYSID_MASK_ALL { {[0 ... PHYSID_ARRAY_SIZE-1] = ~0UL} }
+#define PHYSID_MASK_NONE { {[0 ... PHYSID_ARRAY_SIZE-1] = 0UL} }
+
+extern physid_mask_t phys_cpu_present_map;
+
#endif

diff -prauN mm1-2.5.74-1/include/asm-i386/smp.h physid-2.5.74-1/include/asm-i386/smp.h
--- mm1-2.5.74-1/include/asm-i386/smp.h 2003-07-03 12:23:56.000000000 -0700
+++ physid-2.5.74-1/include/asm-i386/smp.h 2003-07-04 02:45:17.000000000 -0700
@@ -32,7 +32,7 @@
*/

extern void smp_alloc_memory(void);
-extern cpumask_t phys_cpu_present_map;
+extern physid_mask_t phys_cpu_present_map;
extern int pic_mode;
extern int smp_num_siblings;
extern int cpu_sibling_map[];

2003-07-04 09:48:21

by William Lee Irwin III

[permalink] [raw]
Subject: Re: 2.5.74-mm1 fails to boot due to APIC trouble, 2.5.73mm3 works.

On Fri, Jul 04, 2003 at 02:50:04AM -0700, William Lee Irwin III wrote:
> This time diffed against the right tree:

And this time with a one-line typo fixed (it seemed to compile anyway):
s/CPU_MASK_NONE/PHYSID_MASK_NONE/ somewhere in io_apic.c where a physid
mask was being initialized.


diff -prauN mm1-2.5.74-1/arch/i386/kernel/apic.c physid-2.5.74-1/arch/i386/kernel/apic.c
--- mm1-2.5.74-1/arch/i386/kernel/apic.c 2003-07-03 12:23:55.000000000 -0700
+++ physid-2.5.74-1/arch/i386/kernel/apic.c 2003-07-04 02:45:17.000000000 -0700
@@ -1137,7 +1137,7 @@ int __init APIC_init_uniprocessor (void)

connect_bsp_APIC();

- phys_cpu_present_map = cpumask_of_cpu(boot_cpu_physical_apicid);
+ phys_cpu_present_map = physid_mask_of_physid(boot_cpu_physical_apicid);

setup_local_APIC();

diff -prauN mm1-2.5.74-1/arch/i386/kernel/io_apic.c physid-2.5.74-1/arch/i386/kernel/io_apic.c
--- mm1-2.5.74-1/arch/i386/kernel/io_apic.c 2003-07-03 12:23:55.000000000 -0700
+++ physid-2.5.74-1/arch/i386/kernel/io_apic.c 2003-07-04 02:53:32.000000000 -0700
@@ -1601,7 +1601,7 @@ void disable_IO_APIC(void)
static void __init setup_ioapic_ids_from_mpc(void)
{
union IO_APIC_reg_00 reg_00;
- cpumask_t phys_id_present_map;
+ physid_mask_t phys_id_present_map;
int apic;
int i;
unsigned char old_id;
@@ -1615,8 +1615,7 @@ static void __init setup_ioapic_ids_from
* This is broken; anything with a real cpu count has to
* circumvent this idiocy regardless.
*/
- phys_id_present_map =
- ioapic_phys_id_map(mk_cpumask_const(phys_cpu_present_map));
+ phys_id_present_map = ioapic_phys_id_map(phys_cpu_present_map);

/*
* Set the IOAPIC ID to the value stored in the MPC table.
@@ -1647,20 +1646,20 @@ static void __init setup_ioapic_ids_from
mp_ioapics[apic].mpc_apicid)) {
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))
+ for (i = 0; i < APIC_BROADCAST_ID; i++)
+ if (!physid_isset(i, phys_id_present_map))
break;
- if (i >= 0xf)
+ if (i >= APIC_BROADCAST_ID)
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 {
- cpumask_t tmp;
+ physid_mask_t tmp;
tmp = apicid_to_cpu_present(mp_ioapics[apic].mpc_apicid);
printk("Setting %d in the phys_id_present_map\n", mp_ioapics[apic].mpc_apicid);
- cpus_or(phys_id_present_map, phys_id_present_map, tmp);
+ physids_or(phys_id_present_map, phys_id_present_map, tmp);
}


@@ -2230,8 +2229,8 @@ late_initcall(io_apic_bug_finalize);
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 = CPU_MASK_NONE;
- cpumask_t tmp;
+ static physid_mask_t apic_id_map = PHYSID_MASK_NONE;
+ physid_mask_t tmp;
unsigned long flags;
int i = 0;

@@ -2244,8 +2243,8 @@ int __init io_apic_get_unique_id (int io
* advantage of new APIC bus architecture.
*/

- if (cpus_empty(apic_id_map))
- apic_id_map = ioapic_phys_id_map(mk_cpumask_const(phys_cpu_present_map));
+ if (physids_empty(apic_id_map))
+ apic_id_map = ioapic_phys_id_map(phys_cpu_present_map);

spin_lock_irqsave(&ioapic_lock, flags);
reg_00.raw = io_apic_read(ioapic, 0);
@@ -2278,7 +2277,7 @@ int __init io_apic_get_unique_id (int io
}

tmp = apicid_to_cpu_present(apic_id);
- cpus_or(apic_id_map, apic_id_map, tmp);
+ physids_or(apic_id_map, apic_id_map, tmp);

if (reg_00.bits.ID != apic_id) {
reg_00.bits.ID = apic_id;
diff -prauN mm1-2.5.74-1/arch/i386/kernel/mpparse.c physid-2.5.74-1/arch/i386/kernel/mpparse.c
--- mm1-2.5.74-1/arch/i386/kernel/mpparse.c 2003-07-03 12:23:55.000000000 -0700
+++ physid-2.5.74-1/arch/i386/kernel/mpparse.c 2003-07-04 02:45:17.000000000 -0700
@@ -71,7 +71,7 @@ unsigned int boot_cpu_logical_apicid = -
static unsigned int __initdata num_processors;

/* Bitmask of physically existing CPUs */
-cpumask_t phys_cpu_present_map;
+physid_mask_t phys_cpu_present_map;

u8 bios_cpu_apicid[NR_CPUS] = { [0 ... NR_CPUS-1] = BAD_APICID };

@@ -106,7 +106,7 @@ static struct mpc_config_translation *tr
void __init MP_processor_info (struct mpc_config_processor *m)
{
int ver, apicid;
- cpumask_t tmp;
+ physid_mask_t tmp;

if (!(m->mpc_cpuflag & CPU_ENABLED))
return;
@@ -178,7 +178,7 @@ void __init MP_processor_info (struct mp
ver = m->mpc_apicver;

tmp = apicid_to_cpu_present(apicid);
- cpus_or(phys_cpu_present_map, phys_cpu_present_map, tmp);
+ physids_or(phys_cpu_present_map, phys_cpu_present_map, tmp);

/*
* Validate version
diff -prauN mm1-2.5.74-1/arch/i386/kernel/smpboot.c physid-2.5.74-1/arch/i386/kernel/smpboot.c
--- mm1-2.5.74-1/arch/i386/kernel/smpboot.c 2003-07-03 12:23:55.000000000 -0700
+++ physid-2.5.74-1/arch/i386/kernel/smpboot.c 2003-07-04 02:45:17.000000000 -0700
@@ -957,7 +957,7 @@ static void __init smp_boot_cpus(unsigne
if (!smp_found_config) {
printk(KERN_NOTICE "SMP motherboard not detected.\n");
smpboot_clear_io_apic_irqs();
- 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");
@@ -984,7 +984,7 @@ static void __init smp_boot_cpus(unsigne
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 = cpumask_of_cpu(0);
+ phys_cpu_present_map = physid_mask_of_physid(0);
return;
}

@@ -997,7 +997,7 @@ static void __init smp_boot_cpus(unsigne
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 = cpumask_of_cpu(0);
+ phys_cpu_present_map = physid_mask_of_physid(0);
return;
}

@@ -1020,7 +1020,7 @@ static void __init smp_boot_cpus(unsigne
Dprintk("CPU present map: %lx\n", cpus_coerce(phys_cpu_present_map));

kicked = 1;
- for (bit = 0; kicked < NR_CPUS && bit < 8*sizeof(cpumask_t); bit++) {
+ for (bit = 0; kicked < NR_CPUS && bit < MAX_APICS; bit++) {
apicid = cpu_present_to_apicid(bit);
/*
* Don't even attempt to start the boot CPU!
diff -prauN mm1-2.5.74-1/include/asm-i386/genapic.h physid-2.5.74-1/include/asm-i386/genapic.h
--- mm1-2.5.74-1/include/asm-i386/genapic.h 2003-07-03 12:23:56.000000000 -0700
+++ physid-2.5.74-1/include/asm-i386/genapic.h 2003-07-04 02:48:52.000000000 -0700
@@ -27,18 +27,18 @@ struct genapic {
int int_dest_mode;
int apic_broadcast_id;
int esr_disable;
- unsigned long (*check_apicid_used)(cpumask_const_t bitmap, int apicid);
+ unsigned long (*check_apicid_used)(physid_mask_t bitmap, int apicid);
unsigned long (*check_apicid_present)(int apicid);
int no_balance_irq;
void (*init_apic_ldr)(void);
- cpumask_t (*ioapic_phys_id_map)(cpumask_const_t map);
+ physid_mask_t (*ioapic_phys_id_map)(physid_mask_t map);

void (*clustered_apic_check)(void);
int (*multi_timer_check)(int apic, int irq);
int (*apicid_to_node)(int logical_apicid);
int (*cpu_to_logical_apicid)(int cpu);
int (*cpu_present_to_apicid)(int mps_cpu);
- cpumask_t (*apicid_to_cpu_present)(int phys_apicid);
+ physid_mask_t (*apicid_to_cpu_present)(int phys_apicid);
int (*mpc_apic_id)(struct mpc_config_processor *m,
struct mpc_config_translation *t);
void (*setup_portio_remap)(void);
diff -prauN mm1-2.5.74-1/include/asm-i386/mach-bigsmp/mach_apic.h physid-2.5.74-1/include/asm-i386/mach-bigsmp/mach_apic.h
--- mm1-2.5.74-1/include/asm-i386/mach-bigsmp/mach_apic.h 2003-07-03 12:23:56.000000000 -0700
+++ physid-2.5.74-1/include/asm-i386/mach-bigsmp/mach_apic.h 2003-07-04 02:47:45.000000000 -0700
@@ -29,15 +29,15 @@ static inline cpumask_t target_cpus(void
#define INT_DELIVERY_MODE dest_LowestPrio
#define INT_DEST_MODE 1 /* logical delivery broadcast to all procs */

-#define APIC_BROADCAST_ID (0x0f)
-static inline unsigned long check_apicid_used(cpumask_const_t bitmap, int apicid)
+#define APIC_BROADCAST_ID (0xff)
+static inline unsigned long check_apicid_used(physid_mask_t bitmap, int apicid)
{
return 0;
}

static inline unsigned long check_apicid_present(int bit)
{
- return cpu_isset(bit, phys_cpu_present_map);
+ return physid_isset(bit, phys_cpu_present_map);
}

#define apicid_cluster(apicid) (apicid & 0xF0)
@@ -89,9 +89,9 @@ static inline int cpu_present_to_apicid(
return (int) bios_cpu_apicid[mps_cpu];
}

-static inline cpumask_t apicid_to_cpu_present(int phys_apicid)
+static inline physid_mask_t apicid_to_cpu_present(int phys_apicid)
{
- return cpumask_of_cpu(phys_apicid);
+ return physid_mask_of_physid(phys_apicid);
}

extern volatile u8 cpu_2_logical_apicid[];
@@ -112,10 +112,10 @@ static inline int mpc_apic_id(struct mpc
return m->mpc_apicid;
}

-static inline cpumask_t ioapic_phys_id_map(cpumask_const_t phys_map)
+static inline physid_mask_t ioapic_phys_id_map(physid_mask_t phys_map)
{
/* For clustered we don't have a good way to do this yet - hack */
- return cpus_promote(0xFUL);
+ return physids_promote(0xFUL);
}

#define WAKE_SECONDARY_VIA_INIT
diff -prauN mm1-2.5.74-1/include/asm-i386/mach-default/mach_apic.h physid-2.5.74-1/include/asm-i386/mach-default/mach_apic.h
--- mm1-2.5.74-1/include/asm-i386/mach-default/mach_apic.h 2003-07-03 12:23:56.000000000 -0700
+++ physid-2.5.74-1/include/asm-i386/mach-default/mach_apic.h 2003-07-04 02:45:17.000000000 -0700
@@ -21,16 +21,20 @@ static inline cpumask_t target_cpus(void
#define INT_DELIVERY_MODE dest_LowestPrio
#define INT_DEST_MODE 1 /* logical delivery broadcast to all procs */

+/*
+ * this isn't really broadcast, just a (potentially inaccurate) upper
+ * bound for valid physical APIC id's
+ */
#define APIC_BROADCAST_ID 0x0F

-static inline unsigned long check_apicid_used(cpumask_const_t bitmap, int apicid)
+static inline unsigned long check_apicid_used(physid_mask_t bitmap, int apicid)
{
- return cpu_isset_const(apicid, bitmap);
+ return physid_isset(apicid, bitmap);
}

static inline unsigned long check_apicid_present(int bit)
{
- return cpu_isset(bit, phys_cpu_present_map);
+ return physid_isset(bit, phys_cpu_present_map);
}

/*
@@ -50,11 +54,9 @@ static inline void init_apic_ldr(void)
apic_write_around(APIC_LDR, val);
}

-static inline cpumask_t ioapic_phys_id_map(cpumask_const_t phys_map)
+static inline physid_mask_t ioapic_phys_id_map(physid_mask_t phys_map)
{
- cpumask_t ret;
- cpus_copy_const(ret, phys_map);
- return ret;
+ return phys_map;
}

static inline void clustered_apic_check(void)
@@ -84,9 +86,9 @@ static inline int cpu_present_to_apicid(
return mps_cpu;
}

-static inline cpumask_t apicid_to_cpu_present(int phys_apicid)
+static inline physid_mask_t apicid_to_cpu_present(int phys_apicid)
{
- return cpumask_of_cpu(phys_apicid);
+ return physid_mask_of_physid(phys_apicid);
}

static inline int mpc_apic_id(struct mpc_config_processor *m,
@@ -106,12 +108,12 @@ static inline void setup_portio_remap(vo

static inline int check_phys_apicid_present(int boot_cpu_physical_apicid)
{
- return cpu_isset(boot_cpu_physical_apicid, phys_cpu_present_map);
+ return physid_isset(boot_cpu_physical_apicid, phys_cpu_present_map);
}

static inline int apic_id_registered(void)
{
- return cpu_isset(GET_APIC_ID(apic_read(APIC_ID)), phys_cpu_present_map);
+ return physid_isset(GET_APIC_ID(apic_read(APIC_ID)), phys_cpu_present_map);
}

static inline unsigned int cpu_mask_to_apicid(cpumask_const_t cpumask)
diff -prauN mm1-2.5.74-1/include/asm-i386/mach-es7000/mach_apic.h physid-2.5.74-1/include/asm-i386/mach-es7000/mach_apic.h
--- mm1-2.5.74-1/include/asm-i386/mach-es7000/mach_apic.h 2003-07-03 12:23:56.000000000 -0700
+++ physid-2.5.74-1/include/asm-i386/mach-es7000/mach_apic.h 2003-07-04 02:46:36.000000000 -0700
@@ -40,13 +40,13 @@ static inline cpumask_t target_cpus(void

#define APIC_BROADCAST_ID (0xff)

-static inline unsigned long check_apicid_used(cpumask_const_t bitmap, int apicid)
+static inline unsigned long check_apicid_used(physid_mask_t bitmap, int apicid)
{
return 0;
}
static inline unsigned long check_apicid_present(int bit)
{
- return cpu_isset(bit, phys_cpu_present_map);
+ return physid_isset(bit, phys_cpu_present_map);
}

#define apicid_cluster(apicid) (apicid & 0xF0)
@@ -110,12 +110,12 @@ static inline int cpu_present_to_apicid(
return (int) bios_cpu_apicid[mps_cpu];
}

-static inline cpumask_t apicid_to_cpu_present(int phys_apicid)
+static inline physid_mask_t apicid_to_cpu_present(int phys_apicid)
{
- static int cpu = 0;
- cpumask_t mask;
- mask = cpumask_of_cpu(cpu);
- ++cpu;
+ static int id = 0;
+ physid_mask_t mask;
+ mask = physid_mask_of_physid(id);
+ ++id;
return mask;
}

@@ -136,10 +136,10 @@ static inline int mpc_apic_id(struct mpc
return (m->mpc_apicid);
}

-static inline cpumask_t ioapic_phys_id_map(cpumask_const_t phys_map)
+static inline physid_mask_t ioapic_phys_id_map(physid_mask_t phys_map)
{
/* For clustered we don't have a good way to do this yet - hack */
- return cpus_promote(0xff);
+ return physids_promote(0xff);
}


diff -prauN mm1-2.5.74-1/include/asm-i386/mach-numaq/mach_apic.h physid-2.5.74-1/include/asm-i386/mach-numaq/mach_apic.h
--- mm1-2.5.74-1/include/asm-i386/mach-numaq/mach_apic.h 2003-07-03 12:23:56.000000000 -0700
+++ physid-2.5.74-1/include/asm-i386/mach-numaq/mach_apic.h 2003-07-04 02:45:17.000000000 -0700
@@ -21,8 +21,8 @@ static inline cpumask_t target_cpus(void
#define INT_DEST_MODE 0 /* physical delivery on LOCAL quad */

#define APIC_BROADCAST_ID 0x0F
-#define check_apicid_used(bitmap, apicid) cpu_isset_const(apicid, bitmap)
-#define check_apicid_present(bit) cpu_isset(bit, phys_cpu_present_map)
+#define check_apicid_used(bitmap, apicid) physid_isset(apicid, bitmap)
+#define check_apicid_present(bit) physid_isset(bit, phys_cpu_present_map)
#define apicid_cluster(apicid) (apicid & 0xF0)

static inline int apic_id_registered(void)
@@ -50,10 +50,10 @@ static inline int multi_timer_check(int
return apic != 0 && irq == 0;
}

-static inline cpumask_t ioapic_phys_id_map(cpumask_const_t phys_map)
+static inline physid_mask_t ioapic_phys_id_map(physid_mask_t phys_map)
{
/* We don't have a good way to do this yet - hack */
- return cpus_promote(0xFUL);
+ return physids_promote(0xFUL);
}

/* Mapping from cpu number to logical apicid */
@@ -78,12 +78,12 @@ static inline int apicid_to_node(int log
return logical_apicid >> 4;
}

-static inline cpumask_t apicid_to_cpu_present(int logical_apicid)
+static inline physid_mask_t apicid_to_cpu_present(int logical_apicid)
{
int node = apicid_to_node(logical_apicid);
int cpu = __ffs(logical_apicid & 0xf);

- return cpumask_of_cpu(cpu + 4*node);
+ return physid_mask_of_physid(cpu + 4*node);
}

static inline int mpc_apic_id(struct mpc_config_processor *m,
diff -prauN mm1-2.5.74-1/include/asm-i386/mach-summit/mach_apic.h physid-2.5.74-1/include/asm-i386/mach-summit/mach_apic.h
--- mm1-2.5.74-1/include/asm-i386/mach-summit/mach_apic.h 2003-07-03 12:23:56.000000000 -0700
+++ physid-2.5.74-1/include/asm-i386/mach-summit/mach_apic.h 2003-07-04 02:47:00.000000000 -0700
@@ -28,8 +28,8 @@ static inline cpumask_t target_cpus(void
#define INT_DELIVERY_MODE (dest_Fixed)
#define INT_DEST_MODE 1 /* logical delivery broadcast to all procs */

-#define APIC_BROADCAST_ID (0x0F)
-static inline unsigned long check_apicid_used(cpumask_const_t bitmap, int apicid)
+#define APIC_BROADCAST_ID (0xFF)
+static inline unsigned long check_apicid_used(physid_mask_t bitmap, int apicid)
{
return 0;
}
@@ -88,15 +88,15 @@ static inline int cpu_present_to_apicid(
return (int) bios_cpu_apicid[mps_cpu];
}

-static inline cpumask_t ioapic_phys_id_map(cpumask_const_t phys_id_map)
+static inline physid_mask_t ioapic_phys_id_map(physid_mask_t phys_id_map)
{
/* For clustered we don't have a good way to do this yet - hack */
- return cpus_promote(0x0F);
+ return physids_promote(0x0F);
}

-static inline cpumask_t apicid_to_cpu_present(int apicid)
+static inline physid_mask_t apicid_to_cpu_present(int apicid)
{
- return cpumask_of_cpu(0);
+ return physid_mask_of_physid(0);
}

static inline int mpc_apic_id(struct mpc_config_processor *m,
diff -prauN mm1-2.5.74-1/include/asm-i386/mach-visws/mach_apic.h physid-2.5.74-1/include/asm-i386/mach-visws/mach_apic.h
--- mm1-2.5.74-1/include/asm-i386/mach-visws/mach_apic.h 2003-07-03 12:23:56.000000000 -0700
+++ physid-2.5.74-1/include/asm-i386/mach-visws/mach_apic.h 2003-07-04 02:45:17.000000000 -0700
@@ -16,12 +16,12 @@
#endif

#define APIC_BROADCAST_ID 0x0F
-#define check_apicid_used(bitmap, apicid) cpu_isset_const(apicid, bitmap)
-#define check_apicid_present(bit) cpu_isset(bit, phys_cpu_present_map)
+#define check_apicid_used(bitmap, apicid) physid_isset(apicid, bitmap)
+#define check_apicid_present(bit) physid_isset(bit, phys_cpu_present_map)

static inline int apic_id_registered(void)
{
- return cpu_isset(GET_APIC_ID(apic_read(APIC_ID)), phys_cpu_present_map);
+ return physid_isset(GET_APIC_ID(apic_read(APIC_ID)), phys_cpu_present_map);
}

/*
@@ -60,9 +60,9 @@ static inline int cpu_present_to_apicid(
return mps_cpu;
}

-static inline cpumask_t apicid_to_cpu_present(int apicid)
+static inline physid_mask_t apicid_to_cpu_present(int apicid)
{
- return cpumask_of_cpu(apicid);
+ return physid_mask_of_physid(apicid);
}

#define WAKE_SECONDARY_VIA_INIT
@@ -77,7 +77,7 @@ static inline void enable_apic_mode(void

static inline int check_phys_apicid_present(int boot_cpu_physical_apicid)
{
- return cpu_isset(boot_cpu_physical_apicid, phys_cpu_present_map);
+ return physid_isset(boot_cpu_physical_apicid, phys_cpu_present_map);
}

static inline unsigned int cpu_mask_to_apicid(cpumask_const_t cpumask)
diff -prauN mm1-2.5.74-1/include/asm-i386/mpspec.h physid-2.5.74-1/include/asm-i386/mpspec.h
--- mm1-2.5.74-1/include/asm-i386/mpspec.h 2003-07-03 12:23:56.000000000 -0700
+++ physid-2.5.74-1/include/asm-i386/mpspec.h 2003-07-04 02:45:17.000000000 -0700
@@ -12,7 +12,6 @@ extern int quad_local_to_mp_bus_id [NR_C
extern int mp_bus_id_to_pci_bus [MAX_MP_BUSSES];

extern unsigned int boot_cpu_physical_apicid;
-extern cpumask_t phys_cpu_present_map;
extern int smp_found_config;
extern void find_smp_config (void);
extern void get_smp_config (void);
@@ -42,5 +41,49 @@ extern void mp_config_ioapic_for_sci(int
extern void mp_parse_prt (void);
#endif /*CONFIG_ACPI_BOOT*/

+#define PHYSID_ARRAY_SIZE BITS_TO_LONGS(MAX_APICS)
+
+struct physid_mask
+{
+ unsigned long mask[PHYSID_ARRAY_SIZE];
+};
+
+typedef struct physid_mask physid_mask_t;
+
+#define physid_set(physid, map) set_bit(physid, (map).mask)
+#define physid_clear(physid, map) clear_bit(physid, (map).mask)
+#define physid_isset(physid, map) test_bit(physid, (map).mask)
+#define physid_test_and_set(physid, map) test_and_set_bit(physid, (map).mask)
+
+#define physids_and(dst, src1, src2) bitmap_and((dst).mask, (src1).mask, (src2).mask, MAX_APICS)
+#define physids_or(dst, src1, src2) bitmap_or((dst).mask, (src1).mask, (src2).mask, MAX_APICS)
+#define physids_clear(map) bitmap_clear((map).mask, MAX_APICS)
+#define physids_complement(map) bitmap_complement((map).mask, MAX_APICS)
+#define physids_empty(map) bitmap_empty((map).mask, MAX_APICS)
+#define physids_equal(map1, map2) bitmap_equal((map1).mask, (map2).mask, MAX_APICS)
+#define physids_weight(map) bitmap_weight((map).mask, MAX_APICS)
+#define physids_shift_right(d, s, n) bitmap_shift_right((d).mask, (s).mask, n, MAX_APICS)
+#define physids_shift_left(d, s, n) bitmap_shift_left((d).mask, (s).mask, n, MAX_APICS)
+#define physids_coerce(map) ((map).mask[0])
+
+#define physids_promote(physids) \
+ ({ \
+ physid_mask_t __physid_mask = PHYSID_MASK_NONE; \
+ __physid_mask.mask[0] = physids; \
+ __physid_mask; \
+ })
+
+#define physid_mask_of_physid(physid) \
+ ({ \
+ physid_mask_t __physid_mask = PHYSID_MASK_NONE; \
+ physid_set(physid, __physid_mask); \
+ __physid_mask; \
+ })
+
+#define PHYSID_MASK_ALL { {[0 ... PHYSID_ARRAY_SIZE-1] = ~0UL} }
+#define PHYSID_MASK_NONE { {[0 ... PHYSID_ARRAY_SIZE-1] = 0UL} }
+
+extern physid_mask_t phys_cpu_present_map;
+
#endif

diff -prauN mm1-2.5.74-1/include/asm-i386/smp.h physid-2.5.74-1/include/asm-i386/smp.h
--- mm1-2.5.74-1/include/asm-i386/smp.h 2003-07-03 12:23:56.000000000 -0700
+++ physid-2.5.74-1/include/asm-i386/smp.h 2003-07-04 02:45:17.000000000 -0700
@@ -32,7 +32,7 @@
*/

extern void smp_alloc_memory(void);
-extern cpumask_t phys_cpu_present_map;
+extern physid_mask_t phys_cpu_present_map;
extern int pic_mode;
extern int smp_num_siblings;
extern int cpu_sibling_map[];

2003-07-04 09:53:32

by William Lee Irwin III

[permalink] [raw]
Subject: Re: 2.5.74-mm1 fails to boot due to APIC trouble, 2.5.73mm3 works.

On Fri, Jul 04, 2003 at 02:50:04AM -0700, William Lee Irwin III wrote:
>> This time diffed against the right tree:

On Fri, Jul 04, 2003 at 03:02:17AM -0700, William Lee Irwin III wrote:
> And this time with a one-line typo fixed (it seemed to compile anyway):
> s/CPU_MASK_NONE/PHYSID_MASK_NONE/ somewhere in io_apic.c where a physid
> mask was being initialized.

Unrelated tangent:

Nuke cpumask_arith.h; it's unused as of the requested updates to use
strong typing for all arches.


-- wli


diff -prauN physid-2.5.74-1/include/asm-generic/cpumask_arith.h physid-2.5.74-2/include/asm-generic/cpumask_arith.h
--- physid-2.5.74-1/include/asm-generic/cpumask_arith.h 2003-07-03 12:23:56.000000000 -0700
+++ physid-2.5.74-2/include/asm-generic/cpumask_arith.h 1969-12-31 16:00:00.000000000 -0800
@@ -1,61 +0,0 @@
-#ifndef __ASM_GENERIC_CPUMASK_ARITH_H
-#define __ASM_GENERIC_CPUMASK_ARITH_H
-
-#define cpu_set(cpu, map) \
- do { \
- map |= ((cpumask_t)1) << (cpu); \
- } while (0)
-#define cpu_clear(cpu, map) \
- do { \
- map &= ~(((cpumask_t)1) << (cpu)); \
- } while (0)
-#define cpu_isset(cpu, map) \
- ((map) & (((cpumask_t)1) << (cpu)))
-#define cpu_test_and_set(cpu, map) \
- test_and_set_bit(cpu, (unsigned long *)(&(map)))
-
-#define cpus_and(dst,src1,src2) do { dst = (src1) & (src2); } while (0)
-#define cpus_or(dst,src1,src2) do { dst = (src1) | (src2); } while (0)
-#define cpus_clear(map) do { map = 0; } while (0)
-#define cpus_complement(map) do { map = ~(map); } while (0)
-#define cpus_equal(map1, map2) ((map1) == (map2))
-#define cpus_empty(map) ((map) == 0)
-
-#if BITS_PER_LONG == 32
-#if NR_CPUS <= 32
-#define cpus_weight(map) hweight32(map)
-#else
-#define cpus_weight(map) \
-({ \
- u32 *__map = (u32 *)(&(map)); \
- hweight32(__map[0]) + hweight32(__map[1]); \
-})
-#endif
-#elif BITS_PER_LONG == 64
-#define cpus_weight(map) hweight64(map)
-#endif
-
-#define cpus_shift_right(dst, src, n) do { dst = (src) >> (n); } while (0)
-#define cpus_shift_left(dst, src, n) do { dst = (src) >> (n); } while (0)
-
-#define any_online_cpu(map) (!cpus_empty(map))
-
-
-#define CPU_MASK_ALL (~((cpumask_t)0) >> (8*sizeof(cpumask_t) - NR_CPUS))
-#define CPU_MASK_NONE ((cpumask_t)0)
-
-/* only ever use this for things that are _never_ used on large boxen */
-#define cpus_coerce(map) ((unsigned long)(map))
-#define cpus_promote(map) ({ map; })
-#define cpumask_of_cpu(cpu) ({ ((cpumask_t)1) << (cpu); })
-
-#ifdef CONFIG_SMP
-#define first_cpu(map) __ffs(map)
-#define next_cpu(cpu, map) \
- __ffs((map) & ~(((cpumask_t)1 << (cpu)) - 1))
-#else
-#define first_cpu(map) 0
-#define next_cpu(cpu, map) 1
-#endif /* CONFIG_SMP */
-
-#endif /* __ASM_GENERIC_CPUMASK_ARITH_H */

2003-07-04 15:27:50

by Martin J. Bligh

[permalink] [raw]
Subject: Re: 2.5.74-mm1 fails to boot due to APIC trouble, 2.5.73mm3 works.

> On Fri, Jul 04, 2003 at 02:35:31AM -0700, William Lee Irwin III wrote:
>> Okay, now for the "final solution" wrt. sparse physical APIC ID's
>> in addition to what I hope is a fix for your bug. This uses a separate
>> bitmap type (of a NR_CPUS -independent width MAX_APICS) for physical
>> APIC ID bitmaps.
>> \begin{cross-fingers}

Is it really necessary to turn half the apic code upside down in order
to fix this? What's the actual bugfix that's buried in this cleanup?

Despite the fact you seem to have gone out of your way to make this
hard to review, there are a few things I can see that strike me as odd.
Not necessarily wrong, but requiring more explanation.

> - if (i >= 0xf)
> + if (i >= APIC_BROADCAST_ID)

Is that always correct? it's not equivalent.

> - for (bit = 0; kicked < NR_CPUS && bit < 8*sizeof(cpumask_t); bit++) {
> + for (bit = 0; kicked < NR_CPUS && bit < MAX_APICS; bit++) {

Is that the actual one-line bugfix this is all about?

> diff -prauN mm1-2.5.74-1/include/asm-i386/mach-bigsmp/mach_apic.h physid-2.5.74-1/include/asm-i386/mach-bigsmp/mach_apic.h
> --- mm1-2.5.74-1/include/asm-i386/mach-bigsmp/mach_apic.h 2003-07-03 12:23:56.000000000 -0700
> +++ physid-2.5.74-1/include/asm-i386/mach-bigsmp/mach_apic.h 2003-07-04 02:47:45.000000000 -0700
> @@ -29,15 +29,15 @@ static inline cpumask_t target_cpus(void
> #define INT_DELIVERY_MODE dest_LowestPrio
> #define INT_DEST_MODE 1 /* logical delivery broadcast to all procs */
>
> -#define APIC_BROADCAST_ID (0x0f)
> +#define APIC_BROADCAST_ID (0xff)

So ... you've tested that change on a bigsmp machine, right?
At least, provide some reasoning here. Like this comment further down the
patch ...

> +/*
> + * this isn't really broadcast, just a (potentially inaccurate) upper
> + * bound for valid physical APIC id's
> + */

Which makes the change just look wrong to me. If you're thinking
"physical clustered mode" that terminology just utterly confusing crap,
and the change is wrong, as far as I can see.

> +++ physid-2.5.74-1/include/asm-i386/mach-numaq/mach_apic.h
> 2003-07-04 02:45:17.000000000 -0700
>
> -static inline cpumask_t apicid_to_cpu_present(int logical_apicid)
> +static inline physid_mask_t apicid_to_cpu_present(int logical_apicid)
> {
> int node = apicid_to_node(logical_apicid);
> int cpu = __ffs(logical_apicid & 0xf);
>
> - return cpumask_of_cpu(cpu + 4*node);
> + return physid_mask_of_physid(cpu + 4*node);
> }

Hmmmm. What are you using physical apicids here for? They seem
irrelevant to this function.

M.

2003-07-04 15:43:51

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: 2.5.74-mm1 fails to boot due to APIC trouble, 2.5.73mm3 works.

On Fri, 4 Jul 2003, Martin J. Bligh wrote:

> Is it really necessary to turn half the apic code upside down in order
> to fix this? What's the actual bugfix that's buried in this cleanup?

The way i see it is that you can't use NR_CPUS to determine the upper
bound on APIC IDs. e.g. my 3way is normally configured with NR_CPUS = 3
but has APIC IDs of 0, 3 and 4. We need to make a distinction.

> Despite the fact you seem to have gone out of your way to make this
> hard to review, there are a few things I can see that strike me as odd.
> Not necessarily wrong, but requiring more explanation.
>
> > - if (i >= 0xf)
> > + if (i >= APIC_BROADCAST_ID)
>
> Is that always correct? it's not equivalent.

Well we really want APIC_MAX_ID (or whatever it's called)

> > - for (bit = 0; kicked < NR_CPUS && bit < 8*sizeof(cpumask_t); bit++) {
> > + for (bit = 0; kicked < NR_CPUS && bit < MAX_APICS; bit++) {
>
> Is that the actual one-line bugfix this is all about?

No, the problem is no space for physical ids in cpumask bitmaps, this
could manifest itself later on unless we fix it now.

> > -#define APIC_BROADCAST_ID (0x0f)
> > +#define APIC_BROADCAST_ID (0xff)
>
> So ... you've tested that change on a bigsmp machine, right?
> At least, provide some reasoning here. Like this comment further down the
> patch ...

That one is slightly worrying, yes.

> > + * this isn't really broadcast, just a (potentially inaccurate) upper
> > + * bound for valid physical APIC id's
>
> Which makes the change just look wrong to me. If you're thinking
> "physical clustered mode" that terminology just utterly confusing crap,
> and the change is wrong, as far as I can see.
>
> > +++ physid-2.5.74-1/include/asm-i386/mach-numaq/mach_apic.h
> > 2003-07-04 02:45:17.000000000 -0700
> >
> > -static inline cpumask_t apicid_to_cpu_present(int logical_apicid)
> > +static inline physid_mask_t apicid_to_cpu_present(int logical_apicid)
> > {
> > int node = apicid_to_node(logical_apicid);
> > int cpu = __ffs(logical_apicid & 0xf);
> >
> > - return cpumask_of_cpu(cpu + 4*node);
> > + return physid_mask_of_physid(cpu + 4*node);
> > }
>
> Hmmmm. What are you using physical apicids here for? They seem
> irrelevant to this function.

Urgh, it's really hard to determine what these functions really want half
the time. But that change does look wrong.

Zwane
--
function.linuxpower.ca

2003-07-04 16:04:24

by Martin J. Bligh

[permalink] [raw]
Subject: Re: 2.5.74-mm1 fails to boot due to APIC trouble, 2.5.73mm3 works.

> On Fri, 4 Jul 2003, Martin J. Bligh wrote:
>
>> Is it really necessary to turn half the apic code upside down in order
>> to fix this? What's the actual bugfix that's buried in this cleanup?
>
> The way i see it is that you can't use NR_CPUS to determine the upper
> bound on APIC IDs. e.g. my 3way is normally configured with NR_CPUS = 3
> but has APIC IDs of 0, 3 and 4. We need to make a distinction.

Fair enough. But that would seem to be a simpler operation than this patch.

>> > - if (i >= 0xf)
>> > + if (i >= APIC_BROADCAST_ID)
>>
>> Is that always correct? it's not equivalent.
>
> Well we really want APIC_MAX_ID (or whatever it's called)

Indeed. maybe MAX_PHYS_APIC_ID or something (it's different for logical).
We break it out in subarch, but it's the same everywhere, which seems
utterly useless - is probably historical cruft that needs to die.
But that sounds like a separate issue, and a separate patch to me.

>> > - for (bit = 0; kicked < NR_CPUS && bit < 8*sizeof(cpumask_t); bit++) {
>> > + for (bit = 0; kicked < NR_CPUS && bit < MAX_APICS; bit++) {
>>
>> Is that the actual one-line bugfix this is all about?
>
> No, the problem is no space for physical ids in cpumask bitmaps, this
> could manifest itself later on unless we fix it now.

Ugh, are you saying the cpumask stuff shrinks masks to < 32 bits if
NR_CPUS is low enough? If so, I can see more point to the patch, but
it still seems like violent overkill. Stopping it doing that would
probably fix it ... I can't imagine it buys you much.

phys_cpu_present_map started off as an unsigned long, and I reused it
in a fairly twisted way for NUMA-Q. As it's an array that's bounded
by apic space, using the bios_cpu_apicid method that summit uses
would be a much cleaner fix, and just leave the old one as a long
bitmask like it used to be - which is fine for non- clustered apic
systems, and saves inventing a whole new data type. See the
cpu_present_to_apicid abstraction.

>> Hmmmm. What are you using physical apicids here for? They seem
>> irrelevant to this function.
>
> Urgh, it's really hard to determine what these functions really want half
> the time. But that change does look wrong.

Yeah, things taking logical apicids, and turning them into cpu numbers
presumably shouldn't have to touch that.

M.

2003-07-04 16:13:26

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: 2.5.74-mm1 fails to boot due to APIC trouble, 2.5.73mm3 works.

On Fri, 4 Jul 2003, Martin J. Bligh wrote:

> > No, the problem is no space for physical ids in cpumask bitmaps, this
> > could manifest itself later on unless we fix it now.
>
> Ugh, are you saying the cpumask stuff shrinks masks to < 32 bits if
> NR_CPUS is low enough? If so, I can see more point to the patch, but
> it still seems like violent overkill. Stopping it doing that would
> probably fix it ... I can't imagine it buys you much.

Hmm i hope not, Bill can you verify that? Looking at the source it doesn't
appear to be so;

#define BITS_TO_LONGS(bits) \
(((bits)+BITS_PER_LONG-1)/BITS_PER_LONG)
#define DECLARE_BITMAP(name,bits) \
unsigned long name[BITS_TO_LONGS(bits)]

> phys_cpu_present_map started off as an unsigned long, and I reused it
> in a fairly twisted way for NUMA-Q. As it's an array that's bounded
> by apic space, using the bios_cpu_apicid method that summit uses
> would be a much cleaner fix, and just leave the old one as a long
> bitmask like it used to be - which is fine for non- clustered apic
> systems, and saves inventing a whole new data type. See the
> cpu_present_to_apicid abstraction.

Thanks i'll have a look.

--
function.linuxpower.ca

2003-07-04 18:10:37

by William Lee Irwin III

[permalink] [raw]
Subject: Re: 2.5.74-mm1 fails to boot due to APIC trouble, 2.5.73mm3 works.

On Fri, Jul 04, 2003 at 02:35:31AM -0700, William Lee Irwin III wrote:
>>> Okay, now for the "final solution" wrt. sparse physical APIC ID's
>>> in addition to what I hope is a fix for your bug. This uses a separate
>>> bitmap type (of a NR_CPUS -independent width MAX_APICS) for physical
>>> APIC ID bitmaps.
>>> \begin{cross-fingers}

On Fri, Jul 04, 2003 at 08:41:38AM -0700, Martin J. Bligh wrote:
> Is it really necessary to turn half the apic code upside down in order
> to fix this? What's the actual bugfix that's buried in this cleanup?
> Despite the fact you seem to have gone out of your way to make this
> hard to review, there are a few things I can see that strike me as odd.
> Not necessarily wrong, but requiring more explanation.

It's not a cleanup, and it doesn't touch trailing whitespace etc.


On Fri, Jul 04, 2003 at 02:35:31AM -0700, William Lee Irwin III wrote:
>> - if (i >= 0xf)
>> + if (i >= APIC_BROADCAST_ID)

On Fri, Jul 04, 2003 at 08:41:38AM -0700, Martin J. Bligh wrote:
> Is that always correct? it's not equivalent.

It is.


On Fri, Jul 04, 2003 at 02:35:31AM -0700, William Lee Irwin III wrote:
>> - for (bit = 0; kicked < NR_CPUS && bit < 8*sizeof(cpumask_t); bit++) {
>> + for (bit = 0; kicked < NR_CPUS && bit < MAX_APICS; bit++) {

On Fri, Jul 04, 2003 at 08:41:38AM -0700, Martin J. Bligh wrote:
> Is that the actual one-line bugfix this is all about?

No.

On Fri, Jul 04, 2003 at 02:35:31AM -0700, William Lee Irwin III wrote:
>> diff -prauN mm1-2.5.74-1/include/asm-i386/mach-bigsmp/mach_apic.h physid-2.5.74-1/include/asm-i386/mach-bigsmp/mach_apic.h
>> --- mm1-2.5.74-1/include/asm-i386/mach-bigsmp/mach_apic.h 2003-07-03 12:23:56.000000000 -0700
>> +++ physid-2.5.74-1/include/asm-i386/mach-bigsmp/mach_apic.h 2003-07-04 02:47:45.000000000 -0700
>> @@ -29,15 +29,15 @@ static inline cpumask_t target_cpus(void
>> #define INT_DELIVERY_MODE dest_LowestPrio
>> #define INT_DEST_MODE 1 /* logical delivery broadcast to all procs */
>> -#define APIC_BROADCAST_ID (0x0f)
>> +#define APIC_BROADCAST_ID (0xff)

On Fri, Jul 04, 2003 at 08:41:38AM -0700, Martin J. Bligh wrote:
> So ... you've tested that change on a bigsmp machine, right?
> At least, provide some reasoning here. Like this comment further down the
> patch ...

APIC_BROADCAST_ID is an upper bound on valid physical APIC ID's as it
is used in the code. That actually was commented in the patch.


On Fri, Jul 04, 2003 at 02:35:31AM -0700, William Lee Irwin III wrote:
>> +/*
>> + * this isn't really broadcast, just a (potentially inaccurate) upper
>> + * bound for valid physical APIC id's
>> + */

On Fri, Jul 04, 2003 at 08:41:38AM -0700, Martin J. Bligh wrote:
> Which makes the change just look wrong to me. If you're thinking
> "physical clustered mode" that terminology just utterly confusing crap,
> and the change is wrong, as far as I can see.

The change is correct, and I am not thinking of any such thing.
APIC_BROADCAST_ID's sole usage is for terminating loops over physical
APIC ID's while setting the physical APIC ID's of IO-APIC's.


On Fri, Jul 04, 2003 at 02:35:31AM -0700, William Lee Irwin III wrote:
>> +++ physid-2.5.74-1/include/asm-i386/mach-numaq/mach_apic.h
>> 2003-07-04 02:45:17.000000000 -0700
>>
>> -static inline cpumask_t apicid_to_cpu_present(int logical_apicid)
>> +static inline physid_mask_t apicid_to_cpu_present(int logical_apicid)
>> {
>> int node = apicid_to_node(logical_apicid);
>> int cpu = __ffs(logical_apicid & 0xf);
>>
>> - return cpumask_of_cpu(cpu + 4*node);
>> + return physid_mask_of_physid(cpu + 4*node);
>> }

On Fri, Jul 04, 2003 at 08:41:38AM -0700, Martin J. Bligh wrote:
> Hmmmm. What are you using physical apicids here for? They seem
> irrelevant to this function.

Look at where it's used.


-- wli

2003-07-04 18:14:08

by William Lee Irwin III

[permalink] [raw]
Subject: Re: 2.5.74-mm1 fails to boot due to APIC trouble, 2.5.73mm3 works.

At some point in the past, I wrote:
>>> -#define APIC_BROADCAST_ID (0x0f)
>>> +#define APIC_BROADCAST_ID (0xff)

On Fri, 4 Jul 2003, Martin J. Bligh wrote:
>> So ... you've tested that change on a bigsmp machine, right?
>> At least, provide some reasoning here. Like this comment further down the
>> patch ...

On Fri, Jul 04, 2003 at 11:47:03AM -0400, Zwane Mwaikambo wrote:
> That one is slightly worrying, yes.

It is not. It's a bound on physical APIC ID's. bigsmp is xAPIC-based.


At some point in the past, I wrote:
>>> +++ physid-2.5.74-1/include/asm-i386/mach-numaq/mach_apic.h
>>> 2003-07-04 02:45:17.000000000 -0700
>>>
>>> -static inline cpumask_t apicid_to_cpu_present(int logical_apicid)
>>> +static inline physid_mask_t apicid_to_cpu_present(int logical_apicid)
>>> {
>>> int node = apicid_to_node(logical_apicid);
>>> int cpu = __ffs(logical_apicid & 0xf);
>>>
>>> - return cpumask_of_cpu(cpu + 4*node);
>>> + return physid_mask_of_physid(cpu + 4*node);
>>> }

On Fri, 4 Jul 2003, Martin J. Bligh wrote:
>> Hmmmm. What are you using physical apicids here for? They seem
>> irrelevant to this function.

On Fri, Jul 04, 2003 at 11:47:03AM -0400, Zwane Mwaikambo wrote:
> Urgh, it's really hard to determine what these functions really want half
> the time. But that change does look wrong.

It's fine. It's used to generate a bitmask that's or'd with
phys_cpu_present_map.


-- wli

2003-07-04 18:17:04

by William Lee Irwin III

[permalink] [raw]
Subject: Re: 2.5.74-mm1 fails to boot due to APIC trouble, 2.5.73mm3 works.

On Fri, Jul 04, 2003 at 09:18:12AM -0700, Martin J. Bligh wrote:
> Ugh, are you saying the cpumask stuff shrinks masks to < 32 bits if
> NR_CPUS is low enough? If so, I can see more point to the patch, but
> it still seems like violent overkill. Stopping it doing that would
> probably fix it ... I can't imagine it buys you much.

Step off it. This is not overkill. This is correct.


-- wli

2003-07-04 18:15:28

by William Lee Irwin III

[permalink] [raw]
Subject: Re: 2.5.74-mm1 fails to boot due to APIC trouble, 2.5.73mm3 works.

On Fri, Jul 04, 2003 at 09:18:12AM -0700, Martin J. Bligh wrote:
> Yeah, things taking logical apicids, and turning them into cpu numbers
> presumably shouldn't have to touch that.

The bitmap is wider than the function wants. The change is fine, despite
your abuse of phys_cpu_present_map.


-- wli

2003-07-04 18:21:11

by William Lee Irwin III

[permalink] [raw]
Subject: Re: 2.5.74-mm1 fails to boot due to APIC trouble, 2.5.73mm3 works.

At some point in the past, zwane wrote:
>> The way i see it is that you can't use NR_CPUS to determine the upper
>> bound on APIC IDs. e.g. my 3way is normally configured with NR_CPUS = 3
>> but has APIC IDs of 0, 3 and 4. We need to make a distinction.

On Fri, Jul 04, 2003 at 09:18:12AM -0700, Martin J. Bligh wrote:
> Fair enough. But that would seem to be a simpler operation than this patch.

It is not a simpler operation. It is the patch.

On Fri, Jul 04, 2003 at 09:18:12AM -0700, Martin J. Bligh wrote:
> We break it out in subarch, but it's the same everywhere, which seems
> utterly useless - is probably historical cruft that needs to die.
> But that sounds like a separate issue, and a separate patch to me.

The change is correct. Let it stand.


On Fri, Jul 04, 2003 at 09:18:12AM -0700, Martin J. Bligh wrote:
> Ugh, are you saying the cpumask stuff shrinks masks to < 32 bits if
> NR_CPUS is low enough? If so, I can see more point to the patch, but
> it still seems like violent overkill. Stopping it doing that would
> probably fix it ... I can't imagine it buys you much.

This change is also correct. Let it stand.


On Fri, Jul 04, 2003 at 09:18:12AM -0700, Martin J. Bligh wrote:
> phys_cpu_present_map started off as an unsigned long, and I reused it
> in a fairly twisted way for NUMA-Q. As it's an array that's bounded
> by apic space, using the bios_cpu_apicid method that summit uses
> would be a much cleaner fix, and just leave the old one as a long
> bitmask like it used to be - which is fine for non- clustered apic
> systems, and saves inventing a whole new data type. See the
> cpu_present_to_apicid abstraction.

The precise thing this does is making phys_cpu_present_map an array
bounded by the APIC space. The change is correct. Let it stand.


At some point in the past, zwane wrote:
>> Urgh, it's really hard to determine what these functions really want half
>> the time. But that change does look wrong.

On Fri, Jul 04, 2003 at 09:18:12AM -0700, Martin J. Bligh wrote:
> Yeah, things taking logical apicids, and turning them into cpu numbers
> presumably shouldn't have to touch that.

The cpu bitmasks change width with NR_CPUS. The APIC ID spaces do not.
They must hence be bitmasks of separate widths. Hence the change is
correct. Let it stand.


-- wli

2003-07-04 19:06:15

by Martin J. Bligh

[permalink] [raw]
Subject: Re: 2.5.74-mm1 fails to boot due to APIC trouble, 2.5.73mm3 works.

>> Yeah, things taking logical apicids, and turning them into cpu numbers
>> presumably shouldn't have to touch that.
>
> The bitmap is wider than the function wants. The change is fine, despite
> your abuse of phys_cpu_present_map.

I'm happy to remove the abuse of phys_cpu_present_map, seeing as we now
have a reason to do so. That would actually seem a much cleaner solution
to these problems than creating a whole new data type, which still doesn't
represent what it claims to


M.

2003-07-04 19:15:53

by William Lee Irwin III

[permalink] [raw]
Subject: Re: 2.5.74-mm1 fails to boot due to APIC trouble, 2.5.73mm3 works.

At some point in the past, I wrote:
>> The bitmap is wider than the function wants. The change is fine, despite
>> your abuse of phys_cpu_present_map.

On Fri, Jul 04, 2003 at 12:20:02PM -0700, Martin J. Bligh wrote:
> I'm happy to remove the abuse of phys_cpu_present_map, seeing as we now
> have a reason to do so. That would actually seem a much cleaner solution
> to these problems than creating a whole new data type, which still doesn't
> represent what it claims to

Dirtier, but possibly lower line count.


-- wli

2003-07-04 19:24:18

by Martin J. Bligh

[permalink] [raw]
Subject: Re: 2.5.74-mm1 fails to boot due to APIC trouble, 2.5.73mm3 works.

> On Fri, Jul 04, 2003 at 02:35:31AM -0700, William Lee Irwin III wrote:
>>>> Okay, now for the "final solution" wrt. sparse physical APIC ID's
>>>> in addition to what I hope is a fix for your bug. This uses a separate
>>>> bitmap type (of a NR_CPUS -independent width MAX_APICS) for physical
>>>> APIC ID bitmaps.
>>>> \begin{cross-fingers}
>
> On Fri, Jul 04, 2003 at 08:41:38AM -0700, Martin J. Bligh wrote:
>> Is it really necessary to turn half the apic code upside down in order
>> to fix this? What's the actual bugfix that's buried in this cleanup?
>> Despite the fact you seem to have gone out of your way to make this
>> hard to review, there are a few things I can see that strike me as odd.
>> Not necessarily wrong, but requiring more explanation.
>
> It's not a cleanup, and it doesn't touch trailing whitespace etc.

Maybe not, but it looks like one. Maybe if you actually explain
what you're trying to fix, and why?

I think this kind of change deserves a better explanation that
"I'm right" ... that's my main objection.

> On Fri, Jul 04, 2003 at 02:35:31AM -0700, William Lee Irwin III wrote:
>>> - if (i >= 0xf)
>>> + if (i >= APIC_BROADCAST_ID)
>
> On Fri, Jul 04, 2003 at 08:41:38AM -0700, Martin J. Bligh wrote:
>> Is that always correct? it's not equivalent.
>
> It is.

Explain. Not obvious to the casual observer.

> On Fri, Jul 04, 2003 at 02:35:31AM -0700, William Lee Irwin III wrote:
>>> diff -prauN mm1-2.5.74-1/include/asm-i386/mach-bigsmp/mach_apic.h physid-2.5.74-1/include/asm-i386/mach-bigsmp/mach_apic.h
>>> --- mm1-2.5.74-1/include/asm-i386/mach-bigsmp/mach_apic.h 2003-07-03 12:23:56.000000000 -0700
>>> +++ physid-2.5.74-1/include/asm-i386/mach-bigsmp/mach_apic.h 2003-07-04 02:47:45.000000000 -0700
>>> @@ -29,15 +29,15 @@ static inline cpumask_t target_cpus(void
>>> # define INT_DELIVERY_MODE dest_LowestPrio
>>> # define INT_DEST_MODE 1 /* logical delivery broadcast to all procs */
>>> -#define APIC_BROADCAST_ID (0x0f)
>>> +#define APIC_BROADCAST_ID (0xff)
>
> On Fri, Jul 04, 2003 at 08:41:38AM -0700, Martin J. Bligh wrote:
>> So ... you've tested that change on a bigsmp machine, right?
>> At least, provide some reasoning here. Like this comment further down the
>> patch ...
>
> APIC_BROADCAST_ID is an upper bound on valid physical APIC ID's as it
> is used in the code. That actually was commented in the patch.

I find it odd that this worked before then. Also seems to be a separate
issue from the rest of the patch. Is quite probably correct, is just
non-obvious in the context of the rest of the patch.

> On Fri, Jul 04, 2003 at 02:35:31AM -0700, William Lee Irwin III wrote:
>>> +/*
>>> + * this isn't really broadcast, just a (potentially inaccurate) upper
>>> + * bound for valid physical APIC id's
>>> + */
>
> On Fri, Jul 04, 2003 at 08:41:38AM -0700, Martin J. Bligh wrote:
>> Which makes the change just look wrong to me. If you're thinking
>> "physical clustered mode" that terminology just utterly confusing crap,
>> and the change is wrong, as far as I can see.
>
> The change is correct, and I am not thinking of any such thing.
> APIC_BROADCAST_ID's sole usage is for terminating loops over physical
> APIC ID's while setting the physical APIC ID's of IO-APIC's.

Why is Summit 0xF, and bigsmp 0xFF then?

> On Fri, Jul 04, 2003 at 02:35:31AM -0700, William Lee Irwin III wrote:
>>> +++ physid-2.5.74-1/include/asm-i386/mach-numaq/mach_apic.h
>>> 2003-07-04 02:45:17.000000000 -0700
>>>
>>> -static inline cpumask_t apicid_to_cpu_present(int logical_apicid)
>>> +static inline physid_mask_t apicid_to_cpu_present(int logical_apicid)
>>> {
>>> int node = apicid_to_node(logical_apicid);
>>> int cpu = __ffs(logical_apicid & 0xf);
>>>
>>> - return cpumask_of_cpu(cpu + 4*node);
>>> + return physid_mask_of_physid(cpu + 4*node);
>>> }
>
> On Fri, Jul 04, 2003 at 08:41:38AM -0700, Martin J. Bligh wrote:
>> Hmmmm. What are you using physical apicids here for? They seem
>> irrelevant to this function.
>
> Look at where it's used.

I did. Still unclear why you think this is correct, or what physical
apicids have to do with a function that maps from apicids to the
phys_cpu_present_map, which is a compact mapping of logical apicids
for NUMA-Q.

Sorry, but this needs more explanation.

M.

2003-07-04 19:39:57

by Martin J. Bligh

[permalink] [raw]
Subject: Re: 2.5.74-mm1 fails to boot due to APIC trouble, 2.5.73mm3 works.

--William Lee Irwin III <[email protected]> wrote (on Friday, July 04, 2003 12:31:35 -0700):

> At some point in the past, I wrote:
>>> The bitmap is wider than the function wants. The change is fine, despite
>>> your abuse of phys_cpu_present_map.
>
> On Fri, Jul 04, 2003 at 12:20:02PM -0700, Martin J. Bligh wrote:
>> I'm happy to remove the abuse of phys_cpu_present_map, seeing as we now
>> have a reason to do so. That would actually seem a much cleaner solution
>> to these problems than creating a whole new data type, which still doesn't
>> represent what it claims to
>
> Dirtier, but possibly lower line count.

I disagree with the "dirtier" bit, but still. I'd rather have this sort
of stuff put into subarch, where most people don't have to look at it.

More to the point, the changes would be confined to the big-iron arches,
and have less chance of breaking anyone else for things they don't
care about, nor do them any benefit. Touching this code is fragile as
hell, so if it can be confined, it should be ...

It'd also remove the long-standing abuse of phys_cpu_present_map, which
would probably make the rest of the code clearer.

M.

2003-07-04 19:52:13

by William Lee Irwin III

[permalink] [raw]
Subject: Re: 2.5.74-mm1 fails to boot due to APIC trouble, 2.5.73mm3 works.

On Fri, Jul 04, 2003 at 02:35:31AM -0700, William Lee Irwin III wrote:
>> It's not a cleanup, and it doesn't touch trailing whitespace etc.

On Fri, Jul 04, 2003 at 12:38:19PM -0700, Martin J. Bligh wrote:
> Maybe not, but it looks like one. Maybe if you actually explain
> what you're trying to fix, and why?
> I think this kind of change deserves a better explanation that
> "I'm right" ... that's my main objection.

I'll try to be more verbose, then.


On Fri, Jul 04, 2003 at 02:35:31AM -0700, William Lee Irwin III wrote:
>> It is.

On Fri, Jul 04, 2003 at 12:38:19PM -0700, Martin J. Bligh wrote:
> Explain. Not obvious to the casual observer.

The function assigns physical APIC ID's to IO-APIC's. The loop is
intended to iterate over the physical APIC ID space. 0xf is an
inaccurate description of the upper bound on the physical APIC ID space.
APIC_BROADCAST_ID is a more accurate upper bound.


On Fri, Jul 04, 2003 at 02:35:31AM -0700, William Lee Irwin III wrote:
>> APIC_BROADCAST_ID is an upper bound on valid physical APIC ID's as it
>> is used in the code. That actually was commented in the patch.

On Fri, Jul 04, 2003 at 12:38:19PM -0700, Martin J. Bligh wrote:
> I find it odd that this worked before then. Also seems to be a separate
> issue from the rest of the patch. Is quite probably correct, is just
> non-obvious in the context of the rest of the patch.

I audited not only for usage of limited-width bitmaps for APIC ID
spaces, but also improper bounds on iterations over APIC ID spaces.
Things ran out of APIC ID's when phys_cpu_present_map was NR_CPUS
wide. This patch makes the limits accurate to the hardware with
the brute-force application of bitmaps. The semantic impact of
dropping in a bitmap is very low. The issue that arose was that it
wasn't wide enough, which was obvious enough to spot as a thinko
without even testing.


On Fri, Jul 04, 2003 at 02:35:31AM -0700, William Lee Irwin III wrote:
>> The change is correct, and I am not thinking of any such thing.
>> APIC_BROADCAST_ID's sole usage is for terminating loops over physical
>> APIC ID's while setting the physical APIC ID's of IO-APIC's.

On Fri, Jul 04, 2003 at 12:38:19PM -0700, Martin J. Bligh wrote:
> Why is Summit 0xF, and bigsmp 0xFF then?

Summit (and all other xAPIC-based subarches) should be 0xFF; I missed
it in the sweep.


On Fri, Jul 04, 2003 at 02:35:31AM -0700, William Lee Irwin III wrote:
>> Look at where it's used.

On Fri, Jul 04, 2003 at 12:38:19PM -0700, Martin J. Bligh wrote:
> I did. Still unclear why you think this is correct, or what physical
> apicids have to do with a function that maps from apicids to the
> phys_cpu_present_map, which is a compact mapping of logical apicids
> for NUMA-Q.
> Sorry, but this needs more explanation.

The bitmap width is sufficient. NUMA-Q abuses what everything else
uses for physical APIC ID's (partly because of the BIOS). It so happens
that the array is MAX_APICS wide, which suffices for NUMA-Q (and
anything else that cares to use it).

No. This was not written for or around NUMA-Q; it's meant for the
io_apic.c loops and sparse physid wakeup on non-NUMA-Q machines.


-- wli

2003-07-04 20:01:34

by William Lee Irwin III

[permalink] [raw]
Subject: Re: 2.5.74-mm1 fails to boot due to APIC trouble, 2.5.73mm3 works.

William Lee Irwin III <[email protected]> wrote (on Friday, July 04, 2003 12:31:35 -0700):
>> Dirtier, but possibly lower line count.

On Fri, Jul 04, 2003 at 12:53:54PM -0700, Martin J. Bligh wrote:
> I disagree with the "dirtier" bit, but still. I'd rather have this sort
> of stuff put into subarch, where most people don't have to look at it.
> More to the point, the changes would be confined to the big-iron arches,
> and have less chance of breaking anyone else for things they don't
> care about, nor do them any benefit. Touching this code is fragile as
> hell, so if it can be confined, it should be ...
> It'd also remove the long-standing abuse of phys_cpu_present_map, which
> would probably make the rest of the code clearer.

That's a change with deeper semantic implications as it's relying on
different information.

The phys_cpu_present_map bits were to divorce its width from NR_CPUS
in a portable way. Shifting to bios_cpu_map[] should only change cpu
wakeup. IO-APIC physid reassignment code still needs a variable-width
map (I suppose you could use integers) of some kind.

-- wli

2003-07-04 20:23:57

by Martin J. Bligh

[permalink] [raw]
Subject: Re: 2.5.74-mm1 fails to boot due to APIC trouble, 2.5.73mm3 works.

>> Maybe not, but it looks like one. Maybe if you actually explain
>> what you're trying to fix, and why?
>> I think this kind of change deserves a better explanation that
>> "I'm right" ... that's my main objection.
>
> I'll try to be more verbose, then.

Thanks ... will help a lot ;-)

> On Fri, Jul 04, 2003 at 12:38:19PM -0700, Martin J. Bligh wrote:
>> Explain. Not obvious to the casual observer.
>
> The function assigns physical APIC ID's to IO-APIC's. The loop is
> intended to iterate over the physical APIC ID space. 0xf is an
> inaccurate description of the upper bound on the physical APIC ID space.
> APIC_BROADCAST_ID is a more accurate upper bound.

OK, you're right. Is just confusing because it works as it is right
now ... even on a 32x system - however, that's only because Summit
doesn't actually run that region of code, and NUMA-Q ignores it.

> On Fri, Jul 04, 2003 at 02:35:31AM -0700, William Lee Irwin III wrote:
>>> APIC_BROADCAST_ID is an upper bound on valid physical APIC ID's as it
>>> is used in the code. That actually was commented in the patch.
>
> On Fri, Jul 04, 2003 at 12:38:19PM -0700, Martin J. Bligh wrote:
>> I find it odd that this worked before then. Also seems to be a separate
>> issue from the rest of the patch. Is quite probably correct, is just
>> non-obvious in the context of the rest of the patch.
>
> I audited not only for usage of limited-width bitmaps for APIC ID
> spaces, but also improper bounds on iterations over APIC ID spaces.
> Things ran out of APIC ID's when phys_cpu_present_map was NR_CPUS
> wide. This patch makes the limits accurate to the hardware with
> the brute-force application of bitmaps. The semantic impact of
> dropping in a bitmap is very low. The issue that arose was that it
> wasn't wide enough, which was obvious enough to spot as a thinko
> without even testing.
>
>> Why is Summit 0xF, and bigsmp 0xFF then?
>
> Summit (and all other xAPIC-based subarches) should be 0xFF; I missed
> it in the sweep.

OK. Makes more sense now if both are that way.

> On Fri, Jul 04, 2003 at 02:35:31AM -0700, William Lee Irwin III wrote:
>>> Look at where it's used.
>
> On Fri, Jul 04, 2003 at 12:38:19PM -0700, Martin J. Bligh wrote:
>> I did. Still unclear why you think this is correct, or what physical
>> apicids have to do with a function that maps from apicids to the
>> phys_cpu_present_map, which is a compact mapping of logical apicids
>> for NUMA-Q.
>> Sorry, but this needs more explanation.
>
> The bitmap width is sufficient. NUMA-Q abuses what everything else
> uses for physical APIC ID's (partly because of the BIOS). It so happens
> that the array is MAX_APICS wide, which suffices for NUMA-Q (and
> anything else that cares to use it).
>
> No. This was not written for or around NUMA-Q; it's meant for the
> io_apic.c loops and sparse physid wakeup on non-NUMA-Q machines.

OK, maybe this is just an extension of my earlier abuse - in which
case, let's just remove it. Was bad enough before, but now even I
can't understand it, and I wrote the damned thing.

So yes, it looks correct. I'll see if I can see a neat way to bury this
under the existing abstractions like Summit does ... I'm not sure it's
a good idea to have two different methods for this; that whole area of
code is getting horribly complicated ...

Thanks very much for the explanations,

M.