[PATCH] x86: do_boot_cpu - check for ESR apic register presence before touching

[PATCH] x86: do_boot_cpu - check for ESR apic register presence before touching

[Cyrill Gorcunov]

We should check first if there is ESR register before read/write it.

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---

Please review and test if possible. Thanks.

Index: linux-2.6.git/arch/x86/kernel/smpboot.c
===================================================================
--- linux-2.6.git.orig/arch/x86/kernel/smpboot.c	2008-09-14 21:49:36.000000000 +0400
+++ linux-2.6.git/arch/x86/kernel/smpboot.c	2008-09-15 18:30:37.000000000 +0400
@@ -893,8 +893,10 @@ do_rest:
 		/*
 		 * Be paranoid about clearing APIC errors.
 	 	*/
-		apic_write(APIC_ESR, 0);
-		apic_read(APIC_ESR);
+		if (APIC_INTEGRATED(apic_version[phys_apicid])) {
+			apic_write(APIC_ESR, 0);
+			apic_read(APIC_ESR);
+		}
 	}
 
 	/*

Re: [PATCH] x86: do_boot_cpu - check for ESR apic register presence before touching

[Yinghai Lu]

On Mon, Sep 15, 2008 at 8:05 AM, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
> We should check first if there is ESR register before read/write it.
>
> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> ---
>
> Please review and test if possible. Thanks.
>
> Index: linux-2.6.git/arch/x86/kernel/smpboot.c
> ===================================================================
> --- linux-2.6.git.orig/arch/x86/kernel/smpboot.c        2008-09-14 21:49:36.000000000 +0400
> +++ linux-2.6.git/arch/x86/kernel/smpboot.c     2008-09-15 18:30:37.000000000 +0400
> @@ -893,8 +893,10 @@ do_rest:
>                /*
>                 * Be paranoid about clearing APIC errors.
>                */
> -               apic_write(APIC_ESR, 0);
> -               apic_read(APIC_ESR);
> +               if (APIC_INTEGRATED(apic_version[phys_apicid])) {
> +                       apic_write(APIC_ESR, 0);
> +                       apic_read(APIC_ESR);
> +               }
>        }
>
>        /*
>

maybe unrelated. Do we need to keep apic_version array?

YH

Re: [PATCH] x86: do_boot_cpu - check for ESR apic register presence before touching

[Cyrill Gorcunov]

[Yinghai Lu - Mon, Sep 15, 2008 at 09:12:37AM -0700]
| On Mon, Sep 15, 2008 at 8:05 AM, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
| > We should check first if there is ESR register before read/write it.
| >
| > Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
| > ---
| >
| > Please review and test if possible. Thanks.
| >
| > Index: linux-2.6.git/arch/x86/kernel/smpboot.c
| > ===================================================================
| > --- linux-2.6.git.orig/arch/x86/kernel/smpboot.c        2008-09-14 21:49:36.000000000 +0400
| > +++ linux-2.6.git/arch/x86/kernel/smpboot.c     2008-09-15 18:30:37.000000000 +0400
| > @@ -893,8 +893,10 @@ do_rest:
| >                /*
| >                 * Be paranoid about clearing APIC errors.
| >                */
| > -               apic_write(APIC_ESR, 0);
| > -               apic_read(APIC_ESR);
| > +               if (APIC_INTEGRATED(apic_version[phys_apicid])) {
| > +                       apic_write(APIC_ESR, 0);
| > +                       apic_read(APIC_ESR);
| > +               }
| >        }
| >
| >        /*
| >
| 
| maybe unrelated. Do we need to keep apic_version array?
| 
| YH
|

as I see ACPI/MP parsing code use it - actually I think we could
bring another variable - but don't think it worth it. Anyway
we have to get APIC info from these tables and save it somwhere
maybe in this array or maybe in another variable. Or you mean
anything else that I missed?
 
		- Cyrill -

Re: [PATCH] x86: do_boot_cpu - check for ESR apic register presence before touching

[Yinghai Lu]

On Mon, Sep 15, 2008 at 9:22 AM, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
> [Yinghai Lu - Mon, Sep 15, 2008 at 09:12:37AM -0700]
> | On Mon, Sep 15, 2008 at 8:05 AM, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
> | > We should check first if there is ESR register before read/write it.
> | >
> | > Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> | > ---
> | >
> | > Please review and test if possible. Thanks.
> | >
> | > Index: linux-2.6.git/arch/x86/kernel/smpboot.c
> | > ===================================================================
> | > --- linux-2.6.git.orig/arch/x86/kernel/smpboot.c        2008-09-14 21:49:36.000000000 +0400
> | > +++ linux-2.6.git/arch/x86/kernel/smpboot.c     2008-09-15 18:30:37.000000000 +0400
> | > @@ -893,8 +893,10 @@ do_rest:
> | >                /*
> | >                 * Be paranoid about clearing APIC errors.
> | >                */
> | > -               apic_write(APIC_ESR, 0);
> | > -               apic_read(APIC_ESR);
> | > +               if (APIC_INTEGRATED(apic_version[phys_apicid])) {
> | > +                       apic_write(APIC_ESR, 0);
> | > +                       apic_read(APIC_ESR);
> | > +               }
> | >        }
> | >
> | >        /*
> | >
> |
> | maybe unrelated. Do we need to keep apic_version array?
> |
> | YH
> |
>
> as I see ACPI/MP parsing code use it - actually I think we could
> bring another variable - but don't think it worth it. Anyway
> we have to get APIC info from these tables and save it somwhere
> maybe in this array or maybe in another variable. Or you mean
> anything else that I missed?

one apic_version or boot_cpu_apic_version could be enough.

YH

Re: [PATCH] x86: do_boot_cpu - check for ESR apic register presence before touching

[Cyrill Gorcunov]

[Yinghai Lu - Mon, Sep 15, 2008 at 09:25:14AM -0700]
| On Mon, Sep 15, 2008 at 9:22 AM, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
| > [Yinghai Lu - Mon, Sep 15, 2008 at 09:12:37AM -0700]
| > | On Mon, Sep 15, 2008 at 8:05 AM, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
| > | > We should check first if there is ESR register before read/write it.
| > | >
| > | > Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
| > | > ---
| > | >
| > | > Please review and test if possible. Thanks.
| > | >
| > | > Index: linux-2.6.git/arch/x86/kernel/smpboot.c
| > | > ===================================================================
| > | > --- linux-2.6.git.orig/arch/x86/kernel/smpboot.c        2008-09-14 21:49:36.000000000 +0400
| > | > +++ linux-2.6.git/arch/x86/kernel/smpboot.c     2008-09-15 18:30:37.000000000 +0400
| > | > @@ -893,8 +893,10 @@ do_rest:
| > | >                /*
| > | >                 * Be paranoid about clearing APIC errors.
| > | >                */
| > | > -               apic_write(APIC_ESR, 0);
| > | > -               apic_read(APIC_ESR);
| > | > +               if (APIC_INTEGRATED(apic_version[phys_apicid])) {
| > | > +                       apic_write(APIC_ESR, 0);
| > | > +                       apic_read(APIC_ESR);
| > | > +               }
| > | >        }
| > | >
| > | >        /*
| > | >
| > |
| > | maybe unrelated. Do we need to keep apic_version array?
| > |
| > | YH
| > |
| >
| > as I see ACPI/MP parsing code use it - actually I think we could
| > bring another variable - but don't think it worth it. Anyway
| > we have to get APIC info from these tables and save it somwhere
| > maybe in this array or maybe in another variable. Or you mean
| > anything else that I missed?
| 
| one apic_version or boot_cpu_apic_version could be enough.
| 
| YH
|

Yinghai I didn't check it - if you find it convenient -
I'll not object against... Maciej?

		- Cyrill -