From 68da950d81cbd4357238723cfc449ac74a856a86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20L=C3=B3pez?= Date: Fri, 27 Oct 2023 12:42:15 +0200 Subject: [PATCH 1/4] mm/pagetable: refactor entry flags as PTEntryFlags constructors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Construct different predefined sets of page table entry flags from the PTEntryFlags type instead of the PageTable type. Signed-off-by: Carlos López --- src/cpu/percpu.rs | 4 +-- src/mm/pagetable.rs | 63 +++++++++++++++++++-------------------------- src/mm/ptguards.rs | 4 +-- src/mm/stack.rs | 4 +-- src/svsm_paging.rs | 10 +++---- 5 files changed, 37 insertions(+), 48 deletions(-) diff --git a/src/cpu/percpu.rs b/src/cpu/percpu.rs index 5b6020f8c..75dd838aa 100644 --- a/src/cpu/percpu.rs +++ b/src/cpu/percpu.rs @@ -14,7 +14,7 @@ use crate::cpu::vmsa::init_guest_vmsa; use crate::error::SvsmError; use crate::locking::{LockGuard, RWLock, SpinLock}; use crate::mm::alloc::{allocate_page, allocate_zeroed_page}; -use crate::mm::pagetable::{get_init_pgtable_locked, PTEntryFlags, PageTable, PageTableRef}; +use crate::mm::pagetable::{get_init_pgtable_locked, PTEntryFlags, PageTableRef}; use crate::mm::virtualrange::VirtualRange; use crate::mm::vm::{Mapping, VMKernelStack, VMPhysMem, VMRMapping, VMReserved, VMR}; use crate::mm::{ @@ -304,7 +304,7 @@ impl PerCpu { pub fn map_self_stage2(&mut self) -> Result<(), SvsmError> { let vaddr = VirtAddr::from(self as *const PerCpu); let paddr = virt_to_phys(vaddr); - let flags = PageTable::data_flags(); + let flags = PTEntryFlags::data(); self.get_pgtable().map_4k(SVSM_PERCPU_BASE, paddr, flags) } diff --git a/src/mm/pagetable.rs b/src/mm/pagetable.rs index 47daf5c4d..27de666fe 100644 --- a/src/mm/pagetable.rs +++ b/src/mm/pagetable.rs @@ -115,6 +115,32 @@ bitflags! { } } +impl PTEntryFlags { + pub fn exec() -> Self { + Self::PRESENT | Self::GLOBAL | Self::ACCESSED | Self::DIRTY + } + + pub fn data() -> Self { + Self::PRESENT | Self::GLOBAL | Self::WRITABLE | Self::NX | Self::ACCESSED | Self::DIRTY + } + + pub fn data_ro() -> Self { + Self::PRESENT | Self::GLOBAL | Self::NX | Self::ACCESSED | Self::DIRTY + } + + pub fn task_exec() -> Self { + Self::PRESENT | Self::ACCESSED | Self::DIRTY + } + + pub fn task_data() -> Self { + Self::PRESENT | Self::WRITABLE | Self::NX | Self::ACCESSED | Self::DIRTY + } + + pub fn task_data_ro() -> Self { + Self::PRESENT | Self::NX | Self::ACCESSED | Self::DIRTY + } +} + #[repr(C)] #[derive(Copy, Clone, Debug, Default)] pub struct PTEntry(PhysAddr); @@ -221,43 +247,6 @@ impl PageTable { self.root.entries[entry] = other.root.entries[entry]; } - pub fn exec_flags() -> PTEntryFlags { - PTEntryFlags::PRESENT | PTEntryFlags::GLOBAL | PTEntryFlags::ACCESSED | PTEntryFlags::DIRTY - } - - pub fn data_flags() -> PTEntryFlags { - PTEntryFlags::PRESENT - | PTEntryFlags::GLOBAL - | PTEntryFlags::WRITABLE - | PTEntryFlags::NX - | PTEntryFlags::ACCESSED - | PTEntryFlags::DIRTY - } - - pub fn data_ro_flags() -> PTEntryFlags { - PTEntryFlags::PRESENT - | PTEntryFlags::GLOBAL - | PTEntryFlags::NX - | PTEntryFlags::ACCESSED - | PTEntryFlags::DIRTY - } - - pub fn task_data_flags() -> PTEntryFlags { - PTEntryFlags::PRESENT - | PTEntryFlags::WRITABLE - | PTEntryFlags::NX - | PTEntryFlags::ACCESSED - | PTEntryFlags::DIRTY - } - - pub fn task_data_ro_flags() -> PTEntryFlags { - PTEntryFlags::PRESENT | PTEntryFlags::NX | PTEntryFlags::ACCESSED | PTEntryFlags::DIRTY - } - - pub fn task_exec_flags() -> PTEntryFlags { - PTEntryFlags::PRESENT | PTEntryFlags::ACCESSED | PTEntryFlags::DIRTY - } - fn allocate_page_table() -> Result<*mut PTPage, SvsmError> { let ptr = allocate_zeroed_page()?; Ok(ptr.as_mut_ptr::()) diff --git a/src/mm/ptguards.rs b/src/mm/ptguards.rs index 99f5af078..8347eaef9 100644 --- a/src/mm/ptguards.rs +++ b/src/mm/ptguards.rs @@ -4,7 +4,7 @@ // // Author: Joerg Roedel -use super::pagetable::PageTable; +use super::pagetable::PTEntryFlags; use crate::address::{Address, PhysAddr, VirtAddr}; use crate::cpu::percpu::this_cpu_mut; use crate::cpu::tlb::flush_address_sync; @@ -43,7 +43,7 @@ impl PerCPUPageMappingGuard { assert!((paddr_start.bits() & align_mask) == 0); assert!((paddr_end.bits() & align_mask) == 0); - let flags = PageTable::data_flags(); + let flags = PTEntryFlags::data(); let huge = ((paddr_start.bits() & (PAGE_SIZE_2M - 1)) == 0) && ((paddr_end.bits() & (PAGE_SIZE_2M - 1)) == 0); let vaddr = if huge { diff --git a/src/mm/stack.rs b/src/mm/stack.rs index 542aace17..b02cf0696 100644 --- a/src/mm/stack.rs +++ b/src/mm/stack.rs @@ -9,7 +9,7 @@ use crate::cpu::flush_tlb_global_sync; use crate::error::SvsmError; use crate::locking::SpinLock; use crate::mm::alloc::{allocate_zeroed_page, free_page}; -use crate::mm::pagetable::{get_init_pgtable_locked, PageTable, PageTableRef}; +use crate::mm::pagetable::{get_init_pgtable_locked, PTEntryFlags, PageTableRef}; use crate::mm::{phys_to_virt, virt_to_phys}; use crate::mm::{ STACK_PAGES, STACK_SIZE, STACK_TOTAL_SIZE, SVSM_SHARED_STACK_BASE, SVSM_SHARED_STACK_END, @@ -80,7 +80,7 @@ static STACK_ALLOC: SpinLock = SpinLock::new(StackRange::new( )); pub fn allocate_stack_addr(stack: VirtAddr, pgtable: &mut PageTableRef) -> Result<(), SvsmError> { - let flags = PageTable::data_flags(); + let flags = PTEntryFlags::data(); for i in 0..STACK_PAGES { let page = allocate_zeroed_page()?; let paddr = virt_to_phys(page); diff --git a/src/svsm_paging.rs b/src/svsm_paging.rs index 5ba333407..bae5bc331 100644 --- a/src/svsm_paging.rs +++ b/src/svsm_paging.rs @@ -10,7 +10,7 @@ use crate::elf; use crate::error::SvsmError; use crate::kernel_launch::KernelLaunchInfo; use crate::mm; -use crate::mm::pagetable::{set_init_pgtable, PageTable, PageTableRef}; +use crate::mm::pagetable::{set_init_pgtable, PTEntryFlags, PageTable, PageTableRef}; use crate::mm::PerCPUPageMappingGuard; use crate::sev::ghcb::PageStateChangeOp; use crate::sev::{pvalidate, PvalidateOp}; @@ -30,11 +30,11 @@ pub fn init_page_table(launch_info: &KernelLaunchInfo, kernel_elf: &elf::Elf64Fi let aligned_vaddr_end = vaddr_end.page_align_up(); let segment_len = aligned_vaddr_end - vaddr_start; let flags = if segment.flags.contains(elf::Elf64PhdrFlags::EXECUTE) { - PageTable::exec_flags() + PTEntryFlags::exec() } else if segment.flags.contains(elf::Elf64PhdrFlags::WRITE) { - PageTable::data_flags() + PTEntryFlags::data() } else { - PageTable::data_ro_flags() + PTEntryFlags::data_ro() }; pgtable @@ -50,7 +50,7 @@ pub fn init_page_table(launch_info: &KernelLaunchInfo, kernel_elf: &elf::Elf64Fi VirtAddr::from(launch_info.heap_area_virt_start), VirtAddr::from(launch_info.heap_area_virt_end()), PhysAddr::from(launch_info.heap_area_phys_start), - PageTable::data_flags(), + PTEntryFlags::data(), ) .expect("Failed to map heap"); From 5d2e6a0816f12fbd24aa13afc97f5f84a228d863 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20L=C3=B3pez?= Date: Fri, 27 Oct 2023 13:49:25 +0200 Subject: [PATCH 2/4] mm/pagetable: use PageSize type instead of usize when allocating PTEs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When allocating page table entries we simply need to know whether the allocation corresponds to a regular or huge page, so use the PageSize enum instead of a usize. Signed-off-by: Carlos López --- src/mm/pagetable.rs | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/mm/pagetable.rs b/src/mm/pagetable.rs index 27de666fe..d84b0158c 100644 --- a/src/mm/pagetable.rs +++ b/src/mm/pagetable.rs @@ -13,7 +13,7 @@ use crate::error::SvsmError; use crate::locking::{LockGuard, SpinLock}; use crate::mm::alloc::{allocate_zeroed_page, free_page}; use crate::mm::{phys_to_virt, virt_to_phys, PGTABLE_LVL3_IDX_SHARED}; -use crate::types::{PAGE_SIZE, PAGE_SIZE_2M}; +use crate::types::{PageSize, PAGE_SIZE, PAGE_SIZE_2M}; use crate::utils::immut_after_init::ImmutAfterInitCell; use bitflags::bitflags; use core::ops::{Deref, DerefMut, Index, IndexMut}; @@ -309,7 +309,7 @@ impl PageTable { PageTable::walk_addr_lvl3(&mut self.root, vaddr) } - fn alloc_pte_lvl3(entry: &mut PTEntry, vaddr: VirtAddr, pgsize: usize) -> Mapping { + fn alloc_pte_lvl3(entry: &mut PTEntry, vaddr: VirtAddr, size: PageSize) -> Mapping { let flags = entry.flags(); if flags.contains(PTEntryFlags::PRESENT) { @@ -331,10 +331,10 @@ impl PageTable { let idx = PageTable::index::<2>(vaddr); - unsafe { PageTable::alloc_pte_lvl2(&mut (*page)[idx], vaddr, pgsize) } + unsafe { PageTable::alloc_pte_lvl2(&mut (*page)[idx], vaddr, size) } } - fn alloc_pte_lvl2(entry: &mut PTEntry, vaddr: VirtAddr, pgsize: usize) -> Mapping { + fn alloc_pte_lvl2(entry: &mut PTEntry, vaddr: VirtAddr, size: PageSize) -> Mapping { let flags = entry.flags(); if flags.contains(PTEntryFlags::PRESENT) { @@ -356,13 +356,13 @@ impl PageTable { let idx = PageTable::index::<1>(vaddr); - unsafe { PageTable::alloc_pte_lvl1(&mut (*page)[idx], vaddr, pgsize) } + unsafe { PageTable::alloc_pte_lvl1(&mut (*page)[idx], vaddr, size) } } - fn alloc_pte_lvl1(entry: &mut PTEntry, vaddr: VirtAddr, pgsize: usize) -> Mapping { + fn alloc_pte_lvl1(entry: &mut PTEntry, vaddr: VirtAddr, size: PageSize) -> Mapping { let flags = entry.flags(); - if pgsize == PAGE_SIZE_2M || flags.contains(PTEntryFlags::PRESENT) { + if size == PageSize::Huge || flags.contains(PTEntryFlags::PRESENT) { return Mapping::Level1(entry); } @@ -389,9 +389,9 @@ impl PageTable { match m { Mapping::Level0(entry) => Mapping::Level0(entry), - Mapping::Level1(entry) => PageTable::alloc_pte_lvl1(entry, vaddr, PAGE_SIZE), - Mapping::Level2(entry) => PageTable::alloc_pte_lvl2(entry, vaddr, PAGE_SIZE), - Mapping::Level3(entry) => PageTable::alloc_pte_lvl3(entry, vaddr, PAGE_SIZE), + Mapping::Level1(entry) => PageTable::alloc_pte_lvl1(entry, vaddr, PageSize::Regular), + Mapping::Level2(entry) => PageTable::alloc_pte_lvl2(entry, vaddr, PageSize::Regular), + Mapping::Level3(entry) => PageTable::alloc_pte_lvl3(entry, vaddr, PageSize::Regular), } } @@ -401,8 +401,8 @@ impl PageTable { match m { Mapping::Level0(entry) => Mapping::Level0(entry), Mapping::Level1(entry) => Mapping::Level1(entry), - Mapping::Level2(entry) => PageTable::alloc_pte_lvl2(entry, vaddr, PAGE_SIZE_2M), - Mapping::Level3(entry) => PageTable::alloc_pte_lvl3(entry, vaddr, PAGE_SIZE_2M), + Mapping::Level2(entry) => PageTable::alloc_pte_lvl2(entry, vaddr, PageSize::Huge), + Mapping::Level3(entry) => PageTable::alloc_pte_lvl3(entry, vaddr, PageSize::Huge), } } @@ -800,8 +800,8 @@ impl RawPageTablePart { match m { Mapping::Level0(entry) => Mapping::Level0(entry), - Mapping::Level1(entry) => PageTable::alloc_pte_lvl1(entry, vaddr, PAGE_SIZE), - Mapping::Level2(entry) => PageTable::alloc_pte_lvl2(entry, vaddr, PAGE_SIZE), + Mapping::Level1(entry) => PageTable::alloc_pte_lvl1(entry, vaddr, PageSize::Regular), + Mapping::Level2(entry) => PageTable::alloc_pte_lvl2(entry, vaddr, PageSize::Regular), Mapping::Level3(_) => panic!("PT level 3 not possible in PageTablePart"), } } @@ -812,8 +812,8 @@ impl RawPageTablePart { match m { Mapping::Level0(entry) => Mapping::Level0(entry), Mapping::Level1(entry) => Mapping::Level1(entry), - Mapping::Level2(entry) => PageTable::alloc_pte_lvl2(entry, vaddr, PAGE_SIZE_2M), - Mapping::Level3(entry) => PageTable::alloc_pte_lvl3(entry, vaddr, PAGE_SIZE_2M), + Mapping::Level2(entry) => PageTable::alloc_pte_lvl2(entry, vaddr, PageSize::Huge), + Mapping::Level3(entry) => PageTable::alloc_pte_lvl3(entry, vaddr, PageSize::Huge), } } From e281d1ab280f432e8a16e3a7a398a1dba05a4aba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20L=C3=B3pez?= Date: Fri, 27 Oct 2023 14:18:29 +0200 Subject: [PATCH 3/4] mm/pagetable: remove useless PTE clears MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PTEntry::set() already fully sets the entry, so there's no need to clear it beforehand. Signed-off-by: Carlos López --- src/mm/pagetable.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/mm/pagetable.rs b/src/mm/pagetable.rs index d84b0158c..62e82c8ca 100644 --- a/src/mm/pagetable.rs +++ b/src/mm/pagetable.rs @@ -326,7 +326,6 @@ impl PageTable { | PTEntryFlags::WRITABLE | PTEntryFlags::USER | PTEntryFlags::ACCESSED; - entry.clear(); entry.set(set_c_bit(paddr), flags); let idx = PageTable::index::<2>(vaddr); @@ -351,7 +350,6 @@ impl PageTable { | PTEntryFlags::WRITABLE | PTEntryFlags::USER | PTEntryFlags::ACCESSED; - entry.clear(); entry.set(set_c_bit(paddr), flags); let idx = PageTable::index::<1>(vaddr); @@ -376,7 +374,6 @@ impl PageTable { | PTEntryFlags::WRITABLE | PTEntryFlags::USER | PTEntryFlags::ACCESSED; - entry.clear(); entry.set(set_c_bit(paddr), flags); let idx = PageTable::index::<0>(vaddr); From 371ca457437917adb66d34e86875936d077eb55c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20L=C3=B3pez?= Date: Fri, 27 Oct 2023 14:25:51 +0200 Subject: [PATCH 4/4] mm/pagetable: increase log level for attempts to unmap unmapped memory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Attempting to unmap unmapped memory usually indicates something went wrong in the memory management subsystem, so log it as an error. Signed-off-by: Carlos López --- src/mm/pagetable.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mm/pagetable.rs b/src/mm/pagetable.rs index 62e82c8ca..082fb4f90 100644 --- a/src/mm/pagetable.rs +++ b/src/mm/pagetable.rs @@ -668,7 +668,7 @@ impl PageTable { vaddr = vaddr + PAGE_SIZE_2M; } _ => { - log::debug!("Can't unmap - address not mapped {:#x}", vaddr); + log::error!("Can't unmap - address not mapped {:#x}", vaddr); } } }