* [pve-devel] [PATCH qemu-server 2/3] pci: code cleanup: remove superfluous machine type parameter from print_pci_addr
2025-06-18 15:20 [pve-devel] [PATCH-SERIES qemu-server master+stable-bookworm 0/3] fix #6466: aarch64: re-allow PXE boot on OVMF Fiona Ebner
2025-06-18 15:20 ` [pve-devel] [PATCH qemu-server 1/3] cfg2cmd: print vga: fix call to print_pcie_addr() Fiona Ebner
@ 2025-06-18 15:20 ` Fiona Ebner
2025-06-18 15:20 ` [pve-devel] [PATCH qemu-server 3/3] fix #6466: aarch64: pci: properly print higher-index PCI bridge addresses Fiona Ebner
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Fiona Ebner @ 2025-06-18 15:20 UTC (permalink / raw)
To: pve-devel
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
^ permalink raw reply [flat|nested] 6+ messages in thread