All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC][PATCH] AMD CPU core topology detection
@ 2011-01-06 16:47 Wei Huang
  2011-01-07  8:56 ` Jan Beulich
  0 siblings, 1 reply; 9+ messages in thread
From: Wei Huang @ 2011-01-06 16:47 UTC (permalink / raw)
  To: 'xen-devel@lists.xensource.com', keir, Jan Beulich

[-- Attachment #1: Type: text/plain, Size: 517 bytes --]

Hi Keir and Jan,

This patch is to support the new core pair topology introduced by AMD 
new CPUs, which introduces a new concept of core/compute unit/node. 
There is a new feature bit for topology extension in CPUID:0x80000001. 
Also a new CPUID (0x8000001E) is introduced for CPU topology 
enumeration. This patch collects the sibling information from a new 
CPUID and the info will be stored into the sibling map in Xen.

Please help review it. Comments are welcome.

Signed-off-by: Wei Huang <wei.huang2@amd.com>

[-- Attachment #2: xen_amd_core_topology.txt --]
[-- Type: text/plain, Size: 3172 bytes --]

diff -r 94c5d7ee424a -r f8e36046ee17 xen/arch/x86/cpu/amd.c
--- a/xen/arch/x86/cpu/amd.c	Wed Jan 05 13:58:21 2011 -0600
+++ b/xen/arch/x86/cpu/amd.c	Wed Jan 05 16:48:01 2011 -0600
@@ -337,6 +337,56 @@
 		on_each_cpu(disable_c1e, NULL, 1);
 }
 
+#ifdef CONFIG_X86_HT
+/* This function detects system CPU topology */
+static void amd_detect_topology(struct cpuinfo_x86 *c)
+{
+	u32 eax, ebx, ecx, edx;
+	int cpu = smp_processor_id();
+
+	if (c->x86_max_cores <= 1)
+		return;
+
+	if (cpu_has(c, X86_FEATURE_TOPO_EXT)) {
+		/* AMD new CPUs introduce a new term called compute unit. But 
+		 * we still keep the names of existing variables for the 
+		 * purpose of consistency. Keep in mind that cpu_core_id here 
+		 * represents the computer unit ID; and we use the node ID as 
+                 * the processor ID because it is unique across the whole 
+		 * system. 
+		 */
+		cpuid(0x8000001e, &eax, &ebx, &ecx, &edx);
+		cpu_core_id[cpu] = ebx & 0xFF;
+		phys_proc_id[cpu] = ecx & 0xFF;
+
+		c->x86_num_siblings = ((ebx >> 8) & 0x3) + 1;
+
+		if (opt_cpu_info)
+			printk("CPU %d(%d) -> Compute Unit %d, Node %d\n",
+			       cpu, c->x86_max_cores, cpu_core_id[cpu],
+			       phys_proc_id[cpu]);
+	} else {
+		/* On a AMD multi core setup the lower bits of the APIC id
+		 * distingush the cores.
+		 */
+		unsigned bits = (cpuid_ecx(0x80000008) >> 12) & 0xf;
+
+		if (bits == 0) {
+			while ((1 << bits) < c->x86_max_cores)
+				bits++;
+		}
+		cpu_core_id[cpu] = phys_proc_id[cpu] & ((1<<bits)-1);
+		phys_proc_id[cpu] >>= bits;
+
+		if (opt_cpu_info)
+			printk("CPU %d(%d) -> Core %d\n",
+			       cpu, c->x86_max_cores, cpu_core_id[cpu]);
+	}
+}
+#else
+#define amd_detect_topology(struct cpuinfo_x86 *c) (void(0))
+#endif
+
 static void __devinit init_amd(struct cpuinfo_x86 *c)
 {
 	u32 l, h;
@@ -574,26 +624,7 @@
 		}
 	}
 
-#ifdef CONFIG_X86_HT
-	/*
-	 * On a AMD multi core setup the lower bits of the APIC id
-	 * distingush the cores.
-	 */
-	if (c->x86_max_cores > 1) {
-		int cpu = smp_processor_id();
-		unsigned bits = (cpuid_ecx(0x80000008) >> 12) & 0xf;
-
-		if (bits == 0) {
-			while ((1 << bits) < c->x86_max_cores)
-				bits++;
-		}
-		cpu_core_id[cpu] = phys_proc_id[cpu] & ((1<<bits)-1);
-		phys_proc_id[cpu] >>= bits;
-		if (opt_cpu_info)
-			printk("CPU %d(%d) -> Core %d\n",
-			       cpu, c->x86_max_cores, cpu_core_id[cpu]);
-	}
-#endif
+	amd_detect_topology(c);
 
 	/* Pointless to use MWAIT on Family10 as it does not deep sleep. */
 	if (c->x86 >= 0x10 && !force_mwait)
diff -r 94c5d7ee424a -r f8e36046ee17 xen/include/asm-x86/cpufeature.h
--- a/xen/include/asm-x86/cpufeature.h	Wed Jan 05 13:58:21 2011 -0600
+++ b/xen/include/asm-x86/cpufeature.h	Wed Jan 05 16:48:01 2011 -0600
@@ -132,6 +132,7 @@
 #define X86_FEATURE_SSE5	(6*32+ 11) /* AMD Streaming SIMD Extensions-5 */
 #define X86_FEATURE_SKINIT	(6*32+ 12) /* SKINIT, STGI/CLGI, DEV */
 #define X86_FEATURE_WDT		(6*32+ 13) /* Watchdog Timer */
+#define X86_FEATURE_TOPO_EXT    (6*32+ 22) /* Topology Extension Support */
 
 /* Intel-defined CPU features, CPUID level 0x00000007:0 (ebx), word 9 */
 #define X86_FEATURE_FSGSBASE	(7*32+ 0) /* {RD,WR}{FS,GS}BASE instructions */

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC][PATCH] AMD CPU core topology detection
  2011-01-06 16:47 [RFC][PATCH] AMD CPU core topology detection Wei Huang
@ 2011-01-07  8:56 ` Jan Beulich
  2011-01-07 15:52   ` Huang2, Wei
  0 siblings, 1 reply; 9+ messages in thread
From: Jan Beulich @ 2011-01-07  8:56 UTC (permalink / raw)
  To: Wei Huang; +Cc: xen-devel, keir

>>> On 06.01.11 at 17:47, Wei Huang <wei.huang2@amd.com> wrote:
>+#ifdef CONFIG_X86_HT
>+/* This function detects system CPU topology */
>+static void amd_detect_topology(struct cpuinfo_x86 *c)
>+{
>+	u32 eax, ebx, ecx, edx;
>+	int cpu = smp_processor_id();
>+
>+	if (c->x86_max_cores <= 1)
>+		return;
>+
>+	if (cpu_has(c, X86_FEATURE_TOPO_EXT)) {
>+		/* AMD new CPUs introduce a new term called compute unit. But 
>+		 * we still keep the names of existing variables for the 
>+		 * purpose of consistency. Keep in mind that cpu_core_id here 
>+		 * represents the computer unit ID; and we use the node ID as 
>+                 * the processor ID because it is unique across the whole 
>+		 * system. 
>+		 */
>+		cpuid(0x8000001e, &eax, &ebx, &ecx, &edx);
>+		cpu_core_id[cpu] = ebx & 0xFF;
>+		phys_proc_id[cpu] = ecx & 0xFF;
>+
>+		c->x86_num_siblings = ((ebx >> 8) & 0x3) + 1;

Did you check the consequences of re-using these for other than
their original purpose? I'm particularly wondering whether the code
in xen/arch/x86/cpu/mcheck/mce.c won't need updating, but that
isn't the only place that needs looking at.

If indeed your intention was to superimpose the new AMD topology
onto the existing one (partly other than Linux, which added a new
field to struct cpuinfo_x86, but uses cpu_{sibling,core}_map like
what results with your patch), won't there be consequences on (at
least) the credit scheduler (as you may not want cores to be
treated like threads in _csched_cpu_pick())?

>@@ -132,6 +132,7 @@
> #define X86_FEATURE_SSE5	(6*32+ 11) /* AMD Streaming SIMD Extensions-5 */
> #define X86_FEATURE_SKINIT	(6*32+ 12) /* SKINIT, STGI/CLGI, DEV */
> #define X86_FEATURE_WDT		(6*32+ 13) /* Watchdog Timer */
>+#define X86_FEATURE_TOPO_EXT    (6*32+ 22) /* Topology Extension Support */

Would be nice to use the same name as Linux does (i.e. without
the last underscore).

Jan

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [RFC][PATCH] AMD CPU core topology detection
  2011-01-07  8:56 ` Jan Beulich
@ 2011-01-07 15:52   ` Huang2, Wei
  2011-01-07 22:56     ` Huang2, Wei
  2011-01-11 10:58     ` George Dunlap
  0 siblings, 2 replies; 9+ messages in thread
From: Huang2, Wei @ 2011-01-07 15:52 UTC (permalink / raw)
  To: Jan Beulich; +Cc: xen-devel, keir

Hi Jan,

Thanks for your comments. Please my answers below.

-Wei
-----Original Message-----
From: Jan Beulich [mailto:JBeulich@novell.com] 
Sent: Friday, January 07, 2011 2:57 AM
To: Huang2, Wei
Cc: xen-devel@lists.xensource.com; keir@xen.org
Subject: Re: [RFC][PATCH] AMD CPU core topology detection

>>> On 06.01.11 at 17:47, Wei Huang <wei.huang2@amd.com> wrote:
>+#ifdef CONFIG_X86_HT
>+/* This function detects system CPU topology */
>+static void amd_detect_topology(struct cpuinfo_x86 *c)
>+{
>+	u32 eax, ebx, ecx, edx;
>+	int cpu = smp_processor_id();
>+
>+	if (c->x86_max_cores <= 1)
>+		return;
>+
>+	if (cpu_has(c, X86_FEATURE_TOPO_EXT)) {
>+		/* AMD new CPUs introduce a new term called compute unit. But 
>+		 * we still keep the names of existing variables for the 
>+		 * purpose of consistency. Keep in mind that cpu_core_id here 
>+		 * represents the computer unit ID; and we use the node ID as 
>+                 * the processor ID because it is unique across the whole 
>+		 * system. 
>+		 */
>+		cpuid(0x8000001e, &eax, &ebx, &ecx, &edx);
>+		cpu_core_id[cpu] = ebx & 0xFF;
>+		phys_proc_id[cpu] = ecx & 0xFF;
>+
>+		c->x86_num_siblings = ((ebx >> 8) & 0x3) + 1;

Did you check the consequences of re-using these for other than
their original purpose? I'm particularly wondering whether the code
in xen/arch/x86/cpu/mcheck/mce.c won't need updating, but that
isn't the only place that needs looking at.

[From my understanding, cpu_core_id and phys_proc_id are collected during boot (mostly in smpboot.c and under cpu/ directory) for sibling map. The sibling info is used for scheduler later on. Old AMD CPUs don't have HyperThreading, so the cpu_sibling_map isn't so useful. New CPUs will have core/compute unit/node. Using Intel's HT as an analogy, we have the following relationship: core=>hyper-thread, compute unit=>core, node=>processor). From that perspective, the change is reasonable. But I might have missed other parts. 

I will check mce implementation. This is a good point.]

If indeed your intention was to superimpose the new AMD topology
onto the existing one (partly other than Linux, which added a new
field to struct cpuinfo_x86, but uses cpu_{sibling,core}_map like
what results with your patch), won't there be consequences on (at
least) the credit scheduler (as you may not want cores to be
treated like threads in _csched_cpu_pick())?

[This is a tricky question. Based on my comments above, Bulldozer core can be treated as thread because it shares FPU with other cores. But different from HT, it also has many dedicated resources (int scheduler, pipeline and L1 cache). The scheduler needs more information in order to tell VCPUs apart on-the-fly and schedule them accordingly. Maybe George can take this into consideration for his credit2 scheduler (or beyond).

I will take a look at Linux's implementation. If you have new ideas, I would appreciate them too.
]

>@@ -132,6 +132,7 @@
> #define X86_FEATURE_SSE5	(6*32+ 11) /* AMD Streaming SIMD Extensions-5 */
> #define X86_FEATURE_SKINIT	(6*32+ 12) /* SKINIT, STGI/CLGI, DEV */
> #define X86_FEATURE_WDT		(6*32+ 13) /* Watchdog Timer */
>+#define X86_FEATURE_TOPO_EXT    (6*32+ 22) /* Topology Extension Support */

Would be nice to use the same name as Linux does (i.e. without
the last underscore).

[I can surely do so.]

Jan

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [RFC][PATCH] AMD CPU core topology detection
  2011-01-07 15:52   ` Huang2, Wei
@ 2011-01-07 22:56     ` Huang2, Wei
  2011-01-10  8:20       ` Jan Beulich
  2011-01-11 10:58     ` George Dunlap
  1 sibling, 1 reply; 9+ messages in thread
From: Huang2, Wei @ 2011-01-07 22:56 UTC (permalink / raw)
  To: Huang2, Wei, Jan Beulich; +Cc: xen-devel, keir

Hi Jan,

Looking at kernel 2.6.37, I think its implementation is very similar to what I did. Here is a code snip from smpboot.c file. The kernel treats compute_unit_id and cpu_core_id equally. 

If you don't like the idea of piggybacking on existing cpu_core_id, I can add a compute_unit_id. Also we might want to move phys_proc_id and cpu_core_id to cpuinfo_x86?

Thanks,
-Wei

        if (smp_num_siblings > 1) {
                for_each_cpu(i, cpu_sibling_setup_mask) {
                        struct cpuinfo_x86 *o = &cpu_data(i);

                        if (cpu_has(c, X86_FEATURE_TOPOEXT)) {
                                if (c->phys_proc_id == o->phys_proc_id &&
                                    c->compute_unit_id == o->compute_unit_id)
                                        link_thread_siblings(cpu, i);
                        } else if (c->phys_proc_id == o->phys_proc_id &&
                                   c->cpu_core_id == o->cpu_core_id) {
                                link_thread_siblings(cpu, i);
                        }
                }
        } else {
                cpumask_set_cpu(cpu, cpu_sibling_mask(cpu));
        }



-----Original Message-----
From: xen-devel-bounces@lists.xensource.com [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Huang2, Wei
Sent: Friday, January 07, 2011 9:52 AM
To: Jan Beulich
Cc: xen-devel@lists.xensource.com; keir@xen.org
Subject: [Xen-devel] RE: [RFC][PATCH] AMD CPU core topology detection

Hi Jan,

Thanks for your comments. Please my answers below.

-Wei
-----Original Message-----
From: Jan Beulich [mailto:JBeulich@novell.com] 
Sent: Friday, January 07, 2011 2:57 AM
To: Huang2, Wei
Cc: xen-devel@lists.xensource.com; keir@xen.org
Subject: Re: [RFC][PATCH] AMD CPU core topology detection

>>> On 06.01.11 at 17:47, Wei Huang <wei.huang2@amd.com> wrote:
>+#ifdef CONFIG_X86_HT
>+/* This function detects system CPU topology */
>+static void amd_detect_topology(struct cpuinfo_x86 *c)
>+{
>+	u32 eax, ebx, ecx, edx;
>+	int cpu = smp_processor_id();
>+
>+	if (c->x86_max_cores <= 1)
>+		return;
>+
>+	if (cpu_has(c, X86_FEATURE_TOPO_EXT)) {
>+		/* AMD new CPUs introduce a new term called compute unit. But 
>+		 * we still keep the names of existing variables for the 
>+		 * purpose of consistency. Keep in mind that cpu_core_id here 
>+		 * represents the computer unit ID; and we use the node ID as 
>+                 * the processor ID because it is unique across the whole 
>+		 * system. 
>+		 */
>+		cpuid(0x8000001e, &eax, &ebx, &ecx, &edx);
>+		cpu_core_id[cpu] = ebx & 0xFF;
>+		phys_proc_id[cpu] = ecx & 0xFF;
>+
>+		c->x86_num_siblings = ((ebx >> 8) & 0x3) + 1;

Did you check the consequences of re-using these for other than
their original purpose? I'm particularly wondering whether the code
in xen/arch/x86/cpu/mcheck/mce.c won't need updating, but that
isn't the only place that needs looking at.

[From my understanding, cpu_core_id and phys_proc_id are collected during boot (mostly in smpboot.c and under cpu/ directory) for sibling map. The sibling info is used for scheduler later on. Old AMD CPUs don't have HyperThreading, so the cpu_sibling_map isn't so useful. New CPUs will have core/compute unit/node. Using Intel's HT as an analogy, we have the following relationship: core=>hyper-thread, compute unit=>core, node=>processor). From that perspective, the change is reasonable. But I might have missed other parts. 

I will check mce implementation. This is a good point.]

If indeed your intention was to superimpose the new AMD topology
onto the existing one (partly other than Linux, which added a new
field to struct cpuinfo_x86, but uses cpu_{sibling,core}_map like
what results with your patch), won't there be consequences on (at
least) the credit scheduler (as you may not want cores to be
treated like threads in _csched_cpu_pick())?

[This is a tricky question. Based on my comments above, Bulldozer core can be treated as thread because it shares FPU with other cores. But different from HT, it also has many dedicated resources (int scheduler, pipeline and L1 cache). The scheduler needs more information in order to tell VCPUs apart on-the-fly and schedule them accordingly. Maybe George can take this into consideration for his credit2 scheduler (or beyond).

I will take a look at Linux's implementation. If you have new ideas, I would appreciate them too.
]

>@@ -132,6 +132,7 @@
> #define X86_FEATURE_SSE5	(6*32+ 11) /* AMD Streaming SIMD Extensions-5 */
> #define X86_FEATURE_SKINIT	(6*32+ 12) /* SKINIT, STGI/CLGI, DEV */
> #define X86_FEATURE_WDT		(6*32+ 13) /* Watchdog Timer */
>+#define X86_FEATURE_TOPO_EXT    (6*32+ 22) /* Topology Extension Support */

Would be nice to use the same name as Linux does (i.e. without
the last underscore).

[I can surely do so.]

Jan




_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [RFC][PATCH] AMD CPU core topology detection
  2011-01-07 22:56     ` Huang2, Wei
@ 2011-01-10  8:20       ` Jan Beulich
  0 siblings, 0 replies; 9+ messages in thread
From: Jan Beulich @ 2011-01-10  8:20 UTC (permalink / raw)
  To: Wei Huang2; +Cc: xen-devel, keir

>>> On 07.01.11 at 23:56, "Huang2, Wei" <Wei.Huang2@amd.com> wrote:
> Looking at kernel 2.6.37, I think its implementation is very similar to what 
> I did. Here is a code snip from smpboot.c file. The kernel treats 
> compute_unit_id and cpu_core_id equally. 

Yes, cpu_{core,sibling}_map are being re-used (as I indicated earlier).

> If you don't like the idea of piggybacking on existing cpu_core_id, I can 
> add a compute_unit_id. Also we might want to move phys_proc_id and 
> cpu_core_id to cpuinfo_x86?

That both seems a good idea to me.

Jan

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: RE: [RFC][PATCH] AMD CPU core topology detection
  2011-01-07 15:52   ` Huang2, Wei
  2011-01-07 22:56     ` Huang2, Wei
@ 2011-01-11 10:58     ` George Dunlap
  2011-01-11 11:12       ` George Dunlap
  2011-01-11 15:38       ` Huang2, Wei
  1 sibling, 2 replies; 9+ messages in thread
From: George Dunlap @ 2011-01-11 10:58 UTC (permalink / raw)
  To: Huang2, Wei; +Cc: xen-devel, keir, Jan Beulich

On Fri, Jan 7, 2011 at 3:52 PM, Huang2, Wei <Wei.Huang2@amd.com> wrote:
> [From my understanding, cpu_core_id and phys_proc_id are collected during boot (mostly in smpboot.c and under cpu/ directory) for sibling map. The sibling info is used for scheduler later on. Old AMD CPUs don't have HyperThreading, so the cpu_sibling_map isn't so useful. New CPUs will have core/compute unit/node. Using Intel's HT as an analogy, we have the following relationship: core=>hyper-thread, compute unit=>core, node=>processor). From that perspective, the change is reasonable. But I might have missed other parts.

Wei, can you point me to some documentation about exactly what the
"compute unit" is?

>
> I will check mce implementation. This is a good point.]
>
> If indeed your intention was to superimpose the new AMD topology
> onto the existing one (partly other than Linux, which added a new
> field to struct cpuinfo_x86, but uses cpu_{sibling,core}_map like
> what results with your patch), won't there be consequences on (at
> least) the credit scheduler (as you may not want cores to be
> treated like threads in _csched_cpu_pick())?
>
> [This is a tricky question. Based on my comments above, Bulldozer core can be treated as thread because it shares FPU with other cores. But different from HT, it also has many dedicated resources (int scheduler, pipeline and L1 cache). The scheduler needs more information in order to tell VCPUs apart on-the-fly and schedule them accordingly. Maybe George can take this into consideration for his credit2 scheduler (or beyond).
>
> I will take a look at Linux's implementation. If you have new ideas, I would appreciate them too.
> ]
>
>>@@ -132,6 +132,7 @@
>> #define X86_FEATURE_SSE5      (6*32+ 11) /* AMD Streaming SIMD Extensions-5 */
>> #define X86_FEATURE_SKINIT    (6*32+ 12) /* SKINIT, STGI/CLGI, DEV */
>> #define X86_FEATURE_WDT               (6*32+ 13) /* Watchdog Timer */
>>+#define X86_FEATURE_TOPO_EXT    (6*32+ 22) /* Topology Extension Support */
>
> Would be nice to use the same name as Linux does (i.e. without
> the last underscore).
>
> [I can surely do so.]
>
> Jan
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: RE: [RFC][PATCH] AMD CPU core topology detection
  2011-01-11 10:58     ` George Dunlap
@ 2011-01-11 11:12       ` George Dunlap
  2011-01-11 17:05         ` Wei Huang
  2011-01-11 15:38       ` Huang2, Wei
  1 sibling, 1 reply; 9+ messages in thread
From: George Dunlap @ 2011-01-11 11:12 UTC (permalink / raw)
  To: Huang2, Wei; +Cc: xen-devel, keir, Jan Beulich

On Tue, Jan 11, 2011 at 10:58 AM, George Dunlap <dunlapg@umich.edu> wrote:
> On Fri, Jan 7, 2011 at 3:52 PM, Huang2, Wei <Wei.Huang2@amd.com> wrote:
>> [From my understanding, cpu_core_id and phys_proc_id are collected during boot (mostly in smpboot.c and under cpu/ directory) for sibling map. The sibling info is used for scheduler later on. Old AMD CPUs don't have HyperThreading, so the cpu_sibling_map isn't so useful. New CPUs will have core/compute unit/node. Using Intel's HT as an analogy, we have the following relationship: core=>hyper-thread, compute unit=>core, node=>processor). >From that perspective, the change is reasonable. But I might have missed other parts.
>
> Wei, can you point me to some documentation about exactly what the
> "compute unit" is?

[Sorry, accidentally sent too early]

If there's a strong logical correspondence between Intel's "thread ->
core -> socket" and AMD's "core -> compute unit -> node", then I think
reusing the maps makes sense; but if there's a fairly significant
difference, then I think coming up with a different naming convention
would be best.  I don't like the idea of having code that says
"if(is_intel()) { /* Do things one way */} ; else if (is_amd()) { /*
Do things a different way */}".  Not only is it ugly, it's a set-up
for bugs.

 -George

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: RE: [RFC][PATCH] AMD CPU core topology detection
  2011-01-11 10:58     ` George Dunlap
  2011-01-11 11:12       ` George Dunlap
@ 2011-01-11 15:38       ` Huang2, Wei
  1 sibling, 0 replies; 9+ messages in thread
From: Huang2, Wei @ 2011-01-11 15:38 UTC (permalink / raw)
  To: George Dunlap; +Cc: xen-devel, keir, Jan Beulich

The compute unit is a circuit device which contains 2 cores. Each core has an independent integer execution logic and L1 data cache. But L2 cache, FPU, L1 instruction cache are shared between cores. You can find a description at http://tinyurl.com/ygq8jgu. 

The CPUID spec is available at http://support.amd.com/us/Processor_TechDocs/25481.pdf.

Thanks,
-Wei

-----Original Message-----
From: dunlapg@gmail.com [mailto:dunlapg@gmail.com] On Behalf Of George Dunlap
Sent: Tuesday, January 11, 2011 4:58 AM
To: Huang2, Wei
Cc: Jan Beulich; xen-devel@lists.xensource.com; keir@xen.org
Subject: Re: [Xen-devel] RE: [RFC][PATCH] AMD CPU core topology detection

On Fri, Jan 7, 2011 at 3:52 PM, Huang2, Wei <Wei.Huang2@amd.com> wrote:
> [From my understanding, cpu_core_id and phys_proc_id are collected during boot (mostly in smpboot.c and under cpu/ directory) for sibling map. The sibling info is used for scheduler later on. Old AMD CPUs don't have HyperThreading, so the cpu_sibling_map isn't so useful. New CPUs will have core/compute unit/node. Using Intel's HT as an analogy, we have the following relationship: core=>hyper-thread, compute unit=>core, node=>processor). From that perspective, the change is reasonable. But I might have missed other parts.

Wei, can you point me to some documentation about exactly what the
"compute unit" is?

>
> I will check mce implementation. This is a good point.]
>
> If indeed your intention was to superimpose the new AMD topology
> onto the existing one (partly other than Linux, which added a new
> field to struct cpuinfo_x86, but uses cpu_{sibling,core}_map like
> what results with your patch), won't there be consequences on (at
> least) the credit scheduler (as you may not want cores to be
> treated like threads in _csched_cpu_pick())?
>
> [This is a tricky question. Based on my comments above, Bulldozer core can be treated as thread because it shares FPU with other cores. But different from HT, it also has many dedicated resources (int scheduler, pipeline and L1 cache). The scheduler needs more information in order to tell VCPUs apart on-the-fly and schedule them accordingly. Maybe George can take this into consideration for his credit2 scheduler (or beyond).
>
> I will take a look at Linux's implementation. If you have new ideas, I would appreciate them too.
> ]
>
>>@@ -132,6 +132,7 @@
>> #define X86_FEATURE_SSE5      (6*32+ 11) /* AMD Streaming SIMD Extensions-5 */
>> #define X86_FEATURE_SKINIT    (6*32+ 12) /* SKINIT, STGI/CLGI, DEV */
>> #define X86_FEATURE_WDT               (6*32+ 13) /* Watchdog Timer */
>>+#define X86_FEATURE_TOPO_EXT    (6*32+ 22) /* Topology Extension Support */
>
> Would be nice to use the same name as Linux does (i.e. without
> the last underscore).
>
> [I can surely do so.]
>
> Jan
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: RE: [RFC][PATCH] AMD CPU core topology detection
  2011-01-11 11:12       ` George Dunlap
@ 2011-01-11 17:05         ` Wei Huang
  0 siblings, 0 replies; 9+ messages in thread
From: Wei Huang @ 2011-01-11 17:05 UTC (permalink / raw)
  To: George Dunlap; +Cc: xen-devel, keir, Jan Beulich

Hi George,

The analogy holds for most parts. Following with Jan's recommendation, I 
will add a new compute_unit_id to each logic processor, with the hope of 
minimizing the impacts to Xen. No, it won't use is_amd() style of 
statements.

-Wei

On 01/11/2011 05:12 AM, George Dunlap wrote:
> On Tue, Jan 11, 2011 at 10:58 AM, George Dunlap<dunlapg@umich.edu>  wrote:
>> On Fri, Jan 7, 2011 at 3:52 PM, Huang2, Wei<Wei.Huang2@amd.com>  wrote:
>>> [From my understanding, cpu_core_id and phys_proc_id are collected during boot (mostly in smpboot.c and under cpu/ directory) for sibling map. The sibling info is used for scheduler later on. Old AMD CPUs don't have HyperThreading, so the cpu_sibling_map isn't so useful. New CPUs will have core/compute unit/node. Using Intel's HT as an analogy, we have the following relationship: core=>hyper-thread, compute unit=>core, node=>processor). From that perspective, the change is reasonable. But I might have missed other parts.
>> Wei, can you point me to some documentation about exactly what the
>> "compute unit" is?
> [Sorry, accidentally sent too early]
>
> If there's a strong logical correspondence between Intel's "thread ->
> core ->  socket" and AMD's "core ->  compute unit ->  node", then I think
> reusing the maps makes sense; but if there's a fairly significant
> difference, then I think coming up with a different naming convention
> would be best.  I don't like the idea of having code that says
> "if(is_intel()) { /* Do things one way */} ; else if (is_amd()) { /*
> Do things a different way */}".  Not only is it ugly, it's a set-up
> for bugs.
>
>   -George
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2011-01-11 17:05 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-06 16:47 [RFC][PATCH] AMD CPU core topology detection Wei Huang
2011-01-07  8:56 ` Jan Beulich
2011-01-07 15:52   ` Huang2, Wei
2011-01-07 22:56     ` Huang2, Wei
2011-01-10  8:20       ` Jan Beulich
2011-01-11 10:58     ` George Dunlap
2011-01-11 11:12       ` George Dunlap
2011-01-11 17:05         ` Wei Huang
2011-01-11 15:38       ` Huang2, Wei

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.