Please help with the OMAP static mapping mess

Please help with the OMAP static mapping mess

[Nicolas Pitre]

<rant>

I must state up front that I'm starting to share the frustration that 
was publicly expressed by some other kernel maintainers on a few 
occasions during the last year.  I'm sorry that this frustration 
build-up often ends up bursting out on the OMAP code, but the OMAP 
kernel community is not (or at least used not to) always play fair with 
the rest of the kernel code and this is probably what is tipping us over 
the edge.  There is a faint "let's hack our way through locally and work 
around the generic code limitations" smell here that is not pleasant at 
all.

</rant>

So... here's the issue:

In arch/arm/mach-omap2/common.c:

static void __init __omap2_set_globals(struct omap_globals *omap2_globals)
{
        omap2_set_globals_tap(omap2_globals);
        omap2_set_globals_sdrc(omap2_globals);
        omap2_set_globals_control(omap2_globals);
        omap2_set_globals_prcm(omap2_globals);
}

and also

void __init omap2_set_globals_443x(void)
{
        omap2_set_globals_tap(&omap4_globals);
        omap2_set_globals_control(&omap4_globals);
        omap2_set_globals_prcm(&omap4_globals);
}

Except for omap2_set_globals_tap(), those calls all look like:

void __init omap2_set_globals_prcm(struct omap_globals *omap2_globals)
{
        /* Static mapping, never released */
        if (omap2_globals->prm) {
                prm_base = ioremap(omap2_globals->prm, SZ_8K);
                WARN_ON(!prm_base);
        }
        if (omap2_globals->cm) {
                cm_base = ioremap(omap2_globals->cm, SZ_8K);
                WARN_ON(!cm_base);
        }
        if (omap2_globals->cm2) {
                cm2_base = ioremap(omap2_globals->cm2, SZ_8K);
                WARN_ON(!cm2_base);
        }
}

The problem is that those ioremap() calls are performed _*before*_ the 
memory is fully set up yet, and also even before the corresponding 
static mappings are even in place!  So not only is the ioremap code 
unoperational at this point, but a generic way to trap ioremap() calls 
in order to toss a static mapping back won't even work here because 
iotable_init() was not even called yet.

The current code get away with it because OMAP is providing its own 
__arch_ioremap() which does the interception and provide a virtual 
address from hardcoded but yet to exist mappings.  But the goal of 
global kernel maintenance is to _remove_ such SOC-specific special cases 
and move such a perfectly valid optimization into core code where it can 
be applied uniformly to all.  So the OMAP __arch_ioremap() is going 
away, meaning that static mappings have to be in place before ioremap() 
calls can return something minimally usable.

OK, so let's modify omap4_panda_map_io() just to test this one board and 
reverse the omap44xx_map_common_io() and omap2_set_globals_443x() call 
order.  Now the mappings will be there before ioremap() is called.  But 
that, too, doesn't work and the kernel now complains with:

|OMAP revision unknown, please fix!
|Uninitialized omap_chip, please fix!
|Could not detect SRAM size

But it looks like omap2_set_globals_tap() still has to be called first!  
Isn't this wonderfully convoluted?

Also the repeated local_flush_tlb_all()/flush_cache_all() calls found in 
the OMAP mdesc->map_io code paths (one in _omap2_map_common_io(), 
another in omap_map_sram(), just to pick those as examples) is a sure 
sign that too much is being done via this call and layering violations 
are present.

So could someone in the OMAP camp fix this nonsense ASAP please?
Of course, yesterday would be best.

Furthermore... there is also a static mapping for physical address 
0x4e000000 using virtual address 0xff100000 which is already reserved 
for other purposes i.e. the consistent DMA area.  It is not immediately 
obvious where this comes from without being intimate with the OMAP code. 
Can this be fixed as well i.e. moved elsewhere please?

Patches will be extremely welcome.
Thank you.


Nicolas

Please help with the OMAP static mapping mess

[Nicolas Pitre]

<rant>

I must state up front that I'm starting to share the frustration that 
was publicly expressed by some other kernel maintainers on a few 
occasions during the last year.  I'm sorry that this frustration 
build-up often ends up bursting out on the OMAP code, but the OMAP 
kernel community is not (or at least used not to) always play fair with 
the rest of the kernel code and this is probably what is tipping us over 
the edge.  There is a faint "let's hack our way through locally and work 
around the generic code limitations" smell here that is not pleasant at 
all.

</rant>

So... here's the issue:

In arch/arm/mach-omap2/common.c:

static void __init __omap2_set_globals(struct omap_globals *omap2_globals)
{
        omap2_set_globals_tap(omap2_globals);
        omap2_set_globals_sdrc(omap2_globals);
        omap2_set_globals_control(omap2_globals);
        omap2_set_globals_prcm(omap2_globals);
}

and also

void __init omap2_set_globals_443x(void)
{
        omap2_set_globals_tap(&omap4_globals);
        omap2_set_globals_control(&omap4_globals);
        omap2_set_globals_prcm(&omap4_globals);
}

Except for omap2_set_globals_tap(), those calls all look like:

void __init omap2_set_globals_prcm(struct omap_globals *omap2_globals)
{
        /* Static mapping, never released */
        if (omap2_globals->prm) {
                prm_base = ioremap(omap2_globals->prm, SZ_8K);
                WARN_ON(!prm_base);
        }
        if (omap2_globals->cm) {
                cm_base = ioremap(omap2_globals->cm, SZ_8K);
                WARN_ON(!cm_base);
        }
        if (omap2_globals->cm2) {
                cm2_base = ioremap(omap2_globals->cm2, SZ_8K);
                WARN_ON(!cm2_base);
        }
}

The problem is that those ioremap() calls are performed _*before*_ the 
memory is fully set up yet, and also even before the corresponding 
static mappings are even in place!  So not only is the ioremap code 
unoperational at this point, but a generic way to trap ioremap() calls 
in order to toss a static mapping back won't even work here because 
iotable_init() was not even called yet.

The current code get away with it because OMAP is providing its own 
__arch_ioremap() which does the interception and provide a virtual 
address from hardcoded but yet to exist mappings.  But the goal of 
global kernel maintenance is to _remove_ such SOC-specific special cases 
and move such a perfectly valid optimization into core code where it can 
be applied uniformly to all.  So the OMAP __arch_ioremap() is going 
away, meaning that static mappings have to be in place before ioremap() 
calls can return something minimally usable.

OK, so let's modify omap4_panda_map_io() just to test this one board and 
reverse the omap44xx_map_common_io() and omap2_set_globals_443x() call 
order.  Now the mappings will be there before ioremap() is called.  But 
that, too, doesn't work and the kernel now complains with:

|OMAP revision unknown, please fix!
|Uninitialized omap_chip, please fix!
|Could not detect SRAM size

But it looks like omap2_set_globals_tap() still has to be called first!  
Isn't this wonderfully convoluted?

Also the repeated local_flush_tlb_all()/flush_cache_all() calls found in 
the OMAP mdesc->map_io code paths (one in _omap2_map_common_io(), 
another in omap_map_sram(), just to pick those as examples) is a sure 
sign that too much is being done via this call and layering violations 
are present.

So could someone in the OMAP camp fix this nonsense ASAP please?
Of course, yesterday would be best.

Furthermore... there is also a static mapping for physical address 
0x4e000000 using virtual address 0xff100000 which is already reserved 
for other purposes i.e. the consistent DMA area.  It is not immediately 
obvious where this comes from without being intimate with the OMAP code. 
Can this be fixed as well i.e. moved elsewhere please?

Patches will be extremely welcome.
Thank you.


Nicolas

Re: Please help with the OMAP static mapping mess

[Tony Lindgren]

* Nicolas Pitre <nico@fluxnic.net> [111003 11:26]:
> 
> The problem is that those ioremap() calls are performed _*before*_ the 
> memory is fully set up yet, and also even before the corresponding 
> static mappings are even in place!  So not only is the ioremap code 
> unoperational at this point, but a generic way to trap ioremap() calls 
> in order to toss a static mapping back won't even work here because 
> iotable_init() was not even called yet.
> 
> The current code get away with it because OMAP is providing its own 
> __arch_ioremap() which does the interception and provide a virtual 
> address from hardcoded but yet to exist mappings.  But the goal of 
> global kernel maintenance is to _remove_ such SOC-specific special cases 
> and move such a perfectly valid optimization into core code where it can 
> be applied uniformly to all.  So the OMAP __arch_ioremap() is going 
> away, meaning that static mappings have to be in place before ioremap() 
> calls can return something minimally usable.

Sure this would be nice to fix, see below.
 
> OK, so let's modify omap4_panda_map_io() just to test this one board and 
> reverse the omap44xx_map_common_io() and omap2_set_globals_443x() call 
> order.  Now the mappings will be there before ioremap() is called.  But 
> that, too, doesn't work and the kernel now complains with:
> 
> |OMAP revision unknown, please fix!
> |Uninitialized omap_chip, please fix!
> |Could not detect SRAM size
> 
> But it looks like omap2_set_globals_tap() still has to be called first!  
> Isn't this wonderfully convoluted?

We've already unravelled some of that with the init_early changes.

Earlier having the IO space moving around between 2420/2430/3430
meant that we had to map some IO to detect the SoC. Now we have
SoC specific initcalls where we assume the SoC category is initialized
from board-*.c file (and from DT at some point).

Having the SRAM base address move around with different sizes also
requires the SoC detection.. Otherwise we can end up mapping wrong
size and end up trying to access secure SRAM that will hang the system.

The way to fix it is to move SRAM init happen much later so we don't
have to map it early. I guess now we could use ioremap for SRAM,
although we may not want device attributes for the executable code?
Got any suggestions here on how we should map SRAM later on?
 
> Also the repeated local_flush_tlb_all()/flush_cache_all() calls found in 
> the OMAP mdesc->map_io code paths (one in _omap2_map_common_io(), 
> another in omap_map_sram(), just to pick those as examples) is a sure 
> sign that too much is being done via this call and layering violations 
> are present.

This should go away too if we can avoid SoC detection early and
map SRAM later. I guess the only issue remaining with that is what we
should use for mapping SRAM later on.
 
> So could someone in the OMAP camp fix this nonsense ASAP please?
> Of course, yesterday would be best.

Heh. Were working on it. So far it's been moving things to get initialized
later, separate sys_timer code from dmtimer driver features, initialize
only the hwmods needed for sys_timer early, SoC specific initcalls to
clear the SoC detection out of the early init path and so on.
 
> Furthermore... there is also a static mapping for physical address 
> 0x4e000000 using virtual address 0xff100000 which is already reserved 
> for other purposes i.e. the consistent DMA area.  It is not immediately 
> obvious where this comes from without being intimate with the OMAP code. 
> Can this be fixed as well i.e. moved elsewhere please?

This sounds like a bug somewhere. Which omap are you seeing this on?

Regards,

Tony

Please help with the OMAP static mapping mess

[Tony Lindgren]

* Nicolas Pitre <nico@fluxnic.net> [111003 11:26]:
> 
> The problem is that those ioremap() calls are performed _*before*_ the 
> memory is fully set up yet, and also even before the corresponding 
> static mappings are even in place!  So not only is the ioremap code 
> unoperational at this point, but a generic way to trap ioremap() calls 
> in order to toss a static mapping back won't even work here because 
> iotable_init() was not even called yet.
> 
> The current code get away with it because OMAP is providing its own 
> __arch_ioremap() which does the interception and provide a virtual 
> address from hardcoded but yet to exist mappings.  But the goal of 
> global kernel maintenance is to _remove_ such SOC-specific special cases 
> and move such a perfectly valid optimization into core code where it can 
> be applied uniformly to all.  So the OMAP __arch_ioremap() is going 
> away, meaning that static mappings have to be in place before ioremap() 
> calls can return something minimally usable.

Sure this would be nice to fix, see below.
 
> OK, so let's modify omap4_panda_map_io() just to test this one board and 
> reverse the omap44xx_map_common_io() and omap2_set_globals_443x() call 
> order.  Now the mappings will be there before ioremap() is called.  But 
> that, too, doesn't work and the kernel now complains with:
> 
> |OMAP revision unknown, please fix!
> |Uninitialized omap_chip, please fix!
> |Could not detect SRAM size
> 
> But it looks like omap2_set_globals_tap() still has to be called first!  
> Isn't this wonderfully convoluted?

We've already unravelled some of that with the init_early changes.

Earlier having the IO space moving around between 2420/2430/3430
meant that we had to map some IO to detect the SoC. Now we have
SoC specific initcalls where we assume the SoC category is initialized
from board-*.c file (and from DT at some point).

Having the SRAM base address move around with different sizes also
requires the SoC detection.. Otherwise we can end up mapping wrong
size and end up trying to access secure SRAM that will hang the system.

The way to fix it is to move SRAM init happen much later so we don't
have to map it early. I guess now we could use ioremap for SRAM,
although we may not want device attributes for the executable code?
Got any suggestions here on how we should map SRAM later on?
 
> Also the repeated local_flush_tlb_all()/flush_cache_all() calls found in 
> the OMAP mdesc->map_io code paths (one in _omap2_map_common_io(), 
> another in omap_map_sram(), just to pick those as examples) is a sure 
> sign that too much is being done via this call and layering violations 
> are present.

This should go away too if we can avoid SoC detection early and
map SRAM later. I guess the only issue remaining with that is what we
should use for mapping SRAM later on.
 
> So could someone in the OMAP camp fix this nonsense ASAP please?
> Of course, yesterday would be best.

Heh. Were working on it. So far it's been moving things to get initialized
later, separate sys_timer code from dmtimer driver features, initialize
only the hwmods needed for sys_timer early, SoC specific initcalls to
clear the SoC detection out of the early init path and so on.
 
> Furthermore... there is also a static mapping for physical address 
> 0x4e000000 using virtual address 0xff100000 which is already reserved 
> for other purposes i.e. the consistent DMA area.  It is not immediately 
> obvious where this comes from without being intimate with the OMAP code. 
> Can this be fixed as well i.e. moved elsewhere please?

This sounds like a bug somewhere. Which omap are you seeing this on?

Regards,

Tony

Re: Please help with the OMAP static mapping mess

[Nicolas Pitre]

On Mon, 3 Oct 2011, Tony Lindgren wrote:

> * Nicolas Pitre <nico@fluxnic.net> [111003 11:26]:
> > 
> > The problem is that those ioremap() calls are performed _*before*_ the 
> > memory is fully set up yet, and also even before the corresponding 
> > static mappings are even in place!  So not only is the ioremap code 
> > unoperational at this point, but a generic way to trap ioremap() calls 
> > in order to toss a static mapping back won't even work here because 
> > iotable_init() was not even called yet.
> > 
> > The current code get away with it because OMAP is providing its own 
> > __arch_ioremap() which does the interception and provide a virtual 
> > address from hardcoded but yet to exist mappings.  But the goal of 
> > global kernel maintenance is to _remove_ such SOC-specific special cases 
> > and move such a perfectly valid optimization into core code where it can 
> > be applied uniformly to all.  So the OMAP __arch_ioremap() is going 
> > away, meaning that static mappings have to be in place before ioremap() 
> > calls can return something minimally usable.
> 
> Sure this would be nice to fix, see below.

Great!

> > OK, so let's modify omap4_panda_map_io() just to test this one board and 
> > reverse the omap44xx_map_common_io() and omap2_set_globals_443x() call 
> > order.  Now the mappings will be there before ioremap() is called.  But 
> > that, too, doesn't work and the kernel now complains with:
> > 
> > |OMAP revision unknown, please fix!
> > |Uninitialized omap_chip, please fix!
> > |Could not detect SRAM size
> > 
> > But it looks like omap2_set_globals_tap() still has to be called first!  
> > Isn't this wonderfully convoluted?
> 
> We've already unravelled some of that with the init_early changes.
> 
> Earlier having the IO space moving around between 2420/2430/3430
> meant that we had to map some IO to detect the SoC. Now we have
> SoC specific initcalls where we assume the SoC category is initialized
> from board-*.c file (and from DT at some point).

But the map_io method always has been tied to machine specific 
descriptors.  That always implied a fixed SoC category, no?  Unless you 
have a machine which can accommodate multiple different SOCs but that's 
very uncommon.

> Having the SRAM base address move around with different sizes also
> requires the SoC detection.. Otherwise we can end up mapping wrong
> size and end up trying to access secure SRAM that will hang the system.
> 
> The way to fix it is to move SRAM init happen much later so we don't
> have to map it early. I guess now we could use ioremap for SRAM,
> although we may not want device attributes for the executable code?
> Got any suggestions here on how we should map SRAM later on?

You can use a variant of ioremap() such as __arm_ioremap() which let you 
specify the memory attribute.

> > So could someone in the OMAP camp fix this nonsense ASAP please?
> > Of course, yesterday would be best.
> 
> Heh. Were working on it. So far it's been moving things to get initialized
> later, separate sys_timer code from dmtimer driver features, initialize
> only the hwmods needed for sys_timer early, SoC specific initcalls to
> clear the SoC detection out of the early init path and so on.

Wonderful!

> > Furthermore... there is also a static mapping for physical address 
> > 0x4e000000 using virtual address 0xff100000 which is already reserved 
> > for other purposes i.e. the consistent DMA area.  It is not immediately 
> > obvious where this comes from without being intimate with the OMAP code. 
> > Can this be fixed as well i.e. moved elsewhere please?
> 
> This sounds like a bug somewhere. Which omap are you seeing this on?

OMAP4430 on a Panda board.

Here are the static mappings I'm seeing:

phys = 0x44000000 virt = 0xf8000000 size = 0x100000
phys = 0x4a000000 virt = 0xfc000000 size = 0x400000
phys = 0x50000000 virt = 0xf9000000 size = 0x100000
phys = 0x4c000000 virt = 0xfd100000 size = 0x100000
phys = 0x4d000000 virt = 0xfe100000 size = 0x100000
phys = 0x4e000000 virt = 0xff100000 size = 0x100000 <---
phys = 0x48000000 virt = 0xfa000000 size = 0x400000
phys = 0x54000000 virt = 0xfe800000 size = 0x800000

It is also possible that I might have screwed something up on my side.  
What is located at 0x4e000000?


Nicolas

Please help with the OMAP static mapping mess

[Nicolas Pitre]

On Mon, 3 Oct 2011, Tony Lindgren wrote:

> * Nicolas Pitre <nico@fluxnic.net> [111003 11:26]:
> > 
> > The problem is that those ioremap() calls are performed _*before*_ the 
> > memory is fully set up yet, and also even before the corresponding 
> > static mappings are even in place!  So not only is the ioremap code 
> > unoperational at this point, but a generic way to trap ioremap() calls 
> > in order to toss a static mapping back won't even work here because 
> > iotable_init() was not even called yet.
> > 
> > The current code get away with it because OMAP is providing its own 
> > __arch_ioremap() which does the interception and provide a virtual 
> > address from hardcoded but yet to exist mappings.  But the goal of 
> > global kernel maintenance is to _remove_ such SOC-specific special cases 
> > and move such a perfectly valid optimization into core code where it can 
> > be applied uniformly to all.  So the OMAP __arch_ioremap() is going 
> > away, meaning that static mappings have to be in place before ioremap() 
> > calls can return something minimally usable.
> 
> Sure this would be nice to fix, see below.

Great!

> > OK, so let's modify omap4_panda_map_io() just to test this one board and 
> > reverse the omap44xx_map_common_io() and omap2_set_globals_443x() call 
> > order.  Now the mappings will be there before ioremap() is called.  But 
> > that, too, doesn't work and the kernel now complains with:
> > 
> > |OMAP revision unknown, please fix!
> > |Uninitialized omap_chip, please fix!
> > |Could not detect SRAM size
> > 
> > But it looks like omap2_set_globals_tap() still has to be called first!  
> > Isn't this wonderfully convoluted?
> 
> We've already unravelled some of that with the init_early changes.
> 
> Earlier having the IO space moving around between 2420/2430/3430
> meant that we had to map some IO to detect the SoC. Now we have
> SoC specific initcalls where we assume the SoC category is initialized
> from board-*.c file (and from DT at some point).

But the map_io method always has been tied to machine specific 
descriptors.  That always implied a fixed SoC category, no?  Unless you 
have a machine which can accommodate multiple different SOCs but that's 
very uncommon.

> Having the SRAM base address move around with different sizes also
> requires the SoC detection.. Otherwise we can end up mapping wrong
> size and end up trying to access secure SRAM that will hang the system.
> 
> The way to fix it is to move SRAM init happen much later so we don't
> have to map it early. I guess now we could use ioremap for SRAM,
> although we may not want device attributes for the executable code?
> Got any suggestions here on how we should map SRAM later on?

You can use a variant of ioremap() such as __arm_ioremap() which let you 
specify the memory attribute.

> > So could someone in the OMAP camp fix this nonsense ASAP please?
> > Of course, yesterday would be best.
> 
> Heh. Were working on it. So far it's been moving things to get initialized
> later, separate sys_timer code from dmtimer driver features, initialize
> only the hwmods needed for sys_timer early, SoC specific initcalls to
> clear the SoC detection out of the early init path and so on.

Wonderful!

> > Furthermore... there is also a static mapping for physical address 
> > 0x4e000000 using virtual address 0xff100000 which is already reserved 
> > for other purposes i.e. the consistent DMA area.  It is not immediately 
> > obvious where this comes from without being intimate with the OMAP code. 
> > Can this be fixed as well i.e. moved elsewhere please?
> 
> This sounds like a bug somewhere. Which omap are you seeing this on?

OMAP4430 on a Panda board.

Here are the static mappings I'm seeing:

phys = 0x44000000 virt = 0xf8000000 size = 0x100000
phys = 0x4a000000 virt = 0xfc000000 size = 0x400000
phys = 0x50000000 virt = 0xf9000000 size = 0x100000
phys = 0x4c000000 virt = 0xfd100000 size = 0x100000
phys = 0x4d000000 virt = 0xfe100000 size = 0x100000
phys = 0x4e000000 virt = 0xff100000 size = 0x100000 <---
phys = 0x48000000 virt = 0xfa000000 size = 0x400000
phys = 0x54000000 virt = 0xfe800000 size = 0x800000

It is also possible that I might have screwed something up on my side.  
What is located@0x4e000000?


Nicolas

Re: Please help with the OMAP static mapping mess

[Tony Lindgren]

* Nicolas Pitre <nico@fluxnic.net> [111003 14:36]:
> On Mon, 3 Oct 2011, Tony Lindgren wrote:
> 
> > * Nicolas Pitre <nico@fluxnic.net> [111003 11:26]:
> 
> > > OK, so let's modify omap4_panda_map_io() just to test this one board and 
> > > reverse the omap44xx_map_common_io() and omap2_set_globals_443x() call 
> > > order.  Now the mappings will be there before ioremap() is called.  But 
> > > that, too, doesn't work and the kernel now complains with:
> > > 
> > > |OMAP revision unknown, please fix!
> > > |Uninitialized omap_chip, please fix!
> > > |Could not detect SRAM size
> > > 
> > > But it looks like omap2_set_globals_tap() still has to be called first!  
> > > Isn't this wonderfully convoluted?
> > 
> > We've already unravelled some of that with the init_early changes.
> > 
> > Earlier having the IO space moving around between 2420/2430/3430
> > meant that we had to map some IO to detect the SoC. Now we have
> > SoC specific initcalls where we assume the SoC category is initialized
> > from board-*.c file (and from DT at some point).
> 
> But the map_io method always has been tied to machine specific 
> descriptors.  That always implied a fixed SoC category, no?  Unless you 
> have a machine which can accommodate multiple different SOCs but that's 
> very uncommon.

Hmm I think we initially tried to use board-generic.c with custom ATAGs
to boot multiple SoCs and that's why we needed SoC detection for map_io.

Now the only variable SoC headache left is that board-omap3beagle.c
is using the same machine_id for 3430 and 3630 beagle which are somewhat
different SoCs, but Luckily not from map_io point of view though. So that
should be fixable with DT when the SoC type will be passed from DT.
 
> > Having the SRAM base address move around with different sizes also
> > requires the SoC detection.. Otherwise we can end up mapping wrong
> > size and end up trying to access secure SRAM that will hang the system.
> > 
> > The way to fix it is to move SRAM init happen much later so we don't
> > have to map it early. I guess now we could use ioremap for SRAM,
> > although we may not want device attributes for the executable code?
> > Got any suggestions here on how we should map SRAM later on?
> 
> You can use a variant of ioremap() such as __arm_ioremap() which let you 
> specify the memory attribute.

OK, I'll take a look at that.
 
> > > Furthermore... there is also a static mapping for physical address 
> > > 0x4e000000 using virtual address 0xff100000 which is already reserved 
> > > for other purposes i.e. the consistent DMA area.  It is not immediately 
> > > obvious where this comes from without being intimate with the OMAP code. 
> > > Can this be fixed as well i.e. moved elsewhere please?
> > 
> > This sounds like a bug somewhere. Which omap are you seeing this on?
> 
> OMAP4430 on a Panda board.
> 
> Here are the static mappings I'm seeing:
> 
> phys = 0x44000000 virt = 0xf8000000 size = 0x100000
> phys = 0x4a000000 virt = 0xfc000000 size = 0x400000
> phys = 0x50000000 virt = 0xf9000000 size = 0x100000
> phys = 0x4c000000 virt = 0xfd100000 size = 0x100000
> phys = 0x4d000000 virt = 0xfe100000 size = 0x100000
> phys = 0x4e000000 virt = 0xff100000 size = 0x100000 <---
> phys = 0x48000000 virt = 0xfa000000 size = 0x400000
> phys = 0x54000000 virt = 0xfe800000 size = 0x800000
> 
> It is also possible that I might have screwed something up on my side.  
> What is located at 0x4e000000?

It seems to be DMM (Dynamic Memory Manager), some more info at:

http://lwn.net/Articles/417790/

Tony

Please help with the OMAP static mapping mess

[Tony Lindgren]

* Nicolas Pitre <nico@fluxnic.net> [111003 14:36]:
> On Mon, 3 Oct 2011, Tony Lindgren wrote:
> 
> > * Nicolas Pitre <nico@fluxnic.net> [111003 11:26]:
> 
> > > OK, so let's modify omap4_panda_map_io() just to test this one board and 
> > > reverse the omap44xx_map_common_io() and omap2_set_globals_443x() call 
> > > order.  Now the mappings will be there before ioremap() is called.  But 
> > > that, too, doesn't work and the kernel now complains with:
> > > 
> > > |OMAP revision unknown, please fix!
> > > |Uninitialized omap_chip, please fix!
> > > |Could not detect SRAM size
> > > 
> > > But it looks like omap2_set_globals_tap() still has to be called first!  
> > > Isn't this wonderfully convoluted?
> > 
> > We've already unravelled some of that with the init_early changes.
> > 
> > Earlier having the IO space moving around between 2420/2430/3430
> > meant that we had to map some IO to detect the SoC. Now we have
> > SoC specific initcalls where we assume the SoC category is initialized
> > from board-*.c file (and from DT at some point).
> 
> But the map_io method always has been tied to machine specific 
> descriptors.  That always implied a fixed SoC category, no?  Unless you 
> have a machine which can accommodate multiple different SOCs but that's 
> very uncommon.

Hmm I think we initially tried to use board-generic.c with custom ATAGs
to boot multiple SoCs and that's why we needed SoC detection for map_io.

Now the only variable SoC headache left is that board-omap3beagle.c
is using the same machine_id for 3430 and 3630 beagle which are somewhat
different SoCs, but Luckily not from map_io point of view though. So that
should be fixable with DT when the SoC type will be passed from DT.
 
> > Having the SRAM base address move around with different sizes also
> > requires the SoC detection.. Otherwise we can end up mapping wrong
> > size and end up trying to access secure SRAM that will hang the system.
> > 
> > The way to fix it is to move SRAM init happen much later so we don't
> > have to map it early. I guess now we could use ioremap for SRAM,
> > although we may not want device attributes for the executable code?
> > Got any suggestions here on how we should map SRAM later on?
> 
> You can use a variant of ioremap() such as __arm_ioremap() which let you 
> specify the memory attribute.

OK, I'll take a look at that.
 
> > > Furthermore... there is also a static mapping for physical address 
> > > 0x4e000000 using virtual address 0xff100000 which is already reserved 
> > > for other purposes i.e. the consistent DMA area.  It is not immediately 
> > > obvious where this comes from without being intimate with the OMAP code. 
> > > Can this be fixed as well i.e. moved elsewhere please?
> > 
> > This sounds like a bug somewhere. Which omap are you seeing this on?
> 
> OMAP4430 on a Panda board.
> 
> Here are the static mappings I'm seeing:
> 
> phys = 0x44000000 virt = 0xf8000000 size = 0x100000
> phys = 0x4a000000 virt = 0xfc000000 size = 0x400000
> phys = 0x50000000 virt = 0xf9000000 size = 0x100000
> phys = 0x4c000000 virt = 0xfd100000 size = 0x100000
> phys = 0x4d000000 virt = 0xfe100000 size = 0x100000
> phys = 0x4e000000 virt = 0xff100000 size = 0x100000 <---
> phys = 0x48000000 virt = 0xfa000000 size = 0x400000
> phys = 0x54000000 virt = 0xfe800000 size = 0x800000
> 
> It is also possible that I might have screwed something up on my side.  
> What is located at 0x4e000000?

It seems to be DMM (Dynamic Memory Manager), some more info at:

http://lwn.net/Articles/417790/

Tony

Re: Please help with the OMAP static mapping mess

[Nicolas Pitre]

On Mon, 3 Oct 2011, Nicolas Pitre wrote:

> On Mon, 3 Oct 2011, Tony Lindgren wrote:
> 
> > * Nicolas Pitre <nico@fluxnic.net> [111003 11:26]:
> > > 
> > > Furthermore... there is also a static mapping for physical address 
> > > 0x4e000000 using virtual address 0xff100000 which is already reserved 
> > > for other purposes i.e. the consistent DMA area.  It is not immediately 
> > > obvious where this comes from without being intimate with the OMAP code. 
> > > Can this be fixed as well i.e. moved elsewhere please?
> > 
> > This sounds like a bug somewhere. Which omap are you seeing this on?
> 
> OMAP4430 on a Panda board.
> 
> Here are the static mappings I'm seeing:
> 
> phys = 0x44000000 virt = 0xf8000000 size = 0x100000
> phys = 0x4a000000 virt = 0xfc000000 size = 0x400000
> phys = 0x50000000 virt = 0xf9000000 size = 0x100000
> phys = 0x4c000000 virt = 0xfd100000 size = 0x100000
> phys = 0x4d000000 virt = 0xfe100000 size = 0x100000
> phys = 0x4e000000 virt = 0xff100000 size = 0x100000 <---
> phys = 0x48000000 virt = 0xfa000000 size = 0x400000
> phys = 0x54000000 virt = 0xfe800000 size = 0x800000

It looks like this comes from OMAP44XX_DMM_VIRT.

#define OMAP44XX_DMM_PHYS       OMAP44XX_DMM_BASE
                                                /* 0x4e000000 --> 0xfd300000 */
#define OMAP44XX_DMM_VIRT       (OMAP44XX_DMM_PHYS + OMAP4_L3_PER_IO_OFFSET)
#define OMAP44XX_DMM_SIZE       SZ_1M

The comment suggesting a mapping correspondance is obviously wrong. We have:

#define OMAP44XX_DMM_BASE       0x4e000000
#define OMAP4_L3_PER_IO_OFFSET  0xb1100000

Hence 0x4e000000 + 0xb1100000 = 0xff100000.


Nicolas








> 
> It is also possible that I might have screwed something up on my side.  
> What is located at 0x4e000000?
> 
> 
> Nicolas
> 

Please help with the OMAP static mapping mess

[Nicolas Pitre]

On Mon, 3 Oct 2011, Nicolas Pitre wrote:

> On Mon, 3 Oct 2011, Tony Lindgren wrote:
> 
> > * Nicolas Pitre <nico@fluxnic.net> [111003 11:26]:
> > > 
> > > Furthermore... there is also a static mapping for physical address 
> > > 0x4e000000 using virtual address 0xff100000 which is already reserved 
> > > for other purposes i.e. the consistent DMA area.  It is not immediately 
> > > obvious where this comes from without being intimate with the OMAP code. 
> > > Can this be fixed as well i.e. moved elsewhere please?
> > 
> > This sounds like a bug somewhere. Which omap are you seeing this on?
> 
> OMAP4430 on a Panda board.
> 
> Here are the static mappings I'm seeing:
> 
> phys = 0x44000000 virt = 0xf8000000 size = 0x100000
> phys = 0x4a000000 virt = 0xfc000000 size = 0x400000
> phys = 0x50000000 virt = 0xf9000000 size = 0x100000
> phys = 0x4c000000 virt = 0xfd100000 size = 0x100000
> phys = 0x4d000000 virt = 0xfe100000 size = 0x100000
> phys = 0x4e000000 virt = 0xff100000 size = 0x100000 <---
> phys = 0x48000000 virt = 0xfa000000 size = 0x400000
> phys = 0x54000000 virt = 0xfe800000 size = 0x800000

It looks like this comes from OMAP44XX_DMM_VIRT.

#define OMAP44XX_DMM_PHYS       OMAP44XX_DMM_BASE
                                                /* 0x4e000000 --> 0xfd300000 */
#define OMAP44XX_DMM_VIRT       (OMAP44XX_DMM_PHYS + OMAP4_L3_PER_IO_OFFSET)
#define OMAP44XX_DMM_SIZE       SZ_1M

The comment suggesting a mapping correspondance is obviously wrong. We have:

#define OMAP44XX_DMM_BASE       0x4e000000
#define OMAP4_L3_PER_IO_OFFSET  0xb1100000

Hence 0x4e000000 + 0xb1100000 = 0xff100000.


Nicolas








> 
> It is also possible that I might have screwed something up on my side.  
> What is located at 0x4e000000?
> 
> 
> Nicolas
> 

Re: Please help with the OMAP static mapping mess

[Russell King - ARM Linux]

On Mon, Oct 03, 2011 at 06:09:57PM -0400, Nicolas Pitre wrote:
> On Mon, 3 Oct 2011, Tony Lindgren wrote:
> > Having the SRAM base address move around with different sizes also
> > requires the SoC detection.. Otherwise we can end up mapping wrong
> > size and end up trying to access secure SRAM that will hang the system.
> > 
> > The way to fix it is to move SRAM init happen much later so we don't
> > have to map it early. I guess now we could use ioremap for SRAM,
> > although we may not want device attributes for the executable code?
> > Got any suggestions here on how we should map SRAM later on?
> 
> You can use a variant of ioremap() such as __arm_ioremap() which let you 
> specify the memory attribute.

Just be aware that __arm_ioremap() always ends up with stuff in the
kernel domain, but that's not what you end up with using create_mapping().
So I'd prefer it if you didn't suggest that __arm_ioremap() should be used
with types not listed in asm/io.h.

Please help with the OMAP static mapping mess

[Russell King - ARM Linux]

On Mon, Oct 03, 2011 at 06:09:57PM -0400, Nicolas Pitre wrote:
> On Mon, 3 Oct 2011, Tony Lindgren wrote:
> > Having the SRAM base address move around with different sizes also
> > requires the SoC detection.. Otherwise we can end up mapping wrong
> > size and end up trying to access secure SRAM that will hang the system.
> > 
> > The way to fix it is to move SRAM init happen much later so we don't
> > have to map it early. I guess now we could use ioremap for SRAM,
> > although we may not want device attributes for the executable code?
> > Got any suggestions here on how we should map SRAM later on?
> 
> You can use a variant of ioremap() such as __arm_ioremap() which let you 
> specify the memory attribute.

Just be aware that __arm_ioremap() always ends up with stuff in the
kernel domain, but that's not what you end up with using create_mapping().
So I'd prefer it if you didn't suggest that __arm_ioremap() should be used
with types not listed in asm/io.h.

Re: Please help with the OMAP static mapping mess

[Tony Lindgren]

* Nicolas Pitre <nico@fluxnic.net> [111003 15:05]:
> On Mon, 3 Oct 2011, Nicolas Pitre wrote:
> 
> > On Mon, 3 Oct 2011, Tony Lindgren wrote:
> > 
> > > * Nicolas Pitre <nico@fluxnic.net> [111003 11:26]:
> > > > 
> > > > Furthermore... there is also a static mapping for physical address 
> > > > 0x4e000000 using virtual address 0xff100000 which is already reserved 
> > > > for other purposes i.e. the consistent DMA area.  It is not immediately 
> > > > obvious where this comes from without being intimate with the OMAP code. 
> > > > Can this be fixed as well i.e. moved elsewhere please?
> > > 
> > > This sounds like a bug somewhere. Which omap are you seeing this on?
> > 
> > OMAP4430 on a Panda board.
> > 
> > Here are the static mappings I'm seeing:
> > 
> > phys = 0x44000000 virt = 0xf8000000 size = 0x100000
> > phys = 0x4a000000 virt = 0xfc000000 size = 0x400000
> > phys = 0x50000000 virt = 0xf9000000 size = 0x100000
> > phys = 0x4c000000 virt = 0xfd100000 size = 0x100000
> > phys = 0x4d000000 virt = 0xfe100000 size = 0x100000
> > phys = 0x4e000000 virt = 0xff100000 size = 0x100000 <---
> > phys = 0x48000000 virt = 0xfa000000 size = 0x400000
> > phys = 0x54000000 virt = 0xfe800000 size = 0x800000
> 
> It looks like this comes from OMAP44XX_DMM_VIRT.
> 
> #define OMAP44XX_DMM_PHYS       OMAP44XX_DMM_BASE
>                                                 /* 0x4e000000 --> 0xfd300000 */
> #define OMAP44XX_DMM_VIRT       (OMAP44XX_DMM_PHYS + OMAP4_L3_PER_IO_OFFSET)
> #define OMAP44XX_DMM_SIZE       SZ_1M
> 
> The comment suggesting a mapping correspondance is obviously wrong. We have:
> 
> #define OMAP44XX_DMM_BASE       0x4e000000
> #define OMAP4_L3_PER_IO_OFFSET  0xb1100000
> 
> Hence 0x4e000000 + 0xb1100000 = 0xff100000.

Seem like it might cause some random patterns in tiler :)
Santosh, can youp please check it?

Regards,

Tony

Please help with the OMAP static mapping mess

[Tony Lindgren]

* Nicolas Pitre <nico@fluxnic.net> [111003 15:05]:
> On Mon, 3 Oct 2011, Nicolas Pitre wrote:
> 
> > On Mon, 3 Oct 2011, Tony Lindgren wrote:
> > 
> > > * Nicolas Pitre <nico@fluxnic.net> [111003 11:26]:
> > > > 
> > > > Furthermore... there is also a static mapping for physical address 
> > > > 0x4e000000 using virtual address 0xff100000 which is already reserved 
> > > > for other purposes i.e. the consistent DMA area.  It is not immediately 
> > > > obvious where this comes from without being intimate with the OMAP code. 
> > > > Can this be fixed as well i.e. moved elsewhere please?
> > > 
> > > This sounds like a bug somewhere. Which omap are you seeing this on?
> > 
> > OMAP4430 on a Panda board.
> > 
> > Here are the static mappings I'm seeing:
> > 
> > phys = 0x44000000 virt = 0xf8000000 size = 0x100000
> > phys = 0x4a000000 virt = 0xfc000000 size = 0x400000
> > phys = 0x50000000 virt = 0xf9000000 size = 0x100000
> > phys = 0x4c000000 virt = 0xfd100000 size = 0x100000
> > phys = 0x4d000000 virt = 0xfe100000 size = 0x100000
> > phys = 0x4e000000 virt = 0xff100000 size = 0x100000 <---
> > phys = 0x48000000 virt = 0xfa000000 size = 0x400000
> > phys = 0x54000000 virt = 0xfe800000 size = 0x800000
> 
> It looks like this comes from OMAP44XX_DMM_VIRT.
> 
> #define OMAP44XX_DMM_PHYS       OMAP44XX_DMM_BASE
>                                                 /* 0x4e000000 --> 0xfd300000 */
> #define OMAP44XX_DMM_VIRT       (OMAP44XX_DMM_PHYS + OMAP4_L3_PER_IO_OFFSET)
> #define OMAP44XX_DMM_SIZE       SZ_1M
> 
> The comment suggesting a mapping correspondance is obviously wrong. We have:
> 
> #define OMAP44XX_DMM_BASE       0x4e000000
> #define OMAP4_L3_PER_IO_OFFSET  0xb1100000
> 
> Hence 0x4e000000 + 0xb1100000 = 0xff100000.

Seem like it might cause some random patterns in tiler :)
Santosh, can youp please check it?

Regards,

Tony

Re: Please help with the OMAP static mapping mess

[Shilimkar, Santosh]

On Tue, Oct 4, 2011 at 4:29 AM, Tony Lindgren <tony@atomide.com> wrote:
> * Nicolas Pitre <nico@fluxnic.net> [111003 15:05]:
>> On Mon, 3 Oct 2011, Nicolas Pitre wrote:
>>
>> > On Mon, 3 Oct 2011, Tony Lindgren wrote:
>> >
>> > > * Nicolas Pitre <nico@fluxnic.net> [111003 11:26]:
>> > > >
>> > > > Furthermore... there is also a static mapping for physical address
>> > > > 0x4e000000 using virtual address 0xff100000 which is already reserved
>> > > > for other purposes i.e. the consistent DMA area.  It is not immediately
>> > > > obvious where this comes from without being intimate with the OMAP code.
>> > > > Can this be fixed as well i.e. moved elsewhere please?
>> > >
>> > > This sounds like a bug somewhere. Which omap are you seeing this on?
>> >
>> > OMAP4430 on a Panda board.
>> >
>> > Here are the static mappings I'm seeing:
>> >
>> > phys = 0x44000000 virt = 0xf8000000 size = 0x100000
>> > phys = 0x4a000000 virt = 0xfc000000 size = 0x400000
>> > phys = 0x50000000 virt = 0xf9000000 size = 0x100000
>> > phys = 0x4c000000 virt = 0xfd100000 size = 0x100000
>> > phys = 0x4d000000 virt = 0xfe100000 size = 0x100000
>> > phys = 0x4e000000 virt = 0xff100000 size = 0x100000 <---
>> > phys = 0x48000000 virt = 0xfa000000 size = 0x400000
>> > phys = 0x54000000 virt = 0xfe800000 size = 0x800000
>>
>> It looks like this comes from OMAP44XX_DMM_VIRT.
>>
>> #define OMAP44XX_DMM_PHYS       OMAP44XX_DMM_BASE
>>                                                 /* 0x4e000000 --> 0xfd300000 */
>> #define OMAP44XX_DMM_VIRT       (OMAP44XX_DMM_PHYS + OMAP4_L3_PER_IO_OFFSET)
>> #define OMAP44XX_DMM_SIZE       SZ_1M
>>
>> The comment suggesting a mapping correspondance is obviously wrong. We have:
>>
>> #define OMAP44XX_DMM_BASE       0x4e000000
>> #define OMAP4_L3_PER_IO_OFFSET  0xb1100000
>>
>> Hence 0x4e000000 + 0xb1100000 = 0xff100000.
>
> Seem like it might cause some random patterns in tiler :)
> Santosh, can youp please check it?
>
This is already fixed Tony. You have pulled that patch.
http://www.mail-archive.com/linux-omap@vger.kernel.org/msg55258.html

Regards
Santosh
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Please help with the OMAP static mapping mess

[Shilimkar, Santosh]

On Tue, Oct 4, 2011 at 4:29 AM, Tony Lindgren <tony@atomide.com> wrote:
> * Nicolas Pitre <nico@fluxnic.net> [111003 15:05]:
>> On Mon, 3 Oct 2011, Nicolas Pitre wrote:
>>
>> > On Mon, 3 Oct 2011, Tony Lindgren wrote:
>> >
>> > > * Nicolas Pitre <nico@fluxnic.net> [111003 11:26]:
>> > > >
>> > > > Furthermore... there is also a static mapping for physical address
>> > > > 0x4e000000 using virtual address 0xff100000 which is already reserved
>> > > > for other purposes i.e. the consistent DMA area. ?It is not immediately
>> > > > obvious where this comes from without being intimate with the OMAP code.
>> > > > Can this be fixed as well i.e. moved elsewhere please?
>> > >
>> > > This sounds like a bug somewhere. Which omap are you seeing this on?
>> >
>> > OMAP4430 on a Panda board.
>> >
>> > Here are the static mappings I'm seeing:
>> >
>> > phys = 0x44000000 virt = 0xf8000000 size = 0x100000
>> > phys = 0x4a000000 virt = 0xfc000000 size = 0x400000
>> > phys = 0x50000000 virt = 0xf9000000 size = 0x100000
>> > phys = 0x4c000000 virt = 0xfd100000 size = 0x100000
>> > phys = 0x4d000000 virt = 0xfe100000 size = 0x100000
>> > phys = 0x4e000000 virt = 0xff100000 size = 0x100000 <---
>> > phys = 0x48000000 virt = 0xfa000000 size = 0x400000
>> > phys = 0x54000000 virt = 0xfe800000 size = 0x800000
>>
>> It looks like this comes from OMAP44XX_DMM_VIRT.
>>
>> #define OMAP44XX_DMM_PHYS ? ? ? OMAP44XX_DMM_BASE
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? /* 0x4e000000 --> 0xfd300000 */
>> #define OMAP44XX_DMM_VIRT ? ? ? (OMAP44XX_DMM_PHYS + OMAP4_L3_PER_IO_OFFSET)
>> #define OMAP44XX_DMM_SIZE ? ? ? SZ_1M
>>
>> The comment suggesting a mapping correspondance is obviously wrong. We have:
>>
>> #define OMAP44XX_DMM_BASE ? ? ? 0x4e000000
>> #define OMAP4_L3_PER_IO_OFFSET ?0xb1100000
>>
>> Hence 0x4e000000 + 0xb1100000 = 0xff100000.
>
> Seem like it might cause some random patterns in tiler :)
> Santosh, can youp please check it?
>
This is already fixed Tony. You have pulled that patch.
http://www.mail-archive.com/linux-omap at vger.kernel.org/msg55258.html

Regards
Santosh

Re: Please help with the OMAP static mapping mess

[Santosh Shilimkar]

Nicolas,

On Tuesday 04 October 2011 04:08 AM, Tony Lindgren wrote:
> * Nicolas Pitre <nico@fluxnic.net> [111003 14:36]:
>> On Mon, 3 Oct 2011, Tony Lindgren wrote:
>>
>>> * Nicolas Pitre <nico@fluxnic.net> [111003 11:26]:
>>
>>>> OK, so let's modify omap4_panda_map_io() just to test this one board and 
>>>> reverse the omap44xx_map_common_io() and omap2_set_globals_443x() call 
>>>> order.  Now the mappings will be there before ioremap() is called.  But 
>>>> that, too, doesn't work and the kernel now complains with:
>>>>
>>>> |OMAP revision unknown, please fix!
>>>> |Uninitialized omap_chip, please fix!
>>>> |Could not detect SRAM size
>>>>
>>>> But it looks like omap2_set_globals_tap() still has to be called first!  
>>>> Isn't this wonderfully convoluted?
>>>
>>> We've already unravelled some of that with the init_early changes.
>>>
Sorry about that. We revamped io_map last time around to avoid
use of omap_readl()/omap_writel() and as Tony pointed out, as part
of early init code re-factoring, some more clean-up is happening.

>>> Earlier having the IO space moving around between 2420/2430/3430
>>> meant that we had to map some IO to detect the SoC. Now we have
>>> SoC specific initcalls where we assume the SoC category is initialized
>>> from board-*.c file (and from DT at some point).
>>
>> But the map_io method always has been tied to machine specific 
>> descriptors.  That always implied a fixed SoC category, no?  Unless you 
>> have a machine which can accommodate multiple different SOCs but that's 
>> very uncommon.
> 
> Hmm I think we initially tried to use board-generic.c with custom ATAGs
> to boot multiple SoCs and that's why we needed SoC detection for map_io.
> 
> Now the only variable SoC headache left is that board-omap3beagle.c
> is using the same machine_id for 3430 and 3630 beagle which are somewhat
> different SoCs, but Luckily not from map_io point of view though. So that
> should be fixable with DT when the SoC type will be passed from DT.
>  
>>> Having the SRAM base address move around with different sizes also
>>> requires the SoC detection.. Otherwise we can end up mapping wrong
>>> size and end up trying to access secure SRAM that will hang the system.
>>>
>>> The way to fix it is to move SRAM init happen much later so we don't
>>> have to map it early. I guess now we could use ioremap for SRAM,
>>> although we may not want device attributes for the executable code?
>>> Got any suggestions here on how we should map SRAM later on?
>>
>> You can use a variant of ioremap() such as __arm_ioremap() which let you 
>> specify the memory attribute.
> 
> OK, I'll take a look at that.
> 
I have tried __arm_ioremap_pfn() for some DDR mapping and it didn't
work as expected. The mapping was not getting created. Needless to
say this can't be an IO memory. I later managed to get around with
it by using iotable_init() though downside is I have to pick a
static virtual address for the mapping.

But I agree that SRAM mapping can be moved further down. We
just need to ensure that it's ready before we initialise SDRC
and PM code. SDRC reconfigure of DDR needs to be executed from
SRAM and of-course the PM WFI routine. Today we do SDRC init early
and that's the reason SRAM is mapped before that. So both of them
needs to be moved down in the boot to make it work.

>>>> Furthermore... there is also a static mapping for physical address 
>>>> 0x4e000000 using virtual address 0xff100000 which is already reserved 
>>>> for other purposes i.e. the consistent DMA area.  It is not immediately 
>>>> obvious where this comes from without being intimate with the OMAP code. 
>>>> Can this be fixed as well i.e. moved elsewhere please?
>>>
>>> This sounds like a bug somewhere. Which omap are you seeing this on?
>>
>> OMAP4430 on a Panda board.
>>
>> Here are the static mappings I'm seeing:
>>
>> phys = 0x44000000 virt = 0xf8000000 size = 0x100000
>> phys = 0x4a000000 virt = 0xfc000000 size = 0x400000
>> phys = 0x50000000 virt = 0xf9000000 size = 0x100000
>> phys = 0x4c000000 virt = 0xfd100000 size = 0x100000
>> phys = 0x4d000000 virt = 0xfe100000 size = 0x100000
>> phys = 0x4e000000 virt = 0xff100000 size = 0x100000 <---
>> phys = 0x48000000 virt = 0xfa000000 size = 0x400000
>> phys = 0x54000000 virt = 0xfe800000 size = 0x800000
>>
>> It is also possible that I might have screwed something up on my side.  
>> What is located at 0x4e000000?
> 
This was definitely a BUG which I noticed last merge window. The fix for
this is already getting into 3.2.

Regards
Santosh

Please help with the OMAP static mapping mess

[Santosh Shilimkar]

Nicolas,

On Tuesday 04 October 2011 04:08 AM, Tony Lindgren wrote:
> * Nicolas Pitre <nico@fluxnic.net> [111003 14:36]:
>> On Mon, 3 Oct 2011, Tony Lindgren wrote:
>>
>>> * Nicolas Pitre <nico@fluxnic.net> [111003 11:26]:
>>
>>>> OK, so let's modify omap4_panda_map_io() just to test this one board and 
>>>> reverse the omap44xx_map_common_io() and omap2_set_globals_443x() call 
>>>> order.  Now the mappings will be there before ioremap() is called.  But 
>>>> that, too, doesn't work and the kernel now complains with:
>>>>
>>>> |OMAP revision unknown, please fix!
>>>> |Uninitialized omap_chip, please fix!
>>>> |Could not detect SRAM size
>>>>
>>>> But it looks like omap2_set_globals_tap() still has to be called first!  
>>>> Isn't this wonderfully convoluted?
>>>
>>> We've already unravelled some of that with the init_early changes.
>>>
Sorry about that. We revamped io_map last time around to avoid
use of omap_readl()/omap_writel() and as Tony pointed out, as part
of early init code re-factoring, some more clean-up is happening.

>>> Earlier having the IO space moving around between 2420/2430/3430
>>> meant that we had to map some IO to detect the SoC. Now we have
>>> SoC specific initcalls where we assume the SoC category is initialized
>>> from board-*.c file (and from DT at some point).
>>
>> But the map_io method always has been tied to machine specific 
>> descriptors.  That always implied a fixed SoC category, no?  Unless you 
>> have a machine which can accommodate multiple different SOCs but that's 
>> very uncommon.
> 
> Hmm I think we initially tried to use board-generic.c with custom ATAGs
> to boot multiple SoCs and that's why we needed SoC detection for map_io.
> 
> Now the only variable SoC headache left is that board-omap3beagle.c
> is using the same machine_id for 3430 and 3630 beagle which are somewhat
> different SoCs, but Luckily not from map_io point of view though. So that
> should be fixable with DT when the SoC type will be passed from DT.
>  
>>> Having the SRAM base address move around with different sizes also
>>> requires the SoC detection.. Otherwise we can end up mapping wrong
>>> size and end up trying to access secure SRAM that will hang the system.
>>>
>>> The way to fix it is to move SRAM init happen much later so we don't
>>> have to map it early. I guess now we could use ioremap for SRAM,
>>> although we may not want device attributes for the executable code?
>>> Got any suggestions here on how we should map SRAM later on?
>>
>> You can use a variant of ioremap() such as __arm_ioremap() which let you 
>> specify the memory attribute.
> 
> OK, I'll take a look at that.
> 
I have tried __arm_ioremap_pfn() for some DDR mapping and it didn't
work as expected. The mapping was not getting created. Needless to
say this can't be an IO memory. I later managed to get around with
it by using iotable_init() though downside is I have to pick a
static virtual address for the mapping.

But I agree that SRAM mapping can be moved further down. We
just need to ensure that it's ready before we initialise SDRC
and PM code. SDRC reconfigure of DDR needs to be executed from
SRAM and of-course the PM WFI routine. Today we do SDRC init early
and that's the reason SRAM is mapped before that. So both of them
needs to be moved down in the boot to make it work.

>>>> Furthermore... there is also a static mapping for physical address 
>>>> 0x4e000000 using virtual address 0xff100000 which is already reserved 
>>>> for other purposes i.e. the consistent DMA area.  It is not immediately 
>>>> obvious where this comes from without being intimate with the OMAP code. 
>>>> Can this be fixed as well i.e. moved elsewhere please?
>>>
>>> This sounds like a bug somewhere. Which omap are you seeing this on?
>>
>> OMAP4430 on a Panda board.
>>
>> Here are the static mappings I'm seeing:
>>
>> phys = 0x44000000 virt = 0xf8000000 size = 0x100000
>> phys = 0x4a000000 virt = 0xfc000000 size = 0x400000
>> phys = 0x50000000 virt = 0xf9000000 size = 0x100000
>> phys = 0x4c000000 virt = 0xfd100000 size = 0x100000
>> phys = 0x4d000000 virt = 0xfe100000 size = 0x100000
>> phys = 0x4e000000 virt = 0xff100000 size = 0x100000 <---
>> phys = 0x48000000 virt = 0xfa000000 size = 0x400000
>> phys = 0x54000000 virt = 0xfe800000 size = 0x800000
>>
>> It is also possible that I might have screwed something up on my side.  
>> What is located at 0x4e000000?
> 
This was definitely a BUG which I noticed last merge window. The fix for
this is already getting into 3.2.

Regards
Santosh

Re: Please help with the OMAP static mapping mess

[Tony Lindgren]

* Shilimkar, Santosh <santosh.shilimkar@ti.com> [111003 22:45]:
> On Tue, Oct 4, 2011 at 4:29 AM, Tony Lindgren <tony@atomide.com> wrote:
> > * Nicolas Pitre <nico@fluxnic.net> [111003 15:05]:
> >> On Mon, 3 Oct 2011, Nicolas Pitre wrote:
> >>
> >> > On Mon, 3 Oct 2011, Tony Lindgren wrote:
> >> >
> >> > > * Nicolas Pitre <nico@fluxnic.net> [111003 11:26]:
> >> > > >
> >> > > > Furthermore... there is also a static mapping for physical address
> >> > > > 0x4e000000 using virtual address 0xff100000 which is already reserved
> >> > > > for other purposes i.e. the consistent DMA area.  It is not immediately
> >> > > > obvious where this comes from without being intimate with the OMAP code.
> >> > > > Can this be fixed as well i.e. moved elsewhere please?
> >> > >
> >> > > This sounds like a bug somewhere. Which omap are you seeing this on?
> >> >
> >> > OMAP4430 on a Panda board.
> >> >
> >> > Here are the static mappings I'm seeing:
> >> >
> >> > phys = 0x44000000 virt = 0xf8000000 size = 0x100000
> >> > phys = 0x4a000000 virt = 0xfc000000 size = 0x400000
> >> > phys = 0x50000000 virt = 0xf9000000 size = 0x100000
> >> > phys = 0x4c000000 virt = 0xfd100000 size = 0x100000
> >> > phys = 0x4d000000 virt = 0xfe100000 size = 0x100000
> >> > phys = 0x4e000000 virt = 0xff100000 size = 0x100000 <---
> >> > phys = 0x48000000 virt = 0xfa000000 size = 0x400000
> >> > phys = 0x54000000 virt = 0xfe800000 size = 0x800000
> >>
> >> It looks like this comes from OMAP44XX_DMM_VIRT.
> >>
> >> #define OMAP44XX_DMM_PHYS       OMAP44XX_DMM_BASE
> >>                                                 /* 0x4e000000 --> 0xfd300000 */
> >> #define OMAP44XX_DMM_VIRT       (OMAP44XX_DMM_PHYS + OMAP4_L3_PER_IO_OFFSET)
> >> #define OMAP44XX_DMM_SIZE       SZ_1M
> >>
> >> The comment suggesting a mapping correspondance is obviously wrong. We have:
> >>
> >> #define OMAP44XX_DMM_BASE       0x4e000000
> >> #define OMAP4_L3_PER_IO_OFFSET  0xb1100000
> >>
> >> Hence 0x4e000000 + 0xb1100000 = 0xff100000.
> >
> > Seem like it might cause some random patterns in tiler :)
> > Santosh, can youp please check it?
> >
> This is already fixed Tony. You have pulled that patch.
> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg55258.html

OK thanks. Yup, looks like it's queued up in l3 branch.

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Please help with the OMAP static mapping mess

[Tony Lindgren]

* Shilimkar, Santosh <santosh.shilimkar@ti.com> [111003 22:45]:
> On Tue, Oct 4, 2011 at 4:29 AM, Tony Lindgren <tony@atomide.com> wrote:
> > * Nicolas Pitre <nico@fluxnic.net> [111003 15:05]:
> >> On Mon, 3 Oct 2011, Nicolas Pitre wrote:
> >>
> >> > On Mon, 3 Oct 2011, Tony Lindgren wrote:
> >> >
> >> > > * Nicolas Pitre <nico@fluxnic.net> [111003 11:26]:
> >> > > >
> >> > > > Furthermore... there is also a static mapping for physical address
> >> > > > 0x4e000000 using virtual address 0xff100000 which is already reserved
> >> > > > for other purposes i.e. the consistent DMA area. ?It is not immediately
> >> > > > obvious where this comes from without being intimate with the OMAP code.
> >> > > > Can this be fixed as well i.e. moved elsewhere please?
> >> > >
> >> > > This sounds like a bug somewhere. Which omap are you seeing this on?
> >> >
> >> > OMAP4430 on a Panda board.
> >> >
> >> > Here are the static mappings I'm seeing:
> >> >
> >> > phys = 0x44000000 virt = 0xf8000000 size = 0x100000
> >> > phys = 0x4a000000 virt = 0xfc000000 size = 0x400000
> >> > phys = 0x50000000 virt = 0xf9000000 size = 0x100000
> >> > phys = 0x4c000000 virt = 0xfd100000 size = 0x100000
> >> > phys = 0x4d000000 virt = 0xfe100000 size = 0x100000
> >> > phys = 0x4e000000 virt = 0xff100000 size = 0x100000 <---
> >> > phys = 0x48000000 virt = 0xfa000000 size = 0x400000
> >> > phys = 0x54000000 virt = 0xfe800000 size = 0x800000
> >>
> >> It looks like this comes from OMAP44XX_DMM_VIRT.
> >>
> >> #define OMAP44XX_DMM_PHYS ? ? ? OMAP44XX_DMM_BASE
> >> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? /* 0x4e000000 --> 0xfd300000 */
> >> #define OMAP44XX_DMM_VIRT ? ? ? (OMAP44XX_DMM_PHYS + OMAP4_L3_PER_IO_OFFSET)
> >> #define OMAP44XX_DMM_SIZE ? ? ? SZ_1M
> >>
> >> The comment suggesting a mapping correspondance is obviously wrong. We have:
> >>
> >> #define OMAP44XX_DMM_BASE ? ? ? 0x4e000000
> >> #define OMAP4_L3_PER_IO_OFFSET ?0xb1100000
> >>
> >> Hence 0x4e000000 + 0xb1100000 = 0xff100000.
> >
> > Seem like it might cause some random patterns in tiler :)
> > Santosh, can youp please check it?
> >
> This is already fixed Tony. You have pulled that patch.
> http://www.mail-archive.com/linux-omap at vger.kernel.org/msg55258.html

OK thanks. Yup, looks like it's queued up in l3 branch.

Tony

Re: Please help with the OMAP static mapping mess

[Nicolas Pitre]

On Mon, 3 Oct 2011, Russell King - ARM Linux wrote:

> On Mon, Oct 03, 2011 at 06:09:57PM -0400, Nicolas Pitre wrote:
> > On Mon, 3 Oct 2011, Tony Lindgren wrote:
> > > Having the SRAM base address move around with different sizes also
> > > requires the SoC detection.. Otherwise we can end up mapping wrong
> > > size and end up trying to access secure SRAM that will hang the system.
> > > 
> > > The way to fix it is to move SRAM init happen much later so we don't
> > > have to map it early. I guess now we could use ioremap for SRAM,
> > > although we may not want device attributes for the executable code?
> > > Got any suggestions here on how we should map SRAM later on?
> > 
> > You can use a variant of ioremap() such as __arm_ioremap() which let you 
> > specify the memory attribute.
> 
> Just be aware that __arm_ioremap() always ends up with stuff in the
> kernel domain, but that's not what you end up with using create_mapping().
> So I'd prefer it if you didn't suggest that __arm_ioremap() should be used
> with types not listed in asm/io.h.

Well, here we're talking about MT_MEMORY and MT_MEMORY_NONCACHED, and 
they are using DOMAIN_KERNEL already.  So no difference there.

Which makes me think... with all those architectures intercepting 
ioremap calls in order to provide an equivalent static mapping address, 
they already get an unexpected domain given that static mappings are 
mostly DOMAIN_IO and not DOMAIN_KERNEL as would result from an non 
intercepted ioremap call.


Nicolas

Please help with the OMAP static mapping mess

[Nicolas Pitre]

On Mon, 3 Oct 2011, Russell King - ARM Linux wrote:

> On Mon, Oct 03, 2011 at 06:09:57PM -0400, Nicolas Pitre wrote:
> > On Mon, 3 Oct 2011, Tony Lindgren wrote:
> > > Having the SRAM base address move around with different sizes also
> > > requires the SoC detection.. Otherwise we can end up mapping wrong
> > > size and end up trying to access secure SRAM that will hang the system.
> > > 
> > > The way to fix it is to move SRAM init happen much later so we don't
> > > have to map it early. I guess now we could use ioremap for SRAM,
> > > although we may not want device attributes for the executable code?
> > > Got any suggestions here on how we should map SRAM later on?
> > 
> > You can use a variant of ioremap() such as __arm_ioremap() which let you 
> > specify the memory attribute.
> 
> Just be aware that __arm_ioremap() always ends up with stuff in the
> kernel domain, but that's not what you end up with using create_mapping().
> So I'd prefer it if you didn't suggest that __arm_ioremap() should be used
> with types not listed in asm/io.h.

Well, here we're talking about MT_MEMORY and MT_MEMORY_NONCACHED, and 
they are using DOMAIN_KERNEL already.  So no difference there.

Which makes me think... with all those architectures intercepting 
ioremap calls in order to provide an equivalent static mapping address, 
they already get an unexpected domain given that static mappings are 
mostly DOMAIN_IO and not DOMAIN_KERNEL as would result from an non 
intercepted ioremap call.


Nicolas

Re: Please help with the OMAP static mapping mess

[Nicolas Pitre]

On Tue, 4 Oct 2011, Santosh Shilimkar wrote:

> On Tuesday 04 October 2011 04:08 AM, Tony Lindgren wrote:
> > * Nicolas Pitre <nico@fluxnic.net> [111003 14:36]:
> >> On Mon, 3 Oct 2011, Tony Lindgren wrote:
> >>
> >>> Having the SRAM base address move around with different sizes also
> >>> requires the SoC detection.. Otherwise we can end up mapping wrong
> >>> size and end up trying to access secure SRAM that will hang the system.
> >>>
> >>> The way to fix it is to move SRAM init happen much later so we don't
> >>> have to map it early. I guess now we could use ioremap for SRAM,
> >>> although we may not want device attributes for the executable code?
> >>> Got any suggestions here on how we should map SRAM later on?
> >>
> >> You can use a variant of ioremap() such as __arm_ioremap() which let you 
> >> specify the memory attribute.
> > 
> > OK, I'll take a look at that.
> > 
> I have tried __arm_ioremap_pfn() for some DDR mapping and it didn't
> work as expected. The mapping was not getting created.

Did you investigate why it wasn't created?  Must have been a trivial 
issue surely?  But you have to wait until memory management is fully 
initialized to call the real ioremap() though, which happens later 
during the boot.

> Needless to say this can't be an IO memory. I later managed to get 
> around with it by using iotable_init() though downside is I have to 
> pick a static virtual address for the mapping.

You are using either MT_MEMORY or MT_MEMORY_NONCACHED in your map_desc 
entry.  So using e.g. __arm_ioremap(phys_addr, size, MT_MEMORY) should 
give you what you need, given that this is called late enough of course.

> But I agree that SRAM mapping can be moved further down. We
> just need to ensure that it's ready before we initialise SDRC
> and PM code. SDRC reconfigure of DDR needs to be executed from
> SRAM and of-course the PM WFI routine. Today we do SDRC init early
> and that's the reason SRAM is mapped before that. So both of them
> needs to be moved down in the boot to make it work.

Note that it is best not to call iotable_init() outside of the 
mdesc->map_io call path.  So either you reshuffle the initialization 
order so that the static mappings alre always in place before doing the 
ioremap() trick, or you use __arm_ioremap() later on.


Nicolas

Please help with the OMAP static mapping mess

[Nicolas Pitre]

On Tue, 4 Oct 2011, Santosh Shilimkar wrote:

> On Tuesday 04 October 2011 04:08 AM, Tony Lindgren wrote:
> > * Nicolas Pitre <nico@fluxnic.net> [111003 14:36]:
> >> On Mon, 3 Oct 2011, Tony Lindgren wrote:
> >>
> >>> Having the SRAM base address move around with different sizes also
> >>> requires the SoC detection.. Otherwise we can end up mapping wrong
> >>> size and end up trying to access secure SRAM that will hang the system.
> >>>
> >>> The way to fix it is to move SRAM init happen much later so we don't
> >>> have to map it early. I guess now we could use ioremap for SRAM,
> >>> although we may not want device attributes for the executable code?
> >>> Got any suggestions here on how we should map SRAM later on?
> >>
> >> You can use a variant of ioremap() such as __arm_ioremap() which let you 
> >> specify the memory attribute.
> > 
> > OK, I'll take a look at that.
> > 
> I have tried __arm_ioremap_pfn() for some DDR mapping and it didn't
> work as expected. The mapping was not getting created.

Did you investigate why it wasn't created?  Must have been a trivial 
issue surely?  But you have to wait until memory management is fully 
initialized to call the real ioremap() though, which happens later 
during the boot.

> Needless to say this can't be an IO memory. I later managed to get 
> around with it by using iotable_init() though downside is I have to 
> pick a static virtual address for the mapping.

You are using either MT_MEMORY or MT_MEMORY_NONCACHED in your map_desc 
entry.  So using e.g. __arm_ioremap(phys_addr, size, MT_MEMORY) should 
give you what you need, given that this is called late enough of course.

> But I agree that SRAM mapping can be moved further down. We
> just need to ensure that it's ready before we initialise SDRC
> and PM code. SDRC reconfigure of DDR needs to be executed from
> SRAM and of-course the PM WFI routine. Today we do SDRC init early
> and that's the reason SRAM is mapped before that. So both of them
> needs to be moved down in the boot to make it work.

Note that it is best not to call iotable_init() outside of the 
mdesc->map_io call path.  So either you reshuffle the initialization 
order so that the static mappings alre always in place before doing the 
ioremap() trick, or you use __arm_ioremap() later on.


Nicolas

Re: Please help with the OMAP static mapping mess

[Russell King - ARM Linux]

On Tue, Oct 04, 2011 at 05:10:36PM -0400, Nicolas Pitre wrote:
> Which makes me think... with all those architectures intercepting 
> ioremap calls in order to provide an equivalent static mapping address, 
> they already get an unexpected domain given that static mappings are 
> mostly DOMAIN_IO and not DOMAIN_KERNEL as would result from an non 
> intercepted ioremap call.

That's a necessary evil - otherwise we have to separate out the
ioremap from vmalloc.

Incidentally, how are you dealing with the problem of a static mapping
setting up a L1 page table entry for DOMAIN_IO, and then a vmalloc
request coming in, overlapping that L1 page table?

If this memory then gets accessed with get_user() with set_fs(get_ds()),
the kernel will oops as we don't switch DOMAIN_IO memory on set_fs().
(I don't know if this happens in practice, but there's nothing to say
that it's illegal to do this.)

Please help with the OMAP static mapping mess

[Russell King - ARM Linux]

On Tue, Oct 04, 2011 at 05:10:36PM -0400, Nicolas Pitre wrote:
> Which makes me think... with all those architectures intercepting 
> ioremap calls in order to provide an equivalent static mapping address, 
> they already get an unexpected domain given that static mappings are 
> mostly DOMAIN_IO and not DOMAIN_KERNEL as would result from an non 
> intercepted ioremap call.

That's a necessary evil - otherwise we have to separate out the
ioremap from vmalloc.

Incidentally, how are you dealing with the problem of a static mapping
setting up a L1 page table entry for DOMAIN_IO, and then a vmalloc
request coming in, overlapping that L1 page table?

If this memory then gets accessed with get_user() with set_fs(get_ds()),
the kernel will oops as we don't switch DOMAIN_IO memory on set_fs().
(I don't know if this happens in practice, but there's nothing to say
that it's illegal to do this.)

Re: Please help with the OMAP static mapping mess

[Nicolas Pitre]

On Tue, 4 Oct 2011, Russell King - ARM Linux wrote:

> On Tue, Oct 04, 2011 at 05:10:36PM -0400, Nicolas Pitre wrote:
> > Which makes me think... with all those architectures intercepting 
> > ioremap calls in order to provide an equivalent static mapping address, 
> > they already get an unexpected domain given that static mappings are 
> > mostly DOMAIN_IO and not DOMAIN_KERNEL as would result from an non 
> > intercepted ioremap call.
> 
> That's a necessary evil - otherwise we have to separate out the
> ioremap from vmalloc.
> 
> Incidentally, how are you dealing with the problem of a static mapping
> setting up a L1 page table entry for DOMAIN_IO, and then a vmalloc
> request coming in, overlapping that L1 page table?
> 
> If this memory then gets accessed with get_user() with set_fs(get_ds()),
> the kernel will oops as we don't switch DOMAIN_IO memory on set_fs().
> (I don't know if this happens in practice, but there's nothing to say
> that it's illegal to do this.)

I suppose that didn't happen so far.  Granted, moving the ioremap 
optimization into core code for all machines will increase the 
possibility for this to happen, even if still small.

With CPU_USE_DOMAINS not set, set_fs() is a no-op anyway, besides 
addr_limit that is.

Is there a strong benefit in having static mappings being DOMAIN_IO 
instead of DOMAIN_KERNEL?


Nicolas

Please help with the OMAP static mapping mess

[Nicolas Pitre]

On Tue, 4 Oct 2011, Russell King - ARM Linux wrote:

> On Tue, Oct 04, 2011 at 05:10:36PM -0400, Nicolas Pitre wrote:
> > Which makes me think... with all those architectures intercepting 
> > ioremap calls in order to provide an equivalent static mapping address, 
> > they already get an unexpected domain given that static mappings are 
> > mostly DOMAIN_IO and not DOMAIN_KERNEL as would result from an non 
> > intercepted ioremap call.
> 
> That's a necessary evil - otherwise we have to separate out the
> ioremap from vmalloc.
> 
> Incidentally, how are you dealing with the problem of a static mapping
> setting up a L1 page table entry for DOMAIN_IO, and then a vmalloc
> request coming in, overlapping that L1 page table?
> 
> If this memory then gets accessed with get_user() with set_fs(get_ds()),
> the kernel will oops as we don't switch DOMAIN_IO memory on set_fs().
> (I don't know if this happens in practice, but there's nothing to say
> that it's illegal to do this.)

I suppose that didn't happen so far.  Granted, moving the ioremap 
optimization into core code for all machines will increase the 
possibility for this to happen, even if still small.

With CPU_USE_DOMAINS not set, set_fs() is a no-op anyway, besides 
addr_limit that is.

Is there a strong benefit in having static mappings being DOMAIN_IO 
instead of DOMAIN_KERNEL?


Nicolas

Re: Please help with the OMAP static mapping mess

[Tony Lindgren]

* Nicolas Pitre <nico@fluxnic.net> [111004 15:47]:
> On Tue, 4 Oct 2011, Russell King - ARM Linux wrote:
> 
> > On Tue, Oct 04, 2011 at 05:10:36PM -0400, Nicolas Pitre wrote:
> > > Which makes me think... with all those architectures intercepting 
> > > ioremap calls in order to provide an equivalent static mapping address, 
> > > they already get an unexpected domain given that static mappings are 
> > > mostly DOMAIN_IO and not DOMAIN_KERNEL as would result from an non 
> > > intercepted ioremap call.
> > 
> > That's a necessary evil - otherwise we have to separate out the
> > ioremap from vmalloc.
> > 
> > Incidentally, how are you dealing with the problem of a static mapping
> > setting up a L1 page table entry for DOMAIN_IO, and then a vmalloc
> > request coming in, overlapping that L1 page table?
> > 
> > If this memory then gets accessed with get_user() with set_fs(get_ds()),
> > the kernel will oops as we don't switch DOMAIN_IO memory on set_fs().
> > (I don't know if this happens in practice, but there's nothing to say
> > that it's illegal to do this.)
> 
> I suppose that didn't happen so far.  Granted, moving the ioremap 
> optimization into core code for all machines will increase the 
> possibility for this to happen, even if still small.
> 
> With CPU_USE_DOMAINS not set, set_fs() is a no-op anyway, besides 
> addr_limit that is.
> 
> Is there a strong benefit in having static mappings being DOMAIN_IO 
> instead of DOMAIN_KERNEL?

In any case, I suggest we add the following __arm_ioremap_exec patch
and then sort out issues with it as they show up.

This allows further work on the common SRAM genalloc patches and generic
map_io patches.

Nico, I already have a series converting omap2+ to use the the patch
below, and it seems to be working just fine for SRAM. I'll post that
series separately shortly. Maybe take a look and see if that allows
you to do the generic map_io changes?

I still also need to convert omap1 too, but will do that a bit later.

Regards,

Tony


From: Tony Lindgren <tony@atomide.com>
Date: Tue, 4 Oct 2011 17:22:16 -0700
Subject: [PATCH] ARM: Add __arm_ioremap_exec for mapping external memory as MT_MEMORY

This allows mapping external memory such as SRAM for use.

This is needed for some small chunks of code, such as reprogramming
SDRAM memory source clocks that can't be executed in SDRAM. Other
use cases include some PM related code.

Signed-off-by: Tony Lindgren <tony@atomide.com>

--- a/arch/arm/include/asm/io.h
+++ b/arch/arm/include/asm/io.h
@@ -80,6 +80,7 @@ extern void __iomem *__arm_ioremap_caller(unsigned long, size_t, unsigned int,
 
 extern void __iomem *__arm_ioremap_pfn(unsigned long, unsigned long, size_t, unsigned int);
 extern void __iomem *__arm_ioremap(unsigned long, size_t, unsigned int);
+extern void __iomem *__arm_ioremap_exec(unsigned long, size_t, int cached);
 extern void __iounmap(volatile void __iomem *addr);
 
 /*
--- a/arch/arm/mm/ioremap.c
+++ b/arch/arm/mm/ioremap.c
@@ -289,6 +289,28 @@ __arm_ioremap(unsigned long phys_addr, size_t size, unsigned int mtype)
 }
 EXPORT_SYMBOL(__arm_ioremap);
 
+/*
+ * Remap an arbitrary physical address space into the kernel virtual
+ * address space as memory. Needed when the kernel wants to execute
+ * code in external memory. This is needed for reprogramming source
+ * clocks that would affect normal memory for example. Please see
+ * CONFIG_GENERIC_ALLOCATOR for allocating external memory.
+ */
+void __iomem *
+__arm_ioremap_exec(unsigned long phys_addr, size_t size, int cached)
+{
+	unsigned int mtype;
+
+	if (cached)
+		mtype = MT_MEMORY;
+	else
+		mtype = MT_MEMORY_NONCACHED;
+
+	return __arm_ioremap_caller(phys_addr, size, mtype,
+			__builtin_return_address(0));
+}
+EXPORT_SYMBOL(__arm_ioremap_exec);
+
 void __iounmap(volatile void __iomem *io_addr)
 {
 	void *addr = (void *)(PAGE_MASK & (unsigned long)io_addr);

Please help with the OMAP static mapping mess

[Tony Lindgren]

* Nicolas Pitre <nico@fluxnic.net> [111004 15:47]:
> On Tue, 4 Oct 2011, Russell King - ARM Linux wrote:
> 
> > On Tue, Oct 04, 2011 at 05:10:36PM -0400, Nicolas Pitre wrote:
> > > Which makes me think... with all those architectures intercepting 
> > > ioremap calls in order to provide an equivalent static mapping address, 
> > > they already get an unexpected domain given that static mappings are 
> > > mostly DOMAIN_IO and not DOMAIN_KERNEL as would result from an non 
> > > intercepted ioremap call.
> > 
> > That's a necessary evil - otherwise we have to separate out the
> > ioremap from vmalloc.
> > 
> > Incidentally, how are you dealing with the problem of a static mapping
> > setting up a L1 page table entry for DOMAIN_IO, and then a vmalloc
> > request coming in, overlapping that L1 page table?
> > 
> > If this memory then gets accessed with get_user() with set_fs(get_ds()),
> > the kernel will oops as we don't switch DOMAIN_IO memory on set_fs().
> > (I don't know if this happens in practice, but there's nothing to say
> > that it's illegal to do this.)
> 
> I suppose that didn't happen so far.  Granted, moving the ioremap 
> optimization into core code for all machines will increase the 
> possibility for this to happen, even if still small.
> 
> With CPU_USE_DOMAINS not set, set_fs() is a no-op anyway, besides 
> addr_limit that is.
> 
> Is there a strong benefit in having static mappings being DOMAIN_IO 
> instead of DOMAIN_KERNEL?

In any case, I suggest we add the following __arm_ioremap_exec patch
and then sort out issues with it as they show up.

This allows further work on the common SRAM genalloc patches and generic
map_io patches.

Nico, I already have a series converting omap2+ to use the the patch
below, and it seems to be working just fine for SRAM. I'll post that
series separately shortly. Maybe take a look and see if that allows
you to do the generic map_io changes?

I still also need to convert omap1 too, but will do that a bit later.

Regards,

Tony


From: Tony Lindgren <tony@atomide.com>
Date: Tue, 4 Oct 2011 17:22:16 -0700
Subject: [PATCH] ARM: Add __arm_ioremap_exec for mapping external memory as MT_MEMORY

This allows mapping external memory such as SRAM for use.

This is needed for some small chunks of code, such as reprogramming
SDRAM memory source clocks that can't be executed in SDRAM. Other
use cases include some PM related code.

Signed-off-by: Tony Lindgren <tony@atomide.com>

--- a/arch/arm/include/asm/io.h
+++ b/arch/arm/include/asm/io.h
@@ -80,6 +80,7 @@ extern void __iomem *__arm_ioremap_caller(unsigned long, size_t, unsigned int,
 
 extern void __iomem *__arm_ioremap_pfn(unsigned long, unsigned long, size_t, unsigned int);
 extern void __iomem *__arm_ioremap(unsigned long, size_t, unsigned int);
+extern void __iomem *__arm_ioremap_exec(unsigned long, size_t, int cached);
 extern void __iounmap(volatile void __iomem *addr);
 
 /*
--- a/arch/arm/mm/ioremap.c
+++ b/arch/arm/mm/ioremap.c
@@ -289,6 +289,28 @@ __arm_ioremap(unsigned long phys_addr, size_t size, unsigned int mtype)
 }
 EXPORT_SYMBOL(__arm_ioremap);
 
+/*
+ * Remap an arbitrary physical address space into the kernel virtual
+ * address space as memory. Needed when the kernel wants to execute
+ * code in external memory. This is needed for reprogramming source
+ * clocks that would affect normal memory for example. Please see
+ * CONFIG_GENERIC_ALLOCATOR for allocating external memory.
+ */
+void __iomem *
+__arm_ioremap_exec(unsigned long phys_addr, size_t size, int cached)
+{
+	unsigned int mtype;
+
+	if (cached)
+		mtype = MT_MEMORY;
+	else
+		mtype = MT_MEMORY_NONCACHED;
+
+	return __arm_ioremap_caller(phys_addr, size, mtype,
+			__builtin_return_address(0));
+}
+EXPORT_SYMBOL(__arm_ioremap_exec);
+
 void __iounmap(volatile void __iomem *io_addr)
 {
 	void *addr = (void *)(PAGE_MASK & (unsigned long)io_addr);

Re: Please help with the OMAP static mapping mess

[Nicolas Pitre]

On Tue, 4 Oct 2011, Tony Lindgren wrote:

> In any case, I suggest we add the following __arm_ioremap_exec patch
> and then sort out issues with it as they show up.
> 
> This allows further work on the common SRAM genalloc patches and generic
> map_io patches.
> 
> Nico, I already have a series converting omap2+ to use the the patch
> below, and it seems to be working just fine for SRAM. I'll post that
> series separately shortly. Maybe take a look and see if that allows
> you to do the generic map_io changes?

Yes, that looks fine.  Maybe you could avoid exporting the symbol until 
it is actually needed though.

The other thing I need is for the code using ioremap() such as in 
omap2_set_globals_control() to be called only after the corresponding 
static mappings are already installed.  Then everything should go 
smoothly.


Nicolas

Please help with the OMAP static mapping mess

[Nicolas Pitre]

On Tue, 4 Oct 2011, Tony Lindgren wrote:

> In any case, I suggest we add the following __arm_ioremap_exec patch
> and then sort out issues with it as they show up.
> 
> This allows further work on the common SRAM genalloc patches and generic
> map_io patches.
> 
> Nico, I already have a series converting omap2+ to use the the patch
> below, and it seems to be working just fine for SRAM. I'll post that
> series separately shortly. Maybe take a look and see if that allows
> you to do the generic map_io changes?

Yes, that looks fine.  Maybe you could avoid exporting the symbol until 
it is actually needed though.

The other thing I need is for the code using ioremap() such as in 
omap2_set_globals_control() to be called only after the corresponding 
static mappings are already installed.  Then everything should go 
smoothly.


Nicolas

Re: Please help with the OMAP static mapping mess

[Tony Lindgren]

* Nicolas Pitre <nico@fluxnic.net> [111004 17:23]:
> On Tue, 4 Oct 2011, Tony Lindgren wrote:
> 
> > In any case, I suggest we add the following __arm_ioremap_exec patch
> > and then sort out issues with it as they show up.
> > 
> > This allows further work on the common SRAM genalloc patches and generic
> > map_io patches.
> > 
> > Nico, I already have a series converting omap2+ to use the the patch
> > below, and it seems to be working just fine for SRAM. I'll post that
> > series separately shortly. Maybe take a look and see if that allows
> > you to do the generic map_io changes?
> 
> Yes, that looks fine.  Maybe you could avoid exporting the symbol until 
> it is actually needed though.

OK good, will drop the export.
 
> The other thing I need is for the code using ioremap() such as in 
> omap2_set_globals_control() to be called only after the corresponding 
> static mappings are already installed.  Then everything should go 
> smoothly.

Ah, set_globals inits can now move to init_early as well. That happens
after map_io so sounds like that should do for what you need.

Will reply with a patch for that to the series. BTW, these patches
depends on the various cleanup related branches that we have queued,
I think there may be few more patches missing from Arnd's tree still.
I also need to do a bit more testing on it tomorrow.

Regards,

Tony

Please help with the OMAP static mapping mess

[Tony Lindgren]

* Nicolas Pitre <nico@fluxnic.net> [111004 17:23]:
> On Tue, 4 Oct 2011, Tony Lindgren wrote:
> 
> > In any case, I suggest we add the following __arm_ioremap_exec patch
> > and then sort out issues with it as they show up.
> > 
> > This allows further work on the common SRAM genalloc patches and generic
> > map_io patches.
> > 
> > Nico, I already have a series converting omap2+ to use the the patch
> > below, and it seems to be working just fine for SRAM. I'll post that
> > series separately shortly. Maybe take a look and see if that allows
> > you to do the generic map_io changes?
> 
> Yes, that looks fine.  Maybe you could avoid exporting the symbol until 
> it is actually needed though.

OK good, will drop the export.
 
> The other thing I need is for the code using ioremap() such as in 
> omap2_set_globals_control() to be called only after the corresponding 
> static mappings are already installed.  Then everything should go 
> smoothly.

Ah, set_globals inits can now move to init_early as well. That happens
after map_io so sounds like that should do for what you need.

Will reply with a patch for that to the series. BTW, these patches
depends on the various cleanup related branches that we have queued,
I think there may be few more patches missing from Arnd's tree still.
I also need to do a bit more testing on it tomorrow.

Regards,

Tony

Re: Please help with the OMAP static mapping mess

[Rob Herring]

On 10/04/2011 04:21 PM, Nicolas Pitre wrote:
> On Tue, 4 Oct 2011, Santosh Shilimkar wrote:
> 
>> On Tuesday 04 October 2011 04:08 AM, Tony Lindgren wrote:
>>> * Nicolas Pitre <nico@fluxnic.net> [111003 14:36]:
>>>> On Mon, 3 Oct 2011, Tony Lindgren wrote:
>>>>
>>>>> Having the SRAM base address move around with different sizes also
>>>>> requires the SoC detection.. Otherwise we can end up mapping wrong
>>>>> size and end up trying to access secure SRAM that will hang the system.
>>>>>
>>>>> The way to fix it is to move SRAM init happen much later so we don't
>>>>> have to map it early. I guess now we could use ioremap for SRAM,
>>>>> although we may not want device attributes for the executable code?
>>>>> Got any suggestions here on how we should map SRAM later on?
>>>>
>>>> You can use a variant of ioremap() such as __arm_ioremap() which let you 
>>>> specify the memory attribute.
>>>
>>> OK, I'll take a look at that.
>>>
>> I have tried __arm_ioremap_pfn() for some DDR mapping and it didn't
>> work as expected. The mapping was not getting created.
> 
> Did you investigate why it wasn't created?  Must have been a trivial 
> issue surely?  But you have to wait until memory management is fully 
> initialized to call the real ioremap() though, which happens later 
> during the boot.
> 

Isn't ioremap prevented from using main memory now?

Rob

Please help with the OMAP static mapping mess

[Rob Herring]

On 10/04/2011 04:21 PM, Nicolas Pitre wrote:
> On Tue, 4 Oct 2011, Santosh Shilimkar wrote:
> 
>> On Tuesday 04 October 2011 04:08 AM, Tony Lindgren wrote:
>>> * Nicolas Pitre <nico@fluxnic.net> [111003 14:36]:
>>>> On Mon, 3 Oct 2011, Tony Lindgren wrote:
>>>>
>>>>> Having the SRAM base address move around with different sizes also
>>>>> requires the SoC detection.. Otherwise we can end up mapping wrong
>>>>> size and end up trying to access secure SRAM that will hang the system.
>>>>>
>>>>> The way to fix it is to move SRAM init happen much later so we don't
>>>>> have to map it early. I guess now we could use ioremap for SRAM,
>>>>> although we may not want device attributes for the executable code?
>>>>> Got any suggestions here on how we should map SRAM later on?
>>>>
>>>> You can use a variant of ioremap() such as __arm_ioremap() which let you 
>>>> specify the memory attribute.
>>>
>>> OK, I'll take a look at that.
>>>
>> I have tried __arm_ioremap_pfn() for some DDR mapping and it didn't
>> work as expected. The mapping was not getting created.
> 
> Did you investigate why it wasn't created?  Must have been a trivial 
> issue surely?  But you have to wait until memory management is fully 
> initialized to call the real ioremap() though, which happens later 
> during the boot.
> 

Isn't ioremap prevented from using main memory now?

Rob

Re: Please help with the OMAP static mapping mess

[Nicolas Pitre]

On Tue, 4 Oct 2011, Rob Herring wrote:

> On 10/04/2011 04:21 PM, Nicolas Pitre wrote:
> > On Tue, 4 Oct 2011, Santosh Shilimkar wrote:
> > 
> >> On Tuesday 04 October 2011 04:08 AM, Tony Lindgren wrote:
> >>> * Nicolas Pitre <nico@fluxnic.net> [111003 14:36]:
> >>>> On Mon, 3 Oct 2011, Tony Lindgren wrote:
> >>>>
> >>>>> Having the SRAM base address move around with different sizes also
> >>>>> requires the SoC detection.. Otherwise we can end up mapping wrong
> >>>>> size and end up trying to access secure SRAM that will hang the system.
> >>>>>
> >>>>> The way to fix it is to move SRAM init happen much later so we don't
> >>>>> have to map it early. I guess now we could use ioremap for SRAM,
> >>>>> although we may not want device attributes for the executable code?
> >>>>> Got any suggestions here on how we should map SRAM later on?
> >>>>
> >>>> You can use a variant of ioremap() such as __arm_ioremap() which let you 
> >>>> specify the memory attribute.
> >>>
> >>> OK, I'll take a look at that.
> >>>
> >> I have tried __arm_ioremap_pfn() for some DDR mapping and it didn't
> >> work as expected. The mapping was not getting created.
> > 
> > Did you investigate why it wasn't created?  Must have been a trivial 
> > issue surely?  But you have to wait until memory management is fully 
> > initialized to call the real ioremap() though, which happens later 
> > during the boot.
> > 
> 
> Isn't ioremap prevented from using main memory now?

My point is that the memory allocator has to be fully initialized 
because memory might need to be allocated when ioremap() is called.  At 
the point where static mappings are created, the regular memory 
allocator is not available yet.


Nicolas

Please help with the OMAP static mapping mess

[Nicolas Pitre]

On Tue, 4 Oct 2011, Rob Herring wrote:

> On 10/04/2011 04:21 PM, Nicolas Pitre wrote:
> > On Tue, 4 Oct 2011, Santosh Shilimkar wrote:
> > 
> >> On Tuesday 04 October 2011 04:08 AM, Tony Lindgren wrote:
> >>> * Nicolas Pitre <nico@fluxnic.net> [111003 14:36]:
> >>>> On Mon, 3 Oct 2011, Tony Lindgren wrote:
> >>>>
> >>>>> Having the SRAM base address move around with different sizes also
> >>>>> requires the SoC detection.. Otherwise we can end up mapping wrong
> >>>>> size and end up trying to access secure SRAM that will hang the system.
> >>>>>
> >>>>> The way to fix it is to move SRAM init happen much later so we don't
> >>>>> have to map it early. I guess now we could use ioremap for SRAM,
> >>>>> although we may not want device attributes for the executable code?
> >>>>> Got any suggestions here on how we should map SRAM later on?
> >>>>
> >>>> You can use a variant of ioremap() such as __arm_ioremap() which let you 
> >>>> specify the memory attribute.
> >>>
> >>> OK, I'll take a look at that.
> >>>
> >> I have tried __arm_ioremap_pfn() for some DDR mapping and it didn't
> >> work as expected. The mapping was not getting created.
> > 
> > Did you investigate why it wasn't created?  Must have been a trivial 
> > issue surely?  But you have to wait until memory management is fully 
> > initialized to call the real ioremap() though, which happens later 
> > during the boot.
> > 
> 
> Isn't ioremap prevented from using main memory now?

My point is that the memory allocator has to be fully initialized 
because memory might need to be allocated when ioremap() is called.  At 
the point where static mappings are created, the regular memory 
allocator is not available yet.


Nicolas

Re: Please help with the OMAP static mapping mess

[Santosh Shilimkar]

On Wednesday 05 October 2011 08:09 AM, Nicolas Pitre wrote:
> On Tue, 4 Oct 2011, Rob Herring wrote:
> 
>> On 10/04/2011 04:21 PM, Nicolas Pitre wrote:
>>> On Tue, 4 Oct 2011, Santosh Shilimkar wrote:
>>>
>>>> On Tuesday 04 October 2011 04:08 AM, Tony Lindgren wrote:
>>>>> * Nicolas Pitre <nico@fluxnic.net> [111003 14:36]:
>>>>>> On Mon, 3 Oct 2011, Tony Lindgren wrote:
>>>>>>
>>>>>>> Having the SRAM base address move around with different sizes also
>>>>>>> requires the SoC detection.. Otherwise we can end up mapping wrong
>>>>>>> size and end up trying to access secure SRAM that will hang the system.
>>>>>>>
>>>>>>> The way to fix it is to move SRAM init happen much later so we don't
>>>>>>> have to map it early. I guess now we could use ioremap for SRAM,
>>>>>>> although we may not want device attributes for the executable code?
>>>>>>> Got any suggestions here on how we should map SRAM later on?
>>>>>>
>>>>>> You can use a variant of ioremap() such as __arm_ioremap() which let you 
>>>>>> specify the memory attribute.
>>>>>
>>>>> OK, I'll take a look at that.
>>>>>
>>>> I have tried __arm_ioremap_pfn() for some DDR mapping and it didn't
>>>> work as expected. The mapping was not getting created.
>>>
>>> Did you investigate why it wasn't created?  Must have been a trivial 
>>> issue surely?  But you have to wait until memory management is fully 
>>> initialized to call the real ioremap() though, which happens later 
>>> during the boot.
>>>
I didn't investigate further on it but may be it was because of the
ordering as you pointed out. The new __arm_ioremap_exe() seems to me
a good idea and should solve the problem.

Regards
Santosh

Please help with the OMAP static mapping mess

[Santosh Shilimkar]

On Wednesday 05 October 2011 08:09 AM, Nicolas Pitre wrote:
> On Tue, 4 Oct 2011, Rob Herring wrote:
> 
>> On 10/04/2011 04:21 PM, Nicolas Pitre wrote:
>>> On Tue, 4 Oct 2011, Santosh Shilimkar wrote:
>>>
>>>> On Tuesday 04 October 2011 04:08 AM, Tony Lindgren wrote:
>>>>> * Nicolas Pitre <nico@fluxnic.net> [111003 14:36]:
>>>>>> On Mon, 3 Oct 2011, Tony Lindgren wrote:
>>>>>>
>>>>>>> Having the SRAM base address move around with different sizes also
>>>>>>> requires the SoC detection.. Otherwise we can end up mapping wrong
>>>>>>> size and end up trying to access secure SRAM that will hang the system.
>>>>>>>
>>>>>>> The way to fix it is to move SRAM init happen much later so we don't
>>>>>>> have to map it early. I guess now we could use ioremap for SRAM,
>>>>>>> although we may not want device attributes for the executable code?
>>>>>>> Got any suggestions here on how we should map SRAM later on?
>>>>>>
>>>>>> You can use a variant of ioremap() such as __arm_ioremap() which let you 
>>>>>> specify the memory attribute.
>>>>>
>>>>> OK, I'll take a look at that.
>>>>>
>>>> I have tried __arm_ioremap_pfn() for some DDR mapping and it didn't
>>>> work as expected. The mapping was not getting created.
>>>
>>> Did you investigate why it wasn't created?  Must have been a trivial 
>>> issue surely?  But you have to wait until memory management is fully 
>>> initialized to call the real ioremap() though, which happens later 
>>> during the boot.
>>>
I didn't investigate further on it but may be it was because of the
ordering as you pointed out. The new __arm_ioremap_exe() seems to me
a good idea and should solve the problem.

Regards
Santosh