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

[Albert Herranz <albert_herranz@yahoo.es>]

Grant Likely wrote:
> On Sun, Nov 22, 2009 at 3:01 PM, Albert Herranz <albert_herranz@yahoo.es> wrote:
>> The Nintendo Wii has two discontiguous RAM memory areas called
>> MEM1 and MEM2.
>> MEM1 starts at 0x00000000 and contains 24MB of 1T-SRAM.
>> MEM2 starts at 0x10000000 and contains 64MB of DDR2 RAM.
>> Between both memory address ranges there is an address space
>> where memory-mapped I/O registers are found.
>>
>> Currently, Linux 32-bit PowerPC does not support RAM in
>> discontiguous memory address spaces. Thus, in order to use
>> both RAM areas, we declare as RAM the range from the start of
>> MEM1 to the end of useable MEM2 and exclude the needed parts
>> with /memreserve/ statements, at the expense of wasting a bit
>> of memory.
>>
>> As a side effect, we need to allow ioremapping RAM areas
>> because the I/O address space sits within the memreserve'd part
>> of the declared RAM region.
>> Note that this is not safe if the region ioremapped is covered
>> by an existing BAT mapping used to map RAM, so this is
>> specifically banned here.
>>
>> Signed-off-by: Albert Herranz <albert_herranz@yahoo.es>
>> ---
>>  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)

Do you have any other suggestions?

Thanks,
Albert