Re: [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 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