[U-Boot] [RFC] ARM ISA/cpu/SoC code organization for cache and other functions

[U-Boot] [RFC] ARM ISA/cpu/SoC code organization for cache and other functions

[Albert ARIBAUD]

(re-posting cleaned up and outside any other discussion)

This RFC is for discussing how cache operation functions should be 
managed in the ARM tree.

ARM boards are frequently based on a SoC, itseff based on a processor 
(or cpu), itself based on an (instruction set) architecture, 'isa' for 
short. For instance, board edminiv2 is based on SoC orion5x which has an 
ARM926EJ-S processor/cpu, based on an ARMv5 isa.

Cache operations are normally defined at the isa level and all 
processors, SoCs and boards based on an architecture should use this 
architecture's cache operations.

However, some processors or SoCs may provide specialized instructions or 
instruction sequences for some cache operations, or may have a broken 
implementation for some other cache op.

There is thus a need to be able to redefine some isa-defined cache ops 
with processor-specific ops, and also to redefine some isa- or 
processor-defined ops with SoC-specific ops.

For instance, say some armv5 based cpu badly implements full flush but 
correctly implements cache line flush. The cpu code should then provide 
its own version of the full cache flush op, based on looping through the 
cache lines instead of the armv5 single coprocessor instruction.

If, OTOH, a SoC has broken cache ops to the point that cache is 
unuseable, then it should have its cache disabled, which can be done 
through configuration options or by providing SoC-specific cache ops 
implementations that will emit appropriate messages instead of trying to 
exercize the faulty cache functionality.

The source code implementation for ARM cache ops would be:

- a header file declaring the ops as weak C functions for at least full 
cache flush, full cache invalidate, cache line flush and cache line 
invalidate;

- SoC-specific, cpu-specific, and isa-specific definitions for these 
functions, in the form of C files.

- a default implementation in a lib/ file.

The object files shall be linked in decreasing precedence order, i.e. 
SoC file first, then cpu file, then isa file, then lib last, so that for 
each cache op, the weak symbol mechanism uses the most specific one.

One possible organisation for files would be (switch to fixed size font)

         (isa)   (cpu)     SoC)
arch/arm
         /armv5t/
                 cache-ops.c
                 arm926ejs/
                           cache-ops.c
                           orion5x/
                                   cache-ops.c

Plus of course arch/arm/lib/cache-ops.c.

There was a proposal to put SoC families right below arch/arm, because 
members of such SoC families may have many common devices but different 
cpus. We already deal with the common devices by putting them in their 
correct location in U-Boot, e.g. the Ethernet controller of orion5x is 
in drivers/net/, but we don't deal with SoCs which exist in various CPU 
flavors.

Comments more than welcome.

Amicalement,
-- 
Albert.

[U-Boot] [RFC] ARM ISA/cpu/SoC code organization for cache and other functions

[Jason]

Albert,

On Thu, Sep 15, 2011 at 11:42:12PM +0200, Albert ARIBAUD wrote:
> (re-posting cleaned up and outside any other discussion)
> 
> This RFC is for discussing how cache operation functions should be 
> managed in the ARM tree.
...
> The source code implementation for ARM cache ops would be:
> 
> - a header file declaring the ops as weak C functions for at least full 
> cache flush, full cache invalidate, cache line flush and cache line 
> invalidate;
> 
> - SoC-specific, cpu-specific, and isa-specific definitions for these 
> functions, in the form of C files.
> 
> - a default implementation in a lib/ file.
> 
> The object files shall be linked in decreasing precedence order, i.e. 
> SoC file first, then cpu file, then isa file, then lib last, so that for 
> each cache op, the weak symbol mechanism uses the most specific one.
> 
> One possible organisation for files would be (switch to fixed size font)
> 
>          (isa)   (cpu)     SoC)
> arch/arm
>          /armv5t/
>                  cache-ops.c
>                  arm926ejs/
>                            cache-ops.c
>                            orion5x/
>                                    cache-ops.c
> 
> Plus of course arch/arm/lib/cache-ops.c.

As a new-comer to the u-boot code, is there a difference between this
implementation and say, a struct of function pointers?  eg the struct
would default to armv5t ops, and arm926ejs init could redefine, say,
cache line invalidate.

The reason I ask is that I think the struct of function pointers would
be easier for new folks to sort out with cscope, et al.  Whereas, the
weak declarations with multiple implementations would muddy the waters
when learning the code.

Am I missing something?  What are the advantages of weak declarations
over redefined function pointers?

thx,

Jason.

[U-Boot] [RFC] ARM ISA/cpu/SoC code organization for cache and other functions

[Jason]

On Thu, Sep 15, 2011 at 07:10:49PM -0400, Jason wrote:
> Albert,
> 
> On Thu, Sep 15, 2011 at 11:42:12PM +0200, Albert ARIBAUD wrote:
> > (re-posting cleaned up and outside any other discussion)
> > 
> > This RFC is for discussing how cache operation functions should be 
> > managed in the ARM tree.
> ...
> > The source code implementation for ARM cache ops would be:
> > 
> > - a header file declaring the ops as weak C functions for at least full 
> > cache flush, full cache invalidate, cache line flush and cache line 
> > invalidate;
> > 
> > - SoC-specific, cpu-specific, and isa-specific definitions for these 
> > functions, in the form of C files.
> > 
> > - a default implementation in a lib/ file.
> > 
> > The object files shall be linked in decreasing precedence order, i.e. 
> > SoC file first, then cpu file, then isa file, then lib last, so that for 
> > each cache op, the weak symbol mechanism uses the most specific one.
> > 
> > One possible organisation for files would be (switch to fixed size font)
> > 
> >          (isa)   (cpu)     SoC)
> > arch/arm
> >          /armv5t/
> >                  cache-ops.c
> >                  arm926ejs/
> >                            cache-ops.c
> >                            orion5x/
> >                                    cache-ops.c
> > 
> > Plus of course arch/arm/lib/cache-ops.c.
> 
> As a new-comer to the u-boot code, is there a difference between this
> implementation and say, a struct of function pointers?  eg the struct
> would default to armv5t ops, and arm926ejs init could redefine, say,

s/redefine/reassign/

> cache line invalidate.
> 
> The reason I ask is that I think the struct of function pointers would
> be easier for new folks to sort out with cscope, et al.  Whereas, the
> weak declarations with multiple implementations would muddy the waters
> when learning the code.
> 
> Am I missing something?  What are the advantages of weak declarations
> over redefined function pointers?
> 
> thx,
> 
> Jason.
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

[U-Boot] [RFC] ARM ISA/cpu/SoC code organization for cache and other functions

[Albert ARIBAUD]

Le 16/09/2011 01:18, Jason a ?crit :
> On Thu, Sep 15, 2011 at 07:10:49PM -0400, Jason wrote:
>> Albert,
>>
>> On Thu, Sep 15, 2011 at 11:42:12PM +0200, Albert ARIBAUD wrote:
>>> (re-posting cleaned up and outside any other discussion)
>>>
>>> This RFC is for discussing how cache operation functions should be
>>> managed in the ARM tree.
>> ...
>>> The source code implementation for ARM cache ops would be:
>>>
>>> - a header file declaring the ops as weak C functions for at least full
>>> cache flush, full cache invalidate, cache line flush and cache line
>>> invalidate;
>>>
>>> - SoC-specific, cpu-specific, and isa-specific definitions for these
>>> functions, in the form of C files.
>>>
>>> - a default implementation in a lib/ file.
>>>
>>> The object files shall be linked in decreasing precedence order, i.e.
>>> SoC file first, then cpu file, then isa file, then lib last, so that for
>>> each cache op, the weak symbol mechanism uses the most specific one.
>>>
>>> One possible organisation for files would be (switch to fixed size font)
>>>
>>>           (isa)   (cpu)     SoC)
>>> arch/arm
>>>           /armv5t/
>>>                   cache-ops.c
>>>                   arm926ejs/
>>>                             cache-ops.c
>>>                             orion5x/
>>>                                     cache-ops.c
>>>
>>> Plus of course arch/arm/lib/cache-ops.c.
>>
>> As a new-comer to the u-boot code, is there a difference between this
>> implementation and say, a struct of function pointers?  eg the struct
>> would default to armv5t ops, and arm926ejs init could redefine, say,
>
> s/redefine/reassign/
>
>> cache line invalidate.
>>
>> The reason I ask is that I think the struct of function pointers would
>> be easier for new folks to sort out with cscope, et al.  Whereas, the
>> weak declarations with multiple implementations would muddy the waters
>> when learning the code.
>>
>> Am I missing something?  What are the advantages of weak declarations
>> over redefined function pointers?

At first sight, implementing weak symbols through pointers might look 
more understandable because is leverages on C concepts such as pointers, 
which are better known than weak symbols.

However, the simplicity of the base concept must be paid for, in 
complexity of handling: you need to initialize these pointers, you need 
to make sure they are initialized the right way, and that no 
initialization is missed.

You also pay in performance, as each access to the weak symbol means a 
pointer indirection.

Last, and not least, the concept of weak symbols is purely a link-time 
question ; it is only one variant in the ways the linker determines the 
value of each symbol. Why, then, implement it in run-time?

So yes, you could re-implement weak symbols at run time with (arrays of) 
pointers, but you would not gain clarity overall, and you would distract 
the reader's attention from what is actually going on by explicitly 
interposing an implementation.

OTOH, weak symbols, as implemented by gcc, simply tell the linker that 
if a symbol is defined more than once, the linker should do nothing 
rather than complain, and keep the first definition. End of story. :)

I *think* what worries you is that when defining a weak symbol, you 
don't necessarily tell the reader that this symbol is weak, and he might 
have a hard time finding out. That's a valid point: although the symbol 
also has a declaration which explicitly says it is weak, the reader 
might not look up that declaration. But the solution is easy: just add a 
C comment before the symbol, reminding the reader that the symbol is 
weak and why it is, and/or referring him/her to the ./doc/README.xxx 
file which explains it.

>> thx,

You're welcome.

>> Jason.

Amicalement,
-- 
Albert.

[U-Boot] [RFC] ARM ISA/cpu/SoC code organization for cache and other functions

[Mike Frysinger]

On Thursday 15 September 2011 17:42:12 Albert ARIBAUD wrote:
> The object files shall be linked in decreasing precedence order, i.e.
> SoC file first, then cpu file, then isa file, then lib last, so that for
> each cache op, the weak symbol mechanism uses the most specific one.

as long as the weak symbols don't cause a pain in terms of making sure the 
right one is selected.  having one depth of weak and strong is fine, but two 
deep sounds like it could be fragile.

i wonder if cascading aliases would work ?  that way each symbol only has one 
weak/strong pair to deal with.  hopefully it's not too confusing ?

>          (isa)   (cpu)     SoC)
> arch/arm
>          /armv5t/
>                  cache-ops.c

__weak void arm_isa_icache_flush(...) { ... }
void icache_flush(...) __attribute__((alias("arm_isa_icache_flush")));

>                  arm926ejs/
>                            cache-ops.c

__weak void arm_cpu_icache_flush(...) { ... }
void arm_isa_icache_flush(...) __attribute__((alias("arm_cpu_icache_flush")));

>                            orion5x/
>                                    cache-ops.c

void arm_soc_icache_flush(...) { ... }
void arm_cpu_icache_flush(...) __attribute__((alias("arm_soc_icache_flush")));
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111014/5584858a/attachment.pgp 

[U-Boot] [RFC] ARM ISA/cpu/SoC code organization for cache and other functions

[Albert ARIBAUD]

Le 14/10/2011 06:43, Mike Frysinger a ?crit :
> On Thursday 15 September 2011 17:42:12 Albert ARIBAUD wrote:
>> The object files shall be linked in decreasing precedence order, i.e.
>> SoC file first, then cpu file, then isa file, then lib last, so that for
>> each cache op, the weak symbol mechanism uses the most specific one.
>
> as long as the weak symbols don't cause a pain in terms of making sure the
> right one is selected.  having one depth of weak and strong is fine, but two
> deep sounds like it could be fragile.

I did some checks, and the symbol take is always the first one met by 
the linker.

> i wonder if cascading aliases would work ?  that way each symbol only has one
> weak/strong pair to deal with.  hopefully it's not too confusing ?
>
>>           (isa)   (cpu)     SoC)
>> arch/arm
>>           /armv5t/
>>                   cache-ops.c
>
> __weak void arm_isa_icache_flush(...) { ... }
> void icache_flush(...) __attribute__((alias("arm_isa_icache_flush")));
>
>>                   arm926ejs/
>>                             cache-ops.c
>
> __weak void arm_cpu_icache_flush(...) { ... }
> void arm_isa_icache_flush(...) __attribute__((alias("arm_cpu_icache_flush")));
>
>>                             orion5x/
>>                                     cache-ops.c
>
> void arm_soc_icache_flush(...) { ... }
> void arm_cpu_icache_flush(...) __attribute__((alias("arm_soc_icache_flush")));
> -mike

How would priority order work?

Amicalement,
-- 
Albert.