From 274e3dc082d5d6d1c9784f205f4b55ddd1b71efa Mon Sep 17 00:00:00 2001 From: Philipp Oppermann Date: Mon, 28 Dec 2020 12:09:15 +0100 Subject: [PATCH 1/4] Restructure the `TranslateResult` type --- .../paging/mapper/mapped_page_table.rs | 22 ++--- src/structures/paging/mapper/mod.rs | 81 ++++++++++--------- .../paging/mapper/offset_page_table.rs | 2 +- .../paging/mapper/recursive_page_table.rs | 22 ++--- src/structures/paging/mod.rs | 2 +- 5 files changed, 66 insertions(+), 63 deletions(-) diff --git a/src/structures/paging/mapper/mapped_page_table.rs b/src/structures/paging/mapper/mapped_page_table.rs index c415df3b7..ebdf491db 100644 --- a/src/structures/paging/mapper/mapped_page_table.rs +++ b/src/structures/paging/mapper/mapped_page_table.rs @@ -522,27 +522,27 @@ impl<'a, P: PhysToVirt> Mapper 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, }; @@ -550,14 +550,14 @@ impl<'a, P: PhysToVirt> MapperAllSizes for MappedPageTable<'a, P> { }; 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 index 8bbab95cf..6fe1db35b 100644 --- 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 + Mapper + Mapper { +/// An empty convencience trait that requires the `Mapper` trait for all page sizes. +pub trait MapperAllSizes: Mapper + Mapper + Mapper {} + +impl MapperAllSizes for T where T: Mapper + Mapper + Mapper {} + +/// 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. /// @@ -38,16 +42,8 @@ pub trait MapperAllSizes: Mapper + Mapper + Mapper #[inline] fn translate_addr(&self, addr: VirtAddr) -> Option { 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), } } } @@ -58,39 +54,46 @@ pub trait MapperAllSizes: Mapper + Mapper + Mapper /// 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, - /// 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, - /// 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, + 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), + /// The virtual address is mapped to a "large" 2MiB frame. + Size2MiB(PhysFrame), + /// The virtual address is mapped to a "huge" 1GiB frame. + Size1GiB(PhysFrame), +} + +impl MappedFrame { + 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(), + } + } +} + /// A trait for common page table operations on pages of size `S`. pub trait Mapper { /// Creates a new mapping in the page table. @@ -460,4 +463,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 index 022b7646d..bafc82642 100644 --- 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 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 index 3f2afb529..b66aec176 100644 --- a/src/structures/paging/mapper/recursive_page_table.rs +++ b/src/structures/paging/mapper/recursive_page_table.rs @@ -758,7 +758,7 @@ impl<'a> Mapper 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); @@ -766,7 +766,7 @@ impl<'a> MapperAllSizes for RecursivePageTable<'a> { 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 index a2dd1322d..25fe8847f 100644 --- 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}; From 663a2ebaf5672d5d5b9e152e8f72764037a075e3 Mon Sep 17 00:00:00 2001 From: Philipp Oppermann Date: Mon, 28 Dec 2020 12:24:57 +0100 Subject: [PATCH 2/4] Update changelog for #211 --- Changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/Changelog.md b/Changelog.md index 22e729043..ecc3141ee 100644 --- 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)) From 5c795ad064eee22556c60375c3dc43068483992a Mon Sep 17 00:00:00 2001 From: Philipp Oppermann Date: Mon, 28 Dec 2020 12:25:07 +0100 Subject: [PATCH 3/4] Fix doc links --- src/structures/paging/mapper/mod.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/structures/paging/mapper/mod.rs b/src/structures/paging/mapper/mod.rs index 6fe1db35b..eee84af34 100644 --- a/src/structures/paging/mapper/mod.rs +++ b/src/structures/paging/mapper/mod.rs @@ -38,7 +38,7 @@ pub trait Translate { /// 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 { match self.translate(addr) { @@ -48,7 +48,7 @@ pub trait Translate { } } -/// 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. From ec87383468664bae8ff1f843d5bdf727beaf9a77 Mon Sep 17 00:00:00 2001 From: Philipp Oppermann Date: Mon, 28 Dec 2020 12:28:47 +0100 Subject: [PATCH 4/4] Make `MappedFrame::start_address` public and const add add `size` method --- src/structures/paging/mapper/mod.rs | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/src/structures/paging/mapper/mod.rs b/src/structures/paging/mapper/mod.rs index eee84af34..d70ae8277 100644 --- a/src/structures/paging/mapper/mod.rs +++ b/src/structures/paging/mapper/mod.rs @@ -85,11 +85,25 @@ pub enum MappedFrame { } impl MappedFrame { - 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 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(), + } } } }