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.31; helo=mga06.intel.com; envelope-from=ruiyu.ni@intel.com; receiver=edk2-devel@lists.01.org Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) (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 9FF2921A00AE6 for ; Wed, 7 Nov 2018 07:35:48 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga104.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 07 Nov 2018 07:35:47 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,475,1534834800"; d="scan'208";a="84690620" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by fmsmga008.fm.intel.com with ESMTP; 07 Nov 2018 07:35:47 -0800 Received: from fmsmsx112.amr.corp.intel.com (10.18.116.6) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.408.0; Wed, 7 Nov 2018 07:35:47 -0800 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by FMSMSX112.amr.corp.intel.com (10.18.116.6) with Microsoft SMTP Server (TLS) id 14.3.408.0; Wed, 7 Nov 2018 07:35:46 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.117]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.199]) with mapi id 14.03.0415.000; Wed, 7 Nov 2018 23:35:44 +0800 From: "Ni, Ruiyu" To: "Dong, Eric" , "edk2-devel@lists.01.org" CC: Laszlo Ersek Thread-Topic: [Patch] UefiCpuPkg/RegisterCpuFeaturesLib: Adjust Order. Thread-Index: AQHUdnOQVreV839ZhEGc2jlWU7tLzqVEcdeg Date: Wed, 7 Nov 2018 15:35:44 +0000 Deferred-Delivery: Wed, 7 Nov 2018 15:35:00 +0000 Message-ID: <734D49CCEBEEF84792F5B80ED585239D5BEF5B02@SHSMSX104.ccr.corp.intel.com> References: <20181107082607.6608-1-eric.dong@intel.com> In-Reply-To: <20181107082607.6608-1-eric.dong@intel.com> Accept-Language: en-US, zh-CN X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiOTVjZDRlZGItYTVjMi00ODFiLWFjMWEtMDJhN2Y4MjMyYjE3IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiVEhcL0FIN3NBaGM0TmpYNjhYZ0I1elF4dWMzVk1FMUpMN2huV2JSNkJaK0swZXQ4TVpnbFJQN1prVXZkNDFPQ1oifQ== 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: Adjust Order. 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: Wed, 07 Nov 2018 15:35:48 -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: Wednesday, November 7, 2018 4:26 PM > To: edk2-devel@lists.01.org > Cc: Laszlo Ersek ; Ni, Ruiyu > Subject: [Patch] UefiCpuPkg/RegisterCpuFeaturesLib: Adjust Order. >=20 > In current code logic, only adjust feature position if current CPU featur= e position > not follow the request order. Just like Feature A need to be executed bef= ore > feature B, but current feature A registers after feature B. So code will = adjust the > position for feature A, move it to just before feature B. If the position= already > met the requirement, code will not adjust the position. >=20 > This logic has issue when met all below cases: > 1. feature A has core or package level dependence with feature B. > 2. feature A is register before feature B. > 3. Also exist other features exist between feature A and B. >=20 > Root cause is driver ignores the dependence for this case, so threads may > execute not follow the dependence order. >=20 > Fix this issue by change code logic to adjust feature position for CPU fe= atures > which has dependence relationship. >=20 > Change-Id: I86171cb1dbf44a2f6fd8d5d2209cafee9451b866 > Cc: Laszlo Ersek > Cc: Ruiyu Ni > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Eric Dong > --- > .../RegisterCpuFeaturesLib.c | 62 ++++++++++++++++= ++++-- > 1 file changed, 58 insertions(+), 4 deletions(-) >=20 > diff --git > a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c > b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c > index 394695baf2..31a44b6bad 100644 > --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c > +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c > @@ -271,6 +271,10 @@ AdjustEntry ( > PreviousEntry =3D GetNextNode (FeatureList, FindEntry); > } >=20 > + if (PreviousEntry =3D=3D CurrentEntry) { > + return; > + } > + > CurrentFeature =3D CPU_FEATURE_ENTRY_FROM_LINK (CurrentEntry); > RemoveEntryList (CurrentEntry); >=20 > @@ -389,6 +393,56 @@ InsertToAfterEntry ( > return Swapped; > } >=20 > +/** > + Checks and adjusts current CPU features per dependency relationship. > + > + @param[in] FeatureList Pointer to CPU feature list > + @param[in] CurrentEntry Pointer to current checked CPU feature > + @param[in] FeatureMask The feature bit mask. > + @param[in] Before The dependence is before type or after = type. > + > + @retval return Swapped info. > +**/ > +BOOLEAN > +AdjustToAllEntry ( > + IN LIST_ENTRY *FeatureList, > + IN LIST_ENTRY *CurrentEntry, > + IN UINT8 *FeatureMask, > + IN BOOLEAN Before > + ) > +{ > + LIST_ENTRY *CheckEntry; > + CPU_FEATURES_ENTRY *CheckFeature; > + BOOLEAN Swapped; > + LIST_ENTRY *NextEntry; > + > + Swapped =3D FALSE; > + // > + // Record neighbor for later compre. > + // > + NextEntry =3D CurrentEntry->ForwardLink; // // Check all features in > + current list. > + // > + CheckEntry =3D GetFirstNode (FeatureList); while (!IsNull > + (FeatureList, CheckEntry)) { > + CheckFeature =3D CPU_FEATURE_ENTRY_FROM_LINK (CheckEntry); > + if (IsBitMaskMatchCheck (CheckFeature->FeatureMask, FeatureMask)) { > + AdjustEntry (FeatureList, CheckEntry, CurrentEntry, Before); > + // > + // Base on former record neighbor to detect whether current entry > + // adjust the position. > + // Current Entry position maybe changed in AdjustEntry function. > + // > + Swapped =3D (NextEntry !=3D CurrentEntry->ForwardLink); > + break; > + } > + CheckEntry =3D CheckEntry->ForwardLink; } > + > + return Swapped; > +} > + > /** > Checks and adjusts CPU features order per dependency relationship. >=20 > @@ -479,28 +533,28 @@ CheckCpuFeaturesDependency ( > } >=20 > if (CpuFeature->CoreBeforeFeatureBitMask !=3D NULL) { > - Swapped =3D InsertToBeforeEntry (FeatureList, CurrentEntry, CpuFea= ture- > >CoreBeforeFeatureBitMask); > + Swapped =3D AdjustToAllEntry (FeatureList, CurrentEntry, > + CpuFeature->CoreBeforeFeatureBitMask, TRUE); > if (Swapped) { > continue; > } > } >=20 > if (CpuFeature->CoreAfterFeatureBitMask !=3D NULL) { > - Swapped =3D InsertToAfterEntry (FeatureList, CurrentEntry, CpuFeat= ure- > >CoreAfterFeatureBitMask); > + Swapped =3D AdjustToAllEntry (FeatureList, CurrentEntry, > + CpuFeature->CoreAfterFeatureBitMask, FALSE); > if (Swapped) { > continue; > } > } >=20 > if (CpuFeature->PackageBeforeFeatureBitMask !=3D NULL) { > - Swapped =3D InsertToBeforeEntry (FeatureList, CurrentEntry, CpuFea= ture- > >PackageBeforeFeatureBitMask); > + Swapped =3D AdjustToAllEntry (FeatureList, CurrentEntry, > + CpuFeature->PackageBeforeFeatureBitMask, TRUE); > if (Swapped) { > continue; > } > } >=20 > if (CpuFeature->PackageAfterFeatureBitMask !=3D NULL) { > - Swapped =3D InsertToAfterEntry (FeatureList, CurrentEntry, CpuFeat= ure- > >PackageAfterFeatureBitMask); > + Swapped =3D AdjustToAllEntry (FeatureList, CurrentEntry, > + CpuFeature->PackageAfterFeatureBitMask, FALSE); > if (Swapped) { > continue; > } > -- > 2.15.0.windows.1