Re: [Patch v2 3/4] UefiCpuPkg/RegisterCpuFeaturesLib: Simplify PcdCpuFeaturesSupport.

["Ni, Ray" <ray.ni@intel.com>]

Reviewed-by: Ray Ni <ray.ni@intel.com>

> -----Original Message-----
> From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of Eric Dong
> Sent: Friday, March 1, 2019 1:40 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [Patch v2 3/4] UefiCpuPkg/RegisterCpuFeaturesLib: Simplify
> PcdCpuFeaturesSupport.
> 
> PcdCpuFeaturesSupport used to specify the platform policy about what CPU
> features this platform supports. This PCD will be used in IsCpuFeatureSupported
> 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
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> ---
>  .../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c | 43 +++++++++-------------
>  .../RegisterCpuFeaturesLib/RegisterCpuFeatures.h   |  1 -
>  .../RegisterCpuFeaturesLib.c                       | 10 ++---
>  3 files changed, 22 insertions(+), 32 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> index d877caff74..c82f848b97 100644
> --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> @@ -245,11 +245,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 ();
>  }
> 
>  /**
> @@ -269,7 +264,7 @@ SupportedMaskOr (
>    UINT8                  *Data1;
>    UINT8                  *Data2;
> 
> -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSupport);
> +  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
>    Data1 = SupportedFeatureMask;
>    Data2 = OrFeatureBitMask;
>    for (Index = 0; Index < BitMaskSize; Index++) { @@ -294,7 +289,7 @@
> SupportedMaskAnd (
>    UINT8                  *Data1;
>    UINT8                  *Data2;
> 
> -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSupport);
> +  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
>    Data1 = SupportedFeatureMask;
>    Data2 = AndFeatureBitMask;
>    for (Index = 0; Index < BitMaskSize; Index++) { @@ -319,7 +314,7 @@
> SupportedMaskCleanBit (
>    UINT8                  *Data1;
>    UINT8                  *Data2;
> 
> -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSupport);
> +  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
>    Data1 = SupportedFeatureMask;
>    Data2 = AndFeatureBitMask;
>    for (Index = 0; Index < BitMaskSize; Index++) { @@ -350,7 +345,7 @@
> IsBitMaskMatch (
>    UINT8                  *Data1;
>    UINT8                  *Data2;
> 
> -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSupport);
> +  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
> 
>    Data1 = SupportedFeatureMask;
>    Data2 = ComparedFeatureBitMask;
> @@ -389,21 +384,19 @@ 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;
>    }
> @@ -596,8 +589,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, "Origin 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);
> --
> 2.15.0.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel