[pve-devel] [PATCH-SERIES qemu-server master+stable-bookworm 0/3] fix #6466: aarch64: re-allow PXE boot on OVMF

[pve-devel] [PATCH-SERIES qemu-server master+stable-bookworm 0/3] fix #6466: aarch64: re-allow PXE boot on OVMF

[Fiona Ebner]

Fix an indirect regression to make booting via PXE for OVMF possible
again. For security reasons, OVMF requires an RNG device to boot via
PXE and this series makes it possible to add an RNG device for
aarch64.

First two patches are cleanups also included in [0].

For aarch64, the virt machine has an initial pcie.0 bus. The other pci
bridges that get added on top are called pci.N, see the relevant
section in config_to_command() which adds them.

[0]: https://lore.proxmox.com/pve-devel/20250618130209.90649-1-f.ebner@proxmox.com/

Fiona Ebner (3):
  cfg2cmd: print vga: fix call to print_pcie_addr()
  pci: code cleanup: remove superfluous machine type parameter from
    print_pci_addr
  fix #6466: aarch64: pci: properly print higher-index PCI bridge
    addresses

 src/PVE/QemuServer.pm     | 81 ++++++++++++---------------------------
 src/PVE/QemuServer/PCI.pm | 19 +++++----
 src/PVE/QemuServer/RNG.pm |  4 +-
 src/PVE/QemuServer/USB.pm |  8 ++--
 4 files changed, 40 insertions(+), 72 deletions(-)

-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH qemu-server 1/3] cfg2cmd: print vga: fix call to print_pcie_addr()

[Fiona Ebner]

The function print_pcie_addr() only takes a single parameter.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/QemuServer.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 8eecbd53..23fde3f8 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -1827,7 +1827,7 @@ sub print_vga_device {
     my $pciaddr;
     if ($q35 && $vgaid eq 'vga') {
         # the first display uses pcie.0 bus on q35 machines
-        $pciaddr = print_pcie_addr($vgaid, $bridges, $arch, $machine);
+        $pciaddr = print_pcie_addr($vgaid);
     } else {
         $pciaddr = print_pci_addr($vgaid, $bridges, $arch, $machine);
     }
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH qemu-server 2/3] pci: code cleanup: remove superfluous machine type parameter from print_pci_addr

[Fiona Ebner]

The only supported machine for aarch64 is 'virt', so there is no need
to check if that is the machine. Also many (all?) other machines for
aarch64 in QEMU also don't have a 'pci' bus by default.

The parameter is also transitively removed from the functions:
1. print_hostpci_devices()
2. print_rng_device_commandline()
3. get_usb_controllers()
4. print_netdevice_full()
5. print_vga_device()

Should this ever be required in the future again, or also for the
$arch itself right now, it would be nicer to properly abstract this
away instead of passing the parameters along everywhere, e.g. by using
a class.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/QemuServer.pm     | 79 ++++++++++++---------------------------
 src/PVE/QemuServer/PCI.pm | 10 ++---
 src/PVE/QemuServer/RNG.pm |  4 +-
 src/PVE/QemuServer/USB.pm |  8 ++--
 4 files changed, 35 insertions(+), 66 deletions(-)

diff --git a/src/PVE/QemuServer.pm b/src/PVE/QemuServer.pm
index 23fde3f8..22b6a11e 100644
--- a/src/PVE/QemuServer.pm
+++ b/src/PVE/QemuServer.pm
@@ -1396,7 +1396,7 @@ sub print_drivedevice_full {
 
     my $drive_id = PVE::QemuServer::Drive::get_drive_id($drive);
     if ($drive->{interface} eq 'virtio') {
-        my $pciaddr = print_pci_addr("$drive_id", $bridges, $arch, $machine_type);
+        my $pciaddr = print_pci_addr("$drive_id", $bridges, $arch);
         $device = "virtio-blk-pci,drive=drive-$drive_id,id=${drive_id}${pciaddr}";
         $device .= ",iothread=iothread-$drive_id" if $drive->{iothread};
     } elsif ($drive->{interface} eq 'scsi') {
@@ -1657,24 +1657,14 @@ sub print_pbs_blockdev {
 }
 
 sub print_netdevice_full {
-    my (
-        $vmid,
-        $conf,
-        $net,
-        $netid,
-        $bridges,
-        $use_old_bios_files,
-        $arch,
-        $machine_type,
-        $machine_version,
-    ) = @_;
+    my ($vmid, $conf, $net, $netid, $bridges, $use_old_bios_files, $arch, $machine_version) = @_;
 
     my $device = $net->{model};
     if ($net->{model} eq 'virtio') {
         $device = 'virtio-net-pci';
     }
 
-    my $pciaddr = print_pci_addr("$netid", $bridges, $arch, $machine_type);
+    my $pciaddr = print_pci_addr("$netid", $bridges, $arch);
     my $tmpstr = "$device,mac=$net->{macaddr},netdev=$netid$pciaddr,id=$netid";
     if ($net->{queues} && $net->{queues} > 1 && $net->{model} eq 'virtio') {
         # Consider we have N queues, the number of vectors needed is 2 * N + 2, i.e., one per in
@@ -1777,7 +1767,7 @@ my $vga_map = {
 };
 
 sub print_vga_device {
-    my ($conf, $vga, $arch, $machine_version, $machine, $id, $qxlnum, $bridges) = @_;
+    my ($conf, $vga, $arch, $machine_version, $id, $qxlnum, $bridges) = @_;
 
     my $type = $vga_map->{ $vga->{type} };
     if ($arch eq 'aarch64' && defined($type) && $type eq 'virtio-vga') {
@@ -1829,7 +1819,7 @@ sub print_vga_device {
         # the first display uses pcie.0 bus on q35 machines
         $pciaddr = print_pcie_addr($vgaid);
     } else {
-        $pciaddr = print_pci_addr($vgaid, $bridges, $arch, $machine);
+        $pciaddr = print_pci_addr($vgaid, $bridges, $arch);
     }
 
     if ($vga->{type} eq 'virtio-gl') {
@@ -3761,9 +3751,8 @@ sub config_to_command {
     }
 
     # add usb controllers
-    my @usbcontrollers = PVE::QemuServer::USB::get_usb_controllers(
-        $conf, $bridges, $arch, $machine_type, $machine_version,
-    );
+    my @usbcontrollers =
+        PVE::QemuServer::USB::get_usb_controllers($conf, $bridges, $arch, $machine_version);
     push @$devices, @usbcontrollers if @usbcontrollers;
 
     my ($vga, $qxlnum) = get_vga_properties($conf, $arch, $machine_version, $winversion);
@@ -3787,15 +3776,7 @@ sub config_to_command {
     # host pci device passthrough
     my ($kvm_off, $gpu_passthrough, $legacy_igd, $pci_devices) =
         PVE::QemuServer::PCI::print_hostpci_devices(
-            $vmid,
-            $conf,
-            $devices,
-            $vga,
-            $winversion,
-            $bridges,
-            $arch,
-            $machine_type,
-            $bootorder,
+            $vmid, $conf, $devices, $vga, $winversion, $bridges, $arch, $bootorder,
         );
 
     # usb devices
@@ -3839,7 +3820,7 @@ sub config_to_command {
     }
 
     if (min_version($machine_version, 4, 0) && (my $audio = conf_has_audio($conf))) {
-        my $audiopciaddr = print_pci_addr("audio0", $bridges, $arch, $machine_type);
+        my $audiopciaddr = print_pci_addr("audio0", $bridges, $arch);
         my $audio_devs = audio_devs($audio, $audiopciaddr, $machine_version);
         push @$devices, @$audio_devs;
     }
@@ -3884,9 +3865,7 @@ sub config_to_command {
 
     if ($vga->{type} && $vga->{type} !~ m/^serial\d+$/ && $vga->{type} ne 'none') {
         push @$devices, '-device',
-            print_vga_device(
-                $conf, $vga, $arch, $machine_version, $machine_type, undef, $qxlnum, $bridges,
-            );
+            print_vga_device($conf, $vga, $arch, $machine_version, undef, $qxlnum, $bridges);
 
         push @$cmd, '-display', 'egl-headless,gl=core' if $vga->{type} eq 'virtio-gl'; # VIRGL
 
@@ -3956,7 +3935,7 @@ sub config_to_command {
         push @$devices, '-chardev', "socket,path=$qgasocket,server=on,wait=off,id=qga0";
 
         if (!$guest_agent->{type} || $guest_agent->{type} eq 'virtio') {
-            my $pciaddr = print_pci_addr("qga0", $bridges, $arch, $machine_type);
+            my $pciaddr = print_pci_addr("qga0", $bridges, $arch);
             push @$devices, '-device', "virtio-serial,id=qga0$pciaddr";
             push @$devices, '-device', 'virtserialport,chardev=qga0,name=org.qemu.guest_agent.0';
         } elsif ($guest_agent->{type} eq 'isa') {
@@ -3967,7 +3946,7 @@ sub config_to_command {
     my $rng = $conf->{rng0} ? parse_rng($conf->{rng0}) : undef;
     if ($rng && $version_guard->(4, 1, 2)) {
         my $rng_object = print_rng_object_commandline('rng0', $rng);
-        my $rng_device = print_rng_device_commandline('rng0', $rng, $bridges, $arch, $machine_type);
+        my $rng_device = print_rng_device_commandline('rng0', $rng, $bridges, $arch);
         push @$devices, '-object', $rng_object;
         push @$devices, '-device', $rng_device;
     }
@@ -3983,14 +3962,7 @@ sub config_to_command {
                 for (my $i = 1; $i < $qxlnum; $i++) {
                     push @$devices, '-device',
                         print_vga_device(
-                            $conf,
-                            $vga,
-                            $arch,
-                            $machine_version,
-                            $machine_type,
-                            $i,
-                            $qxlnum,
-                            $bridges,
+                            $conf, $vga, $arch, $machine_version, $i, $qxlnum, $bridges,
                         );
                 }
             } else {
@@ -4005,7 +3977,7 @@ sub config_to_command {
             }
         }
 
-        my $pciaddr = print_pci_addr("spice", $bridges, $arch, $machine_type);
+        my $pciaddr = print_pci_addr("spice", $bridges, $arch);
 
         push @$devices, '-device', "virtio-serial,id=spice$pciaddr";
         if ($vga->{'clipboard'} && $vga->{'clipboard'} eq 'vnc') {
@@ -4043,7 +4015,7 @@ sub config_to_command {
 
     # enable balloon by default, unless explicitly disabled
     if (!defined($conf->{balloon}) || $conf->{balloon}) {
-        my $pciaddr = print_pci_addr("balloon0", $bridges, $arch, $machine_type);
+        my $pciaddr = print_pci_addr("balloon0", $bridges, $arch);
         my $ballooncmd = "virtio-balloon-pci,id=balloon0$pciaddr";
         $ballooncmd .= ",free-page-reporting=on" if min_version($machine_version, 6, 2);
         push @$devices, '-device', $ballooncmd;
@@ -4051,7 +4023,7 @@ sub config_to_command {
 
     if ($conf->{watchdog}) {
         my $wdopts = parse_watchdog($conf->{watchdog});
-        my $pciaddr = print_pci_addr("watchdog", $bridges, $arch, $machine_type);
+        my $pciaddr = print_pci_addr("watchdog", $bridges, $arch);
         my $watchdog = $wdopts->{model} || 'i6300esb';
         push @$devices, '-device', "$watchdog$pciaddr";
         push @$devices, '-watchdog-action', $wdopts->{action} if $wdopts->{action};
@@ -4098,8 +4070,7 @@ sub config_to_command {
                     "scsi$drive->{index}: machine version 4.1~pve2 or higher is required to use more than 14 SCSI disks\n"
                     if $drive->{index} > 13 && !&$version_guard(4, 1, 2);
 
-                my $pciaddr =
-                    print_pci_addr("$controller_prefix$controller", $bridges, $arch, $machine_type);
+                my $pciaddr = print_pci_addr("$controller_prefix$controller", $bridges, $arch);
                 my $scsihw_type =
                     $scsihw =~ m/^virtio-scsi-single/ ? "virtio-scsi-pci" : $scsihw;
 
@@ -4134,7 +4105,7 @@ sub config_to_command {
 
             if ($drive->{interface} eq 'sata') {
                 my $controller = int($drive->{index} / $PVE::QemuServer::Drive::MAX_SATA_DISKS);
-                my $pciaddr = print_pci_addr("ahci$controller", $bridges, $arch, $machine_type);
+                my $pciaddr = print_pci_addr("ahci$controller", $bridges, $arch);
                 push @$devices, '-device', "ahci,id=ahci$controller,multifunction=on$pciaddr"
                     if !$ahcicontroller->{$controller};
                 $ahcicontroller->{$controller} = 1;
@@ -4189,7 +4160,6 @@ sub config_to_command {
             $bridges,
             $use_old_bios_files,
             $arch,
-            $machine_type,
             $machine_version,
         );
 
@@ -4203,7 +4173,7 @@ sub config_to_command {
         if ($q35) {
             $bus = print_pcie_addr("ivshmem");
         } else {
-            $bus = print_pci_addr("ivshmem", $bridges, $arch, $machine_type);
+            $bus = print_pci_addr("ivshmem", $bridges, $arch);
         }
 
         my $ivshmem_name = $ivshmem->{name} // $vmid;
@@ -4232,7 +4202,7 @@ sub config_to_command {
         if ($k == 2 && $legacy_igd) {
             $k_name = "$k-igd";
         }
-        my $pciaddr = print_pci_addr("pci.$k_name", undef, $arch, $machine_type);
+        my $pciaddr = print_pci_addr("pci.$k_name", undef, $arch);
         my $devstr = "pci-bridge,id=pci.$k,chassis_nr=$k$pciaddr";
 
         if ($q35) { # add after -readconfig pve-q35.cfg
@@ -4397,7 +4367,7 @@ sub vm_deviceplug {
         }
     } elsif ($deviceid =~ m/^(virtioscsi|scsihw)(\d+)$/) {
         my $scsihw = defined($conf->{scsihw}) ? $conf->{scsihw} : "lsi";
-        my $pciaddr = print_pci_addr($deviceid, undef, $arch, $machine_type);
+        my $pciaddr = print_pci_addr($deviceid, undef, $arch);
         my $scsihw_type = $scsihw eq 'virtio-scsi-single' ? "virtio-scsi-pci" : $scsihw;
 
         my $devicefull = "$scsihw_type,id=$deviceid$pciaddr";
@@ -4441,7 +4411,6 @@ sub vm_deviceplug {
             undef,
             $use_old_bios_files,
             $arch,
-            $machine_type,
             $machine_version,
         );
         qemu_deviceadd($vmid, $netdevicefull);
@@ -4456,7 +4425,7 @@ sub vm_deviceplug {
         }
     } elsif (!$q35 && $deviceid =~ m/^(pci\.)(\d+)$/) {
         my $bridgeid = $2;
-        my $pciaddr = print_pci_addr($deviceid, undef, $arch, $machine_type);
+        my $pciaddr = print_pci_addr($deviceid, undef, $arch);
         my $devicefull = "pci-bridge,id=pci.$bridgeid,chassis_nr=$bridgeid$pciaddr";
 
         qemu_deviceadd($vmid, $devicefull);
@@ -4664,7 +4633,7 @@ sub qemu_add_pci_bridge {
 
     my $bridgeid;
 
-    print_pci_addr($device, $bridges, $arch, $machine_type);
+    print_pci_addr($device, $bridges, $arch);
 
     while (my ($k, $v) = each %$bridges) {
         $bridgeid = $k;
@@ -4727,7 +4696,7 @@ sub qemu_usb_hotplug {
     my $devicelist = vm_devices_list($vmid);
 
     if (!$devicelist->{xhci}) {
-        my $pciaddr = print_pci_addr("xhci", undef, $arch, $machine_type);
+        my $pciaddr = print_pci_addr("xhci", undef, $arch);
         qemu_deviceadd($vmid, PVE::QemuServer::USB::print_qemu_xhci_controller($pciaddr));
     }
 
diff --git a/src/PVE/QemuServer/PCI.pm b/src/PVE/QemuServer/PCI.pm
index 66bd1534..89931465 100644
--- a/src/PVE/QemuServer/PCI.pm
+++ b/src/PVE/QemuServer/PCI.pm
@@ -284,14 +284,14 @@ my $get_addr_mapping_from_id = sub {
 };
 
 sub print_pci_addr {
-    my ($id, $bridges, $arch, $machine) = @_;
+    my ($id, $bridges, $arch) = @_;
 
     my $res = '';
 
     # 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/;
+    if ($arch eq 'aarch64') {
+        die "aarch64 cannot use IDE devices\n" if $id =~ /^ide/;
         $busname = 'pcie';
     }
 
@@ -640,7 +640,7 @@ my sub choose_hostpci_devices {
 }
 
 sub print_hostpci_devices {
-    my ($vmid, $conf, $devices, $vga, $winversion, $bridges, $arch, $machine_type, $bootorder) = @_;
+    my ($vmid, $conf, $devices, $vga, $winversion, $bridges, $arch, $bootorder) = @_;
 
     my $kvm_off = 0;
     my $gpu_passthrough = 0;
@@ -671,7 +671,7 @@ sub print_hostpci_devices {
             }
         } else {
             my $pci_name = $d->{'legacy-igd'} ? 'legacy-igd' : $id;
-            $pciaddr = print_pci_addr($pci_name, $bridges, $arch, $machine_type);
+            $pciaddr = print_pci_addr($pci_name, $bridges, $arch);
         }
 
         my $num_devices = scalar($d->{ids}->@*);
diff --git a/src/PVE/QemuServer/RNG.pm b/src/PVE/QemuServer/RNG.pm
index 6e0949be..eb477a5d 100644
--- a/src/PVE/QemuServer/RNG.pm
+++ b/src/PVE/QemuServer/RNG.pm
@@ -87,7 +87,7 @@ sub check_rng_source {
 }
 
 sub print_rng_device_commandline {
-    my ($id, $rng, $bridges, $arch, $machine) = @_;
+    my ($id, $rng, $bridges, $arch) = @_;
 
     die "no rng device specified\n" if !$rng;
 
@@ -98,7 +98,7 @@ sub print_rng_device_commandline {
         $limiter_str = ",max-bytes=$max_bytes,period=$period";
     }
 
-    my $rng_addr = print_pci_addr($id, $bridges, $arch, $machine);
+    my $rng_addr = print_pci_addr($id, $bridges, $arch);
 
     return "virtio-rng-pci,rng=$id$limiter_str$rng_addr";
 }
diff --git a/src/PVE/QemuServer/USB.pm b/src/PVE/QemuServer/USB.pm
index 5b3d1c5d..c9408a42 100644
--- a/src/PVE/QemuServer/USB.pm
+++ b/src/PVE/QemuServer/USB.pm
@@ -120,7 +120,7 @@ my sub assert_usb_index_is_useable {
 }
 
 sub get_usb_controllers {
-    my ($conf, $bridges, $arch, $machine, $machine_version) = @_;
+    my ($conf, $bridges, $arch, $machine_version) = @_;
 
     my $devices = [];
     my $pciaddr = "";
@@ -134,10 +134,10 @@ sub get_usb_controllers {
     my $is_q35 = PVE::QemuServer::Machine::machine_type_is_q35($conf);
 
     if ($arch eq 'aarch64') {
-        $pciaddr = print_pci_addr('ehci', $bridges, $arch, $machine);
+        $pciaddr = print_pci_addr('ehci', $bridges, $arch);
         push @$devices, '-device', "usb-ehci,id=ehci$pciaddr";
     } elsif (!$is_q35) {
-        $pciaddr = print_pci_addr("piix3", $bridges, $arch, $machine);
+        $pciaddr = print_pci_addr("piix3", $bridges, $arch);
         push @$devices, '-device', "piix3-usb-uhci,id=uhci$pciaddr.0x2";
     }
 
@@ -157,7 +157,7 @@ sub get_usb_controllers {
         push @$devices, '-readconfig', '/usr/share/qemu-server/pve-usb.cfg';
     }
 
-    $pciaddr = print_pci_addr("xhci", $bridges, $arch, $machine);
+    $pciaddr = print_pci_addr("xhci", $bridges, $arch);
     if ($use_qemu_xhci && $any_usb) {
         push @$devices, '-device', print_qemu_xhci_controller($pciaddr);
     } elsif ($use_usb3) {
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel

[pve-devel] [PATCH qemu-server 3/3] fix #6466: aarch64: pci: properly print higher-index PCI bridge addresses

[Fiona Ebner]

For aarch64, the virt machine has an initial pcie.0 bus. The other pci
bridges that get added on top are called pci.N, see the relevant
section in config_to_command() which adds them.

In particular this fixes adding an RNG device, which is require for
OVMF PXE boot.

Signed-off-by: Fiona Ebner <f.ebner@proxmox.com>
---
 src/PVE/QemuServer/PCI.pm | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/src/PVE/QemuServer/PCI.pm b/src/PVE/QemuServer/PCI.pm
index 89931465..59106876 100644
--- a/src/PVE/QemuServer/PCI.pm
+++ b/src/PVE/QemuServer/PCI.pm
@@ -286,17 +286,16 @@ my $get_addr_mapping_from_id = sub {
 sub print_pci_addr {
     my ($id, $bridges, $arch) = @_;
 
-    my $res = '';
+    die "aarch64 cannot use IDE devices\n" if $arch eq 'aarch64' && $id =~ /^ide/;
 
-    # using same bus slots on all HW, so we need to check special cases here:
-    my $busname = 'pci';
-    if ($arch eq 'aarch64') {
-        die "aarch64 cannot use IDE devices\n" if $id =~ /^ide/;
-        $busname = 'pcie';
-    }
+    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}";
         $bridges->{ $d->{bus} } = 1 if $bridges;
     }
-- 
2.39.5



_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel