From 9208c2910eaa2089cddac1d6f803eeefc6a1f4dd Mon Sep 17 00:00:00 2001 From: Sangho Lee Date: Sat, 11 Jul 2026 05:00:34 +0000 Subject: [PATCH 1/2] shareable physical pointer --- dev_tests/src/ratchet.rs | 2 +- .../src/arch/x86/mm/paging.rs | 35 +--- litebox_platform_lvbs/src/lib.rs | 166 ++++++---------- litebox_platform_lvbs/src/mm/mod.rs | 22 --- litebox_platform_lvbs/src/mm/vmap.rs | 184 +++++------------- litebox_runner_lvbs/src/lib.rs | 5 +- litebox_shim_optee/src/msg_handler.rs | 47 ++--- 7 files changed, 122 insertions(+), 339 deletions(-) diff --git a/dev_tests/src/ratchet.rs b/dev_tests/src/ratchet.rs index 68288b8e39..c94a856d64 100644 --- a/dev_tests/src/ratchet.rs +++ b/dev_tests/src/ratchet.rs @@ -43,7 +43,7 @@ fn ratchet_globals() -> Result<()> { ("litebox_runner_lvbs/", 5), ("litebox_runner_snp/", 2), ("litebox_shim_linux/", 1), - ("litebox_shim_optee/", 5), + ("litebox_shim_optee/", 4), ], |file| { Ok(file diff --git a/litebox_platform_lvbs/src/arch/x86/mm/paging.rs b/litebox_platform_lvbs/src/arch/x86/mm/paging.rs index 5ce73b30df..165f29584d 100644 --- a/litebox_platform_lvbs/src/arch/x86/mm/paging.rs +++ b/litebox_platform_lvbs/src/arch/x86/mm/paging.rs @@ -558,42 +558,13 @@ impl X64PageTable<'_, M, ALIGN> { frame_range: PhysFrameRange, flags: PageTableFlags, exec_ranges: Option<&[Range]>, - ) -> Result<*mut u8, MapToError> { - self.map_phys_frame_range_with(frame_range, flags, exec_ranges, M::pa_to_va) - } - - /// Map physical frame range to the page table using the direct-map offset - /// ([`MemoryProvider::pa_to_va_direct`], i.e., `PA + GVA_OFFSET`). - /// - /// Use this for VTL0 / external physical memory that should be accessible - /// through the direct-map region. - pub(crate) fn map_phys_frame_range_direct( - &self, - frame_range: PhysFrameRange, - flags: PageTableFlags, - exec_ranges: Option<&[Range]>, - ) -> Result<*mut u8, MapToError> { - self.map_phys_frame_range_with(frame_range, flags, exec_ranges, M::pa_to_va_direct) - } - - /// Common implementation for [`Self::map_phys_frame_range`] and - /// [`Self::map_phys_frame_range_direct`]. - /// - /// `pa_to_va` selects how physical addresses are translated to virtual - /// addresses — either via `KERNEL_OFFSET` or `GVA_OFFSET`. - fn map_phys_frame_range_with( - &self, - frame_range: PhysFrameRange, - flags: PageTableFlags, - exec_ranges: Option<&[Range]>, - pa_to_va: fn(PhysAddr) -> VirtAddr, ) -> Result<*mut u8, MapToError> { let mut allocator = PageTableAllocator::::new(); let mut inner = self.inner.lock(); for target_frame in frame_range { let page: Page = - Page::containing_address(pa_to_va(target_frame.start_address())); + Page::containing_address(M::pa_to_va(target_frame.start_address())); match inner.translate(page.start_address()) { TranslateResult::Mapped { @@ -655,12 +626,12 @@ impl X64PageTable<'_, M, ALIGN> { } let start_page = - Page::::containing_address(pa_to_va(frame_range.start.start_address())); + Page::::containing_address(M::pa_to_va(frame_range.start.start_address())); let count = (frame_range.end.start_address() - frame_range.start.start_address()) / Size4KiB::SIZE; flush_tlb_range(start_page, count.trunc()); - Ok(pa_to_va(frame_range.start.start_address()).as_mut_ptr()) + Ok(M::pa_to_va(frame_range.start.start_address()).as_mut_ptr()) } /// Map non-contiguous physical frames to virtually contiguous addresses. diff --git a/litebox_platform_lvbs/src/lib.rs b/litebox_platform_lvbs/src/lib.rs index 461cdbddee..8eb29305d4 100644 --- a/litebox_platform_lvbs/src/lib.rs +++ b/litebox_platform_lvbs/src/lib.rs @@ -23,7 +23,7 @@ use litebox::{ }; use litebox_common_linux::errno::Errno; use litebox_common_linux::vmap::{ - GlobalVmapManager, PhysPageAddr, PhysPageAddrArray, PhysPageMapInfo, PhysPageMapPermissions, + GlobalVmapManager, PhysPageAddrArray, PhysPageMapInfo, PhysPageMapPermissions, PhysPointerError, VmapManager, }; use x86_64::{ @@ -94,7 +94,7 @@ pub const BASE_PAGE_TABLE_ID: usize = 0; // 0xFFFF_C000_0000_0000 ├─────────────────────────────────┤ // │ Direct map region (64 TiB) │ // │ VA = PA + GVA_OFFSET │ -// │ VTL0 memory mapped on demand │ +// │ Currently unused │ // │ │ // │ ┄ ┄ ┄ ┄ ┄ ┄ ┄ ┄ ┄ ┄ ┄ ┄ ┄ ┄ │ // │ VTL1 PA range = unmapped gap │ @@ -108,12 +108,13 @@ pub const BASE_PAGE_TABLE_ID: usize = 0; // │ mmap / TA memory │ // 0x0000_0000_0001_0000 └─────────────────────────────────┘ ← USER_ADDR_MIN // -// The 64 TiB direct map reservation ensures that any physical address -// up to 64 TiB can be mapped via the simple PA + GVA_OFFSET formula -// without colliding with the vmap region. A 1 TiB guard gap between -// the direct map and the vmap region catches stray accesses. -// VTL1 memory is never mapped in the direct map; it lives exclusively -// in the VTL1 kernel region at KERNEL_OFFSET. +// The 64 TiB direct map region is reserved for possible future use (e.g., device +// drivers, persistent mapping). Foreign physical memory currently uses private +// mappings in the vmap region instead. If direct mapping is restored, physical +// addresses up to 64 TiB can use the PA + GVA_OFFSET formula without colliding +// with vmap. A 1 TiB guard gap catches stray accesses. VTL1 memory must never +// be mapped in the direct map; it lives exclusively in the VTL1 kernel region +// at KERNEL_OFFSET. // // The VTL1 kernel region at the top of the address space maps the // entire VTL1 kernel via PA + KERNEL_OFFSET. A 1 TiB guard gap @@ -1106,22 +1107,6 @@ impl litebox::platform::SystemInfoProvider for LinuxKernel< } } -/// Checks whether the given physical addresses are contiguous with respect to ALIGN. -fn is_contiguous(addrs: &[PhysPageAddr]) -> bool { - for window in addrs.windows(2) { - let first = window[0].as_usize(); - let second = window[1].as_usize(); - if let Some(expected) = first.checked_add(ALIGN) { - if second != expected { - return false; - } - } else { - return false; - } - } - true -} - unsafe impl VmapManager for LinuxKernel { type MapInfo = LvbsPhysPageMapInfo; @@ -1162,7 +1147,8 @@ unsafe impl VmapManager for Linu // Reject duplicates early as an API-level validation. The page-table implementation also // rejects duplicate/shared mappings, but this keeps the error local to the input array. - if !is_contiguous(pages) { + // A single page can never collide with itself, so skip the set allocation. + if pages.len() > 1 { let mut seen = hashbrown::HashSet::with_capacity(pages.len()); for page in pages { if !seen.insert(page.as_usize()) { @@ -1177,79 +1163,54 @@ unsafe impl VmapManager for Linu flags |= PageTableFlags::WRITABLE; } - // `validate_unowned` rejects VTL1-owned PA before callers reach `vmap`, so these pages - // are foreign. Contiguous foreign PA uses the foreign direct-map VA range; non-contiguous - // foreign PA uses the vmap VA range. Neither range aliases VTL1-owned Rust memory. - if is_contiguous(pages) { - let phys_start = x86_64::PhysAddr::new(pages[0].as_usize() as u64); - let phys_end = x86_64::PhysAddr::new( - pages - .last() - .unwrap() - .as_usize() - .checked_add(ALIGN) - .ok_or(PhysPointerError::Overflow)? as u64, - ); - let frame_range = PhysFrame::range( - PhysFrame::::containing_address(phys_start), - PhysFrame::::containing_address(phys_end), - ); - - match self - .page_table_manager - .current_page_table() - .map_phys_frame_range_direct(frame_range, flags, None) - { - Ok(page_addr) => Ok(LvbsPhysPageMapInfo::new(page_addr, pages.len() * ALIGN)), - Err(MapToError::PageAlreadyMapped(_)) => { - Err(PhysPointerError::AlreadyMapped(pages[0].as_usize())) + // Always allocate a fresh, private virtual address window for the mapping. This lets + // multiple cores map the same physical frame(s) concurrently at distinct VAs (used only for + // transient data copy in/out via raw pointers), so a core unmapping its window never + // disturbs another core's access to the same frame. + // + // `validate_unowned` rejects VTL1-owned PA before callers reach `vmap`, so these pages are + // foreign and the vmap VA range never aliases VTL1-owned Rust memory. + let frames: alloc::vec::Vec> = pages + .iter() + .map(|p| { + let address = p.as_usize(); + x86_64::PhysAddr::try_new(address as u64) + .map(PhysFrame::containing_address) + .map_err(|_| PhysPointerError::InvalidPhysicalAddress(address)) + }) + .collect::>()?; + + let base_va = vmap_allocator() + .allocate_va(frames.len()) + .map_err(|e| match e { + crate::mm::vmap::VmapAllocError::VaSpaceExhausted => { + PhysPointerError::VaSpaceExhausted } - Err(MapToError::FrameAllocationFailed) => { - Err(PhysPointerError::FrameAllocationFailed) + // `pages` was checked non-empty above and `frames` is built 1:1 from it, so the + // allocator cannot report an empty input here. + crate::mm::vmap::VmapAllocError::EmptyInput => { + unreachable!("frames is derived 1:1 from a non-empty pages slice") } - Err(MapToError::ParentEntryHugePage) => Err( - PhysPointerError::InvalidPhysicalAddress(pages[0].as_usize()), - ), - } - } else { - let frames: alloc::vec::Vec> = pages - .iter() - .map(|p| PhysFrame::containing_address(x86_64::PhysAddr::new(p.as_usize() as u64))) - .collect(); - - let base_va = vmap_allocator() - .allocate_va_and_register_map(&frames) - .map_err(|e| match e { - crate::mm::vmap::VmapAllocError::EmptyInput => { - PhysPointerError::InvalidPhysicalAddress(0) - } - crate::mm::vmap::VmapAllocError::DuplicateMapping => { - PhysPointerError::AlreadyMapped(pages[0].as_usize()) - } - crate::mm::vmap::VmapAllocError::VaSpaceExhausted => { - PhysPointerError::VaSpaceExhausted - } - })?; + })?; - match self - .page_table_manager - .current_page_table() - .map_non_contiguous_phys_frames(&frames, base_va, flags) - { - Ok(page_addr) => Ok(LvbsPhysPageMapInfo::new(page_addr, pages.len() * ALIGN)), - Err(e) => { - let _ = vmap_allocator().unregister_allocation(base_va); - match e { - MapToError::PageAlreadyMapped(_) => { - Err(PhysPointerError::AlreadyMapped(pages[0].as_usize())) - } - MapToError::FrameAllocationFailed => { - Err(PhysPointerError::FrameAllocationFailed) - } - MapToError::ParentEntryHugePage => Err( - PhysPointerError::InvalidPhysicalAddress(pages[0].as_usize()), - ), + match self + .page_table_manager + .current_page_table() + .map_non_contiguous_phys_frames(&frames, base_va, flags) + { + Ok(page_addr) => Ok(LvbsPhysPageMapInfo::new(page_addr, pages.len() * ALIGN)), + Err(e) => { + vmap_allocator().free_va(base_va, frames.len()); + match e { + MapToError::PageAlreadyMapped(_) => { + Err(PhysPointerError::AlreadyMapped(pages[0].as_usize())) + } + MapToError::FrameAllocationFailed => { + Err(PhysPointerError::FrameAllocationFailed) } + MapToError::ParentEntryHugePage => Err( + PhysPointerError::InvalidPhysicalAddress(pages[0].as_usize()), + ), } } } @@ -1275,17 +1236,12 @@ unsafe impl VmapManager for Linu } // PTEs are already cleared at this point, so the mapping is functionally gone - // and a retry would only re-fail against empty page-table entries. If the VA - // allocator's bookkeeping is inconsistent, surface it via `debug_assert!`. The - // VA region is leaked but cannot be safely recycled. - let unregister_ok = !crate::mm::vmap::is_vmap_address(base_va) - || crate::mm::vmap::vmap_allocator() - .unregister_allocation(base_va) - .is_some(); - debug_assert!( - unregister_ok, - "vmap allocator unregister failed at {base_va:?}", - ); + // and a retry would only re-fail against empty page-table entries. Return the VA + // range to the allocator. `vmap_info` is consumed by value and never cloned, so this + // range is freed exactly once. + if crate::mm::vmap::is_vmap_address(base_va) { + crate::mm::vmap::vmap_allocator().free_va(base_va, size / ALIGN); + } Ok(()) } diff --git a/litebox_platform_lvbs/src/mm/mod.rs b/litebox_platform_lvbs/src/mm/mod.rs index a96d5fd128..fd1fc4427c 100644 --- a/litebox_platform_lvbs/src/mm/mod.rs +++ b/litebox_platform_lvbs/src/mm/mod.rs @@ -40,28 +40,6 @@ pub trait MemoryProvider { /// The caller must ensure that the memory range is valid and not used by any others. unsafe fn mem_fill_pages(start: usize, size: usize); - /// Obtain physical address (PA) of a page given its direct-map VA. - /// - /// The direct map covers all physical memory via `VA = PA + GVA_OFFSET`. - /// Use this for VTL0 / external physical memory. - fn va_to_pa_direct(va: VirtAddr) -> PhysAddr { - PhysAddr::new_truncate(va - Self::GVA_OFFSET) - } - - /// Obtain the direct-map virtual address (VA) of a page given its PA. - /// - /// The direct map covers all physical memory via `VA = PA + GVA_OFFSET`. - /// Use this for VTL0 / external physical memory. - fn pa_to_va_direct(pa: PhysAddr) -> VirtAddr { - let pa = pa.as_u64() & !Self::PRIVATE_PTE_MASK; - let va = VirtAddr::new_truncate(pa + Self::GVA_OFFSET.as_u64()); - assert!( - va.as_u64() < crate::VMAP_START as u64, - "VA {va:#x} is out of range for direct mapping" - ); - va - } - /// Obtain physical address (PA) of a page given its kernel VA. /// /// The VTL1 kernel region maps kernel memory via `VA = PA + KERNEL_OFFSET`. diff --git a/litebox_platform_lvbs/src/mm/vmap.rs b/litebox_platform_lvbs/src/mm/vmap.rs index 3f04477a6e..2416bc87fc 100644 --- a/litebox_platform_lvbs/src/mm/vmap.rs +++ b/litebox_platform_lvbs/src/mm/vmap.rs @@ -5,16 +5,16 @@ //! //! This module provides functionality similar to Linux kernel's `vmap()` and `vunmap()`: //! - Reserves a virtual address region for vmap mappings -//! - Maintains PA→VA mappings using HashMap for duplicate detection and cleanup +//! - Tracks allocations by base virtual address for cleanup +//! +//! The same physical frame may be mapped at multiple virtual addresses simultaneously, so no +//! PA→VA uniqueness is enforced: each mapping is a private, transient window. -use alloc::boxed::Box; -use hashbrown::HashMap; use litebox::utils::TruncateExt; use rangemap::RangeSet; use spin::Once; use spin::mutex::SpinMutex; use x86_64::VirtAddr; -use x86_64::structures::paging::{PhysFrame, Size4KiB}; use crate::mshv::vtl1_mem_layout::PAGE_SIZE; @@ -24,9 +24,6 @@ pub enum VmapAllocError { /// The input frame slice was empty. #[error("empty frame slice")] EmptyInput, - /// At least one physical frame is already mapped. - #[error("physical frame already mapped")] - DuplicateMapping, /// The vmap virtual address region has no contiguous range large enough. #[error("vmap virtual address space exhausted")] VaSpaceExhausted, @@ -41,26 +38,19 @@ const VMAP_END_VPN: usize = VMAP_END / PAGE_SIZE; /// Number of unmapped guard pages appended after each vmap allocation. const GUARD_PAGES: usize = 1; -/// Information about a single vmap allocation. -#[derive(Clone, Debug)] -struct VmapAllocation { - /// Physical frames of the mapped pages (in order). - frames: Box<[PhysFrame]>, -} - /// Inner state for the vmap region allocator. /// -/// Uses a bump allocator with a `RangeSet` free list for virtual page numbers -/// and HashMap for maintaining mappings between physical and virtual addresses. +/// Uses a bump allocator with a `RangeSet` free list for virtual page numbers. +/// +/// The same physical frame may be mapped at multiple virtual addresses simultaneously: each +/// mapping is a private, transient window (used only to copy data in/out). The allocator only +/// tracks free VA ranges; the caller owns the page count for each live mapping (it is recoverable +/// from the mapping info) and passes it back on teardown. struct VmapRegionAllocatorInner { /// Next available virtual page number for allocation (bump allocator). next_vpn: usize, /// Free set of previously allocated and freed VPN ranges (auto-coalescing). free_set: RangeSet, - /// Map from physical frame to virtual address. - pa_to_va_map: HashMap, VirtAddr>, - /// Allocation metadata indexed by starting virtual address. - allocations: HashMap, } impl VmapRegionAllocatorInner { @@ -69,8 +59,6 @@ impl VmapRegionAllocatorInner { Self { next_vpn: VMAP_START_VPN, free_set: RangeSet::new(), - pa_to_va_map: HashMap::new(), - allocations: HashMap::new(), } } @@ -134,7 +122,7 @@ pub fn is_vmap_address(va: VirtAddr) -> bool { (VMAP_START..VMAP_END).contains(&va.as_u64().trunc()) } -/// Vmap region allocator that manages virtual address allocation and PA↔VA mappings. +/// Vmap region allocator that manages virtual address allocation for transient physical mappings. pub struct VmapRegionAllocator { inner: SpinMutex, } @@ -146,72 +134,30 @@ impl VmapRegionAllocator { } } - /// Atomically allocates VA range, registers mappings, and records allocation. - /// - /// This ensures consistency: either the entire operation succeeds or nothing changes. + /// Allocates a fresh VA range covering `num_pages` mapped pages (plus trailing guard pages). /// /// # Errors /// - /// - [`VmapAllocError::EmptyInput`] — `frames` is empty. - /// - [`VmapAllocError::DuplicateMapping`] — a physical frame is already mapped. + /// - [`VmapAllocError::EmptyInput`] — `num_pages` is zero. /// - [`VmapAllocError::VaSpaceExhausted`] — no contiguous VA range is available. - pub fn allocate_va_and_register_map( - &self, - frames: &[PhysFrame], - ) -> Result { - if frames.is_empty() { + pub fn allocate_va(&self, num_pages: usize) -> Result { + if num_pages == 0 { return Err(VmapAllocError::EmptyInput); } - let mut inner = self.inner.lock(); - - // Check for duplicate PA mappings before allocating - for frame in frames { - if inner.pa_to_va_map.contains_key(frame) { - return Err(VmapAllocError::DuplicateMapping); - } - } - - let base_va = inner - .allocate_va_range(frames.len()) - .ok_or(VmapAllocError::VaSpaceExhausted)?; - let end_va = base_va + (frames.len() as u64) * (PAGE_SIZE as u64); - - for (va, &frame) in (base_va.as_u64()..end_va.as_u64()) - .step_by(PAGE_SIZE) - .map(VirtAddr::new) - .zip(frames.iter()) - { - inner.pa_to_va_map.insert(frame, va); - } - - inner.allocations.insert( - base_va, - VmapAllocation { - frames: frames.into(), - }, - ); - - Ok(base_va) + self.inner + .lock() + .allocate_va_range(num_pages) + .ok_or(VmapAllocError::VaSpaceExhausted) } - /// Unregisters all mappings for an allocation starting at the given virtual address - /// and returns its VA range to the free list. + /// Returns a `num_pages`-page VA range starting at `base_va` to the free list. /// - /// This is used both for normal `vunmap` teardown and to roll back a failed - /// page-table mapping after `allocate_va_and_register_map` succeeds. - /// - /// Returns the number of pages that were unmapped, or `None` if no allocation was found. - pub fn unregister_allocation(&self, base_va: VirtAddr) -> Option { - let mut inner = self.inner.lock(); - let allocation = inner.allocations.remove(&base_va)?; - for frame in &allocation.frames { - inner.pa_to_va_map.remove(frame); - } - - inner.free_va_range(base_va, allocation.frames.len()); - - Some(allocation.frames.len()) + /// This is used both for normal `vunmap` teardown and to roll back a failed page-table mapping + /// after [`Self::allocate_va`] succeeds. `base_va`/`num_pages` must match a value pair from a + /// prior `allocate_va`; mapping-info move semantics guarantee each range is freed at most once. + pub fn free_va(&self, base_va: VirtAddr, num_pages: usize) { + self.inner.lock().free_va_range(base_va, num_pages); } } @@ -224,7 +170,6 @@ pub fn vmap_allocator() -> &'static VmapRegionAllocator { #[cfg(test)] mod tests { use super::*; - use x86_64::PhysAddr; #[test] fn test_allocate_va_range() { @@ -268,94 +213,53 @@ mod tests { } #[test] - fn test_allocate_va_and_register_map() { + fn test_allocate_va() { let allocator = VmapRegionAllocator::new(); - let frames = alloc::vec![ - PhysFrame::::containing_address(PhysAddr::new(0x1000)), - PhysFrame::::containing_address(PhysAddr::new(0x3000)), - PhysFrame::::containing_address(PhysAddr::new(0x5000)), - ]; - - // Allocate and register - let base_va = allocator.allocate_va_and_register_map(&frames); + // Allocate a 3-page range + let base_va = allocator.allocate_va(3); assert!(base_va.is_ok()); - let base_va = base_va.unwrap(); - assert_eq!(base_va.as_u64(), VMAP_START as u64); - - // Duplicate PA should fail with DuplicateMapping - let duplicate = allocator - .allocate_va_and_register_map(&[PhysFrame::containing_address(PhysAddr::new(0x1000))]); - assert!(matches!(duplicate, Err(VmapAllocError::DuplicateMapping))); + assert_eq!(base_va.unwrap().as_u64(), VMAP_START as u64); - // Empty input should fail with EmptyInput + // Zero pages should fail with EmptyInput assert!(matches!( - allocator.allocate_va_and_register_map(&[]), + allocator.allocate_va(0), Err(VmapAllocError::EmptyInput) )); } #[test] - fn test_rollback_via_unregister() { + fn test_rollback_via_free() { let allocator = VmapRegionAllocator::new(); - let frames = alloc::vec![ - PhysFrame::::containing_address(PhysAddr::new(0x1000)), - PhysFrame::::containing_address(PhysAddr::new(0x2000)), - ]; - - let base_va = allocator.allocate_va_and_register_map(&frames).unwrap(); + let base_va = allocator.allocate_va(2).unwrap(); - // Simulate rollback by unregistering immediately - let count = allocator.unregister_allocation(base_va); - assert_eq!(count, Some(2)); + // Simulate rollback by freeing immediately + allocator.free_va(base_va, 2); - // Mappings should be gone — re-registering the same PAs must succeed - let new_va = allocator.allocate_va_and_register_map(&frames).unwrap(); + // The VA range should be gone — re-allocating must succeed and reuse it + let new_va = allocator.allocate_va(2).unwrap(); assert_eq!(new_va, base_va); } #[test] - fn test_unregister_allocation() { + fn test_free_va() { let allocator = VmapRegionAllocator::new(); - let frames = alloc::vec![ - PhysFrame::::containing_address(PhysAddr::new(0x1000)), - PhysFrame::::containing_address(PhysAddr::new(0x3000)), - PhysFrame::::containing_address(PhysAddr::new(0x5000)), - ]; + let base_va = allocator.allocate_va(3).unwrap(); - let base_va = allocator.allocate_va_and_register_map(&frames).unwrap(); - - // Unregister - let num_pages = allocator.unregister_allocation(base_va); - assert_eq!(num_pages, Some(3)); - - // Mappings should be gone — re-registering the same PAs must succeed - // and reuse the freed VA range - let new_va = allocator.allocate_va_and_register_map(&frames).unwrap(); + // Free, then re-allocating the same size must reuse the freed VA range + allocator.free_va(base_va, 3); + let new_va = allocator.allocate_va(3).unwrap(); assert_eq!(new_va, base_va); - - // Unregistering an unknown VA returns None - assert_eq!( - allocator.unregister_allocation(VirtAddr::new(VMAP_END as u64 - 0x1000)), - None - ); } #[test] fn test_guard_page_gap() { let allocator = VmapRegionAllocator::new(); - let frames_a = alloc::vec![PhysFrame::::containing_address(PhysAddr::new( - 0x1000 - )),]; - let frames_b = alloc::vec![PhysFrame::::containing_address(PhysAddr::new( - 0x2000 - )),]; - - let va_a = allocator.allocate_va_and_register_map(&frames_a).unwrap(); - let va_b = allocator.allocate_va_and_register_map(&frames_b).unwrap(); + let va_a = allocator.allocate_va(1).unwrap(); + let va_b = allocator.allocate_va(1).unwrap(); // Allocations should be separated by at least GUARD_PAGES unmapped pages let gap_pages = (va_b.as_u64() - va_a.as_u64()) / PAGE_SIZE as u64; diff --git a/litebox_runner_lvbs/src/lib.rs b/litebox_runner_lvbs/src/lib.rs index d9877ad71e..3731651e5a 100644 --- a/litebox_runner_lvbs/src/lib.rs +++ b/litebox_runner_lvbs/src/lib.rs @@ -39,8 +39,7 @@ use litebox_platform_lvbs::{ }; use litebox_platform_multiplex::Platform; use litebox_shim_optee::msg_handler::{ - decode_ta_request, handle_optee_msg_args, handle_optee_smc_args, packed_msg_args_lock, - update_optee_msg_args, + decode_ta_request, handle_optee_msg_args, handle_optee_smc_args, update_optee_msg_args, }; use litebox_shim_optee::session::{OpenSessionTarget, TaInstance, session_manager}; use litebox_shim_optee::{NormalWorldConstPtr, NormalWorldMutPtr, UserConstPtr}; @@ -1244,8 +1243,6 @@ fn write_non_ta_msg_args_to_normal_world( msg_args_phys_addr.trunc(), msg_args_size, )?; - // Serialize the packed-page write. See `packed_msg_args_lock`. - let _packed_guard = packed_msg_args_lock(); ptr.write_slice_at_offset(0, &blob)?; Ok(()) } diff --git a/litebox_shim_optee/src/msg_handler.rs b/litebox_shim_optee/src/msg_handler.rs index f989e20239..ae323b4816 100644 --- a/litebox_shim_optee/src/msg_handler.rs +++ b/litebox_shim_optee/src/msg_handler.rs @@ -142,6 +142,15 @@ fn parse_optee_msg_args( /// the main one at offset `optee_msg_args_total_size(num_params)` (the *actual* `num_params`, /// not `MAX_ARG_PARAM_COUNT`). This matches the Linux driver's layout. /// +/// # Concurrency +/// +/// This read is intentionally not serialized against a concurrent write-back on the same 4 KiB +/// frame. The Linux OP-TEE driver packs multiple `optee_msg_arg`s into sub-page slots and hands +/// out one slot per in-flight call (bitmap under `shm_arg_cache.mutex`), so concurrent cores touch +/// disjoint slots. Each access maps the frame into its own private, transient VA window (see the +/// `vmap`-based `PhysMutPtr`), so no page-table conflict arises either. A malicious normal world +/// that overlaps slots is contained by the copy-once + `FromBytes` + re-validate discipline above. +/// /// VTL0 physical memory layout at `phys_addr`: /// /// ```text @@ -208,12 +217,7 @@ pub fn handle_optee_smc_args( OpteeSmcFunction::CallWithArg => { let msg_args_addr = smc.optee_msg_args_phys_addr()?; let msg_args_addr: usize = msg_args_addr.trunc(); - // Serialize the packed-page read against a concurrent write-back. See - // `packed_msg_args_lock`. - let (msg_args, _) = { - let _packed_guard = packed_msg_args_lock(); - read_optee_msg_args_from_phys(msg_args_addr, false)? - }; + let (msg_args, _) = read_optee_msg_args_from_phys(msg_args_addr, false)?; Ok(OpteeSmcResult::CallWithArg { msg_args, rpc_args: None, @@ -223,10 +227,7 @@ pub fn handle_optee_smc_args( OpteeSmcFunction::CallWithRpcArg => { let msg_args_addr = smc.optee_msg_args_phys_addr()?; let msg_args_addr: usize = msg_args_addr.trunc(); - let (msg_args, rpc_args) = { - let _packed_guard = packed_msg_args_lock(); - read_optee_msg_args_from_phys(msg_args_addr, true)? - }; + let (msg_args, rpc_args) = read_optee_msg_args_from_phys(msg_args_addr, true)?; Ok(OpteeSmcResult::CallWithArg { msg_args, rpc_args, @@ -246,12 +247,7 @@ pub fn handle_optee_smc_args( main_max + optee_msg_args_total_size(OpteeRpcArgs::MAX_RPC_ARG_PARAM_COUNT.trunc()); let mut blob = alloc::vec![0u8; copy_size]; - // Serialize the packed-page read against a concurrent write-back. See - // `packed_msg_args_lock`. - { - let _packed_guard = packed_msg_args_lock(); - shm_info.read_at(offset, &mut blob)?; - } + shm_info.read_at(offset, &mut blob)?; let (msg_args, rpc_args) = parse_optee_msg_args(&blob, true)?; // Compute the physical address of `OpteeMsgArgs` @@ -397,25 +393,6 @@ pub struct TaRequestInfo { pub out_shm_info: [Option>; UteeParamOwned::TEE_NUM_PARAMS], } -/// Acquire the lock serializing packed-`OpteeMsgArgs` page access on the base page table. -/// -/// The OP-TEE driver packs multiple requests into sub-page slots of one frame which can be -/// concurrently access by multiple cores which are on the base page table. Since LiteBox -/// currently doesn't support shared mapping, it uses this lock to serialize the concurrent -/// access. Note that cores on different task page tables (i.e., instances) do not need to -/// acquire this lock since they maintain their own mappings. -/// -/// Hold the guard only across the packed-page read/write. -/// -/// TODO: This is a temporary mitigation. It should be replaced by a more fundamental -/// approach such as shared mapping support, physical address range reservation, and/or -/// sub-page access control. -#[must_use] -pub fn packed_msg_args_lock() -> spin::mutex::SpinMutexGuard<'static, ()> { - static PACKED_MSG_ARGS_LOCK: spin::mutex::SpinMutex<()> = spin::mutex::SpinMutex::new(()); - PACKED_MSG_ARGS_LOCK.lock() -} - /// This function decodes a TA request contained in `OpteeMsgArgs`. /// /// It copies the entire parameter data from the normal world shared memory into the secure world's From bc6cda5d3c8c2d2a2b7892a9642018bf38b9631f Mon Sep 17 00:00:00 2001 From: Sangho Lee Date: Fri, 17 Jul 2026 17:59:07 +0000 Subject: [PATCH 2/2] revise comment --- litebox_shim_optee/src/msg_handler.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/litebox_shim_optee/src/msg_handler.rs b/litebox_shim_optee/src/msg_handler.rs index ae323b4816..3f73043b5b 100644 --- a/litebox_shim_optee/src/msg_handler.rs +++ b/litebox_shim_optee/src/msg_handler.rs @@ -148,8 +148,11 @@ fn parse_optee_msg_args( /// frame. The Linux OP-TEE driver packs multiple `optee_msg_arg`s into sub-page slots and hands /// out one slot per in-flight call (bitmap under `shm_arg_cache.mutex`), so concurrent cores touch /// disjoint slots. Each access maps the frame into its own private, transient VA window (see the -/// `vmap`-based `PhysMutPtr`), so no page-table conflict arises either. A malicious normal world -/// that overlaps slots is contained by the copy-once + `FromBytes` + re-validate discipline above. +/// `vmap`-based `PhysMutPtr`), so no page-table conflict arises either. A malicious normal-world +/// kernel can provide overlapped slots, but this only results in copying invalid `optee_msg_arg` +/// and/or corrupting normal-world memory - the malicious kernel can easily do these even without +/// slot overlaps. Our fallible memcpy with `FromBytes` ensures this copy-in does not result in +/// Rust safety/soundness issues. /// /// VTL0 physical memory layout at `phys_addr`: ///