Re: [edk2] [Patch 3/3] UefiCpuPkg/RegisterCpuFeaturesLib: Simplify PcdCpuFeaturesSupport.
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 (>FeatureList); while (!IsNull (>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
Re: [edk2] [Patch 3/3] UefiCpuPkg/RegisterCpuFeaturesLib: Simplify PcdCpuFeaturesSupport.
Hi Laszlo, > -Original Message- > From: Laszlo Ersek [mailto:ler...@redhat.com] > Sent: Wednesday, February 13, 2019 11:01 AM > To: Dong, Eric ; edk2-devel@lists.01.org > Cc: Ni, Ray > Subject: Re: [Patch 3/3] UefiCpuPkg/RegisterCpuFeaturesLib: Simplify > PcdCpuFeaturesSupport. > > On 02/13/19 03:04, 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 (>FeatureList); > >while (!IsNull (>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, > > -
Re: [edk2] [Patch 3/3] UefiCpuPkg/RegisterCpuFeaturesLib: Simplify PcdCpuFeaturesSupport.
On 02/13/19 03:04, 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 (>FeatureList); >while (!IsNull (>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 ( > +