Merge pull request #211 from rust-osdev/translate-redesign

phil-opp · web-flow · commit d9bc78a66d68 · 2020-12-28T12:36:02.000+01:00
Restructure the `TranslateResult` type and create separate `Translate` trait
diff --git a/Changelog.md b/Changelog.md
@@ -1,6 +1,7 @@
 # Unreleased
 
 - **Breaking:** Also return flags for `MapperAllSizes::translate()` ([#207](https://github.com/rust-osdev/x86_64/pull/207))
+- **Breaking:** Restructure the `TranslateResult` type and create separate `Translate` trait ([#211](https://github.com/rust-osdev/x86_64/pull/211))
 - **Breaking:** Use custom error types instead of `()` ([#199](https://github.com/rust-osdev/x86_64/pull/199))
 - Relaxe `Sized` requirement for `FrameAllocator` in `Mapper::map_to` ([204](https://github.com/rust-osdev/x86_64/pull/204))
 
diff --git a/src/structures/paging/mapper/mapped_page_table.rs b/src/structures/paging/mapper/mapped_page_table.rs
@@ -522,42 +522,42 @@ impl<'a, P: PhysToVirt> Mapper<Size4KiB> for MappedPageTable<'a, P> {
     }
 }
 
-impl<'a, P: PhysToVirt> MapperAllSizes for MappedPageTable<'a, P> {
+impl<'a, P: PhysToVirt> Translate for MappedPageTable<'a, P> {
     #[allow(clippy::inconsistent_digit_grouping)]
     fn translate(&self, addr: VirtAddr) -> TranslateResult {
         let p4 = &self.level_4_table;
         let p3 = match self.page_table_walker.next_table(&p4[addr.p4_index()]) {
             Ok(page_table) => page_table,
-            Err(PageTableWalkError::NotMapped) => return TranslateResult::PageNotMapped,
+            Err(PageTableWalkError::NotMapped) => return TranslateResult::NotMapped,
             Err(PageTableWalkError::MappedToHugePage) => {
                 panic!("level 4 entry has huge page bit set")
             }
         };
         let p2 = match self.page_table_walker.next_table(&p3[addr.p3_index()]) {
             Ok(page_table) => page_table,
-            Err(PageTableWalkError::NotMapped) => return TranslateResult::PageNotMapped,
+            Err(PageTableWalkError::NotMapped) => return TranslateResult::NotMapped,
             Err(PageTableWalkError::MappedToHugePage) => {
                 let entry = &p3[addr.p3_index()];
                 let frame = PhysFrame::containing_address(entry.addr());
                 let offset = addr.as_u64() & 0o_777_777_7777;
                 let flags = entry.flags();
-                return TranslateResult::Frame1GiB {
-                    frame,
+                return TranslateResult::Mapped {
+                    frame: MappedFrame::Size1GiB(frame),
                     offset,
                     flags,
                 };
             }
         };
         let p1 = match self.page_table_walker.next_table(&p2[addr.p2_index()]) {
             Ok(page_table) => page_table,
-            Err(PageTableWalkError::NotMapped) => return TranslateResult::PageNotMapped,
+            Err(PageTableWalkError::NotMapped) => return TranslateResult::NotMapped,
             Err(PageTableWalkError::MappedToHugePage) => {
                 let entry = &p2[addr.p2_index()];
                 let frame = PhysFrame::containing_address(entry.addr());
                 let offset = addr.as_u64() & 0o_777_7777;
                 let flags = entry.flags();
-                return TranslateResult::Frame2MiB {
-                    frame,
+                return TranslateResult::Mapped {
+                    frame: MappedFrame::Size2MiB(frame),
                     offset,
                     flags,
                 };
@@ -567,7 +567,7 @@ impl<'a, P: PhysToVirt> MapperAllSizes for MappedPageTable<'a, P> {
         let p1_entry = &p1[addr.p1_index()];
 
         if p1_entry.is_unused() {
-            return TranslateResult::PageNotMapped;
+            return TranslateResult::NotMapped;
         }
 
         let frame = match PhysFrame::from_start_address(p1_entry.addr()) {
@@ -576,8 +576,8 @@ impl<'a, P: PhysToVirt> MapperAllSizes for MappedPageTable<'a, P> {
         };
         let offset = u64::from(addr.page_offset());
         let flags = p1_entry.flags();
-        TranslateResult::Frame4KiB {
-            frame,
+        TranslateResult::Mapped {
+            frame: MappedFrame::Size4KiB(frame),
             offset,
             flags,
         }
diff --git a/src/structures/paging/mapper/mod.rs b/src/structures/paging/mapper/mod.rs
@@ -17,9 +17,13 @@ mod offset_page_table;
 #[cfg(feature = "instructions")]
 mod recursive_page_table;
 
-/// This trait defines page table operations that work for all page sizes of the x86_64
-/// architecture.
-pub trait MapperAllSizes: Mapper<Size4KiB> + Mapper<Size2MiB> + Mapper<Size1GiB> {
+/// An empty convencience trait that requires the `Mapper` trait for all page sizes.
+pub trait MapperAllSizes: Mapper<Size4KiB> + Mapper<Size2MiB> + Mapper<Size1GiB> {}
+
+impl<T> MapperAllSizes for T where T: Mapper<Size4KiB> + Mapper<Size2MiB> + Mapper<Size1GiB> {}
+
+/// Provides methods for translating virtual addresses.
+pub trait Translate {
     /// Return the frame that the given virtual address is mapped to and the offset within that
     /// frame.
     ///
@@ -34,63 +38,76 @@ pub trait MapperAllSizes: Mapper<Size4KiB> + Mapper<Size2MiB> + Mapper<Size1GiB>
     /// Returns `None` if there is no valid mapping for the given address.
     ///
     /// This is a convenience method. For more information about a mapping see the
-    /// [`translate`](MapperAllSizes::translate) method.
+    /// [`translate`](Translate::translate) method.
     #[inline]
     fn translate_addr(&self, addr: VirtAddr) -> Option<PhysAddr> {
         match self.translate(addr) {
-            TranslateResult::PageNotMapped | TranslateResult::InvalidFrameAddress(_) => None,
-            TranslateResult::Frame4KiB { frame, offset, .. } => {
-                Some(frame.start_address() + offset)
-            }
-            TranslateResult::Frame2MiB { frame, offset, .. } => {
-                Some(frame.start_address() + offset)
-            }
-            TranslateResult::Frame1GiB { frame, offset, .. } => {
-                Some(frame.start_address() + offset)
-            }
+            TranslateResult::NotMapped | TranslateResult::InvalidFrameAddress(_) => None,
+            TranslateResult::Mapped { frame, offset, .. } => Some(frame.start_address() + offset),
         }
     }
 }
 
-/// The return value of the [`MapperAllSizes::translate`] function.
+/// The return value of the [`Translate::translate`] function.
 ///
 /// If the given address has a valid mapping, a `Frame4KiB`, `Frame2MiB`, or `Frame1GiB` variant
 /// is returned, depending on the size of the mapped page. The remaining variants indicate errors.
 #[derive(Debug)]
 pub enum TranslateResult {
-    /// The page is mapped to a physical frame of size 4KiB.
-    Frame4KiB {
-        /// The mapped frame.
-        frame: PhysFrame<Size4KiB>,
-        /// The offset whithin the mapped frame.
-        offset: u64,
-        /// The flags for the frame.
-        flags: PageTableFlags,
-    },
-    /// The page is mapped to a physical frame of size 2MiB.
-    Frame2MiB {
-        /// The mapped frame.
-        frame: PhysFrame<Size2MiB>,
-        /// The offset whithin the mapped frame.
-        offset: u64,
-        /// The flags for the frame.
-        flags: PageTableFlags,
-    },
-    /// The page is mapped to a physical frame of size 2MiB.
-    Frame1GiB {
+    /// The virtual address is mapped to a physical frame.
+    Mapped {
         /// The mapped frame.
-        frame: PhysFrame<Size1GiB>,
+        frame: MappedFrame,
         /// The offset whithin the mapped frame.
         offset: u64,
-        /// The flags for the frame.
+        /// The entry flags in the lowest-level page table.
+        ///
+        /// Flags of higher-level page table entries are not included here, but they can still
+        /// affect the effective flags for an address, for example when the WRITABLE flag is not
+        /// set for a level 3 entry.
         flags: PageTableFlags,
     },
-    /// The given page is not mapped to a physical frame.
-    PageNotMapped,
-    /// The page table entry for the given page points to an invalid physical address.
+    /// The given virtual address is not mapped to a physical frame.
+    NotMapped,
+    /// The page table entry for the given virtual address points to an invalid physical address.
     InvalidFrameAddress(PhysAddr),
 }
 
+/// Represents a physical frame mapped in a page table.
+#[derive(Debug)]
+pub enum MappedFrame {
+    /// The virtual address is mapped to a 4KiB frame.
+    Size4KiB(PhysFrame<Size4KiB>),
+    /// The virtual address is mapped to a "large" 2MiB frame.
+    Size2MiB(PhysFrame<Size2MiB>),
+    /// The virtual address is mapped to a "huge" 1GiB frame.
+    Size1GiB(PhysFrame<Size1GiB>),
+}
+
+impl MappedFrame {
+    const_fn! {
+        /// Returns the start address of the frame.
+        pub fn start_address(&self) -> PhysAddr {
+            match self {
+                MappedFrame::Size4KiB(frame) => frame.start_address(),
+                MappedFrame::Size2MiB(frame) => frame.start_address(),
+                MappedFrame::Size1GiB(frame) => frame.start_address(),
+            }
+        }
+    }
+
+    const_fn! {
+        /// Returns the size the frame (4KB, 2MB or 1GB).
+        pub fn size(&self) -> u64 {
+            match self {
+                MappedFrame::Size4KiB(frame) => frame.size(),
+                MappedFrame::Size2MiB(frame) => frame.size(),
+                MappedFrame::Size1GiB(frame) => frame.size(),
+            }
+        }
+    }
+}
+
 /// A trait for common page table operations on pages of size `S`.
 pub trait Mapper<S: PageSize> {
     /// Creates a new mapping in the page table.
@@ -460,4 +477,4 @@ pub enum TranslateError {
     InvalidFrameAddress(PhysAddr),
 }
 
-static _ASSERT_OBJECT_SAFE: Option<&(dyn MapperAllSizes + Sync)> = None;
+static _ASSERT_OBJECT_SAFE: Option<&(dyn Translate + Sync)> = None;
diff --git a/src/structures/paging/mapper/offset_page_table.rs b/src/structures/paging/mapper/offset_page_table.rs
@@ -260,7 +260,7 @@ impl<'a> Mapper<Size4KiB> for OffsetPageTable<'a> {
     }
 }
 
-impl<'a> MapperAllSizes for OffsetPageTable<'a> {
+impl<'a> Translate for OffsetPageTable<'a> {
     #[inline]
     fn translate(&self, addr: VirtAddr) -> TranslateResult {
         self.inner.translate(addr)
diff --git a/src/structures/paging/mapper/recursive_page_table.rs b/src/structures/paging/mapper/recursive_page_table.rs
@@ -758,15 +758,15 @@ impl<'a> Mapper<Size4KiB> for RecursivePageTable<'a> {
     }
 }
 
-impl<'a> MapperAllSizes for RecursivePageTable<'a> {
+impl<'a> Translate for RecursivePageTable<'a> {
     #[allow(clippy::inconsistent_digit_grouping)]
     fn translate(&self, addr: VirtAddr) -> TranslateResult {
         let page = Page::containing_address(addr);
 
         let p4 = &self.p4;
         let p4_entry = &p4[addr.p4_index()];
         if p4_entry.is_unused() {
-            return TranslateResult::PageNotMapped;
+            return TranslateResult::NotMapped;
         }
         if p4_entry.flags().contains(PageTableFlags::HUGE_PAGE) {
             panic!("level 4 entry has huge page bit set")
@@ -775,15 +775,15 @@ impl<'a> MapperAllSizes for RecursivePageTable<'a> {
         let p3 = unsafe { &*(p3_ptr(page, self.recursive_index)) };
         let p3_entry = &p3[addr.p3_index()];
         if p3_entry.is_unused() {
-            return TranslateResult::PageNotMapped;
+            return TranslateResult::NotMapped;
         }
         if p3_entry.flags().contains(PageTableFlags::HUGE_PAGE) {
             let entry = &p3[addr.p3_index()];
             let frame = PhysFrame::containing_address(entry.addr());
             let offset = addr.as_u64() & 0o_777_777_7777;
             let flags = entry.flags();
-            return TranslateResult::Frame1GiB {
-                frame,
+            return TranslateResult::Mapped {
+                frame: MappedFrame::Size1GiB(frame),
                 offset,
                 flags,
             };
@@ -792,15 +792,15 @@ impl<'a> MapperAllSizes for RecursivePageTable<'a> {
         let p2 = unsafe { &*(p2_ptr(page, self.recursive_index)) };
         let p2_entry = &p2[addr.p2_index()];
         if p2_entry.is_unused() {
-            return TranslateResult::PageNotMapped;
+            return TranslateResult::NotMapped;
         }
         if p2_entry.flags().contains(PageTableFlags::HUGE_PAGE) {
             let entry = &p2[addr.p2_index()];
             let frame = PhysFrame::containing_address(entry.addr());
             let offset = addr.as_u64() & 0o_777_7777;
             let flags = entry.flags();
-            return TranslateResult::Frame2MiB {
-                frame,
+            return TranslateResult::Mapped {
+                frame: MappedFrame::Size2MiB(frame),
                 offset,
                 flags,
             };
@@ -809,7 +809,7 @@ impl<'a> MapperAllSizes for RecursivePageTable<'a> {
         let p1 = unsafe { &*(p1_ptr(page, self.recursive_index)) };
         let p1_entry = &p1[addr.p1_index()];
         if p1_entry.is_unused() {
-            return TranslateResult::PageNotMapped;
+            return TranslateResult::NotMapped;
         }
         if p1_entry.flags().contains(PageTableFlags::HUGE_PAGE) {
             panic!("level 1 entry has huge page bit set")
@@ -821,8 +821,8 @@ impl<'a> MapperAllSizes for RecursivePageTable<'a> {
         };
         let offset = u64::from(addr.page_offset());
         let flags = p1_entry.flags();
-        TranslateResult::Frame4KiB {
-            frame,
+        TranslateResult::Mapped {
+            frame: MappedFrame::Size4KiB(frame),
             offset,
             flags,
         }
diff --git a/src/structures/paging/mod.rs b/src/structures/paging/mod.rs
@@ -14,7 +14,7 @@ pub use self::mapper::OffsetPageTable;
 #[cfg(feature = "instructions")]
 #[doc(no_inline)]
 pub use self::mapper::RecursivePageTable;
-pub use self::mapper::{Mapper, MapperAllSizes};
+pub use self::mapper::{Mapper, Translate};
 pub use self::page::{Page, PageSize, Size1GiB, Size2MiB, Size4KiB};
 pub use self::page_table::{PageOffset, PageTable, PageTableFlags, PageTableIndex};
 

Original file line number	Diff line number	Diff line change
`@@ -260,7 +260,7 @@ impl<'a> Mapper<Size4KiB> for OffsetPageTable<'a> {`
`260`	`260`	`}`
`261`	`261`	`}`
`262`	`262`
`263`		`-impl<'a> MapperAllSizes for OffsetPageTable<'a> {`
	`263`	`+impl<'a> Translate for OffsetPageTable<'a> {`
`264`	`264`	`#[inline]`
`265`	`265`	`fn translate(&self, addr: VirtAddr) -> TranslateResult {`
`266`	`266`	`self.inner.translate(addr)`