[PATCH pve-qemu-server-rs v2 4/9] pci: layout: add grouped PCI and PCIe layouts

[Dominik Csapak <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       |   8 +
 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  | 469 +++++++++++++++++++++
 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             |  49 ++-
 pve-qemu-server-pci/src/types/mod.rs       |   3 +
 8 files changed, 538 insertions(+), 6 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 ea6db21..29e5d87 100644
--- a/pve-qemu-server-pci/src/constants.rs
+++ b/pve-qemu-server-pci/src/constants.rs
@@ -1,5 +1,13 @@
 //! 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;
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 677fe0a..95bcda6 100644
--- a/pve-qemu-server-pci/src/hardware_config.rs
+++ b/pve-qemu-server-pci/src/hardware_config.rs
@@ -4,7 +4,7 @@ use std::sync::atomic::AtomicU16;
 
 use pve_api_types::{ClusterResourceHostArch as Arch, QemuConfigOstype, QemuConfigScsihw};
 
-use crate::Device;
+use crate::{Device, PciLayout};
 
 #[derive(Default, Clone, Debug, PartialEq)]
 pub enum Mainboard {
@@ -59,6 +59,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>,
@@ -81,6 +84,7 @@ impl HardwareConfig {
         major: u16,
         minor: u16,
         pve: Option<u16>,
+        layout: &str,
     ) -> Self {
         let arch = match arch.parse().unwrap_or(Arch::X8664) {
             Arch::UnknownEnumValue(_) => Arch::X8664,
@@ -103,6 +107,7 @@ impl HardwareConfig {
             ostype,
             machine_version: (major, minor, pve),
             required_pve_version: Arc::new(AtomicU16::new(0)),
+            layout: layout.parse().unwrap_or(PciLayout::Legacy),
             hostpcie_devices: BTreeMap::new(),
         }
     }
@@ -123,7 +128,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..5c5a8cc
--- /dev/null
+++ b/pve-qemu-server-pci/src/layout/grouped.rs
@@ -0,0 +1,469 @@
+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(0)),
+    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 addr.function == 0 {
+            ",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, 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(0));
+                }
+                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 modern 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 mut hw = HardwareConfig::new(
+                "x86_64",
+                "pc-q35-10.0+pve1".into(),
+                None,
+                0,
+                false,
+                None,
+                10,
+                0,
+                Some(1),
+                "grouped",
+            );
+            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 d89c0bf..e594ab9 100644
--- a/pve-qemu-server-pci/src/layout/legacy.rs
+++ b/pve-qemu-server-pci/src/layout/legacy.rs
@@ -837,6 +837,7 @@ pub(crate) mod test {
             9,
             2,
             None,
+            "legacy",
         )
     }
 
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 a5d82a4..5417fa5 100644
--- a/pve-qemu-server-pci/src/lib.rs
+++ b/pve-qemu-server-pci/src/lib.rs
@@ -21,10 +21,45 @@ pub use error::LayoutError;
 
 pub mod layout;
 
+/// The set of layouts a guest's addresses are taken from.
+#[derive(Default, Debug, Clone, Copy, PartialEq, Eq)]
+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 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)
+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`
@@ -33,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/mod.rs b/pve-qemu-server-pci/src/types/mod.rs
index 1605066..43b08c0 100644
--- a/pve-qemu-server-pci/src/types/mod.rs
+++ b/pve-qemu-server-pci/src/types/mod.rs
@@ -3,6 +3,9 @@ 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