[pve-devel] [PATCH qemu-server 5/7] fix #4324: USB: use qemu-xhci for machine versions >= 7.1

[Dominik Csapak <d.csapak@proxmox.com>]

going by reports in the forum (e.g. [0]) and semi-official qemu
information[1], we should prefer qemu-xhci over nec-usb-xhci

for compatibility purposes, we guard that behind the machine version,
so that guests with a fixed version don't suddenly have a different usb
controller after a reboot (which could potentially break some hardcoded
guest configs)

0: https://forum.proxmox.com/threads/proxmox-usb-connect-disconnect-loop.117063/
1: https://www.kraxel.org/blog/2018/08/qemu-usb-tips/

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 PVE/QemuServer.pm                   |  8 +--
 PVE/QemuServer/USB.pm               | 76 +++++++++++++++++++++--------
 test/cfg2cmd/qemu-xhci-7.1.conf     | 12 +++++
 test/cfg2cmd/qemu-xhci-7.1.conf.cmd | 34 +++++++++++++
 4 files changed, 108 insertions(+), 22 deletions(-)
 create mode 100644 test/cfg2cmd/qemu-xhci-7.1.conf
 create mode 100644 test/cfg2cmd/qemu-xhci-7.1.conf.cmd

diff --git a/PVE/QemuServer.pm b/PVE/QemuServer.pm
index 06ffead..452eb10 100644
--- a/PVE/QemuServer.pm
+++ b/PVE/QemuServer.pm
@@ -1092,7 +1092,9 @@ EODESCR
     usb3 => {
 	optional => 1,
 	type => 'boolean',
-	description => "Specifies whether if given host option is a USB3 device or port.",
+	description => "Specifies whether if given host option is a USB3 device or port."
+	    ." For modern guests (machine version >= 7.1 and ostype l26 and windows > 7), this flag"
+	    ." is irrelevant (all devices are plugged into a xhci controller).",
         default => 0,
     },
 };
@@ -3677,7 +3679,7 @@ sub config_to_command {
 
     # add usb controllers
     my @usbcontrollers = PVE::QemuServer::USB::get_usb_controllers(
-        $conf, $bridges, $arch, $machine_type, $usbdesc->{format}, $MAX_USB_DEVICES);
+	$conf, $bridges, $arch, $machine_type, $usbdesc->{format}, $MAX_USB_DEVICES, $machine_version);
     push @$devices, @usbcontrollers if @usbcontrollers;
     my $vga = parse_vga($conf->{vga});
 
@@ -3719,7 +3721,7 @@ sub config_to_command {
     $usb_dev_features->{spice_usb3} = 1 if min_version($machine_version, 4, 0);
 
     my @usbdevices = PVE::QemuServer::USB::get_usb_devices(
-        $conf, $usbdesc->{format}, $MAX_USB_DEVICES, $usb_dev_features, $bootorder);
+	$conf, $usbdesc->{format}, $MAX_USB_DEVICES, $usb_dev_features, $bootorder, $machine_version);
     push @$devices, @usbdevices if @usbdevices;
 
     # serial devices
diff --git a/PVE/QemuServer/USB.pm b/PVE/QemuServer/USB.pm
index 10c9bca..96931d4 100644
--- a/PVE/QemuServer/USB.pm
+++ b/PVE/QemuServer/USB.pm
@@ -4,6 +4,7 @@ use strict;
 use warnings;
 use PVE::QemuServer::PCI qw(print_pci_addr);
 use PVE::QemuServer::Machine;
+use PVE::QemuServer::Helpers qw(min_version windows_version);
 use PVE::JSONSchema;
 use base 'Exporter';
 
@@ -35,11 +36,16 @@ sub parse_usb_device {
 }
 
 sub get_usb_controllers {
-    my ($conf, $bridges, $arch, $machine, $format, $max_usb_devices) = @_;
+    my ($conf, $bridges, $arch, $machine, $format, $max_usb_devices, $machine_version) = @_;
 
     my $devices = [];
     my $pciaddr = "";
 
+    my $ostype = $conf->{ostype};
+
+    my $use_qemu_xhci = min_version($machine_version, 7, 1)
+	&& defined($ostype) && ($ostype eq 'l26' || windows_version($ostype) > 7);
+
     if ($arch eq 'aarch64') {
         $pciaddr = print_pci_addr('ehci', $bridges, $arch, $machine);
         push @$devices, '-device', "usb-ehci,id=ehci$pciaddr";
@@ -47,58 +53,75 @@ sub get_usb_controllers {
         $pciaddr = print_pci_addr("piix3", $bridges, $arch, $machine);
         push @$devices, '-device', "piix3-usb-uhci,id=uhci$pciaddr.0x2";
 
-        my $use_usb2 = 0;
-	for (my $i = 0; $i < $max_usb_devices; $i++)  {
-	    next if !$conf->{"usb$i"};
-	    my $d = eval { PVE::JSONSchema::parse_property_string($format,$conf->{"usb$i"}) };
-	    next if !$d || $d->{usb3}; # do not add usb2 controller if we have only usb3 devices
-	    $use_usb2 = 1;
+	if (!$use_qemu_xhci) {
+	    my $use_usb2 = 0;
+	    for (my $i = 0; $i < $max_usb_devices; $i++)  {
+		next if !$conf->{"usb$i"};
+		my $d = eval { PVE::JSONSchema::parse_property_string($format,$conf->{"usb$i"}) };
+		next if !$d || $d->{usb3}; # do not add usb2 controller if we have only usb3 devices
+		$use_usb2 = 1;
+	    }
+	    # include usb device config
+	    push @$devices, '-readconfig', '/usr/share/qemu-server/pve-usb.cfg' if $use_usb2;
 	}
-	# include usb device config
-	push @$devices, '-readconfig', '/usr/share/qemu-server/pve-usb.cfg' if $use_usb2;
     }
 
     # add usb3 controller if needed
 
     my $use_usb3 = 0;
+    my $use_usb = 0;
     for (my $i = 0; $i < $max_usb_devices; $i++)  {
 	next if !$conf->{"usb$i"};
 	my $d = eval { PVE::JSONSchema::parse_property_string($format,$conf->{"usb$i"}) };
-	next if !$d || !$d->{usb3};
-	$use_usb3 = 1;
+	next if !$d;
+	$use_usb = 1;
+	$use_usb3 = 1 if $d->{usb3};
     }
 
     $pciaddr = print_pci_addr("xhci", $bridges, $arch, $machine);
-    push @$devices, '-device', "nec-usb-xhci,id=xhci$pciaddr" if $use_usb3;
+    if ($use_qemu_xhci && $use_usb) {
+	push @$devices, '-device', print_qemu_xhci_controller($pciaddr);
+    } else {
+	push @$devices, '-device', "nec-usb-xhci,id=xhci$pciaddr" if $use_usb3;
+    }
 
     return @$devices;
 }
 
 sub get_usb_devices {
-    my ($conf, $format, $max_usb_devices, $features, $bootorder) = @_;
+    my ($conf, $format, $max_usb_devices, $features, $bootorder, $machine_version) = @_;
 
     my $devices = [];
 
+    my $ostype = $conf->{ostype};
+    my $use_qemu_xhci = min_version($machine_version, 7, 1)
+	&& defined($ostype) && ($ostype eq 'l26' || windows_version($ostype) > 7);
+
     for (my $i = 0; $i < $max_usb_devices; $i++)  {
 	my $devname = "usb$i";
 	next if !$conf->{$devname};
 	my $d = eval { PVE::JSONSchema::parse_property_string($format,$conf->{$devname}) };
 	next if !$d;
 
+	my $port;
+	if ($use_qemu_xhci) {
+	    $port = $i + 1;
+	}
+
 	if (defined($d->{host})) {
 	    my $hostdevice = parse_usb_device($d->{host});
 	    $hostdevice->{usb3} = $d->{usb3};
 	    if ($hostdevice->{spice}) {
 		# usb redir support for spice
 		my $bus = 'ehci';
-		$bus = 'xhci' if $hostdevice->{usb3} && $features->{spice_usb3};
+		$bus = 'xhci' if ($hostdevice->{usb3} && $features->{spice_usb3}) || $use_qemu_xhci;
 
 		push @$devices, '-chardev', "spicevmc,id=usbredirchardev$i,name=usbredir";
-		push @$devices, '-device', "usb-redir,chardev=usbredirchardev$i,id=usbredirdev$i,bus=$bus.0";
+		push @$devices, '-device', print_spice_usbdevice($i, $bus, $port);
 
 		warn "warning: spice usb port set as bootdevice, ignoring\n" if $bootorder->{$devname};
 	    } else {
-		push @$devices, '-device', print_usbdevice_full($conf, $devname, $hostdevice, $bootorder);
+		push @$devices, '-device', print_usbdevice_full($conf, $devname, $hostdevice, $bootorder, $port);
 	    }
 	}
     }
@@ -106,15 +129,30 @@ sub get_usb_devices {
     return @$devices;
 }
 
+sub print_qemu_xhci_controller {
+    my ($pciaddr) = @_;
+    return "qemu-xhci,p2=15,p3=15,id=xhci$pciaddr";
+}
+
+sub print_spice_usbdevice {
+    my ($index, $bus, $port) = @_;
+    my $device = "usb-redir,chardev=usbredirchardev$index,id=usbredirdev$index,bus=$bus.0";
+    if (defined($port)) {
+	$device .= ",port=$port";
+    }
+    return $device;
+}
+
 sub print_usbdevice_full {
-    my ($conf, $deviceid, $device, $bootorder) = @_;
+    my ($conf, $deviceid, $device, $bootorder, $port) = @_;
 
     return if !$device;
     my $usbdevice = "usb-host";
 
-    # if it is a usb3 device, attach it to the xhci controller, else omit the bus option
-    if($device->{usb3}) {
+    # if it is a usb3 device or with newer qemu, attach it to the xhci controller, else omit the bus option
+    if($device->{usb3} || defined($port)) {
 	$usbdevice .= ",bus=xhci.0";
+	$usbdevice .= ",port=$port" if defined($port);
     }
 
     if (defined($device->{vendorid}) && defined($device->{productid})) {
diff --git a/test/cfg2cmd/qemu-xhci-7.1.conf b/test/cfg2cmd/qemu-xhci-7.1.conf
new file mode 100644
index 0000000..3d50a1d
--- /dev/null
+++ b/test/cfg2cmd/qemu-xhci-7.1.conf
@@ -0,0 +1,12 @@
+# TEST: Test for new xhci controller with new machine version
+# QEMU_VERSION: 7.1.0
+cores: 2
+memory: 768
+name: spiceusb3
+net0: virtio=A2:C0:43:77:08:A1,bridge=vmbr0
+ostype: l26
+scsihw: virtio-scsi-pci
+smbios1: uuid=7b10d7af-b932-4c66-b2c3-3996152ec465
+vmgenid: c773c261-d800-4348-9f5d-167fadd53cf8
+vga: qxl
+usb1: spice
diff --git a/test/cfg2cmd/qemu-xhci-7.1.conf.cmd b/test/cfg2cmd/qemu-xhci-7.1.conf.cmd
new file mode 100644
index 0000000..af3a7e0
--- /dev/null
+++ b/test/cfg2cmd/qemu-xhci-7.1.conf.cmd
@@ -0,0 +1,34 @@
+/usr/bin/kvm \
+  -id 8006 \
+  -name 'spiceusb3,debug-threads=on' \
+  -no-shutdown \
+  -chardev 'socket,id=qmp,path=/var/run/qemu-server/8006.qmp,server=on,wait=off' \
+  -mon 'chardev=qmp,mode=control' \
+  -chardev 'socket,id=qmp-event,path=/var/run/qmeventd.sock,reconnect=5' \
+  -mon 'chardev=qmp-event,mode=control' \
+  -pidfile /var/run/qemu-server/8006.pid \
+  -daemonize \
+  -smbios 'type=1,uuid=7b10d7af-b932-4c66-b2c3-3996152ec465' \
+  -smp '2,sockets=1,cores=2,maxcpus=2' \
+  -nodefaults \
+  -boot 'menu=on,strict=on,reboot-timeout=1000,splash=/usr/share/qemu-server/bootsplash.jpg' \
+  -vnc 'unix:/var/run/qemu-server/8006.vnc,password=on' \
+  -cpu kvm64,enforce,+kvm_pv_eoi,+kvm_pv_unhalt,+lahf_lm,+sep \
+  -m 768 \
+  -device 'pci-bridge,id=pci.1,chassis_nr=1,bus=pci.0,addr=0x1e' \
+  -device 'pci-bridge,id=pci.2,chassis_nr=2,bus=pci.0,addr=0x1f' \
+  -device 'vmgenid,guid=c773c261-d800-4348-9f5d-167fadd53cf8' \
+  -device 'piix3-usb-uhci,id=uhci,bus=pci.0,addr=0x1.0x2' \
+  -device 'qemu-xhci,p2=15,p3=15,id=xhci,bus=pci.1,addr=0x1b' \
+  -chardev 'spicevmc,id=usbredirchardev1,name=usbredir' \
+  -device 'usb-redir,chardev=usbredirchardev1,id=usbredirdev1,bus=xhci.0,port=2' \
+  -device 'qxl-vga,id=vga,max_outputs=4,bus=pci.0,addr=0x2' \
+  -device 'virtio-serial,id=spice,bus=pci.0,addr=0x9' \
+  -chardev 'spicevmc,id=vdagent,name=vdagent' \
+  -device 'virtserialport,chardev=vdagent,name=com.redhat.spice.0' \
+  -spice 'tls-port=61000,addr=127.0.0.1,tls-ciphers=HIGH,seamless-migration=on' \
+  -device 'virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x3,free-page-reporting=on' \
+  -iscsi 'initiator-name=iqn.1993-08.org.debian:01:aabbccddeeff' \
+  -netdev 'type=tap,id=net0,ifname=tap8006i0,script=/var/lib/qemu-server/pve-bridge,downscript=/var/lib/qemu-server/pve-bridgedown,vhost=on' \
+  -device 'virtio-net-pci,mac=A2:C0:43:77:08:A1,netdev=net0,bus=pci.0,addr=0x12,id=net0,bootindex=300' \
+  -machine 'type=pc+pve0'
-- 
2.30.2