From 506f7fa944bf23e0fc8942c55e1996de698189ae Mon Sep 17 00:00:00 2001 From: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com> Date: Thu, 4 Jun 2026 17:33:07 -0700 Subject: [PATCH] Remove Snapshot's empty regions field Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com> --- src/hyperlight_host/src/mem/mgr.rs | 59 +------------------ .../src/sandbox/initialized_multi_use.rs | 16 +---- .../src/sandbox/snapshot/mod.rs | 21 ------- 3 files changed, 4 insertions(+), 92 deletions(-) diff --git a/src/hyperlight_host/src/mem/mgr.rs b/src/hyperlight_host/src/mem/mgr.rs index 3a59918b7..26963f4c2 100644 --- a/src/hyperlight_host/src/mem/mgr.rs +++ b/src/hyperlight_host/src/mem/mgr.rs @@ -871,24 +871,20 @@ impl SandboxMemoryManager { #[cfg(test)] #[cfg(all(not(feature = "i686-guest"), target_arch = "x86_64"))] mod tests { - use hyperlight_common::vmem::{MappingKind, PAGE_TABLE_SIZE}; use hyperlight_testing::sandbox_sizes::{LARGE_HEAP_SIZE, MEDIUM_HEAP_SIZE, SMALL_HEAP_SIZE}; use hyperlight_testing::simple_guest_as_string; use crate::GuestBinary; - use crate::mem::memory_region::MemoryRegionFlags; use crate::sandbox::SandboxConfiguration; use crate::sandbox::snapshot::Snapshot; - /// Verify page tables for a given configuration. - /// Creates a Snapshot and verifies every page in every region has correct PTEs. + /// Build a Snapshot for the given configuration and verify the + /// NULL page is not mapped in its page tables. fn verify_page_tables(name: &str, config: SandboxConfiguration) { let path = simple_guest_as_string().expect("failed to get simple guest path"); let snapshot = Snapshot::from_env(GuestBinary::FilePath(path), config) .unwrap_or_else(|e| panic!("{}: failed to create snapshot: {}", name, e)); - let regions = snapshot.regions(); - // Verify NULL page (0x0) is NOT mapped assert!( unsafe { hyperlight_common::vmem::virt_to_phys(&snapshot, 0, 1) } @@ -897,57 +893,6 @@ mod tests { "{}: NULL page (0x0) should NOT be mapped", name ); - - // Verify every page in every region - for region in regions { - let mut addr = region.guest_region.start as u64; - - while addr < region.guest_region.end as u64 { - let mapping = unsafe { hyperlight_common::vmem::virt_to_phys(&snapshot, addr, 1) } - .next() - .unwrap_or_else(|| { - panic!( - "{}: {:?} region: address 0x{:x} is not mapped", - name, region.region_type, addr - ) - }); - - // Verify identity mapping (phys == virt for low memory) - assert_eq!( - mapping.phys_base, addr, - "{}: {:?} region: address 0x{:x} should identity map, got phys 0x{:x}", - name, region.region_type, addr, mapping.phys_base - ); - - // Verify kind is Basic - let MappingKind::Basic(bm) = mapping.kind else { - panic!( - "{}: {:?} region: address 0x{:x} should be kind basic, got {:?}", - name, region.region_type, addr, mapping.kind - ); - }; - - // Verify writable - let actual = bm.writable; - let expected = region.flags.contains(MemoryRegionFlags::WRITE); - assert_eq!( - actual, expected, - "{}: {:?} region: address 0x{:x} has writable {}, expected {} (region flags: {:?})", - name, region.region_type, addr, actual, expected, region.flags - ); - - // Verify executable - let actual = bm.executable; - let expected = region.flags.contains(MemoryRegionFlags::EXECUTE); - assert_eq!( - actual, expected, - "{}: {:?} region: address 0x{:x} has executable {}, expected {} (region flags: {:?})", - name, region.region_type, addr, actual, expected, region.flags - ); - - addr += PAGE_TABLE_SIZE as u64; - } - } } #[test] diff --git a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs index a8761eed2..ba18e2f65 100644 --- a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs +++ b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs @@ -14,7 +14,6 @@ See the License for the specific language governing permissions and limitations under the License. */ -use std::collections::HashSet; use std::path::Path; use std::sync::atomic::Ordering; use std::sync::{Arc, Mutex}; @@ -533,24 +532,13 @@ impl MultiUseSandbox { self.vm.set_stack_top(snapshot.stack_top_gva()); self.vm.set_entrypoint(snapshot.entrypoint()); - let current_regions: HashSet<_> = self.vm.get_mapped_regions().cloned().collect(); - let snapshot_regions: HashSet<_> = snapshot.regions().iter().cloned().collect(); - - let regions_to_unmap = current_regions.difference(&snapshot_regions); - let regions_to_map = snapshot_regions.difference(¤t_regions); - - for region in regions_to_unmap { + let current_regions: Vec = self.vm.get_mapped_regions().cloned().collect(); + for region in ¤t_regions { self.vm .unmap_region(region) .map_err(HyperlightVmError::UnmapRegion)?; } - for region in regions_to_map { - // Safety: The region has been mapped before, and at that point the caller promised that the memory region is valid - // in their call to `MultiUseSandbox::map_region` - unsafe { self.vm.map_region(region) }.map_err(HyperlightVmError::MapRegion)?; - } - // The restored snapshot is now our most current snapshot self.snapshot = Some(snapshot.clone()); diff --git a/src/hyperlight_host/src/sandbox/snapshot/mod.rs b/src/hyperlight_host/src/sandbox/snapshot/mod.rs index 6d4eae452..eb51083ac 100644 --- a/src/hyperlight_host/src/sandbox/snapshot/mod.rs +++ b/src/hyperlight_host/src/sandbox/snapshot/mod.rs @@ -77,9 +77,6 @@ pub struct Snapshot { layout: crate::mem::layout::SandboxMemoryLayout, /// Memory of the sandbox at the time this snapshot was taken memory: ReadonlySharedMemory, - /// The memory regions that were mapped when this snapshot was - /// taken (excluding initial sandbox regions) - regions: Vec, /// Extra debug information about the binary in this snapshot, /// from when the binary was first loaded into the snapshot. /// @@ -373,13 +370,10 @@ impl Snapshot { - hyperlight_common::layout::SCRATCH_TOP_EXN_STACK_OFFSET + 1; - let extra_regions = Vec::new(); - Ok(Self { sandbox_id: SANDBOX_CONFIGURATION_COUNTER.fetch_add(1, Ordering::Relaxed), memory: ReadonlySharedMemory::from_bytes(&memory, layout.snapshot_size)?, layout, - regions: extra_regions, load_info, stack_top_gva: exn_stack_top_gva, sregs: None, @@ -558,20 +552,10 @@ impl Snapshot { debug_assert!(guest_visible_size.is_multiple_of(PAGE_SIZE)); layout.set_snapshot_size(guest_visible_size); - // Drop the embedder-provided regions: post-compaction every - // VA that used to map into a `map_file_cow` region has been - // rewritten to point at the new copy inside the snapshot blob - // (see the `guest_page` walk above). Re-mapping the originals - // on restore is unnecessary for the translation to work and - // actively risks corrupting the snapshot if the new snapshot - // PAs overlap the old region PAs. - let regions: Vec = Vec::new(); - Ok(Self { sandbox_id, layout, memory: ReadonlySharedMemory::from_bytes(&memory, guest_visible_size)?, - regions, load_info, stack_top_gva, sregs: Some(sregs), @@ -591,11 +575,6 @@ impl Snapshot { self.sandbox_id } - /// Get the mapped regions from this snapshot - pub(crate) fn regions(&self) -> &[MemoryRegion] { - &self.regions - } - /// Return the main memory contents of the snapshot #[instrument(skip_all, parent = Span::current(), level= "Trace")] pub(crate) fn memory(&self) -> &ReadonlySharedMemory {