From: Dominik Csapak <d.csapak@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH pve-qemu-server-rs v3 4/9] pci: layout: add grouped PCI and PCIe layouts
Date: Fri, 6 Nov 2026 14:25:47 +0100 [thread overview]
Message-ID: <20261106132604.2556840-5-d.csapak@proxmox.com> (raw)
In-Reply-To: <20261106132604.2556840-1-d.csapak@proxmox.com>
The legacy layout grew over many releases and it shows: devices of one
kind are spread over several buses, the bus a device lands on depends on
how many devices of other kinds exist, and the remaining free slots are
scattered. That makes it hard to raise any of the per-kind limits
without further scattering the types over the available addresses.
Add a second set of layouts that gives every device kind its own set of
bridges instead. A device's address then only depends on its own index,
so raising a limit adds bridges at the end rather than shifting anything
that exists, and the built-in devices move to a bridge of their own to
keep the root bus free.
The new `layout` field of `HardwareConfig` selects which set is used,
so `pci_bridge_for_device` now takes the config too. Since the layout
is part of the guest ABI, the grouped one is only intended for new
guests.
`pci_bus_devices` returns its bridges and the hostpcie root ports,
with the same function 0 handling as for the legacy layout.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
pve-qemu-server-pci/src/constants.rs | 9 +
pve-qemu-server-pci/src/error.rs | 3 +
pve-qemu-server-pci/src/hardware_config.rs | 9 +-
pve-qemu-server-pci/src/layout/grouped.rs | 467 +++++++++++++++++++++
pve-qemu-server-pci/src/layout/legacy.rs | 1 +
pve-qemu-server-pci/src/layout/mod.rs | 2 +
pve-qemu-server-pci/src/lib.rs | 56 ++-
pve-qemu-server-pci/src/types/bus.rs | 118 +++++-
pve-qemu-server-pci/src/types/layout.rs | 59 ++-
pve-qemu-server-pci/src/types/mod.rs | 5 +-
10 files changed, 712 insertions(+), 17 deletions(-)
create mode 100644 pve-qemu-server-pci/src/layout/grouped.rs
diff --git a/pve-qemu-server-pci/src/constants.rs b/pve-qemu-server-pci/src/constants.rs
index 21fc805..29e5d87 100644
--- a/pve-qemu-server-pci/src/constants.rs
+++ b/pve-qemu-server-pci/src/constants.rs
@@ -1,6 +1,15 @@
//! Limits the layouts are built around.
+// Upper bounds for the per-guest device counts the layouts reserve slots for,
+// re-exposing the limits enforced by the qemu-server configuration schema.
+pub(crate) const MAX_NET_DEVICES: u8 = pve_api_types::QemuConfigNetArray::MAX as u8;
+pub(crate) const MAX_SCSI_DEVICES: u8 = pve_api_types::QemuConfigScsiArray::MAX as u8;
+pub(crate) const MAX_VIRTIO_BLK_DEVICES: u8 = pve_api_types::QemuConfigVirtioArray::MAX as u8;
+pub(crate) const MAX_VIRTIOFS_DEVICES: u8 = pve_api_types::QemuConfigVirtiofsArray::MAX as u8;
+pub(crate) const MAX_HOSTPCI_DEVICES: u8 = pve_api_types::QemuConfigHostpciArray::MAX as u8;
+
/// Number of slots on a PCI bus. Slot 0x00 is always taken by the bridge
/// itself, so only [`USABLE_SLOTS_PER_BUS`] of them can hold a device.
pub(crate) const SLOTS_PER_BUS: usize = 32;
+pub(crate) const USABLE_SLOTS_PER_BUS: u8 = SLOTS_PER_BUS as u8 - 1;
pub(crate) const FUNCTIONS_PER_DEVICE: usize = 8;
diff --git a/pve-qemu-server-pci/src/error.rs b/pve-qemu-server-pci/src/error.rs
index 5c4f7de..d67b3df 100644
--- a/pve-qemu-server-pci/src/error.rs
+++ b/pve-qemu-server-pci/src/error.rs
@@ -10,6 +10,8 @@ pub enum LayoutError {
InvalidConfigId,
/// The device is not part of the layout it was looked up in.
NoAddressFound(Device),
+ /// The selected layout is not known.
+ InvalidLayout,
}
impl Display for LayoutError {
@@ -17,6 +19,7 @@ impl Display for LayoutError {
match self {
LayoutError::InvalidConfigId => write!(f, "invalid configuration id"),
LayoutError::NoAddressFound(device) => write!(f, "address '{device}' not found"),
+ LayoutError::InvalidLayout => write!(f, "invalid layout"),
}
}
}
diff --git a/pve-qemu-server-pci/src/hardware_config.rs b/pve-qemu-server-pci/src/hardware_config.rs
index 88acb5c..c6352d5 100644
--- a/pve-qemu-server-pci/src/hardware_config.rs
+++ b/pve-qemu-server-pci/src/hardware_config.rs
@@ -6,7 +6,7 @@ use serde::Deserialize;
use pve_api_types::{ClusterResourceHostArch, QemuConfigOstype, QemuConfigScsihw};
-use crate::Device;
+use crate::{Device, PciLayout};
#[derive(Default, Clone, Debug, PartialEq)]
pub enum Mainboard {
@@ -79,6 +79,7 @@ pub struct HardwareConfigParams {
pub minor: u16,
pub pve: Option<u16>,
pub hostpcie_devices: Vec<HostpcieDevice>,
+ pub layout: PciLayout,
}
/// A passed through PCIe device of the guest.
@@ -109,6 +110,9 @@ pub struct HardwareConfig {
pub machine_version: (u16, u16, Option<u16>),
/// The recorded required `+pveX` version
pub required_pve_version: Arc<AtomicU16>,
+ /// The layouts the guest's addresses are taken from. They are part of
+ /// the guest's ABI, so this must not change for an existing guest.
+ pub layout: PciLayout,
/// The configured `hostpcie` devices, keyed by their index, which decide
/// what root ports are needed.
pub hostpcie_devices: BTreeMap<u8, HostpcieDevice>,
@@ -150,6 +154,7 @@ impl HardwareConfig {
ostype,
machine_version: (params.major, params.minor, params.pve),
required_pve_version: Arc::new(AtomicU16::new(0)),
+ layout: params.layout,
hostpcie_devices,
}
}
@@ -170,7 +175,7 @@ impl HardwareConfig {
/// get them from [`crate::pci_bus_devices`], so older guests stay unchanged
/// for live migration.
pub fn uses_q35_config_file(&self) -> bool {
- self.is_q35() && !self.version_at_least(11, 1, Some(1))
+ self.is_q35() && self.layout == PciLayout::Legacy && !self.version_at_least(11, 1, Some(1))
}
/// Whether each SCSI disk gets its own controller, which needs the extra
diff --git a/pve-qemu-server-pci/src/layout/grouped.rs b/pve-qemu-server-pci/src/layout/grouped.rs
new file mode 100644
index 0000000..7ee1458
--- /dev/null
+++ b/pve-qemu-server-pci/src/layout/grouped.rs
@@ -0,0 +1,467 @@
+use crate::constants::{
+ MAX_HOSTPCI_DEVICES, MAX_NET_DEVICES, MAX_SCSI_DEVICES, MAX_VIRTIO_BLK_DEVICES,
+ MAX_VIRTIOFS_DEVICES, SLOTS_PER_BUS,
+};
+use crate::layout::bridge_slots;
+use crate::{
+ Bus, Device, Function, HardwareConfig, Layout, LayoutError, PciAddress, PveBridge, Slot,
+};
+
+/// The guest's built-in devices, which all live on a bridge of their own so
+/// that the root bus stays free for the per-kind bridges.
+///
+/// NOTE: only append here, don't reorder. The order implies the address, which
+/// has to stay stable.
+const BUILTIN_DEVICES: [Slot; SLOTS_PER_BUS] = bridge_slots(&[
+ single_device!(Ahci),
+ single_device!(Balloon),
+ single_device!(XhciController),
+ single_device!(GuestAgent),
+ single_device!(SpiceSerial),
+ single_device!(Rng),
+ single_device!(Audio),
+ single_device!(Watchdog),
+ single_device!(Ivshmem),
+]);
+
+/// The layout for guests with a PCI root bus.
+pub static PCI_LAYOUT: Layout = Layout {
+ pcie: false,
+ root: bridge_slots(&[
+ Slot::Reserved,
+ Slot::Multi(&[
+ Function::Device(Device::Vga(0)),
+ Function::Device(Device::Vga(1)),
+ Function::Device(Device::Vga(2)),
+ Function::Device(Device::Vga(3)),
+ Function::Unused,
+ Function::Unused,
+ Function::Unused,
+ Function::Unused,
+ ]),
+ Slot::Single(Function::Device(Device::Viommu)),
+ Slot::Single(Function::FixedBridge(Bus::Sys(0), &BUILTIN_DEVICES)),
+ Slot::DynamicBridges {
+ kind: PveBridge::VirtioFs,
+ max: MAX_VIRTIOFS_DEVICES,
+ },
+ Slot::DynamicBridges {
+ kind: PveBridge::Net,
+ max: MAX_NET_DEVICES,
+ },
+ Slot::DynamicBridges {
+ kind: PveBridge::Scsi,
+ max: MAX_SCSI_DEVICES,
+ },
+ Slot::DynamicBridges {
+ kind: PveBridge::VirtioBlk,
+ max: MAX_VIRTIO_BLK_DEVICES,
+ },
+ Slot::DynamicBridges {
+ kind: PveBridge::Hostpci,
+ max: MAX_HOSTPCI_DEVICES,
+ },
+ ]),
+};
+
+/// The layout for guests with a PCIe root bus.
+///
+/// Identical to [`PCI_LAYOUT`] except for the root bus type and the root ports
+/// for passed through PCIe devices, which a PCI machine cannot have.
+pub static PCIE_LAYOUT: Layout = Layout {
+ pcie: true,
+ root: [
+ Slot::Reserved,
+ Slot::Reserved,
+ Slot::Multi(&[
+ Function::Device(Device::Vga(0)),
+ Function::Device(Device::Vga(1)),
+ Function::Device(Device::Vga(2)),
+ Function::Device(Device::Vga(3)),
+ Function::Unused,
+ Function::Unused,
+ Function::Unused,
+ Function::Unused,
+ ]),
+ Slot::Single(Function::Device(Device::Viommu)),
+ Slot::Single(Function::FixedBridge(Bus::Sys(0), &BUILTIN_DEVICES)),
+ Slot::DynamicBridges {
+ kind: PveBridge::VirtioFs,
+ max: MAX_VIRTIOFS_DEVICES,
+ },
+ Slot::DynamicBridges {
+ kind: PveBridge::Net,
+ max: MAX_NET_DEVICES,
+ },
+ Slot::DynamicBridges {
+ kind: PveBridge::Scsi,
+ max: MAX_SCSI_DEVICES,
+ },
+ Slot::DynamicBridges {
+ kind: PveBridge::VirtioBlk,
+ max: MAX_VIRTIO_BLK_DEVICES,
+ },
+ Slot::DynamicBridges {
+ kind: PveBridge::Hostpci,
+ max: MAX_HOSTPCI_DEVICES,
+ },
+ Slot::Unused,
+ Slot::Unused,
+ Slot::Unused,
+ Slot::Unused,
+ Slot::Multi(&[
+ Function::RootPort(Bus::RootPort(0), Device::Hostpcie(0)),
+ Function::RootPort(Bus::RootPort(1), Device::Hostpcie(1)),
+ Function::RootPort(Bus::RootPort(2), Device::Hostpcie(2)),
+ Function::RootPort(Bus::RootPort(3), Device::Hostpcie(3)),
+ Function::RootPort(Bus::RootPort(4), Device::Hostpcie(4)),
+ Function::RootPort(Bus::RootPort(5), Device::Hostpcie(5)),
+ Function::RootPort(Bus::RootPort(6), Device::Hostpcie(6)),
+ Function::RootPort(Bus::RootPort(7), Device::Hostpcie(7)),
+ ]),
+ Slot::Multi(&[
+ Function::RootPort(Bus::RootPort(8), Device::Hostpcie(8)),
+ Function::RootPort(Bus::RootPort(9), Device::Hostpcie(9)),
+ Function::RootPort(Bus::RootPort(10), Device::Hostpcie(10)),
+ Function::RootPort(Bus::RootPort(11), Device::Hostpcie(11)),
+ Function::RootPort(Bus::RootPort(12), Device::Hostpcie(12)),
+ Function::RootPort(Bus::RootPort(13), Device::Hostpcie(13)),
+ Function::RootPort(Bus::RootPort(14), Device::Hostpcie(14)),
+ Function::RootPort(Bus::RootPort(15), Device::Hostpcie(15)),
+ ]),
+ Slot::Unused,
+ Slot::Unused,
+ Slot::Unused,
+ Slot::Unused,
+ Slot::Unused,
+ Slot::Unused,
+ Slot::Unused,
+ Slot::Unused,
+ Slot::Unused,
+ Slot::Unused,
+ Slot::Unused,
+ Slot::Unused,
+ Slot::Unused,
+ Slot::Unused,
+ Slot::Unused,
+ Slot::Reserved, // ICH9 South bridge
+ ],
+};
+
+fn layout(pcie: bool) -> &'static Layout<'static> {
+ if pcie { &PCIE_LAYOUT } else { &PCI_LAYOUT }
+}
+
+pub(crate) fn address(hw: &HardwareConfig, id: &str) -> Result<PciAddress, LayoutError> {
+ let layout = layout(hw.has_pcie());
+ layout.find_device(&id.parse()?)
+}
+
+pub(crate) fn pci_bridge_for_device(hw: &HardwareConfig, id: &str) -> Option<Bus> {
+ let addr = address(hw, id).ok()?;
+ Some(addr.bus)
+}
+
+pub(crate) fn pci_bus_devices(hw: &HardwareConfig) -> Result<Vec<String>, LayoutError> {
+ let mut bridges = Vec::new();
+ let layout = layout(hw.has_pcie());
+ let mut chassis_nr = 1;
+ for bridge in layout
+ .iter()
+ .filter(|d| matches!(d.device, Device::Bridge(_)))
+ {
+ let addr = bridge.addr;
+ let (model, id) = match bridge.device {
+ Device::Bridge(bus) => (bus.bridge_model(addr.bus), bus),
+ _ => continue,
+ };
+ let Some(model) = model else {
+ continue;
+ };
+ let mf = if bridge.multifunction {
+ ",multifunction=on"
+ } else {
+ ""
+ };
+
+ let chassis = if model == "pci-bridge" {
+ let text = format!(",chassis_nr={chassis_nr}");
+ chassis_nr += 1;
+ text
+ } else {
+ String::new()
+ };
+ bridges.push("-device".to_string());
+ bridges.push(format!(
+ "{model},id={id}{chassis}{}{mf}",
+ addr.to_qemu_addr()
+ ));
+ }
+
+ for port in layout.root_ports(&hw.hostpcie()) {
+ bridges.push("-device".to_string());
+ bridges.push(port.to_qemu_device());
+ }
+
+ Ok(bridges)
+}
+
+#[cfg(test)]
+mod test {
+ use std::collections::{HashMap, HashSet};
+
+ use strum::VariantArray;
+
+ use super::{PCI_LAYOUT, PCIE_LAYOUT};
+
+ use super::pci_bus_devices;
+ use crate::constants::{
+ MAX_HOSTPCI_DEVICES, MAX_NET_DEVICES, MAX_SCSI_DEVICES, MAX_VIRTIO_BLK_DEVICES,
+ MAX_VIRTIOFS_DEVICES,
+ };
+ use crate::layout::legacy::test::LEGACY_ADDRS;
+ use crate::{
+ Bus, Device, DeviceDiscriminants, HardwareConfig, HardwareConfigParams, HostpcieDevice,
+ Layout, PciAddress, PveBridge,
+ };
+
+ fn check_device(device: Device, res: Option<PciAddress>) {
+ assert_eq!(PCI_LAYOUT.find_device(&device).ok(), res);
+ }
+
+ #[test]
+ fn pci_find_devices_on_dynamic_bridge() {
+ check_device(
+ Device::Net(0),
+ Some(PciAddress::new(PveBridge::Net.new_bus(0), 1, 0)),
+ );
+ check_device(
+ Device::Net(30),
+ Some(PciAddress::new(PveBridge::Net.new_bus(0), 31, 0)),
+ );
+ check_device(
+ Device::Net(31),
+ Some(PciAddress::new(PveBridge::Net.new_bus(1), 1, 0)),
+ );
+ check_device(Device::Net(100), None);
+ }
+
+ #[test]
+ fn pci_find_devices() {
+ check_device(Device::Vga(0), Some(PciAddress::new(Bus::Pci(0), 2, 0)));
+ check_device(Device::Vga(3), Some(PciAddress::new(Bus::Pci(0), 2, 3)));
+ check_device(Device::Audio, Some(PciAddress::new(Bus::Sys(0), 7, 0)));
+ }
+
+ fn test_all_devices_on_layout(layout: &Layout, exclude_devices: &[DeviceDiscriminants]) {
+ let mut set: HashSet<String> = HashSet::new();
+
+ let mut assert_insert = |device: &Device| {
+ let addr = layout
+ .find_device(device)
+ .unwrap_or_else(|_| panic!("could not find address for device {device:?}"));
+ assert!(set.insert(addr.to_qemu_addr()))
+ };
+
+ for variant in DeviceDiscriminants::VARIANTS {
+ if exclude_devices.contains(variant) {
+ continue;
+ }
+ match variant {
+ DeviceDiscriminants::Vga => {
+ for i in 0..4 {
+ assert_insert(&Device::Vga(i));
+ }
+ }
+ DeviceDiscriminants::Viommu => {
+ assert_insert(&Device::Viommu);
+ }
+ DeviceDiscriminants::Net => {
+ for i in 0..MAX_NET_DEVICES {
+ assert_insert(&Device::Net(i));
+ }
+ }
+ DeviceDiscriminants::ScsiController => {
+ for i in 0..MAX_SCSI_DEVICES {
+ assert_insert(&Device::ScsiController(i));
+ }
+ }
+ DeviceDiscriminants::VirtioBlk => {
+ for i in 0..MAX_VIRTIO_BLK_DEVICES {
+ assert_insert(&Device::VirtioBlk(i));
+ }
+ }
+ DeviceDiscriminants::XhciController => {
+ assert_insert(&Device::XhciController);
+ }
+ DeviceDiscriminants::Hostpci => {
+ for i in 0..MAX_HOSTPCI_DEVICES {
+ assert_insert(&Device::Hostpci(i));
+ }
+ }
+ DeviceDiscriminants::Hostpcie => {
+ for i in 0..MAX_HOSTPCI_DEVICES {
+ assert_insert(&Device::Hostpcie(i));
+ }
+ }
+ DeviceDiscriminants::VirtioFs => {
+ for i in 0..MAX_VIRTIOFS_DEVICES {
+ assert_insert(&Device::VirtioFs(i));
+ }
+ }
+ DeviceDiscriminants::Ich9Ehci
+ | DeviceDiscriminants::Ich9Uhci
+ | DeviceDiscriminants::Ich9Audio => {
+ // only part of the legacy q35 chipset
+ }
+ DeviceDiscriminants::Piix3Controller => {
+ // does not exist in grouped layout
+ }
+ DeviceDiscriminants::Bridge | DeviceDiscriminants::RootPort => {
+ // TODO: what to test?
+ }
+ other => {
+ assert_insert(&(*other).try_into().expect("invalid variant"));
+ }
+ }
+ }
+ }
+
+ #[test]
+ /// Tests that each slot is only given once
+ fn pci_test_no_duplicate_addresses() {
+ test_all_devices_on_layout(&PCI_LAYOUT, &[DeviceDiscriminants::Hostpcie]);
+ }
+
+ #[test]
+ /// Tests that each slot is only given once
+ fn pcie_test_no_duplicate_addresses() {
+ test_all_devices_on_layout(&PCIE_LAYOUT, &[]);
+ }
+
+ #[test]
+ fn test_no_duplicate_device() {
+ let mut set = HashSet::new();
+ for device in PCI_LAYOUT.iter() {
+ if !set.insert(device.device) {
+ panic!("device {:?} added twice in PCI_LAYOUT", device.device)
+ }
+ }
+ let mut set = HashSet::new();
+ for device in PCIE_LAYOUT.iter() {
+ if !set.insert(device.device) {
+ panic!("device {:?} added twice in PCIE_LAYOUT", device.device)
+ }
+ }
+ }
+
+ #[test]
+ fn test_legacy_ids() {
+ let mut set: HashMap<(&str, String), &'static str> = HashMap::new();
+
+ let mut assert_insert = |layout: &Layout, layout_name: &'static str, id: &'static str| {
+ let device = id
+ .parse()
+ .unwrap_or_else(|_| panic!("could not parse {id}"));
+ let addr = layout
+ .find_device(&device)
+ .unwrap_or_else(|_| panic!("could not find address for device {id} {device:?}"));
+ let addr_string = addr.to_qemu_addr();
+ if let Some(old) = set.insert((layout_name, addr_string), id) {
+ panic!("{layout_name}: {id} conflicts with {old} on {addr:?}");
+ }
+ };
+
+ let skipped = &[
+ "ehci", // legacy entries not found on new layouts
+ "piix3",
+ "legacy-igd",
+ "pci.1",
+ "pci.2",
+ "pci.2-igd",
+ "pci.3",
+ "pci.4",
+ "scsihw0", // are handled by virtioscsiX
+ "scsihw1",
+ "scsihw2",
+ "scsihw3",
+ "scsihw4",
+ ];
+ for (id, _, _) in LEGACY_ADDRS {
+ if skipped.contains(&id) {
+ continue;
+ }
+ assert_insert(&PCI_LAYOUT, "pci", id);
+ assert_insert(&PCIE_LAYOUT, "pcie", id);
+ }
+ }
+
+ #[test]
+ /// QEMU needs to be told about multifunction slots, so the flag has to be
+ /// set on function zero of every slot that uses more than one function.
+ fn test_multifunction() {
+ let multifunction: HashMap<Device, bool> = PCI_LAYOUT
+ .iter()
+ .map(|used| (used.device, used.multifunction))
+ .collect();
+
+ // the VGA devices share one slot
+ assert!(multifunction[&Device::Vga(0)]);
+ assert!(!multifunction[&Device::Vga(1)]);
+
+ // 32 net devices need two bridges, which share one slot
+ assert!(multifunction[&Device::Bridge(PveBridge::Net.new_bus(0))]);
+ assert!(!multifunction[&Device::Bridge(PveBridge::Net.new_bus(1))]);
+
+ // 31 SCSI controllers fit on a single bridge
+ assert!(!multifunction[&Device::Bridge(PveBridge::Scsi.new_bus(0))]);
+
+ // devices on a bridge each get their own slot
+ assert!(!multifunction[&Device::Net(0)]);
+ }
+
+ #[test]
+ fn test_root_ports() {
+ let test = |hostpcie: &[u8], expected: &[(u8, &str)]| {
+ let params = HardwareConfigParams {
+ arch: "x86_64".to_string(),
+ machine_type: "pc-q35-10.0+pve1".to_string(),
+ major: 10,
+ minor: 0,
+ pve: Some(1),
+ layout: crate::PciLayout::Grouped,
+ ..Default::default()
+ };
+ let mut hw = HardwareConfig::new(params);
+ hw.hostpcie_devices = hostpcie
+ .iter()
+ .map(|&index| (index, HostpcieDevice::default()))
+ .collect();
+ let expected: Vec<String> = expected
+ .iter()
+ .flat_map(|(index, addr)| {
+ [
+ "-device".to_string(),
+ format!(
+ "pcie-root-port,id=pve.root-port{index},addr={addr},x-speed=16,x-width=32,multifunction=on,bus=pcie.0,port={index},chassis={index}"
+ ),
+ ]
+ })
+ .collect();
+ let res = pci_bus_devices(&hw).expect("valid machine");
+ let res: Vec<String> = res
+ .chunks(2)
+ .filter(|device| device[1].starts_with("pcie-root-port"))
+ .flatten()
+ .cloned()
+ .collect();
+ assert_eq!(res, expected, "for {hostpcie:?}");
+ };
+
+ test(&[], &[]);
+ test(&[0, 1], &[(0, "0e.0"), (1, "0e.1")]);
+ // function 0 of a used slot has to exist for guests to see the others
+ test(&[3], &[(0, "0e.0"), (3, "0e.3")]);
+ test(&[15, 9], &[(8, "0f.0"), (9, "0f.1"), (15, "0f.7")]);
+ }
+}
diff --git a/pve-qemu-server-pci/src/layout/legacy.rs b/pve-qemu-server-pci/src/layout/legacy.rs
index 79346fe..e91f4bd 100644
--- a/pve-qemu-server-pci/src/layout/legacy.rs
+++ b/pve-qemu-server-pci/src/layout/legacy.rs
@@ -830,6 +830,7 @@ pub(crate) mod test {
major: 9,
minor: 2,
pve: None,
+ layout: crate::PciLayout::Legacy,
..Default::default()
};
HardwareConfig::new(params)
diff --git a/pve-qemu-server-pci/src/layout/mod.rs b/pve-qemu-server-pci/src/layout/mod.rs
index 5439c17..9a637ea 100644
--- a/pve-qemu-server-pci/src/layout/mod.rs
+++ b/pve-qemu-server-pci/src/layout/mod.rs
@@ -13,6 +13,8 @@ macro_rules! single_device {
pub(crate) mod legacy;
+pub mod grouped;
+
/// Constructs a correctly sized list of bridge slots, starting at address 0x01
/// because 0x00 is always taken by the bridge itself.
///
diff --git a/pve-qemu-server-pci/src/lib.rs b/pve-qemu-server-pci/src/lib.rs
index a6ebf43..608dc94 100644
--- a/pve-qemu-server-pci/src/lib.rs
+++ b/pve-qemu-server-pci/src/lib.rs
@@ -18,10 +18,48 @@ pub use error::LayoutError;
pub mod layout;
-/// Returns the number of the PCI bridge `id` sits on, or `None` if it is on
-/// the root bus or not a known device.
-pub fn pci_bridge_for_device(id: &str) -> Option<Bus> {
- layout::legacy::pci_bridge_for_device(id)
+use serde::Deserialize;
+
+/// The set of layouts a guest's addresses are taken from.
+#[derive(Default, Debug, Clone, Copy, PartialEq, Eq, Deserialize)]
+#[serde(rename_all = "kebab-case")]
+pub enum PciLayout {
+ /// The layouts reproducing the addresses of the Perl implementation.
+ #[default]
+ Legacy,
+ /// The layouts giving every device kind its own set of bridges, see
+ /// [`layout::grouped`].
+ Grouped,
+}
+
+impl std::str::FromStr for PciLayout {
+ type Err = LayoutError;
+
+ fn from_str(s: &str) -> Result<Self, Self::Err> {
+ match s {
+ "legacy" => Ok(Self::Legacy),
+ "grouped" => Ok(Self::Grouped),
+ _ => Err(LayoutError::InvalidLayout),
+ }
+ }
+}
+
+impl std::fmt::Display for PciLayout {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ f.write_str(match self {
+ PciLayout::Legacy => "legacy",
+ PciLayout::Grouped => "grouped",
+ })
+ }
+}
+
+/// Returns the number of the PCI bridge `id` sits on, or `None` if it is
+/// not a known device.
+pub fn pci_bridge_for_device(hw: &HardwareConfig, id: &str) -> Option<Bus> {
+ match hw.layout {
+ PciLayout::Legacy => layout::legacy::pci_bridge_for_device(id),
+ PciLayout::Grouped => layout::grouped::pci_bridge_for_device(hw, id),
+ }
}
/// Returns the QEMU arguments for all devices that form the buses `hw`
@@ -30,10 +68,16 @@ pub fn pci_bridge_for_device(id: &str) -> Option<Bus> {
/// They are ordered so that every bus is created before the devices on it,
/// and have to be passed before any other device that sits on these buses.
pub fn pci_bus_devices(hw: &HardwareConfig) -> Result<Vec<String>, LayoutError> {
- layout::legacy::pci_bus_devices(hw)
+ match hw.layout {
+ PciLayout::Legacy => layout::legacy::pci_bus_devices(hw),
+ PciLayout::Grouped => layout::grouped::pci_bus_devices(hw),
+ }
}
/// Returns the address of the device with the configuration id `id`.
pub fn address(hw: &HardwareConfig, id: &str) -> Result<PciAddress, LayoutError> {
- layout::legacy::address(hw, id)
+ match hw.layout {
+ PciLayout::Legacy => layout::legacy::address(hw, id),
+ PciLayout::Grouped => layout::grouped::address(hw, id),
+ }
}
diff --git a/pve-qemu-server-pci/src/types/bus.rs b/pve-qemu-server-pci/src/types/bus.rs
index 0fafe1b..052ef00 100644
--- a/pve-qemu-server-pci/src/types/bus.rs
+++ b/pve-qemu-server-pci/src/types/bus.rs
@@ -1,4 +1,69 @@
-use crate::LayoutError;
+#[cfg(test)]
+use strum::VariantArray;
+
+use crate::{Device, LayoutError};
+
+/// The kinds of bridge PVE adds itself, as opposed to the ones a QEMU machine
+/// type already brings along.
+///
+/// Each kind gets its own set of bridges so that the slot of a device only
+/// depends on its own index, not on how many devices of other kinds exist.
+#[cfg_attr(test, derive(VariantArray))]
+#[derive(Hash, Clone, Copy, Debug, PartialEq, Eq)]
+pub enum PveBridge {
+ Net,
+ Scsi,
+ VirtioBlk,
+ VirtioFs,
+ Hostpci,
+}
+
+impl PveBridge {
+ /// The bus formed by the `index`th bridge of this kind.
+ pub const fn new_bus(self, index: u8) -> Bus {
+ Bus::Pve { kind: self, index }
+ }
+
+ /// The device this kind of bridge carries at the given device index.
+ pub const fn device(self, index: u8) -> Device {
+ match self {
+ PveBridge::Net => Device::Net(index),
+ PveBridge::Scsi => Device::ScsiController(index),
+ PveBridge::VirtioBlk => Device::VirtioBlk(index),
+ PveBridge::VirtioFs => Device::VirtioFs(index),
+ PveBridge::Hostpci => Device::Hostpci(index),
+ }
+ }
+}
+
+impl std::fmt::Display for PveBridge {
+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+ let text = match self {
+ PveBridge::Net => "net",
+ PveBridge::Scsi => "scsi",
+ PveBridge::VirtioBlk => "virtio-blk",
+ PveBridge::VirtioFs => "virtio-fs",
+ PveBridge::Hostpci => "hostpci",
+ };
+ f.write_str(text)
+ }
+}
+
+impl std::str::FromStr for PveBridge {
+ type Err = LayoutError;
+
+ fn from_str(s: &str) -> Result<Self, Self::Err> {
+ let kind = match s {
+ "net" => PveBridge::Net,
+ "scsi" => PveBridge::Scsi,
+ "virtio-blk" => PveBridge::VirtioBlk,
+ "virtio-fs" => PveBridge::VirtioFs,
+ "hostpci" => PveBridge::Hostpci,
+ _ => return Err(LayoutError::InvalidConfigId),
+ };
+ Ok(kind)
+ }
+}
/// A bus a device can be attached to, identified the same way QEMU's `bus=`
/// parameter does.
@@ -8,23 +73,51 @@ pub enum Bus {
Pci(u8),
/// A PCIe bus. Index 0 is the root bus of a PCIe machine.
Pcie(u8),
+ /// The bus behind a PCIe root port, which can hold a single device.
+ RootPort(u8),
/// A legacy, deprecated variant of a root port.
#[deprecated]
LegacyRootPort(u8),
/// The PCI bus behind the q35 DMI-to-PCI bridge.
Pcidmi,
+ /// A bridge for the guest's built-in devices.
+ Sys(u8),
+ /// One of the bridges PVE adds per device kind, see [`PveBridge`].
+ Pve { kind: PveBridge, index: u8 },
}
type BusConstructor = fn(u8) -> Bus;
+impl Bus {
+ /// The QEMU device model needed to create this bus below `parent`, or
+ /// `None` if the bus is not created by a bridge device.
+ pub fn bridge_model(self, parent: Bus) -> Option<&'static str> {
+ match self {
+ #[expect(deprecated)]
+ Bus::Pcie(_) | Bus::RootPort(_) | Bus::LegacyRootPort(_) => None,
+ Bus::Pcidmi => Some("i82801b11-bridge"),
+ Bus::Sys(_) | Bus::Pci(_) | Bus::Pve { .. } => Some(match parent {
+ #[expect(deprecated)]
+ Bus::Pcie(_) | Bus::RootPort(_) | Bus::LegacyRootPort(_) => "pcie-pci-bridge",
+ _ => "pci-bridge",
+ }),
+ }
+ }
+}
+
impl std::fmt::Display for Bus {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Bus::Pci(i) => write!(f, "pci.{i}"),
Bus::Pcie(i) => write!(f, "pcie.{i}"),
+ Bus::RootPort(i) => write!(f, "pve.root-port{i}"),
#[expect(deprecated)]
Bus::LegacyRootPort(i) => write!(f, "ich9-pcie-port-{i}"),
+ Bus::Sys(i) => write!(f, "pve.sys{i}"),
Bus::Pcidmi => write!(f, "pcidmi"),
+ Bus::Pve { kind, index } => {
+ write!(f, "pve.{kind}{index}")
+ }
}
}
}
@@ -38,8 +131,10 @@ impl std::str::FromStr for Bus {
const PREFIXES: &[(&str, BusConstructor)] = &[
("pci.", Bus::Pci),
("pcie.", Bus::Pcie),
+ ("pve.root-port", Bus::RootPort),
#[expect(deprecated)]
("ich9-pcie-port-", Bus::LegacyRootPort),
+ ("pve.sys", Bus::Sys),
];
if s == "pcidmi" {
@@ -53,15 +148,30 @@ impl std::str::FromStr for Bus {
}
}
+ if let Some(rest) = s.strip_prefix("pve.")
+ && let Some(idx) = rest.find(|c: char| c.is_ascii_digit())
+ {
+ let kind: PveBridge = rest[0..idx]
+ .parse()
+ .map_err(|_| LayoutError::InvalidConfigId)?;
+ let index: u8 = rest[idx..]
+ .parse()
+ .map_err(|_| LayoutError::InvalidConfigId)?;
+ return Ok(Bus::Pve { kind, index });
+ }
+
Err(LayoutError::InvalidConfigId)
}
}
#[cfg(test)]
mod test {
+ use strum::VariantArray;
use crate::Bus;
+ use super::PveBridge;
+
#[test]
fn test_roundtrip() {
let test = |bus: Bus| {
@@ -76,7 +186,13 @@ mod test {
test(Bus::Pci(99));
test(Bus::Pcie(99));
+ for kind in PveBridge::VARIANTS.iter().copied() {
+ test(Bus::Pve { kind, index: 99 });
+ }
+
+ test(Bus::RootPort(99));
#[expect(deprecated)]
test(Bus::LegacyRootPort(99));
+ test(Bus::Sys(99));
}
}
diff --git a/pve-qemu-server-pci/src/types/layout.rs b/pve-qemu-server-pci/src/types/layout.rs
index 00a5a01..c08eacf 100644
--- a/pve-qemu-server-pci/src/types/layout.rs
+++ b/pve-qemu-server-pci/src/types/layout.rs
@@ -1,7 +1,7 @@
use std::collections::{HashSet, VecDeque};
-use crate::constants::{FUNCTIONS_PER_DEVICE, SLOTS_PER_BUS};
-use crate::{Bus, Device, LayoutError, PciAddress};
+use crate::constants::{FUNCTIONS_PER_DEVICE, SLOTS_PER_BUS, USABLE_SLOTS_PER_BUS};
+use crate::{Bus, Device, LayoutError, PciAddress, PveBridge};
/// What sits at one function number of a slot.
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
@@ -30,6 +30,9 @@ pub enum Slot<'a> {
Single(Function<'a>),
/// A multifunction slot with one entry per function number.
Multi(&'a [Function<'a>; FUNCTIONS_PER_DEVICE]),
+ /// As many bridges of `kind` as are needed to hold `max` devices. The
+ /// bridges share this slot by using one function number each.
+ DynamicBridges { kind: PveBridge, max: u8 },
}
/// The complete slot assignment of a guest, starting at the root bus.
@@ -59,11 +62,13 @@ impl Layout<'_> {
.iter()
.filter_map(|used| match used.device {
#[expect(deprecated)]
- Device::RootPort(id @ Bus::LegacyRootPort(index)) => Some(RootPort {
- addr: used.addr,
- id,
- index,
- }),
+ Device::RootPort(id @ (Bus::RootPort(index) | Bus::LegacyRootPort(index))) => {
+ Some(RootPort {
+ addr: used.addr,
+ id,
+ index,
+ })
+ }
_ => None,
})
.collect();
@@ -200,6 +205,43 @@ impl<'a> AddressAssignments<'a> {
}
}
}
+
+ /// Resolves a slot of dynamic bridges into the queue of address
+ /// assignments.
+ ///
+ /// Unlike fixed bridges, the devices behind them are queued right away,
+ /// as they are generated from the bridge kind rather than listed in the
+ /// layout.
+ fn handle_dynamic_bridges(&mut self, mut addr: PciAddress, kind: PveBridge, max: u8) {
+ // the bridges share one slot, so there cannot be more of them than the
+ // slot has function numbers
+ let num_bridges = max.div_ceil(USABLE_SLOTS_PER_BUS);
+ assert!(usize::from(num_bridges) <= FUNCTIONS_PER_DEVICE);
+
+ for bridge_index in 0..num_bridges {
+ addr.function = bridge_index;
+ let bus = kind.new_bus(bridge_index);
+ self.resolved.push_back(AddressAssignment {
+ addr,
+ device: Device::Bridge(bus),
+ multifunction: bridge_index == 0 && num_bridges > 1,
+ });
+ for slot_nr in 1..=USABLE_SLOTS_PER_BUS {
+ // u16 so that a bigger 'max' cannot overflow
+ let index = u16::from(bridge_index) * u16::from(USABLE_SLOTS_PER_BUS)
+ + u16::from(slot_nr)
+ - 1;
+ if index >= u16::from(max) {
+ break;
+ }
+ self.resolved.push_back(AddressAssignment {
+ addr: PciAddress::new(bus, slot_nr, 0),
+ device: kind.device(index as u8),
+ multifunction: false,
+ });
+ }
+ }
+ }
}
impl Iterator for AddressAssignments<'_> {
@@ -220,6 +262,9 @@ impl Iterator for AddressAssignments<'_> {
self.handle_function(addr, *function, function_nr == 0);
}
}
+ Slot::DynamicBridges { kind, max } => {
+ self.handle_dynamic_bridges(addr, kind, max)
+ }
}
}
diff --git a/pve-qemu-server-pci/src/types/mod.rs b/pve-qemu-server-pci/src/types/mod.rs
index 9178769..43b08c0 100644
--- a/pve-qemu-server-pci/src/types/mod.rs
+++ b/pve-qemu-server-pci/src/types/mod.rs
@@ -1,8 +1,11 @@
mod bus;
-pub use bus::Bus;
+pub use bus::{Bus, PveBridge};
mod device;
pub use device::Device;
+// only needed to enumerate all device kinds in tests
+#[cfg(test)]
+pub(crate) use device::DeviceDiscriminants;
mod pci_address;
pub use pci_address::PciAddress;
--
2.47.3
next prev parent reply other threads:[~2026-11-06 13:26 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-06 13:25 [RFC proxmox-perl-rs/qemu-server/qemu-server-rs v3 0/9] pci-handling rewrite (part 1/2/3) Dominik Csapak
2026-11-06 13:25 ` [PATCH pve-qemu-server-rs v3 1/9] add pve-qemu-server-pci crate for guest PCI address generation Dominik Csapak
2026-11-06 13:25 ` [PATCH pve-qemu-server-rs v3 2/9] pci: legacy: generate pci bridges Dominik Csapak
2026-11-06 13:25 ` [PATCH pve-qemu-server-rs v3 3/9] pci: legacy: generate q35 chipset devices and hostpcie root ports Dominik Csapak
2026-11-06 13:25 ` Dominik Csapak [this message]
2026-11-06 13:25 ` [PATCH proxmox-perl-rs v3 5/9] pve: add bindings for `pve-qemu-server-pci` crate Dominik Csapak
2026-11-06 13:25 ` [PATCH qemu-server v3 6/9] pci: use PVE::RS::PCI bindings for addresses and bus devices Dominik Csapak
2026-11-06 13:25 ` [PATCH qemu-server v3 7/9] tree-wide: use HardwareConfig for the machine version checks Dominik Csapak
2026-11-06 13:25 ` [PATCH qemu-server v3 8/9] cfg2cmd: q35: generate chipset devices with machine version 11.1+pve1 Dominik Csapak
2026-11-06 13:25 ` [PATCH qemu-server v3 9/9] machine: add 'pci-layout' option Dominik Csapak
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20261106132604.2556840-5-d.csapak@proxmox.com \
--to=d.csapak@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox