svn commit: r233961 - head/sys/x86/x86

[Attilio Rao]

Il 09 aprile 2012 17:34, John Baldwin <jhb at freebsd.org> ha scritto:
> On Monday, April 09, 2012 11:45:11 am Jaakko Heinonen wrote:
>>
>> Hi,
>>
>> On 2012-04-06, Justin T. Gibbs wrote:
>> >   Fix interrupt load balancing regression, introduced in revision
>> >   222813, that left all un-pinned interrupts assigned to CPU 0.
>> >
>> >   sys/x86/x86/intr_machdep.c:
>> >     In intr_shuffle_irqs(), remove CPU_SETOF() call that initialized
>> >     the "intr_cpus" cpuset to only contain CPU0.
>> >
>> >     This initialization is too late and nullifies the results of calls
>> >     the intr_add_cpu() that occur much earlier in the boot process.
>> >     Since "intr_cpus" is statically initialized to the empty set, and
>> >     all processors, including the BSP, already add themselves to
>> >     "intr_cpus" no special initialization for the BSP is necessary.
>>
>> My Pentium 4 system hangs on boot after this commit. These are the last
>> lines from a verbose boot:
>>
>> SMP: AP CPU #1 Launched!
>> cpu1 AP:
>>      ID: 0x01000000   VER: 0x00050014 LDR: 0x00000000 DFR: 0xffffffff
>>   lint0: 0x00010700 lint1: 0x00000400 TPR: 0x00000000 SVR: 0x000001ff
>>   timer: 0x000100ef therm: 0x00010000 err: 0x000000f0 pmc: 0x00010400
>>
>> The system boots with r233960.
>>
>> Some information:
>>
>> CPU: Intel(R) Pentium(R) 4 CPU 2.60GHz (2605.96-MHz 686-class CPU)
>>   Origin = "GenuineIntel"  Id = 0xf29  Family = f  Model = 2  Stepping =
>> 9
>>
> Features=0xbfebfbff<FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CLFLUSH,DTS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE>
>>   Features2=0x4400<CNXT-ID,xTPR>
>> real memory  = 2147483648 (2048 MB)
>> avail memory = 2085228544 (1988 MB)
>> Event timer "LAPIC" quality 400
>> ACPI APIC Table: <A M I  OEMAPIC >
>> FreeBSD/SMP: Multiprocessor System Detected: 2 CPUs
>> FreeBSD/SMP: 1 package(s) x 1 core(s) x 2 HTT threads
>>  cpu0 (BSP): APIC ID:  0
>>  cpu1 (AP/HT): APIC ID:  1
>
> I suspect in your case intr_add_cpu() is never called.  I think Attilio is not
> correct in that it is not called for the BSP.
>
> Yes, it is not called for the BSP in set_interrupt_apic_ids().  This used to
> work because bit 0 was assigned statically.  Also, in a UP machine
> set_interrupt_apic_ids() is not called at all.

But why there is a front-end check for the BSP in set_interrupt_apic_ids()?

Anyway, I think a better fix would be like the attached patch.

Thanks,
Attilio

Index: sys/i386/i386/mp_machdep.c
===================================================================
--- sys/i386/i386/mp_machdep.c  (revisione 234046)
+++ sys/i386/i386/mp_machdep.c  (copia locale)
@@ -831,6 +831,8 @@ set_interrupt_apic_ids(void)
                apic_id = cpu_apic_ids[i];
                if (apic_id == -1)
                        continue;
+
+               /* BSP should be already added by cpu_startup(). */
                if (cpu_info[apic_id].cpu_bsp)
                        continue;
                if (cpu_info[apic_id].cpu_disabled)
Index: sys/i386/i386/machdep.c
===================================================================
--- sys/i386/i386/machdep.c     (revisione 234046)
+++ sys/i386/i386/machdep.c     (copia locale)
@@ -336,6 +336,11 @@ cpu_startup(dummy)
 #ifndef XEN
        cpu_setregs();
 #endif
+
+       /*
+        * Add BSP interrupt bitmask.
+        */
+       intr_add_cpu(0);
 }

 /*
Index: sys/amd64/amd64/mp_machdep.c
===================================================================
--- sys/amd64/amd64/mp_machdep.c        (revisione 234046)
+++ sys/amd64/amd64/mp_machdep.c        (copia locale)
@@ -797,6 +797,8 @@ set_interrupt_apic_ids(void)
                apic_id = cpu_apic_ids[i];
                if (apic_id == -1)
                        continue;
+
+               /* BSP should be already added by cpu_startup(). */
                if (cpu_info[apic_id].cpu_bsp)
                        continue;
                if (cpu_info[apic_id].cpu_disabled)
Index: sys/amd64/amd64/machdep.c
===================================================================
--- sys/amd64/amd64/machdep.c   (revisione 234046)
+++ sys/amd64/amd64/machdep.c   (copia locale)
@@ -295,6 +295,11 @@ cpu_startup(dummy)
        vm_pager_bufferinit();

        cpu_setregs();
+
+       /*
+        * Add BSP interrupt bitmask.
+        */
+       intr_add_cpu(0);
 }

 /*