[edk2-devel] [PATCH 5/6] MdeModulePkg/SMM: Support to unregister SMI handler in SMI handlers

["Zhiguang Liu" <zhiguang.liu@intel.com>]

Unregistering SMI handler will free the SMI_HANDLER. However, the
SmiManage() may be using the link node from SMI_HANDLER for loop if
the unregistering happens in SMI handlers.
To avoid that, the idea is to inform SmiHandlerUnRegister() whether
it's running or not running on the stack of SmiManage(), and to
postpone SMI_HANDLER deletion before SmiManage() returns.
Noted SmiManage() may be called recursively, the SmiHandlerUnRegister()
won't take effect until the root SmiManage() returns.

Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
---
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.h |   1 +
 MdeModulePkg/Core/PiSmmCore/Smi.c       | 105 ++++++++++++++++++++++--
 2 files changed, 100 insertions(+), 6 deletions(-)

diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
index b8a490a8c3..6cc32e94d8 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
@@ -93,6 +93,7 @@ typedef struct {
   SMI_ENTRY                       *SmiEntry;
   VOID                            *Context;    // for profile
   UINTN                           ContextSize; // for profile
+  BOOLEAN                         NeedDeleted; // To delete this SMI_HANDLER later
 } SMI_HANDLER;
 
 //
diff --git a/MdeModulePkg/Core/PiSmmCore/Smi.c b/MdeModulePkg/Core/PiSmmCore/Smi.c
index 2985f989c3..b76aaf67e4 100644
--- a/MdeModulePkg/Core/PiSmmCore/Smi.c
+++ b/MdeModulePkg/Core/PiSmmCore/Smi.c
@@ -8,6 +8,11 @@
 
 #include "PiSmmCore.h"
 
+//
+// mSmiManageCallingDepth is used to track the depth of recursive calls of SmiManage.
+//
+UINTN  mSmiManageCallingDepth = 0;
+
 LIST_ENTRY  mSmiEntryList = INITIALIZE_LIST_HEAD_VARIABLE (mSmiEntryList);
 
 SMI_ENTRY  mRootSmiEntry = {
@@ -104,13 +109,15 @@ SmiManage (
 {
   LIST_ENTRY   *Link;
   LIST_ENTRY   *Head;
+  LIST_ENTRY   *EntryLink;
   SMI_ENTRY    *SmiEntry;
   SMI_HANDLER  *SmiHandler;
   BOOLEAN      SuccessReturn;
+  BOOLEAN      CanReturn;
   EFI_STATUS   Status;
 
   PERF_FUNCTION_BEGIN ();
-
+  mSmiManageCallingDepth++;
   Status        = EFI_NOT_FOUND;
   SuccessReturn = FALSE;
   if (HandlerType == NULL) {
@@ -152,7 +159,12 @@ SmiManage (
         //
         if (HandlerType != NULL) {
           PERF_FUNCTION_END ();
-          return EFI_INTERRUPT_PENDING;
+          //
+          // Won't go to next Handler, and will return with EFI_INTERRUPT_PENDING later
+          //
+          SuccessReturn = FALSE;
+          Status        = EFI_INTERRUPT_PENDING;
+          CanReturn     = TRUE;
         }
 
         break;
@@ -165,7 +177,10 @@ SmiManage (
         //
         if (HandlerType != NULL) {
           PERF_FUNCTION_END ();
-          return EFI_SUCCESS;
+          //
+          // Won't go to next Handler, and return with EFI_SUCCESS
+          //
+          CanReturn = TRUE;
         }
 
         SuccessReturn = TRUE;
@@ -193,12 +208,79 @@ SmiManage (
         ASSERT (FALSE);
         break;
     }
+
+    if (CanReturn) {
+      break;
+    }
   }
 
   if (SuccessReturn) {
     Status = EFI_SUCCESS;
   }
 
+  ASSERT (mSmiManageCallingDepth > 0);
+  mSmiManageCallingDepth--;
+
+  //
+  // The SmiHandlerUnRegister won't take effect inside SmiManage.
+  // Before returned from SmiManage, delete the SmiHandler which is
+  // marked as NeedDeleted.
+  // Note that SmiManage can be called recursively.
+  //
+  if (mSmiManageCallingDepth == 0) {
+    //
+    // Go through all SmiHandler in root SMI handlers
+    //
+    SmiHandler = NULL;
+    for ( Link = GetFirstNode (&mRootSmiEntry.SmiHandlers)
+          ; !IsNull (&mRootSmiEntry.SmiHandlers, Link);
+          )
+    {
+      SmiHandler = CR (Link, SMI_HANDLER, Link, SMI_HANDLER_SIGNATURE);
+      Link       = GetNextNode (&mRootSmiEntry.SmiHandlers, Link);
+      if (SmiHandler->NeedDeleted) {
+        //
+        // Remove SmiHandler if the NeedDeleted is set.
+        //
+        RemoveEntryList (&SmiHandler->Link);
+        FreePool (SmiHandler);
+      }
+    }
+
+    //
+    // Go through all SmiHandler in non-root SMI handlers
+    //
+    for ( EntryLink = GetFirstNode (&mSmiEntryList)
+          ; !IsNull (&mSmiEntryList, EntryLink);
+          )
+    {
+      SmiEntry  = CR (EntryLink, SMI_ENTRY, AllEntries, SMI_ENTRY_SIGNATURE);
+      EntryLink = GetNextNode (&mSmiEntryList, EntryLink);
+      for ( Link = GetFirstNode (&SmiEntry->SmiHandlers)
+            ; !IsNull (&SmiEntry->SmiHandlers, Link);
+            )
+      {
+        SmiHandler = CR (Link, SMI_HANDLER, Link, SMI_HANDLER_SIGNATURE);
+        Link       = GetNextNode (&SmiEntry->SmiHandlers, Link);
+        if (SmiHandler->NeedDeleted) {
+          //
+          // Remove SmiHandler if the NeedDeleted is set.
+          //
+          RemoveEntryList (&SmiHandler->Link);
+          FreePool (SmiHandler);
+        }
+      }
+
+      if (IsListEmpty (&SmiEntry->SmiHandlers)) {
+        //
+        // No handler registered for this SmiEntry now, remove the SMI_ENTRY
+        //
+        RemoveEntryList (&SmiEntry->AllEntries);
+        FreePool (SmiEntry);
+      }
+    }
+  }
+
   PERF_FUNCTION_END ();
   return Status;
 }
@@ -235,9 +317,10 @@ SmiHandlerRegister (
     return EFI_OUT_OF_RESOURCES;
   }
 
-  SmiHandler->Signature  = SMI_HANDLER_SIGNATURE;
-  SmiHandler->Handler    = Handler;
-  SmiHandler->CallerAddr = (UINTN)RETURN_ADDRESS (0);
+  SmiHandler->Signature   = SMI_HANDLER_SIGNATURE;
+  SmiHandler->Handler     = Handler;
+  SmiHandler->CallerAddr  = (UINTN)RETURN_ADDRESS (0);
+  SmiHandler->NeedDeleted = FALSE;
 
   if (HandlerType == NULL) {
     //
@@ -324,6 +407,16 @@ SmiHandlerUnRegister (
 
   SmiEntry = SmiHandler->SmiEntry;
 
+  if (mSmiManageCallingDepth > 0) {
+    //
+    // This function is called from SmiManage()
+    // Do not delete or remove SmiHandler or SmiEntry now.
+    // Set the NeedDeleted field in SmiHandler so that SmiManage will delete it later
+    //
+    SmiHandler->NeedDeleted = TRUE;
+    return EFI_SUCCESS;
+  }
+
   RemoveEntryList (&SmiHandler->Link);
   FreePool (SmiHandler);
 
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116831): https://edk2.groups.io/g/devel/message/116831
Mute This Topic: https://groups.io/mt/104996185/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-