Re: [PATCH pve-qemu-server-rs 3/9] pci: layout: add v2 PCI and PCIe layouts

["Elias Huhsovitz" <e.huhsovitz@proxmox.com>]

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 <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(&[

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 = 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)),
> +        ]),

We have 16 device slots via the dynmic bridge and 16 devices slots via
root ports = 32 passthrough slots. But the config schema only allows
for 16 total `hostpciN` devices. 

IMO we need to add `hostpcieN` as a config key in perl and add a new
constant.

pub const MAX_HOSTPCIE_DEVICES: u8 = 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 placement.

> +    ]),
> +};

[snip]