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 032C61FF09B for ; Mon, 28 Sep 2026 14:55:30 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 88280216CD; Mon, 28 Sep 2026 14:55:27 +0200 (CEST) Content-Type: text/plain; charset=UTF-8 Date: Mon, 28 Sep 2026 14:55:21 +0200 Message-Id: To: "Dominik Csapak" , Subject: Re: [PATCH pve-qemu-server-rs 3/9] pci: layout: add v2 PCI and PCIe layouts From: "Elias Huhsovitz" Content-Transfer-Encoding: quoted-printable Mime-Version: 1.0 X-Mailer: aerc 0.20.0 References: <20260922105550.2084078-1-d.csapak@proxmox.com> <20260922105550.2084078-4-d.csapak@proxmox.com> In-Reply-To: <20260922105550.2084078-4-d.csapak@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1790600122016 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.605 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: QDPXI3T5RDNH2DK6NJHLGEATTKGCEZTR X-Message-ID-Hash: QDPXI3T5RDNH2DK6NJHLGEATTKGCEZTR X-MailFrom: e.huhsovitz@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: Since the `v2` name is just a development placeholder, I will start with some suggestions: * `grouped` - describing the property of the new layout. * `per_device` | `per_kind` - since each device gets its own bridges. * `default` - since it is the new default (might age badly if a new default is introduced). On Tue Sep 22, 2026 at 12:55 PM CEST, Dominik Csapak wrote: > 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 { > =20 > pub(crate) mod legacy; > =20 > +pub mod v2; > + > /// Constructs a correctly sized list of bridge slots, starting at addre= ss 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/s= rc/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_DEVI= CES, > + 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 ow= n 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] =3D bridge_slots(&[ nit: name: I would prefer `BUILTIN_BRIDGE_SLOTS` or `BUILTIN_DEVICE_SLOTS`. Since devies are usually in slots, we can also just shorten it to `BUILTIN_DEVICES`. > + 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 =3D 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_SLOT= S)), > + 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 roo= t ports > +/// for passed through PCIe devices, which a PCI machine cannot have. > +pub static PCIE_LAYOUT: DeviceLayout =3D 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_SLOT= S)), > + 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)), > + ]), We have 16 device slots via the dynmic bridge and 16 devices slots via root ports =3D 32 passthrough slots. But the config schema only allows for 16 total `hostpciN` devices.=20 IMO we need to add `hostpcieN` as a config key in perl and add a new constant. pub const MAX_HOSTPCIE_DEVICES: u8 =3D 16; OR what about removing the explicit `Device::Hostpcie` and folding in the placement into the enum, like this: pub enum Device { Hostpci(u8, HostpciPlacement), } pub enum HostpciPlacement { Bridge, RootPort, } Since, if I understand this correctly, we primarily care about the placemen= t. > + ]), > +}; [snip]