linux-kernel - Re: [PATCH v12 7/8] usb: Adding SuperSpeed support to dummy

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20110524101817.GB14371@legolas.emea.dhcp.ti.com>
Date:	Tue, 24 May 2011 13:18:18 +0300
From:	Felipe Balbi <balbi@...com>
To:	Tanya Brokhman <tlinder@...eaurora.org>
Cc:	'Alan Stern' <stern@...land.harvard.edu>,
	'Felipe Balbi' <balbi@...com>,
	'Sarah Sharp' <sarah.a.sharp@...ux.intel.com>, greg@...ah.com,
	linux-usb@...r.kernel.org, linux-arm-msm@...r.kernel.org,
	ablay@...eaurora.org, 'open list' <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v12 7/8] usb: Adding SuperSpeed support to dummy_hcd

Hi,

On Tue, May 24, 2011 at 08:53:15AM +0300, Tanya Brokhman wrote:
> > >
> > > With Tatyana's patches, if we load a USB2 g_zero to dummy_hcd,
> > enumeration
> > > will fail where it shouldn't. This has been my whole point ;-) Maybe
> > I wasn't
> > > clear enough.
> > 
> > I guess not.  I thought Tatyana said she was working to fix that bug...
> 
> This bug is fixed with the module parameter. When dummy_hcd is loaded it
> registers 2 root hubs: HS & SS but unless the is_super_speed parameter is
> true, the device will be connected to the HS hub. This way when loading HS
> g_zero  it enumerates under HS root hub successfully. 
> Note that if load SS gadget without setting is_super_speed=true for dummy
> hcd, it will also enumerate under HS hub! This is basically a way to test
> that SS devices are backward compatible.

but you also need to make sure that HS gadget enumerates on SS host. I
don't agree that the module parameter is the only option.

> > > >> What about the case where SuperSpeed enumeration
> > > >> fails and you have to fall back to high speed?
> > > >
> > > > If SuperSpeed enumeration fails, say because the device doesn't
> > have
> > > > any SuperSpeed descriptors, xhci-hcd doesn't fall back to high
> > speed,
> > > > does it?  dummy-hcd should behave the same way.
> > >
> > > it should at least. Isn't that what happens between EHCI/OHCI ? HS
> > Chirp
> > > sequencing fails, then we fall back to FullSpeed.
> > 
> > That's a failure in initialization, not a failure in enumeration.
> > 
> > There are two reasons why the HS chirp might fail: the device doesn't
> > support high speed operation or hardware errors prevent the chirp from
> > working.  With dummy-hcd there are no hardware errors (because there's
> > no hardware).
> > 
> > As for whether or not the device supports high-speed or SuperSpeed
> > operation, that's determined by the usb_gadget_driver->speed field.  If
> > the field doesn't specify SuperSpeed then dummy-hcd should connect the
> > gadget to the USB-2 root hub rather than the USB-3 root hub.  Isn't
> > that what Tatyana's patch does?  It contains a line saying:
> > 
> > 		dum->gadget.speed = driver->speed;
> 
> You're right. This is exactly what is done with a small update: if
> is_super_speed=false the gadget will connect to a USB2 root hub even if 
> driver->speed=USB_SPEED_SUPER. In that case dum->gadget.speed will be set to
> USB_SPEED_HIGH. The code that sets this now is:
> 	if (mod_data.is_super_speed)
> 		dum->gadget.speed = driver->speed;
> 	else
> 		dum->gadget.speed = min((u8)USB_SPEED_HIGH,

ok ok, I took a closer look at the original patches and you're changing
drivers->speed to super before converting the gadget drivers to
superspeed. That's what's making enumeration fail on all those guys.

> > > >> It seems like you really
> > > >> need to handle both speeds and the speed fall back parameter in
> > the same
> > > >> driver.  Isn't there some other gadget driver that has a fall back
> > to
> > > >> full or low speed when high speed enumeration fails?
> > > >
> > > > That's a property of the gadget driver, not the UDC driver.  dummy-
> > hcd
> > > > is a UDC driver (and an HCD too).
> > >
> > > USB3.0 dummy_hcd should still enumerate USB2.0 gadget drivers.
> > 
> > Yes, certainly it should.  If it doesn't, that's a bug, not a design
> > error.
> > 
> 
> This version of dummy_hcd enumerates HS devices without any issues.

that's not what I saw, but then again, maybe you changed driver->speed
to super and didn't provide superspeed descriptors.

-- 
balbi

Download attachment "signature.asc" of type "application/pgp-signature" (491 bytes)