Re: [BUG] pgprot_noncached() is -NOT- safe for mapping vmalloc buffers into userspace

[Takashi Iwai <tiwai@suse.de>]

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