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 205C5C433E0 for ; Fri, 5 Mar 2021 00:51:14 +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 9C7E36500E for ; Fri, 5 Mar 2021 00:51:13 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 9C7E36500E 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]:56342 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lHygS-0007dU-JA for qemu-devel@archiver.kernel.org; Thu, 04 Mar 2021 19:51:12 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:58506) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lHyff-00077q-Vc for qemu-devel@nongnu.org; Thu, 04 Mar 2021 19:50:23 -0500 Received: from mail-ua1-f45.google.com ([209.85.222.45]:46295) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1lHyfe-0001sK-3S for qemu-devel@nongnu.org; Thu, 04 Mar 2021 19:50:23 -0500 Received: by mail-ua1-f45.google.com with SMTP id 62so162613uar.13 for ; Thu, 04 Mar 2021 16:50:21 -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=WmSZry98ZcAWlYpHZ4KOEfIEbk/uJWwrMdwTtT1qoks=; b=pf6Ke4XSK1eRpubaKEujFrvv5MDdaDjRQO10hwx1RW6D5rmkgAU4i77Jy/JbEO637Z upXmvQRk6NHCHLVWUIUuQ/1+h46PpWagIt9hzTbjIcBdB0vGJ4ZCOhaqkgEODDzAL7Wl HlmWYtFryxZicPR4VHHCvW6Pb/6+LfNPuIO3Q4ZbAcV9hWXQ7SzHWq3G2J4kIMKlt902 xlWzMIOnP8gOidC8zfz+WoASVNFv3sHF9jT4kHH7Cf3p5CDZ6e2yc4EnUvayb152GECW Bvgs1gEAZOPFZnVWJJnoMahvLU2856eomrwXKd9ZclDK3KMov+GrmivrfEApmT2ZAEwB 92Ig== X-Gm-Message-State: AOAM530yTITdBeaw+29b7wwisPcN9g3edkT6NhwJNjB8Cf8v8RrXbqKh kZX54weZFHKjOiorDq394tGrnmfGIDArzO39Ppc= X-Google-Smtp-Source: ABdhPJwfq6pvemWxdnYl/iD/Osl0R0DvUYyHrdyGtuvBG0/l1jiZSt3p5fgsYCPwYTEFqG6fbGQ+FGc90Tj2m5LOG8c= X-Received: by 2002:a9f:3104:: with SMTP id m4mr4525788uab.127.1614905421122; Thu, 04 Mar 2021 16:50:21 -0800 (PST) MIME-Version: 1.0 References: <20210304143149.jc24h6fh35luzhyb@sirius.home.kraxel.org> In-Reply-To: <20210304143149.jc24h6fh35luzhyb@sirius.home.kraxel.org> From: Ben Leslie Date: Fri, 5 Mar 2021 11:50:09 +1100 Message-ID: Subject: Re: USB port claiming / set configuration problems To: Gerd Hoffmann Content-Type: multipart/alternative; boundary="000000000000f8cee305bcbf77a8" Received-SPF: pass client-ip=209.85.222.45; envelope-from=ben.leslie@gmail.com; helo=mail-ua1-f45.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.249, FREEMAIL_FROM=0.001, HEADER_FROM_DIFFERENT_DOMAINS=0.249, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H2=-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" --000000000000f8cee305bcbf77a8 Content-Type: text/plain; charset="UTF-8" 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. Cheers, Ben --000000000000f8cee305bcbf77a8 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
On Fri, 5 Mar 2021 at 01:31, Gerd Hoffman= n <kraxel@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.

Cheers,

Ben



=C2=A0
--000000000000f8cee305bcbf77a8--