USB port claiming / set configuration problems

USB port claiming / set configuration problems

[Ben Leslie]

[-- Attachment #1: Type: text/plain, Size: 3556 bytes --]

I have encountered a number of devices (mostly mobile phones) which seem to
get very confused if a "SET CONFIGURATION" control transfer (for the same
interface) is performed twice.

Specifically, after receiving a 2nd SET CONFIGURATION (for the same
interface) the device times out on future bulk output transfers. (Sometimes
PING-NAK continues until the overall transfer times out, in other cases the
PING itself timeouts after ~200 PING-NAKs).

While I'm fairly confident in saying that the USB firmware on these devices
is broken they seem to work enough to be operable when running a native
operating system.

Unfortunately when running the same operating system virtualized under Qemu
we are able to trigger this bug.

This was originally found using an older (4.2) version of Qemu. It seems
like that the patch bfe44898848614cfcb3a269bc965afbe1f0f331c was able to
solve the issue for some of the devices we see. Specifically, this avoids
actually performing a SET CONFIGURATION control transfer if there is only a
single configuration. The commit message "Seems some devices become
confused when we call libusb_set_configuration()." seems to confirm some of
the behaviour we have been seeing.

Unfortunately, while this appears to have solved the issue for devices with
a single configuration we still appear to have problems when hitting
devices with multiple configurations (which is not surprising given that
the commit only changed behaviour for single-configuration devices).

To attempt a work-around and validate the theory I change the
`usb_host_set_config` function (in host-libusb.c) such that it first checks
if the current active configuration matches the request configuration, and
if so skips performing the actual SET CONFIGURATION control transfer.

Would a patch of this nature be the right approach?
Perhaps this check could replace the number of configurations check?

Taking a step back here, the larger problem is that Linux host performs
various control transfers prior to qemu (and therefore the guest operating
system) gaining control of the device.
This means the sequence of control transfers with the device is inherently
going to be different when the guest OS is virtualized as compared to
running natively. For well behaving devices this really shouldn't matter,
but not all devices are well behaving!

USBDEVFS has support for `USBDEVFS_CLAIM_PORT` (and
`USBDEVFS_RELEASE_PORT`) ioctls. From the definition this seem designed to
limit the interaction that Linux kernel might have with a device on a
claimed port, which seems perfect for this use case. This in fact used in
previous version of qemu if we go back to the host-linux.c days, but with
the change over to host-libusb.c this functionality was lost.

Was this intentional? Would adding support to host-libusb to use these
ioctl to claim the port be beneficial? Based on a simple test program and
hardware USB traces for a device connected to a 'claimed' port the kernel
does indeed leave the device in an unconfigured state. (Although it still
performs some basic control transfers to gather descriptor, and strangely
seems to in this case make an explicit SET CONFIGURATION transfer, but sets
configuration to zero, rather than an actual configuration, which, at least
for the devices I was able to test with, avoided the problems of calling
SET CONFIGURATION (1) twice). Integrating this support back into
host-libusb.c is a little more involved than the work around described
above, so I'd appreciate any feedback before going down that path.

Thanks,

Ben

[-- Attachment #2: Type: text/html, Size: 4078 bytes --]

Re: USB port claiming / set configuration problems

[Gerd Hoffmann]

  Hi,

> To attempt a work-around and validate the theory I change the
> `usb_host_set_config` function (in host-libusb.c) such that it first checks
> if the current active configuration matches the request configuration, and
> if so skips performing the actual SET CONFIGURATION control transfer.

If libusb is able to correctly report what the current configuration is
(i.e. what the kernel has picked at device detection time) then yes, we
can do that.  And it would probably the best way to tackle the problem
as it should not have bad side effects on other devices so we don't need
a config option to enable/disable this.

> USBDEVFS has support for `USBDEVFS_CLAIM_PORT` (and
> `USBDEVFS_RELEASE_PORT`) ioctls. From the definition this seem designed to
> limit the interaction that Linux kernel might have with a device on a
> claimed port, which seems perfect for this use case. This in fact used in
> previous version of qemu if we go back to the host-linux.c days, but with
> the change over to host-libusb.c this functionality was lost.
> 
> Was this intentional?

The switch to libusb sure.  That solves lots of portability issues.  Not
claiming the port was probably lost because libusb doesn't offer
support for that (don't remember all details after years).

> Would adding support to host-libusb to use these
> ioctl to claim the port be beneficial?

I don't feel like side-stepping libusb.  That is asking for trouble
because usbdevfs might behave differently then and confuse libusb.

So, if anything, libusb would need support that, then qemu can use it.

> Based on a simple test program and
> hardware USB traces for a device connected to a 'claimed' port the kernel
> does indeed leave the device in an unconfigured state. (Although it still
> performs some basic control transfers to gather descriptor, and strangely
> seems to in this case make an explicit SET CONFIGURATION transfer, but sets
> configuration to zero, rather than an actual configuration, which, at least
> for the devices I was able to test with, avoided the problems of calling
> SET CONFIGURATION (1) twice).

We could try that too (set config to zero first, then set the config we
actually want) and see if that works better.

Another option could maybe be a device reset + set config.

These two would probably need a config option to enable/disable the
quirk though.

take care,
  Gerd

Re: USB port claiming / set configuration problems

[Ben Leslie]

[-- Attachment #1: Type: text/plain, Size: 2749 bytes --]

On Fri, 5 Mar 2021 at 01:31, Gerd Hoffmann <kraxel@redhat.com> wrote:

>   Hi,
>
> > Would adding support to host-libusb to use these
> > ioctl to claim the port be beneficial?
>
> I don't feel like side-stepping libusb.  That is asking for trouble
> because usbdevfs might behave differently then and confuse libusb.
>
> So, if anything, libusb would need support that, then qemu can use it.
>
> > Based on a simple test program and
> > hardware USB traces for a device connected to a 'claimed' port the kernel
> > does indeed leave the device in an unconfigured state. (Although it still
> > performs some basic control transfers to gather descriptor, and strangely
> > seems to in this case make an explicit SET CONFIGURATION transfer, but
> sets
> > configuration to zero, rather than an actual configuration, which, at
> least
> > for the devices I was able to test with, avoided the problems of calling
> > SET CONFIGURATION (1) twice).
>
> We could try that too (set config to zero first, then set the config we
> actually want) and see if that works better.
>

This approach seems to work well, and let's me just use libusb, which is a
plus.

It seems I had actually misdiagnosed (or only partially diagnosed) the issue
with my 'problem devices'. It turned out that setting *any* valid
configuration
twice in a row causes problems for the device! So, for example, if the
current
configuration was set to 1, and then set configuration 2 was called that
would
also cause problems. I guess that drivers on other systems ensured that
such a sequence never occurred.

I reverted bfe44898848614cfcb3a269bc965afbe1f0f331c and made this change:

--- a/hw/usb/host-libusb.c
+++ b/hw/usb/host-libusb.c
@@ -955,6 +955,11 @@ static int usb_host_open(USBHostDevice *s,
libusb_device *dev, int hostfd)

     usb_host_detach_kernel(s);

+    rc = libusb_set_configuration(s->dh, 0);
+    if (rc != 0) {
+        goto fail;
+    }
+
     libusb_get_device_descriptor(dev, &s->ddesc);
     usb_host_get_port(s->dev, s->port, sizeof(s->port));

This appears to work for my use cases. (Although I still have more testing
to do).

In terms of the transaction on the wire, this is not quite as good as the
'claim port'
approach. Specifically, with the claim port after setting address and
getting some
basic descriptors the kernel will explicitly set configuration to zero and
not perform
any more transactions. Without the 'claim port' the kernel appears to
configure to
the first configuration and then read a few more descriptors. For my test
cases
at least this doesn't appear to be problematic, but I thought it was worth
calling
out the differences. Of course the great benefit of this approach is that
it uses
existing libusb functionality.

Cheers,

Ben

[-- Attachment #2: Type: text/html, Size: 3662 bytes --]

Re: USB port claiming / set configuration problems

[Ben Leslie]

[-- Attachment #1: Type: text/plain, Size: 3381 bytes --]

On Fri, 5 Mar 2021 at 11:50, Ben Leslie <benno@benno.id.au> wrote:

> On Fri, 5 Mar 2021 at 01:31, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
>>   Hi,
>>
>> > Would adding support to host-libusb to use these
>> > ioctl to claim the port be beneficial?
>>
>> I don't feel like side-stepping libusb.  That is asking for trouble
>> because usbdevfs might behave differently then and confuse libusb.
>>
>> So, if anything, libusb would need support that, then qemu can use it.
>>
>> > Based on a simple test program and
>> > hardware USB traces for a device connected to a 'claimed' port the
>> kernel
>> > does indeed leave the device in an unconfigured state. (Although it
>> still
>> > performs some basic control transfers to gather descriptor, and
>> strangely
>> > seems to in this case make an explicit SET CONFIGURATION transfer, but
>> sets
>> > configuration to zero, rather than an actual configuration, which, at
>> least
>> > for the devices I was able to test with, avoided the problems of calling
>> > SET CONFIGURATION (1) twice).
>>
>> We could try that too (set config to zero first, then set the config we
>> actually want) and see if that works better.
>>
>
> This approach seems to work well, and let's me just use libusb, which is a
> plus.
>
> It seems I had actually misdiagnosed (or only partially diagnosed) the
> issue
> with my 'problem devices'. It turned out that setting *any* valid
> configuration
> twice in a row causes problems for the device! So, for example, if the
> current
> configuration was set to 1, and then set configuration 2 was called that
> would
> also cause problems. I guess that drivers on other systems ensured that
> such a sequence never occurred.
>
> I reverted bfe44898848614cfcb3a269bc965afbe1f0f331c and made this change:
>
> --- a/hw/usb/host-libusb.c
> +++ b/hw/usb/host-libusb.c
> @@ -955,6 +955,11 @@ static int usb_host_open(USBHostDevice *s,
> libusb_device *dev, int hostfd)
>
>      usb_host_detach_kernel(s);
>
> +    rc = libusb_set_configuration(s->dh, 0);
> +    if (rc != 0) {
> +        goto fail;
> +    }
> +
>      libusb_get_device_descriptor(dev, &s->ddesc);
>      usb_host_get_port(s->dev, s->port, sizeof(s->port));
>
> This appears to work for my use cases. (Although I still have more testing
> to do).
>
> In terms of the transaction on the wire, this is not quite as good as the
> 'claim port'
> approach. Specifically, with the claim port after setting address and
> getting some
> basic descriptors the kernel will explicitly set configuration to zero and
> not perform
> any more transactions. Without the 'claim port' the kernel appears to
> configure to
> the first configuration and then read a few more descriptors. For my test
> cases
> at least this doesn't appear to be problematic, but I thought it was worth
> calling
> out the differences. Of course the great benefit of this approach is that
> it uses
> existing libusb functionality.
>
>
For the archives, unfortunately this approach did not work for my use case.
I was able to get something working using the claim port approach, but that
meant going behind the back of libusb, which is obviously not suitable to be
merged, and ends up as a reasonably sizable patch.

I'll see if there is any appetite for adding claim port functionality to
libusb
before raising it again here.

Thanks for the feedback and assistance.

Cheers,

Ben

[-- Attachment #2: Type: text/html, Size: 4545 bytes --]