Skip to content

Add overlap check between MMIO passthrough regions and MMIO intercept handler regions #384

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
li041 merged 1 commit into from
Aug 22, 2026
Merged

Add overlap check between MMIO passthrough regions and MMIO intercept handler regions #384

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions src/memory/mm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,28 @@ where
self.pt.root_paddr()
}

/// Check whether `[start, start+size)` overlaps with any region in this MemorySet.
pub fn is_range_overlap(&self, start: usize, size: usize) -> bool
where
PT::VA: From<usize>,
{
let end = start + size;
let va_start: PT::VA = start.into();
if let Some((_, before)) = self.regions.range(..va_start).last() {
let before_end: usize = before.start.into() + before.size;
if before_end > start {
return true;
}
}
if let Some((_, after)) = self.regions.range(va_start..).next() {
let after_start: usize = after.start.into();
if after_start < end {
return true;
}
}
false
}

fn test_free_area(&self, other: &MemoryRegion<PT::VA>) -> bool {
if let Some((_, before)) = self.regions.range(..other.start).last() {
if before.is_overlap_with(other) {
Expand Down
7 changes: 7 additions & 0 deletions src/memory/mmio.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,13 @@ impl MMIORegion {
pub fn contains_region(&self, addr: GuestPhysAddr, sz: usize) -> bool {
addr >= self.start && addr + (sz as usize) <= self.start + (self.size as usize)
}

/// Check whether this region overlaps with `other`.
pub fn is_overlap_with(&self, other: &MMIORegion) -> bool {
let self_end = self.start + self.size;
let other_end = other.start + other.size;
!(self_end <= other.start || self.start >= other_end)
}
}

pub fn mmio_perform_access(base: usize, mmio: &mut MMIOAccess) {
Expand Down
84 changes: 48 additions & 36 deletions src/pci/pci_handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -784,12 +784,14 @@ fn handle_endpoint_access(
)
.is_ok()
{}
gpm.try_insert_quiet(MemoryRegion::new_with_offset_mapper(
new_vaddr_aligned as GuestPhysAddr,
paddr as HostPhysAddr,
bar_size as _,
MemFlags::READ | MemFlags::WRITE,
))?;
guard.insert_passthrough_region_quiet(
MemoryRegion::new_with_offset_mapper(
new_vaddr_aligned as GuestPhysAddr,
paddr as HostPhysAddr,
bar_size as _,
MemFlags::READ | MemFlags::WRITE,
),
)?;

@li041 li041 Aug 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This path deletes the old passthrough mapping before calling try_insert_passthrough_region_quiet. If the new BAR overlaps an MMIO handler, the helper returns an error and ? exits without restoring the old mapping, even though the BAR and hardware state were already updated. The guest then loses the old BAR mapping. Please make this update transactional by preflighting the new region or rolling back the deletion on error. The ROM and bridge BAR paths need the same treatment.

@Solicey Solicey Aug 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The BAR and ROM paths now preflight the new region with is_mmio_handler_overlap before deleting the old passthrough mapping. A would-be overlap is rejected before any mutation, so the old mapping is never lost. Applied to the four BAR and two ROM sites.

}
drop(guard);
#[cfg(target_arch = "aarch64")]
Expand Down Expand Up @@ -925,12 +927,14 @@ fn handle_endpoint_access(
// warn!("delete bar {}: can not found 0x{:x}", slot, old_vaddr);
}
// Insert new gpm mapping at new address
gpm.try_insert_quiet(MemoryRegion::new_with_offset_mapper(
new_vaddr as GuestPhysAddr,
paddr as HostPhysAddr,
bar_size as _,
MemFlags::READ | MemFlags::WRITE,
))?;
guard.insert_passthrough_region_quiet(
MemoryRegion::new_with_offset_mapper(
new_vaddr as GuestPhysAddr,
paddr as HostPhysAddr,
bar_size as _,
MemFlags::READ | MemFlags::WRITE,
),
)?;
}
drop(guard);
/* after update gpm, mem barrier is needed
Expand Down Expand Up @@ -1072,12 +1076,14 @@ fn handle_endpoint_access(
{
// warn!("delete rom bar: can not found 0x{:x}", old_vaddr);
}
gpm.try_insert_quiet(MemoryRegion::new_with_offset_mapper(
new_vaddr_aligned as GuestPhysAddr,
paddr as HostPhysAddr,
rom_size as _,
MemFlags::READ | MemFlags::WRITE,
))?;
guard.insert_passthrough_region_quiet(
MemoryRegion::new_with_offset_mapper(
new_vaddr_aligned as GuestPhysAddr,
paddr as HostPhysAddr,
rom_size as _,
MemFlags::READ | MemFlags::WRITE,
),
)?;
drop(guard);
/* after update gpm, mem barrier is needed
*/
Expand Down Expand Up @@ -1280,12 +1286,14 @@ fn handle_pci_bridge_access(
)
.is_ok()
{}
gpm.try_insert_quiet(MemoryRegion::new_with_offset_mapper(
new_vaddr_aligned as GuestPhysAddr,
paddr as HostPhysAddr,
bar_size as _,
MemFlags::READ | MemFlags::WRITE,
))?;
guard.insert_passthrough_region_quiet(
MemoryRegion::new_with_offset_mapper(
new_vaddr_aligned as GuestPhysAddr,
paddr as HostPhysAddr,
bar_size as _,
MemFlags::READ | MemFlags::WRITE,
),
)?;
}
drop(guard);
#[cfg(target_arch = "aarch64")]
Expand Down Expand Up @@ -1401,12 +1409,14 @@ fn handle_pci_bridge_access(
// warn!("delete bar {}: can not found 0x{:x}", slot, old_vaddr);
}
// Insert new gpm mapping at new address
gpm.try_insert_quiet(MemoryRegion::new_with_offset_mapper(
new_vaddr_aligned as GuestPhysAddr,
paddr as HostPhysAddr,
bar_size as _,
MemFlags::READ | MemFlags::WRITE,
))?;
guard.insert_passthrough_region_quiet(
MemoryRegion::new_with_offset_mapper(
new_vaddr_aligned as GuestPhysAddr,
paddr as HostPhysAddr,
bar_size as _,
MemFlags::READ | MemFlags::WRITE,
),
)?;
}
drop(guard);
/* after update gpm, mem barrier is needed
Expand Down Expand Up @@ -1543,12 +1553,14 @@ fn handle_pci_bridge_access(
{
// warn!("delete rom bar: can not found 0x{:x}", old_vaddr);
}
gpm.try_insert_quiet(MemoryRegion::new_with_offset_mapper(
new_vaddr_aligned as GuestPhysAddr,
paddr as HostPhysAddr,
rom_size as _,
MemFlags::READ | MemFlags::WRITE,
))?;
guard.insert_passthrough_region_quiet(
MemoryRegion::new_with_offset_mapper(
new_vaddr_aligned as GuestPhysAddr,
paddr as HostPhysAddr,
rom_size as _,
MemFlags::READ | MemFlags::WRITE,
),
)?;
drop(guard);
/* after update gpm, mem barrier is needed
*/
Expand Down
34 changes: 33 additions & 1 deletion src/zone.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use crate::config::{HvZoneBootMode, HvZoneConfig, CONFIG_NAME_MAXLEN};
use crate::cpu_data::{get_cpu_data, this_zone, CpuSet};
use crate::error::HvResult;
use crate::memory::addr::GuestPhysAddr;
use crate::memory::{MMIOConfig, MMIOHandler, MMIORegion, MemorySet};
use crate::memory::{MMIOConfig, MMIOHandler, MMIORegion, MemoryRegion, MemorySet};
use core::panic;
use core::sync::atomic::{AtomicBool, Ordering};

Expand Down Expand Up @@ -221,6 +221,15 @@ impl ZoneInner {
handler: MMIOHandler,
arg: usize,
) {
// TODO: add error handling instead of just warning.
// See https://github.com/syswonder/hvisor/issues/385
if self.gpm.is_range_overlap(start, size) {
warn!(
"MMIO handler region [{:#x}, {:#x}) overlaps with passthrough region",
start,
start + size
);
}
if let Some(mmio) = self.mmio.iter_mut().find(|mmio| mmio.region.start == start) {
warn!("duplicated mmio region {:#x?}", mmio);
if mmio.region.size != size {
Expand Down Expand Up @@ -259,6 +268,29 @@ impl ZoneInner {
.find(|cfg| cfg.region.contains_region(addr, size))
.map(|cfg| (cfg.region, cfg.handler, cfg.arg))
}
/// Check whether `[start, start+size)` overlaps with any registered MMIO handler region.
pub fn is_mmio_handler_overlap(&self, start: GuestPhysAddr, size: usize) -> bool {
let region = MMIORegion { start, size };
self.mmio
.iter()
.any(|cfg| cfg.region.is_overlap_with(&region))
}
/// Insert a passthrough region, warning if it overlaps an MMIO handler region.
pub fn insert_passthrough_region_quiet(
&mut self,
region: MemoryRegion<GuestPhysAddr>,
) -> HvResult {
// TODO: add error handling instead of just warning.
// See https://github.com/syswonder/hvisor/issues/385
if self.is_mmio_handler_overlap(region.start, region.size) {
warn!(
"passthrough region [{:#x}, {:#x}) overlaps with MMIO handler region",
region.start,
region.start + region.size
);
}
self.gpm.try_insert_quiet(region)
}
/// If irq_id belongs to this zone
pub fn irq_in_zone(&self, irq_id: u32) -> bool {
let idx = (irq_id / 32) as usize;
Expand Down
Loading