[RFC] consistent_sync and non L1 cache line aligned buffers

Hi!

I think this is a known problem.

There are drivers or even subsystems which use stack allocated DMA buffers.
To make things worse, those buffers usually non L1 cache line aligned
(start and/or end).

When they use pci_map_* with PCI_DMA_FROMDEVICE, consistent_sync calls
invalidate_dcache_range for the buffer.

invalidate_dcache_range works in L1_CACHE_LINE chunks, so if start and/or
end of the buffer are not aligned we may corrupt data located in the same
cache line (usually stack variable(s) declared before or after buffer
declaration).

According to MV kernel, there are USB devices that use such buffers.

After spending last weekend with RISCWatch :) I can say that SCSI subsystem
is also guilty of this behavior (drivers/scsi/scsi_scan.c::scan_scsis,
scsi_result0).

Unfortunately, I don't know how many similar places of code are still
waiting to be found :(.
To be safe I think it's better to modify consistent_sync to handle such
"bad" buffers.

If start and/or end of the buffer are not properly aligned I use "dcbf" to
flush corresponding cache line(s) and then call invalidate_dcache_range.

This change doesn't affect performance of consistent_sync noticeably (like
in the variant I found in MV kernel, where invalidate_dcache_range was
changed to flush_dcache_range if USB was enabled)

I don't know whether we should "ifdef" this for CONFIG_4xx and I know this
fix is ugly :)
I'm not even sure that such hacks should be included in the kernel :)))
(but I will definitely use it in my tree)

Comments/suggestions are welcome!

Thanks,

Eugene


===== arch/ppc/mm/cachemap.c 1.13 vs edited =====

--- 1.13/arch/ppc/mm/cachemap.c	Thu Feb 27 11:40:16 2003
+++ edited/arch/ppc/mm/cachemap.c	Mon Jul 14 20:49:28 2003

@@ -150,6 +150,21 @@
  	case PCI_DMA_NONE:
  		BUG();
  	case PCI_DMA_FROMDEVICE:	/* invalidate only */
+
+		/* Handle cases when the buffer start and/or end
+		   are not L1 cache line aligned.
+		   Some drivers/subsystems (e.g. USB, SCSI) do DMA
+		   from the stack allocated buffers, to prevent
+		   corruption of the other stack variables located
+		   near the buffer, we flush (instead of invalidate)
+		   these "dangerous" areas                     --ebs
+		*/
+		if (unlikely(start & (L1_CACHE_LINE_SIZE - 1)))
+			__asm__ __volatile__("dcbf 0,%0" : : "r" (start));
+
+		if (unlikely(end & (L1_CACHE_LINE_SIZE - 1)))
+			__asm__ __volatile__("dcbf 0,%0" : : "r" (end));
+
  		invalidate_dcache_range(start, end);
  		break;
  	case PCI_DMA_TODEVICE:		/* writeback only */

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

On Mon, Jul 14, 2003 at 09:32:07PM -0700, Eugene Surovegin wrote:

I think this is a known problem.

Yes, fortunatly.

According to MV kernel, there are USB devices that use such buffers.

After spending last weekend with RISCWatch :) I can say that SCSI subsystem
is also guilty of this behavior (drivers/scsi/scsi_scan.c::scan_scsis,
scsi_result0).

Owch.

Unfortunately, I don't know how many similar places of code are still
waiting to be found :(.
To be safe I think it's better to modify consistent_sync to handle such
"bad" buffers.

If start and/or end of the buffer are not properly aligned I use "dcbf" to
flush corresponding cache line(s) and then call invalidate_dcache_range.

This change doesn't affect performance of consistent_sync noticeably (like
in the variant I found in MV kernel, where invalidate_dcache_range was
changed to flush_dcache_range if USB was enabled)

Good to know.

I don't know whether we should "ifdef" this for CONFIG_4xx and I know this
fix is ugly :)
I'm not even sure that such hacks should be included in the kernel :)))
(but I will definitely use it in my tree)

Comments/suggestions are welcome!

Well, one thing that is worth noting is that the USB people knew this
was a problem, and it was / should have been fixed in the 2.5 cycle.
Similarly, SCSI was cleaned up a lot, so perhaps this has been fixed
there.  I think it's generally known that doing DMA off of the stack is
a bad idea, and should be fixed when found.

--
Tom Rini
http://gate.crashing.org/~trini/

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

At 08:46 AM 7/15/2003, Tom Rini wrote:

Well, one thing that is worth noting is that the USB people knew this
was a problem, and it was / should have been fixed in the 2.5 cycle.
Similarly, SCSI was cleaned up a lot, so perhaps this has been fixed
there.  I think it's generally known that doing DMA off of the stack is
a bad idea, and should be fixed when found.

I agree this is VERY bad idea but the fact is that there is a code which
does such nasty things.

I truly hope all this will/was fixed in 2.5 but frankly I wouldn't be so
sure :)

Unfortunately, for production 2.5 is unusable and will be for some time.
A lot of people (I think majority) still use 2.4. And 2.4 (as of
2.4.22-pre6) is still broken in this respect...

Eugene


** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/

On Tue, Jul 15, 2003 at 09:20:24AM -0700, Eugene Surovegin wrote:

At 08:46 AM 7/15/2003, Tom Rini wrote:

quoted

Well, one thing that is worth noting is that the USB people knew this
was a problem, and it was / should have been fixed in the 2.5 cycle.
Similarly, SCSI was cleaned up a lot, so perhaps this has been fixed
there.  I think it's generally known that doing DMA off of the stack is
a bad idea, and should be fixed when found.

I agree this is VERY bad idea but the fact is that there is a code which
does such nasty things.

I truly hope all this will/was fixed in 2.5 but frankly I wouldn't be so
sure :)

Well, I would be, of the USB code.  SCSI might have had it fixed, and
others that we haven't found yet may or may not.  But the important
point is that doing this is a driver bug and it's OK to beat driver
authors over the head with patches to fix the behavior. :)

Unfortunately, for production 2.5 is unusable and will be for some time.
A lot of people (I think majority) still use 2.4. And 2.4 (as of
2.4.22-pre6) is still broken in this respect...

Yes, the changes to USB and SCSI probably won't be backported for some
time, if ever.  So there is still a question of should we workaround
this in 2.4 (Or, more to the point, do we leave it up to every
$(EMBEDDED VENDOR) to do it, or bite the bullet and commit it to the 2.4
mainline.

--
Tom Rini
http://gate.crashing.org/~trini/

** Sent via the linuxppc-embedded mail list. See http://lists.linuxppc.org/