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: "Wei Huang" <wei.huang2@xxxxxxx>
Subject: [Xen-devel] Re: [RFC][PATCH] AMD CPU core topology detection
From: "Jan Beulich" <JBeulich@xxxxxxxxxx>
Date: Fri, 07 Jan 2011 08:56:32 +0000
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>, keir@xxxxxxx
Delivery-date: Fri, 07 Jan 2011 00:57:19 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4D25F227.4060808@xxxxxxx>
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>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
>>> 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.

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).


Xen-devel mailing list