From: Eric Dong <eric.dong@intel.com>
To: edk2-devel@lists.01.org
Cc: Ruiyu Ni <ruiyu.ni@intel.com>, Laszlo Ersek <lersek@redhat.com>
Subject: [Patch] UefiCpuPkg/RegisterCpuFeaturesLib: Support combo CPU feature style.
Date: Thu, 25 Oct 2018 14:50:22 +0800 [thread overview]
Message-ID: <20181025065022.25884-1-eric.dong@intel.com> (raw)
Current code assume only one dependence (before or after) for one
feature. Enhance code logic to support feature has two dependence
(before and after) type.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
---
.../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c | 5 +-
.../RegisterCpuFeaturesLib/RegisterCpuFeatures.h | 8 +-
.../RegisterCpuFeaturesLib.c | 99 ++++++++--------------
3 files changed, 45 insertions(+), 67 deletions(-)
diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
index 173f2edbea..bc372a338f 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
@@ -671,10 +671,11 @@ AnalysisProcessorFeatures (
// If feature has dependence with the next feature (ONLY care core/package dependency).
// and feature initialize succeed, add sync semaphere here.
//
- BeforeDep = DetectFeatureScope (CpuFeatureInOrder, TRUE);
if (NextCpuFeatureInOrder != NULL) {
- AfterDep = DetectFeatureScope (NextCpuFeatureInOrder, FALSE);
+ BeforeDep = DetectFeatureScope (CpuFeatureInOrder, TRUE, NextCpuFeatureInOrder->FeatureMask);
+ AfterDep = DetectFeatureScope (NextCpuFeatureInOrder, FALSE, CpuFeatureInOrder->FeatureMask);
} else {
+ BeforeDep = DetectFeatureScope (CpuFeatureInOrder, TRUE, NULL);
AfterDep = NoneDepType;
}
//
diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
index 42a3f91fbf..b5fe8fbce1 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
@@ -193,15 +193,17 @@ DumpCpuFeature (
/**
Return feature dependence result.
- @param[in] CpuFeature Pointer to CPU feature.
- @param[in] Before Check before dependence or after.
+ @param[in] CpuFeature Pointer to CPU feature.
+ @param[in] Before Check before dependence or after.
+ @param[in] NextCpuFeatureMask Pointer to next CPU feature Mask.
@retval return the dependence result.
**/
CPU_FEATURE_DEPENDENCE_TYPE
DetectFeatureScope (
IN CPU_FEATURES_ENTRY *CpuFeature,
- IN BOOLEAN Before
+ IN BOOLEAN Before,
+ IN CHAR8 *NextCpuFeatureMask
);
/**
diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
index b6e108b8ad..9a66bc49ff 100644
--- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
+++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
@@ -115,90 +115,69 @@ IsBitMaskMatchCheck (
/**
Return feature dependence result.
- @param[in] CpuFeature Pointer to CPU feature.
- @param[in] Before Check before dependence or after.
+ @param[in] CpuFeature Pointer to CPU feature.
+ @param[in] Before Check before dependence or after.
+ @param[in] NextCpuFeatureMask Pointer to next CPU feature Mask.
@retval return the dependence result.
**/
CPU_FEATURE_DEPENDENCE_TYPE
DetectFeatureScope (
IN CPU_FEATURES_ENTRY *CpuFeature,
- IN BOOLEAN Before
+ IN BOOLEAN Before,
+ IN CHAR8 *NextCpuFeatureMask
)
{
+ //
+ // if need to check before type dependence but the feature after current feature is not
+ // exist, means this before type dependence not valid, just return NoneDepType.
+ // Just like Feature A has a dependence of feature B, but Feature B not installed, so
+ // Feature A maybe insert to the last entry of the list. In this case, for below code,
+ // Featrure A has depend of feature B, but it is the last entry of the list, so the
+ // NextCpuFeatureMask is NULL, so the dependence for feature A here is useless and code
+ // just return NoneDepType.
+ //
+ if (NextCpuFeatureMask == NULL) {
+ return NoneDepType;
+ }
+
if (Before) {
- if (CpuFeature->PackageBeforeFeatureBitMask != NULL) {
+ if ((CpuFeature->PackageBeforeFeatureBitMask != NULL) &&
+ IsBitMaskMatchCheck (NextCpuFeatureMask, CpuFeature->PackageBeforeFeatureBitMask)) {
return PackageDepType;
}
- if (CpuFeature->CoreBeforeFeatureBitMask != NULL) {
+ if ((CpuFeature->CoreBeforeFeatureBitMask != NULL) &&
+ IsBitMaskMatchCheck (NextCpuFeatureMask, CpuFeature->CoreBeforeFeatureBitMask)) {
return CoreDepType;
}
- if (CpuFeature->BeforeFeatureBitMask != NULL) {
+ if ((CpuFeature->BeforeFeatureBitMask != NULL) &&
+ IsBitMaskMatchCheck (NextCpuFeatureMask, CpuFeature->BeforeFeatureBitMask)) {
return ThreadDepType;
}
return NoneDepType;
}
- if (CpuFeature->PackageAfterFeatureBitMask != NULL) {
+ if ((CpuFeature->PackageAfterFeatureBitMask != NULL) &&
+ IsBitMaskMatchCheck (NextCpuFeatureMask, CpuFeature->PackageAfterFeatureBitMask)) {
return PackageDepType;
}
- if (CpuFeature->CoreAfterFeatureBitMask != NULL) {
+ if ((CpuFeature->CoreAfterFeatureBitMask != NULL) &&
+ IsBitMaskMatchCheck (NextCpuFeatureMask, CpuFeature->CoreAfterFeatureBitMask)) {
return CoreDepType;
}
- if (CpuFeature->AfterFeatureBitMask != NULL) {
+ if ((CpuFeature->AfterFeatureBitMask != NULL) &&
+ IsBitMaskMatchCheck (NextCpuFeatureMask, CpuFeature->AfterFeatureBitMask)) {
return ThreadDepType;
}
return NoneDepType;
}
-/**
- Clear dependence for the specified type.
-
- @param[in] CpuFeature Cpu feature need to clear.
- @param[in] Before Before or after dependence relationship.
-
-**/
-VOID
-ClearFeatureScope (
- IN CPU_FEATURES_ENTRY *CpuFeature,
- IN BOOLEAN Before
- )
-{
- if (Before) {
- if (CpuFeature->BeforeFeatureBitMask != NULL) {
- FreePool (CpuFeature->BeforeFeatureBitMask);
- CpuFeature->BeforeFeatureBitMask = NULL;
- }
- if (CpuFeature->CoreBeforeFeatureBitMask != NULL) {
- FreePool (CpuFeature->CoreBeforeFeatureBitMask);
- CpuFeature->CoreBeforeFeatureBitMask = NULL;
- }
- if (CpuFeature->PackageBeforeFeatureBitMask != NULL) {
- FreePool (CpuFeature->PackageBeforeFeatureBitMask);
- CpuFeature->PackageBeforeFeatureBitMask = NULL;
- }
- } else {
- if (CpuFeature->PackageAfterFeatureBitMask != NULL) {
- FreePool (CpuFeature->PackageAfterFeatureBitMask);
- CpuFeature->PackageAfterFeatureBitMask = NULL;
- }
- if (CpuFeature->CoreAfterFeatureBitMask != NULL) {
- FreePool (CpuFeature->CoreAfterFeatureBitMask);
- CpuFeature->CoreAfterFeatureBitMask = NULL;
- }
- if (CpuFeature->AfterFeatureBitMask != NULL) {
- FreePool (CpuFeature->AfterFeatureBitMask);
- CpuFeature->AfterFeatureBitMask = NULL;
- }
- }
-}
-
/**
Base on dependence relationship to asjust feature dependence.
@@ -209,6 +188,7 @@ ClearFeatureScope (
@param[in, out] PreviousFeature CPU feature current before the find one.
@param[in, out] CurrentFeature Cpu feature need to adjust.
+ @param[in] FindFeature Cpu feature which current feature depends.
@param[in] Before Before or after dependence relationship.
@retval TRUE means the current feature dependence has been adjusted.
@@ -221,14 +201,15 @@ BOOLEAN
AdjustFeaturesDependence (
IN OUT CPU_FEATURES_ENTRY *PreviousFeature,
IN OUT CPU_FEATURES_ENTRY *CurrentFeature,
+ IN CPU_FEATURES_ENTRY *FindFeature,
IN BOOLEAN Before
)
{
CPU_FEATURE_DEPENDENCE_TYPE PreDependType;
CPU_FEATURE_DEPENDENCE_TYPE CurrentDependType;
- PreDependType = DetectFeatureScope(PreviousFeature, Before);
- CurrentDependType = DetectFeatureScope(CurrentFeature, Before);
+ PreDependType = DetectFeatureScope(PreviousFeature, Before, FindFeature->FeatureMask);
+ CurrentDependType = DetectFeatureScope(CurrentFeature, Before, FindFeature->FeatureMask);
//
// If previous feature has no dependence with the find featue.
@@ -243,10 +224,8 @@ AdjustFeaturesDependence (
// processors and clear the dependence for the other one.
//
if (PreDependType >= CurrentDependType) {
- ClearFeatureScope (CurrentFeature, Before);
return TRUE;
} else {
- ClearFeatureScope (PreviousFeature, Before);
return FALSE;
}
}
@@ -271,6 +250,7 @@ AdjustEntry (
LIST_ENTRY *PreviousEntry;
CPU_FEATURES_ENTRY *PreviousFeature;
CPU_FEATURES_ENTRY *CurrentFeature;
+ CPU_FEATURES_ENTRY *FindFeature;
//
// For CPU feature which has core or package type dependence, later code need to insert
@@ -308,8 +288,9 @@ AdjustEntry (
// If exist the previous or next entry, need to check it before insert curent entry.
//
PreviousFeature = CPU_FEATURE_ENTRY_FROM_LINK (PreviousEntry);
+ FindFeature = CPU_FEATURE_ENTRY_FROM_LINK (FindEntry);
- if (AdjustFeaturesDependence (PreviousFeature, CurrentFeature, Before)) {
+ if (AdjustFeaturesDependence (PreviousFeature, CurrentFeature, FindFeature, Before)) {
//
// Return TRUE means current feature dependence has been cleared and the previous
// feature dependence has been kept and used. So insert current feature before (or after)
@@ -486,7 +467,6 @@ CheckCpuFeaturesDependency (
if (CpuFeature->BeforeFeatureBitMask != NULL) {
Swapped = InsertToBeforeEntry (FeatureList, CurrentEntry, CpuFeature->BeforeFeatureBitMask);
if (Swapped) {
- CurrentEntry = NextEntry;
continue;
}
}
@@ -494,7 +474,6 @@ CheckCpuFeaturesDependency (
if (CpuFeature->AfterFeatureBitMask != NULL) {
Swapped = InsertToAfterEntry (FeatureList, CurrentEntry, CpuFeature->AfterFeatureBitMask);
if (Swapped) {
- CurrentEntry = NextEntry;
continue;
}
}
@@ -502,7 +481,6 @@ CheckCpuFeaturesDependency (
if (CpuFeature->CoreBeforeFeatureBitMask != NULL) {
Swapped = InsertToBeforeEntry (FeatureList, CurrentEntry, CpuFeature->CoreBeforeFeatureBitMask);
if (Swapped) {
- CurrentEntry = NextEntry;
continue;
}
}
@@ -510,7 +488,6 @@ CheckCpuFeaturesDependency (
if (CpuFeature->CoreAfterFeatureBitMask != NULL) {
Swapped = InsertToAfterEntry (FeatureList, CurrentEntry, CpuFeature->CoreAfterFeatureBitMask);
if (Swapped) {
- CurrentEntry = NextEntry;
continue;
}
}
@@ -518,7 +495,6 @@ CheckCpuFeaturesDependency (
if (CpuFeature->PackageBeforeFeatureBitMask != NULL) {
Swapped = InsertToBeforeEntry (FeatureList, CurrentEntry, CpuFeature->PackageBeforeFeatureBitMask);
if (Swapped) {
- CurrentEntry = NextEntry;
continue;
}
}
@@ -526,7 +502,6 @@ CheckCpuFeaturesDependency (
if (CpuFeature->PackageAfterFeatureBitMask != NULL) {
Swapped = InsertToAfterEntry (FeatureList, CurrentEntry, CpuFeature->PackageAfterFeatureBitMask);
if (Swapped) {
- CurrentEntry = NextEntry;
continue;
}
}
--
2.15.0.windows.1
next reply other threads:[~2018-10-25 6:50 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-25 6:50 Eric Dong [this message]
2018-10-26 4:16 ` [Patch] UefiCpuPkg/RegisterCpuFeaturesLib: Support combo CPU feature style Ni, Ruiyu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181025065022.25884-1-eric.dong@intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox