[PATCH pve-qemu-server-rs 3/9] pci: layout: add v2 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.

These are not wired up to the entry points yet: they are meant for new
guests only, and the machine property selecting them still has to be
added on the qemu-server side.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 pve-qemu-server-pci/src/layout/mod.rs |   2 +
 pve-qemu-server-pci/src/layout/v2.rs  | 344 ++++++++++++++++++++++++++
 pve-qemu-server-pci/src/types/mod.rs  |   3 +
 3 files changed, 349 insertions(+)
 create mode 100644 pve-qemu-server-pci/src/layout/v2.rs

diff --git a/pve-qemu-server-pci/src/layout/mod.rs b/pve-qemu-server-pci/src/layout/mod.rs
index c81d730..0b72abd 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 v2;
+
 /// 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/layout/v2.rs b/pve-qemu-server-pci/src/layout/v2.rs
new file mode 100644
index 0000000..9525492
--- /dev/null
+++ b/pve-qemu-server-pci/src/layout/v2.rs
@@ -0,0 +1,344 @@
+use crate::constants::{
+    BRIDGE_SLOT_NUM, MAX_HOSTPCI_DEVICES, MAX_NET_DEVICES, MAX_SCSI_DEVICES,
+    MAX_VIRTIO_BLK_DEVICES, MAX_VIRTIOFS_DEVICES,
+};
+use crate::layout::bridge_slots;
+use crate::{Bus, Device, DeviceLayout, Function, 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 SYS_BRIDGE_SLOTS: [Slot; BRIDGE_SLOT_NUM] = 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: DeviceLayout = DeviceLayout {
+    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), &SYS_BRIDGE_SLOTS)),
+        Slot::DynamicBridge {
+            kind: PveBridge::VirtioFs,
+            max: MAX_VIRTIOFS_DEVICES,
+        },
+        Slot::DynamicBridge {
+            kind: PveBridge::Net,
+            max: MAX_NET_DEVICES,
+        },
+        Slot::Unused,
+        Slot::DynamicBridge {
+            kind: PveBridge::Scsi,
+            max: MAX_SCSI_DEVICES,
+        },
+        Slot::Unused,
+        Slot::DynamicBridge {
+            kind: PveBridge::VirtioBlk,
+            max: MAX_VIRTIO_BLK_DEVICES,
+        },
+        Slot::Unused,
+        Slot::Unused,
+        Slot::Unused,
+        Slot::DynamicBridge {
+            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: DeviceLayout = DeviceLayout {
+    pcie: true,
+    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), &SYS_BRIDGE_SLOTS)),
+        Slot::DynamicBridge {
+            kind: PveBridge::VirtioFs,
+            max: MAX_VIRTIOFS_DEVICES,
+        },
+        Slot::DynamicBridge {
+            kind: PveBridge::Net,
+            max: MAX_NET_DEVICES,
+        },
+        Slot::Unused,
+        Slot::DynamicBridge {
+            kind: PveBridge::Scsi,
+            max: MAX_SCSI_DEVICES,
+        },
+        Slot::Unused,
+        Slot::DynamicBridge {
+            kind: PveBridge::VirtioBlk,
+            max: MAX_VIRTIO_BLK_DEVICES,
+        },
+        Slot::Unused,
+        Slot::Unused,
+        Slot::Unused,
+        Slot::DynamicBridge {
+            kind: PveBridge::Hostpci,
+            max: MAX_HOSTPCI_DEVICES,
+        },
+        Slot::Unused,
+        Slot::Multi(&[
+            Function::RootPort(0, Device::Hostpcie(0)),
+            Function::RootPort(1, Device::Hostpcie(1)),
+            Function::RootPort(2, Device::Hostpcie(2)),
+            Function::RootPort(3, Device::Hostpcie(3)),
+            Function::RootPort(4, Device::Hostpcie(4)),
+            Function::RootPort(5, Device::Hostpcie(5)),
+            Function::RootPort(6, Device::Hostpcie(6)),
+            Function::RootPort(7, Device::Hostpcie(7)),
+        ]),
+        Slot::Multi(&[
+            Function::RootPort(8, Device::Hostpcie(8)),
+            Function::RootPort(9, Device::Hostpcie(9)),
+            Function::RootPort(10, Device::Hostpcie(10)),
+            Function::RootPort(11, Device::Hostpcie(11)),
+            Function::RootPort(12, Device::Hostpcie(12)),
+            Function::RootPort(13, Device::Hostpcie(13)),
+            Function::RootPort(14, Device::Hostpcie(14)),
+            Function::RootPort(15, Device::Hostpcie(15)),
+        ]),
+    ]),
+};
+
+#[cfg(test)]
+mod test {
+    use std::collections::{HashMap, HashSet};
+
+    use strum::VariantArray;
+
+    use super::{PCI_LAYOUT, PCIE_LAYOUT};
+
+    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, DeviceLayout, 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: &DeviceLayout, 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::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: &DeviceLayout, 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)]);
+    }
+}
diff --git a/pve-qemu-server-pci/src/types/mod.rs b/pve-qemu-server-pci/src/types/mod.rs
index 6348ffc..7ed27e6 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