pci: add conflict tests

best viewed with: git show -w

Signed-off-by: Thomas Lamprecht <t.lamprecht@proxmox.com>

PVE/QemuServer/PCI.pm

@@ -8,14 +8,16 @@ print_pcie_addr
 print_pcie_root_port
 );
 
-my $devices = {
-    piix3 => { bus => 0, addr => 1 },
-    ehci => { bus => 0, addr => 1 }, # instead of piix3 on arm
+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 },
 	balloon0 => { bus => 0, addr => 3 },
 	watchdog => { bus => 0, addr => 4 },
-    scsihw0 => { bus => 0, addr => 5 },
-    'pci.3' => { bus => 0, addr => 5 }, #can also be used for virtio-scsi-single bridge
+	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 },
@@ -124,6 +126,17 @@ my $devices = {
 	'virtioscsi28' => { bus => 3, addr => 29 },
 	'virtioscsi29' => { bus => 3, addr => 30 },
 	'virtioscsi30' => { bus => 3, addr => 31 },
+    } if !defined($pci_addr_map);
+    return $pci_addr_map;
+}
+
+my $get_addr_mapping_from_id = sub {
+    my ($map, $id) = @_;
+
+    my $d = $map->{$id};
+    return undef if !defined($d) || !defined($d->{bus}) || !defined($d->{addr});
+
+    return { bus => $d->{bus}, addr => sprintf("0x%x", $d->{addr}) };
 };
 
 sub print_pci_addr {
@@ -131,30 +144,25 @@ sub print_pci_addr {
 
     my $res = '';
 
-    # We use the same bus slots on all hardware, so we need to check special
-    # cases here:
+    # using same bus slots on all HW, so we need to check special cases here:
     my $busname = 'pci';
     if ($arch eq 'aarch64' && $machine =~ /^virt/) {
-	die "aarch64/virt cannot use IDE devices\n"
-	    if $id =~ /^ide/;
+	die "aarch64/virt cannot use IDE devices\n" if $id =~ /^ide/;
 	$busname = 'pcie';
     }
 
-    if (defined($devices->{$id}->{bus}) && defined($devices->{$id}->{addr})) {
-	   my $addr = sprintf("0x%x", $devices->{$id}->{addr});
-	   my $bus = $devices->{$id}->{bus};
-	   $res = ",bus=$busname.$bus,addr=$addr";
-	   $bridges->{$bus} = 1 if $bridges;
+    my $map = get_pci_addr_map();
+    if (my $d = $get_addr_mapping_from_id->($map, $id)) {
+	$res = ",bus=$busname.$d->{bus},addr=$d->{addr}";
+	$bridges->{$d->{bus}} = 1 if $bridges;
     }
+
     return $res;
-
 }
 
-sub print_pcie_addr {
-    my ($id) = @_;
-
-    my $res = '';
-    my $devices = {
+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 },
@@ -190,15 +198,22 @@ sub print_pcie_addr {
 	hostpci13bus0 => { bus => "pcie.0", addr => 22 },
 	hostpci14bus0 => { bus => "pcie.0", addr => 23 },
 	hostpci15bus0 => { bus => "pcie.0", addr => 24 },
-    };
+    } if !defined($pcie_addr_map);
 
-    if (defined($devices->{$id}->{bus}) && defined($devices->{$id}->{addr})) {
-	   my $addr = sprintf("0x%x", $devices->{$id}->{addr});
-	   my $bus = $devices->{$id}->{bus};
-	   $res = ",bus=$bus,addr=$addr";
+    return $pcie_addr_map;
 }
-    return $res;
 
+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;
 }
 
 # Generates the device strings for additional pcie root ports. The first 4 pcie

test/Makefile

@@ -1,6 +1,6 @@
 all: test
 
-test: test_snapshot test_ovf test_cfg_to_cmd
+test: test_snapshot test_ovf test_cfg_to_cmd test_pci_addr_conflicts
 
 test_snapshot: run_snapshot_tests.pl
 	./run_snapshot_tests.pl
@@ -11,3 +11,6 @@ test_ovf: run_ovf_tests.pl
 
 test_cfg_to_cmd: run_config2command_tests.pl cfg2cmd/*.conf
 	perl -I../ ./run_config2command_tests.pl
+
+test_pci_addr_conflicts: run_pci_addr_checks.pl
+	./run_pci_addr_checks.pl

test/run_pci_addr_checks.pl

@@ -0,0 +1,58 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+use experimental 'smartmatch';
+
+use lib qw(..);
+
+use Test::More;
+
+use PVE::QemuServer::PCI;
+
+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) = @_;
+
+    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;
+	    }
+	}
+	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);
+}
+
+if ($fail) {
+    fail("PCI(e) address conflict check, ignored: $ignored, conflicts: $fail");
+} else {
+    pass("PCI(e) address conflict check, ignored: $ignored");
+}
+
+done_testing();