[coreboot] locking...

[Stefan Reinauer]

On 20.06.2009 2:50 Uhr, ron minnich wrote:
> On Fri, Jun 19, 2009 at 5:17 PM, Stefan Reinauer<stepan at coresystems.de> wrote:
>   
>> Carl-Daniel Hailfinger wrote:
>>     
>>> do_printk is defined in src/arch/i386/lib/printk_init.c as almost
>>> identical function, but without console_tx_flush and without locking. If
>>> only one CPU uses the lockless variant, we lose.
>>>
>>>       
>> This is the version that was intended to be used when CONFIG_USE_INIT is
>> set.
>>     
>
> reminder. do_printk is (or should be) used ONLY in the CAR code. It
> does not use variables that are only available in the RAM code, such
> as the console_drivers structure .
>   

All versions of printk_debug() use a do_printk() implementation.

Then there's print_debug, being the weaker version of printk_debug. But
print_debug which can not do variables should not be used in CAR code.
Instead you should set the CONFIG_USE_PRINTK_IN_CAR (?) option and use
printk_debug instead.

printk_debug is a wrapper around do_printk() which calls do_printk with
BIOS_DEBUG as first parameter, that's it.

There is a do_printk for CAR in src/arch/i386/lib/printk_init.c however
and one for stage2 in src/console/printk.c

> Don't assume you can just stop using it. It is that way for a reason,
> as I found out very recently with the qemu CAR changes.
>
> Note that printk calls console_tx_byte, which does this:
> static void __console_tx_byte(unsigned char byte)
> {
>         struct console_driver *driver;
>         for(driver = console_drivers; driver < econsole_drivers; driver++) {
>                 driver->tx_byte(byte);
>         }
> }
>
>
> do_printk calls its very own console_tx_byte, which is this:
>
> void console_tx_byte(unsigned char byte)
> {
>         if (byte == '\n')
>                 uart8250_tx_byte(TTYS0_BASE, '\r');
>         uart8250_tx_byte(TTYS0_BASE, byte);
> }
>
> So it's a lot more than just not calling console_tx_flush. These are
> not insignificant differences.
>
> I am just trying to wave a caution flag here.
>
>   
Absolutely agreed, and we shouldn't change anything on the struct
console_driver level. At all

> You must take some care before you decide you can replace do_printk
> with printk.
Nope... I started doing some replacements in our internal tree a few
weeks/months back and it works like a charme.


>  They're not even compiled into the same stage (or should
> not be).
>   

There does not even exist a printk in coreboot v2, right now. So they
are only compiled into different projects ;)


Stefan

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

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20090620/40592744/attachment.html>