public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* Managing boot order dependencies with ResetNotification Protocol
@ 2023-06-29  0:23 Ajay Iyengar (QUIC)
  2023-07-11 23:37 ` Andrew Fish
  0 siblings, 1 reply; 2+ messages in thread
From: Ajay Iyengar (QUIC) @ 2023-06-29  0:23 UTC (permalink / raw)
  To: 'devel@edk2.groups.io', 'Andrew Fish',
	Leif Lindholm (QUIC), 'Michael D Kinney'
  Cc: 'Ruiyu Ni', 'Star Zeng',
	'michael.a.rothman@intel.com', 'felixp@ami.com'

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

Hello,

This is a comment and query on the Reset Notification Protocol defined by the UEFI specification.

The specification requires clients to be notified for ResetSystem() without imposing any specific ordering on how these notifications are dispatched: https://uefi.org/specs/UEFI/2.10/39_Micellaneous_Protocols.html#reset-notification-protocol.
For example, this is used for gracefully shutting off NVMe and TPM through their respective spec defined power off notifications:
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c#L950
https://github.com/tianocore/edk2/blob/master/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c#L2597

If we assume a platform implementing fTPM using NVMe-RPMB,  the boot order would require NVMe as a dependency for TPM, but that might result in an issue with the reset notification path. The default EDK2 implementation dispatches these notifications in boot order: https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c#L78.  Here NVMe would power off, before TPM commits to NVMe-RPMB within its reset notification callback. Reversing the order of dispatch (i.e. reverse-boot order) would help with this situation.

We have several similar situations on our platform -- drivers with a Depex ordering registering for ResetNotify which would benefit from being signaled in reverse-boot order. Are there any provisions that could be made with regards to ensuring the ordering requirement?  Could we mimic the behavior of the ExitBootServices event dispatch for ResetNotify:
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Dxe/Event/Event.c#L492 ?
[The proposal would be to use InsertHeadList() instead of InsertTailList() in RegisterResetNotify()]

Thanks,
Ajay

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: Managing boot order dependencies with ResetNotification Protocol
  2023-06-29  0:23 Managing boot order dependencies with ResetNotification Protocol Ajay Iyengar (QUIC)
@ 2023-07-11 23:37 ` Andrew Fish
  0 siblings, 0 replies; 2+ messages in thread
From: Andrew Fish @ 2023-07-11 23:37 UTC (permalink / raw)
  To: Ajay Iyengar (QUIC)
  Cc: devel@edk2.groups.io, Leif Lindholm (QUIC), Mike Kinney, Ruiyu Ni,
	Star Zeng, Michael Rothmam, Felix Poludov

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

Ajay,

In general the order of dispatch when all the DEPEX evaluate to TRUE and the oder of events are not defined by the specification. 

I think you are trying to say stuff that dispatches later is higher up the stack and it would make more sense to send the rest to things higher up the stack? 

I’m not sure how this would work in your example as the Tcg2Dxe driver has a Depex of gEfiVariableArchProtocolGuid and the NVMe driver is a UEFI_DRIVER so it has an implied Depex [1] that is more complex?

To really fix we would probably need to add some kind of new protocol tot he NVMe driver that lets drivers request the reset get delayed. It would have a register, unregister, I’m Done.  
The NVMe driver would do something like:
1) Register for the  reset notification
2) Publish the new protocol
3) When the 1st driver registers the NVMe driver unregisters the reset notification, or I guess it could just ignore it. 

When the shutdown happens and the last registered driver calls back in to say I’m Done then the NVMe driver can shutdown. 
The other drivers will need to RegisterProtocolNotify so there is not dispatch sequence dependency.

I’m not sure that is the best solution, that is just the best I have right now.

[1] UEFI_DRIVER depends on all the protocols required to impelemnt all the UEFI Boot and Runtime Services.
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Dxe/DxeMain/DxeProtocolNotify.c#L21
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Dxe/DxeMain/DxeProtocolNotify.c#L80

Thanks,

Andrew FIsh

> On Jun 28, 2023, at 5:23 PM, Ajay Iyengar (QUIC) <quic_aiyengar@quicinc.com> wrote:
> 
> Hello,
>  
> This is a comment and query on the Reset Notification Protocol defined by the UEFI specification.
>  
> The specification requires clients to be notified for ResetSystem() without imposing any specific ordering on how these notifications are dispatched: https://uefi.org/specs/UEFI/2.10/39_Micellaneous_Protocols.html#reset-notification-protocol. 
> For example, this is used for gracefully shutting off NVMe and TPM through their respective spec defined power off notifications: 
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Bus/Pci/NvmExpressDxe/NvmExpressHci.c#L950
> https://github.com/tianocore/edk2/blob/master/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c#L2597
>  
> If we assume a platform implementing fTPM using NVMe-RPMB,  the boot order would require NVMe as a dependency for TPM, but that might result in an issue with the reset notification path. The default EDK2 implementation dispatches these notifications in boot order: https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c#L78.  Here NVMe would power off, before TPM commits to NVMe-RPMB within its reset notification callback. Reversing the order of dispatch (i.e. reverse-boot order) would help with this situation.
>  
> We have several similar situations on our platform -- drivers with a Depex ordering registering for ResetNotify which would benefit from being signaled in reverse-boot order. Are there any provisions that could be made with regards to ensuring the ordering requirement?  Could we mimic the behavior of the ExitBootServices event dispatch for ResetNotify:
> https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Core/Dxe/Event/Event.c#L492 ?
> [The proposal would be to use InsertHeadList() instead of InsertTailList() in RegisterResetNotify()]
>  
> Thanks,
> Ajay


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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-07-11 23:38 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-29  0:23 Managing boot order dependencies with ResetNotification Protocol Ajay Iyengar (QUIC)
2023-07-11 23:37 ` Andrew Fish

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox