Merge pull request #216 from rust-osdev/efi-mmap

multiboot2: fix handling of efi memory map

Cargo.lock

@@ -27,7 +27,7 @@ checksum = "90ed8c1e510134f979dbc4f070f87d4313098b704861a105fe34231c70a3901c"
 
 [[package]]
 name = "multiboot2"
-version = "0.20.0"
+version = "0.20.1"
 dependencies = [
  "bitflags",
  "derive_more",

integration-test/bins/Cargo.lock

@@ -4,15 +4,15 @@ version = 3
 
 [[package]]
 name = "anyhow"
-version = "1.0.82"
+version = "1.0.86"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "f538837af36e6f6a9be0faa67f9a314f8119e4e4b5867c6ab40ed60360142519"
+checksum = "b3d1d046238990b9cf5bcde22a3fb3584ee5cf65fb2765f454ed428c7a0063da"
 
 [[package]]
 name = "autocfg"
-version = "1.2.0"
+version = "1.3.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "f1fdabc7756949593fe60f30ec81974b613357de856987752631dea1e3394c80"
+checksum = "0c4b4d0bd25bd0b74681c0ad21497610ce1b7c91b1022cd21c80c6fbdd9476b0"
 
 [[package]]
 name = "bit_field"
@@ -45,9 +45,9 @@ dependencies = [
 
 [[package]]
 name = "either"
-version = "1.11.0"
+version = "1.12.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "a47c1c47d2f5964e29c61246e81db715514cd532db6b5116a25ea3c03d6780a2"
+checksum = "3dca9240753cf90908d7e4aac30f630662b02aebaa1b58a3cadabdb23385b58b"
 
 [[package]]
 name = "elf_rs"
@@ -97,6 +97,8 @@ dependencies = [
 [[package]]
 name = "multiboot2"
 version = "0.20.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "3d67e1b461b49127f2226c78a2b4090f72212c44fa27342bcfef93dd39bd6b86"
 dependencies = [
  "bitflags 2.5.0",
  "derive_more",
@@ -107,9 +109,7 @@ dependencies = [
 
 [[package]]
 name = "multiboot2"
-version = "0.20.0"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "3d67e1b461b49127f2226c78a2b4090f72212c44fa27342bcfef93dd39bd6b86"
+version = "0.20.1"
 dependencies = [
  "bitflags 2.5.0",
  "derive_more",
@@ -123,7 +123,7 @@ name = "multiboot2-header"
 version = "0.4.0"
 dependencies = [
  "derive_more",
- "multiboot2 0.20.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "multiboot2 0.20.0",
 ]
 
 [[package]]
@@ -135,7 +135,7 @@ dependencies = [
  "good_memory_allocator",
  "log",
  "multiboot",
- "multiboot2 0.20.0",
+ "multiboot2 0.20.1",
  "multiboot2-header",
  "util",
 ]
@@ -147,31 +147,31 @@ dependencies = [
  "anyhow",
  "good_memory_allocator",
  "log",
- "multiboot2 0.20.0",
+ "multiboot2 0.20.1",
  "util",
  "x86",
 ]
 
 [[package]]
 name = "num-traits"
-version = "0.2.18"
+version = "0.2.19"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "da0df0e5185db44f69b44f26786fe401b6c293d1907744beaa7fa62b2e5a517a"
+checksum = "071dfc062690e90b734c0b2273ce72ad0ffa95f0c74596bc250dcfd960262841"
 dependencies = [
  "autocfg",
 ]
 
 [[package]]
 name = "paste"
-version = "1.0.14"
+version = "1.0.15"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "de3145af08024dea9fa9914f381a17b8fc6034dfb00f3a84013f7ff43f29ed4c"
+checksum = "57c0d7b74b563b49d38dae00a0c37d4d6de9b432382b2892f0574ddcae73fd0a"
 
 [[package]]
 name = "proc-macro2"
-version = "1.0.81"
+version = "1.0.84"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "3d1597b0c024618f09a9c3b8655b7e430397a36d23fdafec26d6965e9eec3eba"
+checksum = "ec96c6a92621310b51366f1e28d05ef11489516e93be030060e5fc12024a49d6"
 dependencies = [
  "unicode-ident",
 ]

multiboot2/Cargo.toml

@@ -6,7 +6,7 @@ Multiboot2-compliant bootloaders, such as GRUB. It supports all tags from the
 specification including full support for the sections of ELF files. This library
 is `no_std` and can be used in a Multiboot2-kernel.
 """
-version = "0.20.0"
+version = "0.20.1"
 authors = [
     "Philipp Oppermann <dev@phil-opp.com>",
     "Calvin Lee <cyrus296@gmail.com>",

multiboot2/Changelog.md

@@ -1,6 +1,12 @@
 # CHANGELOG for crate `multiboot2`
 
-## Unreleased
+## 0.20.1 (2024-05-26)
+
+- fixed the handling of `EFIMemoryMapTag` and `EFIMemoryAreaIter`
+- **BREAKING** Fixed wrong creation of `EFIMemoryMapTag`.
+  `EFIMemoryMapTag::new` was replaced by `EFIMemoryMapTag::new_from_descs` and
+  `EFIMemoryMapTag::new_from_map`.
+- `ModuleTag::new`'s `end` parameter now must be bigger than `start`.
 
 ## 0.20.0 (2024-05-01)
 

multiboot2/src/lib.rs

@@ -77,8 +77,8 @@ pub use end::EndTag;
 pub use framebuffer::{FramebufferColor, FramebufferField, FramebufferTag, FramebufferType};
 pub use image_load_addr::ImageLoadPhysAddrTag;
 pub use memory_map::{
-    BasicMemoryInfoTag, EFIMemoryAreaType, EFIMemoryDesc, EFIMemoryMapTag, MemoryArea,
-    MemoryAreaType, MemoryAreaTypeId, MemoryMapTag,
+    BasicMemoryInfoTag, EFIMemoryAreaType, EFIMemoryAttribute, EFIMemoryDesc, EFIMemoryMapTag,
+    MemoryArea, MemoryAreaType, MemoryAreaTypeId, MemoryMapTag,
 };
 pub use module::{ModuleIter, ModuleTag};
 pub use ptr_meta::Pointee;
@@ -302,7 +302,10 @@ impl<'a> BootInformation<'a> {
         // If the EFIBootServicesNotExited is present, then we should not use
         // the memory map, as it could still be in use.
         match self.get_tag::<EFIBootServicesNotExitedTag>() {
-            Some(_tag) => None,
+            Some(_tag) => {
+                log::debug!("The EFI memory map is present but the UEFI Boot Services Not Existed Tag is present. Returning None.");
+                None
+            }
             None => self.get_tag::<EFIMemoryMapTag>(),
         }
     }
@@ -1450,15 +1453,15 @@ mod tests {
     #[cfg_attr(miri, ignore)]
     fn efi_memory_map() {
         #[repr(C, align(8))]
-        struct Bytes([u8; 72]);
+        struct Bytes([u8; 80]);
         // test that the EFI memory map is detected.
         let bytes: Bytes = Bytes([
-            72, 0, 0, 0, // size
+            80, 0, 0, 0, // size
             0, 0, 0, 0, // reserved
             17, 0, 0, 0, // EFI memory map type
-            56, 0, 0, 0, // EFI memory map size
+            64, 0, 0, 0, // EFI memory map size
             48, 0, 0, 0, // EFI descriptor size
-            1, 0, 0, 0, // EFI descriptor version, don't think this matters.
+            1, 0, 0, 0, // EFI descriptor version
             7, 0, 0, 0, // Type: EfiConventionalMemory
             0, 0, 0, 0, // Padding
             0, 0, 16, 0, // Physical Address: should be 0x100000
@@ -1469,6 +1472,8 @@ mod tests {
             0, 0, 0, 0, // Extension of pages
             0, 0, 0, 0, // Attributes of this memory range.
             0, 0, 0, 0, // Extension of attributes
+            0, 0, 0, 0, // More padding to extend the efi mmap to `desc_size`.
+            0, 0, 0, 0, // padding/alignment for end tag
             0, 0, 0, 0, // end tag type.
             8, 0, 0, 0, // end tag size.
         ]);

multiboot2/src/memory_map.rs

@@ -1,6 +1,7 @@
 //! Module for [`MemoryMapTag`], [`EFIMemoryMapTag`] and [`BasicMemoryInfoTag`]
 //! and corresponding helper types.
 
+pub use uefi_raw::table::boot::MemoryAttribute as EFIMemoryAttribute;
 pub use uefi_raw::table::boot::MemoryDescriptor as EFIMemoryDesc;
 pub use uefi_raw::table::boot::MemoryType as EFIMemoryAreaType;
 
@@ -55,7 +56,10 @@ impl MemoryMapTag {
         self.entry_version
     }
 
-    /// Return the slice with all memory areas.
+    /// Return the slice of the provided [`MemoryArea`]s.
+    ///
+    /// Usually, this should already reflect the memory consumed by the
+    /// code running this.
     pub fn memory_areas(&self) -> &[MemoryArea] {
         // If this ever fails, we need to model this differently in this crate.
         assert_eq!(self.entry_size as usize, mem::size_of::<MemoryArea>());
@@ -74,7 +78,7 @@ impl TagTrait for MemoryMapTag {
     }
 }
 
-/// A memory area entry descriptor.
+/// A descriptor for an available or taken area of physical memory.
 #[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 #[repr(C)]
 pub struct MemoryArea {
@@ -278,15 +282,30 @@ const EFI_METADATA_SIZE: usize = mem::size_of::<TagTypeId>() + 3 * mem::size_of:
 #[cfg(feature = "builder")]
 impl AsBytes for EFIMemoryDesc {}
 
-/// EFI memory map tag. The [`EFIMemoryDesc`] follows the EFI specification.
+/// EFI memory map tag. The embedded [`EFIMemoryDesc`]s follows the EFI
+/// specification.
 #[derive(ptr_meta::Pointee, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
 #[repr(C)]
 pub struct EFIMemoryMapTag {
     typ: TagTypeId,
     size: u32,
+    /// Most likely a little more than the size of a [`EFIMemoryDesc`].
+    /// This is always the reference, and `size_of` never.
+    /// See <https://github.com/tianocore/edk2/blob/7142e648416ff5d3eac6c6d607874805f5de0ca8/MdeModulePkg/Core/PiSmmCore/Page.c#L1059>.
     desc_size: u32,
+    /// Version of the tag. The spec leaves it open to extend the memory
+    /// descriptor in the future. However, this never happened so far.
+    /// At the moment, only version "1" is supported.
     desc_version: u32,
-    descs: [EFIMemoryDesc],
+    /// Contains the UEFI memory map.
+    ///
+    /// To follow the UEFI spec and to allow extendability for future UEFI
+    /// revisions, the length is a multiple of `desc_size` and not a multiple
+    /// of `size_of::<EfiMemoryDescriptor>()`.
+    ///
+    /// This tag is properly `align_of::<EFIMemoryDesc>` aligned, if the tag
+    /// itself is also 8 byte aligned, which every sane MBI guarantees.
+    memory_map: [u8],
 }
 
 impl EFIMemoryMapTag {
@@ -294,34 +313,71 @@ impl EFIMemoryMapTag {
     /// Create a new EFI memory map tag with the given memory descriptors.
     /// Version and size can't be set because you're passing a slice of
     /// EFIMemoryDescs, not the ones you might have gotten from the firmware.
-    pub fn new(descs: &[EFIMemoryDesc]) -> BoxedDst<Self> {
-        // update this when updating EFIMemoryDesc
-        const MEMORY_DESCRIPTOR_VERSION: u32 = 1;
-        let mut bytes = [
-            (mem::size_of::<EFIMemoryDesc>() as u32).to_le_bytes(),
-            MEMORY_DESCRIPTOR_VERSION.to_le_bytes(),
-        ]
-        .concat();
+    pub fn new_from_descs(descs: &[EFIMemoryDesc]) -> BoxedDst<Self> {
+        // TODO replace this EfiMemorydesc::uefi_desc_size() in the next uefi_raw
+        // release.
+
+        let size_base = mem::size_of::<EFIMemoryDesc>();
+        // Taken from https://github.com/tianocore/edk2/blob/7142e648416ff5d3eac6c6d607874805f5de0ca8/MdeModulePkg/Core/PiSmmCore/Page.c#L1059
+        let desc_size_diff = mem::size_of::<u64>() - size_base % mem::size_of::<u64>();
+        let desc_size = size_base + desc_size_diff;
+
+        assert!(desc_size >= size_base);
+
+        let mut efi_mmap = alloc::vec::Vec::with_capacity(descs.len() * desc_size);
         for desc in descs {
-            bytes.extend(desc.as_bytes());
+            efi_mmap.extend(desc.as_bytes());
+            // fill with zeroes
+            efi_mmap.extend([0].repeat(desc_size_diff));
         }
-        BoxedDst::new(bytes.as_slice())
+
+        Self::new_from_map(
+            desc_size as u32,
+            EFIMemoryDesc::VERSION,
+            efi_mmap.as_slice(),
+        )
     }
 
-    /// Return an iterator over ALL marked memory areas.
+    #[cfg(feature = "builder")]
+    /// Create a new EFI memory map tag from the given EFI memory map.
+    pub fn new_from_map(desc_size: u32, desc_version: u32, efi_mmap: &[u8]) -> BoxedDst<Self> {
+        assert!(desc_size > 0);
+        assert_eq!(efi_mmap.len() % desc_size as usize, 0);
+        assert_eq!(
+            efi_mmap
+                .as_ptr()
+                .align_offset(mem::align_of::<EFIMemoryDesc>()),
+            0
+        );
+        let bytes = [
+            &desc_size.to_le_bytes(),
+            &desc_version.to_le_bytes(),
+            efi_mmap,
+        ]
+        .concat();
+        BoxedDst::new(&bytes)
+    }
+
+    /// Returns an iterator over the provided memory areas.
     ///
-    /// This differs from `MemoryMapTag` as for UEFI, the OS needs some non-
-    /// available memory areas for tables and such.
+    /// Usually, this should already reflect the memory consumed by the
+    /// code running this.
     pub fn memory_areas(&self) -> EFIMemoryAreaIter {
-        let self_ptr = self as *const EFIMemoryMapTag;
-        let start_area = (&self.descs[0]) as *const EFIMemoryDesc;
-        EFIMemoryAreaIter {
-            current_area: start_area as u64,
-            // NOTE: `last_area` is only a bound, it doesn't necessarily point exactly to the last element
-            last_area: (self_ptr as *const () as u64 + self.size as u64),
-            entry_size: self.desc_size,
-            phantom: PhantomData,
+        // If this ever fails, this needs to be refactored in a joint-effort
+        // with the uefi-rs project to have all corresponding typings.
+        assert_eq!(self.desc_version, EFIMemoryDesc::VERSION);
+        assert_eq!(
+            self.memory_map
+                .as_ptr()
+                .align_offset(mem::align_of::<EFIMemoryDesc>()),
+            0
+        );
+
+        if self.desc_size as usize > mem::size_of::<EFIMemoryDesc>() {
+            log::debug!("desc_size larger than expected typing. We might miss a few fields.");
         }
+
+        EFIMemoryAreaIter::new(self)
     }
 }
 
@@ -330,30 +386,219 @@ impl TagTrait for EFIMemoryMapTag {
 
     fn dst_size(base_tag: &Tag) -> usize {
         assert!(base_tag.size as usize >= EFI_METADATA_SIZE);
-        let size = base_tag.size as usize - EFI_METADATA_SIZE;
-        assert_eq!(size % mem::size_of::<EFIMemoryDesc>(), 0);
-        size / mem::size_of::<EFIMemoryDesc>()
+        base_tag.size as usize - EFI_METADATA_SIZE
     }
 }
 
-/// An iterator over ALL EFI memory areas.
+/// An iterator over the EFI memory areas emitting [`EFIMemoryDesc`] items.
 #[derive(Clone, Debug)]
 pub struct EFIMemoryAreaIter<'a> {
-    current_area: u64,
-    last_area: u64,
-    entry_size: u32,
+    mmap_tag: &'a EFIMemoryMapTag,
+    i: usize,
+    entries: usize,
     phantom: PhantomData<&'a EFIMemoryDesc>,
 }
 
+impl<'a> EFIMemoryAreaIter<'a> {
+    fn new(mmap_tag: &'a EFIMemoryMapTag) -> Self {
+        let desc_size = mmap_tag.desc_size as usize;
+        let mmap_len = mmap_tag.memory_map.len();
+        assert_eq!(mmap_len % desc_size, 0, "memory map length must be a multiple of `desc_size` by definition. The MBI seems to be corrupt.");
+        Self {
+            mmap_tag,
+            i: 0,
+            entries: mmap_len / desc_size,
+            phantom: PhantomData,
+        }
+    }
+}
+
 impl<'a> Iterator for EFIMemoryAreaIter<'a> {
     type Item = &'a EFIMemoryDesc;
     fn next(&mut self) -> Option<&'a EFIMemoryDesc> {
-        if self.current_area > self.last_area {
-            None
-        } else {
-            let area = unsafe { &*(self.current_area as *const EFIMemoryDesc) };
-            self.current_area += self.entry_size as u64;
-            Some(area)
+        if self.i >= self.entries {
+            return None;
         }
+
+        let desc = unsafe {
+            self.mmap_tag
+                .memory_map
+                .as_ptr()
+                .add(self.i * self.mmap_tag.desc_size as usize)
+                .cast::<EFIMemoryDesc>()
+                .as_ref()
+                .unwrap()
+        };
+
+        self.i += 1;
+
+        Some(desc)
+    }
+}
+
+impl<'a> ExactSizeIterator for EFIMemoryAreaIter<'a> {
+    fn len(&self) -> usize {
+        self.entries
+    }
+}
+
+#[cfg(all(test, feature = "builder", not(miri)))]
+mod tests {
+    use super::*;
+    use std::mem::size_of;
+
+    #[test]
+    fn construction_and_parsing() {
+        let descs = [
+            EFIMemoryDesc {
+                ty: EFIMemoryAreaType::CONVENTIONAL,
+                phys_start: 0x1000,
+                virt_start: 0x1000,
+                page_count: 1,
+                att: Default::default(),
+            },
+            EFIMemoryDesc {
+                ty: EFIMemoryAreaType::LOADER_DATA,
+                phys_start: 0x2000,
+                virt_start: 0x2000,
+                page_count: 3,
+                att: Default::default(),
+            },
+        ];
+        let efi_mmap_tag = EFIMemoryMapTag::new_from_descs(&descs);
+
+        assert_eq!(efi_mmap_tag.desc_size, 48 /* 40 + 8 */);
+
+        let mut iter = efi_mmap_tag.memory_areas();
+
+        assert_eq!(iter.next(), Some(&descs[0]));
+        assert_eq!(iter.next(), Some(&descs[1]));
+
+        assert_eq!(iter.next(), None);
+    }
+
+    /// Tests the EFI memory map parsing using a real world efi memory map.
+    /// This is taken from the uefi-rs repository. See
+    /// <https://github.com/rust-osdev/uefi-rs/pull/1175> for more info.
+    #[test]
+    fn test_real_data() {
+        const DESC_SIZE: u32 = 48;
+        const DESC_VERSION: u32 = 1;
+        /// Sample with 10 entries of a real UEFI memory map extracted from our
+        /// UEFI test runner.
+        const MMAP_RAW: [u64; 60] = [
+            3, 0, 0, 1, 15, 0, 7, 4096, 0, 134, 15, 0, 4, 552960, 0, 1, 15, 0, 7, 557056, 0, 24,
+            15, 0, 7, 1048576, 0, 1792, 15, 0, 10, 8388608, 0, 8, 15, 0, 7, 8421376, 0, 3, 15, 0,
+            10, 8433664, 0, 1, 15, 0, 7, 8437760, 0, 4, 15, 0, 10, 8454144, 0, 240, 15, 0,
+        ];
+        let buf = MMAP_RAW;
+        let buf = unsafe {
+            core::slice::from_raw_parts(buf.as_ptr().cast::<u8>(), buf.len() * size_of::<u64>())
+        };
+        let tag = EFIMemoryMapTag::new_from_map(DESC_SIZE, DESC_VERSION, buf);
+        let entries = tag.memory_areas().copied().collect::<alloc::vec::Vec<_>>();
+        let expected = [
+            EFIMemoryDesc {
+                ty: EFIMemoryAreaType::BOOT_SERVICES_CODE,
+                phys_start: 0x0,
+                virt_start: 0x0,
+                page_count: 0x1,
+                att: EFIMemoryAttribute::UNCACHEABLE
+                    | EFIMemoryAttribute::WRITE_COMBINE
+                    | EFIMemoryAttribute::WRITE_THROUGH
+                    | EFIMemoryAttribute::WRITE_BACK,
+            },
+            EFIMemoryDesc {
+                ty: EFIMemoryAreaType::CONVENTIONAL,
+                phys_start: 0x1000,
+                virt_start: 0x0,
+                page_count: 0x86,
+                att: EFIMemoryAttribute::UNCACHEABLE
+                    | EFIMemoryAttribute::WRITE_COMBINE
+                    | EFIMemoryAttribute::WRITE_THROUGH
+                    | EFIMemoryAttribute::WRITE_BACK,
+            },
+            EFIMemoryDesc {
+                ty: EFIMemoryAreaType::BOOT_SERVICES_DATA,
+                phys_start: 0x87000,
+                virt_start: 0x0,
+                page_count: 0x1,
+                att: EFIMemoryAttribute::UNCACHEABLE
+                    | EFIMemoryAttribute::WRITE_COMBINE
+                    | EFIMemoryAttribute::WRITE_THROUGH
+                    | EFIMemoryAttribute::WRITE_BACK,
+            },
+            EFIMemoryDesc {
+                ty: EFIMemoryAreaType::CONVENTIONAL,
+                phys_start: 0x88000,
+                virt_start: 0x0,
+                page_count: 0x18,
+                att: EFIMemoryAttribute::UNCACHEABLE
+                    | EFIMemoryAttribute::WRITE_COMBINE
+                    | EFIMemoryAttribute::WRITE_THROUGH
+                    | EFIMemoryAttribute::WRITE_BACK,
+            },
+            EFIMemoryDesc {
+                ty: EFIMemoryAreaType::CONVENTIONAL,
+                phys_start: 0x100000,
+                virt_start: 0x0,
+                page_count: 0x700,
+                att: EFIMemoryAttribute::UNCACHEABLE
+                    | EFIMemoryAttribute::WRITE_COMBINE
+                    | EFIMemoryAttribute::WRITE_THROUGH
+                    | EFIMemoryAttribute::WRITE_BACK,
+            },
+            EFIMemoryDesc {
+                ty: EFIMemoryAreaType::ACPI_NON_VOLATILE,
+                phys_start: 0x800000,
+                virt_start: 0x0,
+                page_count: 0x8,
+                att: EFIMemoryAttribute::UNCACHEABLE
+                    | EFIMemoryAttribute::WRITE_COMBINE
+                    | EFIMemoryAttribute::WRITE_THROUGH
+                    | EFIMemoryAttribute::WRITE_BACK,
+            },
+            EFIMemoryDesc {
+                ty: EFIMemoryAreaType::CONVENTIONAL,
+                phys_start: 0x808000,
+                virt_start: 0x0,
+                page_count: 0x3,
+                att: EFIMemoryAttribute::UNCACHEABLE
+                    | EFIMemoryAttribute::WRITE_COMBINE
+                    | EFIMemoryAttribute::WRITE_THROUGH
+                    | EFIMemoryAttribute::WRITE_BACK,
+            },
+            EFIMemoryDesc {
+                ty: EFIMemoryAreaType::ACPI_NON_VOLATILE,
+                phys_start: 0x80b000,
+                virt_start: 0x0,
+                page_count: 0x1,
+                att: EFIMemoryAttribute::UNCACHEABLE
+                    | EFIMemoryAttribute::WRITE_COMBINE
+                    | EFIMemoryAttribute::WRITE_THROUGH
+                    | EFIMemoryAttribute::WRITE_BACK,
+            },
+            EFIMemoryDesc {
+                ty: EFIMemoryAreaType::CONVENTIONAL,
+                phys_start: 0x80c000,
+                virt_start: 0x0,
+                page_count: 0x4,
+                att: EFIMemoryAttribute::UNCACHEABLE
+                    | EFIMemoryAttribute::WRITE_COMBINE
+                    | EFIMemoryAttribute::WRITE_THROUGH
+                    | EFIMemoryAttribute::WRITE_BACK,
+            },
+            EFIMemoryDesc {
+                ty: EFIMemoryAreaType::ACPI_NON_VOLATILE,
+                phys_start: 0x810000,
+                virt_start: 0x0,
+                page_count: 0xf0,
+                att: EFIMemoryAttribute::UNCACHEABLE
+                    | EFIMemoryAttribute::WRITE_COMBINE
+                    | EFIMemoryAttribute::WRITE_THROUGH
+                    | EFIMemoryAttribute::WRITE_BACK,
+            },
+        ];
+        assert_eq!(entries.as_slice(), &expected);
     }
 }

multiboot2/src/module.rs

@@ -26,6 +26,8 @@ pub struct ModuleTag {
 impl ModuleTag {
     #[cfg(feature = "builder")]
     pub fn new(start: u32, end: u32, cmdline: &str) -> BoxedDst<Self> {
+        assert!(end > start, "must have a size");
+
         let mut cmdline_bytes: Vec<_> = cmdline.bytes().collect();
         if !cmdline_bytes.ends_with(&[0]) {
             // terminating null-byte
@@ -135,7 +137,7 @@ mod tests {
             &((TagType::Module.val()).to_le_bytes()),
             &size.to_le_bytes(),
             &0_u32.to_le_bytes(),
-            &0_u32.to_le_bytes(),
+            &1_u32.to_le_bytes(),
             MSG.as_bytes(),
             // Null Byte
             &[0],
@@ -161,15 +163,15 @@ mod tests {
     #[test]
     #[cfg(feature = "builder")]
     fn test_build_str() {
-        let tag = ModuleTag::new(0, 0, MSG);
+        let tag = ModuleTag::new(0, 1, MSG);
         let bytes = tag.as_bytes();
         assert_eq!(bytes, get_bytes());
         assert_eq!(tag.cmdline(), Ok(MSG));
 
         // test also some bigger message
-        let tag = ModuleTag::new(0, 0, "AbCdEfGhUjK YEAH");
+        let tag = ModuleTag::new(0, 1, "AbCdEfGhUjK YEAH");
         assert_eq!(tag.cmdline(), Ok("AbCdEfGhUjK YEAH"));
-        let tag = ModuleTag::new(0, 0, "AbCdEfGhUjK YEAH".repeat(42).as_str());
+        let tag = ModuleTag::new(0, 1, "AbCdEfGhUjK YEAH".repeat(42).as_str());
         assert_eq!(tag.cmdline(), Ok("AbCdEfGhUjK YEAH".repeat(42).as_str()));
     }
 }

multiboot2/src/tag_trait.rs

@@ -51,7 +51,7 @@ pub trait TagTrait: Pointee {
     /// Callers must be sure that the "size" field of the provided [`Tag`] is
     /// sane and the underlying memory valid. The implementation of this trait
     /// **must have** a correct [`Self::dst_size`] implementation.
-    unsafe fn from_base_tag<'a>(tag: &Tag) -> &'a Self {
+    unsafe fn from_base_tag(tag: &Tag) -> &Self {
         let ptr = core::ptr::addr_of!(*tag);
         let ptr = ptr_meta::from_raw_parts(ptr.cast(), Self::dst_size(tag));
         &*ptr