[PATCH] EHCI Oops on CONFIG_NOT_COHERENT_CACHE system

STALE7283d

4 messages, 2 authors, 2006-08-30 · open the first message on its own page

[PATCH] EHCI Oops on CONFIG_NOT_COHERENT_CACHE system

From: Marcus Comstedt <hidden>
Date: 2006-08-27 16:39:54

Hello.

I'm running 2.6.16.27 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
   pages.

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


Re: [PATCH] EHCI Oops on CONFIG_NOT_COHERENT_CACHE system

From: Gerhard Pircher <hidden>
Date: 2006-08-29 15:08:01

Hi Marcus,

I guess this message should also be forwarded to linux-usb-devel@lists.sourceforge.net. I hope the developers there can make some comments.

Gerhard

-------- Original-Nachricht --------
Datum: Sun, 27 Aug 2006 18:17:14 +0200
Von: Marcus Comstedt [off-list ref]
An: linuxppc-dev@ozlabs.org
Betreff: [PATCH] EHCI Oops on CONFIG_NOT_COHERENT_CACHE system
Hello.

I'm running 2.6.16.27 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
   pages.

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

Re: [PATCH] EHCI Oops on CONFIG_NOT_COHERENT_CACHE system

From: Marcus Comstedt <hidden>
Date: 2006-08-29 21:06:02

"Gerhard Pircher" [off-list ref] writes:
Hi Marcus,

I guess this message should also be forwarded to linux-usb-devel@lists.sourceforge.net. I hope the developers there can make some comments.

Gerhard

Well, I figured the first thing to do would be to reach a consensus
here on whether or not atomic_t:s in DMA memory should be ruled as
unallowed.  Judging from the massive silence, there doesn't seem to be
any strong opinions either way though...


  // Marcus

Re: [PATCH] EHCI Oops on CONFIG_NOT_COHERENT_CACHE system

From: Gerhard Pircher <hidden>
Date: 2006-08-30 09:46:02

-------- Original-Nachricht --------
Datum: Tue, 29 Aug 2006 23:04:44 +0200
Von: Marcus Comstedt [off-list ref]
An: "Gerhard Pircher" [off-list ref]
Betreff: Re: [PATCH] EHCI Oops on CONFIG_NOT_COHERENT_CACHE system
"Gerhard Pircher" [off-list ref] writes:
quoted
Hi Marcus,

I guess this message should also be forwarded to
linux-usb-devel@lists.sourceforge.net. I hope the developers there can
make some comments.

Gerhard

Well, I figured the first thing to do would be to reach a consensus
here on whether or not atomic_t:s in DMA memory should be ruled as
unallowed.  Judging from the massive silence, there doesn't seem to be
any strong opinions either way though...


  // Marcus
You're right. Maybe it's a topic for the linuxppc-embedded mailing list, because mostly embedded platforms use the CONFIG_NOT_COHERENT_CACHE option.

Gerhard

-- 


Der GMX SmartSurfer hilft bis zu 70% Ihrer Onlinekosten zu sparen!
Ideal für Modem und ISDN: http://www.gmx.net/de/go/smartsurfer
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help