From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id D6D551FF0B4 for ; Tue, 10 Nov 2026 18:08:12 +0100 (CET) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 497E62164E; Tue, 10 Nov 2026 18:08:09 +0100 (CET) Message-ID: <3e0667f1-b7c2-4988-8ce8-3c6a462c206d@proxmox.com> Date: Tue, 10 Nov 2026 18:08:03 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH pve-qemu-server-rs v3 4/9] pci: layout: add grouped PCI and PCIe layouts To: Dominik Csapak , pve-devel@lists.proxmox.com References: <20261106132604.2556840-1-d.csapak@proxmox.com> <20261106132604.2556840-5-d.csapak@proxmox.com> Content-Language: en-US From: Fiona Ebner In-Reply-To: <20261106132604.2556840-5-d.csapak@proxmox.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1794330483601 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.283 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: YZPIOMQTWAQTYHNPVWWAC6CYE7OALBVK X-Message-ID-Hash: YZPIOMQTWAQTYHNPVWWAC6CYE7OALBVK X-MailFrom: f.ebner@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: Note that I didn't go in-depth here yet when reviewing, just some things I noted: Am 06.11.26 um 2:27 PM schrieb Dominik Csapak: > 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. There currently are 9 (or 13 for pcie, or 9 for the standalone devices) slots in the new layout's root port assigned already. So we can add at most 23 (or 19 for pcie, or 23 for the standalone devices) more kinds of devices, or? Which might seem a lot at first glance, but in the long run might be a bit restrictive? Although I suppose some of them will be standalone and if we really reach that point we can add a switch for new layout again. Still, might be worth a bit of consideration if we can increase those limits a bit without too much trouble? If it's not easy to do so, don't mind me :) > > 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. > ---snip 8<--- > 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), Are we sure we never want to have multiple instances for any of these? Maybe for rng or audio at some point? At least their config keys are named rng0 and audio0 in qemu-server ;) ---snip 8<--- > 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}; Only noticing it now, but maybe MAX_FUNCTIONS_PER_DEVICE? And the type Device is used for one function, so clashes a bit with the naming,so MAX_FUNCTIONS_PER_DEVICE_SLOT or similar? Not sure.