x86, amd: Get multi-node CPU info from NodeId MSR instead of PCI config space

x86, amd: Get multi-node CPU info from NodeId MSR instead of PCI config space

[Andreas Herrmann]

Use newly introduced NodeId MSR to get NodeId and number of nodes per
processor.

Signed-off-by: Andreas Herrmann <andreas.herrmann3@amd.com>
---
 arch/x86/include/asm/cpufeature.h |    1 +
 arch/x86/kernel/cpu/amd.c         |   53 ++++++++++--------------------------
 2 files changed, 16 insertions(+), 38 deletions(-)

Hi,

this is .32 material.
Diffed against .32-rc5.
Please apply.

Thanks,
Andreas


diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 9cfc88b..2cbf0a2 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -153,6 +153,7 @@
 #define X86_FEATURE_SSE5	(6*32+11) /* SSE-5 */
 #define X86_FEATURE_SKINIT	(6*32+12) /* SKINIT/STGI instructions */
 #define X86_FEATURE_WDT		(6*32+13) /* Watchdog timer */
+#define X86_FEATURE_NODEID_MSR	(6*32+19) /* NodeId MSR */
 
 /*
  * Auxiliary flags: Linux defined - For features scattered in various
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index c910a71..85ee6c3 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -254,59 +254,36 @@ static int __cpuinit nearby_node(int apicid)
 
 /*
  * Fixup core topology information for AMD multi-node processors.
- * Assumption 1: Number of cores in each internal node is the same.
- * Assumption 2: Mixed systems with both single-node and dual-node
- *               processors are not supported.
+ * Assumption: Number of cores in each internal node is the same.
  */
 #ifdef CONFIG_X86_HT
 static void __cpuinit amd_fixup_dcm(struct cpuinfo_x86 *c)
 {
-#ifdef CONFIG_PCI
-	u32 t, cpn;
-	u8 n, n_id;
+	unsigned long long value;
+	u32 nodes, cores_per_node;
 	int cpu = smp_processor_id();
 
+	if (!cpu_has(c, X86_FEATURE_NODEID_MSR))
+		return;
+
 	/* fixup topology information only once for a core */
 	if (cpu_has(c, X86_FEATURE_AMD_DCM))
 		return;
 
-	/* check for multi-node processor on boot cpu */
-	t = read_pci_config(0, 24, 3, 0xe8);
-	if (!(t & (1 << 29)))
+	rdmsrl(0xc001100c, value);
+
+	nodes = ((value >> 3) & 7) + 1;
+	if (nodes == 1)
 		return;
 
 	set_cpu_cap(c, X86_FEATURE_AMD_DCM);
+	cores_per_node = c->x86_max_cores / nodes;
 
-	/* cores per node: each internal node has half the number of cores */
-	cpn = c->x86_max_cores >> 1;
+	/* store NodeID, use llc_shared_map to store sibling info */
+	per_cpu(cpu_llc_id, cpu) = value & 7;
 
-	/* even-numbered NB_id of this dual-node processor */
-	n = c->phys_proc_id << 1;
-
-	/*
-	 * determine internal node id and assign cores fifty-fifty to
-	 * each node of the dual-node processor
-	 */
-	t = read_pci_config(0, 24 + n, 3, 0xe8);
-	n = (t>>30) & 0x3;
-	if (n == 0) {
-		if (c->cpu_core_id < cpn)
-			n_id = 0;
-		else
-			n_id = 1;
-	} else {
-		if (c->cpu_core_id < cpn)
-			n_id = 1;
-		else
-			n_id = 0;
-	}
-
-	/* compute entire NodeID, use llc_shared_map to store sibling info */
-	per_cpu(cpu_llc_id, cpu) = (c->phys_proc_id << 1) + n_id;
-
-	/* fixup core id to be in range from 0 to cpn */
-	c->cpu_core_id = c->cpu_core_id % cpn;
-#endif
+	/* fixup core id to be in range from 0 to (cores_per_node - 1) */
+	c->cpu_core_id = c->cpu_core_id % cores_per_node;
 }
 #endif
 
-- 
1.6.5

Re: x86, amd: Get multi-node CPU info from NodeId MSR instead of PCI config space

[Ingo Molnar]

* Andreas Herrmann <herrmann.der.user@googlemail.com> wrote:

> Use newly introduced NodeId MSR to get NodeId and number of nodes per 
> processor.

What will happen on CPUs that dont have this MSR and got this info from 
the PCI config space:

>  static void __cpuinit amd_fixup_dcm(struct cpuinfo_x86 *c)
>  {
> -#ifdef CONFIG_PCI
> -	u32 t, cpn;
> -	u8 n, n_id;
>  	int cpu = smp_processor_id();
>  
>  	/* fixup topology information only once for a core */
>  	if (cpu_has(c, X86_FEATURE_AMD_DCM))
>  		return;
>  
> -	/* check for multi-node processor on boot cpu */
> -	t = read_pci_config(0, 24, 3, 0xe8);
> -	if (!(t & (1 << 29)))
>  		return;
>  
>  	set_cpu_cap(c, X86_FEATURE_AMD_DCM);

are there any such CPUs? I.e. we want to know the effect of this patch 
on various models of AMD CPUs - is the change really .32 safe? Does it 
solve any problem that makes it .32 material versus being for .33?

	Ingo

Re: x86, amd: Get multi-node CPU info from NodeId MSR instead of PCI config space

[Andreas Herrmann]

On Fri, Oct 16, 2009 at 02:38:53PM +0200, Ingo Molnar wrote:
> 
> * Andreas Herrmann <herrmann.der.user@googlemail.com> wrote:
> 
> > Use newly introduced NodeId MSR to get NodeId and number of nodes per 
> > processor.
> 
> What will happen on CPUs that dont have this MSR and got this info from 
> the PCI config space:

No PCI config space needed as all Magny-Cours CPUs should support this
when launched.

> >  static void __cpuinit amd_fixup_dcm(struct cpuinfo_x86 *c)
> >  {
> > -#ifdef CONFIG_PCI
> > -	u32 t, cpn;
> > -	u8 n, n_id;
> >  	int cpu = smp_processor_id();
> >  
> >  	/* fixup topology information only once for a core */
> >  	if (cpu_has(c, X86_FEATURE_AMD_DCM))
> >  		return;
> >  
> > -	/* check for multi-node processor on boot cpu */
> > -	t = read_pci_config(0, 24, 3, 0xe8);
> > -	if (!(t & (1 << 29)))
> >  		return;
> >  
> >  	set_cpu_cap(c, X86_FEATURE_AMD_DCM);
> 
> are there any such CPUs?

Only Magny-Cours so far.

> I.e. we want to know the effect of this patch 
> on various models of AMD CPUs - is the change really .32 safe? Does it 
> solve any problem that makes it .32 material versus being for .33?

IMHO getting rid of the PCI config space accesses as soon as possible
is a benefit, I think.


Andreas

Re: x86, amd: Get multi-node CPU info from NodeId MSR instead of PCI config space

[Ingo Molnar]

* Andreas Herrmann <herrmann.der.user@googlemail.com> wrote:

> > are there any such CPUs?
> 
> Only Magny-Cours so far.
> 
> > I.e. we want to know the effect of this patch on various models of 
> > AMD CPUs - is the change really .32 safe? Does it solve any problem 
> > that makes it .32 material versus being for .33?
> 
> IMHO getting rid of the PCI config space accesses as soon as possible 
> is a benefit, I think.

Agreed.

Is the patch an identity transformation? Or are there CPUs where the 
information from the PCI config space is different from the MSR derived 
one?

	Ingo

Re: x86, amd: Get multi-node CPU info from NodeId MSR instead of PCI config space

[Andreas Herrmann]

On Fri, Oct 16, 2009 at 03:00:00PM +0200, Ingo Molnar wrote:
> 
> * Andreas Herrmann <herrmann.der.user@googlemail.com> wrote:
> 
> > > are there any such CPUs?
> > 
> > Only Magny-Cours so far.
> > 
> > > I.e. we want to know the effect of this patch on various models of 
> > > AMD CPUs - is the change really .32 safe? Does it solve any problem 
> > > that makes it .32 material versus being for .33?
> > 
> > IMHO getting rid of the PCI config space accesses as soon as possible 
> > is a benefit, I think.
> 
> Agreed.
> 
> Is the patch an identity transformation? Or are there CPUs where the 
> information from the PCI config space is different from the MSR derived 
> one?

On production hardware it's an identity transformation.


Regards,
Andreas

[PATCH RESEND] x86, amd: Get multi-node CPU info from NodeId MSR instead of PCI config space

[Andreas Herrmann]

Use newly introduced NodeId MSR to get NodeId and number of nodes per
processor. Production systems will support this from scratch. Thus the
PCI config space accesses are superfluous and should be removed asap.

Signed-off-by: Andreas Herrmann <andreas.herrmann3@amd.com>
---
 arch/x86/include/asm/cpufeature.h |    1 +
 arch/x86/kernel/cpu/amd.c         |   53 ++++++++++--------------------------
 2 files changed, 16 insertions(+), 38 deletions(-)

Please apply for .32.

Thanks,
Andreas


diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index 9cfc88b..2cbf0a2 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -153,6 +153,7 @@
 #define X86_FEATURE_SSE5	(6*32+11) /* SSE-5 */
 #define X86_FEATURE_SKINIT	(6*32+12) /* SKINIT/STGI instructions */
 #define X86_FEATURE_WDT		(6*32+13) /* Watchdog timer */
+#define X86_FEATURE_NODEID_MSR	(6*32+19) /* NodeId MSR */
 
 /*
  * Auxiliary flags: Linux defined - For features scattered in various
diff --git a/arch/x86/kernel/cpu/amd.c b/arch/x86/kernel/cpu/amd.c
index c910a71..3f56759 100644
--- a/arch/x86/kernel/cpu/amd.c
+++ b/arch/x86/kernel/cpu/amd.c
@@ -254,59 +254,36 @@ static int __cpuinit nearby_node(int apicid)
 
 /*
  * Fixup core topology information for AMD multi-node processors.
- * Assumption 1: Number of cores in each internal node is the same.
- * Assumption 2: Mixed systems with both single-node and dual-node
- *               processors are not supported.
+ * Assumption: Number of cores in each internal node is the same.
  */
 #ifdef CONFIG_X86_HT
 static void __cpuinit amd_fixup_dcm(struct cpuinfo_x86 *c)
 {
-#ifdef CONFIG_PCI
-	u32 t, cpn;
-	u8 n, n_id;
+	unsigned long long value;
+	u32 nodes, cores_per_node; 
 	int cpu = smp_processor_id();
 
+	if (!cpu_has(c, X86_FEATURE_NODEID_MSR))
+		return;
+
 	/* fixup topology information only once for a core */
 	if (cpu_has(c, X86_FEATURE_AMD_DCM))
 		return;
 
-	/* check for multi-node processor on boot cpu */
-	t = read_pci_config(0, 24, 3, 0xe8);
-	if (!(t & (1 << 29)))
+	rdmsrl(0xc001100c, value);
+
+	nodes = ((value >> 3) & 7) + 1;
+	if (nodes == 1)
 		return;
 
 	set_cpu_cap(c, X86_FEATURE_AMD_DCM);
+	cores_per_node = c->x86_max_cores / nodes;
 
-	/* cores per node: each internal node has half the number of cores */
-	cpn = c->x86_max_cores >> 1;
+	/* store NodeID, use llc_shared_map to store sibling info */
+	per_cpu(cpu_llc_id, cpu) = value & 7;
 
-	/* even-numbered NB_id of this dual-node processor */
-	n = c->phys_proc_id << 1;
-
-	/*
-	 * determine internal node id and assign cores fifty-fifty to
-	 * each node of the dual-node processor
-	 */
-	t = read_pci_config(0, 24 + n, 3, 0xe8);
-	n = (t>>30) & 0x3;
-	if (n == 0) {
-		if (c->cpu_core_id < cpn)
-			n_id = 0;
-		else
-			n_id = 1;
-	} else {
-		if (c->cpu_core_id < cpn)
-			n_id = 1;
-		else
-			n_id = 0;
-	}
-
-	/* compute entire NodeID, use llc_shared_map to store sibling info */
-	per_cpu(cpu_llc_id, cpu) = (c->phys_proc_id << 1) + n_id;
-
-	/* fixup core id to be in range from 0 to cpn */
-	c->cpu_core_id = c->cpu_core_id % cpn;
-#endif
+	/* fixup core id to be in range from 0 to (cores_per_node - 1) */
+	c->cpu_core_id = c->cpu_core_id % cores_per_node;
 }
 #endif
 
-- 
1.6.0.4