All of lore.kernel.org
 help / color / mirror / Atom feed
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Takashi Iwai <tiwai@suse.de>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
	linux-arch@vger.kernel.org,
	Matthew Evans <matt.evans@au1.ibm.com>
Subject: Re: [BUG] pgprot_noncached() is -NOT- safe for mapping vmalloc buffers into userspace
Date: Fri, 25 Mar 2011 20:15:33 +1100	[thread overview]
Message-ID: <1301044533.2402.478.camel@pasglop> (raw)
In-Reply-To: <s5h8vw3vdb1.wl%tiwai@suse.de>

On Fri, 2011-03-25 at 09:01 +0100, Takashi Iwai wrote:
> > 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?

I suppose so :-)

> > 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...

That's ok, I did a new one, which you have noticed already anyways :-)

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

I haven't actually checked, but I assume that the test

substream->dma_buffer.dev.type == SNDRV_DMA_TYPE_DEV

In snd_pcm_default_mmap() takes care of that, please correct me if
I'm wrong in which case we'll need something else there.

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

Are you taking care of sending it upstream ?

Cheers,
Ben.

WARNING: multiple messages have this Message-ID (diff)
From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
To: Takashi Iwai <tiwai@suse.de>
Cc: linux-arch@vger.kernel.org,
	linuxppc-dev <linuxppc-dev@lists.ozlabs.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Matthew Evans <matt.evans@au1.ibm.com>
Subject: Re: [BUG] pgprot_noncached() is -NOT- safe for mapping vmalloc buffers into userspace
Date: Fri, 25 Mar 2011 20:15:33 +1100	[thread overview]
Message-ID: <1301044533.2402.478.camel@pasglop> (raw)
In-Reply-To: <s5h8vw3vdb1.wl%tiwai@suse.de>

On Fri, 2011-03-25 at 09:01 +0100, Takashi Iwai wrote:
> > 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?

I suppose so :-)

> > 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...

That's ok, I did a new one, which you have noticed already anyways :-)

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

I haven't actually checked, but I assume that the test

substream->dma_buffer.dev.type == SNDRV_DMA_TYPE_DEV

In snd_pcm_default_mmap() takes care of that, please correct me if
I'm wrong in which case we'll need something else there.

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

Are you taking care of sending it upstream ?

Cheers,
Ben.

  reply	other threads:[~2011-03-25  9:15 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-24 22:16 [BUG] pgprot_noncached() is -NOT- safe for mapping vmalloc buffers into userspace Benjamin Herrenschmidt
2011-03-24 22:16 ` Benjamin Herrenschmidt
2011-03-25  8:01 ` Takashi Iwai
2011-03-25  8:01   ` Takashi Iwai
2011-03-25  9:15   ` Benjamin Herrenschmidt [this message]
2011-03-25  9:15     ` Benjamin Herrenschmidt
2011-03-25 10:12     ` Takashi Iwai
2011-03-25 10:12       ` Takashi Iwai
2011-03-25 10:23       ` Benjamin Herrenschmidt
2011-03-25 10:23         ` Benjamin Herrenschmidt
2011-03-25 13:17         ` Takashi Iwai
2011-03-25 13:17           ` Takashi Iwai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1301044533.2402.478.camel@pasglop \
    --to=benh@kernel.crashing.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=matt.evans@au1.ibm.com \
    --cc=tiwai@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.