[PATCH] EHCI Oops on CONFIG_NOT_COHERENT_CACHE system
gerhard_pircher at gmx.net
Wed Aug 30 01:07:59 EST 2006
I guess this message should also be forwarded to linux-usb-devel at lists.sourceforge.net. I hope the developers there can make some comments.
-------- Original-Nachricht --------
Datum: Sun, 27 Aug 2006 18:17:14 +0200
Von: Marcus Comstedt <marcus at mc.pp.se>
An: linuxppc-dev at ozlabs.org
Betreff: [PATCH] EHCI Oops on CONFIG_NOT_COHERENT_CACHE system
> I'm running 126.96.36.199 on an AmigaOneXE, which is a G4 based board
> which has a northbridge (ArticiaS) which does not support cache
> coherency. Because of this, CONFIG_NOT_COHERENT_CACHE is set.
> The problem I've been having is that the EHCI USB2 host driver causes
> a kernel oops (see attachment) immediately on bootup.
> First, let me outline why this oops happens:
> 1) The EHCI driver uses a structure called "echi_qh", which contains
> both data to be accessed by the USB hardware through DMA, and
> private housekeeping data.
> 2) Since part of the structure is for DMA, instances of the structre
> are allocated with dma_pool_alloc().
> 3) Pages allocated with dma_pool_alloc() are cache-inhibited on this
> system, due to the lack of cache coherency support.
> 4) The private data in this structure included a struct kref, which in
> turn contains an atomic_t.
> 5) Incrementing and decrementing an atom_t, and thereby a kref, is
> done with lwarx/stwcx.
> 6) lwarx on a cache-inhibited address is not allowed on G4 (generates
> a DSI).
> Now, the problem is deciding in which of these steps the actual error
> lies, since none of these facts (apart from #6) is set in stone. In
> my opinion though, it makes sense to simply say that atomic_t:s (and
> therefore kref:s) are not supported in DMA memory. This would place
> the error in the EHCI driver, with two possible solutions:
> A) Rewrite qh_get() and qh_put() to use something else than
> kref_get()/kref_put(). Simply using non-atomic increment and
> decrement made the Oops go away, but as I don't know the design
> decision behind using a struct kref, I can't say that atomicity
> isn't needed, so such a simple fix might lead to race conditions.
> B) Break struct ehci_qh into two parts, one allcated with
> dma_pool_alloc() and one allocated with kmalloc(), where the fields
> accessed by the hardware is put into the former, and the driver
> private data (which includes the kref) in the latter. Safe and no
> major performance hit (just one level of indirection added in some
> places, and using cache-enabled memory for the internal data might
> actually improve performance), but the change touches a rather
> large amount of lines (patch attached).
> C) Basically the same as B, but only the kref (and a pointer back to
> the rest of the data, so that qh_destroy can find it) is moved to
> the kmalloced part. This means only ehci_qh_alloc(), qh_get() and
> qh_put() need to be changed, so the changeset is much smaller. I
> don't have a patch ready for this, but I can make one on request.
> A completely different approach would be
> D) Make the DSI exception handler emulate lwarx on cache-inhibited
> This seems like a more complex fix though, and I'm sure the
> performance would be pretty lousy (although only NOT_COHERENT_CACHE
> systems would be affected of course).
> So, what do you guys think? Which is the best way to rectify the
> situation? (Apart from changing to a better northbrige, which I don't
> see happening, realistically. :-/ )
> // Marcus
Der GMX SmartSurfer hilft bis zu 70% Ihrer Onlinekosten zu sparen!
Ideal für Modem und ISDN: http://www.gmx.net/de/go/smartsurfer
More information about the Linuxppc-dev