This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


[Xen-devel] RE: [RFC][PATCH] AMD CPU core topology detection

To: "Huang2, Wei" <Wei.Huang2@xxxxxxx>, Jan Beulich <JBeulich@xxxxxxxxxx>
Subject: [Xen-devel] RE: [RFC][PATCH] AMD CPU core topology detection
From: "Huang2, Wei" <Wei.Huang2@xxxxxxx>
Date: Fri, 7 Jan 2011 16:56:51 -0600
Accept-language: en-US
Acceptlanguage: en-US
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, "keir@xxxxxxx" <keir@xxxxxxx>
Delivery-date: Fri, 07 Jan 2011 14:55:41 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <EE335F95F28A664DB4A21289D2AA053BB4CCB2A7@xxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <4D25F227.4060808@xxxxxxx> <4D26E350020000780002AF0B@xxxxxxxxxxxxxxxxxx> <EE335F95F28A664DB4A21289D2AA053BB4CCB2A7@xxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcuuSMwn1FfvfqsCRw2j0ru4n6qNpAAM4RTQABA2HQA=
Thread-topic: [RFC][PATCH] AMD CPU core topology detection
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 


        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@xxxxxxxxxxxxxxxxxxx 
[mailto:xen-devel-bounces@xxxxxxxxxxxxxxxxxxx] On Behalf Of Huang2, Wei
Sent: Friday, January 07, 2011 9:52 AM
To: Jan Beulich
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx; keir@xxxxxxx
Subject: [Xen-devel] RE: [RFC][PATCH] AMD CPU core topology detection

Hi Jan,

Thanks for your comments. Please my answers below.

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

>>> On 06.01.11 at 17:47, Wei Huang <wei.huang2@xxxxxxx> 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.]


Xen-devel mailing list

Xen-devel mailing list