[edk2-devel] [Patch 2/2] UefiCpuPkg/Library/RegisterCpuFeaturesLib: avoid use dynamic PCD.

Zeng, Star star.zeng at intel.com
Fri Jul 12 11:10:54 UTC 2019


4 comments are added inline.

> -----Original Message-----
> From: devel at edk2.groups.io [mailto:devel at edk2.groups.io] On Behalf Of
> Dong, Eric
> Sent: Friday, July 12, 2019 9:53 AM
> To: devel at edk2.groups.io
> Cc: Ni, Ray <ray.ni at intel.com>; Laszlo Ersek <lersek at redhat.com>; Kumar,
> Chandana C <chandana.c.kumar at intel.com>; Zeng, Star
> <star.zeng at intel.com>
> Subject: [edk2-devel] [Patch 2/2]
> UefiCpuPkg/Library/RegisterCpuFeaturesLib: avoid use dynamic PCD.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1972
> 
> Function in this library may be used by APs. Assert will be trig if AP uses
> dynamic pcd.
> This patch enhance the current code, remove the unnecessary usage of
> dynamic PCD. This change try to avoid report this issue again later.
> 
> Cc: Ray Ni <ray.ni at intel.com>
> Cc: Laszlo Ersek <lersek at redhat.com>
> Cc: Chandana Kumar <chandana.c.kumar at intel.com>
> Cc: Star Zeng <star.zeng at intel.com>
> Signed-off-by: Eric Dong <eric.dong at intel.com>
> ---
>  .../CpuFeaturesInitialize.c                   |  64 +++++-----
>  .../RegisterCpuFeatures.h                     |  10 +-
>  .../RegisterCpuFeaturesLib.c                  | 114 ++++++------------
>  3 files changed, 77 insertions(+), 111 deletions(-)
> 
> diff --git
> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> index 87bfc64250..16b99c0c27 100644
> --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> @@ -21,16 +21,12 @@ CHAR16 *mRegisterTypeStr[] = {L"MSR", L"CR",
> L"MMIO", L"CACHE", L"SEMAP", L"INVA  VOID  SetCapabilityPcd (
>    IN UINT8               *SupportedFeatureMask,
> -  IN UINT32              FeatureMaskSize
> +  IN UINTN               FeatureMaskSize

1. How about using BitMaskSize to be aligned with other places? Notice, the function header also needs to be updated if using BitMaskSize.

>    )
>  {
>    EFI_STATUS             Status;
> -  UINTN                  BitMaskSize;
> 
> -  BitMaskSize = PcdGetSize (PcdCpuFeaturesCapability);
> -  ASSERT (FeatureMaskSize == BitMaskSize);
> -
> -  Status = PcdSetPtrS (PcdCpuFeaturesCapability, &BitMaskSize,
> SupportedFeatureMask);
> +  Status = PcdSetPtrS (PcdCpuFeaturesCapability, &FeatureMaskSize,
> + SupportedFeatureMask);
>    ASSERT_EFI_ERROR (Status);
>  }
> 
> @@ -38,16 +34,16 @@ SetCapabilityPcd (
>    Worker function to save PcdCpuFeaturesSetting.
> 
>    @param[in]  SupportedFeatureMask  The pointer to CPU feature bits mask
> buffer
> +  @param[in]  FeatureMaskSize       CPU feature bits mask buffer size.

2. Need use BitMaskSize to be matched with function parameter name.

>  **/
>  VOID
>  SetSettingPcd (
> -  IN UINT8               *SupportedFeatureMask
> +  IN UINT8               *SupportedFeatureMask,
> +  IN UINTN               BitMaskSize
>    )
>  {
>    EFI_STATUS             Status;
> -  UINTN                  BitMaskSize;
> 
> -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
>    Status = PcdSetPtrS (PcdCpuFeaturesSetting, &BitMaskSize,
> SupportedFeatureMask);
>    ASSERT_EFI_ERROR (Status);
>  }
> @@ -272,19 +268,20 @@ SupportedMaskOr (
> 
>    @param[in]  SupportedFeatureMask  The pointer to CPU feature bits mask
> buffer
>    @param[in]  AndFeatureBitMask     The feature bit mask to do AND
> operation
> +  @param[in]  BitMaskSize           CPU feature bits mask buffer size.
> +
>  **/
>  VOID
>  SupportedMaskAnd (
>    IN       UINT8               *SupportedFeatureMask,
> -  IN CONST UINT8               *AndFeatureBitMask
> +  IN CONST UINT8               *AndFeatureBitMask,
> +  IN       UINT32              BitMaskSize
>    )
>  {
>    UINTN                  Index;
> -  UINTN                  BitMaskSize;
>    UINT8                  *Data1;
>    CONST UINT8            *Data2;
> 
> -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
>    Data1 = SupportedFeatureMask;
>    Data2 = AndFeatureBitMask;
>    for (Index = 0; Index < BitMaskSize; Index++) { @@ -297,19 +294,19 @@
> SupportedMaskAnd (
> 
>    @param[in]  SupportedFeatureMask  The pointer to CPU feature bits mask
> buffer
>    @param[in]  AndFeatureBitMask     The feature bit mask to do XOR
> operation
> +  @param[in]  BitMaskSize           CPU feature bits mask buffer size.
>  **/
>  VOID
>  SupportedMaskCleanBit (
>    IN UINT8               *SupportedFeatureMask,
> -  IN UINT8               *AndFeatureBitMask
> +  IN UINT8               *AndFeatureBitMask,
> +  IN UINT32              BitMaskSize
>    )
>  {
>    UINTN                  Index;
> -  UINTN                  BitMaskSize;
>    UINT8                  *Data1;
>    UINT8                  *Data2;
> 
> -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
>    Data1 = SupportedFeatureMask;
>    Data2 = AndFeatureBitMask;
>    for (Index = 0; Index < BitMaskSize; Index++) { @@ -323,6 +320,7 @@
> SupportedMaskCleanBit (
> 
>    @param[in]  SupportedFeatureMask   The pointer to CPU feature bits mask
> buffer
>    @param[in]  ComparedFeatureBitMask The feature bit mask to be
> compared
> +  @param[in]  BitMaskSize            CPU feature bits mask buffer size.
> 
>    @retval TRUE   The ComparedFeatureBitMask is set in CPU feature
> supported bits
>                   mask buffer.
> @@ -332,16 +330,14 @@ SupportedMaskCleanBit (  BOOLEAN
> IsBitMaskMatch (
>    IN UINT8               *SupportedFeatureMask,
> -  IN UINT8               *ComparedFeatureBitMask
> +  IN UINT8               *ComparedFeatureBitMask,
> +  IN UINT32              BitMaskSize
>    )
>  {
>    UINTN                  Index;
> -  UINTN                  BitMaskSize;
>    UINT8                  *Data1;
>    UINT8                  *Data2;
> 
> -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
> -
>    Data1 = SupportedFeatureMask;
>    Data2 = ComparedFeatureBitMask;
>    for (Index = 0; Index < BitMaskSize; Index++) { @@ -557,14 +553,14 @@
> AnalysisProcessorFeatures (
>      //
>      // Calculate the last capability on all processors
>      //
> -    SupportedMaskAnd (CpuFeaturesData->CapabilityPcd, CpuInitOrder-
> >FeaturesSupportedMask);
> +    SupportedMaskAnd (CpuFeaturesData->CapabilityPcd,
> + CpuInitOrder->FeaturesSupportedMask, CpuFeaturesData->BitMaskSize);
>    }
>    //
>    // Calculate the last setting
>    //
>    CpuFeaturesData->SettingPcd = AllocateCopyPool (CpuFeaturesData-
> >BitMaskSize, CpuFeaturesData->CapabilityPcd);
>    ASSERT (CpuFeaturesData->SettingPcd != NULL);
> -  SupportedMaskAnd (CpuFeaturesData->SettingPcd, PcdGetPtr
> (PcdCpuFeaturesSetting));
> +  SupportedMaskAnd (CpuFeaturesData->SettingPcd, PcdGetPtr
> + (PcdCpuFeaturesSetting), CpuFeaturesData->BitMaskSize);
> 
>    //
>    // Dump the last CPU feature list
> @@ -574,8 +570,8 @@ AnalysisProcessorFeatures (
>      Entry = GetFirstNode (&CpuFeaturesData->FeatureList);
>      while (!IsNull (&CpuFeaturesData->FeatureList, Entry)) {
>        CpuFeature = CPU_FEATURE_ENTRY_FROM_LINK (Entry);
> -      if (IsBitMaskMatch (CpuFeature->FeatureMask, CpuFeaturesData-
> >CapabilityPcd)) {
> -        if (IsBitMaskMatch (CpuFeature->FeatureMask, CpuFeaturesData-
> >SettingPcd)) {
> +      if (IsBitMaskMatch (CpuFeature->FeatureMask, CpuFeaturesData-
> >CapabilityPcd, CpuFeaturesData->BitMaskSize)) {
> +        if (IsBitMaskMatch (CpuFeature->FeatureMask,
> + CpuFeaturesData->SettingPcd, CpuFeaturesData->BitMaskSize)) {
>            DEBUG ((DEBUG_INFO, "[Enable   ] "));
>          } else {
>            DEBUG ((DEBUG_INFO, "[Disable  ] ")); @@ -583,22 +579,22 @@
> AnalysisProcessorFeatures (
>        } else {
>          DEBUG ((DEBUG_INFO, "[Unsupport] "));
>        }
> -      DumpCpuFeature (CpuFeature);
> +      DumpCpuFeature (CpuFeature, CpuFeaturesData->BitMaskSize);
>        Entry = Entry->ForwardLink;
>      }
>      DEBUG ((DEBUG_INFO, "PcdCpuFeaturesCapability:\n"));
> -    DumpCpuFeatureMask (CpuFeaturesData->CapabilityPcd);
> +    DumpCpuFeatureMask (CpuFeaturesData->CapabilityPcd,
> + CpuFeaturesData->BitMaskSize);
>      DEBUG ((DEBUG_INFO, "Origin PcdCpuFeaturesSetting:\n"));
> -    DumpCpuFeatureMask (PcdGetPtr (PcdCpuFeaturesSetting));
> +    DumpCpuFeatureMask (PcdGetPtr (PcdCpuFeaturesSetting),
> + CpuFeaturesData->BitMaskSize);
>      DEBUG ((DEBUG_INFO, "Final PcdCpuFeaturesSetting:\n"));
> -    DumpCpuFeatureMask (CpuFeaturesData->SettingPcd);
> +    DumpCpuFeatureMask (CpuFeaturesData->SettingPcd,
> + CpuFeaturesData->BitMaskSize);
>    );
> 
>    //
>    // Save PCDs and display CPU PCDs
>    //
>    SetCapabilityPcd (CpuFeaturesData->CapabilityPcd, CpuFeaturesData-
> >BitMaskSize);
> -  SetSettingPcd (CpuFeaturesData->SettingPcd);
> +  SetSettingPcd (CpuFeaturesData->SettingPcd,
> + CpuFeaturesData->BitMaskSize);
> 
>    for (ProcessorNumber = 0; ProcessorNumber < NumberOfCpus;
> ProcessorNumber++) {
>      CpuInitOrder = &CpuFeaturesData->InitOrder[ProcessorNumber];
> @@ -608,7 +604,7 @@ AnalysisProcessorFeatures (
>        // Insert each feature into processor's order list
>        //
>        CpuFeature = CPU_FEATURE_ENTRY_FROM_LINK (Entry);
> -      if (IsBitMaskMatch (CpuFeature->FeatureMask, CpuFeaturesData-
> >CapabilityPcd)) {
> +      if (IsBitMaskMatch (CpuFeature->FeatureMask,
> + CpuFeaturesData->CapabilityPcd, CpuFeaturesData->BitMaskSize)) {
>          CpuFeatureInOrder = AllocateCopyPool (sizeof (CPU_FEATURES_ENTRY),
> CpuFeature);
>          ASSERT (CpuFeatureInOrder != NULL);
>          InsertTailList (&CpuInitOrder->OrderList, &CpuFeatureInOrder->Link);
> @@ -624,18 +620,18 @@ AnalysisProcessorFeatures (
>        CpuFeatureInOrder = CPU_FEATURE_ENTRY_FROM_LINK (Entry);
> 
>        Success = FALSE;
> -      if (IsBitMaskMatch (CpuFeatureInOrder->FeatureMask,
> CpuFeaturesData->SettingPcd)) {
> +      if (IsBitMaskMatch (CpuFeatureInOrder->FeatureMask,
> + CpuFeaturesData->SettingPcd, CpuFeaturesData->BitMaskSize)) {
>          Status = CpuFeatureInOrder->InitializeFunc (ProcessorNumber, CpuInfo,
> CpuFeatureInOrder->ConfigData, TRUE);
>          if (EFI_ERROR (Status)) {
>            //
>            // Clean the CpuFeatureInOrder->FeatureMask in setting PCD.
>            //
> -          SupportedMaskCleanBit (CpuFeaturesData->SettingPcd,
> CpuFeatureInOrder->FeatureMask);
> +          SupportedMaskCleanBit (CpuFeaturesData->SettingPcd,
> + CpuFeatureInOrder->FeatureMask, CpuFeaturesData->BitMaskSize);
>            if (CpuFeatureInOrder->FeatureName != NULL) {
>              DEBUG ((DEBUG_WARN, "Warning :: Failed to enable Feature: Name
> = %a.\n", CpuFeatureInOrder->FeatureName));
>            } else {
>              DEBUG ((DEBUG_WARN, "Warning :: Failed to enable Feature: Mask =
> "));
> -            DumpCpuFeatureMask (CpuFeatureInOrder->FeatureMask);
> +            DumpCpuFeatureMask (CpuFeatureInOrder->FeatureMask,
> + CpuFeaturesData->BitMaskSize);
>            }
>          } else {
>            Success = TRUE;
> @@ -647,7 +643,7 @@ AnalysisProcessorFeatures (
>              DEBUG ((DEBUG_WARN, "Warning :: Failed to disable Feature: Name
> = %a.\n", CpuFeatureInOrder->FeatureName));
>            } else {
>              DEBUG ((DEBUG_WARN, "Warning :: Failed to disable Feature: Mask =
> "));
> -            DumpCpuFeatureMask (CpuFeatureInOrder->FeatureMask);
> +            DumpCpuFeatureMask (CpuFeatureInOrder->FeatureMask,
> + CpuFeaturesData->BitMaskSize);
>            }
>          } else {
>            Success = TRUE;
> @@ -699,7 +695,7 @@ AnalysisProcessorFeatures (
>      // again during initialize the features.
>      //
>      DEBUG ((DEBUG_INFO, "Dump final value for
> PcdCpuFeaturesSetting:\n"));
> -    DumpCpuFeatureMask (CpuFeaturesData->SettingPcd);
> +    DumpCpuFeatureMask (CpuFeaturesData->SettingPcd,
> + CpuFeaturesData->BitMaskSize);
> 
>      //
>      // Dump the RegisterTable
> diff --git
> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
> index 5c546ee153..a18f926641 100644
> --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
> +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
> @@ -180,20 +180,26 @@ SwitchNewBsp (
>    Function that uses DEBUG() macros to display the contents of a a CPU
> feature bit mask.
> 
>    @param[in]  FeatureMask  A pointer to the CPU feature bit mask.
> +  @param[in]  BitMaskSize  CPU feature bits mask buffer size.
> +
>  **/
>  VOID
>  DumpCpuFeatureMask (
> -  IN UINT8               *FeatureMask
> +  IN UINT8               *FeatureMask,
> +  IN UINT32              BitMaskSize
>    );
> 
>  /**
>    Dump CPU feature name or CPU feature bit mask.
> 
>    @param[in]  CpuFeature   Pointer to CPU_FEATURES_ENTRY
> +  @param[in]  BitMaskSize  CPU feature bits mask buffer size.
> +
>  **/
>  VOID
>  DumpCpuFeature (
> -  IN CPU_FEATURES_ENTRY  *CpuFeature
> +  IN CPU_FEATURES_ENTRY  *CpuFeature,
> +  IN UINT32              BitMaskSize
>    );
> 
>  /**
> diff --git
> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> index 36aabd7267..283e9d6539 100644
> --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> +++
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> @@ -18,36 +18,34 @@
>    @retval FALSE Two CPU feature bit masks are not equal.
>  **/
>  BOOLEAN
> -IsCpuFeatureMatch (
> +IsBitMaskMatchCheck (

3. Seemingly, original IsCpuFeatureMatch() and IsBitMaskMatchCheck() have different purpose. How they can be merged? Please double confirm that.
And even, they can be merged, it will be better to do that in a separated patch.

>    IN UINT8               *FirstFeatureMask,
>    IN UINT8               *SecondFeatureMask
>    )
>  {
> -  UINTN                 BitMaskSize;
> +  CPU_FEATURES_DATA          *CpuFeaturesData;
> 
> -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
> -  if (CompareMem (FirstFeatureMask, SecondFeatureMask, BitMaskSize) ==
> 0) {
> -    return TRUE;
> -  } else {
> -    return FALSE;
> -  }
> +  CpuFeaturesData = GetCpuFeaturesData ();
> +
> +  return (CompareMem (FirstFeatureMask, SecondFeatureMask,
> + CpuFeaturesData->BitMaskSize) == 0);
>  }
> 
>  /**
>    Function that uses DEBUG() macros to display the contents of a a CPU
> feature bit mask.
> 
>    @param[in]  FeatureMask  A pointer to the CPU feature bit mask.
> +  @param[in]  BitMaskSize  CPU feature bits mask buffer size.
> +
>  **/
>  VOID
>  DumpCpuFeatureMask (
> -  IN UINT8               *FeatureMask
> +  IN UINT8               *FeatureMask,
> +  IN UINT32              BitMaskSize
>    )
>  {
>    UINTN                  Index;
>    UINT8                  *Data8;
> -  UINTN                  BitMaskSize;
> 
> -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
>    Data8       = (UINT8 *) FeatureMask;
>    for (Index = 0; Index < BitMaskSize; Index++) {
>      DEBUG ((DEBUG_INFO, " %02x ", *Data8++)); @@ -59,10 +57,13 @@
> DumpCpuFeatureMask (
>    Dump CPU feature name or CPU feature bit mask.
> 
>    @param[in]  CpuFeature   Pointer to CPU_FEATURES_ENTRY
> +  @param[in]  BitMaskSize  CPU feature bits mask buffer size.
> +
>  **/
>  VOID
>  DumpCpuFeature (
> -  IN CPU_FEATURES_ENTRY  *CpuFeature
> +  IN CPU_FEATURES_ENTRY  *CpuFeature,
> +  IN UINT32              BitMaskSize
>    )
>  {
> 
> @@ -70,42 +71,10 @@ DumpCpuFeature (
>      DEBUG ((DEBUG_INFO, "FeatureName: %a\n", CpuFeature-
> >FeatureName));
>    } else {
>      DEBUG ((DEBUG_INFO, "FeatureMask = "));
> -    DumpCpuFeatureMask (CpuFeature->FeatureMask);
> +    DumpCpuFeatureMask (CpuFeature->FeatureMask, BitMaskSize);
>    }
>  }
> 
> -/**
> -  Determines if the feature bit mask is in dependent CPU feature bit mask
> buffer.
> -
> -  @param[in]  FeatureMask        Pointer to CPU feature bit mask
> -  @param[in]  DependentBitMask   Pointer to dependent CPU feature bit
> mask buffer
> -
> -  @retval TRUE  The feature bit mask is in dependent CPU feature bit mask
> buffer.
> -  @retval FALSE The feature bit mask is not in dependent CPU feature bit
> mask buffer.
> -**/
> -BOOLEAN
> -IsBitMaskMatchCheck (
> -  IN UINT8        *FeatureMask,
> -  IN UINT8        *DependentBitMask
> -  )
> -{
> -  UINTN      Index;
> -  UINTN      BitMaskSize;
> -  UINT8      *Data1;
> -  UINT8      *Data2;
> -
> -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
> -
> -  Data1 = FeatureMask;
> -  Data2 = DependentBitMask;
> -  for (Index = 0; Index < BitMaskSize; Index++) {
> -    if (((*(Data1++)) & (*(Data2++))) != 0) {
> -      return TRUE;
> -    }
> -  }
> -  return FALSE;
> -}
> -
>  /**
>    Try to find the specify cpu featuren in former/after feature list.
> 
> @@ -642,37 +611,21 @@ CheckCpuFeaturesDependency (  **/
> RETURN_STATUS  RegisterCpuFeatureWorker (
> +  IN CPU_FEATURES_DATA       *CpuFeaturesData,

4. New parameter is added, so the function header will need to be updated.


Thanks,
Star

>    IN CPU_FEATURES_ENTRY      *CpuFeature
>    )
>  {
>    EFI_STATUS                 Status;
> -  CPU_FEATURES_DATA          *CpuFeaturesData;
>    CPU_FEATURES_ENTRY         *CpuFeatureEntry;
>    LIST_ENTRY                 *Entry;
> -  UINTN                      BitMaskSize;
>    BOOLEAN                    FeatureExist;
> 
> -  BitMaskSize     = PcdGetSize (PcdCpuFeaturesSetting);
> -  CpuFeaturesData = GetCpuFeaturesData ();
> -  if (CpuFeaturesData->FeaturesCount == 0) {
> -    InitializeListHead (&CpuFeaturesData->FeatureList);
> -    InitializeSpinLock (&CpuFeaturesData->CpuFlags.MemoryMappedLock);
> -    InitializeSpinLock (&CpuFeaturesData->CpuFlags.ConsoleLogLock);
> -    //
> -    // Driver has assumption that these three PCD should has same buffer size.
> -    //
> -    ASSERT (PcdGetSize (PcdCpuFeaturesSetting) == PcdGetSize
> (PcdCpuFeaturesCapability));
> -    ASSERT (PcdGetSize (PcdCpuFeaturesSetting) == PcdGetSize
> (PcdCpuFeaturesSupport));
> -    CpuFeaturesData->BitMaskSize = (UINT32) BitMaskSize;
> -  }
> -  ASSERT (CpuFeaturesData->BitMaskSize == BitMaskSize);
> -
>    FeatureExist = FALSE;
>    CpuFeatureEntry = NULL;
>    Entry = GetFirstNode (&CpuFeaturesData->FeatureList);
>    while (!IsNull (&CpuFeaturesData->FeatureList, Entry)) {
>      CpuFeatureEntry = CPU_FEATURE_ENTRY_FROM_LINK (Entry);
> -    if (IsCpuFeatureMatch (CpuFeature->FeatureMask, CpuFeatureEntry-
> >FeatureMask)) {
> +    if (IsBitMaskMatchCheck (CpuFeature->FeatureMask,
> + CpuFeatureEntry->FeatureMask)) {
>        //
>        // If this feature already registered
>        //
> @@ -684,12 +637,12 @@ RegisterCpuFeatureWorker (
> 
>    if (!FeatureExist) {
>      DEBUG ((DEBUG_INFO, "[NEW] "));
> -    DumpCpuFeature (CpuFeature);
> +    DumpCpuFeature (CpuFeature, CpuFeaturesData->BitMaskSize);
>      InsertTailList (&CpuFeaturesData->FeatureList, &CpuFeature->Link);
>      CpuFeaturesData->FeaturesCount++;
>    } else {
>      DEBUG ((DEBUG_INFO, "[OVERRIDE] "));
> -    DumpCpuFeature (CpuFeature);
> +    DumpCpuFeature (CpuFeature, CpuFeaturesData->BitMaskSize);
>      ASSERT (CpuFeatureEntry != NULL);
>      //
>      // Overwrite original parameters of CPU feature @@ -849,7 +802,6 @@
> RegisterCpuFeature (
>    EFI_STATUS                 Status;
>    VA_LIST                    Marker;
>    UINT32                     Feature;
> -  UINTN                      BitMaskSize;
>    CPU_FEATURES_ENTRY         *CpuFeature;
>    UINT8                      *FeatureMask;
>    UINT8                      *BeforeFeatureBitMask;
> @@ -860,6 +812,7 @@ RegisterCpuFeature (
>    UINT8                      *PackageAfterFeatureBitMask;
>    BOOLEAN                    BeforeAll;
>    BOOLEAN                    AfterAll;
> +  CPU_FEATURES_DATA          *CpuFeaturesData;
> 
>    FeatureMask                 = NULL;
>    BeforeFeatureBitMask        = NULL;
> @@ -871,7 +824,18 @@ RegisterCpuFeature (
>    BeforeAll            = FALSE;
>    AfterAll             = FALSE;
> 
> -  BitMaskSize = PcdGetSize (PcdCpuFeaturesSetting);
> +  CpuFeaturesData = GetCpuFeaturesData ();  if
> + (CpuFeaturesData->FeaturesCount == 0) {
> +    InitializeListHead (&CpuFeaturesData->FeatureList);
> +    InitializeSpinLock (&CpuFeaturesData->CpuFlags.MemoryMappedLock);
> +    InitializeSpinLock (&CpuFeaturesData->CpuFlags.ConsoleLogLock);
> +    //
> +    // Driver has assumption that below three PCDs should has same buffer
> size.
> +    //
> +    ASSERT (PcdGetSize (PcdCpuFeaturesSetting) == PcdGetSize
> (PcdCpuFeaturesCapability));
> +    ASSERT (PcdGetSize (PcdCpuFeaturesSetting) == PcdGetSize
> (PcdCpuFeaturesSupport));
> +    CpuFeaturesData->BitMaskSize = (UINT32) PcdGetSize
> + (PcdCpuFeaturesSetting);  }
> 
>    VA_START (Marker, InitializeFunc);
>    Feature = VA_ARG (Marker, UINT32);
> @@ -889,19 +853,19 @@ RegisterCpuFeature (
>        AfterAll  = ((Feature & CPU_FEATURE_AFTER_ALL) != 0) ? TRUE : FALSE;
>        Feature  &= ~(CPU_FEATURE_BEFORE_ALL | CPU_FEATURE_AFTER_ALL);
>        ASSERT (FeatureMask == NULL);
> -      SetCpuFeaturesBitMask (&FeatureMask, Feature, BitMaskSize);
> +      SetCpuFeaturesBitMask (&FeatureMask, Feature,
> + CpuFeaturesData->BitMaskSize);
>      } else if ((Feature & CPU_FEATURE_BEFORE) != 0) {
> -      SetCpuFeaturesBitMask (&BeforeFeatureBitMask, Feature &
> ~CPU_FEATURE_BEFORE, BitMaskSize);
> +      SetCpuFeaturesBitMask (&BeforeFeatureBitMask, Feature &
> + ~CPU_FEATURE_BEFORE, CpuFeaturesData->BitMaskSize);
>      } else if ((Feature & CPU_FEATURE_AFTER) != 0) {
> -      SetCpuFeaturesBitMask (&AfterFeatureBitMask, Feature &
> ~CPU_FEATURE_AFTER, BitMaskSize);
> +      SetCpuFeaturesBitMask (&AfterFeatureBitMask, Feature &
> + ~CPU_FEATURE_AFTER, CpuFeaturesData->BitMaskSize);
>      } else if ((Feature & CPU_FEATURE_CORE_BEFORE) != 0) {
> -      SetCpuFeaturesBitMask (&CoreBeforeFeatureBitMask, Feature &
> ~CPU_FEATURE_CORE_BEFORE, BitMaskSize);
> +      SetCpuFeaturesBitMask (&CoreBeforeFeatureBitMask, Feature &
> + ~CPU_FEATURE_CORE_BEFORE, CpuFeaturesData->BitMaskSize);
>      } else if ((Feature & CPU_FEATURE_CORE_AFTER) != 0) {
> -      SetCpuFeaturesBitMask (&CoreAfterFeatureBitMask, Feature &
> ~CPU_FEATURE_CORE_AFTER, BitMaskSize);
> +      SetCpuFeaturesBitMask (&CoreAfterFeatureBitMask, Feature &
> + ~CPU_FEATURE_CORE_AFTER, CpuFeaturesData->BitMaskSize);
>      } else if ((Feature & CPU_FEATURE_PACKAGE_BEFORE) != 0) {
> -      SetCpuFeaturesBitMask (&PackageBeforeFeatureBitMask, Feature &
> ~CPU_FEATURE_PACKAGE_BEFORE, BitMaskSize);
> +      SetCpuFeaturesBitMask (&PackageBeforeFeatureBitMask, Feature &
> + ~CPU_FEATURE_PACKAGE_BEFORE, CpuFeaturesData->BitMaskSize);
>      } else if ((Feature & CPU_FEATURE_PACKAGE_AFTER) != 0) {
> -      SetCpuFeaturesBitMask (&PackageAfterFeatureBitMask, Feature &
> ~CPU_FEATURE_PACKAGE_AFTER, BitMaskSize);
> +      SetCpuFeaturesBitMask (&PackageAfterFeatureBitMask, Feature &
> + ~CPU_FEATURE_PACKAGE_AFTER, CpuFeaturesData->BitMaskSize);
>      }
>      Feature = VA_ARG (Marker, UINT32);
>    }
> @@ -929,7 +893,7 @@ RegisterCpuFeature (
>      ASSERT_EFI_ERROR (Status);
>    }
> 
> -  Status = RegisterCpuFeatureWorker (CpuFeature);
> +  Status = RegisterCpuFeatureWorker (CpuFeaturesData, CpuFeature);
>    ASSERT_EFI_ERROR (Status);
> 
>    return RETURN_SUCCESS;
> --
> 2.21.0.windows.1
> 
> 
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#43655): https://edk2.groups.io/g/devel/message/43655
Mute This Topic: https://groups.io/mt/32437608/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-





More information about the edk2-devel-archive mailing list