This patch makes ioremap() can be used to map pages as executable,
this is needed by EFI support.
Signed-off-by: Huang Ying <[email protected]>
---
arch/x86/mm/ioremap.c | 35 +++++++++++++++++++++++------------
include/asm-x86/io_32.h | 14 ++++++++++++++
include/asm-x86/io_64.h | 14 ++++++++++++++
3 files changed, 51 insertions(+), 12 deletions(-)
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -20,11 +20,6 @@
#include <asm/tlbflush.h>
#include <asm/pgalloc.h>
-enum ioremap_mode {
- IOR_MODE_UNCACHED,
- IOR_MODE_CACHED,
-};
-
#ifdef CONFIG_X86_64
unsigned long __phys_addr(unsigned long x)
@@ -71,7 +66,8 @@ int page_is_ram(unsigned long pagenr)
* conflicts.
*/
static int ioremap_change_attr(unsigned long paddr, unsigned long size,
- enum ioremap_mode mode)
+ enum ioremap_mode mode,
+ enum ioremap_xmode xmode)
{
unsigned long vaddr = (unsigned long)__va(paddr);
unsigned long nrpages = size >> PAGE_SHIFT;
@@ -98,6 +94,16 @@ static int ioremap_change_attr(unsigned
break;
}
+ if (err)
+ return err;
+
+ if (__supported_pte_mask & _PAGE_NX) {
+ if (xmode == IOR_XMODE_EXEC)
+ err = set_memory_x(vaddr, nrpages);
+ else
+ err = set_memory_nx(vaddr, nrpages);
+ }
+
return err;
}
@@ -110,8 +116,9 @@ static int ioremap_change_attr(unsigned
* have to convert them into an offset in a page-aligned mapping, but the
* caller shouldn't need to know that small detail.
*/
-static void __iomem *__ioremap(unsigned long phys_addr, unsigned long size,
- enum ioremap_mode mode)
+void __iomem *__ioremap(unsigned long phys_addr, unsigned long size,
+ enum ioremap_mode mode,
+ enum ioremap_xmode xmode)
{
void __iomem *addr;
struct vm_struct *area;
@@ -148,6 +155,9 @@ static void __iomem *__ioremap(unsigned
break;
}
+ if ((__supported_pte_mask & _PAGE_NX) && xmode == IOR_XMODE_EXEC)
+ prot = __pgprot(pgprot_val(prot) & ~_PAGE_NX);
+
/*
* Mappings have to be page-aligned
*/
@@ -169,7 +179,7 @@ static void __iomem *__ioremap(unsigned
return NULL;
}
- if (ioremap_change_attr(phys_addr, size, mode) < 0) {
+ if (ioremap_change_attr(phys_addr, size, mode, xmode) < 0) {
vunmap(addr);
return NULL;
}
@@ -200,13 +210,13 @@ static void __iomem *__ioremap(unsigned
*/
void __iomem *ioremap_nocache(unsigned long phys_addr, unsigned long size)
{
- return __ioremap(phys_addr, size, IOR_MODE_UNCACHED);
+ return __ioremap(phys_addr, size, IOR_MODE_UNCACHED, IOR_XMODE_UNEXEC);
}
EXPORT_SYMBOL(ioremap_nocache);
void __iomem *ioremap_cache(unsigned long phys_addr, unsigned long size)
{
- return __ioremap(phys_addr, size, IOR_MODE_CACHED);
+ return __ioremap(phys_addr, size, IOR_MODE_CACHED, IOR_XMODE_UNEXEC);
}
EXPORT_SYMBOL(ioremap_cache);
@@ -254,7 +264,8 @@ void iounmap(volatile void __iomem *addr
}
/* Reset the direct mapping. Can block */
- ioremap_change_attr(p->phys_addr, p->size, IOR_MODE_CACHED);
+ ioremap_change_attr(p->phys_addr, p->size,
+ IOR_MODE_CACHED, IOR_XMODE_UNEXEC);
/* Finally remove it */
o = remove_vm_area((void *)addr);
--- a/include/asm-x86/io_64.h
+++ b/include/asm-x86/io_64.h
@@ -153,6 +153,20 @@ static inline void * phys_to_virt(unsign
extern void *early_ioremap(unsigned long addr, unsigned long size);
extern void early_iounmap(void *addr, unsigned long size);
+enum ioremap_mode {
+ IOR_MODE_UNCACHED,
+ IOR_MODE_CACHED,
+};
+
+enum ioremap_xmode {
+ IOR_XMODE_EXEC,
+ IOR_XMODE_UNEXEC,
+};
+
+extern void __iomem *__ioremap(unsigned long phys_addr, unsigned long size,
+ enum ioremap_mode mode,
+ enum ioremap_xmode xmode);
+
/*
* This one maps high address device memory and turns off caching for that area.
* it's useful if some control registers are in such an area and write combining
--- a/include/asm-x86/io_32.h
+++ b/include/asm-x86/io_32.h
@@ -100,6 +100,20 @@ static inline void * phys_to_virt(unsign
*/
#define page_to_phys(page) ((dma_addr_t)page_to_pfn(page) << PAGE_SHIFT)
+enum ioremap_mode {
+ IOR_MODE_UNCACHED,
+ IOR_MODE_CACHED,
+};
+
+enum ioremap_xmode {
+ IOR_XMODE_EXEC,
+ IOR_XMODE_UNEXEC,
+};
+
+extern void __iomem *__ioremap(unsigned long phys_addr, unsigned long size,
+ enum ioremap_mode mode,
+ enum ioremap_xmode xmode);
+
/**
* ioremap - map bus memory into CPU space
* @offset: bus address of the memory
On Thu, 31 Jan 2008, Huang, Ying wrote:
> This patch makes ioremap() can be used to map pages as executable,
> this is needed by EFI support.
> +extern void __iomem *__ioremap(unsigned long phys_addr, unsigned long size,
> + enum ioremap_mode mode,
> + enum ioremap_xmode xmode);
> +
Why do you want to add a new API?
addr = ioremap(phys_addr, len);
set_memory_x(addr);
should do what you want already. I think Arjan was not very clear in
his reply yesterday.
We tried to avoid the intermingling of ioremap and the page attribute
settings. We just kept the cached/uncached part.
Now thinking about it further, we should probably fully decouple the
mapping and the attribute change and remove the enum argument from
__ioremap and change the implementations of ioremap_cached and
ioremap_uncached to do the attribute setting after the remap.
That way ioremap_chached and ioremap_uncached just would become what
they should be: conveniance functions for the developers.
Thanks,
tglx
On Jan 31, 2008 9:00 PM, Thomas Gleixner <[email protected]> wrote:
> On Thu, 31 Jan 2008, Huang, Ying wrote:
>
> > This patch makes ioremap() can be used to map pages as executable,
> > this is needed by EFI support.
>
> > +extern void __iomem *__ioremap(unsigned long phys_addr, unsigned long size,
> > + enum ioremap_mode mode,
> > + enum ioremap_xmode xmode);
> > +
>
> Why do you want to add a new API?
>
> addr = ioremap(phys_addr, len);
> set_memory_x(addr);
>
> should do what you want already. I think Arjan was not very clear in
> his reply yesterday.
In current implementation, change_page_attr() can be used to change
identity map only. At least I can find __pa() are used in
change_page_attr_addr() to get physical address. So I think
change_page_attr() should be fixed firstly.
> We tried to avoid the intermingling of ioremap and the page attribute
> settings. We just kept the cached/uncached part.
>
> Now thinking about it further, we should probably fully decouple the
> mapping and the attribute change and remove the enum argument from
> __ioremap and change the implementations of ioremap_cached and
> ioremap_uncached to do the attribute setting after the remap.
>
> That way ioremap_chached and ioremap_uncached just would become what
> they should be: conveniance functions for the developers.
If the memory area be ioremap() has identity map too. We need two
set_memory_xxx(addr). That is, we need:
addr = ioremap(phys_addr, len);
set_memory_x(addr);
if (phys_addr < (end_pfn_mapped >> PAGE_SHIFT))
set_memory_x(__va(phys_addr));
Maybe we can hide the fact of identity map into set_memory_xxx(), just
specify the physical address to set_memory_xxx() too.
Best Regards,
Huang Ying
> addr = ioremap(phys_addr, len);
> set_memory_x(addr);
> if (phys_addr < (end_pfn_mapped >> PAGE_SHIFT))
> set_memory_x(__va(phys_addr));
If you just need the ioremap executable there is no reason to split
up the direct mapping for this too. The x86 architecture does not require
x bits to be coherent between mappings; only the caching attributes.
-Andi
On Thu, 31 Jan 2008, huang ying wrote:
> On Jan 31, 2008 9:00 PM, Thomas Gleixner <[email protected]> wrote:
> > On Thu, 31 Jan 2008, Huang, Ying wrote:
> >
> > > This patch makes ioremap() can be used to map pages as executable,
> > > this is needed by EFI support.
> >
> > > +extern void __iomem *__ioremap(unsigned long phys_addr, unsigned long size,
> > > + enum ioremap_mode mode,
> > > + enum ioremap_xmode xmode);
> > > +
> >
> > Why do you want to add a new API?
> >
> > addr = ioremap(phys_addr, len);
> > set_memory_x(addr);
> >
> > should do what you want already. I think Arjan was not very clear in
> > his reply yesterday.
>
> In current implementation, change_page_attr() can be used to change
> identity map only. At least I can find __pa() are used in
> change_page_attr_addr() to get physical address. So I think
> change_page_attr() should be fixed firstly.
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.
Thanks,
tglx
> 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 ...
huang is completely right.
-Andi
On Thu, 31 Jan 2008, Andi Kleen wrote:
> > addr = ioremap(phys_addr, len);
> > set_memory_x(addr);
> > if (phys_addr < (end_pfn_mapped >> PAGE_SHIFT))
> > set_memory_x(__va(phys_addr));
>
> If you just need the ioremap executable there is no reason to split
> up the direct mapping for this too. The x86 architecture does not require
> x bits to be coherent between mappings; only the caching attributes.
Thanks for pointing this out.
tglx
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.
Thanks,
tglx
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