From: Andi Kleen <ak@suse.de>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Andi Kleen <ak@suse.de>,
huang ying <huang.ying.caritas@gmail.com>,
"Huang, Ying" <ying.huang@intel.com>,
Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/5] x86: add executable mapping support to ioremap
Date: Thu, 31 Jan 2008 17:44:12 +0100 [thread overview]
Message-ID: <20080131164412.GF25989@bingen.suse.de> (raw)
In-Reply-To: <alpine.LFD.1.00.0801311732420.9204@apollo.tec.linutronix.de>
On Thu, Jan 31, 2008 at 05:37:05PM +0100, Thomas Gleixner wrote:
> On Thu, 31 Jan 2008, Andi Kleen wrote:
>
> > > There is nothing to fix. It works on virtual addresses.
> > >
> > > The __pa() in change_page_attr_addr() is only used to check for the
> > > high alias mapping of the kernel, but the call to change_page_attr
> > > uses the virtual address.
> >
> > But __pa() doesn't work for ioremap ...
>
> If it does not, then __pa() needs to be fixed, nothing else.
No if, it doesn't.
Changing that seems like a slippery rope. Would you want to make it work for
user space addresses then too? It would certainly become
much more heavyweight because it requires much more checks
and then a page table lookup.
For a fairly uncommon case.
I always thought the cheap direct va<->pa conversions to be one of the basic
design principles of the Linux VM. It's surprising you want to throw
that overboard that quickly.
Better check at least with Linus first, it's a fairly fundamental change.
To be honest IMHO the simplest way to fix this problem would be to just put the
pgprot_t argument back into __ioremap(). Then you wouldn't have all
these problems.
-Andi
prev parent reply other threads:[~2008-01-31 16:44 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-01-31 7:36 [PATCH 4/5] x86: add executable mapping support to ioremap Huang, Ying
2008-01-31 13:00 ` Thomas Gleixner
2008-01-31 13:22 ` huang ying
2008-01-31 13:27 ` Andi Kleen
2008-01-31 16:30 ` Thomas Gleixner
2008-01-31 16:28 ` Thomas Gleixner
2008-01-31 16:30 ` Andi Kleen
2008-01-31 16:37 ` Thomas Gleixner
2008-01-31 16:44 ` Andi Kleen [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=20080131164412.GF25989@bingen.suse.de \
--to=ak@suse.de \
--cc=hpa@zytor.com \
--cc=huang.ying.caritas@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=tglx@linutronix.de \
--cc=ying.huang@intel.com \
/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