public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
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





  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
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal