From bb8a2a225a6e09e387ba506595f4513b79423d06 Mon Sep 17 00:00:00 2001 From: Jack Schefer Date: Wed, 8 Oct 2025 12:51:58 -0700 Subject: [PATCH 1/2] vmm_core/vmotherboard: chipset bus resolution for pcie enumerators and downstream ports --- Cargo.lock | 1 + openvmm/hvlite_core/src/worker/dispatch.rs | 3 + vmm_core/vmotherboard/Cargo.toml | 1 + vmm_core/vmotherboard/src/base_chipset.rs | 24 ++++++ .../src/chipset/backing/arc_mutex/device.rs | 82 +++++++++++-------- .../src/chipset/backing/arc_mutex/pci.rs | 79 ++++++++++++++++++ .../src/chipset/backing/arc_mutex/services.rs | 9 ++ .../src/chipset/builder/errors.rs | 4 + .../vmotherboard/src/chipset/builder/mod.rs | 67 ++++++++++++++- vmm_core/vmotherboard/src/chipset/mod.rs | 41 ++++++++++ vmm_core/vmotherboard/src/lib.rs | 11 +++ 11 files changed, 289 insertions(+), 33 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7d303f2de1..0a7c84e4fc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9354,6 +9354,7 @@ dependencies = [ "parking_lot", "paste", "pci_bus", + "pcie", "range_map_vec", "state_unit", "thiserror 2.0.16", diff --git a/openvmm/hvlite_core/src/worker/dispatch.rs b/openvmm/hvlite_core/src/worker/dispatch.rs index ed6110e9df..ea0163bcb5 100644 --- a/openvmm/hvlite_core/src/worker/dispatch.rs +++ b/openvmm/hvlite_core/src/worker/dispatch.rs @@ -1796,6 +1796,9 @@ impl InitializedVm { ), }); + let bus_id = vmotherboard::BusId::new(&rc.name); + chipset_builder.register_weak_mutex_pcie_enumerator(bus_id, Box::new(root_complex)); + ecam_address += ecam_size; low_mmio_address -= low_mmio_size; high_mmio_address += rc.high_mmio_size; diff --git a/vmm_core/vmotherboard/Cargo.toml b/vmm_core/vmotherboard/Cargo.toml index 6b5f6c7cae..ce1bc8d2cd 100644 --- a/vmm_core/vmotherboard/Cargo.toml +++ b/vmm_core/vmotherboard/Cargo.toml @@ -39,6 +39,7 @@ guest_watchdog.workspace = true ide.workspace = true missing_dev.workspace = true pci_bus.workspace = true +pcie.workspace = true vga_proxy = { optional = true, workspace = true } vga = { optional = true, workspace = true } watchdog_core.workspace = true diff --git a/vmm_core/vmotherboard/src/base_chipset.rs b/vmm_core/vmotherboard/src/base_chipset.rs index 96ac2f979b..eac1406a76 100644 --- a/vmm_core/vmotherboard/src/base_chipset.rs +++ b/vmm_core/vmotherboard/src/base_chipset.rs @@ -797,7 +797,10 @@ impl ConfigureChipsetDevice for ArcMutexChipsetServices<'_, '_> { mod weak_mutex_pci { use crate::chipset::PciConflict; use crate::chipset::PciConflictReason; + use crate::chipset::PcieConflict; + use crate::chipset::PcieConflictReason; use crate::chipset::backing::arc_mutex::pci::RegisterWeakMutexPci; + use crate::chipset::backing::arc_mutex::pci::RegisterWeakMutexPcie; use chipset_device::ChipsetDevice; use chipset_device::io::IoResult; use closeable_mutex::CloseableMutex; @@ -885,6 +888,27 @@ mod weak_mutex_pci { }) } } + + // wiring to enable using the generic PCIe root port alongside the Arc+CloseableMutex device infra + impl RegisterWeakMutexPcie for Arc> { + fn add_pcie_device( + &mut self, + port: u8, + name: Arc, + dev: Weak>, + ) -> Result<(), PcieConflict> { + self.lock() + .add_pcie_device(port, name.clone(), WeakMutexPciDeviceWrapper(dev)) + .map_err(|existing_dev_name| PcieConflict { + reason: PcieConflictReason::ExistingDev(existing_dev_name), + conflict_dev: name, + }) + } + + fn downstream_ports(&self) -> Vec<(u8, Arc)> { + self.lock().downstream_ports() + } + } } pub struct ArcMutexIsaDmaChannel { diff --git a/vmm_core/vmotherboard/src/chipset/backing/arc_mutex/device.rs b/vmm_core/vmotherboard/src/chipset/backing/arc_mutex/device.rs index 47728ba1e9..c1402f7eba 100644 --- a/vmm_core/vmotherboard/src/chipset/backing/arc_mutex/device.rs +++ b/vmm_core/vmotherboard/src/chipset/backing/arc_mutex/device.rs @@ -6,6 +6,7 @@ use super::services::ArcMutexChipsetServices; use crate::BusIdPci; +use crate::BusIdPcieDownstreamPort; use crate::VmmChipsetDevice; use arc_cyclic_builder::ArcCyclicBuilder; use arc_cyclic_builder::ArcCyclicBuilderExt; @@ -71,6 +72,7 @@ pub struct ArcMutexChipsetDeviceBuilder<'a, 'b, T> { pci_addr: Option<(u8, u8, u8)>, pci_bus_id: Option, + pcie_port: Option, external_pci: bool, } @@ -97,6 +99,7 @@ where pci_addr: None, pci_bus_id: None, + pcie_port: None, external_pci: false, } } @@ -120,6 +123,12 @@ where self } + /// For PCIe devices: place the device on the specified downstream port + pub fn on_pcie_port(mut self, id: BusIdPcieDownstreamPort) -> Self { + self.pcie_port = Some(id); + self + } + /// For PCI devices: do not register the device with any PCI bus. This is /// used when the device is hooked up to a bus (such as a VPCI bus) outside /// of the vmotherboard infrastructure. @@ -156,39 +165,48 @@ where if !self.external_pci { if let Some(dev) = typed_dev.supports_pci() { - // static pci registration - let bdf = match (self.pci_addr, dev.suggested_bdf()) { - (Some(override_bdf), Some(suggested_bdf)) => { - let (ob, od, of) = override_bdf; - let (sb, sd, sf) = suggested_bdf; - tracing::info!( - "overriding suggested bdf: using {:02x}:{:02x}:{} instead of {:02x}:{:02x}:{}", - ob, - od, - of, - sb, - sd, - sf - ); - override_bdf - } - (None, Some(bdf)) | (Some(bdf), None) => bdf, - (None, None) => { - return Err( - AddDeviceErrorKind::NoPciBusAddress.with_dev_name(self.dev_name) - ); - } - }; - - let bus_id = match self.pci_bus_id.take() { - Some(bus_id) => bus_id, - None => panic!( - "wiring error: did not invoke `on_pci_bus` for `{}`", + if self.pci_bus_id.is_some() && self.pcie_port.is_some() { + panic!( + "wiring error: invoked both `on_pci_bus` and `on_pcie_port` for `{}`", self.dev_name - ), - }; - - self.services.register_static_pci(bus_id, bdf); + ); + } else if let Some(bus_id_port) = self.pcie_port { + self.services.register_static_pcie(bus_id_port); + } else { + // static pci registration + let bdf = match (self.pci_addr, dev.suggested_bdf()) { + (Some(override_bdf), Some(suggested_bdf)) => { + let (ob, od, of) = override_bdf; + let (sb, sd, sf) = suggested_bdf; + tracing::info!( + "overriding suggested bdf: using {:02x}:{:02x}:{} instead of {:02x}:{:02x}:{}", + ob, + od, + of, + sb, + sd, + sf + ); + override_bdf + } + (None, Some(bdf)) | (Some(bdf), None) => bdf, + (None, None) => { + return Err( + AddDeviceErrorKind::NoPciBusAddress.with_dev_name(self.dev_name) + ); + } + }; + + let bus_id = match self.pci_bus_id.take() { + Some(bus_id) => bus_id, + None => panic!( + "wiring error: did not invoke `on_pci_bus` for `{}`", + self.dev_name + ), + }; + + self.services.register_static_pci(bus_id, bdf); + } } } diff --git a/vmm_core/vmotherboard/src/chipset/backing/arc_mutex/pci.rs b/vmm_core/vmotherboard/src/chipset/backing/arc_mutex/pci.rs index 7f514ca0c5..482bbdedb9 100644 --- a/vmm_core/vmotherboard/src/chipset/backing/arc_mutex/pci.rs +++ b/vmm_core/vmotherboard/src/chipset/backing/arc_mutex/pci.rs @@ -2,8 +2,12 @@ // Licensed under the MIT License. use crate::BusIdPci; +use crate::BusIdPcieDownstreamPort; +use crate::BusIdPcieEnumerator; use crate::chipset::PciConflict; use crate::chipset::PciConflictReason; +use crate::chipset::PcieConflict; +use crate::chipset::PcieConflictReason; use chipset_device::ChipsetDevice; use closeable_mutex::CloseableMutex; use std::collections::HashMap; @@ -68,3 +72,78 @@ impl BusResolverWeakMutexPci { if !errs.is_empty() { Err(errs) } else { Ok(()) } } } + +/// An abstraction over an upstream PCIe enumerator implementation that +/// is able to route accesses to `Weak>` +/// devices via downstream ports. +pub trait RegisterWeakMutexPcie: Send { + /// Try to add a PCIe device to the enumerator at the sepcified port, + /// reporting any conflicts. + fn add_pcie_device( + &mut self, + port: u8, + name: Arc, + device: Weak>, + ) -> Result<(), PcieConflict>; + + /// Enumerate the downstream ports. + fn downstream_ports(&self) -> Vec<(u8, Arc)>; +} + +pub struct WeakMutexPcieDeviceEntry { + pub bus_id_port: BusIdPcieDownstreamPort, + pub name: Arc, + pub dev: Weak>, +} + +#[derive(Default)] +pub struct BusResolverWeakMutexPcie { + pub enumerators: HashMap>, + pub ports: HashMap, + pub devices: Vec, +} + +impl BusResolverWeakMutexPcie { + pub fn resolve(mut self) -> Result<(), Vec> { + let mut errs = Vec::new(); + + for WeakMutexPcieDeviceEntry { + bus_id_port, + name, + dev, + } in self.devices + { + let (port_number, bus_id_enumerator) = match self.ports.get(&bus_id_port) { + Some(v) => v, + None => { + errs.push(PcieConflict { + conflict_dev: name.clone(), + reason: PcieConflictReason::MissingDownstreamPort, + }); + continue; + } + }; + + let enumerator = match self.enumerators.get_mut(bus_id_enumerator) { + Some(enumerator) => enumerator, + None => { + errs.push(PcieConflict { + conflict_dev: name.clone(), + reason: PcieConflictReason::MissingEnumerator, + }); + continue; + } + }; + + match enumerator.add_pcie_device(*port_number, name, dev) { + Ok(()) => {} + Err(conflict) => { + errs.push(conflict); + continue; + } + }; + } + + if !errs.is_empty() { Err(errs) } else { Ok(()) } + } +} diff --git a/vmm_core/vmotherboard/src/chipset/backing/arc_mutex/services.rs b/vmm_core/vmotherboard/src/chipset/backing/arc_mutex/services.rs index 04393233de..a6a578d08e 100644 --- a/vmm_core/vmotherboard/src/chipset/backing/arc_mutex/services.rs +++ b/vmm_core/vmotherboard/src/chipset/backing/arc_mutex/services.rs @@ -7,6 +7,7 @@ use self::device_range::DeviceRangeMapper; use super::device::ArcMutexChipsetServicesFinalize; use super::state_unit::ArcMutexChipsetDeviceUnit; use crate::BusIdPci; +use crate::BusIdPcieDownstreamPort; use crate::ChipsetBuilder; use crate::VmmChipsetDevice; use crate::chipset::io_ranges::IoRanges; @@ -182,6 +183,14 @@ impl<'a, 'b> ArcMutexChipsetServices<'a, 'b> { ); } + pub fn register_static_pcie(&mut self, bus_id: BusIdPcieDownstreamPort) { + self.builder.register_weak_mutex_pcie_device( + bus_id, + self.dev_name.clone(), + self.dev.clone(), + ); + } + pub fn new_line(&mut self, id: LineSetId, name: &str, vector: u32) -> LineInterrupt { let (line_set, _) = self.builder.line_set(id.clone()); match line_set.new_line(vector, format!("{}:{}", self.dev_name, name)) { diff --git a/vmm_core/vmotherboard/src/chipset/builder/errors.rs b/vmm_core/vmotherboard/src/chipset/builder/errors.rs index 83b16d9cfd..4f0bbafa2d 100644 --- a/vmm_core/vmotherboard/src/chipset/builder/errors.rs +++ b/vmm_core/vmotherboard/src/chipset/builder/errors.rs @@ -2,6 +2,7 @@ // Licensed under the MIT License. use crate::chipset::PciConflict; +use crate::chipset::PcieConflict; use crate::chipset::io_ranges::IoRangeConflict; use std::fmt::Debug; use thiserror::Error; @@ -21,6 +22,9 @@ pub enum ChipsetBuilderError { /// detected static pci address conflict #[error("static pci conflict: {0}")] PciConflict(PciConflict), + /// detected static pcie port conflict + #[error("static pcie port conflict: {0}")] + PcieConflict(PcieConflict), } #[derive(Debug, Error)] diff --git a/vmm_core/vmotherboard/src/chipset/builder/mod.rs b/vmm_core/vmotherboard/src/chipset/builder/mod.rs index 399299b6da..ba8daa909d 100644 --- a/vmm_core/vmotherboard/src/chipset/builder/mod.rs +++ b/vmm_core/vmotherboard/src/chipset/builder/mod.rs @@ -10,11 +10,17 @@ use self::errors::ErrorListExt; use self::errors::FinalChipsetBuilderError; use super::backing::arc_mutex::device::ArcMutexChipsetDeviceBuilder; use super::backing::arc_mutex::pci::BusResolverWeakMutexPci; +use super::backing::arc_mutex::pci::BusResolverWeakMutexPcie; use super::backing::arc_mutex::pci::RegisterWeakMutexPci; +use super::backing::arc_mutex::pci::RegisterWeakMutexPcie; use super::backing::arc_mutex::pci::WeakMutexPciEntry; +use super::backing::arc_mutex::pci::WeakMutexPcieDeviceEntry; use super::backing::arc_mutex::services::ArcMutexChipsetServices; use super::backing::arc_mutex::state_unit::ArcMutexChipsetDeviceUnit; +use crate::BusId; use crate::BusIdPci; +use crate::BusIdPcieDownstreamPort; +use crate::BusIdPcieEnumerator; use crate::DebugEventHandler; use crate::VmmChipsetDevice; use crate::chipset::Chipset; @@ -97,6 +103,7 @@ impl DynamicDeviceUnit { #[derive(Default)] pub(crate) struct BusResolver { pci: BusResolverWeakMutexPci, + pcie: BusResolverWeakMutexPcie, } /// A builder for [`Chipset`] @@ -198,6 +205,55 @@ impl<'a> ChipsetBuilder<'a> { .push(WeakMutexPciEntry { bdf, name, dev }); } + /// Register a PCIe enumerator (ex. root complex or switch), and all of + /// it's downstream ports. + pub fn register_weak_mutex_pcie_enumerator( + &mut self, + bus_id: BusIdPcieEnumerator, + enumerator: Box, + ) { + let downstream_ports = enumerator.downstream_ports(); + let existing = self + .bus_resolver + .pcie + .enumerators + .insert(bus_id.clone(), enumerator); + assert!( + existing.is_none(), + "duplicate pcie enumerator ID: {:?}", + bus_id + ); + + for (port_number, port_name) in downstream_ports { + let existing = self + .bus_resolver + .pcie + .ports + .insert(BusId::new(&port_name), (port_number, bus_id.clone())); + assert!( + existing.is_none(), + "duplicate pcie port ID: {:?}", + port_name + ); + } + } + + pub(crate) fn register_weak_mutex_pcie_device( + &mut self, + bus_id_port: BusIdPcieDownstreamPort, + name: Arc, + dev: Weak>, + ) { + self.bus_resolver + .pcie + .devices + .push(WeakMutexPcieDeviceEntry { + bus_id_port, + name, + dev, + }); + } + pub(crate) fn line_set( &mut self, id: LineSetId, @@ -253,7 +309,7 @@ impl<'a> ChipsetBuilder<'a> { } { - let BusResolver { pci } = self.bus_resolver; + let BusResolver { pci, pcie } = self.bus_resolver; match pci.resolve() { Ok(()) => {} @@ -263,6 +319,15 @@ impl<'a> ChipsetBuilder<'a> { } } } + + match pcie.resolve() { + Ok(()) => {} + Err(conflicts) => { + for conflict in conflicts { + errs.append(ChipsetBuilderError::PcieConflict(conflict)); + } + } + } } if let Some(err) = errs { diff --git a/vmm_core/vmotherboard/src/chipset/mod.rs b/vmm_core/vmotherboard/src/chipset/mod.rs index e5009474d7..89b332e9bc 100644 --- a/vmm_core/vmotherboard/src/chipset/mod.rs +++ b/vmm_core/vmotherboard/src/chipset/mod.rs @@ -323,3 +323,44 @@ impl std::fmt::Display for PciConflict { } } } + +#[derive(Debug)] +pub enum PcieConflictReason { + ExistingDev(Arc), + MissingDownstreamPort, + MissingEnumerator, +} + +#[derive(Debug)] +pub struct PcieConflict { + pub conflict_dev: Arc, + pub reason: PcieConflictReason, +} + +impl std::fmt::Display for PcieConflict { + fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match &self.reason { + PcieConflictReason::ExistingDev(existing_dev) => { + write!( + fmt, + "cannot attach {}, port already occupied by {}", + self.conflict_dev, existing_dev + ) + } + PcieConflictReason::MissingDownstreamPort => { + write!( + fmt, + "cannot attach {}, no valid pcie downstream port", + self.conflict_dev + ) + } + PcieConflictReason::MissingEnumerator => { + write!( + fmt, + "cannot attach {}, no valid pcie enumerator", + self.conflict_dev + ) + } + } + } +} diff --git a/vmm_core/vmotherboard/src/lib.rs b/vmm_core/vmotherboard/src/lib.rs index 3639f5a769..b2c1a68480 100644 --- a/vmm_core/vmotherboard/src/lib.rs +++ b/vmm_core/vmotherboard/src/lib.rs @@ -95,11 +95,22 @@ impl BusId { pub mod bus_kind { #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] pub enum Pci {} + #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] + pub enum PcieEnumerator {} + #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)] + pub enum PcieDownstreamPort {} } /// Type-safe PCI bus ID. pub type BusIdPci = BusId; +/// Type-safe ID for the internal "bus" of a PCIe root +/// complex or switch. +pub type BusIdPcieEnumerator = BusId; + +/// Type-safe ID for a downstream PCIe port. +pub type BusIdPcieDownstreamPort = BusId; + /// A handle to instantiate a chipset device. #[derive(MeshPayload, Debug)] pub struct ChipsetDeviceHandle { From f5532327298c44f842ba8c1d81380ba6b0446820 Mon Sep 17 00:00:00 2001 From: Jack Schefer Date: Wed, 8 Oct 2025 14:08:21 -0700 Subject: [PATCH 2/2] cr: copilot suggestions --- vmm_core/vmotherboard/src/chipset/backing/arc_mutex/device.rs | 4 +++- vmm_core/vmotherboard/src/chipset/backing/arc_mutex/pci.rs | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/vmm_core/vmotherboard/src/chipset/backing/arc_mutex/device.rs b/vmm_core/vmotherboard/src/chipset/backing/arc_mutex/device.rs index c1402f7eba..07820e95bc 100644 --- a/vmm_core/vmotherboard/src/chipset/backing/arc_mutex/device.rs +++ b/vmm_core/vmotherboard/src/chipset/backing/arc_mutex/device.rs @@ -170,7 +170,9 @@ where "wiring error: invoked both `on_pci_bus` and `on_pcie_port` for `{}`", self.dev_name ); - } else if let Some(bus_id_port) = self.pcie_port { + } + + if let Some(bus_id_port) = self.pcie_port { self.services.register_static_pcie(bus_id_port); } else { // static pci registration diff --git a/vmm_core/vmotherboard/src/chipset/backing/arc_mutex/pci.rs b/vmm_core/vmotherboard/src/chipset/backing/arc_mutex/pci.rs index 482bbdedb9..c6df3cde7e 100644 --- a/vmm_core/vmotherboard/src/chipset/backing/arc_mutex/pci.rs +++ b/vmm_core/vmotherboard/src/chipset/backing/arc_mutex/pci.rs @@ -77,7 +77,7 @@ impl BusResolverWeakMutexPci { /// is able to route accesses to `Weak>` /// devices via downstream ports. pub trait RegisterWeakMutexPcie: Send { - /// Try to add a PCIe device to the enumerator at the sepcified port, + /// Try to add a PCIe device to the enumerator at the specified port, /// reporting any conflicts. fn add_pcie_device( &mut self,