[coreboot] [PATCH] Add the AML code generator for runtime code generation

Stefan Reinauer stepan at coresystems.de
Tue Feb 3 11:36:56 CET 2009


ron minnich wrote:
> On Mon, Feb 2, 2009 at 12:53 AM, Stefan Reinauer <stepan at coresystems.de> wrote:
>
>   
>> One thing I wonder though, do we want to call weak symbols
>> unconditionally, as in Rudolf's code? No if() clause catches the case
>> that the symbol isn't there. In a test program that call would segfault
>> a user space program here.
>>     
>
> if you're going to have a weak symbol I can't see any reason NOT to
> call it unconditionally. The symbol is always defined.
>   
A weak function is like a function pointer. The linker does not choke
when it is encountered, but it might still not have a value. So before
calling that function, you need to check if the function pointer is NULL
or your code crashes.

You are correct, if a symbol is weak, you don't have to compile
conditional as we "normally" do:
 
#ifdef CONFIG_ACPI_TABLEDUMPER
...
#endif

uses CONFIG_ACPI_TABLEDUMPER
default CONFIG_ACPI_TABLEDUMPER=1

and some minor bits in Config/Options.lb

But, frankly. This is not an _option_. We're just treating it as one
because that's a cheap way for us to get a #define on a single mainboard.

With a weak symbol, execution still has to be conditional:

if (function_to_call)
       function_to_call(params);
else
       printk_debug("No function_to_call for this mainboard");


I think the above is pretty beautiful and creates code that is a lot
better understandable than code that is marbled with preprocessor macros
to achieve the same thing.

> If you're going to call it unconditionally then you might as well put
> it in a file by itself, and either compile that file in or compile in
> code that has a different definition of the function. That way, if you
> know which files are used to build a mainboard, you also know exactly
> which functions are used -- the ones in the files.
>   
This is not about the function itself. That lives in an extra file
already, as you suggested. And that is exactly the problem.
The question is: How does the code that calls the function in that file
find out if it (the file or the function) is there or not.
- Either it is there on all mainboards (dummy functions)
- or the _call_ is guarded by preprocessor crap
- or the function prototype is defined weak

> Weak symbols can be frustrating when you're using code management
> tools like kscope or eclipse. You've to two symbols defined in the
> source code base -- which do you  use? (yes, I know we can tell people
> 'just ignore the weak one' but the code analysis tools are not always
> that smart).
>   
Wait. That problem is completely unrelated to weak symbols, and we will
continue to have it, with weak symbols or without. Even more if we
continue without weak symbols.

The reason for that is, we call functions the same. 10 mainboards have
write_pirq_routing_table() ... all mainboards have "main" or "amd64_main".

Of course kscope, eclipse, or doxygen choke on that. But not using weak
symbols means:
- we have more preprocessor macros
or
- we have more duplicate dummy functions

and both options are even worse for kscope, eclipse or doxygen.

> I'm still not convinced we need them. We can get the same capability
> (in v3 anyway) just by carefully selecting which files to build into a
> project.
>   
No. The problem arises through this careful selection. If we don't solve
it in v2, we will see the problem in v3 even more.

Stefan

-- 
coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br.
      Tel.: +49 761 7668825 • Fax: +49 761 7664613
Email: info at coresystems.dehttp://www.coresystems.de/
Registergericht: Amtsgericht Freiburg • HRB 7656
Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 249 bytes
Desc: OpenPGP digital signature
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20090203/b6728bf1/attachment.sig>


More information about the coreboot mailing list