* [PATCH 1/2] usb-host: remove 'remote wakeup' flag from configuration descriptor
2019-12-02 12:34 [PATCH 0/2] Remove 'remote wakeup' flag from USB config descriptor Yuri Benditovich
@ 2019-12-02 12:34 ` Yuri Benditovich
2019-12-03 10:48 ` Gerd Hoffmann
2019-12-02 12:34 ` [PATCH 2/2] usb-redir: " Yuri Benditovich
2019-12-02 16:30 ` [PATCH 0/2] Remove 'remote wakeup' flag from USB config descriptor no-reply
2 siblings, 1 reply; 6+ messages in thread
From: Yuri Benditovich @ 2019-12-02 12:34 UTC (permalink / raw)
To: ehabkost, marcel.apfelbaum, kraxel, qemu-devel; +Cc: yan
If the redirected device has this capability, Windows guest may
place the device into D2 and expect it to wake when the device
becomes active, but this will never happen. For example, when
internal Bluetooth adapter is redirected, keyboards and mice
connected to it do not work. Current commit removes this
capability (starting from machine 4.2)
Set 'usb-host.suppress-remote-wake' property to 'off' to keep
'remote wake' as is or to 'on' to remove 'remote wake' on
4.1 or earlier.
Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
---
hw/core/machine.c | 1 +
hw/usb/host-libusb.c | 18 ++++++++++++++++++
hw/usb/trace-events | 1 +
3 files changed, 20 insertions(+)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 1689ad3bf8..8c0eaad091 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -29,6 +29,7 @@
GlobalProperty hw_compat_4_1[] = {
{ "virtio-pci", "x-pcie-flr-init", "off" },
+ { "usb-host", "suppress-remote-wake", "off" },
};
const size_t hw_compat_4_1_len = G_N_ELEMENTS(hw_compat_4_1);
diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c
index fcf48c0193..77c2720ec6 100644
--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -88,6 +88,7 @@ struct USBHostDevice {
bool needs_autoscan;
bool allow_one_guest_reset;
bool allow_all_guest_resets;
+ bool suppress_remote_wake;
/* state */
QTAILQ_ENTRY(USBHostDevice) next;
@@ -386,6 +387,8 @@ static void LIBUSB_CALL usb_host_req_complete_ctrl(struct libusb_transfer *xfer)
r->p->status = status_map[xfer->status];
r->p->actual_length = xfer->actual_length;
if (r->in && xfer->actual_length) {
+ USBDevice *udev = USB_DEVICE(s);
+ struct libusb_config_descriptor *conf = (void *)r->cbuf;
memcpy(r->cbuf, r->buffer + 8, xfer->actual_length);
/* Fix up USB-3 ep0 maxpacket size to allow superspeed connected devices
@@ -394,6 +397,20 @@ static void LIBUSB_CALL usb_host_req_complete_ctrl(struct libusb_transfer *xfer)
r->cbuf[7] == 9) {
r->cbuf[7] = 64;
}
+ /* If this is GET_DESCRIPTOR request for configuration descriptor,
+ * remove 'remote wakeup' flag from it to prevent idle power down
+ * in Windows guest */
+ if (s->suppress_remote_wake &&
+ udev->setup_buf[0] == USB_DIR_IN &&
+ udev->setup_buf[1] == USB_REQ_GET_DESCRIPTOR &&
+ udev->setup_buf[3] == USB_DT_CONFIG && udev->setup_buf[2] == 0 &&
+ xfer->actual_length > offsetof(struct libusb_config_descriptor, bmAttributes) &&
+ (conf->bmAttributes & USB_CFG_ATT_WAKEUP)) {
+ struct libusb_device_descriptor desc;
+ libusb_get_device_descriptor(s->dev, &desc);
+ trace_usb_host_remote_wakeup_removed(desc.idVendor, desc.idProduct);
+ conf->bmAttributes &= ~USB_CFG_ATT_WAKEUP;
+ }
}
trace_usb_host_req_complete(s->bus_num, s->addr, r->p,
r->p->status, r->p->actual_length);
@@ -1596,6 +1613,7 @@ static Property usb_host_dev_properties[] = {
LIBUSB_LOG_LEVEL_WARNING),
DEFINE_PROP_BIT("pipeline", USBHostDevice, options,
USB_HOST_OPT_PIPELINE, true),
+ DEFINE_PROP_BOOL("suppress-remote-wake", USBHostDevice, suppress_remote_wake, true),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/hw/usb/trace-events b/hw/usb/trace-events
index 2d3713351c..6f87b5262a 100644
--- a/hw/usb/trace-events
+++ b/hw/usb/trace-events
@@ -266,3 +266,4 @@ usb_host_parse_config(int bus, int addr, int value, int active) "dev %d:%d, valu
usb_host_parse_interface(int bus, int addr, int num, int alt, int active) "dev %d:%d, num %d, alt %d, active %d"
usb_host_parse_endpoint(int bus, int addr, int ep, const char *dir, const char *type, int active) "dev %d:%d, ep %d, %s, %s, active %d"
usb_host_parse_error(int bus, int addr, const char *errmsg) "dev %d:%d, msg %s"
+usb_host_remote_wakeup_removed(uint16_t vid, uint16_t pid) "dev %04x:%04x"
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] usb-host: remove 'remote wakeup' flag from configuration descriptor
2019-12-02 12:34 ` [PATCH 1/2] usb-host: remove 'remote wakeup' flag from configuration descriptor Yuri Benditovich
@ 2019-12-03 10:48 ` Gerd Hoffmann
0 siblings, 0 replies; 6+ messages in thread
From: Gerd Hoffmann @ 2019-12-03 10:48 UTC (permalink / raw)
To: Yuri Benditovich; +Cc: yan, ehabkost, qemu-devel
Hi,
> + /* If this is GET_DESCRIPTOR request for configuration descriptor,
> + * remove 'remote wakeup' flag from it to prevent idle power down
> + * in Windows guest */
scripts/checkpatch.pl complains about that, please fix (and also the
other checkpatch warnings).
> + if (s->suppress_remote_wake &&
> + udev->setup_buf[0] == USB_DIR_IN &&
> + udev->setup_buf[1] == USB_REQ_GET_DESCRIPTOR &&
> + udev->setup_buf[3] == USB_DT_CONFIG && udev->setup_buf[2] == 0 &&
> + xfer->actual_length > offsetof(struct libusb_config_descriptor, bmAttributes) &&
> + (conf->bmAttributes & USB_CFG_ATT_WAKEUP)) {
> + struct libusb_device_descriptor desc;
> + libusb_get_device_descriptor(s->dev, &desc);
> + trace_usb_host_remote_wakeup_removed(desc.idVendor, desc.idProduct);
Please use s->bus_num and s->addr to identify the device, like all the
other trace points do. I don't think there is a need to log
desc.idVendor and desc.idProduct here.
Otherwise the patch looks fine.
cheers,
Gerd
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] usb-redir: remove 'remote wakeup' flag from configuration descriptor
2019-12-02 12:34 [PATCH 0/2] Remove 'remote wakeup' flag from USB config descriptor Yuri Benditovich
2019-12-02 12:34 ` [PATCH 1/2] usb-host: remove 'remote wakeup' flag from configuration descriptor Yuri Benditovich
@ 2019-12-02 12:34 ` Yuri Benditovich
2019-12-03 10:53 ` Gerd Hoffmann
2019-12-02 16:30 ` [PATCH 0/2] Remove 'remote wakeup' flag from USB config descriptor no-reply
2 siblings, 1 reply; 6+ messages in thread
From: Yuri Benditovich @ 2019-12-02 12:34 UTC (permalink / raw)
To: ehabkost, marcel.apfelbaum, kraxel, qemu-devel; +Cc: yan
If the redirected device has this capability, Windows guest may
place the device into D2 and expect it to wake when the device
becomes active, but this will never happen. For example, when
internal Bluetooth adapter is redirected, keyboards and mice
connected to it do not work. Current commit removes this
capability (starting from machine 4.2)
Set 'usb-redir.suppress-remote-wake' property to 'off' to keep
'remote wake' as is or to 'on' to remove 'remote wake' on
4.1 or earlier.
Signed-off-by: Yuri Benditovich <yuri.benditovich@daynix.com>
---
hw/core/machine.c | 1 +
hw/usb/redirect.c | 19 +++++++++++++++++++
2 files changed, 20 insertions(+)
diff --git a/hw/core/machine.c b/hw/core/machine.c
index 8c0eaad091..44408ff87c 100644
--- a/hw/core/machine.c
+++ b/hw/core/machine.c
@@ -30,6 +30,7 @@
GlobalProperty hw_compat_4_1[] = {
{ "virtio-pci", "x-pcie-flr-init", "off" },
{ "usb-host", "suppress-remote-wake", "off" },
+ { "usb-redir", "suppress-remote-wake", "off" },
};
const size_t hw_compat_4_1_len = G_N_ELEMENTS(hw_compat_4_1);
diff --git a/hw/usb/redirect.c b/hw/usb/redirect.c
index e0f5ca6f81..7cc9c2aa00 100644
--- a/hw/usb/redirect.c
+++ b/hw/usb/redirect.c
@@ -113,6 +113,7 @@ struct USBRedirDevice {
/* Properties */
CharBackend cs;
bool enable_streams;
+ bool suppress_remote_wake;
uint8_t debug;
int32_t bootindex;
char *filter_str;
@@ -1989,6 +1990,23 @@ static void usbredir_control_packet(void *priv, uint64_t id,
memcpy(dev->dev.data_buf, data, data_len);
}
p->actual_length = len;
+ /*
+ * If this is GET_DESCRIPTOR request for configuration descriptor,
+ * remove 'remote wakeup' flag from it to prevent idle power down
+ * in Windows guest
+ */
+ if (dev->suppress_remote_wake &&
+ control_packet->requesttype == USB_DIR_IN &&
+ control_packet->request == USB_REQ_GET_DESCRIPTOR &&
+ control_packet->value == (USB_DT_CONFIG << 8) &&
+ control_packet->index == 0 &&
+ /* bmAttributes field of config descriptor */
+ len > 7 && (dev->dev.data_buf[7] & USB_CFG_ATT_WAKEUP)) {
+ DPRINTF("Removed remote wake %04X:%04X\n",
+ dev->device_info.vendor_id,
+ dev->device_info.product_id);
+ dev->dev.data_buf[7] &= ~USB_CFG_ATT_WAKEUP;
+ }
usb_generic_async_ctrl_complete(&dev->dev, p);
}
free(data);
@@ -2530,6 +2548,7 @@ static Property usbredir_properties[] = {
DEFINE_PROP_UINT8("debug", USBRedirDevice, debug, usbredirparser_warning),
DEFINE_PROP_STRING("filter", USBRedirDevice, filter_str),
DEFINE_PROP_BOOL("streams", USBRedirDevice, enable_streams, true),
+ DEFINE_PROP_BOOL("suppress-remote-wake", USBRedirDevice, suppress_remote_wake, true),
DEFINE_PROP_END_OF_LIST(),
};
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] usb-redir: remove 'remote wakeup' flag from configuration descriptor
2019-12-02 12:34 ` [PATCH 2/2] usb-redir: " Yuri Benditovich
@ 2019-12-03 10:53 ` Gerd Hoffmann
0 siblings, 0 replies; 6+ messages in thread
From: Gerd Hoffmann @ 2019-12-03 10:53 UTC (permalink / raw)
To: Yuri Benditovich; +Cc: yan, ehabkost, qemu-devel
Hi,
> + /*
> + * If this is GET_DESCRIPTOR request for configuration descriptor,
> + * remove 'remote wakeup' flag from it to prevent idle power down
> + * in Windows guest
> + */
> + if (dev->suppress_remote_wake &&
> + control_packet->requesttype == USB_DIR_IN &&
> + control_packet->request == USB_REQ_GET_DESCRIPTOR &&
> + control_packet->value == (USB_DT_CONFIG << 8) &&
> + control_packet->index == 0 &&
> + /* bmAttributes field of config descriptor */
> + len > 7 && (dev->dev.data_buf[7] & USB_CFG_ATT_WAKEUP)) {
> + DPRINTF("Removed remote wake %04X:%04X\n",
> + dev->device_info.vendor_id,
> + dev->device_info.product_id);
> + dev->dev.data_buf[7] &= ~USB_CFG_ATT_WAKEUP;
> + }
Hmm, not much opportunity to factor out stuff to share with usb-host.
Ok then.
I think checkpatch has complains for this too, otherwise it looks fine.
cheers,
Gerd
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] Remove 'remote wakeup' flag from USB config descriptor
2019-12-02 12:34 [PATCH 0/2] Remove 'remote wakeup' flag from USB config descriptor Yuri Benditovich
2019-12-02 12:34 ` [PATCH 1/2] usb-host: remove 'remote wakeup' flag from configuration descriptor Yuri Benditovich
2019-12-02 12:34 ` [PATCH 2/2] usb-redir: " Yuri Benditovich
@ 2019-12-02 16:30 ` no-reply
2 siblings, 0 replies; 6+ messages in thread
From: no-reply @ 2019-12-02 16:30 UTC (permalink / raw)
To: yuri.benditovich; +Cc: qemu-devel, yan, ehabkost, kraxel
Patchew URL: https://patchew.org/QEMU/20191202123430.7125-1-yuri.benditovich@daynix.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Subject: [PATCH 0/2] Remove 'remote wakeup' flag from USB config descriptor
Type: series
Message-id: 20191202123430.7125-1-yuri.benditovich@daynix.com
=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
- [tag update] patchew/20191202060806.77968-1-david@gibson.dropbear.id.au -> patchew/20191202060806.77968-1-david@gibson.dropbear.id.au
Switched to a new branch 'test'
91bc743 usb-redir: remove 'remote wakeup' flag from configuration descriptor
e3a79d7 usb-host: remove 'remote wakeup' flag from configuration descriptor
=== OUTPUT BEGIN ===
1/2 Checking commit e3a79d76824a (usb-host: remove 'remote wakeup' flag from configuration descriptor)
WARNING: Block comments use a leading /* on a separate line
#57: FILE: hw/usb/host-libusb.c:400:
+ /* If this is GET_DESCRIPTOR request for configuration descriptor,
WARNING: Block comments use a trailing */ on a separate line
#59: FILE: hw/usb/host-libusb.c:402:
+ * in Windows guest */
ERROR: line over 90 characters
#64: FILE: hw/usb/host-libusb.c:407:
+ xfer->actual_length > offsetof(struct libusb_config_descriptor, bmAttributes) &&
WARNING: line over 80 characters
#68: FILE: hw/usb/host-libusb.c:411:
+ trace_usb_host_remote_wakeup_removed(desc.idVendor, desc.idProduct);
WARNING: line over 80 characters
#78: FILE: hw/usb/host-libusb.c:1616:
+ DEFINE_PROP_BOOL("suppress-remote-wake", USBHostDevice, suppress_remote_wake, true),
total: 1 errors, 4 warnings, 53 lines checked
Patch 1/2 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/2 Checking commit 91bc743fae4b (usb-redir: remove 'remote wakeup' flag from configuration descriptor)
WARNING: line over 80 characters
#72: FILE: hw/usb/redirect.c:2551:
+ DEFINE_PROP_BOOL("suppress-remote-wake", USBRedirDevice, suppress_remote_wake, true),
total: 0 errors, 1 warnings, 44 lines checked
Patch 2/2 has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20191202123430.7125-1-yuri.benditovich@daynix.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
^ permalink raw reply [flat|nested] 6+ messages in thread