patina_dxe_core: Add DxeDispatch service for driver dispatch#1421
patina_dxe_core: Add DxeDispatch service for driver dispatch#1421kat-perez wants to merge 1 commit intoOpenDevicePartnership:mainfrom
Conversation
✅ QEMU Validation PassedAll QEMU validation jobs completed successfully.
Workflow run: https://github.com/OpenDevicePartnership/patina/actions/runs/23361899596 Boot Time to EFI Shell
Dependencies
This comment was automatically generated by the Patina QEMU PR Validation Post workflow. |
2fa66cb to
b8285d5
Compare
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
b8285d5 to
527b860
Compare
patina_dxe_core/src/lib.rs
Outdated
| /// Performs a single DXE driver dispatch pass. | ||
| /// | ||
| /// Returns `true` if any drivers were dispatched, `false` if no drivers remain. | ||
| pub fn dispatch(&'static self) -> Result<bool> { |
There was a problem hiding this comment.
I don't think making this public makes sense. @Javagedes do you have thoughts on this? Feels like a different paradigm is needed.
There was a problem hiding this comment.
I could move this to the SDK instead?
Like this at sdk/patina/src/pi/dxe_services.rs?
/// Interface for dispatching DXE drivers.
///
/// Boot orchestrators use this to interleave controller connection
/// with driver dispatch.
pub trait DxeServices: Send + Sync + 'static {
/// Run a single dispatch pass. Returns `true` if any drivers were dispatched.
fn dispatch(&self) -> crate::error::Result<bool>;
}
There was a problem hiding this comment.
Yeah, I don't think we want to make this public outside of the core. It leaks containment of functionality that the platform's top level main() function should not have access to, which could result in platforms doing bad things.
Based off of the current usage:
impl DxeServices for CoreDxeServices {
fn dispatch(&self) -> patina::error::Result<bool> {
CORE.dispatch()
}
}
// Boot orchestration with connect-dispatch interleaving
add.component(BootDispatcher::new(
SimpleBootManager::new(
BootConfig::new(create_boot_path())
.with_failure_handler(|| log::error!("Boot failed: all boot options exhausted")),
),
CoreDxeServices,
));I think one of these two options better lean into the patina component model via dependency injection:
- Create a new service trait defined in the
patinacrate for either (1) only dispatch or (2) the entire dxe services table, and implemented in the core (Similar to the CoreMemoryManager), then consume it through dependency injection - Do the same as 1, but instead do it for configuration table functionality, then you can grab the dxe services configuration table and call dispatch from dxe_services
There was a problem hiding this comment.
Thanks, I'll go with (1.) by making a new service trait instead. Would be something like CoreDxeDispatch, implemented like CoreMemoryManager
There was a problem hiding this comment.
The platform binary will never touch dispatch() directly
527b860 to
386036d
Compare
8840c6c to
45799fd
Compare
45799fd to
6f12730
Compare
Description
Add a
DxeDispatchservice trait in the SDK and aCoreDxeDispatchimplementation in patina_dxe_core that delegates to the PI dispatcher.
The service is registered alongside other core services (MemoryManager,
PerfTimer, etc.) and consumed via dependency injection by components
that need to trigger driver dispatch passes.
How This Was Tested
BootDispatcher+SimpleBootManagerconsuming the service via DIIntegration Instructions
The
DxeDispatchservice is registered automatically by the DXE core.Components consume it via dependency injection: