From 8a257cc78094c4f1d7fbaaa8c1c7ba11aa62bafe Mon Sep 17 00:00:00 2001 From: Philipp Oppermann Date: Fri, 6 Mar 2020 13:21:30 +0100 Subject: [PATCH] Revert "Add new UnsafePhysFrame type and use it in Mapper::map_to" This reverts commit 0c8756feb347044654c8da27de2888f7d1f7b879. --- src/structures/paging/frame_alloc.rs | 42 ++----------------- .../paging/mapper/mapped_page_table.rs | 25 +++++------ src/structures/paging/mapper/mod.rs | 17 ++++---- .../paging/mapper/offset_page_table.rs | 12 +++--- .../paging/mapper/recursive_page_table.rs | 22 +++++----- 5 files changed, 41 insertions(+), 77 deletions(-) diff --git a/src/structures/paging/frame_alloc.rs b/src/structures/paging/frame_alloc.rs index 26efca0d1..6e80f15fe 100644 --- a/src/structures/paging/frame_alloc.rs +++ b/src/structures/paging/frame_alloc.rs @@ -1,7 +1,6 @@ //! Traits for abstracting away frame allocation and deallocation. -use crate::structures::paging::{PageSize, PhysFrame, Size4KiB}; -use core::ops::{Deref, DerefMut}; +use crate::structures::paging::{PageSize, PhysFrame}; /// A trait for types that can allocate a frame of memory. /// @@ -9,46 +8,11 @@ use core::ops::{Deref, DerefMut}; /// the `allocate_frame` method returns only unique unused frames. pub unsafe trait FrameAllocator { /// Allocate a frame of the appropriate size and return it if possible. - fn allocate_frame(&mut self) -> Option>; + fn allocate_frame(&mut self) -> Option>; } /// A trait for types that can deallocate a frame of memory. pub trait FrameDeallocator { /// Deallocate the given frame of memory. - fn deallocate_frame(&mut self, frame: UnusedPhysFrame); -} - -/// Represents a physical frame that is not used for any mapping. -#[derive(Debug)] -pub struct UnusedPhysFrame(PhysFrame); - -impl UnusedPhysFrame { - /// Creates a new UnusedPhysFrame from the given frame. - /// - /// ## Safety - /// - /// This method is unsafe because the caller must guarantee - /// that the given frame is unused. - pub unsafe fn new(frame: PhysFrame) -> Self { - Self(frame) - } - - /// Returns the physical frame as `PhysFrame` type. - pub fn frame(self) -> PhysFrame { - self.0 - } -} - -impl Deref for UnusedPhysFrame { - type Target = PhysFrame; - - fn deref(&self) -> &Self::Target { - &self.0 - } -} - -impl DerefMut for UnusedPhysFrame { - fn deref_mut(&mut self) -> &mut Self::Target { - &mut self.0 - } + fn deallocate_frame(&mut self, frame: PhysFrame); } diff --git a/src/structures/paging/mapper/mapped_page_table.rs b/src/structures/paging/mapper/mapped_page_table.rs index b5a4711e0..4e706fa83 100644 --- a/src/structures/paging/mapper/mapped_page_table.rs +++ b/src/structures/paging/mapper/mapped_page_table.rs @@ -41,7 +41,7 @@ impl<'a, P: PhysToVirt> MappedPageTable<'a, P> { fn map_to_1gib( &mut self, page: Page, - frame: UnusedPhysFrame, + frame: PhysFrame, flags: PageTableFlags, allocator: &mut A, ) -> Result, MapToError> @@ -66,7 +66,7 @@ impl<'a, P: PhysToVirt> MappedPageTable<'a, P> { fn map_to_2mib( &mut self, page: Page, - frame: UnusedPhysFrame, + frame: PhysFrame, flags: PageTableFlags, allocator: &mut A, ) -> Result, MapToError> @@ -94,7 +94,7 @@ impl<'a, P: PhysToVirt> MappedPageTable<'a, P> { fn map_to_4kib( &mut self, page: Page, - frame: UnusedPhysFrame, + frame: PhysFrame, flags: PageTableFlags, allocator: &mut A, ) -> Result, MapToError> @@ -115,17 +115,17 @@ impl<'a, P: PhysToVirt> MappedPageTable<'a, P> { if !p1[page.p1_index()].is_unused() { return Err(MapToError::PageAlreadyMapped(frame)); } - p1[page.p1_index()].set_frame(frame.frame(), flags); + p1[page.p1_index()].set_frame(frame, flags); Ok(MapperFlush::new(page)) } } impl<'a, P: PhysToVirt> Mapper for MappedPageTable<'a, P> { - fn map_to( + unsafe fn map_to( &mut self, page: Page, - frame: UnusedPhysFrame, + frame: PhysFrame, flags: PageTableFlags, allocator: &mut A, ) -> Result, MapToError> @@ -195,10 +195,10 @@ impl<'a, P: PhysToVirt> Mapper for MappedPageTable<'a, P> { } impl<'a, P: PhysToVirt> Mapper for MappedPageTable<'a, P> { - fn map_to( + unsafe fn map_to( &mut self, page: Page, - frame: UnusedPhysFrame, + frame: PhysFrame, flags: PageTableFlags, allocator: &mut A, ) -> Result, MapToError> @@ -276,10 +276,10 @@ impl<'a, P: PhysToVirt> Mapper for MappedPageTable<'a, P> { } impl<'a, P: PhysToVirt> Mapper for MappedPageTable<'a, P> { - fn map_to( + unsafe fn map_to( &mut self, page: Page, - frame: UnusedPhysFrame, + frame: PhysFrame, flags: PageTableFlags, allocator: &mut A, ) -> Result, MapToError> @@ -465,10 +465,7 @@ impl PageTableWalker

{ if entry.is_unused() { if let Some(frame) = allocator.allocate_frame() { - entry.set_frame( - frame.frame(), - PageTableFlags::PRESENT | PageTableFlags::WRITABLE, - ); + entry.set_frame(frame, PageTableFlags::PRESENT | PageTableFlags::WRITABLE); created = true; } else { return Err(PageTableCreateError::FrameAllocationFailed); diff --git a/src/structures/paging/mapper/mod.rs b/src/structures/paging/mapper/mod.rs index 96b95a90c..80eb3c98f 100644 --- a/src/structures/paging/mapper/mod.rs +++ b/src/structures/paging/mapper/mod.rs @@ -5,9 +5,8 @@ pub use self::mapped_page_table::{MappedPageTable, PhysToVirt}; pub use self::{offset_page_table::OffsetPageTable, recursive_page_table::RecursivePageTable}; use crate::structures::paging::{ - frame_alloc::{FrameAllocator, UnusedPhysFrame}, - page_table::PageTableFlags, - Page, PageSize, PhysFrame, Size1GiB, Size2MiB, Size4KiB, + frame_alloc::FrameAllocator, page_table::PageTableFlags, Page, PageSize, PhysFrame, Size1GiB, + Size2MiB, Size4KiB, }; use crate::{PhysAddr, VirtAddr}; @@ -83,10 +82,13 @@ pub trait Mapper { /// /// This function might need additional physical frames to create new page tables. These /// frames are allocated from the `allocator` argument. At most three frames are required. - fn map_to( + /// + /// This function is unsafe because the caller must guarantee that passed `frame` is + /// unused, i.e. not used for any other mappings. + unsafe fn map_to( &mut self, page: Page, - frame: UnusedPhysFrame, + frame: PhysFrame, flags: PageTableFlags, frame_allocator: &mut A, ) -> Result, MapToError> @@ -116,11 +118,12 @@ pub trait Mapper { /// /// ## Safety /// - /// TODO: Should this function be safe? + /// This function is unsafe because the caller must guarantee that the passed `frame` is + /// unused, i.e. not used for any other mappings. #[inline] unsafe fn identity_map( &mut self, - frame: UnusedPhysFrame, + frame: PhysFrame, flags: PageTableFlags, frame_allocator: &mut A, ) -> Result, MapToError> diff --git a/src/structures/paging/mapper/offset_page_table.rs b/src/structures/paging/mapper/offset_page_table.rs index 76679113e..a646c93c1 100644 --- a/src/structures/paging/mapper/offset_page_table.rs +++ b/src/structures/paging/mapper/offset_page_table.rs @@ -53,10 +53,10 @@ impl PhysToVirt for PhysOffset { // delegate all trait implementations to inner impl<'a> Mapper for OffsetPageTable<'a> { - fn map_to( + unsafe fn map_to( &mut self, page: Page, - frame: UnusedPhysFrame, + frame: PhysFrame, flags: PageTableFlags, allocator: &mut A, ) -> Result, MapToError> @@ -91,10 +91,10 @@ impl<'a> Mapper for OffsetPageTable<'a> { impl<'a> Mapper for OffsetPageTable<'a> { #[inline] - fn map_to( + unsafe fn map_to( &mut self, page: Page, - frame: UnusedPhysFrame, + frame: PhysFrame, flags: PageTableFlags, allocator: &mut A, ) -> Result, MapToError> @@ -129,10 +129,10 @@ impl<'a> Mapper for OffsetPageTable<'a> { impl<'a> Mapper for OffsetPageTable<'a> { #[inline] - fn map_to( + unsafe fn map_to( &mut self, page: Page, - frame: UnusedPhysFrame, + frame: PhysFrame, flags: PageTableFlags, allocator: &mut A, ) -> Result, MapToError> diff --git a/src/structures/paging/mapper/recursive_page_table.rs b/src/structures/paging/mapper/recursive_page_table.rs index daeafdb0e..7c1c7f37f 100644 --- a/src/structures/paging/mapper/recursive_page_table.rs +++ b/src/structures/paging/mapper/recursive_page_table.rs @@ -113,7 +113,7 @@ impl<'a> RecursivePageTable<'a> { if entry.is_unused() { if let Some(frame) = allocator.allocate_frame() { - entry.set_frame(frame.frame(), Flags::PRESENT | Flags::WRITABLE); + entry.set_frame(frame, Flags::PRESENT | Flags::WRITABLE); created = true; } else { return Err(MapToError::FrameAllocationFailed); @@ -141,7 +141,7 @@ impl<'a> RecursivePageTable<'a> { fn map_to_1gib( &mut self, page: Page, - frame: UnusedPhysFrame, + frame: PhysFrame, flags: PageTableFlags, allocator: &mut A, ) -> Result, MapToError> @@ -167,7 +167,7 @@ impl<'a> RecursivePageTable<'a> { fn map_to_2mib( &mut self, page: Page, - frame: UnusedPhysFrame, + frame: PhysFrame, flags: PageTableFlags, allocator: &mut A, ) -> Result, MapToError> @@ -196,7 +196,7 @@ impl<'a> RecursivePageTable<'a> { fn map_to_4kib( &mut self, page: Page, - frame: UnusedPhysFrame, + frame: PhysFrame, flags: PageTableFlags, allocator: &mut A, ) -> Result, MapToError> @@ -217,17 +217,17 @@ impl<'a> RecursivePageTable<'a> { if !p1[page.p1_index()].is_unused() { return Err(MapToError::PageAlreadyMapped(frame)); } - p1[page.p1_index()].set_frame(frame.frame(), flags); + p1[page.p1_index()].set_frame(frame, flags); Ok(MapperFlush::new(page)) } } impl<'a> Mapper for RecursivePageTable<'a> { - fn map_to( + unsafe fn map_to( &mut self, page: Page, - frame: UnusedPhysFrame, + frame: PhysFrame, flags: PageTableFlags, allocator: &mut A, ) -> Result, MapToError> @@ -310,10 +310,10 @@ impl<'a> Mapper for RecursivePageTable<'a> { impl<'a> Mapper for RecursivePageTable<'a> { #[inline] - fn map_to( + unsafe fn map_to( &mut self, page: Page, - frame: UnusedPhysFrame, + frame: PhysFrame, flags: PageTableFlags, allocator: &mut A, ) -> Result, MapToError> @@ -415,10 +415,10 @@ impl<'a> Mapper for RecursivePageTable<'a> { } impl<'a> Mapper for RecursivePageTable<'a> { - fn map_to( + unsafe fn map_to( &mut self, page: Page, - frame: UnusedPhysFrame, + frame: PhysFrame, flags: PageTableFlags, allocator: &mut A, ) -> Result, MapToError>