From: Dominik Csapak <d.csapak@proxmox.com>
To: pve-devel@lists.proxmox.com
Subject: [PATCH pve-qemu-server-rs 2/9] pci: add machine abstraction and PCI bridge generation
Date: Tue, 22 Sep 2026 12:55:33 +0200 [thread overview]
Message-ID: <20260922105550.2084078-3-d.csapak@proxmox.com> (raw)
In-Reply-To: <20260922105550.2084078-1-d.csapak@proxmox.com>
Besides the address of a single device, callers also need the list of
bridge devices a guest has to be started with. Which bridges those are
depends on more than one property of the guest config, so pass them in
as a `Machine` struct rather than as a growing list of parameters. It
can be extended with the layout variant and other guest wide settings
later on, without touching every call site again.
The bridge list itself follows the Perl implementation: q35 machines
already define the bridges up to three in their machine definition,
pci.3 is only needed for virtio-scsi-single, and pci.4 only once a
SCSI controller above index one exists or the machine version is new
enough to always include it.
Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
pve-qemu-server-pci/Cargo.toml | 1 +
pve-qemu-server-pci/src/layout/legacy.rs | 141 ++++++++++++++++++++++-
pve-qemu-server-pci/src/lib.rs | 18 +++
pve-qemu-server-pci/src/machine.rs | 99 ++++++++++++++++
4 files changed, 255 insertions(+), 4 deletions(-)
create mode 100644 pve-qemu-server-pci/src/machine.rs
diff --git a/pve-qemu-server-pci/Cargo.toml b/pve-qemu-server-pci/Cargo.toml
index d3f4401..60c4ed1 100644
--- a/pve-qemu-server-pci/Cargo.toml
+++ b/pve-qemu-server-pci/Cargo.toml
@@ -10,6 +10,7 @@ exclude.workspace = true
rust-version.workspace = true
[dependencies]
+log.workspace = true
strum.workspace = true
pve-api-types.workspace = true
diff --git a/pve-qemu-server-pci/src/layout/legacy.rs b/pve-qemu-server-pci/src/layout/legacy.rs
index 40b3265..3f544bc 100644
--- a/pve-qemu-server-pci/src/layout/legacy.rs
+++ b/pve-qemu-server-pci/src/layout/legacy.rs
@@ -2,7 +2,7 @@ use pve_api_types::ClusterResourceHostArch;
use crate::constants::BRIDGE_SLOT_NUM;
use crate::layout::bridge_slots;
-use crate::{Bus, Device, DeviceLayout, Function, PciConfigError, Slot};
+use crate::{Bus, Device, DeviceLayout, Function, Machine, PciConfigError, Slot};
const PCI_1: [Slot; BRIDGE_SLOT_NUM] = bridge_slots(&[
single_device!(Net(6)),
@@ -369,6 +369,65 @@ pub(crate) fn print_pcie_root_port(index: u8) -> Result<String, PciConfigError>
Ok(res)
}
+/// Looks up the PCI bridge a device sits on, see
+/// [`crate::get_pci_bridge_for_device`].
+pub(crate) fn get_pci_bridge_for_device(id: &str) -> Option<u8> {
+ let new_id = map_legacy_pci_id(id);
+ // the root bus is never a bridge we have to add ourselves, so it does not
+ // matter whether it is PCI or PCIe here
+ let layout = get_legacy_pci_layout(false, is_virtio_scsi_single_id(new_id));
+ let device = new_id.parse().ok()?;
+ let addr = layout.find_device(&device).ok()?;
+ match addr.bus {
+ Bus::Pci(i) => Some(i),
+ _ => None,
+ }
+}
+
+/// Renders the bridge devices a guest needs, see
+/// [`crate::get_pci_bridges`].
+pub(crate) fn get_pci_bridges(machine: &Machine) -> Vec<String> {
+ // at most four bridges, each contributing a flag and its value
+ let mut res = Vec::with_capacity(8);
+
+ // some SCSI controllers can only have 7 disks each, so scsi14 and upwards
+ // need scsihw2 and above, which live on pci.4
+ let include_pci4 = machine.version_at_least(11, 1, 0) || machine.max_scsihw > 1;
+
+ for i in 1..=4 {
+ // q35.cfg already includes the bridges up to three
+ if i < 4 && machine.q35 {
+ continue;
+ }
+
+ if i == 3 && !machine.virtio_scsi_single() {
+ continue;
+ }
+
+ if i == 4 && !include_pci4 {
+ continue;
+ }
+
+ let id = if i == 2 && machine.legacy_igd {
+ format!("pci.{i}-igd")
+ } else {
+ format!("pci.{i}")
+ };
+
+ let addr = match print_pci_addr(&id, machine.arch) {
+ Ok(addr) => addr,
+ Err(err) => {
+ log::warn!("could not find address for bridge {id}: {err}");
+ continue;
+ }
+ };
+ res.push("-device".to_string());
+ res.push(format!("pci-bridge,id=pci.{i},chassis_nr={i}{addr}"));
+ }
+
+ res
+}
+
#[cfg(test)]
pub(crate) mod test {
use pve_api_types::ClusterResourceHostArch;
@@ -376,10 +435,10 @@ pub(crate) mod test {
use std::collections::HashSet;
use super::{
- LEGACY_PCIE_LAYOUT, get_legacy_pci_layout, print_pci_addr, print_pcie_addr,
- print_pcie_root_port,
+ LEGACY_PCIE_LAYOUT, get_legacy_pci_layout, get_pci_bridge_for_device, get_pci_bridges,
+ print_pci_addr, print_pcie_addr, print_pcie_root_port,
};
- use crate::{Bus, Device, DeviceLayout, PciAddress};
+ use crate::{Bus, Device, DeviceLayout, Machine, PciAddress};
use crate::constants::MAX_HOSTPCI_DEVICES;
@@ -652,4 +711,78 @@ pub(crate) mod test {
assert_buses_before_devices(&get_legacy_pci_layout(false, true));
assert_buses_before_devices(&get_legacy_pci_layout(true, true));
}
+
+ #[test]
+ /// The bus of a device has to agree with the address it is given. Note
+ /// that the root bus is reported as bus zero, not as no bus at all.
+ fn test_pci_bridge_for_device() {
+ for (input, bus, _) in LEGACY_ADDRS {
+ assert_eq!(get_pci_bridge_for_device(input), Some(bus), "for {input}");
+ }
+
+ assert_eq!(get_pci_bridge_for_device("no-such-device"), None);
+ }
+
+ fn machine(q35: bool, scsihw: Option<&str>, max_scsihw: u8, legacy_igd: bool) -> Machine {
+ Machine::new(
+ "x86_64",
+ q35,
+ scsihw.map(str::to_string),
+ max_scsihw,
+ legacy_igd,
+ None,
+ 9,
+ 2,
+ None,
+ )
+ }
+
+ fn bridge(nr: u8, addr: &str) -> [String; 2] {
+ [
+ "-device".to_string(),
+ format!("pci-bridge,id=pci.{nr},chassis_nr={nr}{addr}"),
+ ]
+ }
+
+ #[test]
+ fn test_pci_bridges() {
+ let pci1 = bridge(1, ",bus=pci.0,addr=0x1e");
+ let pci2 = bridge(2, ",bus=pci.0,addr=0x1f");
+ let pci3 = bridge(3, ",bus=pci.0,addr=0x5");
+ let pci4 = bridge(4, ",bus=pci.1,addr=0x1c");
+
+ let test = |machine: Machine, expected: &[&[String; 2]]| {
+ let expected: Vec<String> = expected.iter().flat_map(|b| b.iter().cloned()).collect();
+ assert_eq!(get_pci_bridges(&machine), expected);
+ };
+
+ test(machine(false, None, 0, false), &[&pci1, &pci2]);
+
+ // q35 already brings the bridges up to three along
+ test(machine(true, None, 0, false), &[]);
+
+ // every SCSI disk gets its own controller on pci.3
+ test(
+ machine(false, Some("virtio-scsi-single"), 0, false),
+ &[&pci1, &pci2, &pci3],
+ );
+
+ // scsihw2 and above live on pci.4
+ test(machine(false, Some("lsi"), 1, false), &[&pci1, &pci2]);
+ test(
+ machine(false, Some("lsi"), 2, false),
+ &[&pci1, &pci2, &pci4],
+ );
+ test(machine(true, Some("lsi"), 2, false), &[&pci4]);
+
+ // pci.4 is always added from machine version 11.1 on
+ let mut new_machine = machine(false, None, 0, false);
+ new_machine.version = (11, 1, None);
+ test(new_machine, &[&pci1, &pci2, &pci4]);
+
+ // with legacy IGD passthrough pci.2 moves behind pci.1
+ let mut expected_igd = pci2.clone();
+ expected_igd[1] = "pci-bridge,id=pci.2,chassis_nr=2,bus=pci.1,addr=0x1e".to_string();
+ test(machine(false, None, 0, true), &[&pci1, &expected_igd]);
+ }
}
diff --git a/pve-qemu-server-pci/src/lib.rs b/pve-qemu-server-pci/src/lib.rs
index 5e8b8a7..7d0bd92 100644
--- a/pve-qemu-server-pci/src/lib.rs
+++ b/pve-qemu-server-pci/src/lib.rs
@@ -10,6 +10,9 @@
pub mod constants;
+mod machine;
+pub use machine::Machine;
+
mod types;
use pve_api_types::ClusterResourceHostArch as Arch;
pub use types::*;
@@ -42,3 +45,18 @@ pub fn print_pcie_addr(id: &str) -> Result<String, PciConfigError> {
pub fn print_pcie_root_port(index: u8) -> Result<String, PciConfigError> {
layout::legacy::print_pcie_root_port(index)
}
+
+/// Returns the number of the PCI bridge `id` sits on, or `None` if it is on
+/// the root bus or not a known device.
+pub fn get_pci_bridge_for_device(id: &str) -> Option<u8> {
+ layout::legacy::get_pci_bridge_for_device(id)
+}
+
+/// Returns the QEMU arguments for all PCI bridges `machine` needs, as
+/// alternating `-device` flags and their values.
+///
+/// Bridges whose address cannot be determined are skipped with a warning
+/// rather than failing the whole guest.
+pub fn get_pci_bridges(machine: &Machine) -> Vec<String> {
+ layout::legacy::get_pci_bridges(machine)
+}
diff --git a/pve-qemu-server-pci/src/machine.rs b/pve-qemu-server-pci/src/machine.rs
new file mode 100644
index 0000000..10ee9db
--- /dev/null
+++ b/pve-qemu-server-pci/src/machine.rs
@@ -0,0 +1,99 @@
+use pve_api_types::{ClusterResourceHostArch as Arch, QemuConfigOstype, QemuConfigScsihw};
+
+/// The parts of a guest configuration that influence which PCI layout is used
+/// and which quirks have to be applied to it.
+///
+/// This is deliberately not the full guest config: it only carries what the
+/// address generation needs, so that callers can build it from whatever
+/// configuration representation they have.
+#[derive(Clone, Debug)]
+pub struct Machine {
+ pub arch: Arch,
+ pub q35: bool,
+ pub scsihw: Option<QemuConfigScsihw>,
+ /// Highest `scsihw` controller index in use. Controllers from index 2 on
+ /// live on `pci.4`, so this decides whether that bridge is needed.
+ pub max_scsihw: u8,
+ pub legacy_igd: bool,
+ pub ostype: Option<QemuConfigOstype>,
+ /// QEMU machine version as `(major, minor, pve)`, where the PVE specific
+ /// revision is optional.
+ pub version: (u16, u16, Option<u16>),
+}
+
+impl Machine {
+ /// Builds a [`Machine`] from the raw configuration values.
+ ///
+ /// Unparsable `arch`, `scsihw` and `ostype` values are not an error here:
+ /// `arch` falls back to x86_64 and the other two to `None`, which is the
+ /// same defaulting the Perl implementation does.
+ #[allow(clippy::too_many_arguments)]
+ pub fn new(
+ arch: &str,
+ q35: bool,
+ scsihw: Option<String>,
+ max_scsihw: u8,
+ legacy_igd: bool,
+ ostype: Option<String>,
+ major: u16,
+ minor: u16,
+ pve: Option<u16>,
+ ) -> Self {
+ let arch = arch.parse().unwrap_or(Arch::X8664);
+ let scsihw = scsihw.and_then(|hw| hw.parse().ok());
+ let ostype = ostype.and_then(|ostype| ostype.parse().ok());
+ Self {
+ arch,
+ q35,
+ scsihw,
+ max_scsihw,
+ legacy_igd,
+ ostype,
+ version: (major, minor, pve),
+ }
+ }
+
+ /// Whether the guest's root bus is PCIe. True for q35 machines and for
+ /// aarch64, which only has a PCIe host bridge.
+ pub fn is_pcie(&self) -> bool {
+ self.q35 || self.arch == Arch::Aarch64
+ }
+
+ /// Whether each SCSI disk gets its own controller, which needs the extra
+ /// `pci.3` bridge.
+ pub fn virtio_scsi_single(&self) -> bool {
+ self.scsihw == Some(QemuConfigScsihw::VirtioScsiSingle)
+ }
+
+ /// Whether the machine version is at least `major.minor.pve`.
+ ///
+ /// A machine without a PVE revision counts as being at least any requested
+ /// one, since the plain QEMU version already implies all PVE changes made
+ /// for it.
+ pub fn version_at_least(&self, major: u16, minor: u16, pve: u16) -> bool {
+ if self.version.0 > major {
+ return true;
+ }
+ if self.version.0 < major {
+ return false;
+ }
+
+ if self.version.1 > minor {
+ return true;
+ }
+ if self.version.1 < minor {
+ return false;
+ }
+
+ if let Some(our_pve) = self.version.2 {
+ if our_pve > pve {
+ return true;
+ }
+ if our_pve < pve {
+ return false;
+ }
+ }
+
+ true
+ }
+}
--
2.47.3
next prev parent reply other threads:[~2026-09-22 10:56 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-22 10:55 [RFC proxmox-perl-rs/qemu-server/qemu-server-rs 0/9] pci-handling rewrite (part 1) Dominik Csapak
2026-09-22 10:55 ` [PATCH pve-qemu-server-rs 1/9] add pve-qemu-server-pci crate for guest PCI address generation Dominik Csapak
2026-09-22 10:55 ` Dominik Csapak [this message]
2026-09-22 10:55 ` [PATCH pve-qemu-server-rs 3/9] pci: layout: add v2 PCI and PCIe layouts Dominik Csapak
2026-09-22 10:55 ` [PATCH pve-qemu-server-rs 4/9] fixup! add pve-qemu-server-pci crate for guest PCI address generation Dominik Csapak
2026-09-22 10:55 ` [PATCH proxmox-perl-rs 5/9] pve: add bindings for `pve-qemu-server-pci` crate Dominik Csapak
2026-09-22 10:55 ` [PATCH proxmox-perl-rs 6/9] pve: pci bindings: add bindings for the `Machine` struct Dominik Csapak
2026-09-22 10:55 ` [PATCH qemu-server 7/9] pci: use PVE::RS::PCI bindings Dominik Csapak
2026-09-22 10:55 ` [PATCH qemu-server 8/9] helpers: factor out the version parts parsing Dominik Csapak
2026-09-22 10:55 ` [PATCH qemu-server 9/9] pci: bridges: use the rust `Machine` struct to pass parameters Dominik Csapak
2026-09-22 11:06 ` [RFC proxmox-perl-rs/qemu-server/qemu-server-rs 0/9] pci-handling rewrite (part 1) Dominik Csapak
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260922105550.2084078-3-d.csapak@proxmox.com \
--to=d.csapak@proxmox.com \
--cc=pve-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox