From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) (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 0D0D581E92 for ; Tue, 22 Nov 2016 21:36:08 -0800 (PST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga104.fm.intel.com with ESMTP; 22 Nov 2016 21:36:07 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,684,1473145200"; d="scan'208";a="8271288" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by orsmga002.jf.intel.com with ESMTP; 22 Nov 2016 21:36:07 -0800 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.248.2; Tue, 22 Nov 2016 21:36:06 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.239]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.96]) with mapi id 14.03.0248.002; Wed, 23 Nov 2016 13:36:04 +0800 From: "Fan, Jeff" To: Laszlo Ersek , edk2-devel-01 CC: Igor Mammedov Thread-Topic: [PATCH 3/4] UefiCpuPkg/MpInitLib: allow platforms to provide a known CPU count upfront Thread-Index: AQHSRP6+fDrvPtep9EC3iX58XqQ9uaDmBhkA Date: Wed, 23 Nov 2016 05:36:03 +0000 Message-ID: <542CF652F8836A4AB8DBFAAD40ED192A4A2E573F@shsmsx102.ccr.corp.intel.com> References: <20161122202619.12594-1-lersek@redhat.com> <20161122202619.12594-4-lersek@redhat.com> In-Reply-To: <20161122202619.12594-4-lersek@redhat.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMzg4NmExYzctYWIyMC00Y2JhLWIxNjMtYmIwMjhhNjYwZTQ1IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6InlsMm5ESk10UU5SUXo4YlBCN09IeG1Dc29MRWNFUk5PTkVOMUFpZ0xzOVk9In0= x-ctpclassification: CTP_IC x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH 3/4] UefiCpuPkg/MpInitLib: allow platforms to provide a known CPU count upfront 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:36:08 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Laszlo, I ever submitted one bugzilla for such request at https://bugzilla.tianocor= e.org/show_bug.cgi?id=3D116 My point is not to add new PCD and just to use existing PCD PcdCpuMaxLogica= lProcessorNumber that supports dynamic type. Platform Peim could update it is platform pei module before memory ready. MpInitLib just exits from wait loop once the discovered processors number r= eaches the PcdCpuMaxLogicalProcessorNumber. Does it meet your requirement? Thanks! Jeff -----Original Message----- From: Laszlo Ersek [mailto:lersek@redhat.com]=20 Sent: Wednesday, November 23, 2016 4:26 AM To: edk2-devel-01 Cc: Igor Mammedov; Fan, Jeff Subject: [PATCH 3/4] UefiCpuPkg/MpInitLib: allow platforms to provide a kno= wn CPU count upfront On QEMU/KVM, the VCPU topology for a guest is specified dynamically. It can= be a low number or a high number. Waiting for PcdCpuApInitTimeOutInMicroSeconds during the initial AP collect= ion is impractical, because in a VM, the time needed for an AP to wake up c= an vary widely: - if the APs report back quickly, then we could be wasting time, - if the APs report back late (due to scheduling artifacts or VCPU over-subscription on the virtualization host), then the timeout can elapse before all APs increment CpuMpData->CpuCount in ApWakeupFunction(). Trying to set PcdCpuApInitTimeOutInMicroSeconds dynamically is also impract= ical, as scheduling artifacts on the KVM host may delay AP threads arbitrar= ily. Instead, allow OVMF (and other platforms) to tell MpInitLib the number of b= oot-time CPUs in advance. Cc: Igor Mammedov Cc: Jeff Fan Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek --- UefiCpuPkg/UefiCpuPkg.dec | 11 +++++++++++ UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 1 + UefiCpuPkg/Library/M= pInitLib/PeiMpInitLib.inf | 4 ++-- UefiCpuPkg/Library/MpInitLib/MpLib.c | 16 ++++++++++++---- 4 files changed, 26 insertions(+), 6 deletions(-) diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec index ca= 560398bbef..35903c4386e4 100644 --- a/UefiCpuPkg/UefiCpuPkg.dec +++ b/UefiCpuPkg/UefiCpuPkg.dec @@ -225,5 +225,16 @@ [PcdsDynamic, PcdsDynamicEx] # @ValidList 0x80000001 | 0 gUefiCpuPkgTokenSpaceGuid.PcdCpuHotPlugDataAddress|0x0|UINT64|0x60000011 =20 + ## On platforms where the number of boot-time CPUs can be dynamically =20 + # retrieved from a platform-specific information source, the BSP does=20 + not # have to wait for PcdCpuApInitTimeOutInMicroSeconds in order to=20 + detect all # APs for the first time. On such platforms, this PCD=20 + specifies the number # of CPUs available at boot. If the platform=20 + doesn't support this feature, # this PCD should be set to 0. The=20 + platform is responsible for ensuring that # this PCD is never set to=20 + a value larger than # PcdCpuMaxLogicalProcessorNumber. + # @Prompt Known number of CPUs available at boot. + =20 + gUefiCpuPkgTokenSpaceGuid.PcdCpuKnownLogicalProcessorNumber|0|UINT32|0 + x60000012 + [UserExtensions.TianoCore."ExtraFiles"] UefiCpuPkgExtra.uni diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf b/UefiCpuPkg/Lib= rary/MpInitLib/DxeMpInitLib.inf index 11b230174ec8..dc18eaf6152d 100644 --- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf @@ -61,6 +61,7 @@ [Guids] =20 [Pcd] gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber ## CONS= UMES + gUefiCpuPkgTokenSpaceGuid.PcdCpuKnownLogicalProcessorNumber ## SOME= TIMES_CONSUMES gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds ## SOME= TIMES_CONSUMES gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize ## CONS= UMES gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress ## CONS= UMES diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf b/UefiCpuPkg/Lib= rary/MpInitLib/PeiMpInitLib.inf index 0c6873da79db..2bcfa70ae7e5 100644 --- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf @@ -61,10 +61,10 @@ [Ppis] =20 [Pcd] gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber ## CONS= UMES - gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds ## CONS= UMES + gUefiCpuPkgTokenSpaceGuid.PcdCpuKnownLogicalProcessorNumber ## CONS= UMES + gUefiCpuPkgTokenSpaceGuid.PcdCpuApInitTimeOutInMicroSeconds ## SOME= TIMES_CONSUMES gUefiCpuPkgTokenSpaceGuid.PcdCpuApStackSize ## CONS= UMES gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchAddress ## CONS= UMES gUefiCpuPkgTokenSpaceGuid.PcdCpuMicrocodePatchRegionSize ## CONS= UMES gUefiCpuPkgTokenSpaceGuid.PcdCpuApLoopMode ## CONS= UMES gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate ## SOME= TIMES_CONSUMES - diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpIn= itLib/MpLib.c index 15dbfa1e7d6c..f7dfbd5bad13 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c @@ -707,6 +707,7 @@ WakeUpAP ( CPU_AP_DATA *CpuData; BOOLEAN ResetVectorRequired; CPU_INFO_IN_HOB *CpuInfoInHob; + UINT32 KnownProcessorCount; =20 CpuMpData->FinishedCount =3D 0; ResetVectorRequired =3D FALSE; @@ -745,10 +746,17 @@ WakeUpAP ( SendInitSipiSipiAllExcludingSelf ((UINT32) ExchangeInfo->BufferStart= ); } if (CpuMpData->InitFlag =3D=3D ApInitConfig) { - // - // Wait for all potential APs waken up in one specified period - // - MicroSecondDelay (PcdGet32(PcdCpuApInitTimeOutInMicroSeconds)); + KnownProcessorCount =3D PcdGet32 (PcdCpuKnownLogicalProcessorNumber)= ; + if (KnownProcessorCount > 0) { + while (CpuMpData->FinishedCount < (KnownProcessorCount - 1)) { + CpuPause (); + } + } else { + // + // Wait for all potential APs waken up in one specified period + // + MicroSecondDelay (PcdGet32(PcdCpuApInitTimeOutInMicroSeconds)); + } } else { // // Wait all APs waken up if this is not the 1st broadcast of SIPI -- 2.9.2