From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,HTML_MESSAGE,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5230EC433E0 for ; Tue, 9 Mar 2021 07:54:24 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 80B9464F0F for ; Tue, 9 Mar 2021 07:54:23 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 80B9464F0F Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=benno.id.au Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:35640 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lJXCA-0006gz-Fw for qemu-devel@archiver.kernel.org; Tue, 09 Mar 2021 02:54:22 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:39092) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lJXAz-0006Ej-1c for qemu-devel@nongnu.org; Tue, 09 Mar 2021 02:53:09 -0500 Received: from mail-ej1-f50.google.com ([209.85.218.50]:45634) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1lJXAx-0005Xg-88 for qemu-devel@nongnu.org; Tue, 09 Mar 2021 02:53:08 -0500 Received: by mail-ej1-f50.google.com with SMTP id mm21so25492500ejb.12 for ; Mon, 08 Mar 2021 23:53:06 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=PY2VHsDpxouPcVwsWXx4dyG7j2TPcUj52kPEdMDSBcA=; b=T6fxn8+KuMUS7FIL63XO+JNxEBfEk4MrEF8RViIBZn0z7Hq8KDi4418pm88iqw2TmM ZlHG82MIMHb4bB81jnhGEMhg7mUXcoGOgz5qX4lDKF33R4sx/sBpQfMcsBwy5WNH0vC8 KCU3PlCcXGkeAqfO+83wmF7aIB+r07VSNjCv8gvpikpglknRxPvwpUtDc3OmT2a1vlZH o5Q4pkQ7bnxixWLnrduEC4j94LZLkW6g4ix6nLK5WGcISA/UlO5j6JaUqlTVsKppQScB qsb8l/jaOYYNGx3PnWk1thtmXynX/4367du/x+4Tw9p9i6hf15fnJm/nh1SvkVXIx1mQ rmgA== X-Gm-Message-State: AOAM5302xkj83Nu9v+RtUSABIO9NvKa22kkXhpwRy6AsTch715iuDMdL g8IPIrBgkMlMdtZN4H+x5fQdU064FuEVKmfvBhE= X-Google-Smtp-Source: ABdhPJyhrx2174EvfOzIpBb26KyY+v6Qzo0cu6w9FK1IF6+qfds/euXpgGufRovs79jdi4U6pc3VAlRJpi2Zw1Y2NT0= X-Received: by 2002:a17:906:f0c8:: with SMTP id dk8mr16729919ejb.300.1615276385758; Mon, 08 Mar 2021 23:53:05 -0800 (PST) MIME-Version: 1.0 References: <20210304143149.jc24h6fh35luzhyb@sirius.home.kraxel.org> In-Reply-To: From: Ben Leslie Date: Tue, 9 Mar 2021 18:52:54 +1100 Message-ID: Subject: Re: USB port claiming / set configuration problems To: Gerd Hoffmann Content-Type: multipart/alternative; boundary="0000000000002ff2d505bd15d76e" Received-SPF: pass client-ip=209.85.218.50; envelope-from=ben.leslie@gmail.com; helo=mail-ej1-f50.google.com X-Spam_score_int: -13 X-Spam_score: -1.4 X-Spam_bar: - X-Spam_report: (-1.4 / 5.0 requ) BAYES_00=-1.9, FREEMAIL_FORGED_FROMDOMAIN=0.25, FREEMAIL_FROM=0.001, HEADER_FROM_DIFFERENT_DOMAINS=0.249, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=no autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: qemu-devel@nongnu.org Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" --0000000000002ff2d505bd15d76e Content-Type: text/plain; charset="UTF-8" On Fri, 5 Mar 2021 at 11:50, Ben Leslie wrote: > On Fri, 5 Mar 2021 at 01:31, Gerd Hoffmann 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 --0000000000002ff2d505bd15d76e Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
On Fri, 5 Mar 2021 at 11:50, Ben Leslie &= lt;benno@benno.id.au> wrote:
On Fri, 5 Mar 2021 at 01:31, G= erd Hoffmann <kra= xel@redhat.com> wrote:
=C2=A0 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.=C2=A0 That is asking for troubl= e
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 st= ill
> performs some basic control transfers to gather descriptor, and strang= ely
> 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 calli= ng
> 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, w= hich is a plus.

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

I rever= ted bfe44898848614cfcb3a269bc965afbe1f0f331c and made this change:

--- a/hw/usb/host-libusb.c<= br>+++ b/hw/usb/host-libusb.c
@@ -955,6 +955,11 @@ static int usb_host_o= pen(USBHostDevice *s, libusb_device *dev, int hostfd)

=C2=A0 =C2=A0 = =C2=A0usb_host_detach_kernel(s);

+ =C2=A0 =C2=A0rc =3D libusb_set_co= nfiguration(s->dh, 0);
+ =C2=A0 =C2=A0if (rc !=3D 0) {
+ =C2=A0 = =C2=A0 =C2=A0 =C2=A0goto fail;
+ =C2=A0 =C2=A0}
+
=C2=A0 =C2=A0 = =C2=A0libusb_get_device_descriptor(dev, &s->ddesc);
=C2=A0 =C2=A0= =C2=A0usb_host_get_port(s->dev, s->port, sizeof(s->port));
<= div>
This appears to work for my use cases. (Although I still= have more testing to do).

In terms of the transac= tion on the wire, this is not quite as good as the 'claim port'
approach. Specifically, with the claim port after setting address an= d getting some
basic descriptors the kernel will explicitly set c= onfiguration to zero and not perform
any more transactions. Witho= ut the 'claim port' the kernel appears to configure to
th= e first configuration and then read a few more descriptors. For my test cas= es
at least this doesn't appear to be problematic, but I thou= ght it was worth calling
out the differences. Of course the great= benefit of this approach is that it uses
existing libusb functio= nality.


For the archi= ves, unfortunately this approach did not work for my use case.
I was able to get something working using the claim port approach, but t= hat
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



--0000000000002ff2d505bd15d76e--