public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Eric Dong <eric.dong@intel.com>
To: edk2-devel@lists.01.org
Cc: Laszlo Ersek <lersek@redhat.com>, Ruiyu Ni <ruiyu.ni@intel.com>
Subject: [Patch] UefiCpuPkg/RegisterCpuFeaturesLib: Adjust Order.
Date: Wed,  7 Nov 2018 16:26:07 +0800	[thread overview]
Message-ID: <20181107082607.6608-1-eric.dong@intel.com> (raw)

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 <lersek@redhat.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
---
 .../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



             reply	other threads:[~2018-11-07  8:26 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-07  8:26 Eric Dong [this message]
2018-11-07 13:41 ` [Patch] UefiCpuPkg/RegisterCpuFeaturesLib: Adjust Order Laszlo Ersek
2018-11-07 15:35 ` Ni, Ruiyu
  -- strict thread matches above, loose matches on Subject: below --
2018-11-09  5:20 Eric Dong
2018-11-09 14:55 ` Ni, Ruiyu
2018-11-10  3:16   ` Dong, Eric
2018-11-10 10:00     ` Leif Lindholm
2018-11-11  2:01       ` Dong, Eric
2018-11-12  9:42         ` Laszlo Ersek

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=20181107082607.6608-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