[RFC] Prohibit ioremap() on kernel managed RAM

[RFC] Prohibit ioremap() on kernel managed RAM

[Russell King - ARM Linux]

ARMv6 and above have a restriction whereby aliasing virtual:physical
mappings must not have differing memory type and sharability
attributes.  Strictly, this covers the memory type (strongly ordered,
device, memory), cache attributes (uncached, write combine, write
through, write back read alloc, write back write alloc) and the
shared bit.

However, using ioremap() and its variants on system RAM results in
mappings which differ in these attributes from the main system RAM
mapping.  Other architectures which similar restrictions approch this
problem in the same way - they do not permit ioremap on main system
RAM.

Make ARM behave in the same way, with a WARN_ON() such that users can
be traced and an alternative approach found.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
index 28c8b95..03f1193 100644
--- a/arch/arm/mm/ioremap.c
+++ b/arch/arm/mm/ioremap.c
@@ -268,6 +268,12 @@ void __iomem * __arm_ioremap_pfn_caller(unsigned long pfn,
 	if (pfn >= 0x100000 && (__pfn_to_phys(pfn) & ~SUPERSECTION_MASK))
 		return NULL;
 
+	/*
+	 * Don't allow RAM to be mapped - this causes problems with ARMv6+
+	 */
+	if (WARN_ON(pfn_valid(pfn)))
+		return NULL;
+
 	type = get_mem_type(mtype);
 	if (!type)
 		return NULL;

[RFC] Prohibit ioremap() on kernel managed RAM

[Catalin Marinas]

On Thu, 2010-04-08 at 10:48 +0100, Russell King - ARM Linux wrote:
> ARMv6 and above have a restriction whereby aliasing virtual:physical
> mappings must not have differing memory type and sharability
> attributes.  Strictly, this covers the memory type (strongly ordered,
> device, memory), cache attributes (uncached, write combine, write
> through, write back read alloc, write back write alloc) and the
> shared bit.
> 
> However, using ioremap() and its variants on system RAM results in
> mappings which differ in these attributes from the main system RAM
> mapping.  Other architectures which similar restrictions approch this
> problem in the same way - they do not permit ioremap on main system
> RAM.
> 
> Make ARM behave in the same way, with a WARN_ON() such that users can
> be traced and an alternative approach found.
> 
> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

BTW, why not do this for /dev/mem mappings with O_SYNC? I think I sent
you a patch some time ago but I can re-post it.

-- 
Catalin

[RFC] Prohibit ioremap() on kernel managed RAM

[Shilimkar, Santosh]

> -----Original Message-----
> From: linux-arm-kernel-bounces at lists.infradead.org [mailto:linux-arm-kernel-
> bounces at lists.infradead.org] On Behalf Of Catalin Marinas
> Sent: Friday, April 23, 2010 7:46 PM
> To: Russell King - ARM Linux
> Cc: linux-arm-kernel at lists.infradead.org
> Subject: Re: [RFC] Prohibit ioremap() on kernel managed RAM
> 
> On Thu, 2010-04-08 at 10:48 +0100, Russell King - ARM Linux wrote:
> > ARMv6 and above have a restriction whereby aliasing virtual:physical
> > mappings must not have differing memory type and sharability
> > attributes.  Strictly, this covers the memory type (strongly ordered,
> > device, memory), cache attributes (uncached, write combine, write
> > through, write back read alloc, write back write alloc) and the
> > shared bit.
> >
> > However, using ioremap() and its variants on system RAM results in
> > mappings which differ in these attributes from the main system RAM
> > mapping.  Other architectures which similar restrictions approch this
> > problem in the same way - they do not permit ioremap on main system
> > RAM.
> >
> > Make ARM behave in the same way, with a WARN_ON() such that users can
> > be traced and an alternative approach found.
> >
> > Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> 
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
> 
> BTW, why not do this for /dev/mem mappings with O_SYNC? I think I sent
> you a patch some time ago but I can re-post it.
> 
Above change is necessary but what an alternative approach is for this. 
There are many use case where ioremap* is needed.

Regards,
Santosh

[RFC] Prohibit ioremap() on kernel managed RAM

[Russell King]

> Above change is necessary but what an alternative approach is for this. 
> There are many use case where ioremap* is needed.

This is a very difficult issue to answer; the only way we can safely
remap RAM with different attributes is if we disable the existing
mappings - but since we create those with 1MB sections, that's far
from easy to achieve.

I think a viable safe solution is to set aside some RAM at boot (which
the kernel doesn't manage at all) and then use ioremap on that; that
approach will still work with this patch in place.

Note that I've applied this patch to my devel tree, which has been
pushed out; please note that I'm making use of others 'net connections
to do this.

[RFC] Prohibit ioremap() on kernel managed RAM

[George G. Davis]

Hi,

On Fri, Apr 23, 2010 at 03:40:58PM +0100, Russell King wrote:
> > Above change is necessary but what an alternative approach is for this. 
> > There are many use case where ioremap* is needed.
> 
> This is a very difficult issue to answer; the only way we can safely
> remap RAM with different attributes is if we disable the existing
> mappings - but since we create those with 1MB sections, that's far
> from easy to achieve.
> 
> I think a viable safe solution is to set aside some RAM at boot (which
> the kernel doesn't manage at all) and then use ioremap on that; that
> approach will still work with this patch in place.

So cases such as the omapfb driver which use reserve_bootmem() (in
arch/arm/plat-omap/fb.c) and then later use ioremap_wc() to remap
reserved memory (in drivers/video/omap2/omapfb/omapfb-main.c)
will no longer work with this change.

Is that correct?

Thanks!

--
Regards,
George

[RFC] Prohibit ioremap() on kernel managed RAM

[Catalin Marinas]

On Fri, 2010-04-30 at 17:33 +0100, George G. Davis wrote:
> Hi,
> 
> On Fri, Apr 23, 2010 at 03:40:58PM +0100, Russell King wrote:
> > > Above change is necessary but what an alternative approach is for this.
> > > There are many use case where ioremap* is needed.
> >
> > This is a very difficult issue to answer; the only way we can safely
> > remap RAM with different attributes is if we disable the existing
> > mappings - but since we create those with 1MB sections, that's far
> > from easy to achieve.
> >
> > I think a viable safe solution is to set aside some RAM at boot (which
> > the kernel doesn't manage at all) and then use ioremap on that; that
> > approach will still work with this patch in place.
> 
> So cases such as the omapfb driver which use reserve_bootmem() (in
> arch/arm/plat-omap/fb.c) and then later use ioremap_wc() to remap
> reserved memory (in drivers/video/omap2/omapfb/omapfb-main.c)
> will no longer work with this change.

Another solution would be to allow the unmapping of sections from the
kernel linear mapping. I think x86 does this already for the AGP
aperture.

-- 
Catalin

[RFC] Prohibit ioremap() on kernel managed RAM

[Shilimkar, Santosh]

> -----Original Message-----
> From: Catalin Marinas [mailto:catalin.marinas at arm.com]
> Sent: Friday, April 30, 2010 10:09 PM
> To: George G. Davis
> Cc: Russell King; Shilimkar, Santosh; linux-arm-kernel at lists.infradead.org
> Subject: Re: [RFC] Prohibit ioremap() on kernel managed RAM
> 
> On Fri, 2010-04-30 at 17:33 +0100, George G. Davis wrote:
> > Hi,
> >
> > On Fri, Apr 23, 2010 at 03:40:58PM +0100, Russell King wrote:
> > > > Above change is necessary but what an alternative approach is for this.
> > > > There are many use case where ioremap* is needed.
> > >
> > > This is a very difficult issue to answer; the only way we can safely
> > > remap RAM with different attributes is if we disable the existing
> > > mappings - but since we create those with 1MB sections, that's far
> > > from easy to achieve.
> > >
> > > I think a viable safe solution is to set aside some RAM at boot (which
> > > the kernel doesn't manage at all) and then use ioremap on that; that
> > > approach will still work with this patch in place.
> >
> > So cases such as the omapfb driver which use reserve_bootmem() (in
> > arch/arm/plat-omap/fb.c) and then later use ioremap_wc() to remap
> > reserved memory (in drivers/video/omap2/omapfb/omapfb-main.c)
> > will no longer work with this change.
> 
> Another solution would be to allow the unmapping of sections from the
> kernel linear mapping. I think x86 does this already for the AGP
> aperture.
This seems to be good solution if it's doable. Reserving memory in the boot
is not so flexible and might end up in waste of memory.

Regards,
Santosh

[RFC] Prohibit ioremap() on kernel managed RAM

[Russell King - ARM Linux]

On Sat, May 01, 2010 at 11:54:55AM +0530, Shilimkar, Santosh wrote:
> This seems to be good solution if it's doable. Reserving memory in the boot
> is not so flexible and might end up in waste of memory.

However, allocating large pages (larger than 4K) is unreliable due to
fragmentation.  There's two factors here - the larger the allocation,
and the longer the system has been running, the more likely such an
allocation is to fail.

Since we'd need to enforce a minimum allocation size of 1MB in order to
unmap the direct mapping, we're asking for an order-8 page - and I think
you'll find that'll be fairly unreliable if you want to keep allocating
and freeing it.

The DMA allocators can handle anything up to MAX_ORDER-order allocations
provided it has enough virtual memory space available to it - so if you
want to go down this route, you might as well switch to using that.
(However, eventually the DMA allocator also needs fixing to avoid the
dual-mappings that it creates.)

[RFC] Prohibit ioremap() on kernel managed RAM

[George G. Davis]

Hi,

On Fri, Apr 30, 2010 at 05:38:31PM +0100, Catalin Marinas wrote:
> On Fri, 2010-04-30 at 17:33 +0100, George G. Davis wrote:
> > Hi,
> > 
> > On Fri, Apr 23, 2010 at 03:40:58PM +0100, Russell King wrote:
> > > > Above change is necessary but what an alternative approach is for this.
> > > > There are many use case where ioremap* is needed.
> > >
> > > This is a very difficult issue to answer; the only way we can safely
> > > remap RAM with different attributes is if we disable the existing
> > > mappings - but since we create those with 1MB sections, that's far
> > > from easy to achieve.
> > >
> > > I think a viable safe solution is to set aside some RAM at boot (which
> > > the kernel doesn't manage at all) and then use ioremap on that; that
> > > approach will still work with this patch in place.
> > 
> > So cases such as the omapfb driver which use reserve_bootmem() (in
> > arch/arm/plat-omap/fb.c) and then later use ioremap_wc() to remap
> > reserved memory (in drivers/video/omap2/omapfb/omapfb-main.c)
> > will no longer work with this change.
> 
> Another solution would be to allow the unmapping of sections from the
> kernel linear mapping. I think x86 does this already for the AGP
> aperture.

I've yet to grok that code but have been curious about how this is done
for x86 graphics.  : )

--
Regards,
George
> 
> -- 
> Catalin

[RFC] Prohibit ioremap() on kernel managed RAM

[Catalin Marinas]

On Mon, 2010-05-03 at 16:59 +0100, George G. Davis wrote:
> On Fri, Apr 30, 2010 at 05:38:31PM +0100, Catalin Marinas wrote:
> > On Fri, 2010-04-30 at 17:33 +0100, George G. Davis wrote:
> > > Hi,
> > >
> > > On Fri, Apr 23, 2010 at 03:40:58PM +0100, Russell King wrote:
> > > > > Above change is necessary but what an alternative approach is for this.
> > > > > There are many use case where ioremap* is needed.
> > > >
> > > > This is a very difficult issue to answer; the only way we can safely
> > > > remap RAM with different attributes is if we disable the existing
> > > > mappings - but since we create those with 1MB sections, that's far
> > > > from easy to achieve.
> > > >
> > > > I think a viable safe solution is to set aside some RAM at boot (which
> > > > the kernel doesn't manage at all) and then use ioremap on that; that
> > > > approach will still work with this patch in place.
> > >
> > > So cases such as the omapfb driver which use reserve_bootmem() (in
> > > arch/arm/plat-omap/fb.c) and then later use ioremap_wc() to remap
> > > reserved memory (in drivers/video/omap2/omapfb/omapfb-main.c)
> > > will no longer work with this change.
> >
> > Another solution would be to allow the unmapping of sections from the
> > kernel linear mapping. I think x86 does this already for the AGP
> > aperture.
> 
> I've yet to grok that code but have been curious about how this is done
> for x86 graphics.  : )

I don't know exactly but I've got reports in the past that kmemleak was
trying to scan the AGP aperture on x86 and it wasn't actually mapped,
though it was allocated via alloc_bootmem().

Another option I noticed on x86 is the use of functions like
io_mapping_create_wc() which call iomap_create_wc() and
io_reserve_memtype(). The latter calls kernel_map_sync_memtype() which
seems to change the attributes of the kernel linear mapping for the
given physical address.

-- 
Catalin

[RFC] Prohibit ioremap() on kernel managed RAM

[George G. Davis]

Hi,

On Tue, May 04, 2010 at 04:29:57PM +0100, Catalin Marinas wrote:
> On Mon, 2010-05-03 at 16:59 +0100, George G. Davis wrote:
> > On Fri, Apr 30, 2010 at 05:38:31PM +0100, Catalin Marinas wrote:
> > > On Fri, 2010-04-30 at 17:33 +0100, George G. Davis wrote:
> > > > Hi,
> > > >
> > > > On Fri, Apr 23, 2010 at 03:40:58PM +0100, Russell King wrote:
> > > > > > Above change is necessary but what an alternative approach is for this.
> > > > > > There are many use case where ioremap* is needed.
> > > > >
> > > > > This is a very difficult issue to answer; the only way we can safely
> > > > > remap RAM with different attributes is if we disable the existing
> > > > > mappings - but since we create those with 1MB sections, that's far
> > > > > from easy to achieve.
> > > > >
> > > > > I think a viable safe solution is to set aside some RAM at boot (which
> > > > > the kernel doesn't manage at all) and then use ioremap on that; that
> > > > > approach will still work with this patch in place.
> > > >
> > > > So cases such as the omapfb driver which use reserve_bootmem() (in
> > > > arch/arm/plat-omap/fb.c) and then later use ioremap_wc() to remap
> > > > reserved memory (in drivers/video/omap2/omapfb/omapfb-main.c)
> > > > will no longer work with this change.
> > >
> > > Another solution would be to allow the unmapping of sections from the
> > > kernel linear mapping. I think x86 does this already for the AGP
> > > aperture.
> > 
> > I've yet to grok that code but have been curious about how this is done
> > for x86 graphics.  : )
> 
> I don't know exactly but I've got reports in the past that kmemleak was
> trying to scan the AGP aperture on x86 and it wasn't actually mapped,
> though it was allocated via alloc_bootmem().
> 
> Another option I noticed on x86 is the use of functions like
> io_mapping_create_wc() which call iomap_create_wc() and
> io_reserve_memtype(). The latter calls kernel_map_sync_memtype() which
> seems to change the attributes of the kernel linear mapping for the
> given physical address.

Ah, ok, interesting, __ioremap_caller() in arch/x86/mm/ioremap.c has:

	/*
	 * Don't allow anybody to remap normal RAM that we're using..
	 */
	for (pfn = phys_addr >> PAGE_SHIFT;
				(pfn << PAGE_SHIFT) < (last_addr & PAGE_MASK);
				pfn++) {

		int is_ram = page_is_ram(pfn);

		if (is_ram && pfn_valid(pfn) && !PageReserved(pfn_to_page(pfn)))
                        return NULL;
		WARN_ON_ONCE(is_ram);
	}


I'm still not clear on all the ins and outs of reserving the AGP aperture
but it looks like it's done via agp_generic_create_gatt_table()
in drivers/char/agp/generic.c where alloc_gatt_pages() does a call to
__get_free_pages(), for x86, and does SetPageReserved() for each page
and then uses set_memory_uc() to update the memory type attributes.

So could we similarly we relax this "Prohibit ioremap() on kernel managed
RAM" change for reserved pages?  Like so:

	/*
	 * Don't allow RAM to be mapped - this causes problems with ARMv6+
	 */
	if (WARN_ON(pfn_valid(pfn) && !PageReserved(pfn_to_page(pfn)))
		return NULL;


Thanks again!

--
Regards,
George

> 
> -- 
> Catalin

[RFC] Prohibit ioremap() on kernel managed RAM

[Russell King - ARM Linux]

On Wed, May 05, 2010 at 12:23:45PM -0400, George G. Davis wrote:
> So could we similarly we relax this "Prohibit ioremap() on kernel managed
> RAM" change for reserved pages?  Like so:
> 
> 	/*
> 	 * Don't allow RAM to be mapped - this causes problems with ARMv6+
> 	 */
> 	if (WARN_ON(pfn_valid(pfn) && !PageReserved(pfn_to_page(pfn)))
> 		return NULL;
> 

Only once the rest of the solution gets implemented; making this change
on its own doesn't mean very much or have very much effect.

[RFC] Prohibit ioremap() on kernel managed RAM

[George G. Davis]

Hi,

On Wed, May 05, 2010 at 05:30:12PM +0100, Russell King - ARM Linux wrote:
> On Wed, May 05, 2010 at 12:23:45PM -0400, George G. Davis wrote:
> > So could we similarly we relax this "Prohibit ioremap() on kernel managed
> > RAM" change for reserved pages?  Like so:
> > 
> > 	/*
> > 	 * Don't allow RAM to be mapped - this causes problems with ARMv6+
> > 	 */
> > 	if (WARN_ON(pfn_valid(pfn) && !PageReserved(pfn_to_page(pfn)))
> > 		return NULL;
> > 
> 
> Only once the rest of the solution gets implemented; making this change
> on its own doesn't mean very much or have very much effect.

That's fine, I'm just (trying ; ) to think ahead.

Thanks!

--
Regards,
George

[RFC] Prohibit ioremap() on kernel managed RAM

[Russell King - ARM Linux]

On Fri, Apr 30, 2010 at 12:33:56PM -0400, George G. Davis wrote:
> Hi,
> 
> On Fri, Apr 23, 2010 at 03:40:58PM +0100, Russell King wrote:
> > > Above change is necessary but what an alternative approach is for this. 
> > > There are many use case where ioremap* is needed.
> > 
> > This is a very difficult issue to answer; the only way we can safely
> > remap RAM with different attributes is if we disable the existing
> > mappings - but since we create those with 1MB sections, that's far
> > from easy to achieve.
> > 
> > I think a viable safe solution is to set aside some RAM at boot (which
> > the kernel doesn't manage at all) and then use ioremap on that; that
> > approach will still work with this patch in place.
> 
> So cases such as the omapfb driver which use reserve_bootmem() (in
> arch/arm/plat-omap/fb.c) and then later use ioremap_wc() to remap
> reserved memory (in drivers/video/omap2/omapfb/omapfb-main.c)
> will no longer work with this change.

Correct; and there's two reasons why they'll break in the near future.
The first is down to ioremap not wanting to allow remapping of already
existing memory.

The second is that when we switch to using LMB instead of bootmem, this
makes the use of reserve_bootmem() a problem in fb.c.

There's really one option for this - in the machine's fixup handler, you
can walk the meminfo array and kick out some memory from that.  This will
prevent the kernel mapping that memory and make pfn_valid() fail for that
memory range.  Another advantage of this is that it won't break when we
switch to LMB.

[RFC] Prohibit ioremap() on kernel managed RAM

[George G. Davis]

Hi,

On Fri, Apr 30, 2010 at 07:19:40PM +0100, Russell King - ARM Linux wrote:
> On Fri, Apr 30, 2010 at 12:33:56PM -0400, George G. Davis wrote:
> > Hi,
> > 
> > On Fri, Apr 23, 2010 at 03:40:58PM +0100, Russell King wrote:
> > > > Above change is necessary but what an alternative approach is for this. 
> > > > There are many use case where ioremap* is needed.
> > > 
> > > This is a very difficult issue to answer; the only way we can safely
> > > remap RAM with different attributes is if we disable the existing
> > > mappings - but since we create those with 1MB sections, that's far
> > > from easy to achieve.
> > > 
> > > I think a viable safe solution is to set aside some RAM at boot (which
> > > the kernel doesn't manage at all) and then use ioremap on that; that
> > > approach will still work with this patch in place.
> > 
> > So cases such as the omapfb driver which use reserve_bootmem() (in
> > arch/arm/plat-omap/fb.c) and then later use ioremap_wc() to remap
> > reserved memory (in drivers/video/omap2/omapfb/omapfb-main.c)
> > will no longer work with this change.
> 
> Correct; and there's two reasons why they'll break in the near future.
> The first is down to ioremap not wanting to allow remapping of already
> existing memory.
> 
> The second is that when we switch to using LMB instead of bootmem, this
> makes the use of reserve_bootmem() a problem in fb.c.
> 
> There's really one option for this - in the machine's fixup handler, you
> can walk the meminfo array and kick out some memory from that.  This will
> prevent the kernel mapping that memory and make pfn_valid() fail for that
> memory range.  Another advantage of this is that it won't break when we
> switch to LMB.

I guess another option is to pass mem= on the cmdline to leave memory
free for ioremap'ing as needed.  Yuck.  : )

Thanks!

--
Regards,
George