Andre Przywara
2010-Aug-25 12:04 UTC
[Xen-devel] [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. Signed-off-by: Andre Przywara <andre.przywara@amd.com> -- Andre Przywara AMD-Operating System Research Center (OSRC), Dresden, Germany Tel: +49 351 448-3567-12 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2010-Aug-25 12:42 UTC
[Xen-devel] Re: [PATCH] libxc: remove CPUID core information mangling
On 25/08/2010 13:04, "Andre Przywara" <andre.przywara@amd.com> wrote:> 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.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> Signed-off-by: Andre Przywara <andre.przywara@amd.com>_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Huang2, Wei
2010-Aug-25 15:25 UTC
[Xen-devel] RE: [osrc-patches] [PATCH] libxc: remove CPUID core information mangling
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: osrc-patches-bounces@elbe.amd.com [mailto:osrc-patches-bounces@elbe.amd.com] 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. Signed-off-by: Andre Przywara <andre.przywara@amd.com> -- Andre Przywara AMD-Operating System Research Center (OSRC), Dresden, Germany Tel: +49 351 448-3567-12 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Huang2, Wei
2010-Aug-25 15:28 UTC
RE: [Xen-devel] Re: [PATCH] libxc: remove CPUID core information mangling
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 Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2010-Aug-25 15:39 UTC
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 On 25/08/2010 16:28, "Huang2, Wei" <Wei.Huang2@amd.com> wrote:> 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 > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Huang2, Wei
2010-Aug-25 15:53 UTC
RE: [Xen-devel] Re: [PATCH] libxc: remove CPUID core information mangling
OK. BTW, the old way seems wrong. The correct implementation should be (((regs[2] & 0xf000u) >> 12) + 1) << 12. -----Original Message----- From: Keir Fraser [mailto:keir.fraser@eu.citrix.com] 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 On 25/08/2010 16:28, "Huang2, Wei" <Wei.Huang2@amd.com> wrote:> 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 > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2010-Aug-25 16:00 UTC
Re: [Xen-devel] Re: [PATCH] libxc: remove CPUID core information mangling
Ah yes, I agree. On 25/08/2010 16:53, "Huang2, Wei" <Wei.Huang2@amd.com> wrote:> OK. BTW, the old way seems wrong. The correct implementation should be > (((regs[2] & 0xf000u) >> 12) + 1) << 12. > > -----Original Message----- > From: Keir Fraser [mailto:keir.fraser@eu.citrix.com] > 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 > > On 25/08/2010 16:28, "Huang2, Wei" <Wei.Huang2@amd.com> wrote: > >> 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 >> Xen-devel@lists.xensource.com >> http://lists.xensource.com/xen-devel >> >> > > > >_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Andre Przywara
2010-Aug-26 20:48 UTC
Re: [Xen-devel] Re: [PATCH] libxc: remove CPUID core information mangling
Keir Fraser wrote:> Ah yes, I agree. > > > On 25/08/2010 16:53, "Huang2, Wei" <Wei.Huang2@amd.com> wrote: > >> OK. BTW, the old way seems wrong. The correct implementation should be >> (((regs[2] & 0xf000u) >> 12) + 1) << 12. >> >> -----Original Message----- >> From: Keir Fraser [mailto:keir.fraser@eu.citrix.com] >> 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.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.>> >> -- Keir >> >> On 25/08/2010 16:28, "Huang2, Wei" <Wei.Huang2@amd.com> wrote: >> >>> 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. >>>-- Andre Przywara AMD-Operating System Research Center (OSRC), Dresden, Germany Tel: +49 351 448-3567-12 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel