diff --git a/ostd/libs/linux-bzimage/setup/src/x86/amd64_efi/paging.rs b/ostd/libs/linux-bzimage/setup/src/x86/amd64_efi/paging.rs index 2e55eab2c..4a869af59 100644 --- a/ostd/libs/linux-bzimage/setup/src/x86/amd64_efi/paging.rs +++ b/ostd/libs/linux-bzimage/setup/src/x86/amd64_efi/paging.rs @@ -161,7 +161,9 @@ impl PageTableCreator { } let pt = unsafe { &mut *(pde.paddr() as *mut Ia32eTable) }; let pte = pt.index(1, from.addr()); - pte.update(to.addr(), flags); + // In level-1 PTE, the HUGE bit is the PAT bit (page attribute table). + // We use it as the "valid" bit for the page table entry. + pte.update(to.addr(), flags | Ia32eFlags::HUGE); } pub fn nr_frames_used(&self) -> usize { diff --git a/ostd/src/arch/x86/mm/mod.rs b/ostd/src/arch/x86/mm/mod.rs index 74a29a790..4c0cc2a0a 100644 --- a/ostd/src/arch/x86/mm/mod.rs +++ b/ostd/src/arch/x86/mm/mod.rs @@ -53,7 +53,10 @@ bitflags::bitflags! { const ACCESSED = 1 << 5; /// Whether the memory area represented by this entry is modified. const DIRTY = 1 << 6; - /// Only in the non-starting and non-ending levels, indication of huge page. + /// In level 2 or 3 it indicates that it map to a huge page. + /// In level 1, it is the PAT (page attribute table) bit. + /// We use this bit in level 1, 2 and 3 to indicate that this entry is + /// "valid". For levels above 3, `PRESENT` is used for "valid". const HUGE = 1 << 7; /// Indicates that the mapping is present in all address spaces, so it isn't flushed from /// the TLB on an address space switch. @@ -156,14 +159,15 @@ macro_rules! parse_flags { impl PageTableEntryTrait for PageTableEntry { fn is_present(&self) -> bool { - self.0 & PageTableFlags::PRESENT.bits() != 0 + // For PT child, `PRESENT` should be set; for huge page, `HUGE` should + // be set; for the leaf child page, `PAT`, which is the same bit as + // the `HUGE` bit in upper levels, should be set. + self.0 & PageTableFlags::PRESENT.bits() != 0 || self.0 & PageTableFlags::HUGE.bits() != 0 } - fn new_page(paddr: Paddr, level: PagingLevel, prop: PageProperty) -> Self { - let mut pte = Self( - paddr & Self::PHYS_ADDR_MASK - | ((level != 1) as usize) << PageTableFlags::HUGE.bits().ilog2(), - ); + fn new_page(paddr: Paddr, _level: PagingLevel, prop: PageProperty) -> Self { + let flags = PageTableFlags::HUGE.bits(); + let mut pte = Self(paddr & Self::PHYS_ADDR_MASK | flags); pte.set_prop(prop); pte } @@ -209,8 +213,12 @@ impl PageTableEntryTrait for PageTableEntry { } fn set_prop(&mut self, prop: PageProperty) { - let mut flags = PageTableFlags::PRESENT.bits(); - flags |= parse_flags!(prop.flags.bits(), PageFlags::W, PageTableFlags::WRITABLE) + if !self.is_present() { + return; + } + let mut flags = PageTableFlags::empty().bits(); + flags |= parse_flags!(prop.flags.bits(), PageFlags::R, PageTableFlags::PRESENT) + | parse_flags!(prop.flags.bits(), PageFlags::W, PageTableFlags::WRITABLE) | parse_flags!(!prop.flags.bits(), PageFlags::X, PageTableFlags::NO_EXECUTE) | parse_flags!( prop.flags.bits(), @@ -249,8 +257,8 @@ impl PageTableEntryTrait for PageTableEntry { self.0 = self.0 & !Self::PROP_MASK | flags; } - fn is_last(&self, level: PagingLevel) -> bool { - level == 1 || (self.0 & PageTableFlags::HUGE.bits() != 0) + fn is_last(&self, _level: PagingLevel) -> bool { + self.0 & PageTableFlags::HUGE.bits() != 0 } } diff --git a/ostd/src/mm/page_table/mod.rs b/ostd/src/mm/page_table/mod.rs index 2aec94160..df4efdd95 100644 --- a/ostd/src/mm/page_table/mod.rs +++ b/ostd/src/mm/page_table/mod.rs @@ -327,6 +327,10 @@ pub trait PageTableEntryTrait: Clone + Copy + Debug + Default + Pod + Sized + Sy } /// If the flags are present with valid mappings. + /// + /// For PTEs created by [`Self::new_absent`], this method should return + /// false. And for PTEs created by [`Self::new_page`] or [`Self::new_pt`] + /// and modified with [`Self::set_prop`] this method should return true. fn is_present(&self) -> bool; /// Create a new PTE with the given physical address and flags that map to a page. @@ -343,6 +347,10 @@ pub trait PageTableEntryTrait: Clone + Copy + Debug + Default + Pod + Sized + Sy fn prop(&self) -> PageProperty; + /// Set the page property of the PTE. + /// + /// This will be only done if the PTE is present. If not, this method will + /// do nothing. fn set_prop(&mut self, prop: PageProperty); /// If the PTE maps a page rather than a child page table. diff --git a/ostd/src/mm/page_table/node/entry.rs b/ostd/src/mm/page_table/node/entry.rs index e81beb53c..b96a6c1c7 100644 --- a/ostd/src/mm/page_table/node/entry.rs +++ b/ostd/src/mm/page_table/node/entry.rs @@ -57,9 +57,6 @@ where /// Operates on the mapping properties of the entry. /// /// It only modifies the properties if the entry is present. - // FIXME: in x86_64, you can protect a page with neither of the RWX - // permissions. This would make the page not accessible and leaked. Such a - // behavior is memory-safe but wrong. In RISC-V there's no problem. pub(in crate::mm) fn protect(&mut self, op: &mut impl FnMut(&mut PageProperty)) { if !self.pte.is_present() { return;