| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20090608151646.GB5015@lenovo>
Date: Mon, 8 Jun 2009 19:16:46 +0400
From: Cyrill Gorcunov <gorcunov@...il.com>
To: Yinghai Lu <yhlu.kernel@...il.com>
Cc: mingo@...hat.com, hpa@...or.com, linux-kernel@...r.kernel.org,
tglx@...utronix.de, mingo@...e.hu,
linux-tip-commits@...r.kernel.org
Subject: Re: [tip:irq/numa] x86, apic: Fix dummy apic read operation
together with broken MP handling
[Yinghai Lu - Sun, Jun 07, 2009 at 03:59:28PM -0700]
| On Sun, Jun 7, 2009 at 1:39 PM, Cyrill Gorcunov<gorcunov@...il.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.
|
Yeah, I've been just messed with the number of options.
We dont have smp_found_config set in this case.
|
|
| >
| > 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;
| }
Interesting scenario indeed. So we have the calls chain
start_kernel
setup_arch
setup_disableapic
init_apic_mappings
kernel_init
smp_prepare_cpus
smp_sanity_check (have reached with disable_apic=1, !cpu_has_apic)
...
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;
}
...
There we should disable smp support. I think we could
do this check in verify_local_APIC and return error code
and jump back to disable smp ops. We could not just remove
this check since cpu_has_apic could be cleared without
explicit apic disablement. Will cook a RFC for this case.
|
| >
| > 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
|
-- Cyrill
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists