From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [45.144.208.40]) by lore.proxmox.com (Postfix) with ESMTPS id 3C10D1FF0AA for ; Tue, 22 Sep 2026 12:57:13 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 2E7E1215BE; Tue, 22 Sep 2026 12:56:16 +0200 (CEST) From: Dominik Csapak To: pve-devel@lists.proxmox.com Subject: [PATCH qemu-server 7/9] pci: use PVE::RS::PCI bindings Date: Tue, 22 Sep 2026 12:55:38 +0200 Message-ID: <20260922105550.2084078-8-d.csapak@proxmox.com> X-Mailer: git-send-email 2.47.3 In-Reply-To: <20260922105550.2084078-1-d.csapak@proxmox.com> References: <20260922105550.2084078-1-d.csapak@proxmox.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-SPAM-LEVEL: Spam detection results: 0 AWL -0.543 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) KAM_MAILER 2 Automated Mailer Tag Left in Email 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: ICH43FBUXJ47CCGPRVQDXX7M6QL3JYLS X-Message-ID-Hash: ICH43FBUXJ47CCGPRVQDXX7M6QL3JYLS X-MailFrom: d.csapak@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: This makes use of the rust PCI bindings, where the list of addresses live. With that, the duplicated id test is no longer necessary (and possible), this is now checked by the rust crate itself. Signed-off-by: Dominik Csapak --- src/PVE/QemuServer/PCI.pm | 272 ++------------------------------ src/test/Makefile | 5 +- src/test/run_pci_addr_checks.pl | 141 ----------------- 3 files changed, 10 insertions(+), 408 deletions(-) delete mode 100755 src/test/run_pci_addr_checks.pl diff --git a/src/PVE/QemuServer/PCI.pm b/src/PVE/QemuServer/PCI.pm index d7dc6121..c3f4e918 100644 --- a/src/PVE/QemuServer/PCI.pm +++ b/src/PVE/QemuServer/PCI.pm @@ -7,6 +7,7 @@ use IO::File; use PVE::JSONSchema; use PVE::Mapping::PCI; +use PVE::RS::PCI; use PVE::SysFSTools; use PVE::Tools; @@ -152,264 +153,28 @@ EODESCR }; PVE::JSONSchema::register_standard_option("pve-qm-hostpci", $hostpcidesc); -my $pci_addr_map; - -sub get_pci_addr_map { - $pci_addr_map = { - piix3 => { bus => 0, addr => 1, conflict_ok => qw(ehci) }, - ehci => { bus => 0, addr => 1, conflict_ok => qw(piix3) }, # instead of piix3 on arm - vga => { bus => 0, addr => 2, conflict_ok => qw(legacy-igd) }, - 'legacy-igd' => { bus => 0, addr => 2, conflict_ok => qw(vga) }, # legacy-igd requires vga=none - balloon0 => { bus => 0, addr => 3 }, - watchdog => { bus => 0, addr => 4 }, - scsihw0 => { bus => 0, addr => 5, conflict_ok => qw(pci.3) }, - 'pci.3' => { bus => 0, addr => 5, conflict_ok => qw(scsihw0) }, # also used for virtio-scsi-single bridge - scsihw1 => { bus => 0, addr => 6 }, - ahci0 => { bus => 0, addr => 7 }, - qga0 => { bus => 0, addr => 8 }, - spice => { bus => 0, addr => 9 }, - virtio0 => { bus => 0, addr => 10 }, - virtio1 => { bus => 0, addr => 11 }, - virtio2 => { bus => 0, addr => 12 }, - virtio3 => { bus => 0, addr => 13 }, - virtio4 => { bus => 0, addr => 14 }, - virtio5 => { bus => 0, addr => 15 }, - hostpci0 => { bus => 0, addr => 16 }, - hostpci1 => { bus => 0, addr => 17 }, - net0 => { bus => 0, addr => 18 }, - net1 => { bus => 0, addr => 19 }, - net2 => { bus => 0, addr => 20 }, - net3 => { bus => 0, addr => 21 }, - net4 => { bus => 0, addr => 22 }, - net5 => { bus => 0, addr => 23 }, - vga1 => { bus => 0, addr => 24 }, - vga2 => { bus => 0, addr => 25 }, - vga3 => { bus => 0, addr => 26 }, - hostpci2 => { bus => 0, addr => 27 }, - hostpci3 => { bus => 0, addr => 28 }, - #addr29 : usb-host (pve-usb.cfg) - 'pci.1' => { bus => 0, addr => 30 }, - 'pci.2' => { bus => 0, addr => 31 }, - 'net6' => { bus => 1, addr => 1 }, - 'net7' => { bus => 1, addr => 2 }, - 'net8' => { bus => 1, addr => 3 }, - 'net9' => { bus => 1, addr => 4 }, - 'net10' => { bus => 1, addr => 5 }, - 'net11' => { bus => 1, addr => 6 }, - 'net12' => { bus => 1, addr => 7 }, - 'net13' => { bus => 1, addr => 8 }, - 'net14' => { bus => 1, addr => 9 }, - 'net15' => { bus => 1, addr => 10 }, - 'net16' => { bus => 1, addr => 11 }, - 'net17' => { bus => 1, addr => 12 }, - 'net18' => { bus => 1, addr => 13 }, - 'net19' => { bus => 1, addr => 14 }, - 'net20' => { bus => 1, addr => 15 }, - 'net21' => { bus => 1, addr => 16 }, - 'net22' => { bus => 1, addr => 17 }, - 'net23' => { bus => 1, addr => 18 }, - 'net24' => { bus => 1, addr => 19 }, - 'net25' => { bus => 1, addr => 20 }, - 'net26' => { bus => 1, addr => 21 }, - 'net27' => { bus => 1, addr => 22 }, - 'net28' => { bus => 1, addr => 23 }, - 'net29' => { bus => 1, addr => 24 }, - 'net30' => { bus => 1, addr => 25 }, - 'net31' => { bus => 1, addr => 26 }, - 'xhci' => { bus => 1, addr => 27 }, - 'pci.4' => { bus => 1, addr => 28 }, - 'rng0' => { bus => 1, addr => 29 }, - 'pci.2-igd' => { bus => 1, addr => 30 }, # replaces pci.2 in case a legacy IGD device is passed through - 'virtio6' => { bus => 2, addr => 1 }, - 'virtio7' => { bus => 2, addr => 2 }, - 'virtio8' => { bus => 2, addr => 3 }, - 'virtio9' => { bus => 2, addr => 4 }, - 'virtio10' => { bus => 2, addr => 5 }, - 'virtio11' => { bus => 2, addr => 6 }, - 'virtio12' => { bus => 2, addr => 7 }, - 'virtio13' => { bus => 2, addr => 8 }, - 'virtio14' => { bus => 2, addr => 9 }, - 'virtio15' => { bus => 2, addr => 10 }, - 'ivshmem' => { bus => 2, addr => 11 }, - 'audio0' => { bus => 2, addr => 12 }, - hostpci4 => { bus => 2, addr => 13 }, - hostpci5 => { bus => 2, addr => 14 }, - hostpci6 => { bus => 2, addr => 15 }, - hostpci7 => { bus => 2, addr => 16 }, - hostpci8 => { bus => 2, addr => 17 }, - hostpci9 => { bus => 2, addr => 18 }, - hostpci10 => { bus => 2, addr => 19 }, - hostpci11 => { bus => 2, addr => 20 }, - hostpci12 => { bus => 2, addr => 21 }, - hostpci13 => { bus => 2, addr => 22 }, - hostpci14 => { bus => 2, addr => 23 }, - hostpci15 => { bus => 2, addr => 24 }, - 'virtioscsi0' => { bus => 3, addr => 1 }, - 'virtioscsi1' => { bus => 3, addr => 2 }, - 'virtioscsi2' => { bus => 3, addr => 3 }, - 'virtioscsi3' => { bus => 3, addr => 4 }, - 'virtioscsi4' => { bus => 3, addr => 5 }, - 'virtioscsi5' => { bus => 3, addr => 6 }, - 'virtioscsi6' => { bus => 3, addr => 7 }, - 'virtioscsi7' => { bus => 3, addr => 8 }, - 'virtioscsi8' => { bus => 3, addr => 9 }, - 'virtioscsi9' => { bus => 3, addr => 10 }, - 'virtioscsi10' => { bus => 3, addr => 11 }, - 'virtioscsi11' => { bus => 3, addr => 12 }, - 'virtioscsi12' => { bus => 3, addr => 13 }, - 'virtioscsi13' => { bus => 3, addr => 14 }, - 'virtioscsi14' => { bus => 3, addr => 15 }, - 'virtioscsi15' => { bus => 3, addr => 16 }, - 'virtioscsi16' => { bus => 3, addr => 17 }, - 'virtioscsi17' => { bus => 3, addr => 18 }, - 'virtioscsi18' => { bus => 3, addr => 19 }, - 'virtioscsi19' => { bus => 3, addr => 20 }, - 'virtioscsi20' => { bus => 3, addr => 21 }, - 'virtioscsi21' => { bus => 3, addr => 22 }, - 'virtioscsi22' => { bus => 3, addr => 23 }, - 'virtioscsi23' => { bus => 3, addr => 24 }, - 'virtioscsi24' => { bus => 3, addr => 25 }, - 'virtioscsi25' => { bus => 3, addr => 26 }, - 'virtioscsi26' => { bus => 3, addr => 27 }, - 'virtioscsi27' => { bus => 3, addr => 28 }, - 'virtioscsi28' => { bus => 3, addr => 29 }, - 'virtioscsi29' => { bus => 3, addr => 30 }, - 'virtioscsi30' => { bus => 3, addr => 31 }, - 'scsihw2' => { bus => 4, addr => 1 }, - 'scsihw3' => { bus => 4, addr => 2 }, - 'scsihw4' => { bus => 4, addr => 3 }, - } - if !defined($pci_addr_map); - return $pci_addr_map; -} - -my $get_addr_mapping_from_id = sub { - my ($map, $id) = @_; - - my $d = $map->{$id}; - return if !defined($d) || !defined($d->{bus}) || !defined($d->{addr}); - - return { bus => $d->{bus}, addr => sprintf("0x%x", $d->{addr}) }; -}; - sub get_pci_bridge_for_device { my ($id) = @_; - my $map = get_pci_addr_map(); - if (my $d = $get_addr_mapping_from_id->($map, $id)) { - return $d->{bus}; - } - - return; + return PVE::RS::PCI::get_pci_bridge_for_device($id); } sub print_pci_addr { my ($id, $arch) = @_; - die "aarch64 cannot use IDE devices\n" if $arch eq 'aarch64' && $id =~ /^ide/; - - my $res = ''; - - my $map = get_pci_addr_map(); - if (my $d = $get_addr_mapping_from_id->($map, $id)) { - # Using same bus slots on all HW, so we need to check special cases here. For aarch64, the - # virt machine has an initial pcie.0. The other pci bridges that get added are called pci.N. - my $busname = $arch eq 'aarch64' && $d->{bus} eq 0 ? 'pcie' : 'pci'; - - $res = ",bus=$busname.$d->{bus},addr=$d->{addr}"; - } - - return $res; -} - -my $pcie_addr_map; - -sub get_pcie_addr_map { - $pcie_addr_map = { - vga => { bus => 'pcie.0', addr => 1 }, - hostpci0 => { bus => "ich9-pcie-port-1", addr => 0 }, - hostpci1 => { bus => "ich9-pcie-port-2", addr => 0 }, - hostpci2 => { bus => "ich9-pcie-port-3", addr => 0 }, - hostpci3 => { bus => "ich9-pcie-port-4", addr => 0 }, - hostpci4 => { bus => "ich9-pcie-port-5", addr => 0 }, - hostpci5 => { bus => "ich9-pcie-port-6", addr => 0 }, - hostpci6 => { bus => "ich9-pcie-port-7", addr => 0 }, - hostpci7 => { bus => "ich9-pcie-port-8", addr => 0 }, - hostpci8 => { bus => "ich9-pcie-port-9", addr => 0 }, - hostpci9 => { bus => "ich9-pcie-port-10", addr => 0 }, - hostpci10 => { bus => "ich9-pcie-port-11", addr => 0 }, - hostpci11 => { bus => "ich9-pcie-port-12", addr => 0 }, - hostpci12 => { bus => "ich9-pcie-port-13", addr => 0 }, - hostpci13 => { bus => "ich9-pcie-port-14", addr => 0 }, - hostpci14 => { bus => "ich9-pcie-port-15", addr => 0 }, - hostpci15 => { bus => "ich9-pcie-port-16", addr => 0 }, - # win7 is picky about pcie assignments - hostpci0bus0 => { bus => "pcie.0", addr => 16 }, - hostpci1bus0 => { bus => "pcie.0", addr => 17 }, - hostpci2bus0 => { bus => "pcie.0", addr => 18 }, - hostpci3bus0 => { bus => "pcie.0", addr => 19 }, - ivshmem => { bus => 'pcie.0', addr => 20 }, - hostpci4bus0 => { bus => "pcie.0", addr => 9 }, - hostpci5bus0 => { bus => "pcie.0", addr => 10 }, - hostpci6bus0 => { bus => "pcie.0", addr => 11 }, - hostpci7bus0 => { bus => "pcie.0", addr => 12 }, - hostpci8bus0 => { bus => "pcie.0", addr => 13 }, - hostpci9bus0 => { bus => "pcie.0", addr => 14 }, - hostpci10bus0 => { bus => "pcie.0", addr => 15 }, - hostpci11bus0 => { bus => "pcie.0", addr => 21 }, - hostpci12bus0 => { bus => "pcie.0", addr => 22 }, - hostpci13bus0 => { bus => "pcie.0", addr => 23 }, - hostpci14bus0 => { bus => "pcie.0", addr => 24 }, - hostpci15bus0 => { bus => "pcie.0", addr => 25 }, - } - if !defined($pcie_addr_map); - - return $pcie_addr_map; + return PVE::RS::PCI::print_pci_addr($id, $arch); } sub print_pcie_addr { my ($id) = @_; - my $res = ''; - - my $map = get_pcie_addr_map($id); - if (my $d = $get_addr_mapping_from_id->($map, $id)) { - $res = ",bus=$d->{bus},addr=$d->{addr}"; - } - - return $res; + return PVE::RS::PCI::print_pcie_addr($id); } -# Generates the device strings for additional pcie root ports. The first 4 pcie -# root ports are defined in the pve-q35*.cfg files. my sub print_pcie_root_port { my ($i) = @_; - my $res = ''; - - my $root_port_addresses = { - 4 => "10.0", - 5 => "10.1", - 6 => "10.2", - 7 => "10.3", - 8 => "10.4", - 9 => "10.5", - 10 => "10.6", - 11 => "10.7", - 12 => "11.0", - 13 => "11.1", - 14 => "11.2", - 15 => "11.3", - }; - - if (defined($root_port_addresses->{$i})) { - my $id = $i + 1; - $res = "pcie-root-port,id=ich9-pcie-port-${id}"; - $res .= ",addr=$root_port_addresses->{$i}"; - $res .= ",x-speed=16,x-width=32,multifunction=on,bus=pcie.0"; - $res .= ",port=${id},chassis=${id}"; - } - return $res; + return PVE::RS::PCI::print_pcie_root_port($i); } # returns the parsed pci config but parses the 'host' part into @@ -886,17 +651,11 @@ sub reserve_pci_usage { sub get_pci_bridges { my ($conf, $arch, $q35, $max_scsihw, $version_guard) = @_; - my $bridges = { - # 0 => 1, always present - 1 => 1, - 2 => 1, - }; - - $bridges->{3} = 1 if ($conf->{scsihw} // '') =~ m/^virtio-scsi-single/; + my $virtio_scsi_single = ($conf->{scsihw} // '') =~ m/^virtio-scsi-single/; # some scsi controllers can only have 7 scsi disks per controller, # so scsi14 and upwards need scsihw2,3,4 which live on bridge 4 - $bridges->{4} = 1 if $max_scsihw > 1 || $version_guard->(11, 1); + my $include_pci4 = $max_scsihw > 1 || $version_guard->(11, 1); # use cheap legacy igd check instead of a full parse_hostpci my $legacy_igd = 0; @@ -910,21 +669,8 @@ sub get_pci_bridges { } } - my $devices = []; - for my $k (sort { $a <=> $b } keys %$bridges) { - next if $q35 && $k < 4; # q35.cfg already includes bridges up to 3 - - my $k_name = $k; - if ($k == 2 && $legacy_igd) { - $k_name = "$k-igd"; - } - my $pciaddr = print_pci_addr("pci.$k_name", $arch); - my $devstr = "pci-bridge,id=pci.$k,chassis_nr=$k$pciaddr"; - - push @$devices, '-device', $devstr; - } - - return $devices; + return PVE::RS::PCI::get_pci_bridges($arch, $q35, $virtio_scsi_single, $legacy_igd, + $include_pci4); } 1; diff --git a/src/test/Makefile b/src/test/Makefile index 702c9c03..07fadb99 100644 --- a/src/test/Makefile +++ b/src/test/Makefile @@ -1,6 +1,6 @@ all: test -test: test_snapshot test_cfg_to_cmd test_cfg_to_cmd_aarch64 test_hotplug test_hotplug_aarch64 test_pci_addr_conflicts test_pci_reservation test_qemu_img_convert test_migration test_restore_config test_parse_config +test: test_snapshot test_cfg_to_cmd test_cfg_to_cmd_aarch64 test_hotplug test_hotplug_aarch64 test_pci_reservation test_qemu_img_convert test_migration test_restore_config test_parse_config test_snapshot: run_snapshot_tests.pl ./run_snapshot_tests.pl @@ -21,9 +21,6 @@ test_hotplug_aarch64: run_hotplug_tests.pl TestsCommon/*.pm hotplug/aarch64/*.co test_qemu_img_convert: run_qemu_img_convert_tests.pl perl -I../ ./run_qemu_img_convert_tests.pl -test_pci_addr_conflicts: run_pci_addr_checks.pl - ./run_pci_addr_checks.pl - test_pci_reservation: run_pci_reservation_tests.pl ./run_pci_reservation_tests.pl diff --git a/src/test/run_pci_addr_checks.pl b/src/test/run_pci_addr_checks.pl deleted file mode 100755 index 866c43eb..00000000 --- a/src/test/run_pci_addr_checks.pl +++ /dev/null @@ -1,141 +0,0 @@ -#!/usr/bin/perl - -use strict; -use warnings; -use experimental 'smartmatch'; - -use lib qw(..); - -use Test::More; - -use PVE::Tools qw(file_get_contents); -use PVE::QemuServer::PCI; - -my $qemu_cfg_base_path = "../usr"; - -# not our format but that what QEMU gets passed with '-readconfig' -sub slurp_qemu_config { - my ($fn) = @_; - - my $raw = file_get_contents($fn); - - my $lineno = 0; - my $cfg = {}; - my $group; - my $skip_to_next_group; - while ($raw =~ /^\h*(.*?)\h*$/gm) { - my $line = $1; - $lineno++; - next if !$line || $line =~ /^#/; - - # tried to follow qemu's qemu_config_parse function - if ($line =~ /\[(\S{1,63}) "([^"\]]{1,63})"\]/) { - $group = $2; - $skip_to_next_group = 0; - if ($1 ne 'device') { - $group = undef; - $skip_to_next_group = 1; - } - } elsif ($line =~ /\[([^\]]{1,63})\]/) { - $group = undef; - $skip_to_next_group = 1; - } elsif ($group) { - if ($line =~ /(\S{1,63}) = "([^\"]{1,1023})"/) { - my ($k, $v) = ($1, $2); - $cfg->{$group}->{$k} = $v; - } else { - print "ignoring $fn:$lineno: $line\n"; - } - } else { - warn "ignore $fn:$lineno, currently no group\n" if !$skip_to_next_group; - } - } - - return $cfg; -} - -sub extract_qemu_config_addrs { - my ($qemu_cfg) = @_; - - my $addr_map = {}; - for my $k (keys %$qemu_cfg) { - my $v = $qemu_cfg->{$k}; - next if !$v || !defined($v->{bus}) || !defined($v->{addr}); - - my $bus = $v->{bus}; - $bus =~ s/pci\.//; - - $addr_map->{$k} = { bus => $bus, addr => $v->{addr} }; - } - - return $addr_map; -} - -print "testing PCI(e) address conflicts\n"; - -# exec tests - -#FIXME: make cross PCI <-> PCIe check sense at all?? -my $addr_map = {}; -my ($fail, $ignored) = (0, 0); - -sub check_conflict { - my ($id, $what, $ignore_if_same_key) = @_; - - my ($bus, $addr) = $what->@{qw(bus addr)}; - my $full_addr = "$bus:$addr"; - - if (defined(my $conflict = $addr_map->{$full_addr})) { - if (my @ignores = $what->{conflict_ok}) { - if ($conflict ~~ @ignores) { - note("OK: ignore conflict for '$full_addr' between '$id' and '$conflict'"); - $ignored++; - return; - } - } - # this allows to read multiple pve-*.cfg qemu configs, and check them - # normally their OK if they conflict is on the same key. Else TODO?? - return if $ignore_if_same_key && $id eq $conflict; - - note("ERR: conflict for '$full_addr' between '$id' and '$conflict'"); - $fail++; - } else { - $addr_map->{$full_addr} = $id; - } -} - -my $pci_map = PVE::QemuServer::PCI::get_pci_addr_map(); -while (my ($id, $what) = each %$pci_map) { - check_conflict($id, $what); -} - -my $pcie_map = PVE::QemuServer::PCI::get_pcie_addr_map(); -while (my ($id, $what) = each %$pcie_map) { - check_conflict($id, $what); -} - -my $pve_qm_cfg = slurp_qemu_config("$qemu_cfg_base_path/pve-q35.cfg"); -my $pve_qm_cfg_map = extract_qemu_config_addrs($pve_qm_cfg); -while (my ($id, $what) = each %$pve_qm_cfg_map) { - check_conflict($id, $what); -} - -# FIXME: restart with clean conflict $addr_map with only get_pci*_addr_map ones? -my $pve_qm4_cfg = slurp_qemu_config("$qemu_cfg_base_path/pve-q35-4.0.cfg"); -my $pve_qm4_cfg_map = extract_qemu_config_addrs($pve_qm4_cfg); -while (my ($id, $what) = each %$pve_qm4_cfg_map) { - check_conflict($id, $what, 1); -} -my $pve_qm_usb_cfg = slurp_qemu_config("$qemu_cfg_base_path/pve-usb.cfg"); -my $pve_qm_usb_cfg_map = extract_qemu_config_addrs($pve_qm_usb_cfg); -while (my ($id, $what) = each %$pve_qm_usb_cfg_map) { - check_conflict($id, $what, 1); -} - -if ($fail) { - fail("PCI(e) address conflict check, ignored: $ignored, conflicts: $fail"); -} else { - pass("PCI(e) address conflict check, ignored: $ignored"); -} - -done_testing(); -- 2.47.3