hw/usb/host-libusb.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
Currently, there is a special case for usb-host devices added using the
hostbus= and hostaddr= properties to avoid adding them to the hotplug
watchlist, since the address changes every time the device is plugged
in. However, on Linux, when the host system goes into suspend and then
resumes, those devices stop working in both the guest and the host.
Enabling autoscan and adding those devices to the watchlist allows them
to keep working in the guest after host suspend/resume.
Signed-off-by: Yuri Nesterov <yuri.nesterov@gmail.com>
---
hw/usb/host-libusb.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index c3d642c9d3..32c0251471 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -1227,7 +1227,7 @@ static void usb_host_realize(USBDevice *udev, Error **errp)
!s->match.vendor_id &&
!s->match.product_id &&
!s->match.port) {
- s->needs_autoscan = false;
+ s->needs_autoscan = true;
ldev = usb_host_find_ref(s->match.bus_num,
s->match.addr);
if (!ldev) {
@@ -1244,6 +1244,9 @@ static void usb_host_realize(USBDevice *udev, Error **errp)
}
} else {
s->needs_autoscan = true;
+ }
+
+ if (s->needs_autoscan) {
QTAILQ_INSERT_TAIL(&hostdevs, s, next);
usb_host_auto_check(NULL);
}
--
2.43.0
On Wed, Apr 16, 2025 at 07:19:29PM +0300, Yuri Nesterov wrote: > Currently, there is a special case for usb-host devices added using the > hostbus= and hostaddr= properties to avoid adding them to the hotplug > watchlist, since the address changes every time the device is plugged > in. However, on Linux, when the host system goes into suspend and then > resumes, those devices stop working in both the guest and the host. > > Enabling autoscan and adding those devices to the watchlist allows them > to keep working in the guest after host suspend/resume. So IIUC what you're saying is that on suspend the host device is removed by the kernel, and on resume, the USB device is recreated. So QEMU's open file handle for the USB device is invalid after resume. If the /dev/bus/usb/NNN/NNN file goes away and then gets re-created by the kernel though, we can't assume QEMU is going to be able to re-open the new /dev/bus/usb device file though. When QEMU runs under libvirt, QEMU won't have any access to the /dev/bus/usb device node unless libvirt has set the right permissions and (where appropriate) also set the SELinux label. The current autoscan logic seems rather crude. AFAIK every 2 seconds it will re-scan every host USB device to see if any has gone away and close it, and/or re-open if re-appeared. If we enable this autoscan logic, then under libvirt, AFAICS, QEMU is going to fail to re-open the device, and a counter in the autoscan logic means that after 3 attempts to re-open QEMU will stop attempting it at all...but strangely it appears QEMU will keep the timer running, so every 2 seconds it will iterate over every USB device, but never try to open any of them. Regardless of your current patch this autoscan logic feels like it needs improvement. We shouldn't need to busy-poll to see that a USB device goes away, it should be possible to rely on event notifications in some way. Even then we'd still need a way to prevent this immediate auto re-opening when under libvirt. Libvirt would have to detect the reappearance of the device and perform relabelling and permissions changes, before QEMU is able to be told to try to re-open. Potentially this implies a new QMP command to tell QEMU to try to re-open, unless perhaps QEMU can be triggered off an inotify event for the permissions/label change of the device node. > > Signed-off-by: Yuri Nesterov <yuri.nesterov@gmail.com> > --- > hw/usb/host-libusb.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c > index c3d642c9d3..32c0251471 100644 > --- a/hw/usb/host-libusb.c > +++ b/hw/usb/host-libusb.c > @@ -1227,7 +1227,7 @@ static void usb_host_realize(USBDevice *udev, Error **errp) > !s->match.vendor_id && > !s->match.product_id && > !s->match.port) { > - s->needs_autoscan = false; > + s->needs_autoscan = true; > ldev = usb_host_find_ref(s->match.bus_num, > s->match.addr); > if (!ldev) { > @@ -1244,6 +1244,9 @@ static void usb_host_realize(USBDevice *udev, Error **errp) > } > } else { > s->needs_autoscan = true; > + } > + > + if (s->needs_autoscan) { > QTAILQ_INSERT_TAIL(&hostdevs, s, next); > usb_host_auto_check(NULL); > } > -- > 2.43.0 > > With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Wed, Apr 16, 2025 at 8:07 PM Daniel P. Berrangé <berrange@redhat.com> wrote: > > On Wed, Apr 16, 2025 at 07:19:29PM +0300, Yuri Nesterov wrote: > > Currently, there is a special case for usb-host devices added using the > > hostbus= and hostaddr= properties to avoid adding them to the hotplug > > watchlist, since the address changes every time the device is plugged > > in. However, on Linux, when the host system goes into suspend and then > > resumes, those devices stop working in both the guest and the host. > > > > Enabling autoscan and adding those devices to the watchlist allows them > > to keep working in the guest after host suspend/resume. > > So IIUC what you're saying is that on suspend the host device > is removed by the kernel, and on resume, the USB device is > recreated. So QEMU's open file handle for the USB device is > invalid after resume. > > If the /dev/bus/usb/NNN/NNN file goes away and then gets > re-created by the kernel though, we can't assume QEMU is > going to be able to re-open the new /dev/bus/usb device > file though. I'm not sure if the file actually goes away. It looks like the internal state of the device changes and QEMU receives a "no device" response in usb_host_req_complete_data. However, the file remains in place. At least I don't see any changes in udevadm monitor or inotifywait aside from attribute modifications. After resuming from suspend, the device doesn't work on either host or guest. Probably the device stays with a detached kernel driver since QEMU can't reattach it after receiving the "no device" error. Adding such devices to the hotplug watchlist causes QEMU to reopen them the same way it does for devices specified by vendorid and productid or hostport. The reason bus+addr devices aren't currently added to that list is well explained in commit e058fa2dd599ccc780d334558be9c1d155222b80. A special case was made because the device address changes every time it's replugged. However, it turns out that it doesn't change after a suspend/resume cycle so adding them to the list allows them to keep working after resume. I haven't had a chance to test it with libvirt though. > When QEMU runs under libvirt, QEMU won't have any access > to the /dev/bus/usb device node unless libvirt has set the > right permissions and (where appropriate) also set the > SELinux label. > > The current autoscan logic seems rather crude. AFAIK every 2 > seconds it will re-scan every host USB device to see if any > has gone away and close it, and/or re-open if re-appeared. > > If we enable this autoscan logic, then under libvirt, AFAICS, > QEMU is going to fail to re-open the device, and a counter in > the autoscan logic means that after 3 attempts to re-open QEMU > will stop attempting it at all...but strangely it appears QEMU > will keep the timer running, so every 2 seconds it will iterate > over every USB device, but never try to open any of them. > > Regardless of your current patch this autoscan logic feels like > it needs improvement. We shouldn't need to busy-poll to see that > a USB device goes away, it should be possible to rely on event > notifications in some way. > > Even then we'd still need a way to prevent this immediate auto > re-opening when under libvirt. Libvirt would have to detect the > reappearance of the device and perform relabelling and permissions > changes, before QEMU is able to be told to try to re-open. > Potentially this implies a new QMP command to tell QEMU to try > to re-open, unless perhaps QEMU can be triggered off an inotify > event for the permissions/label change of the device node. > > > > > Signed-off-by: Yuri Nesterov <yuri.nesterov@gmail.com> > > --- > > hw/usb/host-libusb.c | 5 ++++- > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c > > index c3d642c9d3..32c0251471 100644 > > --- a/hw/usb/host-libusb.c > > +++ b/hw/usb/host-libusb.c > > @@ -1227,7 +1227,7 @@ static void usb_host_realize(USBDevice *udev, Error **errp) > > !s->match.vendor_id && > > !s->match.product_id && > > !s->match.port) { > > - s->needs_autoscan = false; > > + s->needs_autoscan = true; > > ldev = usb_host_find_ref(s->match.bus_num, > > s->match.addr); > > if (!ldev) { > > @@ -1244,6 +1244,9 @@ static void usb_host_realize(USBDevice *udev, Error **errp) > > } > > } else { > > s->needs_autoscan = true; > > + } > > + > > + if (s->needs_autoscan) { > > QTAILQ_INSERT_TAIL(&hostdevs, s, next); > > usb_host_auto_check(NULL); > > } > > -- > > 2.43.0 > > > > > > With regards, > Daniel > -- > |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o- https://fstop138.berrange.com :| > |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :| >
On Wed, Apr 16, 2025 at 10:27:54PM +0300, Yuri Nesterov wrote: > On Wed, Apr 16, 2025 at 8:07 PM Daniel P. Berrangé <berrange@redhat.com> wrote: > > > > On Wed, Apr 16, 2025 at 07:19:29PM +0300, Yuri Nesterov wrote: > > > Currently, there is a special case for usb-host devices added using the > > > hostbus= and hostaddr= properties to avoid adding them to the hotplug > > > watchlist, since the address changes every time the device is plugged > > > in. However, on Linux, when the host system goes into suspend and then > > > resumes, those devices stop working in both the guest and the host. > > > > > > Enabling autoscan and adding those devices to the watchlist allows them > > > to keep working in the guest after host suspend/resume. > > > > So IIUC what you're saying is that on suspend the host device > > is removed by the kernel, and on resume, the USB device is > > recreated. So QEMU's open file handle for the USB device is > > invalid after resume. > > > > If the /dev/bus/usb/NNN/NNN file goes away and then gets > > re-created by the kernel though, we can't assume QEMU is > > going to be able to re-open the new /dev/bus/usb device > > file though. > > I'm not sure if the file actually goes away. It looks like the internal > state of the device changes and QEMU receives a "no device" > response in usb_host_req_complete_data. However, the file > remains in place. At least I don't see any changes in udevadm > monitor or inotifywait aside from attribute modifications. > > After resuming from suspend, the device doesn't work on either > host or guest. Probably the device stays with a detached kernel > driver since QEMU can't reattach it after receiving the "no device" > error. Adding such devices to the hotplug watchlist causes QEMU > to reopen them the same way it does for devices specified by > vendorid and productid or hostport. This is a bit odd, as AFAICT from reading the code, the usb_host_auto_check wll only trigger close + re-open of the device, if there is a period of time in which the /dev/bus/usb device node does not exist, but you're saying it remains existing across suspend/resume. > > The reason bus+addr devices aren't currently added to that list is well > explained in commit e058fa2dd599ccc780d334558be9c1d155222b80. > A special case was made because the device address changes every > time it's replugged. However, it turns out that it doesn't change after > a suspend/resume cycle so adding them to the list allows them to > keep working after resume. > With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Thu, Apr 17, 2025 at 12:41 PM Daniel P. Berrangé <berrange@redhat.com> wrote: > > On Wed, Apr 16, 2025 at 10:27:54PM +0300, Yuri Nesterov wrote: > > On Wed, Apr 16, 2025 at 8:07 PM Daniel P. Berrangé <berrange@redhat.com> wrote: > > > > > > On Wed, Apr 16, 2025 at 07:19:29PM +0300, Yuri Nesterov wrote: > > > > Currently, there is a special case for usb-host devices added using the > > > > hostbus= and hostaddr= properties to avoid adding them to the hotplug > > > > watchlist, since the address changes every time the device is plugged > > > > in. However, on Linux, when the host system goes into suspend and then > > > > resumes, those devices stop working in both the guest and the host. > > > > > > > > Enabling autoscan and adding those devices to the watchlist allows them > > > > to keep working in the guest after host suspend/resume. > > > > > > So IIUC what you're saying is that on suspend the host device > > > is removed by the kernel, and on resume, the USB device is > > > recreated. So QEMU's open file handle for the USB device is > > > invalid after resume. > > > > > > If the /dev/bus/usb/NNN/NNN file goes away and then gets > > > re-created by the kernel though, we can't assume QEMU is > > > going to be able to re-open the new /dev/bus/usb device > > > file though. > > > > I'm not sure if the file actually goes away. It looks like the internal > > state of the device changes and QEMU receives a "no device" > > response in usb_host_req_complete_data. However, the file > > remains in place. At least I don't see any changes in udevadm > > monitor or inotifywait aside from attribute modifications. > > > > After resuming from suspend, the device doesn't work on either > > host or guest. Probably the device stays with a detached kernel > > driver since QEMU can't reattach it after receiving the "no device" > > error. Adding such devices to the hotplug watchlist causes QEMU > > to reopen them the same way it does for devices specified by > > vendorid and productid or hostport. > > This is a bit odd, as AFAICT from reading the code, the > usb_host_auto_check wll only trigger close + re-open of > the device, if there is a period of time in which the > /dev/bus/usb device node does not exist, but you're > saying it remains existing across suspend/resume. I guess it depends on how it's configured, both in hardware and software. I'm testing on a regular ThinkPad with Ubuntu and default settings. It's probably possible to configure USB to turn off power during suspend. In that case, it might reappear after resume with a new address. However, with the default settings, it seems to keep the same device node and address. I assume the driver just switches it to low-power mode and then back to normal. In usb_host_auto_check, it reopens the device when the handle s->dh is NULL. This happens after usb_host_req_complete_data receives the "no device" status and closes the device. Maybe one of the kernel functions returns -ENODEV at some point, but after that, the device continues to work fine.
© 2016 - 2025 Red Hat, Inc.