Merge pull request #1613 from rust-osdev/dvcp

uefi: remove duplication in DevicePathHeader; use uefi-raw

Author: phip1611

uefi-raw/CHANGELOG.md

@@ -14,6 +14,11 @@
 - Added `DevicePathUtilitiesProtocol`.
 - Added `UsbIoProtocol`.
 - Added `Usb2HostControllerProtocol`.
+- Added  `DevicePathProtocol::length()` properly constructing the `u16` value
+
+## Changed
+- `DevicePathProtocol` now derives
+  `Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd, Hash`
 
 
 # uefi-raw - 0.10.0 (2025-02-07)

uefi-raw/src/protocol/device_path.rs

@@ -8,22 +8,29 @@ pub use device_path_gen::{acpi, bios_boot_spec, end, hardware, media, messaging}
 
 /// Device path protocol.
 ///
-/// A device path contains one or more device path instances made of up
+/// A device path contains one or more device path instances made up of
 /// variable-length nodes.
 ///
 /// Note that the fields in this struct define the header at the start of each
 /// node; a device path is typically larger than these four bytes.
-#[derive(Debug)]
+#[derive(Clone, Copy, Debug, Eq, PartialEq, Ord, PartialOrd, Hash)]
 #[repr(C)]
 pub struct DevicePathProtocol {
     pub major_type: DeviceType,
     pub sub_type: DeviceSubType,
+    /// Total length of the type including the fixed header as u16 in LE order.
     pub length: [u8; 2],
     // followed by payload (dynamically sized)
 }
 
 impl DevicePathProtocol {
     pub const GUID: Guid = guid!("09576e91-6d3f-11d2-8e39-00a0c969723b");
+
+    /// Returns the total length of the device path node.
+    #[must_use]
+    pub const fn length(&self) -> u16 {
+        u16::from_le_bytes(self.length)
+    }
 }
 
 newtype_enum! {
@@ -252,3 +259,17 @@ pub struct DevicePathUtilitiesProtocol {
 impl DevicePathUtilitiesProtocol {
     pub const GUID: Guid = guid!("0379be4e-d706-437d-b037-edb82fb772a4");
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use core::mem;
+
+    /// Test that ensures the struct is packed. Thus, we don't need to
+    /// explicitly specify `packed`.
+    #[test]
+    fn abi() {
+        assert_eq!(mem::size_of::<DevicePathProtocol>(), 4);
+        assert_eq!(mem::align_of::<DevicePathProtocol>(), 1);
+    }
+}

uefi/CHANGELOG.md

@@ -27,6 +27,8 @@
   `proto::device_path::text` to `proto::device_path`.
 - **Breaking:** `exit_boot_services` now consumes a `Option<MemoryType>` which
   defaults to the recommended value of `MemoryType::LOADER_DATA`.
+- **Breaking:** Removed duplication in `DevicePathHeader`. Instead of public fields,
+  there is now a public constructor combined with public getters.
 - `boot::memory_map()` will never return `Status::BUFFER_TOO_SMALL` from now on,
   as this is considered a hard internal error where users can't do anything
   about it anyway. It will panic instead.

uefi/src/proto/device_path/build.rs

@@ -225,7 +225,7 @@ pub unsafe trait BuildNode {
 
 unsafe impl BuildNode for &DevicePathNode {
     fn size_in_bytes(&self) -> Result<u16, BuildError> {
-        Ok(self.header.length)
+        Ok(self.header.length())
     }
 
     fn write_data(&self, out: &mut [MaybeUninit<u8>]) {

uefi/src/proto/device_path/device_path_gen.rs

@@ -80,6 +80,7 @@ pub mod text;
 pub mod util;
 
 mod device_path_gen;
+
 pub use device_path_gen::{
     acpi, bios_boot_spec, end, hardware, media, messaging, DevicePathNodeEnum,
 };
@@ -92,6 +93,7 @@ use core::fmt::{self, Debug, Display, Formatter};
 use core::ops::Deref;
 use ptr_meta::Pointee;
 
+use uefi_raw::protocol::device_path::DevicePathProtocol;
 #[cfg(feature = "alloc")]
 use {
     crate::boot::{self, OpenProtocolAttributes, OpenProtocolParams, ScopedProtocol, SearchType},
@@ -135,16 +137,41 @@ impl Deref for PoolDevicePathNode {
     }
 }
 
-/// Header that appears at the start of every [`DevicePathNode`].
+/// Fixed header that appears at the start of every [`DevicePathNode`].
+///
+/// This type is ABI-compatible with `EFI_DEVICE_PATH_PROTOCOL`.
 #[derive(Clone, Copy, Debug, Eq, PartialEq)]
-#[repr(C, packed)]
-pub struct DevicePathHeader {
-    /// Type of device
-    pub device_type: DeviceType,
-    /// Sub type of device
-    pub sub_type: DeviceSubType,
-    /// Size (in bytes) of the [`DevicePathNode`], including this header.
-    pub length: u16,
+#[repr(transparent)]
+pub struct DevicePathHeader(DevicePathProtocol);
+
+impl DevicePathHeader {
+    /// Constructs a new [`DevicePathHeader`].
+    #[must_use]
+    pub const fn new(major_type: DeviceType, sub_type: DeviceSubType, length: u16) -> Self {
+        Self(DevicePathProtocol {
+            major_type,
+            sub_type,
+            length: length.to_le_bytes(),
+        })
+    }
+
+    /// Returns the [`DeviceType`].
+    #[must_use]
+    pub const fn device_type(&self) -> DeviceType {
+        self.0.major_type
+    }
+
+    /// Returns the [`DeviceSubType`].
+    #[must_use]
+    pub const fn sub_type(&self) -> DeviceSubType {
+        self.0.sub_type
+    }
+
+    /// Returns the total length of the device path node.
+    #[must_use]
+    pub const fn length(&self) -> u16 {
+        self.0.length()
+    }
 }
 
 impl<'a> TryFrom<&'a [u8]> for &'a DevicePathHeader {
@@ -202,7 +229,7 @@ impl DevicePathNode {
     pub unsafe fn from_ffi_ptr<'a>(ptr: *const FfiDevicePath) -> &'a Self {
         let header = unsafe { *ptr.cast::<DevicePathHeader>() };
 
-        let data_len = usize::from(header.length) - size_of::<DevicePathHeader>();
+        let data_len = usize::from(header.length()) - size_of::<DevicePathHeader>();
         unsafe { &*ptr_meta::from_raw_parts(ptr.cast(), data_len) }
     }
 
@@ -216,25 +243,25 @@ impl DevicePathNode {
     /// Type of device
     #[must_use]
     pub const fn device_type(&self) -> DeviceType {
-        self.header.device_type
+        self.header.device_type()
     }
 
     /// Sub type of device
     #[must_use]
     pub const fn sub_type(&self) -> DeviceSubType {
-        self.header.sub_type
+        self.header.sub_type()
     }
 
     /// Tuple of the node's type and subtype.
     #[must_use]
     pub const fn full_type(&self) -> (DeviceType, DeviceSubType) {
-        (self.header.device_type, self.header.sub_type)
+        (self.device_type(), self.sub_type())
     }
 
     /// Size (in bytes) of the full [`DevicePathNode`], including the header.
     #[must_use]
     pub const fn length(&self) -> u16 {
-        self.header.length
+        self.header.length()
     }
 
     /// True if this node ends an entire [`DevicePath`].
@@ -297,7 +324,7 @@ impl<'a> TryFrom<&'a [u8]> for &'a DevicePathNode {
 
     fn try_from(bytes: &[u8]) -> Result<Self, Self::Error> {
         let dp = <&DevicePathHeader>::try_from(bytes)?;
-        if usize::from(dp.length) <= bytes.len() {
+        if usize::from(dp.length()) <= bytes.len() {
             unsafe { Ok(DevicePathNode::from_ffi_ptr(bytes.as_ptr().cast())) }
         } else {
             Err(ByteConversionError::InvalidLength)

uefi/src/proto/device_path/mod.rs

@@ -473,12 +473,14 @@ impl Node {
                 assert_eq!(size, out.len());
 
                 let out_ptr: *mut u8 = maybe_uninit_slice_as_mut_ptr(out);
+                let length = u16::try_from(size).unwrap();
+                let header = DevicePathHeader::new(
+                    DeviceType::#device_type,
+                    DeviceSubType::#sub_type,
+                    length,
+                );
                 unsafe {
-                    out_ptr.cast::<DevicePathHeader>().write_unaligned(DevicePathHeader {
-                        device_type: DeviceType::#device_type,
-                        sub_type: DeviceSubType::#sub_type,
-                        length: u16::try_from(size).unwrap(),
-                    });
+                    out_ptr.cast::<DevicePathHeader>().write_unaligned(header);
                     #(#copy_stmts)*
                 }
             }