Refactor frame/paging into a single module, make Frame implement Drop

src/devices/vbe.rs

@@ -2,7 +2,7 @@
 
 use core::slice;
 use utils;
-use i386::paging::{self, EntryFlags, PageTablesSet};
+use i386::mem::paging::{self, EntryFlags, PageTablesSet};
 use frame_alloc::PhysicalAddress;
 use multiboot2::{BootInformation, FramebufferInfoTag};
 

src/frame_alloc.rs → src/i386/mem/frame_alloc.rs

@@ -17,34 +17,7 @@ use bit_field::BitArray;
 use utils::BitArrayExt;
 use utils::bit_array_first_one;
 use paging::PAGE_SIZE;
-
-/// Represents a Physical address
-///
-/// Should only be used when paging is off
-#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)]
-pub struct PhysicalAddress(pub usize);
-/// Represents a Virtual address
-#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)]
-pub struct VirtualAddress(pub usize);
-
-impl VirtualAddress  { pub fn addr(&self) -> usize { self.0 } }
-impl PhysicalAddress { pub fn addr(&self) -> usize { self.0 } }
-
-impl ::core::ops::Add<usize> for VirtualAddress {
-    type Output = VirtualAddress;
-
-    fn add(self, other: usize) -> VirtualAddress {
-        VirtualAddress(self.0 + other)
-    }
-}
-
-impl ::core::ops::Add<usize> for PhysicalAddress {
-    type Output = PhysicalAddress;
-
-    fn add(self, other: usize) -> PhysicalAddress {
-        PhysicalAddress(self.0 + other)
-    }
-}
+pub use super::PhysicalAddress;
 
 /// A memory frame is the same size as a page
 pub const MEMORY_FRAME_SIZE: usize = PAGE_SIZE;
@@ -65,11 +38,8 @@ fn addr_to_frame(addr: usize) -> usize {
 
 /// Gets the physical address from a frame number
 #[inline]
-unsafe fn frame_to_addr(frame: usize) -> Frame {
-    let addr = frame << FRAME_BASE_LOG;
-    Frame {
-        physical_addr: addr,
-    }
+fn frame_to_addr(frame: usize) -> usize {
+    frame << FRAME_BASE_LOG
 }
 
 /// Rounds an address to its page address
@@ -110,27 +80,60 @@ static FRAMES_BITMAP: Mutex<AllocatorBitmap> = Mutex::new(AllocatorBitmap {
 /// A frame is 4ko in size
 ///
 /// Should only be used when paging is off
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug)]
 pub struct Frame {
     physical_addr: usize,
+    is_allocated: bool
+}
+
+impl Drop for Frame {
+    fn drop(&mut self) {
+        if self.is_allocated {
+            FrameAllocator::free_frame(self);
+        }
+    }
 }
 
 impl Frame {
     /// Get the physical address of this Frame
     pub fn address(&self) -> PhysicalAddress { PhysicalAddress(self.physical_addr) }
 
+    // TODO: this should be a private implementation detail of the paging stuff.
+    /// Gets the current allocation state
+    pub(super) fn is_allocated(&self) -> bool {
+        self.is_allocated
+    }
+
     /// Constructs a frame structure from a physical address
     ///
     /// This does not guaranty that the frame can be written to, or even exists at all
     ///
     /// # Panic
     ///
     /// Panics when the address is not framesize-aligned
-    ///
+    // TODO: This should almost certainly be unsafe.
     pub fn from_physical_addr(physical_addr: PhysicalAddress) -> Frame {
         assert_eq!(physical_addr.addr() % MEMORY_FRAME_SIZE, 0,
                    "Frame must be constructed from a framesize-aligned pointer");
-        Frame { physical_addr: physical_addr.addr() }
+        // TODO: Check that it falls in a Reserved zone ?
+        Frame { physical_addr: physical_addr.addr(), is_allocated: false }
+    }
+
+    /// Constructs a frame structure from a physical address
+    ///
+    /// This does not guaranty that the frame can be written to, or even exists at all
+    ///
+    /// # Panic
+    ///
+    /// Panics when the address is not framesize-aligned
+    ///
+    /// # Safety
+    ///
+    /// This function should only be used on physical_addr that are reserved -
+    /// that is, addresses that the frame allocator will never give. Otherwise,
+    /// it is unsound.
+    pub unsafe fn from_allocated_addr(physical_addr: PhysicalAddress) -> Frame {
+        Frame { physical_addr: physical_addr.addr(), is_allocated: true }
     }
 }
 
@@ -260,8 +263,9 @@ impl FrameAllocator {
         let frame = bit_array_first_one(&frames_bitmap.memory_bitmap)
             .expect("Cannot allocate frame: No available frame D:");
         frames_bitmap.memory_bitmap.set_bit(frame, FRAME_OCCUPIED);
-        unsafe {
-            frame_to_addr(frame)
+        Frame {
+            physical_addr: frame_to_addr(frame),
+            is_allocated: true
         }
     }
 
@@ -270,7 +274,7 @@ impl FrameAllocator {
     /// # Panic
     ///
     /// Panics if the frame was not allocated
-    pub fn free_frame(frame: Frame) {
+    fn free_frame(frame: &Frame) {
         let mut frames_bitmap = FRAMES_BITMAP.lock();
         FrameAllocator::check_initialized(&*frames_bitmap);
 

src/i386/mem/mod.rs

@@ -0,0 +1,59 @@
+//! Memory handling module. Contains both a physical memory allocator, and the
+//! paging submodule.
+
+pub mod paging;
+pub mod frame_alloc;
+
+use core::fmt::{Formatter, Error, Display, Debug};
+
+/// Represents a Physical address
+///
+/// Should only be used when paging is off
+#[derive(Copy, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)]
+pub struct PhysicalAddress(pub usize);
+/// Represents a Virtual address
+#[derive(Copy, Clone, Hash, PartialEq, Eq, PartialOrd, Ord)]
+pub struct VirtualAddress(pub usize);
+
+impl VirtualAddress  { pub fn addr(&self) -> usize { self.0 } }
+impl PhysicalAddress { pub fn addr(&self) -> usize { self.0 } }
+
+impl ::core::ops::Add<usize> for VirtualAddress {
+    type Output = VirtualAddress;
+
+    fn add(self, other: usize) -> VirtualAddress {
+        VirtualAddress(self.0 + other)
+    }
+}
+
+impl ::core::ops::Add<usize> for PhysicalAddress {
+    type Output = PhysicalAddress;
+
+    fn add(self, other: usize) -> PhysicalAddress {
+        PhysicalAddress(self.0 + other)
+    }
+}
+
+impl Debug for PhysicalAddress {
+    fn fmt(&self, f: &mut Formatter) -> Result<(), Error> {
+        write!(f, "P {:#010x}", self.0)
+    }
+}
+
+impl Display for PhysicalAddress {
+    fn fmt(&self, f: &mut Formatter) -> Result<(), Error> {
+        write!(f, "P {:#010x}", self.0)
+    }
+}
+
+impl Debug for VirtualAddress {
+    fn fmt(&self, f: &mut Formatter) -> Result<(), Error> {
+        write!(f, "V {:#010x}", self.0)
+    }
+}
+
+impl Display for VirtualAddress {
+    fn fmt(&self, f: &mut Formatter) -> Result<(), Error> {
+        write!(f, "V {:#010x}", self.0)
+    }
+}

src/i386/paging/entry.rs → src/i386/mem/paging/entry.rs

@@ -1,6 +1,8 @@
 //! i386 page table entry
 
-use ::frame_alloc::{Frame, PhysicalAddress};
+use i386::mem::frame_alloc::Frame;
+use i386::mem::PhysicalAddress;
+use core::fmt::{Debug, Formatter, Error};
 
 bitflags! {
     /// The flags of a table entry
@@ -15,7 +17,7 @@ bitflags! {
         const HUGE_PAGE =       1 << 7;
         const GLOBAL =          1 << 8;
         const GUARD_PAGE =      1 << 9;     // user_defined_1
-        const USER_DEFINED_2 =  1 << 10;    // user_defined_2
+        const IS_FRAME_ALLOC =  1 << 10;    // user_defined_2
         const USER_DEFINED_3 =  1 << 11;    // user_defined_3
     }
 }
@@ -24,27 +26,46 @@ const ENTRY_PHYS_ADDRESS_MASK: usize = 0xffff_f000;
 
 /// An entry in a page table or page directory. An unused entry is 0
 #[repr(transparent)]
-#[derive(Debug, Clone, Copy)]
+#[derive(Clone, Copy)]
 pub struct Entry(u32);
 
+impl Debug for Entry {
+    fn fmt(&self, f: &mut Formatter) -> Result<(), Error> {
+        f.debug_struct("Entry")
+            .field("flags", &self.flags())
+            .field("frame", &self.pointed_frame().as_option())
+            .finish()
+    }
+}
+
 impl Entry {
     /// Is the entry unused ?
     pub fn is_unused(&self) -> bool { self.0 == 0 }
 
     /// Clear the entry
-    pub fn set_unused(&mut self) { self.0 = 0; }
+    pub fn set_unused(&mut self) -> PageState<Frame> {
+        let ret = self.pointed_frame().map(|addr| {
+            if self.flags().contains(EntryFlags::IS_FRAME_ALLOC) {
+                unsafe { Frame::from_allocated_addr(addr) }
+            } else {
+                Frame::from_physical_addr(addr)
+            }
+        });
+        self.0 = 0;
+        ret
+    }
 
     /// Is the entry a page guard ?
     pub fn is_guard(&self) -> bool { self.flags().contains(EntryFlags::GUARD_PAGE) }
 
     /// Get the current entry flags
     pub fn flags(&self) -> EntryFlags { EntryFlags::from_bits_truncate(self.0) }
 
-    /// Get the associated frame, if available
-    pub fn pointed_frame(&self) -> PageState<Frame> {
+    /// Get the associated physical address, if available
+    pub fn pointed_frame(&self) -> PageState<PhysicalAddress> {
         if self.flags().contains(EntryFlags::PRESENT) {
             let frame_phys_addr = self.0 as usize & ENTRY_PHYS_ADDRESS_MASK;
-            PageState::Present( Frame::from_physical_addr(PhysicalAddress(frame_phys_addr)) )
+            PageState::Present(PhysicalAddress(frame_phys_addr))
         } else if self.flags().contains(EntryFlags::GUARD_PAGE) {
             PageState::Guarded
         } else {
@@ -53,13 +74,25 @@ impl Entry {
     }
 
     /// Sets the entry
-    pub fn set(&mut self, frame: Frame, flags: EntryFlags) {
+    pub fn set(&mut self, frame: Frame, mut flags: EntryFlags) {
         assert_eq!(flags.contains(EntryFlags::PRESENT)
                 && flags.contains(EntryFlags::GUARD_PAGE), false,
                 "a GUARD_PAGE cannot also be PRESENT");
+        assert!(!flags.contains(EntryFlags::IS_FRAME_ALLOC),
+                "IS_FRAME_ALLOC is handled internally");
         let frame_phys_addr = frame.address();
         assert_eq!(frame_phys_addr.addr() & !ENTRY_PHYS_ADDRESS_MASK, 0);
+
+        // Make sure we stay consistent.
+        if frame.is_allocated() {
+            flags.insert(EntryFlags::IS_FRAME_ALLOC);
+        } else {
+            flags.remove(EntryFlags::IS_FRAME_ALLOC);
+        }
+
         self.0 = (frame_phys_addr.addr() as u32) | flags.bits();
+
+        ::core::mem::forget(frame);
     }
 
     /// Make this entry a page guard
@@ -106,4 +139,14 @@ impl<T> PageState<T> {
             PageState::Available => PageState::Available
         }
     }
+
+    /// Turns the PageState into an Option, setting both Guarded and Available
+    /// state to None, and Present(t) state to Some(t).
+    pub fn as_option(&self) -> Option<&T> {
+        match *self {
+            PageState::Present(ref t) => Some(t),
+            PageState::Guarded => None,
+            PageState::Available => None,
+        }
+    }
 }

src/i386/paging/mod.rs → src/i386/mem/paging/mod.rs

@@ -11,10 +11,11 @@ pub use self::table::entry::EntryFlags;
 
 use self::table::*;
 use self::table::entry::Entry;
-pub use frame_alloc::{round_to_page, round_to_page_upper};
+pub use i386::mem::frame_alloc::{round_to_page, round_to_page_upper};
 use spin::Mutex;
-use frame_alloc::{Frame, PhysicalAddress};
-pub use frame_alloc::VirtualAddress;
+use i386::mem::frame_alloc::Frame;
+use i386::mem::PhysicalAddress;
+pub use i386::mem::VirtualAddress;
 use ::devices::vgatext::{VGA_SCREEN_ADDRESS, VGA_SCREEN_MEMORY_SIZE};
 use ::core::fmt::Write;
 use ::core::ops::Deref;
@@ -94,6 +95,7 @@ pub unsafe fn map_kernel(boot_info : &BootInformation) -> PagingOffPageSet {
         if !section.is_allocated() || section.name() == ".boot" {
             continue; // section is not loaded to memory
         }
+
         info!("section {:#010x} - {:#010x} : {}",
                  section.start_address(), section.end_address(),
                  section.name()
@@ -110,6 +112,8 @@ pub unsafe fn map_kernel(boot_info : &BootInformation) -> PagingOffPageSet {
                                       map_flags);
     }
 
+
+
     // Map the vga screen memory
     new_pages.identity_map_region(VGA_SCREEN_ADDRESS, VGA_SCREEN_MEMORY_SIZE,
                                   EntryFlags::PRESENT | EntryFlags::WRITABLE);