From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.88; helo=mga01.intel.com; envelope-from=eric.dong@intel.com; receiver=edk2-devel@lists.01.org Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) (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 6AE6721131DCF for ; Wed, 7 Nov 2018 00:26:12 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 07 Nov 2018 00:26:12 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,475,1534834800"; d="scan'208";a="272048878" Received: from ydong10-win10.ccr.corp.intel.com ([10.239.9.125]) by orsmga005.jf.intel.com with ESMTP; 07 Nov 2018 00:26:10 -0800 From: Eric Dong To: edk2-devel@lists.01.org Cc: Laszlo Ersek , Ruiyu Ni Date: Wed, 7 Nov 2018 16:26:07 +0800 Message-Id: <20181107082607.6608-1-eric.dong@intel.com> X-Mailer: git-send-email 2.15.0.windows.1 Subject: [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 08:26:12 -0000 In current code logic, only adjust feature position if current CPU feature position not follow the request order. Just like Feature A need to be executed before 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. 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. Root cause is driver ignores the dependence for this case, so threads may execute not follow the dependence order. Fix this issue by change code logic to adjust feature position for CPU features which has dependence relationship. 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(-) 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 = GetNextNode (FeatureList, FindEntry); } + if (PreviousEntry == CurrentEntry) { + return; + } + CurrentFeature = CPU_FEATURE_ENTRY_FROM_LINK (CurrentEntry); RemoveEntryList (CurrentEntry); @@ -389,6 +393,56 @@ InsertToAfterEntry ( return Swapped; } +/** + 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 = FALSE; + // + // Record neighbor for later compre. + // + NextEntry = CurrentEntry->ForwardLink; + // + // Check all features in current list. + // + CheckEntry = GetFirstNode (FeatureList); + while (!IsNull (FeatureList, CheckEntry)) { + CheckFeature = 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 = (NextEntry != CurrentEntry->ForwardLink); + break; + } + CheckEntry = CheckEntry->ForwardLink; + } + + return Swapped; +} + /** Checks and adjusts CPU features order per dependency relationship. @@ -479,28 +533,28 @@ CheckCpuFeaturesDependency ( } if (CpuFeature->CoreBeforeFeatureBitMask != NULL) { - Swapped = InsertToBeforeEntry (FeatureList, CurrentEntry, CpuFeature->CoreBeforeFeatureBitMask); + Swapped = AdjustToAllEntry (FeatureList, CurrentEntry, CpuFeature->CoreBeforeFeatureBitMask, TRUE); if (Swapped) { continue; } } if (CpuFeature->CoreAfterFeatureBitMask != NULL) { - Swapped = InsertToAfterEntry (FeatureList, CurrentEntry, CpuFeature->CoreAfterFeatureBitMask); + Swapped = AdjustToAllEntry (FeatureList, CurrentEntry, CpuFeature->CoreAfterFeatureBitMask, FALSE); if (Swapped) { continue; } } if (CpuFeature->PackageBeforeFeatureBitMask != NULL) { - Swapped = InsertToBeforeEntry (FeatureList, CurrentEntry, CpuFeature->PackageBeforeFeatureBitMask); + Swapped = AdjustToAllEntry (FeatureList, CurrentEntry, CpuFeature->PackageBeforeFeatureBitMask, TRUE); if (Swapped) { continue; } } if (CpuFeature->PackageAfterFeatureBitMask != NULL) { - Swapped = InsertToAfterEntry (FeatureList, CurrentEntry, CpuFeature->PackageAfterFeatureBitMask); + Swapped = AdjustToAllEntry (FeatureList, CurrentEntry, CpuFeature->PackageAfterFeatureBitMask, FALSE); if (Swapped) { continue; } -- 2.15.0.windows.1