linux-kernel - Re: [PATCH 3/3] usb: dwc3: add a quirk for device disconnection issue in Synopsis dwc3 core

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-next>] [day] [month] [year] [list]

Message-ID: <20150112172032.GD6525@saruman>
Date:	Mon, 12 Jan 2015 11:20:32 -0600
From:	Felipe Balbi <balbi@...com>
To:	Sneeker Yeh <sneeker.yeh@...il.com>
CC:	Felipe Balbi <balbi@...com>, Rob Herring <robh+dt@...nel.org>,
	Pawel Moll <pawel.moll@....com>,
	Mark Rutland <mark.rutland@....com>,
	Ian Campbell <ijc+devicetree@...lion.org.uk>,
	Kumar Gala <galak@...eaurora.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Mathias Nyman <mathias.nyman@...el.com>,
	Grant Likely <grant.likely@...aro.org>,
	Alan Stern <stern@...land.harvard.edu>,
	Arnd Bergmann <arnd@...db.de>, Paul Bolle <pebolle@...cali.nl>,
	Hans de Goede <hdegoede@...hat.com>,
	Thomas Pugliese <thomas.pugliese@...il.com>,
	David Mosberger <davidm@...uge.net>,
	Peter Griffin <peter.griffin@...aro.org>,
	Sylwester Nawrocki <s.nawrocki@...sung.com>,
	Andrew Bresticker <abrestic@...omium.org>,
	Gregory CLEMENT <gregory.clement@...e-electrons.com>,
	Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>,
	<devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<linux-usb@...r.kernel.org>, <linux-omap@...r.kernel.org>,
	Andy Green <andy.green@...aro.org>,
	Jassi Brar <jaswinder.singh@...aro.org>,
	Sneeker Yeh <Sneeker.Yeh@...fujitsu.com>
Subject: Re: [PATCH 3/3] usb: dwc3: add a quirk for device disconnection
 issue in Synopsis dwc3 core

Hi,

On Sun, Jan 11, 2015 at 11:19:55PM +0800, Sneeker Yeh wrote:
> > > > enable the quirk only for you. Isn't there a better way of enabling the
> > > > quirk based off of revision detection couple with a look on GHWPARAMS*
> > > > registers ?
> > > >
> > > > What's tricking me is this claim that only config-free PHYs would be
> > > > affected, why ?
> > > >
> > >
> > > i'm still struggling now to try to get more information about this.
> > > some security policy inside Fujitsu make me unable to access full
> > > information of this errata today.
> > >
> > > Someday after i get enough information,
> > > i shall take your suggestion here that seems better to incur quirk
> > > dynamically via GHWPARAMS,
> > > and then send it here again.
> >
> > ok, hopefully you'll find a way ;-)
> >
> > I got some update information here finally~
> in case i express unclearly i also put a pdf:
> https://drive.google.com/file/d/0B18MmcvvKjNNbDF6eEdHSzZCazA/view
> 
> This issue is defined by a two-way race at disconnect, between
> 1) class driver interrupt endpoint resheduling attempts if the ISR gave an
> ep error event due to device detach (it would try 3 times)
> 2) Disconnect interrupt on PORTSC_CSC, which is cleared by hub thread
> asynchronously
> 3) The hardware IP was configured in silicon with
>        - DWC_USB3_SUSPEND_ON_DISCONNECT_EN=1 (this is an IP configuration

yeah, aparently this is another configuration which is not exposed on
HWPARAMS registers. Paul, can you confirm for us ? I couldn't find it on
Databook on any of the HWPARAMS registers.

> port whose state cannot be checked from software)
>        - Synopsys IP version is < 3.00a

heh, so pretty much everybody :-)

>        The IP will auto-suspend itself on device detach with some
> phy-specific interval after CSC is cleared by 2)
> 
> If 2) and 3) complete before 1), the interrupts it expects will not be
> generated by the autosuspended IP, leading to a deadlock. Even later
> disconnection procedure would detect that corresponding urb is still
> in-progress and issue a ep stop command, auto-suspended IP still won't
> respond to that command.
> 
> this defect would result in this when device detached:
> -------------------
> [   99.603544] usb 4-1: USB disconnect, device number 2
> [  104.615254] xhci-hcd xhci-hcd.0.auto: xHCI host not responding to stop
> endpoint command.
> [  104.623362] xhci-hcd xhci-hcd.0.auto: Assuming host is dying, halting
> host.
> [  104.653261] xhci-hcd xhci-hcd.0.auto: Host not halted after 16000
> microseconds.
> [  104.660584] xhci-hcd xhci-hcd.0.auto: Non-responsive xHCI host is not
> halting.
> [  104.667817] xhci-hcd xhci-hcd.0.auto: Completing active URBs anyway.
> [  104.674198] xhci-hcd xhci-hcd.0.auto: HC died; cleaning up
> --------------------
> As a result, when device detached, we desired to postpone "PORTCSC clear"
> behind "disable slot". it's found that all executed ep command related to
> disconnetion, are executed before "disable slot".

Now this is all great information and they should all be part of your
commit log and probably a big comment should be added to code as well.

Thanks for going after all these details, now let's figure out a way to
pass dwc3 revision to xhci, or maybe we pass just a flag for the quirk,
something like:

	if (dwc->revision < 3.00a && dwc->has_suspend_on_disconnect)
		xhci_pdata.delay_portcsc_clear = true;

or something similar to that.

cheers

-- 
balbi

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