-
Notifications
You must be signed in to change notification settings - Fork 41
Add new Handle param for patina components
#1244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Add new Handle param for patina components
#1244
Conversation
This commit adds a new `Handle` param that can be requested by patina components. This component is currently only an on-demand wrapper around a r_efi::efi::Handle, which can be used by components when interacting with UEFI services. However future enhancements may include additional metadata useful from within the patina-component context.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
| impl Handle { | ||
| /// Creates an instance of Handle from the given EFI handle. | ||
| /// | ||
| /// This function is intended for testing purposes only. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this intended for unit tests or patina-tests? Seems odd to leave test function without any feature wrapper or otherwise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is intended for unit tests. All of the different Param implementations have one. I'm happy to put it behind a feature if you wish. I'd probably do it in a separate PR and do it for all of them though.
| /// use patina::component::params::Handle; | ||
| /// use patina::boot_services::{BootServices, StandardBootServices}; | ||
| /// | ||
| /// fn my_component(handle: Handle, bs: StandardBootServices) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious the motivation for adding this, is there a particular use case you had in mind?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC it was requested in the past, but the main motivator this time is for patina_boot (#1225), which loads images and needs a valid image handle.
| crate::BinaryGuid::from_string("3346877B-C962-4A34-A42D-50B8437B827D"); | ||
|
|
||
| // SAFETY: We are installing a marker protocol, so the interface is expected to be null. | ||
| let result = unsafe { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way for us to add a log that can help us associate a handle with a component here?
| } | ||
|
|
||
| fn init_state(storage: &mut Storage, _meta: &mut MetaData) -> Result<Self::State, Cow<'static, str>> { | ||
| const PATINA_COMPONENT_GUID: crate::BinaryGuid = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure a marker is sufficient, at least for the purposes of the boot orchestrator that @kat-perez is working on. That wants to use this handle as the parent_handle in the call to load_image, and that means it should be an image handle (i.e. a handle with LoadedImage/LoadedImageDevicePath on it).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah. Got it. I'll look into our ability to add the two protocols like we do for a normal image.
Description
This commit adds a new
Handleparam that can be requested by patina components. This component is currently only an on-demand wrapper around a r_efi::efi::Handle, which can be used by components when interacting with UEFI services. However future enhancements may include additional metadata useful from within the patina-component context.This commit required that the system tables were setup before components added, so that protocols can be added to the protocol db without breaking the current API barrier.
I plan on adding unit tests if this is the route we wish to go.
How This Was Tested
Continued boot to shell.
Integration Instructions
Users may use the provided
Handle.