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

[Dominik Csapak <d.csapak@proxmox.com>]

On 9/28/26 2:55 PM, Elias Huhsovitz wrote:
> 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.

i tried that, but that didn't quite workout with the FromStr 
implementations.

The issue here is that a hostpciX device can be either placed
in a pci or pcie slot, depending on its own config.

but see my reply to patch 1

> 
>> +    ]),
>> +};
> 
> [snip]