Hi Leif, Please find my response inline marked [SAMI].
Regards, Sami Mujawar From: Leif Lindholm <[email protected]> Date: Tuesday, 19 May 2026 at 17:31 To: Sami Mujawar <[email protected]> Cc: Andrew Fish <[email protected]>; Kinney, Michael D <[email protected]>; Ard Biesheuvel <[email protected]>; Michael Kubacki <[email protected]>; edk2-devel-groups-io <[email protected]>; [email protected] <[email protected]>; Pierre Gondois <[email protected]>; [email protected] <[email protected]> Subject: Re: Request to include in edk2-stable202605 - [tianocore/edk2] DynamicTablesPkg: Fix dependency issues with consumed protocols (PR #12562) Hi Sami, On Tue, 19 May 2026 at 16:40, Sami Mujawar <[email protected]> wrote: > I agree the behaviour is not immediately obvious when looking only at the PR. > I was initially confused as well when first reviewing the issue. > > The DynamicTablesManagerDxe installs an event notification for when both > gEfiAcpiTableProtocolGuid and gEfiSmbiosProtocolGuid become available, see > https://github.com/tianocore/edk2/blob/master/DynamicTablesPkg/Drivers/DynamicTableManagerDxe/DynamicTableManagerDxe.c#L61-L90 > > The EfiCreateProtocolNotifyEvent() invokes the event notification handler > function in the following situations > > if the protocol instance already exists > Whenever the protocol is installed/reinstalled > At least once even if the protocol is not installed. > > The documentation extract for the last case is as below > > > If there are no instances of ProtocolGuid in the handle database at the > time this function is invoked, > > then the notification function is still executed one time > > Please see > https://github.com/tianocore/edk2/blob/master/MdePkg/Library/UefiLib/UefiLib.c#L106-L108 Urgh. Thanks for the explanation. OK, I guess that means I reduce the strength of my objection. But I still question why it needs patching during hard freeze if it went unnoticed for seven months and two stable tags. I mean, it's annoying for whoever manages not to have the lines in their build config in an order that hid this problem, but surely a patch such as this would be easy to cherry-pick? [SAMI] I agree this patch can be cherry-picked. [/SAMI] > The check in the PR is handling the case 3 above. > > The DEPEX was removed as we want to support platforms that generate only ACPI > tables without requiring SMBIOS support. > The previous DEPEX effectively enforced both protocols as load-order > dependencies, which prevented this usage model. > > The current functionality should allow processing of ACPI tables or SMBIOS > tables whenever the protocol becomes available. > > Hope that clarifies the situation. It does, thanks. But this effectively also means that depending on module load order, this handler could end up being invoked several times if any other modules also register a notify event on the same protocol, including potentially after the protocol becomes available. Does the driver safely handle that? [SAMI] You have raised an important point here. If EfiCreateProtocolNotifyEvent() is invoked after the ACPI tables have already been processed, the notification handler being called again would process the tables a second time. That could potentially result in incorrect table data. I don’t think this scenario has been explicitly tested or verified, and I agree this needs further investigation. [/SAMI] Best Regards, Leif IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#121963): https://edk2.groups.io/g/devel/message/121963 Mute This Topic: https://groups.io/mt/119390195/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
