From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 0459574003D for ; Wed, 3 Apr 2024 07:50:48 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=YSTtNRqiLEWp9XE2XpwpeT0iQcGm8nyGrXgWtRDXmHI=; c=relaxed/simple; d=groups.io; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References:MIME-Version:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Resent-Date:Resent-From:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Transfer-Encoding; s=20240206; t=1712130647; v=1; b=neujdke7uHad+7VjgFb6uiRwvoX8kotq+ua3JcjG/osWuICZ1+XgtZMe1GHBIjj7AzqgR8v7 f2KcOaUYMcbRtBsVLRKbfTQhyYLF/SVR6y2Mq24CImipFvfhwRRxZM9KYPBQKoRfzqFaktz4Vzm w4yb3ynpIXrkM5VthDpOAf1uesdWZmREGQQT8ZoPmUfxHw43otQ0xEia8E54iPsigqsr7I0Xm9M 6my1zlm0klJ/TVa4T6bHqJigNM5iH47RgMZC9NfzGiN2KSxp5rJdnwcVObOMqzP4UYGFDFR6MnU JVGSRomvT0QfzcOkM4dx5RTzRM5m9MKjGPI/9DqaaVMvw== X-Received: by 127.0.0.2 with SMTP id AF4YYY7687511xCpOJver5Vw; Wed, 03 Apr 2024 00:50:47 -0700 X-Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.13]) by mx.groups.io with SMTP id smtpd.web11.5859.1712130636141857502 for ; Wed, 03 Apr 2024 00:50:47 -0700 X-CSE-ConnectionGUID: rt/W6Bw2RHG7kcHLmYwNBg== X-CSE-MsgGUID: 6a6BuSZDTwCipq7utAHNaA== X-IronPort-AV: E=McAfee;i="6600,9927,11032"; a="10311969" X-IronPort-AV: E=Sophos;i="6.07,176,1708416000"; d="scan'208";a="10311969" X-Received: from fmviesa009.fm.intel.com ([10.60.135.149]) by fmvoesa107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Apr 2024 00:50:46 -0700 X-CSE-ConnectionGUID: 9IZGJ0LzTvCRStO6Am0YwQ== X-CSE-MsgGUID: 4IGOD15BTpe995aKHjk84A== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.07,176,1708416000"; d="scan'208";a="18399087" X-Received: from shwdesfp01.ccr.corp.intel.com ([10.239.158.151]) by fmviesa009-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Apr 2024 00:50:45 -0700 From: "Zhiguang Liu" To: devel@edk2.groups.io Cc: Zhiguang Liu , Liming Gao , Jiaxin Wu , Ray Ni , Laszlo Ersek 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 Message-Id: <20240403075017.1997-6-zhiguang.liu@intel.com> In-Reply-To: <20240403075017.1997-1-zhiguang.liu@intel.com> References: <20240403075017.1997-1-zhiguang.liu@intel.com> MIME-Version: 1.0 Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Resent-Date: Wed, 03 Apr 2024 00:50:47 -0700 Resent-From: zhiguang.liu@intel.com Reply-To: devel@edk2.groups.io,zhiguang.liu@intel.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: n9LbkR5bCC3KR1TojZ0iC4l2x7686176AA= Content-Transfer-Encoding: 8bit X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20240206 header.b=neujdke7; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=intel.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io 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 Cc: Jiaxin Wu Cc: Ray Ni Cc: Laszlo Ersek Cc: Ray Ni Cc: Laszlo Ersek Signed-off-by: Zhiguang Liu --- 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] -=-=-=-=-=-=-=-=-=-=-=-