From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.100; helo=mga07.intel.com; envelope-from=ray.ni@intel.com; receiver=edk2-devel@lists.01.org Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) (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 2FB9C2194EB70 for ; Thu, 14 Feb 2019 00:56:45 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga008.jf.intel.com ([10.7.209.65]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 14 Feb 2019 00:56:44 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.58,368,1544515200"; d="scan'208";a="117835271" Received: from ray-dev.ccr.corp.intel.com (HELO [10.239.9.40]) ([10.239.9.40]) by orsmga008.jf.intel.com with ESMTP; 14 Feb 2019 00:56:43 -0800 To: Eric Dong , edk2-devel@lists.01.org Cc: Laszlo Ersek References: <20190213020405.18800-1-eric.dong@intel.com> <20190213020405.18800-4-eric.dong@intel.com> From: "Ni, Ray" Message-ID: <0605fd29-0ced-351e-c5b7-c592b9946bfe@Intel.com> Date: Thu, 14 Feb 2019 16:59:17 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.5.0 MIME-Version: 1.0 In-Reply-To: <20190213020405.18800-4-eric.dong@intel.com> Subject: Re: [Patch 3/3] UefiCpuPkg/RegisterCpuFeaturesLib: Simplify PcdCpuFeaturesSupport. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 14 Feb 2019 08:56:45 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Reviewed-by: Ray Ni On 2/13/2019 10:04 AM, Eric Dong wrote: > PcdCpuFeaturesSupport used to specify the platform policy about > what CPU features this platform supports. This value is decide by > platform owner, not by hardware. After this change, this PCD will > be used in IsCpuFeatureSupported function only. > > Now RegisterCpuFeaturesLib use this PCD as an template to Get the > pcd size. Update the code logic to replace it with > PcdCpuFeaturesSetting. > > BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=1375 > > Cc: Ray Ni > Cc: Laszlo Ersek > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Eric Dong > --- > .../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c | 66 +++++++--------------- > .../RegisterCpuFeaturesLib/RegisterCpuFeatures.h | 1 - > .../RegisterCpuFeaturesLib.c | 10 ++-- > 3 files changed, 24 insertions(+), 53 deletions(-) > > diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c > index 4ebd0025b4..762eaec277 100644 > --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c > +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c > @@ -74,27 +74,6 @@ GetSettingPcd ( > return SettingBitMask; > } > > -/** > - Worker function to get PcdCpuFeaturesSupport. > - > - @return The pointer to CPU feature bits mask buffer. > -**/ > -UINT8 * > -GetSupportPcd ( > - VOID > - ) > -{ > - UINT8 *SupportBitMask; > - > - SupportBitMask = AllocateCopyPool ( > - PcdGetSize (PcdCpuFeaturesSupport), > - PcdGetPtr (PcdCpuFeaturesSupport) > - ); > - ASSERT (SupportBitMask != NULL); > - > - return SupportBitMask; > -} > - > /** > Collects CPU type and feature information. > > @@ -282,11 +261,6 @@ CpuInitDataInitialize ( > ASSERT (CpuFeaturesData->CpuFlags.CoreSemaphoreCount != NULL); > CpuFeaturesData->CpuFlags.PackageSemaphoreCount = AllocateZeroPool (sizeof (UINT32) * CpuStatus->PackageCount * CpuStatus->MaxCoreCount * CpuStatus->MaxThreadCount); > ASSERT (CpuFeaturesData->CpuFlags.PackageSemaphoreCount != NULL); > - > - // > - // Get support and configuration PCDs > - // > - CpuFeaturesData->SupportPcd = GetSupportPcd (); > } > > /** > @@ -306,7 +280,7 @@ SupportedMaskOr ( > UINT8 *Data1; > UINT8 *Data2; > > - BitMaskSize = PcdGetSize (PcdCpuFeaturesSupport); > + BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting); > Data1 = SupportedFeatureMask; > Data2 = OrFeatureBitMask; > for (Index = 0; Index < BitMaskSize; Index++) { > @@ -331,7 +305,7 @@ SupportedMaskAnd ( > UINT8 *Data1; > UINT8 *Data2; > > - BitMaskSize = PcdGetSize (PcdCpuFeaturesSupport); > + BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting); > Data1 = SupportedFeatureMask; > Data2 = AndFeatureBitMask; > for (Index = 0; Index < BitMaskSize; Index++) { > @@ -356,7 +330,7 @@ SupportedMaskCleanBit ( > UINT8 *Data1; > UINT8 *Data2; > > - BitMaskSize = PcdGetSize (PcdCpuFeaturesSupport); > + BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting); > Data1 = SupportedFeatureMask; > Data2 = AndFeatureBitMask; > for (Index = 0; Index < BitMaskSize; Index++) { > @@ -387,7 +361,7 @@ IsBitMaskMatch ( > UINT8 *Data1; > UINT8 *Data2; > > - BitMaskSize = PcdGetSize (PcdCpuFeaturesSupport); > + BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting); > > Data1 = SupportedFeatureMask; > Data2 = ComparedFeatureBitMask; > @@ -426,22 +400,22 @@ CollectProcessorData ( > Entry = GetFirstNode (&CpuFeaturesData->FeatureList); > while (!IsNull (&CpuFeaturesData->FeatureList, Entry)) { > CpuFeature = CPU_FEATURE_ENTRY_FROM_LINK (Entry); > - if (IsBitMaskMatch (CpuFeaturesData->SupportPcd, CpuFeature->FeatureMask)) { > - if (CpuFeature->SupportFunc == NULL) { > - // > - // If SupportFunc is NULL, then the feature is supported. > - // > - SupportedMaskOr ( > - CpuFeaturesData->InitOrder[ProcessorNumber].FeaturesSupportedMask, > - CpuFeature->FeatureMask > - ); > - } else if (CpuFeature->SupportFunc (ProcessorNumber, CpuInfo, CpuFeature->ConfigData)) { > - SupportedMaskOr ( > - CpuFeaturesData->InitOrder[ProcessorNumber].FeaturesSupportedMask, > - CpuFeature->FeatureMask > - ); > - } > + > + if (CpuFeature->SupportFunc == NULL) { > + // > + // If SupportFunc is NULL, then the feature is supported. > + // > + SupportedMaskOr ( > + CpuFeaturesData->InitOrder[ProcessorNumber].FeaturesSupportedMask, > + CpuFeature->FeatureMask > + ); > + } else if (CpuFeature->SupportFunc (ProcessorNumber, CpuInfo, CpuFeature->ConfigData)) { > + SupportedMaskOr ( > + CpuFeaturesData->InitOrder[ProcessorNumber].FeaturesSupportedMask, > + CpuFeature->FeatureMask > + ); > } > + > Entry = Entry->ForwardLink; > } > } > @@ -646,8 +620,6 @@ AnalysisProcessorFeatures ( > DumpCpuFeature (CpuFeature); > Entry = Entry->ForwardLink; > } > - DEBUG ((DEBUG_INFO, "PcdCpuFeaturesSupport:\n")); > - DumpCpuFeatureMask (CpuFeaturesData->SupportPcd); > DEBUG ((DEBUG_INFO, "PcdCpuFeaturesCapability:\n")); > DumpCpuFeatureMask (CpuFeaturesData->CapabilityPcd); > DEBUG ((DEBUG_INFO, "PcdCpuFeaturesSetting:\n")); > diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h > index 3e0a342fd1..836ed3549c 100644 > --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h > +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h > @@ -81,7 +81,6 @@ typedef struct { > LIST_ENTRY FeatureList; > > CPU_FEATURES_INIT_ORDER *InitOrder; > - UINT8 *SupportPcd; > UINT8 *CapabilityPcd; > UINT8 *SettingPcd; > > diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c > index 3540029079..3e8e899766 100644 > --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c > +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c > @@ -31,7 +31,7 @@ IsCpuFeatureMatch ( > { > UINTN BitMaskSize; > > - BitMaskSize = PcdGetSize (PcdCpuFeaturesSupport); > + BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting); > if (CompareMem (FirstFeatureMask, SecondFeatureMask, BitMaskSize) == 0) { > return TRUE; > } else { > @@ -53,7 +53,7 @@ DumpCpuFeatureMask ( > UINT8 *Data8; > UINTN BitMaskSize; > > - BitMaskSize = PcdGetSize (PcdCpuFeaturesSupport); > + BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting); > Data8 = (UINT8 *) FeatureMask; > for (Index = 0; Index < BitMaskSize; Index++) { > DEBUG ((DEBUG_INFO, " %02x ", *Data8++)); > @@ -100,7 +100,7 @@ IsBitMaskMatchCheck ( > UINT8 *Data1; > UINT8 *Data2; > > - BitMaskSize = PcdGetSize (PcdCpuFeaturesSupport); > + BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting); > > Data1 = FeatureMask; > Data2 = DependentBitMask; > @@ -656,7 +656,7 @@ RegisterCpuFeatureWorker ( > UINTN BitMaskSize; > BOOLEAN FeatureExist; > > - BitMaskSize = PcdGetSize (PcdCpuFeaturesSupport); > + BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting); > CpuFeaturesData = GetCpuFeaturesData (); > if (CpuFeaturesData->FeaturesCount == 0) { > InitializeListHead (&CpuFeaturesData->FeatureList); > @@ -870,7 +870,7 @@ RegisterCpuFeature ( > BeforeAll = FALSE; > AfterAll = FALSE; > > - BitMaskSize = PcdGetSize (PcdCpuFeaturesSupport); > + BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting); > > VA_START (Marker, InitializeFunc); > Feature = VA_ARG (Marker, UINT32); > -- Thanks, Ray