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

["Jan Beulich" <JBeulich@xxxxxxxxxx>]

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

Jan


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