[PATCH -tip] x86: apic - fix dummy apic read operation together with broken MP handling

[PATCH -tip] x86: apic - fix dummy apic read operation together with broken MP handling

[Cyrill Gorcunov]

Ingo Molnar reported that read_apic is screwed up novadays

[    0.000000] Using APIC driver default
[    0.000000] SMP: Allowing 1 CPUs, 0 hotplug CPUs
[    0.000000] Local APIC disabled by BIOS -- you can enable it with "lapic"
[    0.000000] APIC: disable apic facility
[    0.000000] ------------[ cut here ]------------
[    0.000000] WARNING: at arch/x86/kernel/apic/apic.c:254 native_apic_read_dummy+0x2d/0x3b()
[    0.000000] Hardware name: HP OmniBook PC

Indeed we still rely on apic->read operation for SMP compiled
kernel. And instead of disfigure the SMP code with #ifdef we
allow to call apic->read. To capture any unexpected results
we check for apic->read being called for sane reason via
WARN_ON_ONCE but(!) instead of OR we should use AND logical
operation (thanks Yinghai for spotting the root of the problem).

Along with that we could be having screwed MP table and we are
to fix it that way no Symmetric I/O started and no complains
about BIOS bug if apic was just disabled via command line.

CC: Yinghai Lu <yinghai@kernel.org>
Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
---

Please check carefully! Complains are welcome :)

 arch/x86/kernel/apic/apic.c |    9 ++++++++-
 arch/x86/kernel/smpboot.c   |    8 +++++---
 2 files changed, 13 insertions(+), 4 deletions(-)

Index: linux-2.6.git/arch/x86/kernel/apic/apic.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/apic/apic.c
+++ linux-2.6.git/arch/x86/kernel/apic/apic.c
@@ -251,7 +251,7 @@ static void native_apic_write_dummy(u32 
 
 static u32 native_apic_read_dummy(u32 reg)
 {
-	WARN_ON_ONCE((cpu_has_apic || !disable_apic));
+	WARN_ON_ONCE((cpu_has_apic && !disable_apic));
 	return 0;
 }
 
@@ -1612,6 +1612,13 @@ void __init init_apic_mappings(void)
 	new_apicid = read_apic_id();
 	if (boot_cpu_physical_apicid != new_apicid) {
 		boot_cpu_physical_apicid = new_apicid;
+		/*
+		 * yeah -- we lie about apic_version
+		 * in case if apic was disabled via boot option
+		 * but it's not a problem for SMP compiled kernel
+		 * since smp_sanity_check is prepared for such a case
+		 * and disable smp mode
+		 */
 		apic_version[new_apicid] =
 			 GET_APIC_VERSION(apic_read(APIC_LVR));
 	}
Index: linux-2.6.git/arch/x86/kernel/smpboot.c
=====================================================================
--- linux-2.6.git.orig/arch/x86/kernel/smpboot.c
+++ linux-2.6.git/arch/x86/kernel/smpboot.c
@@ -994,10 +994,12 @@ static int __init smp_sanity_check(unsig
 	 */
 	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");
+		}
 		smpboot_clear_io_apic();
 		arch_disable_smp_support();
 		return -1;

[tip:irq/numa] x86, apic: Fix dummy apic read operation together with broken MP handling

[tip-bot for Cyrill Gorcunov]

Commit-ID:  103428e57be323c3c5545db8ad12667099bc6005
Gitweb:     http://git.kernel.org/tip/103428e57be323c3c5545db8ad12667099bc6005
Author:     Cyrill Gorcunov <gorcunov@openvz.org>
AuthorDate: Sun, 7 Jun 2009 16:48:40 +0400
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Sun, 7 Jun 2009 16:08:05 +0200

x86, apic: Fix dummy apic read operation together with broken MP handling

Ingo Molnar reported that read_apic is buggy novadays:

[    0.000000] Using APIC driver default
[    0.000000] SMP: Allowing 1 CPUs, 0 hotplug CPUs
[    0.000000] Local APIC disabled by BIOS -- you can enable it with "lapic"
[    0.000000] APIC: disable apic facility
[    0.000000] ------------[ cut here ]------------
[    0.000000] WARNING: at arch/x86/kernel/apic/apic.c:254 native_apic_read_dummy+0x2d/0x3b()
[    0.000000] Hardware name: HP OmniBook PC

Indeed we still rely on apic->read operation for SMP compiled
kernel. And instead of disfigure the SMP code with #ifdef we
allow to call apic->read. To capture any unexpected results
we check for apic->read being called for sane reason via
WARN_ON_ONCE but(!) instead of OR we should use AND logical
operation (thanks Yinghai for spotting the root of the problem).

Along with that we could be have bad MP table and we are
to fix it that way no SMP started and no complains about
BIOS bug if apic was just disabled via command line.

Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
Cc: Yinghai Lu <yinghai@kernel.org>
LKML-Reference: <20090607124840.GD4547@lenovo>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 arch/x86/kernel/apic/apic.c |    9 ++++++++-
 arch/x86/kernel/smpboot.c   |    8 +++++---
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index e82488d..a4c9cf0 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -249,7 +249,7 @@ static void native_apic_write_dummy(u32 reg, u32 v)
 
 static u32 native_apic_read_dummy(u32 reg)
 {
-	WARN_ON_ONCE((cpu_has_apic || !disable_apic));
+	WARN_ON_ONCE((cpu_has_apic && !disable_apic));
 	return 0;
 }
 
@@ -1609,6 +1609,13 @@ void __init init_apic_mappings(void)
 	new_apicid = read_apic_id();
 	if (boot_cpu_physical_apicid != new_apicid) {
 		boot_cpu_physical_apicid = new_apicid;
+		/*
+		 * yeah -- we lie about apic_version
+		 * in case if apic was disabled via boot option
+		 * but it's not a problem for SMP compiled kernel
+		 * since smp_sanity_check is prepared for such a case
+		 * and disable smp mode
+		 */
 		apic_version[new_apicid] =
 			 GET_APIC_VERSION(apic_read(APIC_LVR));
 	}
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");
+		}
 		smpboot_clear_io_apic();
 		arch_disable_smp_support();
 		return -1;

Re: [tip:irq/numa] x86, apic: Fix dummy apic read operation together with broken MP handling

[Cyrill Gorcunov]

[tip-bot for Cyrill Gorcunov - Sun, Jun 07, 2009 at 02:24:31PM +0000]
| Commit-ID:  103428e57be323c3c5545db8ad12667099bc6005
| Gitweb:     http://git.kernel.org/tip/103428e57be323c3c5545db8ad12667099bc6005
| Author:     Cyrill Gorcunov <gorcunov@openvz.org>
| AuthorDate: Sun, 7 Jun 2009 16:48:40 +0400
| Committer:  Ingo Molnar <mingo@elte.hu>
| CommitDate: Sun, 7 Jun 2009 16:08:05 +0200
| 
| x86, apic: Fix dummy apic read operation together with broken MP handling
|

Hi Ingo, this commit will fix your case but to be fair
I'm a bit scared of disable_apic variable now spreading
all over the files. I've checked Makefile's and Kconfig
for situation when SMP turned on and apic.c is not compiled
(from hw POV on x86 it should not be possible since SMP
 requires APIC chip but dunno -- just something from
 inside of me says there could be a hidden problem).

I was also thinking about some common bitfiels for
apic features -- like one bit to figure out if
it was disabled via boot line, one bit for x2apic
mode enabled and so on. We have the cpu_has_...
helpers maybe the same for apic could be implemented?

How do you think?
 
	-- Cyrill

Re: [tip:irq/numa] x86, apic: Fix dummy apic read operation together with broken MP handling

[Yinghai Lu]

On Sun, Jun 7, 2009 at 7:24 AM, tip-bot for Cyrill
Gorcunov<gorcunov@openvz.org> wrote:
> Commit-ID:  103428e57be323c3c5545db8ad12667099bc6005
> Gitweb:     http://git.kernel.org/tip/103428e57be323c3c5545db8ad12667099bc6005
> Author:     Cyrill Gorcunov <gorcunov@openvz.org>
> AuthorDate: Sun, 7 Jun 2009 16:48:40 +0400
> Committer:  Ingo Molnar <mingo@elte.hu>
> CommitDate: Sun, 7 Jun 2009 16:08:05 +0200
>
> x86, apic: Fix dummy apic read operation together with broken MP handling
>
> Ingo Molnar reported that read_apic is buggy novadays:
>
> [    0.000000] Using APIC driver default
> [    0.000000] SMP: Allowing 1 CPUs, 0 hotplug CPUs
> [    0.000000] Local APIC disabled by BIOS -- you can enable it with "lapic"
> [    0.000000] APIC: disable apic facility
> [    0.000000] ------------[ cut here ]------------
> [    0.000000] WARNING: at arch/x86/kernel/apic/apic.c:254 native_apic_read_dummy+0x2d/0x3b()
> [    0.000000] Hardware name: HP OmniBook PC
>
> Indeed we still rely on apic->read operation for SMP compiled
> kernel. And instead of disfigure the SMP code with #ifdef we
> allow to call apic->read. To capture any unexpected results
> we check for apic->read being called for sane reason via
> WARN_ON_ONCE but(!) instead of OR we should use AND logical
> operation (thanks Yinghai for spotting the root of the problem).
>
> Along with that we could be have bad MP table and we are
> to fix it that way no SMP started and no complains about
> BIOS bug if apic was just disabled via command line.
>
> Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
> Cc: Yinghai Lu <yinghai@kernel.org>
> LKML-Reference: <20090607124840.GD4547@lenovo>
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
>
>
> ---
>  arch/x86/kernel/apic/apic.c |    9 ++++++++-
>  arch/x86/kernel/smpboot.c   |    8 +++++---
>  2 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index e82488d..a4c9cf0 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -249,7 +249,7 @@ static void native_apic_write_dummy(u32 reg, u32 v)
>
>  static u32 native_apic_read_dummy(u32 reg)
>  {
> -       WARN_ON_ONCE((cpu_has_apic || !disable_apic));
> +       WARN_ON_ONCE((cpu_has_apic && !disable_apic));
>        return 0;
>  }
>
> @@ -1609,6 +1609,13 @@ void __init init_apic_mappings(void)
>        new_apicid = read_apic_id();
>        if (boot_cpu_physical_apicid != new_apicid) {
>                boot_cpu_physical_apicid = new_apicid;
> +               /*
> +                * yeah -- we lie about apic_version
> +                * in case if apic was disabled via boot option
> +                * but it's not a problem for SMP compiled kernel
> +                * since smp_sanity_check is prepared for such a case
> +                * and disable smp mode
> +                */
>                apic_version[new_apicid] =
>                         GET_APIC_VERSION(apic_read(APIC_LVR));
>        }
> 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

Re: [tip:irq/numa] x86, apic: Fix dummy apic read operation together with broken MP handling

[Cyrill Gorcunov]

[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_.

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?

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.

	-- Cyrill

Re: [tip:irq/numa] x86, apic: Fix dummy apic read operation together with broken MP handling

[Yinghai Lu]

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

Re: [tip:irq/numa] x86, apic: Fix dummy apic read operation together with broken MP handling

[Cyrill Gorcunov]

[Yinghai Lu - Sun, Jun 07, 2009 at 03:59:28PM -0700]
| 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.
| 

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

Re: [tip:irq/numa] x86, apic: Fix dummy apic read operation together with broken MP handling

[Cyrill Gorcunov]

[Cyrill Gorcunov - Mon, Jun 08, 2009 at 07:16:46PM +0400]
...
| | 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;
| |         }
|
...

I've just emulated the situation where this if() triggered via
disable_apic boot option and without if (!disable_apic) we
have just a wrong message about BIOS bug (which is not since
we've disabled apic by hands). So no need to remove this
"if (!disable_apic)" snippet. In most cases it will be
always-true condition but for rare cases it'll be usefull
as well.

Now I'm trying to fake apic integrated case so we could
need additional check here...
 
	-- Cyrill