uefi-raw: Add Device Path protocol definitions. #1435
Conversation
It does not matter for the header as it only contain u8s. However more complex device paths that need to embed the header are also packed structs. Signed-off-by: Mate Kukri <[email protected]>
Signed-off-by: Mate Kukri <[email protected]>
Some Device Paths have variable length trailer data, so a fully safe API would be rather difficult to provide without reliance on custom DSTs (and somewhat breaking the seperation of concerns between uefi-raw and uefi). Presence of this in just uefi-raw is still useful for applications intending to examine Device Paths. Signed-off-by: Mate Kukri <[email protected]>
| #[derive(Debug)] | ||
| #[repr(C)] | ||
| #[derive(Copy,Clone,Debug)] | ||
| #[repr(C,packed)] |
There was a problem hiding this comment.
TL;DR: I'm fine with a unit test that ensures the ABI/size_of, tho! We also do this in other parts of the crate I think
Please don't use packed. Experience shows that packed makes
it annoying to access members. Only use it if it is necessary to get the
right layout.
Here, it doesn't seem to have an effect. Look:
#[repr(C)]
pub struct Base {
pub major_type: u8,
pub sub_type: u8,
pub length: [u8; 2],
}
#[repr(C, packed)]
pub struct Packed {
pub major_type: u8,
pub sub_type: u8,
pub length: [u8; 2],
}
fn main() {
assert_eq!(size_of::<Base>(), size_of::<Packed>());
assert_eq!(align_of::<Base>(), align_of::<Packed>());
}
|
|
||
| /// PCI Device Path. | ||
| #[derive(Debug)] | ||
| #[repr(C,packed)] |
There was a problem hiding this comment.
| #[repr(C,packed)] | |
| #[repr(C)] |
Same here. Please instead add a unit test that tests the ABI guarantees. Please only use packed if the default layout is wrong
|
|
||
| /// PCCARD Device Path. | ||
| #[derive(Debug)] | ||
| #[repr(C,packed)] |
There was a problem hiding this comment.
| #[repr(C,packed)] | |
| #[repr(C)] |
and here .. and so on
| @@ -1,4 +1,60 @@ | |||
| use crate::{guid, Char16, Guid}; | |||
| /* | |||
There was a problem hiding this comment.
I don't think that this is necessary. Right, @nicholasbishop ?
There was a problem hiding this comment.
I agree; we're just adding types defined in the UEFI spec here, no need to add the EDK2 license.
|
Thanks for the PR! I'll leave final feedback to @nicholasbishop who is more experienced with our Device Path code and infrastructure |
| @@ -1,4 +1,60 @@ | |||
| use crate::{guid, Char16, Guid}; | |||
| /* | |||
There was a problem hiding this comment.
I agree; we're just adding types defined in the UEFI spec here, no need to add the EDK2 license.
| /// 0x00 - Public Device Address | ||
| /// 0x01 - Random Device Address | ||
| pub address_type: u8, | ||
| } |
There was a problem hiding this comment.
I'd like to hold off on manually adding specific device path node types to uefi-raw. In the uefi crate, we're autogenerating code for node types, and I think we can just extend that to generate code in uefi-raw too.
Understood, this was code I origianlly did for for a project (personal) where I've been only using uefi-raw. |
This pull requests adds definitions for examining device path components.
These were derived from edk2 (mainly the comments), so I added the edk2 license on top of the file.
I belive edk2's BSD type license is compatible with MPL-2.0, but is mixed license code okay in this project?
Some Device Paths have variable length trailer data, so a fully safe API
would be rather difficult to provide without reliance on custom DSTs
(and somewhat breaking the seperation of concerns between uefi-raw and uefi).