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

["Laszlo Ersek" <lersek@redhat.com>]

On 1/24/24 18:15, Felix Polyudov via groups.io wrote:
> 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.

Sure, I'm not contesting any of that; it's great that you and Ray seem
to agree. I just wanted to understand. My R-b stands.

Thanks!
Laszlo

> *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 (#114342): https://edk2.groups.io/g/devel/message/114342
Mute This Topic: https://groups.io/mt/103925794/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-