From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.65; helo=mga03.intel.com; envelope-from=ruiyu.ni@intel.com; receiver=edk2-devel@lists.01.org Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 887362117B57A for ; Thu, 25 Oct 2018 21:16:12 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga103.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 25 Oct 2018 21:16:12 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,426,1534834800"; d="scan'208";a="103415026" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by orsmga002.jf.intel.com with ESMTP; 25 Oct 2018 21:16:12 -0700 Received: from fmsmsx117.amr.corp.intel.com (10.18.116.17) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 25 Oct 2018 21:16:11 -0700 Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by fmsmsx117.amr.corp.intel.com (10.18.116.17) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 25 Oct 2018 21:16:11 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.117]) by shsmsx102.ccr.corp.intel.com ([169.254.2.84]) with mapi id 14.03.0415.000; Fri, 26 Oct 2018 12:16:09 +0800 From: "Ni, Ruiyu" To: "Dong, Eric" , "edk2-devel@lists.01.org" CC: Laszlo Ersek Thread-Topic: [Patch] UefiCpuPkg/RegisterCpuFeaturesLib: Support combo CPU feature style. Thread-Index: AQHUbC8NTSdF2x+2M0qxNcO/XhocCaUw7MKw Date: Fri, 26 Oct 2018 04:16:08 +0000 Deferred-Delivery: Fri, 26 Oct 2018 04:16:00 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5BEDEABC@SHSMSX104.ccr.corp.intel.com> References: <20181025065022.25884-1-eric.dong@intel.com> In-Reply-To: <20181025065022.25884-1-eric.dong@intel.com> Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZjNkYWY5NzYtMmFiOC00OWUwLTllOTYtZjk2ZmQ1MjJlNWFlIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiQnJtZFwvZkp0cUVMM3FUcXczWE5YSnYrZmh4YnJtVDFlZklUVWNVZlcycW5obzZSVkx2dFA2Y2dVZm81WFhGeFcifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [Patch] UefiCpuPkg/RegisterCpuFeaturesLib: Support combo CPU feature style. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 26 Oct 2018 04:16:13 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Ruiyu Ni > -----Original Message----- > From: Dong, Eric > Sent: Thursday, October 25, 2018 2:50 PM > To: edk2-devel@lists.01.org > Cc: Ni, Ruiyu ; Laszlo Ersek > Subject: [Patch] UefiCpuPkg/RegisterCpuFeaturesLib: Support combo CPU > feature style. >=20 > Current code assume only one dependence (before or after) for one feature= . > Enhance code logic to support feature has two dependence (before and afte= r) > type. >=20 > Cc: Ruiyu Ni > Cc: Laszlo Ersek > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Eric Dong > --- > .../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c | 5 +- > .../RegisterCpuFeaturesLib/RegisterCpuFeatures.h | 8 +- > .../RegisterCpuFeaturesLib.c | 99 ++++++++--------= ------ > 3 files changed, 45 insertions(+), 67 deletions(-) >=20 > diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitial= ize.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 =3D DetectFeatureScope (CpuFeatureInOrder, TRUE); > if (NextCpuFeatureInOrder !=3D NULL) { > - AfterDep =3D DetectFeatureScope (NextCpuFeatureInOrder, FALSE= ); > + BeforeDep =3D DetectFeatureScope (CpuFeatureInOrder, TRUE, > NextCpuFeatureInOrder->FeatureMask); > + AfterDep =3D DetectFeatureScope (NextCpuFeatureInOrder, FALSE= , > + CpuFeatureInOrder->FeatureMask); > } else { > + BeforeDep =3D DetectFeatureScope (CpuFeatureInOrder, TRUE, > + NULL); > AfterDep =3D NoneDepType; > } > // > diff --git a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeature= s.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. >=20 > - @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. >=20 > @retval return the dependence result. > **/ > CPU_FEATURE_DEPENDENCE_TYPE > DetectFeatureScope ( > IN CPU_FEATURES_ENTRY *CpuFeature, > - IN BOOLEAN Before > + IN BOOLEAN Before, > + IN CHAR8 *NextCpuFeatureMask > ); >=20 > /** > 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. >=20 > - @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. >=20 > @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 use= less > and code // just return NoneDepType. > + // > + if (NextCpuFeatureMask =3D=3D NULL) { > + return NoneDepType; > + } > + > if (Before) { > - if (CpuFeature->PackageBeforeFeatureBitMask !=3D NULL) { > + if ((CpuFeature->PackageBeforeFeatureBitMask !=3D NULL) && > + IsBitMaskMatchCheck (NextCpuFeatureMask, > + CpuFeature->PackageBeforeFeatureBitMask)) { > return PackageDepType; > } >=20 > - if (CpuFeature->CoreBeforeFeatureBitMask !=3D NULL) { > + if ((CpuFeature->CoreBeforeFeatureBitMask !=3D NULL) && > + IsBitMaskMatchCheck (NextCpuFeatureMask, > + CpuFeature->CoreBeforeFeatureBitMask)) { > return CoreDepType; > } >=20 > - if (CpuFeature->BeforeFeatureBitMask !=3D NULL) { > + if ((CpuFeature->BeforeFeatureBitMask !=3D NULL) && > + IsBitMaskMatchCheck (NextCpuFeatureMask, > + CpuFeature->BeforeFeatureBitMask)) { > return ThreadDepType; > } >=20 > return NoneDepType; > } >=20 > - if (CpuFeature->PackageAfterFeatureBitMask !=3D NULL) { > + if ((CpuFeature->PackageAfterFeatureBitMask !=3D NULL) && > + IsBitMaskMatchCheck (NextCpuFeatureMask, > + CpuFeature->PackageAfterFeatureBitMask)) { > return PackageDepType; > } >=20 > - if (CpuFeature->CoreAfterFeatureBitMask !=3D NULL) { > + if ((CpuFeature->CoreAfterFeatureBitMask !=3D NULL) && > + IsBitMaskMatchCheck (NextCpuFeatureMask, > + CpuFeature->CoreAfterFeatureBitMask)) { > return CoreDepType; > } >=20 > - if (CpuFeature->AfterFeatureBitMask !=3D NULL) { > + if ((CpuFeature->AfterFeatureBitMask !=3D NULL) && > + IsBitMaskMatchCheck (NextCpuFeatureMask, > + CpuFeature->AfterFeatureBitMask)) { > return ThreadDepType; > } >=20 > return NoneDepType; > } >=20 > -/** > - 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 !=3D NULL) { > - FreePool (CpuFeature->BeforeFeatureBitMask); > - CpuFeature->BeforeFeatureBitMask =3D NULL; > - } > - if (CpuFeature->CoreBeforeFeatureBitMask !=3D NULL) { > - FreePool (CpuFeature->CoreBeforeFeatureBitMask); > - CpuFeature->CoreBeforeFeatureBitMask =3D NULL; > - } > - if (CpuFeature->PackageBeforeFeatureBitMask !=3D NULL) { > - FreePool (CpuFeature->PackageBeforeFeatureBitMask); > - CpuFeature->PackageBeforeFeatureBitMask =3D NULL; > - } > - } else { > - if (CpuFeature->PackageAfterFeatureBitMask !=3D NULL) { > - FreePool (CpuFeature->PackageAfterFeatureBitMask); > - CpuFeature->PackageAfterFeatureBitMask =3D NULL; > - } > - if (CpuFeature->CoreAfterFeatureBitMask !=3D NULL) { > - FreePool (CpuFeature->CoreAfterFeatureBitMask); > - CpuFeature->CoreAfterFeatureBitMask =3D NULL; > - } > - if (CpuFeature->AfterFeatureBitMask !=3D NULL) { > - FreePool (CpuFeature->AfterFeatureBitMask); > - CpuFeature->AfterFeatureBitMask =3D NULL; > - } > - } > -} > - > /** > Base on dependence relationship to asjust feature dependence. >=20 > @@ -209,6 +188,7 @@ ClearFeatureScope ( >=20 > @param[in, out] PreviousFeature CPU feature current before the fin= d 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 relatio= nship. >=20 > @retval TRUE means the current feature dependence has been adjuste= d. > @@ -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; >=20 > - PreDependType =3D DetectFeatureScope(PreviousFeature, Before); > - CurrentDependType =3D DetectFeatureScope(CurrentFeature, Before); > + PreDependType =3D DetectFeatureScope(PreviousFeature, Before, > FindFeature->FeatureMask); > + CurrentDependType =3D DetectFeatureScope(CurrentFeature, Before, > + FindFeature->FeatureMask); >=20 > // > // 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 >=3D 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; >=20 > // > // For CPU feature which has core or package type dependence, later co= de > need to insert @@ -308,8 +288,9 @@ AdjustEntry ( > // If exist the previous or next entry, need to check it before inse= rt curent > entry. > // > PreviousFeature =3D CPU_FEATURE_ENTRY_FROM_LINK (PreviousEntry); > + FindFeature =3D CPU_FEATURE_ENTRY_FROM_LINK (FindEntry); >=20 > - if (AdjustFeaturesDependence (PreviousFeature, CurrentFeature, Befor= e)) { > + if (AdjustFeaturesDependence (PreviousFeature, CurrentFeature, > + FindFeature, Before)) { > // > // Return TRUE means current feature dependence has been cleared a= nd the > previous > // feature dependence has been kept and used. So insert current fe= ature > before (or after) @@ -486,7 +467,6 @@ CheckCpuFeaturesDependency ( > if (CpuFeature->BeforeFeatureBitMask !=3D NULL) { > Swapped =3D InsertToBeforeEntry (FeatureList, CurrentEntry, CpuFea= ture- > >BeforeFeatureBitMask); > if (Swapped) { > - CurrentEntry =3D NextEntry; > continue; > } > } > @@ -494,7 +474,6 @@ CheckCpuFeaturesDependency ( > if (CpuFeature->AfterFeatureBitMask !=3D NULL) { > Swapped =3D InsertToAfterEntry (FeatureList, CurrentEntry, CpuFeat= ure- > >AfterFeatureBitMask); > if (Swapped) { > - CurrentEntry =3D NextEntry; > continue; > } > } > @@ -502,7 +481,6 @@ CheckCpuFeaturesDependency ( > if (CpuFeature->CoreBeforeFeatureBitMask !=3D NULL) { > Swapped =3D InsertToBeforeEntry (FeatureList, CurrentEntry, CpuFea= ture- > >CoreBeforeFeatureBitMask); > if (Swapped) { > - CurrentEntry =3D NextEntry; > continue; > } > } > @@ -510,7 +488,6 @@ CheckCpuFeaturesDependency ( > if (CpuFeature->CoreAfterFeatureBitMask !=3D NULL) { > Swapped =3D InsertToAfterEntry (FeatureList, CurrentEntry, CpuFeat= ure- > >CoreAfterFeatureBitMask); > if (Swapped) { > - CurrentEntry =3D NextEntry; > continue; > } > } > @@ -518,7 +495,6 @@ CheckCpuFeaturesDependency ( > if (CpuFeature->PackageBeforeFeatureBitMask !=3D NULL) { > Swapped =3D InsertToBeforeEntry (FeatureList, CurrentEntry, CpuFea= ture- > >PackageBeforeFeatureBitMask); > if (Swapped) { > - CurrentEntry =3D NextEntry; > continue; > } > } > @@ -526,7 +502,6 @@ CheckCpuFeaturesDependency ( > if (CpuFeature->PackageAfterFeatureBitMask !=3D NULL) { > Swapped =3D InsertToAfterEntry (FeatureList, CurrentEntry, CpuFeat= ure- > >PackageAfterFeatureBitMask); > if (Swapped) { > - CurrentEntry =3D NextEntry; > continue; > } > } > -- > 2.15.0.windows.1