2008-06-12 10:29:57

by Thomas Gleixner

[permalink] [raw]
Subject: [patch 3/6] x86: use cpuinfo to check for interrupt pending message msr

No need to do a cpuid(1) again. The cpuinfo structure has all
necessary information already.

Signed-off-by: Thomas Gleixner <[email protected]>
Cc: Andreas Herrmann <[email protected]>

---
arch/x86/kernel/cpu/amd.c | 41 +++++++++++++++--------------------------
arch/x86/kernel/setup_64.c | 38 +++++++++++++++-----------------------
2 files changed, 30 insertions(+), 49 deletions(-)

Index: linux-2.6/arch/x86/kernel/cpu/amd.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/cpu/amd.c
+++ linux-2.6/arch/x86/kernel/cpu/amd.c
@@ -25,35 +25,24 @@ extern void vide(void);
__asm__(".align 4\nvide: ret");

#ifdef CONFIG_X86_LOCAL_APIC
-#define CPUID_PROCESSOR_SIGNATURE 1
-#define CPUID_XFAM 0x0ff00000
-#define CPUID_XFAM_K8 0x00000000
-#define CPUID_XFAM_10H 0x00100000
-#define CPUID_XFAM_11H 0x00200000
-#define CPUID_XMOD 0x000f0000
-#define CPUID_XMOD_REV_F 0x00040000

/* AMD systems with C1E don't have a working lAPIC timer. Check for that. */
-static __cpuinit int amd_apic_timer_broken(void)
+static __cpuinit int amd_apic_timer_broken(struct cpuinfo_x86 *c)
{
u32 lo, hi;
- u32 eax = cpuid_eax(CPUID_PROCESSOR_SIGNATURE);
- switch (eax & CPUID_XFAM) {
- case CPUID_XFAM_K8:
- if ((eax & CPUID_XMOD) < CPUID_XMOD_REV_F)
- break;
- case CPUID_XFAM_10H:
- case CPUID_XFAM_11H:
- rdmsr(MSR_K8_INT_PENDING_MSG, lo, hi);
- if (lo & K8_INTP_C1E_ACTIVE_MASK) {
- if (smp_processor_id() != boot_cpu_physical_apicid)
- printk(KERN_INFO "AMD C1E detected late. "
- " Force timer broadcast.\n");
- return 1;
- }
- break;
- default:
- /* err on the side of caution */
+
+ if (c->x86 < 0x0F)
+ return 0;
+
+ /* Family 0x0f models < rev F do not have this MSR */
+ if (c->x86 == 0x0f && c->x86_model < 0x40)
+ return 0;
+
+ rdmsr(MSR_K8_INT_PENDING_MSG, lo, hi);
+ if (lo & K8_INTP_C1E_ACTIVE_MASK) {
+ if (smp_processor_id() != boot_cpu_physical_apicid)
+ printk(KERN_INFO "AMD C1E detected late. "
+ "Force timer broadcast.\n");
return 1;
}
return 0;
@@ -297,7 +286,7 @@ static void __cpuinit init_amd(struct cp
}

#ifdef CONFIG_X86_LOCAL_APIC
- if (amd_apic_timer_broken())
+ if (amd_apic_timer_broken(c))
local_apic_timer_disabled = 1;
#endif

Index: linux-2.6/arch/x86/kernel/setup_64.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/setup_64.c
+++ linux-2.6/arch/x86/kernel/setup_64.c
@@ -682,31 +682,23 @@ static void __cpuinit early_init_amd_mc(
#endif
}

-#define CPUID_PROCESSOR_SIGNATURE 1
-#define CPUID_XFAM 0x0ff00000
-#define CPUID_XFAM_K8 0x00000000
-#define CPUID_XFAM_10H 0x00100000
-#define CPUID_XFAM_11H 0x00200000
-#define CPUID_XMOD 0x000f0000
-#define CPUID_XMOD_REV_F 0x00040000
-
/* AMD systems with C1E don't have a working lAPIC timer. Check for that. */
-static __cpuinit int amd_apic_timer_broken(void)
+static __cpuinit int amd_apic_timer_broken(struct cpuinfo_x86 *c)
{
- u32 lo, hi, eax = cpuid_eax(CPUID_PROCESSOR_SIGNATURE);
+ u32 lo, hi;

- switch (eax & CPUID_XFAM) {
- case CPUID_XFAM_K8:
- if ((eax & CPUID_XMOD) < CPUID_XMOD_REV_F)
- break;
- case CPUID_XFAM_10H:
- case CPUID_XFAM_11H:
- rdmsr(MSR_K8_INT_PENDING_MSG, lo, hi);
- if (lo & K8_INTP_C1E_ACTIVE_MASK)
- return 1;
- break;
- default:
- /* err on the side of caution */
+ if (c->x86 < 0x0F)
+ return 0;
+
+ /* Family 0x0f models < rev F do not have this MSR */
+ if (c->x86 == 0x0f && c->x86_model < 0x40)
+ return 0;
+
+ rdmsr(MSR_K8_INT_PENDING_MSG, lo, hi);
+ if (lo & K8_INTP_C1E_ACTIVE_MASK) {
+ if (smp_processor_id() != boot_cpu_physical_apicid)
+ printk(KERN_INFO "AMD C1E detected late. "
+ "Force timer broadcast.\n");
return 1;
}
return 0;
@@ -789,7 +781,7 @@ static void __cpuinit init_amd(struct cp
if (c->x86 == 0x10)
fam10h_check_enable_mmcfg();

- if (amd_apic_timer_broken())
+ if (amd_apic_timer_broken(c))
disable_apic_timer = 1;

if (c == &boot_cpu_data && c->x86 >= 0xf && c->x86 <= 0x11) {

--


Subject: Re: [patch 3/6] x86: use cpuinfo to check for interrupt pending message msr

On Thu, Jun 12, 2008 at 10:28:47AM -0000, Thomas Gleixner wrote:
> No need to do a cpuid(1) again. The cpuinfo structure has all
> necessary information already.
>
> Signed-off-by: Thomas Gleixner <[email protected]>
> Cc: Andreas Herrmann <[email protected]>
>
> ---
> arch/x86/kernel/cpu/amd.c | 41 +++++++++++++++--------------------------
> arch/x86/kernel/setup_64.c | 38 +++++++++++++++-----------------------
> 2 files changed, 30 insertions(+), 49 deletions(-)
>
> Index: linux-2.6/arch/x86/kernel/cpu/amd.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/cpu/amd.c
> +++ linux-2.6/arch/x86/kernel/cpu/amd.c
> @@ -25,35 +25,24 @@ extern void vide(void);
> __asm__(".align 4\nvide: ret");
>
> #ifdef CONFIG_X86_LOCAL_APIC
> -#define CPUID_PROCESSOR_SIGNATURE 1
> -#define CPUID_XFAM 0x0ff00000
> -#define CPUID_XFAM_K8 0x00000000
> -#define CPUID_XFAM_10H 0x00100000
> -#define CPUID_XFAM_11H 0x00200000
> -#define CPUID_XMOD 0x000f0000
> -#define CPUID_XMOD_REV_F 0x00040000
>
> /* AMD systems with C1E don't have a working lAPIC timer. Check for that. */
> -static __cpuinit int amd_apic_timer_broken(void)
> +static __cpuinit int amd_apic_timer_broken(struct cpuinfo_x86 *c)
> {
> u32 lo, hi;
> - u32 eax = cpuid_eax(CPUID_PROCESSOR_SIGNATURE);
> - switch (eax & CPUID_XFAM) {
> - case CPUID_XFAM_K8:
> - if ((eax & CPUID_XMOD) < CPUID_XMOD_REV_F)
> - break;
> - case CPUID_XFAM_10H:
> - case CPUID_XFAM_11H:
> - rdmsr(MSR_K8_INT_PENDING_MSG, lo, hi);
> - if (lo & K8_INTP_C1E_ACTIVE_MASK) {
> - if (smp_processor_id() != boot_cpu_physical_apicid)
> - printk(KERN_INFO "AMD C1E detected late. "
> - " Force timer broadcast.\n");
> - return 1;
> - }
> - break;
> - default:
> - /* err on the side of caution */
> +
> + if (c->x86 < 0x0F)
> + return 0;
> +
> + /* Family 0x0f models < rev F do not have this MSR */
> + if (c->x86 == 0x0f && c->x86_model < 0x40)
> + return 0;

Just some minor nitpicking.
Older AMD family 0xf CPUs have this Interrupt Pending Message
Register. But they do not support C1E and thus bits 27 and 28 of this
MSR are reserved.


Andreas

2008-06-13 12:38:59

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch 3/6] x86: use cpuinfo to check for interrupt pending message msr

On Fri, 13 Jun 2008, Andreas Herrmann wrote:
> > + /* Family 0x0f models < rev F do not have this MSR */
> > + if (c->x86 == 0x0f && c->x86_model < 0x40)
> > + return 0;
>
> Just some minor nitpicking.
> Older AMD family 0xf CPUs have this Interrupt Pending Message
> Register. But they do not support C1E and thus bits 27 and 28 of this
> MSR are reserved.

So the check can be simplified to always check the MSR for all
family >= 0x0f CPUs ?

Thanks,

tglx

Subject: Re: [patch 3/6] x86: use cpuinfo to check for interrupt pending message msr

On Fri, Jun 13, 2008 at 02:38:30PM +0200, Thomas Gleixner wrote:
> On Fri, 13 Jun 2008, Andreas Herrmann wrote:
> > > + /* Family 0x0f models < rev F do not have this MSR */
> > > + if (c->x86 == 0x0f && c->x86_model < 0x40)
> > > + return 0;
> >
> > Just some minor nitpicking.
> > Older AMD family 0xf CPUs have this Interrupt Pending Message
> > Register. But they do not support C1E and thus bits 27 and 28 of this
> > MSR are reserved.
>
> So the check can be simplified to always check the MSR for all
> family >= 0x0f CPUs ?

First of all I thought of changing the comment.

But now that you ask:
Documentation for older K8 CPUs says that reserved bits in that MSR
are "Read as Zero". But otherwise it also says "Software must not
depend on the state of a reserved field ..."

Maybe I am a little paranoid but I would keep the model check.


Andreas