lkml.org 
[lkml]   [2011]   [Mar]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [BUG] pgprot_noncached() is -NOT- safe for mapping vmalloc buffers into userspace
At Fri, 25 Mar 2011 09:16:48 +1100,
Benjamin Herrenschmidt wrote:
>
> Hi Takashi !
>
> While working on endian-fixing xHCI with Matt (CC), we discovered the
> source of our problems with usb-audio on a board we were working on.
>
> c32d977b8157bf67cdf47729ce7dd054a26eb534
> "ALSA: pcm - Call pgprot_noncached() for vmalloc'ed buffers"
>
> I'm afraid that this is totally bogus :-)
>
> I don't know on what arch it is safe to have the same memory be mapped
> cachable in the kernel (and accessed via this cached mapping) and
> non-cachable in userspace, but I can confidently say that wherever it
> works it does so by accident.
>
> In the case of usb-audio, what we observed is that the user application
> was writing samples using an uncached mapping, so directly to memory,
> which does -not- invalidate conflicting cache lines on the way, an the
> kernel would then memcpy those data to the USB buffers using a cached
> mapping (vmalloc) and essentially get stale stuff from the cache instead
> of the real samples.
>
> Worse, on some processors, it's actually -illegal- to create (and even
> more to -access-) a conflicting mapping of a page of memory, ie, have it
> mapped cached somewhere and uncached somewhere else. It will lockup some
> processors and afaik, some x86 as well.
>
> In fact, cache coherent architectures often don't support mapping memory
> uncached -at-all- so something like snd_pcm_lib_mmap_noncached()
> shouldn't exist, or at least be under arch control. There's no case
> where it's "always safe". There will almost always be a cache alias in
> the linear mapping unless special arch specific sauce has been applied.

I see. I'll take your removal patch.
It should be applied to stable kernel, too, right?

> Now, there's another problem on top of that, which is that
> snd_pcm_default_mmap() will not work properly the "other way around" on
> powerpc, where the mapping -needs- to be uncached bcs you are running on
> a non cache coherent embedded CPU and trying to mmap DMA memory, but
> that's something that needs fixing inside powerpc by properly defining
> dma_mmap_coherent() & ARCH_HAS_DMA_MMAP_COHERENT (I thought we had added
> it a while back but it's not upstream, patch must have got lost).

Mea culpa. I moved to a different team and have had little time for
the upstream development since then...

> We
> must also make sure we don't go down that path for vmalloc memory
> though.

Yes.

Your patch looks good. Thanks for taking care of this!


Takashi


\
 
 \ /
  Last update: 2011-03-25 09:03    [W:0.050 / U:0.504 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site