Subject: Re: [RFC PATCH 14/19] powerpc: allow ioremap within reserved fake ram regions

From: Michael Ellerman <[email-masked]>
Date: Tue, 24 Nov 2009 10:45:09 +1100

On Mon, 2009-11-23 at 21:16 +0100, Albert Herranz wrote:
> >>  arch/powerpc/mm/pgtable_32.c |   19 ++++++++++++++++---
> >>  1 files changed, 16 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/arch/powerpc/mm/pgtable_32.c b/arch/powerpc/mm/pgtable_32.c
> >> index cb96cb2..ba00cb1 100644
> >> --- a/arch/powerpc/mm/pgtable_32.c
> >> +++ b/arch/powerpc/mm/pgtable_32.c
> >> @@ -191,9 +191,22 @@ __ioremap_caller(phys_addr_t addr, unsigned long size, unsigned long flags,
> >>         * Don't allow anybody to remap normal RAM that we're using.
> >>         * mem_init() sets high_memory so only do the check after that.
> >>         */
> >> -       if (mem_init_done && (p < virt_to_phys(high_memory))) {
> >> -               printk("__ioremap(): phys addr 0x%llx is RAM lr %p\n",
> >> -                      (unsigned long long)p, __builtin_return_address(0));
> >> +       if (mem_init_done && (p < virt_to_phys(high_memory))
> >> +#ifdef CONFIG_WII
> >> +               /*
> >> +                * On some systems, though, we may want to remap an area
> >> +                * declared as normal RAM that we have memreserve'd at the
> >> +                * device tree. See wii.dts.
> >> +                * But we can't do that safely if we are using BATs to map
> >> +                * part of that area.
> >> +                */
> >> +           && !__map_without_bats
> >> +#endif
> >> +           ) {
> >> +               printk(KERN_WARNING
> >> +                      "__ioremap(): phys addr 0x%llx is RAM lr %p\n",
> >> +                      (unsigned long long)p,
> >> +                        __builtin_return_address(0));
> > 
> > This could adversely affect multiplatform kernels.  I'd rather get the
> > RAM problem fixed and not hack up common code to work around the hack.
> > 
> > g.
> > 
> 
> Would it be acceptable to create a global var __allow_ioremap_normal_ram that by default would have a value of 0 and would be set _only_ for those platforms needing it?
> 
> The other solutions I see is:
> - add support for discontiguous memory to powerpc 32-bits (which is not something that I can look into now)
> - don't use the precious second 64MB area (which is a waste)

- Implement your own ppc_md.ioremap(), see iseries & cell for example.

Currently that's only called on 64-bit, but you could change that.

If I'm reading your patch right, you should be able to do that check in
your platform's ioremap() and then call the generic implementation to do
the actual work.

cheers