In my opinion this is not a spec conformance fix, but it is a bug fix.
The spec does not explicitly mentions unregistering from within the handler,
but by applying a principle that everything that’s not forbidden is legal,
it’s fair for a caller to expect that unregistering works fine in any valid context
including from within the MM handler.
The spec does mention that UEFI/PI interfaces are not reentrant,
however, reentrancy typically implies repetitive invocation of the same interface,
not a call to one MM Core interface while another MM Core interface is in progress.
From: Ni, Ray <ray.ni@intel.com>
Sent: Wednesday, January 24, 2024 11:17 AM
To: Laszlo Ersek <lersek@redhat.com>; devel@edk2.groups.io; Liu, Zhiguang <zhiguang.liu@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>; Wu, Jiaxin <jiaxin.wu@intel.com>; Felix Polyudov <Felixp@ami.com>
Subject: [EXTERNAL] Re: [edk2-devel] [PATCH] MdeModulePkg/SMM: Support to unregister SMI handler inside SMI handler
**CAUTION: The e-mail below is from an external source. Please exercise caution before opening attachments, clicking
links, or following guidance.**
spec does not say the unregistration is allowed inside handler. it's just to improve the qualiquali the code.
thanks,
ray
From: Laszlo Ersek <lersek@redhat.com>
Sent: Wednesday, January 24, 2024 9:06:13 PM
To: devel@edk2.groups.io <devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com>; Liu, Zhiguang <zhiguang.liu@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>; Wu, Jiaxin <jiaxin.wu@intel.com>; POLUDOV, FELIX <felixp@ami.com>
Subject: Re: [edk2-devel] [PATCH] MdeModulePkg/SMM: Support to unregister SMI handler inside SMI handler
On 1/24/24 09:11, Ni, Ray wrote:
> Felix, I remember you mentioned to me about the usage of SMI handler unregistering itself.
I wanted to ask: is this something that the PI spec comments on? I.e.,
is this usage expected by the spec (in which case this bugfix is a
conformance fix), or is the spec silent on it (in which case I guess we
can call this a quality-of-implementation improvement)?
>
> Reviewed-by: Ray Ni <ray.ni@intel.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks
Laszlo
>
> Thanks,
> Ray
>> -----Original Message-----
>> From: Liu, Zhiguang <zhiguang.liu@intel.com>
>> Sent: Wednesday, January 24, 2024 12:03 PM
>> To: devel@edk2.groups.io
>> Cc: Liu, Zhiguang <zhiguang.liu@intel.com>; Liming Gao
>> <gaoliming@byosoft.com.cn>; Wu, Jiaxin <jiaxin.wu@intel.com>; Ni, Ray
>> <ray.ni@intel.com>
>> Subject: [PATCH] MdeModulePkg/SMM: Support to unregister SMI handler
>> inside SMI handler
>>
>> To support unregister SMI handler inside SMI handler itself,
>> get next node before SMI handler is executed, since LIST_ENTRY that
>> Link points to may be freed if unregister SMI handler in SMI handler
>> itself.
>>
>> Cc: Liming Gao <gaoliming@byosoft.com.cn>
>> Cc: Jiaxin Wu <jiaxin.wu@intel.com>
>> Cc: Ray Ni <ray.ni@intel.com>
>> Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com>
>> ---
>> MdeModulePkg/Core/PiSmmCore/Smi.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/MdeModulePkg/Core/PiSmmCore/Smi.c
>> b/MdeModulePkg/Core/PiSmmCore/Smi.c
>> index 2985f989c3..a75e52b1ae 100644
>> --- a/MdeModulePkg/Core/PiSmmCore/Smi.c
>> +++ b/MdeModulePkg/Core/PiSmmCore/Smi.c
>> @@ -134,8 +134,14 @@ SmiManage (
>>
>> Head = &SmiEntry->SmiHandlers;
>>
>> - for (Link = Head->ForwardLink; Link != Head; Link = Link->ForwardLink) {
>> + for (Link = Head->ForwardLink; Link != Head;) {
>> SmiHandler = CR (Link, SMI_HANDLER, Link,
>> SMI_HANDLER_SIGNATURE);
>> + //
>> + // To support unregiser SMI handler inside SMI handler itself,
>> + // get next node before handler is executed, since LIST_ENTRY that
>> + // Link points to may be freed if unregister SMI handler.
>> + //
>> + Link = Link->ForwardLink;
>>
>> Status = SmiHandler->Handler (
>> (EFI_HANDLE)SmiHandler,
>> --
>> 2.31.1.windows.1
>
>
>
>
>
>