From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 49ECB81E8C for ; Tue, 22 Nov 2016 21:10:10 -0800 (PST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga103.jf.intel.com with ESMTP; 22 Nov 2016 21:10:09 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,684,1473145200"; d="scan'208";a="1072375989" Received: from fmsmsx106.amr.corp.intel.com ([10.18.124.204]) by fmsmga001.fm.intel.com with ESMTP; 22 Nov 2016 21:10:09 -0800 Received: from fmsmsx114.amr.corp.intel.com (10.18.116.8) by FMSMSX106.amr.corp.intel.com (10.18.124.204) with Microsoft SMTP Server (TLS) id 14.3.248.2; Tue, 22 Nov 2016 21:10:09 -0800 Received: from shsmsx104.ccr.corp.intel.com (10.239.4.70) by FMSMSX114.amr.corp.intel.com (10.18.116.8) with Microsoft SMTP Server (TLS) id 14.3.248.2; Tue, 22 Nov 2016 21:10:09 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.239]) by SHSMSX104.ccr.corp.intel.com ([169.254.5.142]) with mapi id 14.03.0248.002; Wed, 23 Nov 2016 13:10:07 +0800 From: "Fan, Jeff" To: Laszlo Ersek , edk2-devel-01 Thread-Topic: [PATCH 1/4] UefiCpuPkg/LocalApicLib: fix feature test for Extended Topology CPUID leaf Thread-Index: AQHSRP61ejcLvqyjEEWOeFqDcDuh+aDmBVuQ Date: Wed, 23 Nov 2016 05:10:07 +0000 Message-ID: <542CF652F8836A4AB8DBFAAD40ED192A4A2E56B2@shsmsx102.ccr.corp.intel.com> References: <20161122202619.12594-1-lersek@redhat.com> <20161122202619.12594-2-lersek@redhat.com> In-Reply-To: <20161122202619.12594-2-lersek@redhat.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNzAyY2UyYTctZTNmYi00MzNiLWI5YzItMjAxODkwZTNhODk5IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6ImVYRU13ZWk3alEwTkdMc283OE9KU1c0dlRPXC9nZGJEcFNXS2JoQmVxKzNVPSJ9 x-ctpclassification: CTP_IC x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH 1/4] UefiCpuPkg/LocalApicLib: fix feature test for Extended Topology CPUID leaf X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 23 Nov 2016 05:10:10 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Yes. This is a bug to get Initial APIC ID. Reviewed-by: Jeff Fan -----Original Message----- From: Laszlo Ersek [mailto:lersek@redhat.com]=20 Sent: Wednesday, November 23, 2016 4:26 AM To: edk2-devel-01 Cc: Fan, Jeff Subject: [PATCH 1/4] UefiCpuPkg/LocalApicLib: fix feature test for Extended= Topology CPUID leaf According to the Intel SDM (325462-060US / September 2016), > INPUT EAX =3D 0BH: Returns Extended Topology Information > > [...] Software must detect the presence of CPUID leaf 0BH by verifying > (a) the highest leaf index supported by CPUID is >=3D 0BH, and > (b) CPUID.0BH:EBX[15:0] reports a non-zero value. [...] The LocalApicLib instances in UefiCpuPkg do not perform check (b). This causes an actual bug in the following OVMF setup: - Intel W3550 host processor , - the QEMU/KVM guest's VCPU model is set to "host", that is, "the CPU visible to the guest should be exactly the same as the host CPU". In the GetInitialApicId() function, check (a) passes: the CPUID level of th= e W3550 is exactly 11 decimal. However, leaf 11 itself is not supported, th= erefore EDX is set to zero: > If a value entered for CPUID.EAX is less than or equal to the maximum=20 > input value and the leaf is not supported on that processor then 0 is=20 > returned in all the registers. Because we don't check (b), we return 0 as initial APIC ID on the BSP and o= n all of the APs as well. Add the missing check. Cc: Jeff Fan Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek --- UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c | 7 +++++-- UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c | 7 +++++-- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c b/UefiCpuPkg/Li= brary/BaseXApicLib/BaseXApicLib.c index 4064049807b7..f81bbb2252d9 100644 --- a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c +++ b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c @@ -314,12 +314,15 @@ GetInitialApicId ( =20 // // If CPUID Leaf B is supported,=20 + // And CPUID.0BH:EBX[15:0] reports a non-zero value, // Then the initial 32-bit APIC ID =3D CPUID.0BH:EDX // Else the initial 8-bit APIC ID =3D CPUID.1:EBX[31:24] // if (MaxCpuIdIndex >=3D CPUID_EXTENDED_TOPOLOGY) { - AsmCpuidEx (CPUID_EXTENDED_TOPOLOGY, 0, NULL, NULL, NULL, &ApicId); - return ApicId; + AsmCpuidEx (CPUID_EXTENDED_TOPOLOGY, 0, NULL, &RegEbx, NULL, &ApicId); + if ((RegEbx & (BIT16 - 1)) !=3D 0) { + return ApicId; + } } =20 AsmCpuid (CPUID_VERSION_INFO, NULL, &RegEbx, NULL, NULL); diff --git a/U= efiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c b/UefiCpuPkg/Libr= ary/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c index 9720d26e60e2..e690d2aa1445 100644 --- a/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c +++ b/UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib.c @@ -411,12 +411,15 @@ GetInitialApicId ( AsmCpuid (CPUID_SIGNATURE, &MaxCpuIdIndex, NULL, NULL, NULL); // // If CPUID Leaf B is supported,=20 + // And CPUID.0BH:EBX[15:0] reports a non-zero value, // Then the initial 32-bit APIC ID =3D CPUID.0BH:EDX // Else the initial 8-bit APIC ID =3D CPUID.1:EBX[31:24] // if (MaxCpuIdIndex >=3D CPUID_EXTENDED_TOPOLOGY) { - AsmCpuidEx (CPUID_EXTENDED_TOPOLOGY, 0, NULL, NULL, NULL, &ApicId); - return ApicId; + AsmCpuidEx (CPUID_EXTENDED_TOPOLOGY, 0, NULL, &RegEbx, NULL, &ApicId= ); + if ((RegEbx & (BIT16 - 1)) !=3D 0) { + return ApicId; + } } AsmCpuid (CPUID_VERSION_INFO, NULL, &RegEbx, NULL, NULL); return RegEbx >> 24; -- 2.9.2