Re: [edk2-devel] [PATCH] MdeModulePkg/SMM: Support to unregister SMI handler inside SMI handler

["Felix Polyudov via groups.io" <felixp=ami.com@groups.io>]

[-- Attachment #1: Type: text/plain, Size: 5598 bytes --]

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<mailto:lersek@redhat.com>>
Sent: Wednesday, January 24, 2024 9:06:13 PM
To: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Liu, Zhiguang <zhiguang.liu@intel.com<mailto:zhiguang.liu@intel.com>>
Cc: Liming Gao <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Wu, Jiaxin <jiaxin.wu@intel.com<mailto:jiaxin.wu@intel.com>>; POLUDOV, FELIX <felixp@ami.com<mailto: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<mailto:ray.ni@intel.com>>

Reviewed-by: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>

Thanks
Laszlo

>
> Thanks,
> Ray
>> -----Original Message-----
>> From: Liu, Zhiguang <zhiguang.liu@intel.com<mailto:zhiguang.liu@intel.com>>
>> Sent: Wednesday, January 24, 2024 12:03 PM
>> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
>> Cc: Liu, Zhiguang <zhiguang.liu@intel.com<mailto:zhiguang.liu@intel.com>>; Liming Gao
>> <gaoliming@byosoft.com.cn<mailto:gaoliming@byosoft.com.cn>>; Wu, Jiaxin <jiaxin.wu@intel.com<mailto:jiaxin.wu@intel.com>>; Ni, Ray
>> <ray.ni@intel.com<mailto: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<mailto:gaoliming@byosoft.com.cn>>
>> Cc: Jiaxin Wu <jiaxin.wu@intel.com<mailto:jiaxin.wu@intel.com>>
>> Cc: Ray Ni <ray.ni@intel.com<mailto:ray.ni@intel.com>>
>> Signed-off-by: Zhiguang Liu <zhiguang.liu@intel.com<mailto: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
>
>
>
> 
>
>
-The information contained in this message may be confidential and proprietary to American Megatrends (AMI). This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.


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



[-- Attachment #2: Type: text/html, Size: 11727 bytes --]