From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id B7AAB1FF0AA for ; Tue, 22 Sep 2026 12:56:46 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 24D8521664; Tue, 22 Sep 2026 12:55:58 +0200 (CEST) From: Dominik Csapak To: pve-devel@lists.proxmox.com Subject: [PATCH pve-qemu-server-rs 3/9] pci: layout: add v2 PCI and PCIe layouts Date: Tue, 22 Sep 2026 12:55:34 +0200 Message-ID: <20260922105550.2084078-4-d.csapak@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260922105550.2084078-1-d.csapak@proxmox.com> References: <20260922105550.2084078-1-d.csapak@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.543 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) KAM_MAILER 2 Automated Mailer Tag Left in Email RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: F6FR2LVZRMATKPMOOCGTLMCAIOS6FXO7 X-Message-ID-Hash: F6FR2LVZRMATKPMOOCGTLMCAIOS6FXO7 X-MailFrom: d.csapak@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: 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 --- 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) { + 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 = 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 = 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