Skip to content

Events with EVT_NOTIFY_SIGNAL type may fail - #1756

Closed
olegilyasov wants to merge 1 commit into
OpenDevicePartnership:mainfrom
olegilyasov:olegilyasov/evt_notify_signal_handling
Closed

Events with EVT_NOTIFY_SIGNAL type may fail#1756
olegilyasov wants to merge 1 commit into
OpenDevicePartnership:mainfrom
olegilyasov:olegilyasov/evt_notify_signal_handling

Conversation

@olegilyasov

Copy link
Copy Markdown
Contributor

Description

  • Events with event groups CAN have notify functions (optional)
  • Events without event groups that are notifiable (EVT_NOTIFY_*) MUST have notify functions
  • TPL validation only applies when notify_function is provided

Modify event handling for this special case.

  • Impacts functionality?
  • Impacts security?
  • Breaking change?
  • Includes tests?
  • Includes documentation?

How This Was Tested

Added test function create_event_group_members_with_optional_notify_functions

Integration Instructions

N/A

- Events with event groups CAN have notify functions (optional)
- Events without event groups that are notifiable (EVT_NOTIFY_*) MUST have notify functions
- TPL validation only applies when notify_function is provided

Modify event handling for this special case.

Signed-off-by: Oleg Ilyasov <olegi@ami.com>
@patina-automation

patina-automation Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

✅ QEMU Validation Passed

All QEMU validation jobs completed successfully.

Note: Q35 is only built on Windows hosts (QEMU boot is disabled due to a QEMU vfat issue).

Workflow run: https://github.com/OpenDevicePartnership/patina/actions/runs/32889539437

Boot Time to EFI Shell

Platform Elapsed
ArmVirt (Linux Host) 26.8s
Q35 (Linux Host) 26.5s

Dependencies

Repository Ref
patina daca028
patina-dxe-core-qemu 2ba7486
patina-fw-patcher c28ab7d
patina-qemu firmware v4.0.5
patina-qemu build script a6f1d59

This comment was automatically generated by the Patina QEMU PR Validation Post workflow.

@codecov

codecov Bot commented Aug 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@os-d

os-d commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

@olegilyasov can you share the spec language directly that you are referring to? In Uefi Spec 2.10A section 7.1.2 I am only seeing this as relevant:

image

Which is what the code is currently doing. I'm not seeing the language about event groups with optional notify functions.

It looks like edk2 has the same behavior: https://github.com/tianocore/edk2/blob/7735ed4f8eb3afaf1a97d27a8862f382c387f0bc/MdeModulePkg/Core/Dxe/Event/Event.c#L429-L438

@olegilyasov

Copy link
Copy Markdown
Contributor Author

@os-d you are correct, I incorrectly assumed UEFI spec allowed event group members to have optional notify functions. UEFI spec requires NotifyFunction to be non-NULL for notifiable events, even for event groups.
The real bug: Patina's pi_dispatcher.rs is creating the event with notify_function: None, which violates the UEFI spec.
EDK2 provides EfiEventEmptyFunction - an empty stub function that does nothing. It uses this for events that only need to signal event groups but don't need custom notification logic.

Undoing the change and adding a notification function stub to pi_dispatcher....

@joschock

Copy link
Copy Markdown
Contributor

@os-d you are correct, I incorrectly assumed UEFI spec allowed event group members to have optional notify functions. UEFI spec requires NotifyFunction to be non-NULL for notifiable events, even for event groups. The real bug: Patina's pi_dispatcher.rs is creating the event with notify_function: None, which violates the UEFI spec. EDK2 provides EfiEventEmptyFunction - an empty stub function that does nothing. It uses this for events that only need to signal event groups but don't need custom notification logic.

Undoing the change and adding a notification function stub to pi_dispatcher....

I looked at pi_dispatcher; both create_event calls I see in there provide Some(notificaton_fn) - can you point more specifically to what you are concerned about in pi_dispatcher?

the two event creations I see in pi_dispatcher are:


and

@makubacki makubacki changed the title Events with EVT_NOFIFY_SIGNAL type may fail Events with EVT_NOTIFY_SIGNAL type may fail Aug 25, 2026
@olegilyasov

Copy link
Copy Markdown
Contributor Author

Apparently the problem is in my code. I added signaling EVENT_GROUP_DXE_DISPATCH after each DXE dispatch round where at least one driver was dispatched, allowing SMM dispatcher and other components to coordinate with DXE driver dispatch.
This allows SMM drivers to get loaded.
And of course I messed up the event preparation. No intention to push that change since Patina does not support SMI handlers.
Sorry about this and thank you for your time.

@olegilyasov
olegilyasov deleted the olegilyasov/evt_notify_signal_handling branch August 26, 2026 00:48
@makubacki

Copy link
Copy Markdown
Collaborator

Apparently the problem is in my code. I added signaling EVENT_GROUP_DXE_DISPATCH after each DXE dispatch round where at least one driver was dispatched, allowing SMM dispatcher and other components to coordinate with DXE driver dispatch. This allows SMM drivers to get loaded. And of course I messed up the event preparation. No intention to push that change since Patina does not support SMI handlers. Sorry about this and thank you for your time.

For clarity to others that might come across this, Patina supports the CPU operating mode called System Management Mode (SMM) and the ability to run what is colloquially referred to as "SMI handlers". At the Platform Initialization (PI) spec (software) level, there are two modes defined - Traditional SMM and Standalone MM, which leads to terminology across architectures and modes simplifying to "MM" and "MMI". Patina intentionally does not support Traditional SMM for the reasons noted in Patina DXE Core Requirements - No Traditional SMM. However, it does support MMI handlers, just running in the Standalone MM model (in which, the actual handler code is usually the same).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

impact:testing Affects testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Events with EVT_NOTIFY_SIGNAL type processing may fail

4 participants