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 <gaolim...@byosoft.com.cn>
Cc: Jiaxin Wu <jiaxin...@intel.com>
Cc: Ray Ni <ray...@intel.com>
Cc: Laszlo Ersek <ler...@redhat.com>
Cc: Ray Ni <ray...@intel.com>
Cc: Laszlo Ersek <ler...@redhat.com>
Signed-off-by: Zhiguang Liu <zhiguang....@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/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
