From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by mx.groups.io with SMTP id smtpd.web11.15588.1572918433419075478 for ; Mon, 04 Nov 2019 17:47:13 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: intel.com, ip: 192.55.52.93, mailfrom: eric.dong@intel.com) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Nov 2019 17:47:13 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.68,269,1569308400"; d="scan'208";a="285157398" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by orsmga001.jf.intel.com with ESMTP; 04 Nov 2019 17:47:12 -0800 Received: from fmsmsx123.amr.corp.intel.com (10.18.125.38) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 4 Nov 2019 17:47:10 -0800 Received: from shsmsx108.ccr.corp.intel.com (10.239.4.97) by fmsmsx123.amr.corp.intel.com (10.18.125.38) with Microsoft SMTP Server (TLS) id 14.3.439.0; Mon, 4 Nov 2019 17:47:09 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.108]) by SHSMSX108.ccr.corp.intel.com ([169.254.8.41]) with mapi id 14.03.0439.000; Tue, 5 Nov 2019 09:47:08 +0800 From: "Dong, Eric" To: "devel@edk2.groups.io" , "Ni, Ray" CC: Laszlo Ersek Subject: Re: [edk2-devel] [PATCH 1/2] UefiCpuPkg/MpInitLib: Set X2ApicEnable flag from BSP Thread-Topic: [edk2-devel] [PATCH 1/2] UefiCpuPkg/MpInitLib: Set X2ApicEnable flag from BSP Thread-Index: AQHVjwf1hogmUtQbUkahCv87abzP86d717RA Date: Tue, 5 Nov 2019 01:47:07 +0000 Message-ID: References: <20191030095233.565420-1-ray.ni@intel.com> <20191030095233.565420-2-ray.ni@intel.com> In-Reply-To: <20191030095233.565420-2-ray.ni@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Return-Path: eric.dong@intel.com Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Eric Dong -----Original Message----- From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Ni, = Ray Sent: Wednesday, October 30, 2019 5:53 PM To: devel@edk2.groups.io Cc: Dong, Eric ; Laszlo Ersek Subject: [edk2-devel] [PATCH 1/2] UefiCpuPkg/MpInitLib: Set X2ApicEnable f= lag from BSP Today's logic sets X2ApicEnable flag in each AP's initialization path when= InitFlag =3D=3D ApInitConfig. Since all CPUs update the same global data, a spin-lock is used to avoid m= odifications from multiple CPUs happen at the same time. The spin-lock causes two problems: 1. Potential performance downgrade. 2. Undefined behavior when improper timer lib is used. For example we saw certain platforms used AcpiTimerLib from PcAtChipsetPkg and that library depends on retrieving PeiServices from idtr. But in fact AP's (idtr - 4) doesn't point to PeiServices. The patch simplifies the code to let BSP set the X2ApicEnable flag so the = spin-lock acquisition from AP is not needed any more. Signed-off-by: Ray Ni Cc: Eric Dong Cc: Laszlo Ersek --- UefiCpuPkg/Library/MpInitLib/MpLib.c | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpI= nitLib/MpLib.c index 622b70ca3c..8f62a8d965 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c @@ -458,6 +458,7 @@ CollectProcessorCount ( ) { UINTN Index; + CPU_INFO_IN_HOB *CpuInfoInHob; =20 // // Send 1st broadcast IPI to APs to wakeup APs @@ -474,12 +475,27 @@ Co= llectProcessorCount ( CpuPause (); } =20 + + // + // Enable x2APIC mode if + // 1. Number of CPU is greater than 255; or // 2. There are any=20 + logical processors reporting an Initial APIC ID of 255 or greater. + // if (CpuMpData->CpuCount > 255) { // // If there are more than 255 processor found, force to enable X2APIC // CpuMpData->X2ApicEnable =3D TRUE; + } else { + CpuInfoInHob =3D (CPU_INFO_IN_HOB *) (UINTN) CpuMpData->CpuInfoInHob; + for (Index =3D 0; Index < CpuMpData->CpuCount; Index++) { + if (CpuInfoInHob[Index].InitialApicId >=3D 0xFF) { + CpuMpData->X2ApicEnable =3D TRUE; + break; + } + } } + if (CpuMpData->X2ApicEnable) { DEBUG ((DEBUG_INFO, "Force x2APIC mode!\n")); // @@ -541,15 +557,6 @@ InitializeApData ( =20 CpuMpData->CpuData[ProcessorNumber].Waiting =3D FALSE; CpuMpData->CpuData[ProcessorNumber].CpuHealthy =3D (BistData =3D=3D 0) = ? TRUE : FALSE; - if (CpuInfoInHob[ProcessorNumber].InitialApicId >=3D 0xFF) { - // - // Set x2APIC mode if there are any logical processor reporting - // an Initial APIC ID of 255 or greater. - // - AcquireSpinLock(&CpuMpData->MpLock); - CpuMpData->X2ApicEnable =3D TRUE; - ReleaseSpinLock(&CpuMpData->MpLock); - } =20 InitializeSpinLock(&CpuMpData->CpuData[ProcessorNumber].ApLock); SetApState (&CpuMpData->CpuData[ProcessorNumber], CpuStateIdle); -- 2.21.0.windows.1