[PATCH] x86: do_boot_cpu - add check if we have ESR register

[PATCH] x86: do_boot_cpu - add check if we have ESR register

[Cyrill Gorcunov]

We should touch ESR register if only we have one.
Check its presence first.

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
CC: Yinghai Lu <yhlu.kernel@gmail.com>
---

Index: linux-2.6.git/arch/x86/kernel/smpboot.c
===================================================================
--- linux-2.6.git.orig/arch/x86/kernel/smpboot.c	2008-09-18 19:17:16.000000000 +0400
+++ linux-2.6.git/arch/x86/kernel/smpboot.c	2008-09-22 13:30:36.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 - add check if we have ESR register

[Cyrill Gorcunov]

[Cyrill Gorcunov - Mon, Sep 22, 2008 at 01:36:03PM +0400]
| We should touch ESR register if only we have one.
| Check its presence first.
| 
| Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
| CC: Yinghai Lu <yhlu.kernel@gmail.com>
| ---
| 
| Index: linux-2.6.git/arch/x86/kernel/smpboot.c
| ===================================================================
| --- linux-2.6.git.orig/arch/x86/kernel/smpboot.c	2008-09-18 19:17:16.000000000 +0400
| +++ linux-2.6.git/arch/x86/kernel/smpboot.c	2008-09-22 13:30:36.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);
| +		}
|  	}
|  
|  	/*

Actually it's resend of the patch. On previous attempt
Yinghai was proposed to choose:

> one apic_version or boot_cpu_apic_version could be enough

but I think it should be different patch which does cleaning
up variables usage. So for now I think _this_ patch is enough
since idea is to prevent of touching nonexistant register rather
code cleaning (whci could be done later).

Yinghai?

		- Cyrill -

Re: [PATCH] x86: do_boot_cpu - add check if we have ESR register

[Yinghai Lu]

>
> Actually it's resend of the patch. On previous attempt
> Yinghai was proposed to choose:
>
>> one apic_version or boot_cpu_apic_version could be enough
>
> but I think it should be different patch which does cleaning
> up variables usage. So for now I think _this_ patch is enough
> since idea is to prevent of touching nonexistant register rather
> code cleaning (whci could be done later).

do we have systems that have mixed cpu supported with different apic version?

YH

Re: [PATCH] x86: do_boot_cpu - add check if we have ESR register

[Ingo Molnar]

* Yinghai Lu <yhlu.kernel@gmail.com> wrote:

> > Actually it's resend of the patch. On previous attempt Yinghai was 
> > proposed to choose:
> >
> >> one apic_version or boot_cpu_apic_version could be enough
> >
> > but I think it should be different patch which does cleaning up 
> > variables usage. So for now I think _this_ patch is enough since 
> > idea is to prevent of touching nonexistant register rather code 
> > cleaning (whci could be done later).
> 
> do we have systems that have mixed cpu supported with different apic 
> version?

i dont think that ever happened in the past. Vendors have trouble 
keeping same-cpu type systems going ;-)

OTOH, if it's simple to have multi-version support then lets do that 
throughout. (although we certainly cannot test it. Perhaps we can 
provoke it artificially via a hack that artificially degrades one cpu's 
apic version or so.)

	Ingo

Re: [PATCH] x86: do_boot_cpu - add check if we have ESR register

[Cyrill Gorcunov]

[Yinghai Lu - Mon, Sep 22, 2008 at 02:57:17AM -0700]
| >
| > Actually it's resend of the patch. On previous attempt
| > Yinghai was proposed to choose:
| >
| >> one apic_version or boot_cpu_apic_version could be enough
| >
| > but I think it should be different patch which does cleaning
| > up variables usage. So for now I think _this_ patch is enough
| > since idea is to prevent of touching nonexistant register rather
| > code cleaning (whci could be done later).
| 
| do we have systems that have mixed cpu supported with different apic version?
| 
| YH
| 

I don't think it's possible on a single motherboard. For integrated
apics it will require different families of cpu.

		- Cyrill -

Re: [PATCH] x86: do_boot_cpu - add check if we have ESR register

[Cyrill Gorcunov]

[Yinghai Lu - Mon, Sep 22, 2008 at 02:57:17AM -0700]
| >
| > Actually it's resend of the patch. On previous attempt
| > Yinghai was proposed to choose:
| >
| >> one apic_version or boot_cpu_apic_version could be enough
| >
| > but I think it should be different patch which does cleaning
| > up variables usage. So for now I think _this_ patch is enough
| > since idea is to prevent of touching nonexistant register rather
| > code cleaning (whci could be done later).
| 
| do we have systems that have mixed cpu supported with different apic version?
| 
| YH
| 

Yinghai what was the reason of this question? I've a suspicious
you're planning to test apic version early and only _once_ so
eliminating APIC_INTERGRATED call? Am I right?

		- Cyrill -

Re: [PATCH] x86: do_boot_cpu - add check if we have ESR register

[Yinghai Lu]

On Mon, Sep 22, 2008 at 3:15 AM, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
> [Yinghai Lu - Mon, Sep 22, 2008 at 02:57:17AM -0700]
> | >
> | > Actually it's resend of the patch. On previous attempt
> | > Yinghai was proposed to choose:
> | >
> | >> one apic_version or boot_cpu_apic_version could be enough
> | >
> | > but I think it should be different patch which does cleaning
> | > up variables usage. So for now I think _this_ patch is enough
> | > since idea is to prevent of touching nonexistant register rather
> | > code cleaning (whci could be done later).
> |
> | do we have systems that have mixed cpu supported with different apic version?
> |
> | YH
> |
>
> Yinghai what was the reason of this question? I've a suspicious
> you're planning to test apic version early and only _once_ so
> eliminating APIC_INTERGRATED call? Am I right?

want to put more thing in genapic...

YH

Re: [PATCH] x86: do_boot_cpu - add check if we have ESR register

[Cyrill Gorcunov]

[Yinghai Lu - Mon, Sep 22, 2008 at 11:37:21AM -0700]
| On Mon, Sep 22, 2008 at 3:15 AM, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
| > [Yinghai Lu - Mon, Sep 22, 2008 at 02:57:17AM -0700]
| > | >
| > | > Actually it's resend of the patch. On previous attempt
| > | > Yinghai was proposed to choose:
| > | >
| > | >> one apic_version or boot_cpu_apic_version could be enough
| > | >
| > | > but I think it should be different patch which does cleaning
| > | > up variables usage. So for now I think _this_ patch is enough
| > | > since idea is to prevent of touching nonexistant register rather
| > | > code cleaning (whci could be done later).
| > |
| > | do we have systems that have mixed cpu supported with different apic version?
| > |
| > | YH
| > |
| >
| > Yinghai what was the reason of this question? I've a suspicious
| > you're planning to test apic version early and only _once_ so
| > eliminating APIC_INTERGRATED call? Am I right?
| 
| want to put more thing in genapic...
| 
| YH
| 

I think Ingo could drop this patch then at all.

		- Cyrill -