lkml.org 
[lkml]   [2009]   [Jun]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [tip:irq/numa] x86, apic: Fix dummy apic read operation together with broken MP handling
From
On Sun, Jun 7, 2009 at 1:39 PM, Cyrill Gorcunov<gorcunov@gmail.com> wrote:
> [Yinghai Lu - Sun, Jun 07, 2009 at 12:23:47PM -0700]
> ...
> | > diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> | > index d2e8de9..7c80007 100644
> | > --- a/arch/x86/kernel/smpboot.c
> | > +++ b/arch/x86/kernel/smpboot.c
> | > @@ -992,10 +992,12 @@ static int __init smp_sanity_check(unsigned max_cpus)
> | >         */
> | >        if (APIC_INTEGRATED(apic_version[boot_cpu_physical_apicid]) &&
> | >            !cpu_has_apic) {
> | > -               printk(KERN_ERR "BIOS bug, local APIC #%d not detected!...\n",
> | > -                       boot_cpu_physical_apicid);
> | > -               printk(KERN_ERR "... forcing use of dummy APIC emulation."
> | > +               if (!disable_apic) {
> | > +                       pr_err("BIOS bug, local APIC #%d not detected!...\n",
> | > +                               boot_cpu_physical_apicid);
> | > +                       pr_err("... forcing use of dummy APIC emulation."
> | >                                "(tell your hw vendor)\n");
> | > +               }
> |
> | It seems we don't need this check here.
> | when we have disable_apic, cpu_has_apic is cleared forcely.
> |
> | YH
> |
>
> No Yinghai, it's needed. The check is for !disable_apic
> and if we really has a BIOS bug we should report about
> it _only_ in case if it's a bios bug not apic being
> disabled via boot line. I could be missing something
> of course. Rechecking...
>
> Ah, I remember the scenario I've kept in mind while
> was cooking the patch.
>
> 1) MP apic entry is broken.
> 2) apic was disabled via boot option.
> 3) kernel compiled with smp support.
>
> So we have the calls
>
> init_apic_mappings
>                (due to boot option)
>                apic_disable()
>        (due to broken MP)
>        apic_version[new_apicid] = 0
> smp_sanity_check
>        if (APIC_INTEGRATED(apic_version[boot_cpu_physical_apicid]) &&
>            !cpu_has_apic) {
> Stop! So APIC_INTEGRATED returns false now regargless of what
> really we have on HW level. Darn! Actually I was in idea
> this if() should be true so SMP support will be turned _off_.

in Ingo' case:
before that check
/*
* If we couldn't find an SMP configuration at boot time,
* get out of here now!
*/
if (!smp_found_config && !acpi_lapic) {
preempt_enable();
printk(KERN_NOTICE "SMP motherboard not detected.\n");
disable_smp();
if (APIC_init_uniprocessor())
printk(KERN_NOTICE "Local APIC not detected."
" Using dummy APIC emulation.\n");
return -1;
}

will get out earlier.



>
> Yinghai, I think we need to set apic_version[boot_cpu_physical_apicid]
> to 0xf0 in case if apic is disabled via cmdline option together with
> broken MP. Thoughts?

should be other case:
when MADT is right, but disablelapic is used.
will get cpu_has_apic == 0, and we are not using dummy apic read/write.

so don't need to check
/*
* If we couldn't find a local APIC, then get out of here now!
*/
if (APIC_INTEGRATED(apic_version[boot_cpu_physical_apicid]) &&
!cpu_has_apic) {
if (!disable_apic) {
pr_err("BIOS bug, local APIC #%d not detected!...\n",
boot_cpu_physical_apicid);
pr_err("... forcing use of dummy APIC emulation."
"(tell your hw vendor)\n");
}
smpboot_clear_io_apic();
arch_disable_smp_support();
return -1;
}

>
> To Ingo: this !disable_apic will be needed if Yinghai confirm
> that my idea is right. Meanwhile -- it's just an always-true
> cpu consuming operation. So should not be harming. But sorry
> anyway -- was thinking about one way but reached another.

YH
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2009-06-08 01:01    [W:0.047 / U:12.968 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site