vmm_core/vmotherboard: chipset bus resolution for pcie enumerators and downstream ports (#2122)

This change introduces `vmotherboard` support in OpenVMM for wiring PCIe
topology components together. This PR builds on previous changes to
introduce root complex and root port emulators, and in subsequent PRs
will be used to wire endpoint devices up to their parent devices. For a
view of how this fits together, see #1976

Specifially, this change:
- Introduces two `vmotherboard::BusId` types. which are separate because
they have distinct wiring requirements.
- The first such type represents a multi-port upstream device such as a
root complex or in the future a switch. These devices represent an
internal bus, with multiple downstream ports
- The second such type represents each downstream port of the multi-port
devices, at most one downstream device can be attached to each
downstream port.
- Adds a bus resolver to the motherboard, with associated error handling
types, to facilitate the motherboard connecting upstream and downstream
devices based on port names
- These are modeled off of the legacy PCI bus resolution architecture,
though had to be separate. The legacy PCI bus infrastructure models a
multi-device bus, where BDFs are statically defined at VM-start, whereas
the PCIe bus infrastructure models both multi-port semantics for
upstream devices and single-parent semantics for the PCIe links and
resolution is done based on string identifiers not BDFs
- When an enumerator is registered with the motherboard, the motherboard
queries the downstream ports of the enumerator and tracks them
internally for resolution
- Updates the VM worker to register PCIe root complexes with this new
resolution architecture.

Author: jackschefer-msft

Cargo.lock

@@ -9360,6 +9360,7 @@ dependencies = [
  "parking_lot",
  "paste",
  "pci_bus",
+ "pcie",
  "range_map_vec",
  "state_unit",
  "thiserror 2.0.16",

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;

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

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<CloseableMutex<pcie::root::GenericPcieRootComplex>> {
+        fn add_pcie_device(
+            &mut self,
+            port: u8,
+            name: Arc<str>,
+            dev: Weak<CloseableMutex<dyn ChipsetDevice>>,
+        ) -> 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<str>)> {
+            self.lock().downstream_ports()
+        }
+    }
 }
 
 pub struct ArcMutexIsaDmaChannel {

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<BusIdPci>,
+    pcie_port: Option<BusIdPcieDownstreamPort>,
     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,50 @@ 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);
+                    );
+                }
+
+                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);
+                }
             }
         }
 

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<CloseableMutex<dyn ChipsetDevice>>`
+/// devices via downstream ports.
+pub trait RegisterWeakMutexPcie: Send {
+    /// Try to add a PCIe device to the enumerator at the specified port,
+    /// reporting any conflicts.
+    fn add_pcie_device(
+        &mut self,
+        port: u8,
+        name: Arc<str>,
+        device: Weak<CloseableMutex<dyn ChipsetDevice>>,
+    ) -> Result<(), PcieConflict>;
+
+    /// Enumerate the downstream ports.
+    fn downstream_ports(&self) -> Vec<(u8, Arc<str>)>;
+}
+
+pub struct WeakMutexPcieDeviceEntry {
+    pub bus_id_port: BusIdPcieDownstreamPort,
+    pub name: Arc<str>,
+    pub dev: Weak<CloseableMutex<dyn ChipsetDevice>>,
+}
+
+#[derive(Default)]
+pub struct BusResolverWeakMutexPcie {
+    pub enumerators: HashMap<BusIdPcieEnumerator, Box<dyn RegisterWeakMutexPcie>>,
+    pub ports: HashMap<BusIdPcieDownstreamPort, (u8, BusIdPcieEnumerator)>,
+    pub devices: Vec<WeakMutexPcieDeviceEntry>,
+}
+
+impl BusResolverWeakMutexPcie {
+    pub fn resolve(mut self) -> Result<(), Vec<PcieConflict>> {
+        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(()) }
+    }
+}

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)) {

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)]