From bc1910e17819ce24114bb575b67276d58b010df5 Mon Sep 17 00:00:00 2001 From: Chris Oo Date: Wed, 29 Oct 2025 16:36:15 -0700 Subject: [PATCH] vm/devices/vpci: track vtom and register for mmio with vtom (#2306) On hardware isolated guests with vtom, the guest may access vpci config space with vtom set. Add this tracking in vpci and register for mmio with vtom, so we can correctly handle guest accesses on SNP & TDX. This fixes vpci relay on TDX, as both Linux and Windows will access config space with vtom set. --- openhcl/underhill_core/src/worker.rs | 2 ++ openvmm/hvlite_core/src/worker/dispatch.rs | 3 ++ vm/devices/pci/vpci/src/bus.rs | 23 +++++++++++++++ vm/devices/pci/vpci/src/device.rs | 34 +++++++++++++++++++--- vm/devices/pci/vpci_client/src/tests.rs | 1 + vm/devices/pci/vpci_relay/src/lib.rs | 5 ++++ vmm_core/src/device_builder.rs | 2 ++ 7 files changed, 66 insertions(+), 4 deletions(-) diff --git a/openhcl/underhill_core/src/worker.rs b/openhcl/underhill_core/src/worker.rs index fce713e242..d35ed09fb8 100644 --- a/openhcl/underhill_core/src/worker.rs +++ b/openhcl/underhill_core/src/worker.rs @@ -2861,6 +2861,7 @@ async fn new_underhill_vm( .context("failed to create direct mmio accessor")?, ) }, + vtom, ); // Allow NVMe devices. @@ -2990,6 +2991,7 @@ async fn new_underhill_vm( let device = Arc::new(device); Ok((device.clone(), VpciInterruptMapper::new(device))) }, + vtom, ) .await?; } diff --git a/openvmm/hvlite_core/src/worker/dispatch.rs b/openvmm/hvlite_core/src/worker/dispatch.rs index 016a4499c1..40c6d5ef2b 100644 --- a/openvmm/hvlite_core/src/worker/dispatch.rs +++ b/openvmm/hvlite_core/src/worker/dispatch.rs @@ -1833,6 +1833,7 @@ impl InitializedVm { &mut mmio, vmbus.control().as_ref(), interrupt_mapper, + None, ) .await?; @@ -1947,6 +1948,7 @@ impl InitializedVm { hv_device.clone().interrupt_mapper(), )) }, + None, ) .await?; } @@ -1988,6 +1990,7 @@ impl InitializedVm { &mut services.register_mmio(), vmbus, crate::partition::VpciDevice::interrupt_mapper(hv_device), + None, ) .await }) diff --git a/vm/devices/pci/vpci/src/bus.rs b/vm/devices/pci/vpci/src/bus.rs index 9b32641538..4730bc0a6f 100644 --- a/vm/devices/pci/vpci/src/bus.rs +++ b/vm/devices/pci/vpci/src/bus.rs @@ -7,6 +7,7 @@ use crate::device::NotPciDevice; use crate::device::VpciChannel; use crate::device::VpciConfigSpace; use crate::device::VpciConfigSpaceOffset; +use crate::device::VpciConfigSpaceVtom; use chipset_device::ChipsetDevice; use chipset_device::io::IoError; use chipset_device::io::IoResult; @@ -57,6 +58,9 @@ pub struct VpciBusDevice { config_space_offset: VpciConfigSpaceOffset, #[inspect(with = "|&x| u32::from(x)")] current_slot: SlotNumber, + /// Track vtom as when isolated with vtom enabled, guests may access mmio + /// with or without vtom set. + vtom: Option, } /// An error creating a VPCI bus. @@ -78,9 +82,15 @@ impl VpciBusDevice { device: Arc>, register_mmio: &mut dyn RegisterMmioIntercept, msi_controller: VpciInterruptMapper, + vtom: Option, ) -> Result<(Self, VpciChannel), NotPciDevice> { let config_space = VpciConfigSpace::new( register_mmio.new_io_region(&format!("vpci-{instance_id}-config"), 2 * HV_PAGE_SIZE), + vtom.map(|vtom| VpciConfigSpaceVtom { + vtom, + control_mmio: register_mmio + .new_io_region(&format!("vpci-{instance_id}-config-vtom"), 2 * HV_PAGE_SIZE), + }), ); let config_space_offset = config_space.offset().clone(); let channel = VpciChannel::new(&device, instance_id, config_space, msi_controller)?; @@ -89,6 +99,7 @@ impl VpciBusDevice { device, config_space_offset, current_slot: SlotNumber::from(0), + vtom, }; Ok((this, channel)) @@ -104,12 +115,14 @@ impl VpciBus { register_mmio: &mut dyn RegisterMmioIntercept, vmbus: &dyn vmbus_channel::bus::ParentBus, msi_controller: VpciInterruptMapper, + vtom: Option, ) -> Result { let (bus, channel) = VpciBusDevice::new( instance_id, device.clone(), register_mmio, msi_controller.clone(), + vtom, ) .map_err(CreateBusError::NotPci)?; let channel = offer_simple_device(driver_source, vmbus, channel) @@ -164,6 +177,11 @@ impl ChipsetDevice for VpciBusDevice { impl MmioIntercept for VpciBusDevice { fn mmio_read(&mut self, addr: u64, data: &mut [u8]) -> IoResult { + tracing::trace!(addr, "VPCI bus MMIO read"); + + // Remove vtom, as the guest may access it with or without set. + let addr = addr & !self.vtom.unwrap_or(0); + let reg = match self.register(addr, data.len()) { Ok(reg) => reg, Err(err) => return IoResult::Err(err), @@ -192,6 +210,11 @@ impl MmioIntercept for VpciBusDevice { } fn mmio_write(&mut self, addr: u64, data: &[u8]) -> IoResult { + tracing::trace!(addr, "VPCI bus MMIO write"); + + // Remove vtom, as the guest may access it with or without set. + let addr = addr & !self.vtom.unwrap_or(0); + let reg = match self.register(addr, data.len()) { Ok(reg) => reg, Err(err) => return IoResult::Err(err), diff --git a/vm/devices/pci/vpci/src/device.rs b/vm/devices/pci/vpci/src/device.rs index 2e5d9cf655..748550e3b6 100644 --- a/vm/devices/pci/vpci/src/device.rs +++ b/vm/devices/pci/vpci/src/device.rs @@ -1048,14 +1048,27 @@ pub struct VpciChannel { pub struct VpciConfigSpace { offset: VpciConfigSpaceOffset, control_mmio: Box, + vtom: Option, +} + +/// The vtom info used by config space. +pub struct VpciConfigSpaceVtom { + /// The vtom bit. + pub vtom: u64, + /// The mmio control region to be registered with vtom. + pub control_mmio: Box, } impl VpciConfigSpace { - /// Create New PCI Config space - pub fn new(control_mmio: Box) -> Self { + /// Create New PCI Config space. + pub fn new( + control_mmio: Box, + vtom: Option, + ) -> Self { Self { offset: VpciConfigSpaceOffset::new(), control_mmio, + vtom, } } @@ -1065,13 +1078,22 @@ impl VpciConfigSpace { } fn map(&mut self, addr: u64) { - tracing::debug!(addr, "mapping config space"); + tracing::trace!(addr, "mapping config space"); + + // Remove the vtom bit if set + let vtom_bit = self.vtom.as_ref().map(|v| v.vtom).unwrap_or(0); + let addr = addr & !vtom_bit; + self.offset.0.store(addr, Ordering::Relaxed); self.control_mmio.map(addr); + + if let Some(vtom) = self.vtom.as_mut() { + vtom.control_mmio.map(addr | vtom_bit); + } } fn unmap(&mut self) { - tracing::debug!( + tracing::trace!( addr = self.offset.0.load(Ordering::Relaxed), "unmapping config space" ); @@ -1081,6 +1103,9 @@ impl VpciConfigSpace { // // This is idempotent. See [`impl_device_range!`]. self.control_mmio.unmap(); + if let Some(vtom) = self.vtom.as_mut() { + vtom.control_mmio.unmap(); + } self.offset .0 .store(VpciConfigSpaceOffset::INVALID, Ordering::Relaxed); @@ -1287,6 +1312,7 @@ mod tests { } let config_space = VpciConfigSpace::new( ExternallyManagedMmioIntercepts.new_io_region("test", 2 * HV_PAGE_SIZE), + None, ); let mut state = VpciChannel { msi_mapper: VpciInterruptMapper::new(msi_mapper), diff --git a/vm/devices/pci/vpci_client/src/tests.rs b/vm/devices/pci/vpci_client/src/tests.rs index 2df7e014aa..44d72d0cbf 100644 --- a/vm/devices/pci/vpci_client/src/tests.rs +++ b/vm/devices/pci/vpci_client/src/tests.rs @@ -80,6 +80,7 @@ async fn test_negotiate_version(driver: DefaultDriver) { device, &mut ExternallyManagedMmioIntercepts, VpciInterruptMapper::new(msi_controller), + None, ) .unwrap(); diff --git a/vm/devices/pci/vpci_relay/src/lib.rs b/vm/devices/pci/vpci_relay/src/lib.rs index 1cc9fdd951..cdf736f23c 100644 --- a/vm/devices/pci/vpci_relay/src/lib.rs +++ b/vm/devices/pci/vpci_relay/src/lib.rs @@ -74,6 +74,8 @@ pub struct VpciRelay { mmio_access: Box, #[inspect(iter_by_index)] allowed_devices: Vec, + #[inspect(hex)] + vtom: Option, } #[derive(Inspect)] @@ -157,6 +159,7 @@ impl VpciRelay { dma_client: Arc, mmio_range: MemoryRange, mmio_access: Box, + vtom: Option, ) -> Self { Self { driver_source, @@ -168,6 +171,7 @@ impl VpciRelay { mmio_range, mmio_access, allowed_devices: Vec::new(), + vtom, } } @@ -304,6 +308,7 @@ impl VpciRelay { mmio, self.vmbus.as_ref(), interrupt_mapper, + self.vtom, ) .await?; diff --git a/vmm_core/src/device_builder.rs b/vmm_core/src/device_builder.rs index e702db5852..2adef8e540 100644 --- a/vmm_core/src/device_builder.rs +++ b/vmm_core/src/device_builder.rs @@ -36,6 +36,7 @@ pub async fn build_vpci_device( Arc, VpciInterruptMapper, )>, + vtom: Option, ) -> anyhow::Result<()> { let device_name = format!("{}:vpci-{instance_id}", resource.id()); @@ -82,6 +83,7 @@ pub async fn build_vpci_device( &mut services.register_mmio(), vmbus, interrupt_mapper, + vtom, ) .await?;