libxc: remove CPUID core information mangling

Hi,

c/s 18560:782599274bf9ae8857c55856c9c7fdf082967808 introduced CPUID mangling resulting in a doubled number of cores/processor exposed to the guest. According to comments in this patch the rationale behind this is to match the APIC numbering used by Xen. In my understanding the CPUID leafs dealing with number of cores always talk about logical numbers and not APIC IDs. So we don't need to adjust the CPUID readout to match the APIC ID enumeration scheme.
If there were any serious reasons resulting in the old patch I'd love to hear them.

The attached patch fixes this and solves an issue I saw with certain NUMA guest configurations.

Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448-3567-12

diff -r eccfdeb41b80 tools/libxc/xc_cpuid_x86.c
@@ -117,9 +117,8 @@
     case 0x80000008:
         /*
          * ECX[15:12] is ApicIdCoreSize: ECX[7:0] is NumberOfCores (minus one).
-         * Update to reflect vLAPIC_ID = vCPU_ID * 2.
          */
-        regs[2] = ((regs[2] & 0xf000u) + 1) | ((regs[2] & 0xffu) << 1) | 1u;
+        regs[2] &= 0xf0ffu;
         break;
     }
 }

@@ -134,11 +133,9 @@
     case 0x00000004:
         /*
          * EAX[31:26] is Maximum Cores Per Package (minus one).
-         * Update to reflect vLAPIC_ID = vCPU_ID * 2.
          */
-        regs[0] = (((regs[0] & 0x7c000000u) << 1) | 0x04000000u |
-                   (regs[0] & 0x3ffu));
-        regs[3] &= 0x3ffu;
+        regs[0] &= 0xfc0003ffu;
+        regs[3] &= 0x00000003u;
         break;
 
     case 0x80000001: {

@@ -185,9 +182,8 @@
     case 0x00000001:
         /*
          * EBX[23:16] is Maximum Logical Processors Per Package.
-         * Update to reflect vLAPIC_ID = vCPU_ID * 2.
          */
-        regs[1] = (regs[1] & 0x0000ffffu) | ((regs[1] & 0x007f0000u) << 1);
+        regs[1] & 0x00ffffffu;
 
         regs[2] &= (bitmaskof(X86_FEATURE_XMM3) |
                     bitmaskof(X86_FEATURE_SSSE3) |

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

I think you shouldn't change handling of 80000008:ECX[15:12] since that does explicitly refer to APIC ID arrangement. The rest of your changes could be correct as far as I can tell from the reference manuals.

The intent by the way was to pass through host cores-per-package info so that software licenses based on #packages would operate correctly in a virtualised environment. It's pretty hacky though indeed.

-- Keir

Hi Keir,

Do you mean that we should leave 80000008:ECX[15:12] as zero or in old way (i.e. (regs[2] & 0xf000u) + 1))? These bits can't be zero, unless we want to use legacy method in multi-core calculation.

-Wei

========
I think you shouldn't change handling of 80000008:ECX[15:12] since that does explicitly refer to APIC ID arrangement. The rest of your changes could be correct as far as I can tell from the reference manuals.

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

I meant it should remain the old way, since HVM virtual APIC IDs are vcpu_id*2.

-- Keir

OK. BTW, the old way seems wrong. The correct implementation should be (((regs[2] & 0xf000u) >> 12) + 1) << 12.

-----Original Message-----
From: Keir Fraser [mailto:[email protected]]
Sent: Wednesday, August 25, 2010 10:39 AM
To: Huang2, Wei; Przywara, Andre; Nitin Kamble
Cc: xen-devel
Subject: Re: [Xen-devel] Re: [PATCH] libxc: remove CPUID core information mangling

I meant it should remain the old way, since HVM virtual APIC IDs are vcpu_id*2.

-- Keir

Ah yes, I agree.

I agree, that seems to be best way for the time being. Although this value is actually meant to tell different processors apart, so I guess it needs a revisit later.

FYI:
Real machines use different ways to assign APIC-IDs, for example my 4-way Magny-Cours (4*12 cores) has this:
0x00-0x02: used for I/O-APICs, (could be 4-bit constrained)

    -0x0f: reserved for IOAPICs

0x10-0x1b: LAPIC-IDs for cores from the 1st processor
0x20-0x2b: LAPIC-IDs for cores from the 2nd processor
0x30-0x3b: LAPIC-IDs for cores from the 3rd processor
0x40-0x4b: LAPIC-IDs for cores from the 4th processor
The 80000008:ECX[15:12] value is 4, which means the lower 4 bits of the LAPIC ID indicate the core number within each package. Obviously this scheme does not fit the Xen one's.

Thanks Wei for spotting the calculation error!

Regards,
Andre.

Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448-3567-12

Few comments on this patch.

* First, AMD CPUID:0x80000008:ECX[NC(7:0)] should match with the total number of cores (VCPUS) assigned to guest VM. Assigning 0xff to NC is a bit scary to me. According to AMD CPUID spec, NC represents the "actually implemented or enabled on the processor, as indicated by CPUID Fn8000_0008_ECX[NC]". Most OS doesn't enforce this check; but I saw our internal test program does that.
* I am OK 80000008:ECX[15:12] change. But this change has an implication that all cores (VCPUS) are on the same physical processor. OS might not care about these bits. But this ought to be set correctly in my mind.

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Andre Przywara
Sent: Wednesday, August 25, 2010 7:04 AM
To: Keir Fraser; Nitin Kamble
Cc: xen-devel
Subject: [osrc-patches] [PATCH] libxc: remove CPUID core information mangling

Hi,

c/s 18560:782599274bf9ae8857c55856c9c7fdf082967808 introduced CPUID mangling resulting in a doubled number of cores/processor exposed to the guest. According to comments in this patch the rationale behind this is to match the APIC numbering used by Xen. In my understanding the CPUID leafs dealing with number of cores always talk about logical numbers and not APIC IDs. So we don't need to adjust the CPUID readout to match the APIC ID enumeration scheme.
If there were any serious reasons resulting in the old patch I'd love to hear them.

The attached patch fixes this and solves an issue I saw with certain NUMA guest configurations.

--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany
Tel: +49 351 448-3567-12