From ec391e77fa715b1c964609b933335092cb9b6527 Mon Sep 17 00:00:00 2001 From: li041 Date: Fri, 13 Mar 2026 16:24:26 +0800 Subject: [PATCH] Rollback partial page allocation on failure --- src/arch/aarch64/ivc.rs | 13 ++++---- src/arch/aarch64/paging.rs | 54 +++++++++++++++++++++----------- src/arch/aarch64/zone.rs | 6 ++-- src/arch/loongarch64/paging.rs | 56 +++++++++++++++++++++++----------- src/arch/riscv64/paging.rs | 50 ++++++++++++++++++++++-------- src/arch/x86_64/paging.rs | 53 ++++++++++++++++++++++---------- src/memory/mm.rs | 1 + 7 files changed, 162 insertions(+), 71 deletions(-) diff --git a/src/arch/aarch64/ivc.rs b/src/arch/aarch64/ivc.rs index 1de98422..e6bdf7a0 100644 --- a/src/arch/aarch64/ivc.rs +++ b/src/arch/aarch64/ivc.rs @@ -150,7 +150,7 @@ impl From<&HvIvcConfig> for IvcRecord { } impl Zone { - pub fn ivc_init(&mut self, ivc_configs: &[HvIvcConfig]) { + pub fn ivc_init(&mut self, ivc_configs: &[HvIvcConfig]) -> HvResult { let mut inner = self.write(); for ivc_config in ivc_configs { // is_new is ok to remove @@ -170,8 +170,7 @@ impl Zone { start_paddr, rw_sec_size as _, MemFlags::READ | MemFlags::WRITE, - )) - .unwrap(); + ))?; for i in 0..ivc_config.max_peers as usize { let flags = if i == ivc_config.peer_id as _ { MemFlags::READ | MemFlags::WRITE @@ -185,8 +184,7 @@ impl Zone { start_paddr + rw_sec_size + i * out_sec_size, out_sec_size as _, flags, - )) - .unwrap(); + ))?; } inner.mmio_region_register( ivc_config.control_table_ipa as _, @@ -195,12 +193,15 @@ impl Zone { ivc_config.control_table_ipa as _, ); } else { - return; + return hv_result_err!(EINVAL); } } + IVC_INFOS .lock() .insert(self.id(), IvcInfo::from(ivc_configs)); + + Ok(()) } } diff --git a/src/arch/aarch64/paging.rs b/src/arch/aarch64/paging.rs index 18544053..f225288e 100644 --- a/src/arch/aarch64/paging.rs +++ b/src/arch/aarch64/paging.rs @@ -346,19 +346,19 @@ where Ok(p1e) } - fn map_page( - &mut self, - page: Page, - paddr: PhysAddr, - flags: MemFlags, - ) -> PagingResult<&mut PTE> { - let entry: &mut PTE = self.get_entry_mut_or_create(page)?; + fn map_page(&mut self, page: Page, paddr: PhysAddr, flags: MemFlags) -> PagingResult { + // Record the number of intermediate tables before allocation to enable rollback on failure. + let intrm_tables_len_before = self.intrm_tables.len(); + + let entry = self.get_entry_mut_or_create(page)?; if !entry.is_unused() { + // Rollback before returning error (entry ref goes out of scope here). + self.intrm_tables.truncate(intrm_tables_len_before); return Err(PagingError::AlreadyMapped); } entry.set_addr(page.size.align_down(paddr)); entry.set_flags(flags, page.size.is_huge()); - Ok(entry) + Ok(()) } fn unmap_page(&mut self, vaddr: VA) -> PagingResult<(PhysAddr, PageSize)> { @@ -473,6 +473,7 @@ where let _lock = self.clonee_lock.lock(); let mut vaddr = region.start.into(); let mut size = region.size; + let mut mapped_size = 0usize; while size > 0 { let paddr = region.mapper.map_fn(vaddr); let page_size = if PageSize::Size1G.is_aligned(vaddr) @@ -491,15 +492,34 @@ where PageSize::Size4K }; let page = Page::new_aligned(vaddr.into(), page_size); - self.inner - .map_page(page, paddr, region.flags) - .map_err(|e: PagingError| { - error!( - "failed to map page: {:#x?}({:?}) -> {:#x?}, {:?}", - vaddr, page_size, paddr, e - ); - e - })?; + if let Err(map_err) = self.inner.map_page(page, paddr, region.flags) { + error!( + "failed to map page: {:#x?}({:?}) -> {:#x?}, {:?}", + vaddr, page_size, paddr, map_err + ); + let mut rollback_vaddr = region.start.into(); + let mut rollback_size = mapped_size; + while rollback_size > 0 { + let (_, rollback_page_size) = + self.inner.unmap_page(rollback_vaddr.into()).map_err(|rollback_err| { + error!( + "failed to rollback mapped page: {:#x?}, rollback error: {:?}, original map error: {:?}", + rollback_vaddr, rollback_err, map_err + ); + rollback_err + })?; + if !rollback_page_size.is_aligned(rollback_vaddr) { + error!("rollback alignment error vaddr={:#x?}", rollback_vaddr); + loop {} + } + assert!(rollback_page_size.is_aligned(rollback_vaddr)); + assert!(rollback_page_size as usize <= rollback_size); + rollback_vaddr += rollback_page_size as usize; + rollback_size -= rollback_page_size as usize; + } + return Err(map_err.into()); + } + mapped_size += page_size as usize; vaddr += page_size as usize; size -= page_size as usize; } diff --git a/src/arch/aarch64/zone.rs b/src/arch/aarch64/zone.rs index 6c400ed7..15883ba8 100644 --- a/src/arch/aarch64/zone.rs +++ b/src/arch/aarch64/zone.rs @@ -128,8 +128,7 @@ impl Zone { } pub fn arch_zone_pre_configuration(&mut self, config: &HvZoneConfig) -> HvResult { - self.ivc_init(config.ivc_config()); - Ok(()) + self.ivc_init(config.ivc_config()) } pub fn arch_zone_post_configuration(&mut self, _config: &HvZoneConfig) -> HvResult { @@ -147,7 +146,8 @@ impl Zone { let ctr_el0: u64; core::arch::asm!("mrs {0}, ctr_el0", out(reg) ctr_el0, options(nostack, preserves_flags)); let dcache_line_size = (1 << ((ctr_el0 >> 16 & 0xF) as usize)) * 4; - self.gpm.for_each_region(|region| { + let inner = self.read(); + inner.gpm().for_each_region(|region| { // Invalidate all RAM regions of the guest if !region.flags.contains(MemFlags::IO) { // TODO: need to enrich the types and exercise more precise control // Calculate the physical start address of the region diff --git a/src/arch/loongarch64/paging.rs b/src/arch/loongarch64/paging.rs index 9a3231ee..582760e1 100644 --- a/src/arch/loongarch64/paging.rs +++ b/src/arch/loongarch64/paging.rs @@ -357,12 +357,10 @@ where Ok(p1e) } - fn map_page( - &mut self, - page: Page, - paddr: PhysAddr, - flags: MemFlags, - ) -> PagingResult<&mut PTE> { + fn map_page(&mut self, page: Page, paddr: PhysAddr, flags: MemFlags) -> PagingResult<()> { + // Record the number of intermediate tables before allocation to enable rollback on failure. + let intrm_tables_len_before = self.intrm_tables.len(); + trace!( "loongarch64: map_page: vaddr={:#x}, size={:?}, paddr={:#x}, flags={:?}", page.vaddr.into(), @@ -370,8 +368,12 @@ where paddr, flags ); - let entry: &mut PTE = self.get_entry_mut_or_create(page)?; + + let entry = self.get_entry_mut_or_create(page)?; if !entry.is_unused() { + // Rollback before returning error (entry ref goes out of scope here). + let _ = entry; + self.intrm_tables.truncate(intrm_tables_len_before); return Err(PagingError::AlreadyMapped); } trace!("loongarch64: map_page: entry is unused, continue"); @@ -384,7 +386,7 @@ where entry.addr(), entry.flags() ); - Ok(entry) + Ok(()) } fn unmap_page(&mut self, vaddr: VA) -> PagingResult<(PhysAddr, PageSize)> { @@ -491,6 +493,7 @@ where let _lock = self.clonee_lock.lock(); let mut vaddr = region.start.into(); let mut size = region.size; + let mut mapped_size = 0usize; while size > 0 { let paddr = region.mapper.map_fn(vaddr); let page_size = PageSize::Size4K; // now let's support STLB only @@ -502,15 +505,34 @@ where region.flags ); let page = Page::new_aligned(vaddr.into(), page_size); - self.inner - .map_page(page, paddr, region.flags) - .map_err(|e: PagingError| { - error!( - "failed to map page: {:#x?}({:?}) -> {:#x?}, {:?}", - vaddr, page_size, paddr, e - ); - e - })?; + if let Err(map_err) = self.inner.map_page(page, paddr, region.flags) { + error!( + "failed to map page: {:#x?}({:?}) -> {:#x?}, {:?}", + vaddr, page_size, paddr, map_err + ); + let mut rollback_vaddr = region.start.into(); + let mut rollback_size = mapped_size; + while rollback_size > 0 { + let (_, rollback_page_size) = + self.inner.unmap_page(rollback_vaddr.into()).map_err(|rollback_err| { + error!( + "failed to rollback mapped page: {:#x?}, rollback error: {:?}, original map error: {:?}", + rollback_vaddr, rollback_err, map_err + ); + rollback_err + })?; + if !rollback_page_size.is_aligned(rollback_vaddr) { + error!("rollback alignment error vaddr={:#x?}", rollback_vaddr); + loop {} + } + assert!(rollback_page_size.is_aligned(rollback_vaddr)); + assert!(rollback_page_size as usize <= rollback_size); + rollback_vaddr += rollback_page_size as usize; + rollback_size -= rollback_page_size as usize; + } + return Err(map_err.into()); + } + mapped_size += page_size as usize; vaddr += page_size as usize; size -= page_size as usize; } diff --git a/src/arch/riscv64/paging.rs b/src/arch/riscv64/paging.rs index 963b3eab..d9167d84 100644 --- a/src/arch/riscv64/paging.rs +++ b/src/arch/riscv64/paging.rs @@ -381,14 +381,20 @@ where page: Page, paddr: PhysAddr, mut flags: MemFlags, - ) -> PagingResult<&mut PTE> { - let entry: &mut PTE = self.get_entry_mut_or_create(page, &mut flags)?; + ) -> PagingResult<()> { + // Record the number of intermediate tables before allocation to enable rollback on failure. + let intrm_tables_len_before = self.intrm_tables.len(); + + let entry = self.get_entry_mut_or_create(page, &mut flags)?; if !entry.is_unused() { + // Rollback before returning error (entry ref goes out of scope here). + let _ = entry; + self.intrm_tables.truncate(intrm_tables_len_before); return Err(PagingError::AlreadyMapped); } entry.set_addr(page.size.align_down(paddr)); entry.set_flags(flags); - Ok(entry) + Ok(()) } fn unmap_page(&mut self, vaddr: VA) -> PagingResult<(PhysAddr, PageSize)> { @@ -501,6 +507,7 @@ where let _lock = self.clonee_lock.lock(); let mut vaddr = region.start.into(); let mut size = region.size; + let mut mapped_size = 0usize; while size > 0 { // Hvisor don't support huge page which > 1G. let paddr = region.mapper.map_fn(vaddr); @@ -523,15 +530,34 @@ where }; // debug!("page_size: {:#x?}", page_size); let page = Page::new_aligned(vaddr.into(), page_size); - self.inner - .map_page(page, paddr, region.flags) - .map_err(|e: PagingError| { - error!( - "failed to map page: {:#x?}({:?}) -> {:#x?}, {:?}", - vaddr, page_size, paddr, e - ); - e - })?; + if let Err(map_err) = self.inner.map_page(page, paddr, region.flags) { + error!( + "failed to map page: {:#x?}({:?}) -> {:#x?}, {:?}", + vaddr, page_size, paddr, map_err + ); + let mut rollback_vaddr = region.start.into(); + let mut rollback_size = mapped_size; + while rollback_size > 0 { + let (_, rollback_page_size) = + self.inner.unmap_page(rollback_vaddr.into()).map_err(|rollback_err| { + error!( + "failed to rollback mapped page: {:#x?}, rollback error: {:?}, original map error: {:?}", + rollback_vaddr, rollback_err, map_err + ); + rollback_err + })?; + if !rollback_page_size.is_aligned(rollback_vaddr) { + error!("rollback alignment error vaddr={:#x?}", rollback_vaddr); + loop {} + } + assert!(rollback_page_size.is_aligned(rollback_vaddr)); + assert!(rollback_page_size as usize <= rollback_size); + rollback_vaddr += rollback_page_size as usize; + rollback_size -= rollback_page_size as usize; + } + return Err(map_err.into()); + } + mapped_size += page_size as usize; vaddr += page_size as usize; size -= page_size as usize; } diff --git a/src/arch/x86_64/paging.rs b/src/arch/x86_64/paging.rs index 94aedb61..8456b412 100644 --- a/src/arch/x86_64/paging.rs +++ b/src/arch/x86_64/paging.rs @@ -324,19 +324,20 @@ where Ok(p1e) } - fn map_page( - &mut self, - page: Page, - paddr: PhysAddr, - flags: MemFlags, - ) -> PagingResult<&mut PTE> { + fn map_page(&mut self, page: Page, paddr: PhysAddr, flags: MemFlags) -> PagingResult<()> { + // Record the number of intermediate tables before allocation to enable rollback on failure. + let intrm_tables_len_before = self.intrm_tables.len(); + let entry = self.get_entry_mut_or_create(page)?; if !entry.is_unused() { + // Rollback before returning error (entry ref goes out of scope here). + let _ = entry; + self.intrm_tables.truncate(intrm_tables_len_before); return Err(PagingError::AlreadyMapped); } entry.set_addr(page.size.align_down(paddr)); entry.set_flags(flags, page.size.is_huge()); - Ok(entry) + Ok(()) } fn unmap_page(&mut self, vaddr: VA) -> PagingResult<(PhysAddr, PageSize)> { @@ -442,6 +443,7 @@ where let _lock = self.clonee_lock.lock(); let mut vaddr = region.start.into(); let mut size = region.size; + let mut mapped_size = 0usize; while size > 0 { let paddr = region.mapper.map_fn(vaddr); let page_size = if PageSize::Size1G.is_aligned(vaddr) @@ -460,15 +462,34 @@ where PageSize::Size4K }; let page = Page::new_aligned(vaddr.into(), page_size); - self.inner - .map_page(page, paddr, region.flags) - .map_err(|e: PagingError| { - error!( - "failed to map page: {:#x?}({:?}) -> {:#x?}, {:?}", - vaddr, page_size, paddr, e - ); - e - })?; + if let Err(map_err) = self.inner.map_page(page, paddr, region.flags) { + error!( + "failed to map page: {:#x?}({:?}) -> {:#x?}, {:?}", + vaddr, page_size, paddr, map_err + ); + let mut rollback_vaddr = region.start.into(); + let mut rollback_size = mapped_size; + while rollback_size > 0 { + let (_, rollback_page_size) = + self.inner.unmap_page(rollback_vaddr.into()).map_err(|rollback_err| { + error!( + "failed to rollback mapped page: {:#x?}, rollback error: {:?}, original map error: {:?}", + rollback_vaddr, rollback_err, map_err + ); + rollback_err + })?; + if !rollback_page_size.is_aligned(rollback_vaddr) { + error!("rollback alignment error vaddr={:#x?}", rollback_vaddr); + loop {} + } + assert!(rollback_page_size.is_aligned(rollback_vaddr)); + assert!(rollback_page_size as usize <= rollback_size); + rollback_vaddr += rollback_page_size as usize; + rollback_size -= rollback_page_size as usize; + } + return Err(map_err.into()); + } + mapped_size += page_size as usize; vaddr += page_size as usize; size -= page_size as usize; } diff --git a/src/memory/mm.rs b/src/memory/mm.rs index aa4d347d..26adab8a 100644 --- a/src/memory/mm.rs +++ b/src/memory/mm.rs @@ -137,6 +137,7 @@ where ); return hv_result_err!(EINVAL); } + // Keep metadata and page table consistent: pt.map must be all-or-nothing. self.pt.map(®ion)?; self.regions.insert(region.start, region); Ok(())