When the HDC driver writes the data to the transfer buffers it pollutes
the D-cache (unlike DMA drivers where the device writes the data). If
the corresponding pages get mapped into user space, there are no
additional cache flushing operations performed and this causes random
user space faults on architectures with separate I and D caches
(Harvard) or those with aliasing D-cache.
Signed-off-by: Catalin Marinas <[email protected]>
Cc: Matthew Dharm <[email protected]>
Cc: Greg KH <[email protected]>
Cc: Sebastian Siewior <[email protected]>
---
drivers/usb/host/isp1760-hcd.c | 10 ++++++++++
1 files changed, 10 insertions(+), 0 deletions(-)
diff --git a/drivers/usb/host/isp1760-hcd.c b/drivers/usb/host/isp1760-hcd.c
index 27b8f7c..51445ad 100644
--- a/drivers/usb/host/isp1760-hcd.c
+++ b/drivers/usb/host/isp1760-hcd.c
@@ -17,7 +17,9 @@
#include <linux/debugfs.h>
#include <linux/uaccess.h>
#include <linux/io.h>
+#include <linux/mm.h>
#include <asm/unaligned.h>
+#include <asm/cacheflush.h>
#include "../core/hcd.h"
#include "isp1760-hcd.h"
@@ -904,6 +906,14 @@ __acquires(priv->lock)
status = 0;
}
+ if (usb_pipein(urb->pipe) && usb_pipetype(urb->pipe) == PIPE_BULK) {
+ void *ptr;
+ for (ptr = urb->transfer_buffer;
+ ptr < urb->transfer_buffer + urb->transfer_buffer_length;
+ ptr += PAGE_SIZE)
+ flush_dcache_page(virt_to_page(ptr));
+ }
+
/* complete() can reenter this HCD */
usb_hcd_unlink_urb_from_ep(priv_to_hcd(priv), urb);
spin_unlock(&priv->lock);
* Catalin Marinas | 2010-02-02 11:11:35 [+0000]:
>When the HDC driver writes the data to the transfer buffers it pollutes
>the D-cache (unlike DMA drivers where the device writes the data). If
>the corresponding pages get mapped into user space, there are no
>additional cache flushing operations performed and this causes random
>user space faults on architectures with separate I and D caches
>(Harvard) or those with aliasing D-cache.
After looking through lib/scatterlist.c it uses
#include <linux/highmem.h>
and
flush_kernel_dcache_page().
Wouldn't this do the job here as well?
Sebastain
On Tue, 2010-02-02 at 12:08 +0000, Sebastian Andrzej Siewior wrote:
> * Catalin Marinas | 2010-02-02 11:11:35 [+0000]:
>
> >When the HDC driver writes the data to the transfer buffers it pollutes
> >the D-cache (unlike DMA drivers where the device writes the data). If
> >the corresponding pages get mapped into user space, there are no
> >additional cache flushing operations performed and this causes random
> >user space faults on architectures with separate I and D caches
> >(Harvard) or those with aliasing D-cache.
>
> After looking through lib/scatterlist.c it uses
> #include <linux/highmem.h>
>
> and
>
> flush_kernel_dcache_page().
> Wouldn't this do the job here as well?
The documentation implies that this is to be used only with pages
obtained with kmap(), which doesn't seem to be the case here. On some
ARM processors it isn't even implemented (kunmap does the necessary
flushing).
--
Catalin
2010/2/2 Catalin Marinas <[email protected]>:
> + ? ? ? if (usb_pipein(urb->pipe) && usb_pipetype(urb->pipe) == PIPE_BULK) {
> + ? ? ? ? ? ? ? void *ptr;
> + ? ? ? ? ? ? ? for (ptr = urb->transfer_buffer;
> + ? ? ? ? ? ? ? ? ? ?ptr < urb->transfer_buffer + urb->transfer_buffer_length;
> + ? ? ? ? ? ? ? ? ? ?ptr += PAGE_SIZE)
> + ? ? ? ? ? ? ? ? ? ? ? flush_dcache_page(virt_to_page(ptr));
If the page is mapped into highmem, seems virt_to_page doesn't work well.
--
Lei Ming
On Tue, 2010-02-02 at 15:41 +0000, Ming Lei wrote:
> 2010/2/2 Catalin Marinas <[email protected]>:
>
> > + if (usb_pipein(urb->pipe) && usb_pipetype(urb->pipe) == PIPE_BULK) {
> > + void *ptr;
> > + for (ptr = urb->transfer_buffer;
> > + ptr < urb->transfer_buffer + urb->transfer_buffer_length;
> > + ptr += PAGE_SIZE)
> > + flush_dcache_page(virt_to_page(ptr));
>
> If the page is mapped into highmem, seems virt_to_page doesn't work well.
You are right but is it possible that we get a highmem virtual address
in this case?
As for a general solution, maybe trying to define something like a pio_*
API would work better.
--
Catalin
Ming Lei wrote:
> 2010/2/2 Catalin Marinas <[email protected]>:
>
>> + if (usb_pipein(urb->pipe) && usb_pipetype(urb->pipe) == PIPE_BULK) {
>> + void *ptr;
>> + for (ptr = urb->transfer_buffer;
>> + ptr < urb->transfer_buffer + urb->transfer_buffer_length;
>> + ptr += PAGE_SIZE)
>> + flush_dcache_page(virt_to_page(ptr));
>
> If the page is mapped into highmem, seems virt_to_page doesn't work well.
This does not matter because this can not happen :). If the buffer is
highmem then a bounce buffer is used. See commit 96983d2d8 aka "USB:
storage: set bounce limit for non-DMA-capable host controllers"
Sebastian