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@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Chandana Kumar <chandana.c.kumar@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Signed-off-by: Eric Dong <eric.dong@intel.com>
---
.../CpuFeaturesInitialize.c | 64 +++++++--------
.../RegisterCpuFeatures.h | 10 ++-
.../RegisterCpuFeaturesLib.c | 79 +++++++++----------
3 files changed, 76 insertions(+), 77 deletions(-)
diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
index 1e25ba8b32..33752c1a9f 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
@@ -15,20 +15,16 @@ CHAR16 *mRegisterTypeStr[] = {L"MSR", L"CR", L"MMIO", L"CACHE", L"SEMAP", L"INVA
Worker function to save PcdCpuFeaturesCapability.
@param[in] SupportedFeatureMask The pointer to CPU feature bits mask buffer
- @param[in] FeatureMaskSize CPU feature bits mask buffer size.
+ @param[in] BitMaskSize CPU feature bits mask buffer size.
**/
VOID
SetCapabilityPcd (
IN UINT8 *SupportedFeatureMask,
- IN UINT32 FeatureMaskSize
+ IN UINTN BitMaskSize
)
{
EFI_STATUS Status;
- UINTN BitMaskSize;
-
- BitMaskSize = PcdGetSize (PcdCpuFeaturesCapability);
- ASSERT (FeatureMaskSize == BitMaskSize);
Status = PcdSetPtrS (PcdCpuFeaturesCapability, &BitMaskSize, 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] BitMaskSize CPU feature bits mask buffer size.
**/
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..796d722498 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
@@ -23,31 +23,29 @@ IsCpuFeatureMatch (
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,7 +71,7 @@ DumpCpuFeature (
DEBUG ((DEBUG_INFO, "FeatureName: %a\n", CpuFeature->FeatureName));
} else {
DEBUG ((DEBUG_INFO, "FeatureMask = "));
- DumpCpuFeatureMask (CpuFeature->FeatureMask);
+ DumpCpuFeatureMask (CpuFeature->FeatureMask, BitMaskSize);
}
}
@@ -631,6 +632,7 @@ CheckCpuFeaturesDependency (
/**
Worker function to register CPU Feature.
+ @param[in] CpuFeaturesData Pointer to CPU feature data structure.
@param[in] CpuFeature Pointer to CPU feature entry
@retval RETURN_SUCCESS The CPU feature was successfully registered.
@@ -642,31 +644,15 @@ CheckCpuFeaturesDependency (
**/
RETURN_STATUS
RegisterCpuFeatureWorker (
+ IN CPU_FEATURES_DATA *CpuFeaturesData,
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);
@@ -684,12 +670,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 +835,6 @@ RegisterCpuFeature (
EFI_STATUS Status;
VA_LIST Marker;
UINT32 Feature;
- UINTN BitMaskSize;
CPU_FEATURES_ENTRY *CpuFeature;
UINT8 *FeatureMask;
UINT8 *BeforeFeatureBitMask;
@@ -860,6 +845,7 @@ RegisterCpuFeature (
UINT8 *PackageAfterFeatureBitMask;
BOOLEAN BeforeAll;
BOOLEAN AfterAll;
+ CPU_FEATURES_DATA *CpuFeaturesData;
FeatureMask = NULL;
BeforeFeatureBitMask = NULL;
@@ -871,7 +857,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);
+ //
+ // Code assumes below three PCDs have PCD 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 +886,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 +926,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 (#43707): https://edk2.groups.io/g/devel/message/43707
Mute This Topic: https://groups.io/mt/32475993/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Reviewed-by: Ray Ni <ray.ni@intel.com>
> -----Original Message-----
> From: Dong, Eric
> Sent: Monday, July 15, 2019 3:01 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; Kumar, Chandana C <chandana.c.kumar@intel.com>; Zeng,
> Star <star.zeng@intel.com>
> Subject: [Patch v2 2/2] UefiCpuPkg/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@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Chandana Kumar <chandana.c.kumar@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> ---
> .../CpuFeaturesInitialize.c | 64 +++++++--------
> .../RegisterCpuFeatures.h | 10 ++-
> .../RegisterCpuFeaturesLib.c | 79 +++++++++----------
> 3 files changed, 76 insertions(+), 77 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> index 1e25ba8b32..33752c1a9f 100644
> --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> @@ -15,20 +15,16 @@ CHAR16 *mRegisterTypeStr[] = {L"MSR", L"CR", L"MMIO", L"CACHE", L"SEMAP", L"INVA
> Worker function to save PcdCpuFeaturesCapability.
>
> @param[in] SupportedFeatureMask The pointer to CPU feature bits mask buffer
> - @param[in] FeatureMaskSize CPU feature bits mask buffer size.
> + @param[in] BitMaskSize CPU feature bits mask buffer size.
>
> **/
> VOID
> SetCapabilityPcd (
> IN UINT8 *SupportedFeatureMask,
> - IN UINT32 FeatureMaskSize
> + IN UINTN BitMaskSize
> )
> {
> EFI_STATUS Status;
> - UINTN BitMaskSize;
> -
> - BitMaskSize = PcdGetSize (PcdCpuFeaturesCapability);
> - ASSERT (FeatureMaskSize == BitMaskSize);
>
> Status = PcdSetPtrS (PcdCpuFeaturesCapability, &BitMaskSize, 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] BitMaskSize CPU feature bits mask buffer size.
> **/
> 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..796d722498 100644
> --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> @@ -23,31 +23,29 @@ IsCpuFeatureMatch (
> 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,7 +71,7 @@ DumpCpuFeature (
> DEBUG ((DEBUG_INFO, "FeatureName: %a\n", CpuFeature->FeatureName));
> } else {
> DEBUG ((DEBUG_INFO, "FeatureMask = "));
> - DumpCpuFeatureMask (CpuFeature->FeatureMask);
> + DumpCpuFeatureMask (CpuFeature->FeatureMask, BitMaskSize);
> }
> }
>
> @@ -631,6 +632,7 @@ CheckCpuFeaturesDependency (
> /**
> Worker function to register CPU Feature.
>
> + @param[in] CpuFeaturesData Pointer to CPU feature data structure.
> @param[in] CpuFeature Pointer to CPU feature entry
>
> @retval RETURN_SUCCESS The CPU feature was successfully registered.
> @@ -642,31 +644,15 @@ CheckCpuFeaturesDependency (
> **/
> RETURN_STATUS
> RegisterCpuFeatureWorker (
> + IN CPU_FEATURES_DATA *CpuFeaturesData,
> 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);
> @@ -684,12 +670,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 +835,6 @@ RegisterCpuFeature (
> EFI_STATUS Status;
> VA_LIST Marker;
> UINT32 Feature;
> - UINTN BitMaskSize;
> CPU_FEATURES_ENTRY *CpuFeature;
> UINT8 *FeatureMask;
> UINT8 *BeforeFeatureBitMask;
> @@ -860,6 +845,7 @@ RegisterCpuFeature (
> UINT8 *PackageAfterFeatureBitMask;
> BOOLEAN BeforeAll;
> BOOLEAN AfterAll;
> + CPU_FEATURES_DATA *CpuFeaturesData;
>
> FeatureMask = NULL;
> BeforeFeatureBitMask = NULL;
> @@ -871,7 +857,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);
> + //
> + // Code assumes below three PCDs have PCD 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 +886,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 +926,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 (#43709): https://edk2.groups.io/g/devel/message/43709
Mute This Topic: https://groups.io/mt/32475993/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
IsBitMaskMatchCheck() is still using PcdGetSize (PcdCpuFeaturesSetting).
Compared to V1 patch, I guess the patch wants to
1. add BitMaskSize input parameter for IsCpuFeatureMatch().
2. use GetCpuFeaturesData () in IsBitMaskMatchCheck().
Right?
Thanks,
Star
> -----Original Message-----
> From: Dong, Eric
> Sent: Monday, July 15, 2019 3:01 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; Kumar,
> Chandana C <chandana.c.kumar@intel.com>; Zeng, Star
> <star.zeng@intel.com>
> Subject: [Patch v2 2/2] UefiCpuPkg/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@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Chandana Kumar <chandana.c.kumar@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> ---
> .../CpuFeaturesInitialize.c | 64 +++++++--------
> .../RegisterCpuFeatures.h | 10 ++-
> .../RegisterCpuFeaturesLib.c | 79 +++++++++----------
> 3 files changed, 76 insertions(+), 77 deletions(-)
>
> diff --git
> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> index 1e25ba8b32..33752c1a9f 100644
> --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> @@ -15,20 +15,16 @@ CHAR16 *mRegisterTypeStr[] = {L"MSR", L"CR",
> L"MMIO", L"CACHE", L"SEMAP", L"INVA
> Worker function to save PcdCpuFeaturesCapability.
>
> @param[in] SupportedFeatureMask The pointer to CPU feature bits mask
> buffer
> - @param[in] FeatureMaskSize CPU feature bits mask buffer size.
> + @param[in] BitMaskSize CPU feature bits mask buffer size.
>
> **/
> VOID
> SetCapabilityPcd (
> IN UINT8 *SupportedFeatureMask,
> - IN UINT32 FeatureMaskSize
> + IN UINTN BitMaskSize
> )
> {
> EFI_STATUS Status;
> - UINTN BitMaskSize;
> -
> - BitMaskSize = PcdGetSize (PcdCpuFeaturesCapability);
> - ASSERT (FeatureMaskSize == BitMaskSize);
>
> Status = PcdSetPtrS (PcdCpuFeaturesCapability, &BitMaskSize,
> 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] BitMaskSize CPU feature bits mask buffer size.
> **/
> 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..796d722498 100644
> --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> +++
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
> @@ -23,31 +23,29 @@ IsCpuFeatureMatch (
> 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,7 +71,7 @@ DumpCpuFeature (
> DEBUG ((DEBUG_INFO, "FeatureName: %a\n", CpuFeature-
> >FeatureName));
> } else {
> DEBUG ((DEBUG_INFO, "FeatureMask = "));
> - DumpCpuFeatureMask (CpuFeature->FeatureMask);
> + DumpCpuFeatureMask (CpuFeature->FeatureMask, BitMaskSize);
> }
> }
>
> @@ -631,6 +632,7 @@ CheckCpuFeaturesDependency (
> /**
> Worker function to register CPU Feature.
>
> + @param[in] CpuFeaturesData Pointer to CPU feature data structure.
> @param[in] CpuFeature Pointer to CPU feature entry
>
> @retval RETURN_SUCCESS The CPU feature was successfully
> registered.
> @@ -642,31 +644,15 @@ CheckCpuFeaturesDependency ( **/
> RETURN_STATUS RegisterCpuFeatureWorker (
> + IN CPU_FEATURES_DATA *CpuFeaturesData,
> 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); @@ -684,12
> +670,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 +835,6 @@
> RegisterCpuFeature (
> EFI_STATUS Status;
> VA_LIST Marker;
> UINT32 Feature;
> - UINTN BitMaskSize;
> CPU_FEATURES_ENTRY *CpuFeature;
> UINT8 *FeatureMask;
> UINT8 *BeforeFeatureBitMask;
> @@ -860,6 +845,7 @@ RegisterCpuFeature (
> UINT8 *PackageAfterFeatureBitMask;
> BOOLEAN BeforeAll;
> BOOLEAN AfterAll;
> + CPU_FEATURES_DATA *CpuFeaturesData;
>
> FeatureMask = NULL;
> BeforeFeatureBitMask = NULL;
> @@ -871,7 +857,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);
> + //
> + // Code assumes below three PCDs have PCD 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 +886,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 +926,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 (#43714): https://edk2.groups.io/g/devel/message/43714
Mute This Topic: https://groups.io/mt/32475993/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2026 Red Hat, Inc.