Re: [PATCH pve-qemu-server-rs v2 3/9] pci: legacy: generate q35 chipset devices and hostpcie root ports

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

Comments inline.

On Thu Oct 15, 2026 at 10:10 AM CEST, Dominik Csapak wrote:
> Until now, the q35 chipset devices were added via the pve-q35-4.0.cfg
> machine config file, and the root ports for hostpcie4 and above were
> added by the caller for each configured device.
>
> The latter can leave a root port without a function 0 in its slot,
> e.g. when only hostpci5 is configured. Since guests only scan the
> other functions of a slot when function 0 exists (Linux stops in
> `pci_scan_slot()`), such a device was not visible in the guest.
>
> To fix that and to have the layout as the single source for the pci
> hierarchy, generate both in `pci_bus_devices`:
> * the chipset devices with the same ids, properties and order as in
>   the config file, which a test checks against a copy of it. Machines
>   older than 4.0 used pve-q35.cfg instead, but those are not supported
>   anymore.
> * the root ports of the configured hostpcie devices, plus the one on
>   function 0 of each used slot. Devices that were already visible keep
>   their address, and Windows 7 guests still don't get any root ports.
>
> Since the devices of running guests must not change for live
> migration, we only generate the chipset devices starting with machine
> version 11.1+pve1. For older machines, `uses_q35_config_file` tells
> the caller to keep using the config file.
>
> This replaces `print_pcie_root_port`, as we now decide here which root
> ports are needed.
>
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---

[snip]

> diff --git a/pve-qemu-server-pci/src/lib.rs b/pve-qemu-server-pci/src/lib.rs
> index fc982ce..a5d82a4 100644
> --- a/pve-qemu-server-pci/src/lib.rs
> +++ b/pve-qemu-server-pci/src/lib.rs
> @@ -11,7 +11,7 @@
>  pub mod constants;
>  
>  mod hardware_config;
> -pub use hardware_config::HardwareConfig;
> +pub use hardware_config::{HardwareConfig, HostpcieDevice};
>  
>  mod types;
>  pub use types::*;
> diff --git a/pve-qemu-server-pci/src/types/layout.rs b/pve-qemu-server-pci/src/types/layout.rs
> index 889b961..91b1d78 100644
> --- a/pve-qemu-server-pci/src/types/layout.rs
> +++ b/pve-qemu-server-pci/src/types/layout.rs
> @@ -1,4 +1,4 @@
> -use std::collections::VecDeque;
> +use std::collections::{HashSet, VecDeque};
>  
>  use crate::constants::{FUNCTIONS_PER_DEVICE, SLOTS_PER_BUS, USABLE_SLOTS_PER_BUS};
>  use crate::{Bus, Device, LayoutError, PciAddress, PveBridge};
> @@ -48,6 +48,46 @@ pub struct Layout<'a> {
>  }
>  
>  impl Layout<'_> {
> +    /// Returns the root ports `devices` are attached to in layout order, plus
> +    /// the one on function 0 of each used slot, as guests only look for further
> +    /// functions if function 0 exists.
> +    pub fn root_ports(&self, devices: &[Device]) -> Vec<RootPort> {
> +        let attached: HashSet<PciAddress> = devices
> +            .iter()
> +            .filter_map(|device| self.find_device(device).ok())
> +            .collect();
> +        let is_used = |port: &RootPort| attached.contains(&PciAddress::new(port.id, 0, 0));
> +
> +        let ports: Vec<RootPort> = self
> +            .iter()
> +            .filter_map(|used| match used.device {
> +                #[expect(deprecated)]
> +                Device::RootPort(id @ (Bus::RootPort(index) | Bus::LegacyRootPort(index))) => {
> +                    Some(RootPort {
> +                        addr: used.addr,
> +                        id,
> +                        index,
> +                    })
> +                }
> +                _ => None,
> +            })
> +            .collect();
> +        let used_slots: HashSet<(Bus, u8)> = ports
> +            .iter()
> +            .filter(|port| is_used(port))
> +            .map(|port| (port.addr.bus, port.addr.device))
> +            .collect();
> +
> +        ports
> +            .into_iter()
> +            .filter(|port| {
> +                is_used(port)
> +                    || (port.addr.function == 0
> +                        && used_slots.contains(&(port.addr.bus, port.addr.device)))
> +            })
> +            .collect()
> +    }
> +
>      /// Looks up the address of a device in this layout.
>      ///
>      /// The first match wins, so the order of the layout is significant: a
> @@ -68,6 +108,27 @@ impl Layout<'_> {
>      }
>  }
>  
> +/// A PCIe root port of a layout.
> +#[derive(Debug, Clone, Copy, PartialEq, Eq)]
> +pub struct RootPort {
> +    pub addr: PciAddress,
> +    /// The bus the root port forms.
> +    pub id: Bus,
> +    /// The number of the root port, also used as port and chassis number.
> +    pub index: u8,
> +}
> +
> +impl RootPort {
> +    /// Renders the QEMU `-device` argument for this root port.
> +    pub fn to_qemu_device(&self) -> String {
> +        let RootPort { addr, id, index } = self;
> +        format!(
> +            "pcie-root-port,id={id},addr={:02x}.{},x-speed=16,x-width=32,multifunction=on,bus={},port={index},chassis={index}",
> +            addr.device, addr.function, addr.bus,

nit: I understand that setting multifunction=on shouldnt cause any harm
here, but IMO it should still be encoded in RootPort. e.g.

pub struct RootPort {
    pub addr: PciAddress,
    /// The bus the root port forms.
    pub id: Bus,
    /// The number of the root port, also used as port and chassis number.
    pub index: u8,
	pub multifunction: bool,
}

> +        )
> +    }
> +}
> +

nit: I understand that setting multifunction=on shouldnt cause any harm
here, but IMO it should still be encoded in RootPort. e.g.

pub struct RootPort {
    pub addr: PciAddress,
    /// The bus the root port forms.
    pub id: Bus,
    /// The number of the root port, also used as port and chassis number.
    pub index: u8,
    pub multifunction: bool,
}

>  /// The address a layout assigns to a device.
>  #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
>  pub struct AddressAssignment {

[snip]