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 7A9621FF0AF for ; Fri, 06 Nov 2026 14:26:48 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id E13372171B; Fri, 06 Nov 2026 14:26:20 +0100 (CET) From: Dominik Csapak To: pve-devel@lists.proxmox.com Subject: [RFC proxmox-perl-rs/qemu-server/qemu-server-rs v3 0/9] pci-handling rewrite (part 1/2/3) Date: Fri, 6 Nov 2026 14:25:43 +0100 Message-ID: <20261106132604.2556840-1-d.csapak@proxmox.com> X-Mailer: git-send-email 2.47.3 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.315 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) 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: Z5O6J3EMU7HYTMLHJ3HUPTORDYRLI7DQ X-Message-ID-Hash: Z5O6J3EMU7HYTMLHJ3HUPTORDYRLI7DQ 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: # Motivation We want to extend the PCI layout mechanism to be easily extendable, especially for NUMA awareness and replicating host-topology (e.g. see bug #7283 [0]). This is necessary for some performance gains in guests, especially with high-end hardware where users pay extra for higher performance. # The Problem PCI addresses (and hardware layout in general) must be fixed for our qemu guests, since this is part of the machine state and must be stable for live-migration suspend/resume, etc. Currently, these addresses are hardcoded in perl hashes, with a long history of devs (me included) piling new addresses on top, using whatever address was free. This led to a very convoluted and scattered assignment list, without any underlying structure, which makes it harder the more new devices are added. (Using a wrong address only surfaces when running some tests or starting a guest with it) # My Proposed Solution So to fix the current situation I propose a plan in multiple phases: ## Phase 1 - untangling perl code to rust (this series) To extend/switch between multiple PCI layouts, we first must untangle the current situation and make it easier to see what is actually used. I did this in rust, by using the type system to represent the PCI layout, and inferring the addresses from the layout, not the other way around. That makes it easier to see where there is actually free space and where we can add new things. It also highlights how messy the current layout is. (See the legacy layout in the `pve-qemu-server-pci` crate) I also introduced here a generic `HardwareConfig` struct that's intended to hold information useful to more call sites in contrast to passing a lot of arguments everywhere, similar to the Cfg2Cmd class, but usable in the rust codebase. ## Phase 2 - extending pci layouts (this series) This introduces a new layout ("grouped") that can be used in an opt-in manner, and is vastly more logically constructed, which makes it easy to extend and reason about. This part entails restructuring the addr calls in qemu-server's perl code, so that there is only a single entry point ("address" for example) instead of splitting between 'print_pci_addr' and 'print_pcie_addr'. This is folded into the commit to using the rust bindings, since it does not make sense to recreate the old interface in rust, just to replace it with the next patches anyway. This now also creates the commandline for the q35 devices instead of using 'readconfig' (guarded behind a new machine version). ## Phase 3 - more perl to rust code port (this series) The HardwareConfig also contains a version comparison, so we can widely reuse it in many methods, since we need it for the 'addr' interface anyway. These changes are also folded for the same reason as above. ## Phase 4 - Host and NUMA layouts (the next part) With all the pieces in place, we can then easily extend the new layout by PCI switches that are needed for assigning to NUMA nodes and replicating the host topology for passed through devices. To answer the question from v2 how this relates to the 'pci-layout' option: I plan to make this a separate setting on top of the grouped layout instead of adding more layouts, e.g.: machine: q35,pci-layout=grouped,hostpci-topology= * flat (default): every hostpcie device gets its own root port on the root bus, the layout in this series * numa: adds a pxb-pcie expander bridge per guest NUMA node and puts the root ports of that node's devices behind it * host: additionally replicates the host PCIe switches that more than one passed through device is behind The expander bridges go into slots of the grouped layout that are left unused in this series, so enabling a topology does not move any of the existing devices, and 'pci-layout' does not need more values for it. qemu-server provides the NUMA node and the host switches of each device, so nothing has to be configured per device. The name and place of the option are not fixed yet, so suggestions are welcome. # Notes * pve-qemu-server-rs is a new git repo, but the crate in it could easily be integrated somewhere else. Having this as a separate repo could replace 'qemu-server' at one point though. * This is an RFC, so comments about the general design/plan are desired. * Names and crate placement are not fixed, I know that some names I chose are not optimal, if you do have better names/places, please suggest them. * Opted to do this in rust for now, since I think that is the best way forward. If the consensus is to do the layouting changes in perl, or keep some parts in perl, that's fine with me too and I'll change my efforts accordingly * @Elias: i did not implement every change: - the layout type stayed in the types folder (IMO makes more sense) - chassis_nr stayed the same (number of bridges can't change dynamically) - no const fn (i don't think it's possible with a single const fn and the same short syntax, but if it is, please do show me that) - didn't rename the pci_ methods (IMO this is fine) - no thiserror used, IMO not necessary here changes from v2: - pve-qemu-server-rs: * moved some code into later commits where it's necessary * `HardwareConfig::new` now takes a `HardwareConfigParams` struct, which can be deserialized directly and rejects unknown keys * the layout is deserialized directly into `PciLayout`, so an unknown layout is now an error instead of silently falling back to the legacy one * hostpcie devices are passed as a list of objects (`{ index => N }`) instead of plain indices, so they can be extended later without changing the interface * use an internal `Arch` enum, so the config can't hold an unknown architecture * grouped layout: only set 'multifunction=on' on bridges that share their slot with other bridges (currently only the net bridges) * the `Display` implementation of `Device` now prints the configuration ids (e.g. 'qga0', 'balloon0', 'virtioN') and is exhaustive * the xhci controller has no index anymore (we can add more ports with hubs) * document that `version_at_least` records the required pve version, and use relaxed ordering for it * check the bridge count with `assert!` instead of `debug_assert!` * drop the unused anyhow and log dependencies * code style and doc fixes (`'static` layout lifetime, `match` in `map_legacy_pci_id`, comments) - proxmox-perl-rs: * `new` takes a single hash with the parameters instead of a list of positional arguments * `addr` returns undef for devices that are not part of the layout instead of dying * use 'this' for the object parameter and add the 'Class method'/'Method' doc prefixes * doc fixes - qemu-server: * rebased on current master * construct the HardwareConfig with a parameter hash, and pass the hostpcie devices as objects * drop the now unused `PVE::QemuServer::Machine` import from USB.pm * grouped layout tests: adapt to the bridges without 'multifunction=on' 0: https://bugzilla.proxmox.com/show_bug.cgi?id=7283 pve-qemu-server-rs: Dominik Csapak (4): add pve-qemu-server-pci crate for guest PCI address generation pci: legacy: generate pci bridges pci: legacy: generate q35 chipset devices and hostpcie root ports pci: layout: add grouped PCI and PCIe layouts proxmox-perl-rs: Dominik Csapak (1): pve: add bindings for `pve-qemu-server-pci` crate pve-rs/Cargo.toml | 1 + pve-rs/Makefile | 1 + pve-rs/src/bindings/mod.rs | 3 + pve-rs/src/bindings/pci/hardware_config.rs | 120 +++++++++++++++++++++ pve-rs/src/bindings/pci/mod.rs | 2 + 5 files changed, 127 insertions(+) create mode 100644 pve-rs/src/bindings/pci/hardware_config.rs create mode 100644 pve-rs/src/bindings/pci/mod.rs qemu-server: Dominik Csapak (4): pci: use PVE::RS::PCI bindings for addresses and bus devices tree-wide: use HardwareConfig for the machine version checks cfg2cmd: q35: generate chipset devices with machine version 11.1+pve1 machine: add 'pci-layout' option src/PVE/API2/Qemu.pm | 9 +- src/PVE/QemuServer.pm | 266 +++++++------- src/PVE/QemuServer/Drive.pm | 7 +- src/PVE/QemuServer/DriveDevice.pm | 12 +- src/PVE/QemuServer/Machine.pm | 16 + src/PVE/QemuServer/PCI.pm | 340 ++---------------- src/PVE/QemuServer/RNG.pm | 6 +- src/PVE/QemuServer/USB.pm | 59 +-- src/PVE/QemuServer/Virtiofs.pm | 7 +- src/test/Makefile | 5 +- src/test/QemuServerTest/CommandLineMocks.pm | 27 ++ .../cfg2cmd/efi-secboot-and-tpm-q35.conf.cmd | 21 +- src/test/cfg2cmd/q35-grouped-layout.conf | 17 + src/test/cfg2cmd/q35-grouped-layout.conf.cmd | 47 +++ .../cfg2cmd/q35-grouped-virtiofs-viommu.conf | 14 + .../q35-grouped-virtiofs-viommu.conf.cmd | 46 +++ src/test/cfg2cmd/q35-ide.conf.cmd | 21 +- .../cfg2cmd/q35-linux-hostpci-11.1+pve1.conf | 21 ++ .../q35-linux-hostpci-11.1+pve1.conf.cmd | 61 ++++ .../q35-linux-hostpci-driver-keep.conf.cmd | 25 +- .../q35-linux-hostpci-mapping.conf.cmd | 21 +- .../q35-linux-hostpci-multifunction.conf.cmd | 21 +- src/test/cfg2cmd/q35-linux-hostpci-wrong.conf | 16 + .../cfg2cmd/q35-linux-hostpci-wrong.conf.cmd | 41 +++ ...q35-linux-hostpci-x-pci-overrides.conf.cmd | 21 +- src/test/cfg2cmd/q35-linux-hostpci.conf.cmd | 25 +- src/test/cfg2cmd/q35-linux-ivshmem.conf.cmd | 21 +- src/test/cfg2cmd/q35-scsihw2.conf.cmd | 21 +- src/test/cfg2cmd/q35-simple.conf.cmd | 21 +- .../cfg2cmd/q35-viommu-intel-aw-bits.conf.cmd | 21 +- .../q35-viommu-intel-guest-phys-bits.conf.cmd | 21 +- src/test/cfg2cmd/q35-viommu-intel.conf.cmd | 21 +- .../q35-viommu-virtio-aw-bits.conf.cmd | 21 +- src/test/cfg2cmd/q35-viommu-virtio.conf.cmd | 21 +- src/test/cfg2cmd/q35-win10-hostpci.conf.cmd | 2 +- .../simple-grouped-virtiofs-viommu.conf | 16 + .../simple-grouped-virtiofs-viommu.conf.cmd | 45 +++ src/test/cfg2cmd/simple-grouped.conf | 15 + src/test/cfg2cmd/simple-grouped.conf.cmd | 41 +++ src/test/cfg2cmd/spice-win-grouped.conf | 14 + src/test/cfg2cmd/spice-win-grouped.conf.cmd | 48 +++ src/test/cfg2cmd/virtiofs-viommu.conf | 16 + src/test/cfg2cmd/virtiofs-viommu.conf.cmd | 40 +++ src/test/run_pci_addr_checks.pl | 141 -------- 44 files changed, 1039 insertions(+), 679 deletions(-) create mode 100644 src/test/cfg2cmd/q35-grouped-layout.conf create mode 100644 src/test/cfg2cmd/q35-grouped-layout.conf.cmd create mode 100644 src/test/cfg2cmd/q35-grouped-virtiofs-viommu.conf create mode 100644 src/test/cfg2cmd/q35-grouped-virtiofs-viommu.conf.cmd create mode 100644 src/test/cfg2cmd/q35-linux-hostpci-11.1+pve1.conf create mode 100644 src/test/cfg2cmd/q35-linux-hostpci-11.1+pve1.conf.cmd create mode 100644 src/test/cfg2cmd/q35-linux-hostpci-wrong.conf create mode 100644 src/test/cfg2cmd/q35-linux-hostpci-wrong.conf.cmd create mode 100644 src/test/cfg2cmd/simple-grouped-virtiofs-viommu.conf create mode 100644 src/test/cfg2cmd/simple-grouped-virtiofs-viommu.conf.cmd create mode 100644 src/test/cfg2cmd/simple-grouped.conf create mode 100644 src/test/cfg2cmd/simple-grouped.conf.cmd create mode 100644 src/test/cfg2cmd/spice-win-grouped.conf create mode 100644 src/test/cfg2cmd/spice-win-grouped.conf.cmd create mode 100644 src/test/cfg2cmd/virtiofs-viommu.conf create mode 100644 src/test/cfg2cmd/virtiofs-viommu.conf.cmd delete mode 100755 src/test/run_pci_addr_checks.pl Summary over all repositories: 49 files changed, 1166 insertions(+), 679 deletions(-) -- Generated by murpp 0.11.0