public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Zhiguang Liu" <zhiguang.liu@intel.com>
To: devel@edk2.groups.io
Cc: Zhiguang Liu <zhiguang.liu@intel.com>,
	Liming Gao <gaoliming@byosoft.com.cn>,
	Jiaxin Wu <jiaxin.wu@intel.com>, Ray Ni <ray.ni@intel.com>,
	Laszlo Ersek <lersek@redhat.com>
Subject: [edk2-devel] [PATCH v2 5/6] MdeModulePkg/SMM: Support to unregister SMI handler in SMI handlers
Date: Wed,  3 Apr 2024 15:50:16 +0800	[thread overview]
Message-ID: <20240403075017.1997-6-zhiguang.liu@intel.com> (raw)
In-Reply-To: <20240403075017.1997-1-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       | 147 ++++++++++++++++++++----
 2 files changed, 123 insertions(+), 25 deletions(-)

diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
index b8a490a8c3..72e0ee2cf0 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                         ToDelete;    // To delete this SMI_HANDLER later
 } SMI_HANDLER;
 
 //
diff --git a/MdeModulePkg/Core/PiSmmCore/Smi.c b/MdeModulePkg/Core/PiSmmCore/Smi.c
index 2985f989c3..b8400b25a4 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 = {
@@ -79,6 +84,39 @@ SmmCoreFindSmiEntry (
   return SmiEntry;
 }
 
+/**
+  Remove SmiHandler and free the memory it used.
+  If SmiEntry is empty, remove SmiEntry and free the memory it used.
+
+  @param  SmiHandler Points to SMI handler.
+  @param  SmiEntry   Points to SMI Entry or NULL for root SMI handlers.
+
+  @retval TRUE        SmiEntry is removed.
+  @retval FALSE       SmiEntry is not removed.
+**/
+BOOLEAN
+RemoveSmiHandler (
+  IN SMI_HANDLER  *SmiHandler,
+  IN SMI_ENTRY    *SmiEntry
+  )
+{
+  RemoveEntryList (&SmiHandler->Link);
+  FreePool (SmiHandler);
+
+  if (SmiEntry != NULL) {
+    if (IsListEmpty (&SmiEntry->SmiHandlers)) {
+      //
+      // No handler registered for this SMI_ENTRY now, remove the SMI_ENTRY
+      //
+      RemoveEntryList (&SmiEntry->AllEntries);
+      FreePool (SmiEntry);
+      return TRUE;
+    }
+  }
+
+  return FALSE;
+}
+
 /**
   Manage SMI of a particular type.
 
@@ -104,15 +142,17 @@ SmiManage (
 {
   LIST_ENTRY   *Link;
   LIST_ENTRY   *Head;
+  LIST_ENTRY   *EntryLink;
   SMI_ENTRY    *SmiEntry;
   SMI_HANDLER  *SmiHandler;
-  BOOLEAN      SuccessReturn;
+  EFI_STATUS   ReturnStatus;
+  BOOLEAN      WillReturn;
   EFI_STATUS   Status;
 
   PERF_FUNCTION_BEGIN ();
-
-  Status        = EFI_NOT_FOUND;
-  SuccessReturn = FALSE;
+  mSmiManageCallingDepth++;
+  Status       = EFI_NOT_FOUND;
+  ReturnStatus = Status;
   if (HandlerType == NULL) {
     //
     // Root SMI handler
@@ -152,7 +192,16 @@ SmiManage (
         //
         if (HandlerType != NULL) {
           PERF_FUNCTION_END ();
-          return EFI_INTERRUPT_PENDING;
+          ReturnStatus = EFI_INTERRUPT_PENDING;
+          WillReturn   = TRUE;
+        } else {
+          //
+          // If any other handler's result sets ReturnStatus as EFI_SUCCESS, the return status
+          // will be EFI_SUCCESS.
+          //
+          if (ReturnStatus != EFI_SUCCESS) {
+            ReturnStatus = Status;
+          }
         }
 
         break;
@@ -165,10 +214,10 @@ SmiManage (
         //
         if (HandlerType != NULL) {
           PERF_FUNCTION_END ();
-          return EFI_SUCCESS;
+          WillReturn = TRUE;
         }
 
-        SuccessReturn = TRUE;
+        ReturnStatus = EFI_SUCCESS;
         break;
 
       case EFI_WARN_INTERRUPT_SOURCE_QUIESCED:
@@ -176,7 +225,7 @@ SmiManage (
         // If at least one of the handlers returns EFI_WARN_INTERRUPT_SOURCE_QUIESCED
         // then the function will return EFI_SUCCESS.
         //
-        SuccessReturn = TRUE;
+        ReturnStatus = EFI_SUCCESS;
         break;
 
       case EFI_WARN_INTERRUPT_SOURCE_PENDING:
@@ -184,6 +233,10 @@ SmiManage (
         // If all the handlers returned EFI_WARN_INTERRUPT_SOURCE_PENDING
         // then EFI_WARN_INTERRUPT_SOURCE_PENDING will be returned.
         //
+        if (ReturnStatus != EFI_SUCCESS) {
+          ReturnStatus = Status;
+        }
+
         break;
 
       default:
@@ -193,14 +246,65 @@ SmiManage (
         ASSERT (FALSE);
         break;
     }
+
+    if (WillReturn) {
+      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 ToDelete.
+  // Note that SmiManage can be called recursively.
+  //
+  if (mSmiManageCallingDepth == 0) {
+    //
+    // Go through all SmiHandler in root SMI handlers
+    //
+    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->ToDelete) {
+        //
+        // Remove SmiHandler if the ToDelete is set.
+        //
+        RemoveSmiHandler (SmiHandler, NULL);
+      }
+    }
+
+    //
+    // 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->ToDelete) {
+          if (RemoveSmiHandler (SmiHandler, SmiEntry)) {
+            break;
+          }
+        }
+      }
+    }
   }
 
   PERF_FUNCTION_END ();
-  return Status;
+  return ReturnStatus;
 }
 
 /**
@@ -238,6 +342,7 @@ SmiHandlerRegister (
   SmiHandler->Signature  = SMI_HANDLER_SIGNATURE;
   SmiHandler->Handler    = Handler;
   SmiHandler->CallerAddr = (UINTN)RETURN_ADDRESS (0);
+  SmiHandler->ToDelete   = FALSE;
 
   if (HandlerType == NULL) {
     //
@@ -324,24 +429,16 @@ SmiHandlerUnRegister (
 
   SmiEntry = SmiHandler->SmiEntry;
 
-  RemoveEntryList (&SmiHandler->Link);
-  FreePool (SmiHandler);
-
-  if (SmiEntry == NULL) {
+  if (mSmiManageCallingDepth > 0) {
     //
-    // This is root SMI handler
+    // This function is called from SmiManage()
+    // Do not delete or remove SmiHandler or SmiEntry now.
+    // Set the ToDelete field in SmiHandler so that SmiManage will delete it later
     //
+    SmiHandler->ToDelete = TRUE;
     return EFI_SUCCESS;
   }
 
-  if (IsListEmpty (&SmiEntry->SmiHandlers)) {
-    //
-    // No handler registered for this interrupt now, remove the SMI_ENTRY
-    //
-    RemoveEntryList (&SmiEntry->AllEntries);
-
-    FreePool (SmiEntry);
-  }
-
+  RemoveSmiHandler (SmiHandler, SmiEntry);
   return EFI_SUCCESS;
 }
-- 
2.31.1.windows.1



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



  parent reply	other threads:[~2024-04-03  7:50 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-03  7:50 [edk2-devel] [PATCH v2 0/6] Support to unregister SMI handler in SMI handlers Zhiguang Liu
2024-04-03  7:50 ` [edk2-devel] [PATCH v2 1/6] Revert 2ec8f0c6407f062441b205b900038933865c7b3c Zhiguang Liu
2024-04-03  7:50 ` [edk2-devel] [PATCH v2 2/6] Revert 049ff6c39c73edd3709c05bd0e46184320471358 Zhiguang Liu
2024-04-03  7:50 ` [edk2-devel] [PATCH v2 3/6] Revert 17b28722008eab745ce186b72cd325944cbe6bf0 Zhiguang Liu
2024-04-03  7:50 ` [edk2-devel] [PATCH v2 4/6] Revert ae1079b386a597108a8070652bf7cdaa4ec3dda3 Zhiguang Liu
2024-04-03  7:50 ` Zhiguang Liu [this message]
2024-04-03  7:50 ` [edk2-devel] [PATCH v2 6/6] StandaloneMmPkg: Support to unregister MMI handler in MMI handlers Zhiguang Liu

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=20240403075017.1997-6-zhiguang.liu@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