linux-fbdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: linux-fbdev@vger.kernel.org
Subject: Re: fbmem: VM_IO set, but not propagated.
Date: Tue, 27 Jul 2010 14:53:36 +0000	[thread overview]
Message-ID: <20100727145336.GA4611@phenom.dumpdata.com> (raw)
In-Reply-To: <20100722213128.GA27012@phenom.dumpdata.com>

On Mon, Jul 26, 2010 at 03:38:12PM -0700, Andrew Morton wrote:
> On Thu, 22 Jul 2010 17:31:28 -0400
> Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> 
> > This bug was found when Linux kernel was running under Xen.
> > In that scenario, any page that has VM_IO flag to it, means that it
> > MUST be a MMIO/VRAM backend memory , _not_ System RAM. That is what the
> > fbmem.c does: sets VM_IO, ioremaps the region - everything is peachy.
> > 
> > Well, not exactly. The vm_page_prot does not get the relevant
> > PTE flags set (_PAGE_IOMAP) which under Xen is a death-kneel to pages
> > that are referencing real physical devices but don't have that flag set.
> > 
> > Here is the patch:
> > 
> > Author: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> > Date:   Wed Jul 21 16:52:46 2010 -0400
> > 
> >     fb: propagate VM_IO to VMA.
> >     
> >     When we setup up the VMA flags for the mmap flag and we end up using
> >     the fallback mmap functionality we set the vma->vm_flags |= VM_IO.
> >     However we neglect to propagate the flag to the vma->vm_page_prot.
> >     
> >     This patch fixes this.
> >     
> >     Tested-by: Eamon Walsh <ewalsh@tycho.nsa.gov>
> >     Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > 
> 
> Confused.  We have From:Konrad and Author:Daniel and no signoff from Daniel.

So Daniel came up with the original fix, I've came to him and suggested
that perhaps we should use the vm_get_page_prot, and he agreed.

Since I've kept the patch in my branch of "Xen-KMS/DRM/TTM-fixes" I
figured I should post the patch upstream and hence I signed off on it.

> 
> I've committed the patch assuming that Daniel was the author, but
> didn't sign off the patch.  Your signoff is sufficient for merging

Yes. He is the author. Let me refer back to my notes but I figured
if the patch has an Author: set there was no need for an extra S-o-b?


> purposes.
> 
> But maybe I was wrong.
> 
> 
> I'm also assuming that we can merge this into 2.6.36 and not backport
> it into -stable.  But maybe I'm wrong about that too!  Talk to me.

No need to backport it.

      parent reply	other threads:[~2010-07-27 14:53 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-07-22 21:31 fbmem: VM_IO set, but not propagated Konrad Rzeszutek Wilk
2010-07-26 22:38 ` Andrew Morton
2010-07-27 14:42 ` Daniel De Graaf
2010-07-27 14:53 ` Konrad Rzeszutek Wilk [this message]

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=20100727145336.GA4611@phenom.dumpdata.com \
    --to=konrad.wilk@oracle.com \
    --cc=linux-fbdev@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).