USB mass storage and ARM cache coherency
in [Kernel]
|
Prev: [PATCH 0/3] HID: make raw output callback more flexible
Next: kprobes: Disable booster when CONFIG_PREEMPT=y
From: Catalin Marinas on 29 Jan 2010 12:20 On Fri, 2010-01-29 at 16:41 +0000, Oliver Neukum wrote: > Am Freitag, 29. Januar 2010 17:34:03 schrieb Catalin Marinas: > > > I was thinking about checking dev->bus->controller->dma_mask which the > > code (though not the storage one) seems to imply that if the dma_mask is > > 0, the HCD driver is only capable of PIO. > > That a HCD is capable of DMA need not imply that DMA is used for every > transfer. Actually the DMA drivers are safe in this respect only if the transfer happens directly to a page cache page that may be (later) mapped into user space. I'm not familiar with the USB drivers to fully understand the data flow, so any help would be appreciated. > > That would be a more general solution rather than going through each HCD > > driver since my understanding is that flush_dcache_page() is only needed > > together with the mass storage support. > > What about ub, nfs or nbd over a USB<->ethernet converter? > This, I am afraid is best solved at the HCD or glue layer. NFS handles the cache flushing itself, so in this case there is no need to duplicate the cache flushing at the HCD level. AFAICT, the HCD driver may be used in several cases and it's only the storage case (via either ub, mass storage etc.) that requires cache flushing. Is there a way to differentiate between these at the HCD driver level? Regarding nbd, is there any copying happening between the HCD driver receiving the network packet from the USB-ethernet converter and the nbd bio_vec buffers (most likely during the TCP/IP stack flow)? In this case it would be for the nbd driver (doesn't seem to be the case now) to flush the D-cache as the HCD flushing is not necessary as long as it doesn't write directly to the page cache page. The ub case is similar to the USB mass storage one, so they could both benefit from flushing at the HCD driver level. But is this possible without duplicating the flushing in the nfs case? Regards. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Sergei Shtylyov on 29 Jan 2010 13:00 Hello. Catalin Marinas wrote: >>> I've been trying for some time to use a rootfs (ext2) on a USB memory >>> stick on ARM platforms but without any success. The USB HCD driver is >>> ISP1760 which doesn't use DMA. >>> >>> ARM has a Harvard cache architecture and what I get is incoherency >>> between the I and D caches. The CPU I'm using (ARM11MPCore) has PIPT >>> caches with D-cache lines allocation on write. >>> >>> Basically, when user space tries to execute from a new page, it faults >>> and the data is requested via the VFS layer, SCSI block device and USB >>> mass storage from the ISP1760 driver. The page is then mapped into user >>> space and update_mmu_cache() called. >>> >>> However, since the driver is PIO, the data copied from the USB device >>> into RAM gets stuck in the D-cache. On the above page requesting path >>> there is no call to flush_dcache_page() to handle D-cache maintenance >>> (for DMA drivers, that's handled by the DMA API). >>> >>> Since the USB mass storage code has the information about the USB driver >>> >> Sorry, I am a little confused that usb mass storage has what information >> about DMA or PIO of low level usb transfer? >> > > I was thinking about checking dev->bus->controller->dma_mask which the > code (though not the storage one) seems to imply that if the dma_mask is > 0, the HCD driver is only capable of PIO. > > That would be a more general solution rather than going through each HCD > driver since my understanding is that flush_dcache_page() is only needed > together with the mass storage support. Note that DMA capable driver can be doing some transfers in PIO mode or falling back to PIO mode if DMA mode transfer is unsuccessful (the musb driver is an example of the latter and if the DMA rewrite patches will get accepted, it'll do short transfers in PIO mode). MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Greg KH on 29 Jan 2010 14:40 On Fri, Jan 29, 2010 at 10:54:34AM -0800, Matthew Dharm wrote: > On Fri, Jan 29, 2010 at 08:51:47PM +0300, Sergei Shtylyov wrote: > > Catalin Marinas wrote: > > > > >That would be a more general solution rather than going through each HCD > > >driver since my understanding is that flush_dcache_page() is only needed > > >together with the mass storage support. > > > > Note that DMA capable driver can be doing some transfers in PIO mode > > or falling back to PIO mode if DMA mode transfer is unsuccessful (the > > musb driver is an example of the latter and if the DMA rewrite patches > > will get accepted, it'll do short transfers in PIO mode). > > Given that an HCD can choose, on the fly, if it's using DMA or PIO, the HCD > driver is the only place to reasonably put any cache-synchronization code. > > That said, what do the other SCSI HCDs do? I'm guessing the question gets > kinda muddy there, since the other SCSI HCDs all talk directly to some > piece of hardware, and thus are responsible for the cache management > themselves. > > Based on that, one could argue that ub and usb-storage should be doing > this. > > HOWEVER, I firmly believe that the cache-management functions belong with > the driver that actually talks to the low-level hardware, as that's the > only place where you can be 100% certain of what cache operations are > needed. After all, I think someone is working on a USB-over-IP transport, > and trying to manage cache at the usb-storage level in that scenario is > just silly. > > So, let's put this in the HCD drivers and be done with it. I agree, that's the place to fix this issue. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Catalin Marinas on 1 Feb 2010 09:00 On Fri, 2010-01-29 at 18:54 +0000, Matthew Dharm wrote: > On Fri, Jan 29, 2010 at 08:51:47PM +0300, Sergei Shtylyov wrote: > > Catalin Marinas wrote: > > > > >That would be a more general solution rather than going through each HCD > > >driver since my understanding is that flush_dcache_page() is only needed > > >together with the mass storage support. > > > > Note that DMA capable driver can be doing some transfers in PIO mode > > or falling back to PIO mode if DMA mode transfer is unsuccessful (the > > musb driver is an example of the latter and if the DMA rewrite patches > > will get accepted, it'll do short transfers in PIO mode). > > Given that an HCD can choose, on the fly, if it's using DMA or PIO, the HCD > driver is the only place to reasonably put any cache-synchronization code. > > That said, what do the other SCSI HCDs do? I'm guessing the question gets > kinda muddy there, since the other SCSI HCDs all talk directly to some > piece of hardware, and thus are responsible for the cache management > themselves. > > Based on that, one could argue that ub and usb-storage should be doing > this. > > HOWEVER, I firmly believe that the cache-management functions belong with > the driver that actually talks to the low-level hardware, as that's the > only place where you can be 100% certain of what cache operations are > needed. After all, I think someone is working on a USB-over-IP transport, > and trying to manage cache at the usb-storage level in that scenario is > just silly. > > So, let's put this in the HCD drivers and be done with it. Doing this (flush_dcache_page) in the HCD driver (ISP1760) solves my problem. I'll post a patch and also cc the driver maintainer. Thanks. -- Catalin -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Catalin Marinas on 1 Feb 2010 12:30
On Fri, 2010-01-29 at 18:54 +0000, Matthew Dharm wrote: > HOWEVER, I firmly believe that the cache-management functions belong with > the driver that actually talks to the low-level hardware, as that's the > only place where you can be 100% certain of what cache operations are > needed. After all, I think someone is working on a USB-over-IP transport, > and trying to manage cache at the usb-storage level in that scenario is > just silly. > > So, let's put this in the HCD drivers and be done with it. The patch below is what fixes the I-D cache incoherency issues on ARM. I don't particularly like the solution but it seems to be the only one available. IMHO, Linux should have functions similar to the DMA API but for PIO drivers (e.g. pio_map_single/pio_unmap_single) that non-coherent architectures can define, otherwise being no-ops. Any thoughts? Thanks. isp1760: Flush the D-cache for the pipe-in transfer buffers From: Catalin Marinas <catalin.marinas(a)arm.com> 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 <catalin.marinas(a)arm.com> Cc: Matthew Dharm <mdharm-kernel(a)one-eyed-alien.net> Cc: Greg KH <greg(a)kroah.com> Cc: Sebastian Siewior <bigeasy(a)linutronix.de> --- 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..4d3eeee 100644 --- a/drivers/usb/host/isp1760-hcd.c +++ b/drivers/usb/host/isp1760-hcd.c @@ -18,6 +18,8 @@ #include <linux/uaccess.h> #include <linux/io.h> #include <asm/unaligned.h> +#include <asm/cacheflush.h> +#include <asm/memory.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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |