linux-kernel - RE: [PATCH v2] usb: dwc2: call dwc2_is_controller

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: <A2CA0424C0A6F04399FB9E1CD98E03048450774D@US01WEMBX2.internal.synopsys.com>
Date:	Wed, 14 Jan 2015 22:45:26 +0000
From:	Paul Zimmerman <Paul.Zimmerman@...opsys.com>
To:	"balbi@...com" <balbi@...com>
CC:	Alan Stern <stern@...land.harvard.edu>,
	Robert Baldyga <r.baldyga@...sung.com>,
	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"dinguyen@...nsource.altera.com" <dinguyen@...nsource.altera.com>,
	"yousaf.kaukab@...el.com" <yousaf.kaukab@...el.com>,
	"m.szyprowski@...sung.com" <m.szyprowski@...sung.com>
Subject: RE: [PATCH v2] usb: dwc2: call dwc2_is_controller_alive() under
 spinlock

> From: Felipe Balbi [mailto:balbi@...com]
> Sent: Wednesday, January 14, 2015 2:40 PM
> 
> On Wed, Jan 14, 2015 at 10:28:54PM +0000, Paul Zimmerman wrote:
> > > From: Felipe Balbi [mailto:balbi@...com]
> > > Sent: Wednesday, January 14, 2015 1:46 PM
> > >
> > > On Wed, Jan 14, 2015 at 04:41:23PM -0500, Alan Stern wrote:
> > > > > > > This is really, really odd. Register accesses are atomic, so the lock
> > > > > > > isn't really doing anything. Besides, you're calling
> > > > > > > dwc2_is_controller_alive() from within the IRQ handler, so IRQs are
> > > > > > > already disabled.
> > > > > >
> > > > > > Spinlocks sometimes do more than you think.  For instance, here the
> > > > > > lock prevents the register access from happening while some other CPU
> > > > > > is holding the lock.  If a silicon quirk causes the register access to
> > > > > > interfere with other activities, this could be important.
> > > > >
> > > > > readl() (which is used by dwc2_is_controller_alive()) adds a memory
> > > > > barrier to the register accesses, that should force all register
> > > > > accesses the be correctly ordered.
> > > >
> > > > Memory barriers will order accesses that are all made on the same CPU
> > > > with respect to each other.  They do not order these accesses against
> > > > accesses made from another CPU -- that's why we have spinlocks.  :-)
> > >
> > > a fair point :-) The register is still read-only, so that shouldn't
> > > matter either :-)
> > >
> > > > >  I fail to see how a silicon quirk
> > > > > could cause this and if, indeed, it does, I'd be more comfortable with a
> > > > > proper STARS tickect number from synopsys :-s
> > > >
> > > > Maybe accessing this register somehow resets something else.  I don't
> > > > know.  It seems unlikely, but at least it explains how adding a
> > > > spinlock could fix the problem.
> > >
> > > I would really need Paul (or someone at Synopsys) to confirm this
> > > somehow. Maybe it has something to do with how the register is
> > > implemented, dunno.
> > >
> > > Paul, do you have any idea what could cause this ? Could the HW into
> > > some weird state if we read GSNPSID at random locations or when data is
> > > being transferred, or anything like that ?
> >
> > Only thing I can think of is that there is some silicon bug in Robert's
> > platform. But I am not aware of any STARs that mention accesses to the
> > GSNPSID register as being problematic.
> >
> > Funny thing is, this code has been basically the same since at least
> > November 2013. So I think some other recent change must have modified
> > the timing of the register accesses, or something like that. But that's
> > just handwaving, really.
> 
> Alright, I'll apply this patch but for 3.20 with a stable tag as I have
> already sent my last pull request to Greg. Unless someone has a really
> big complaint about doing things as such.

It should go to 3.19-rc shouldn't it? It's a fix, and Robert's platform
is broken without it, IIUC.

-- 
Paul

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/