[major] Add Initial Patina UEFI Services [Rebase & FF] - #1743
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
|
I'm working on some changes to improve unit test coverage. |
ℹ️ QEMU Validation Skipped - Unsupported Target BranchThe Patina QEMU PR validation workflow did not run because the PR targets an unsupported branch. Workflow run: https://github.com/OpenDevicePartnership/patina/actions/runs/33698059661
This comment was automatically generated by the Patina QEMU PR Validation Post workflow. |
f63f1af to
3e810b7
Compare
Pushed. Also updated the |
cfernald
left a comment
There was a problem hiding this comment.
just reviewed the SDK interface for now. Will review the full changes later.
kat-perez
left a comment
There was a problem hiding this comment.
I found several safety and contract issues in the new service layer that should be addressed before merge. The inline comments focus on callback lifetime, protocol reference lifetime, marker protocol compatibility, and duration conversion.
Withdrawn by reviewer for additional cross-checking before feedback is submitted.
b9d0dbd to
75530e3
Compare
|
@Javagedes, @cfernald, @kat-perez, this is ready for you review again when you get time. |
Splits MM performance record collection out of the monolithic Performance component. `mm_comm_service` is now a required `Service<dyn MmCommunication>` rather than `Option<Service<...>>`, so the dispatcher itself skips this component on platforms with no MM communication region instead of branching on an `Option<Service<...>>` at runtime. Also adds a `core::error::Error` `impl` for `MmPerformanceError` while it is already being moved. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
The monolithic Performance component has now been split into separate single-purpose components, each with its own responsibilities, so it can be removed. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Updates docs in the mdbook and the patina_performance component crate to reflect the recent split of the monolithic patina_performance component into separate components. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Adds an `impl` of `TplController` for `Service<dyn TplServices>` along with a small helper that converts a raw TPL level to the newer `Tpl` enum. This lets any `TplMutex` use the trait object TPL service instead of requiring a concrete `BootServices` implementation. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Adds `SmbiosError::Busy` so lock contention can be reported as a normal error. It maps to `EfiError::DeviceError`. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
…methods Adds a couple of methods to the `Smbios` trait so the C protocol shim will be able to look up the next record after a handle and the address of a published record without touching the `SmbiosManager` or its `TplMutex` directly. Both return owned data instead of a borrowing iterator, since the existing `SmbiosRecordsIter` cannot outlive a lock guard. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
`SmbiosImpl` no longer takes a generic `BootServices` parameter. Its `TplMutex` now uses `Service<dyn TplServices>`. Every lock acquisition goes through `try_lock`, returning `SmbiosError::Busy` on contention instead of panicking. `SmbiosImpl` stores a `Service<dyn ConfigurationTableServices>` and uses it to install the SMBIOS configuration table entry. `SmbiosProtocolInternal` now holds only the protocol struct and a `Service<dyn Smbios>`, dropping the raw `TplMutex` reference. The extern functions call through the service trait instead of locking the manager directly, and the protocol is installed through `ProtocolServices` instead of a raw unsafe boot services call. component.rs is updated minimally to keep the crate compiling, still fetching the newly needed services through `storage.get_service` in the same style it already used. It will be split into two components with typed parameters in a future change. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
…Publisher Moves `SmbiosProvider` into component/provider.rs and migrates its entry point to use typed service parameters for memory, configuration table, and TPL services, plus `Commands` for registration, dropping the `storage.get_service` and `storage.boot_services` calls this replaces. Buffer allocation now uses `leak_as_slice` instead of `into_raw_slice`. Adds a new `SmbiosProtocolPublisher` component that installs the protocol for C drivers depending only on `Service<dyn Smbios>` and `Service<dyn ProtocolServices>`. This is the piece the earlier `Smbios` trait extension made it possible to split out on its own. Adds a new entry point test for each component, now that every dependency is mockable. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Updates the crate README and the patina_samples SMBIOS example to register both `SmbiosProvider` and `SmbiosProtocolPublisher`, matching the new split. Updates the "Global State Justification" section, which described a global manager that the crate no longer has. The C protocol shim now reaches the `Smbios` service through a field on the leaked protocol struct instead of any global state. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Replaces boot services calls to `StandardBootServices` with calls to the Patina UEFI Services. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Updates the entry point to depend on `ProtocolServices`. Replaces raw pointer handling with safe code using `on_protocol_installed()`. Implements the `ProtocolInterface` trait for `MmCommBufferUpdateProtocol` to allow safe access to the protocol's fields without unsafe pointer casts. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
…ocking Add a `mockall` and a `mockall-featured` patina dev-dependency to `patina_test` so its tests can mock the sdk/patina `uefi_services` traits (`EventServices`, `TimerEventServices`) instead of using a raw `EFI_BOOT_SERVICES` table. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Replaces every raw `StandardBootServices` call in the `TestRunner` component with UEFI Services: - `TestRecord::schedule_run` now takes `Service<dyn EventServices>` and `Service<dyn TimerEventServices>` instead of using `Storage` to get `StandardBootServices`. Event-triggered tests register through `EventServicesExt::on_event_group`. Timer-triggered tests register through `TimerEventServicesExt::on_timer_event` and `TimerEventServices::set_timer`. `Recorder::initialize` report callbacks are registered the same way. - The extern "efiapi" `run_test`/`run_tests_and_report`/`disable_timer` callbacks and manual `Box::leak`'d context handling can be replaced with new `EventServicesExt`/`TimerEventServicesExt` closures that capture what they need directly and are boxed internally. Note: `Service<dyn TimerEventServices>` is an interface, not necessarily tied to any particular producer. patina_dxe_core's own implementation happens to defer registration until the Timer Architectural Protocol is installed, but that's not required by the trait. A different producer could produce `Service<dyn TimerEventServices>` without a dependency on the Timer Architectural Protocol in the future to reduce or eliminate the dispatch delay this dependency implies for `TestRunner::entry_point` today. Tests that were previously ignored and removed in cb064c3 due to the lack of UEFI service mocking have been re-enabled given the addition of `MockEventServices` and `MockTimerEventServices` Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Improve the code coverage in this module by adding tests that cover variants that are not currently covered. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
`StandardAcpiProvider` is about to stop depending on `BootServices` entirely when it transitions to UEFI Services. This renames the `BootServicesAlreadyInitialized` error variant to `ConfigTableServicesAlreadyInitialized` so the next commit can focus on behavioral changes. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Updates the ACPI component to use the new UEFI Services instead of `StandardBootServices`. Overall changes are relatively minimal to focus on the transition itself and keeping in mind that more ACPI component refactoring is expected in the future. `StandardAcpiProvider` no longer needs to be generic over `BootServices`. Testing is simplified overall using `MockTplServices` and `MockConfigurationTableServices` instead of `MockBootServices`. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Updates `acpi_protocol_test` to use `ProtocolServices` instead of `StandardBootServices` to locate the ACPI Table and ACPI Get protocols. This allows two unsafe code blocks at the call sites to be removed. This was the last `StandardBootServices` usage in the crate, so `patina_acpi` no longer depends on it at all. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
`CpuArchProtocolInstaller`, `HwInterruptProtocolInstaller`, `SystemTableChecksumInstaller`, and `DecompressProtocolInstaller` were the last components in patina_dxe_core still depending on `StandardBootServices` directly. This change moves them onto `Service<dyn ProtocolServices>` so `StandardBootServices` can be removed from the component model in a follow up change. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Now that no component in patina_dxe_core depends on `StandardBootServices` directly, this drops `ComponentDispatcher::set_boot_services` and the code in `Core::initialize_system_table` that constructed a `StandardBootServices` for the component dispatcher. `StandardRuntimeServices` not modified. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Removes the `StandardBootServices` param from the component benchmark entrry points since `StandardBootServices` is about to be removed from the component model. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Removes the `StandardBootServices` `Param` implementation and all references to it. Removes the `boot_services` field from `Storage`. Removes logic in the `#[component]` parameter validation macro that checked for duplicate `StandardBootServices` parameters. In cases where `StandardBootServices` was used as an example type in tests for `Option<T>` and tuple `Param` implementations, it was replaced with `StandardRuntimeServices`. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Provides a `locate_first_handle()` method to more conveniently find a single handle that supports a given protocol. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Adds a `DriverBinding` trait that can be implemented and passed to `install_driver_binding()` to produce a driver binding protocol in a component. Since components are all statically compiled with the DXE Core, they cannot use image handles as agent handles as is common in C-based DXE drivers. Therefore, a `register_agent()` function is added to `ProtocolServices` that generates a unique agent handle for a component to use when opening protocols. An example of using this API and opening a protocol is added to `patina_samples`. The `open_protocol()` interface is updated to accept an agent handle and `OpenAttributes` that describe how the protocol is being opened. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Straightforward update to docs that referenced `StandardBootServices` to either drop the reference or replace it with the equivalent info or example from UEFI Services. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Adds a document that explains common scenario where services need be shared and stored and patterns for those scenarios. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Adds a document that explains when to use Patina UEFI Services and links to individual service documentation. Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
75530e3 to
1fed1b9
Compare
Merged main into the major branch (at commit 35f9055) and rebased the PR onto it. The main set of changes added are in commit 59cc862. |
Description
Includes an initial merge commit from the current main branch into major.
Adds a new set of Patina component services called "UEFI Services". These services are defined in the Patina SDK, produced by the Patina DXE Core, and used by Patina components.
Main changes:
sdk\patina\src\component\service\uefi_servicespatina_dxe_core\src\uefi_servicescomponents\patina_samples\src\component\uefi_services/components) to Patina UEFI Services fromStandardBootServices.ServiceCell<T>for uses cases whereService<T>::new_unit()is used today + an additional case. Slightly controversial. Considered an incremental improvement in clarity and safety. See the commit message for reasoning.StandardBootServicesand related changes to do so across the codebasedocs\src\component\storing_state.md.docs\src\component\storing_state.md.How This Was Tested
cargo make allInitial component dispatch (DXE core and sample components):
Later after the Timer Architectural protocol is installed:
End of DXE messages:
All Patina on-system tests pass:
Integration Instructions