[coreboot] [PATCH] Move cmos.default handling to bootblock
Patrick Georgi
patrick at georgi-clan.de
Fri Mar 4 21:07:30 CET 2011
Am 04.03.2011 09:32, schrieb Stefan Reinauer:
>> index 895a185..a808cec 100644
>> --- a/src/arch/x86/include/bootblock_common.h
>> +++ b/src/arch/x86/include/bootblock_common.h
>> @@ -17,17 +17,45 @@ static void bootblock_northbridge_init(void) { }
>> static void bootblock_southbridge_init(void) { }
>> #endif
>>
>> -static unsigned long findstage(char* target)
>> +static void *walkcbfs(char *target)
>> {
>> - unsigned long entry;
>> + void *entry;
>> asm volatile (
>> "mov $1f, %%esp\n\t"
>> - "jmp walkcbfs\n\t"
>> + "jmp walkcbfs_asm\n\t"
> maybe just call it _walkcbfs ?
I thought about it, but I wanted to avoid confusion in case some
compiler uses _ prefixes (in which case that would be __walkcbfs).
>> +/* just enough to support findstage. copied because the original version doesn't easily pass through romcc */
>> +struct cbfs_stage {
>> + unsigned long compression;
>> + unsigned long entry; // this is really 64bit, but properly endianized
> Would it make sense to add an unsigned long entry_high after this, in
> this case? Or use a union or uint64_t for entry?
unsigned long entry_high; is better (romcc and all that)
>> +#if CONFIG_USE_OPTION_TABLE
>> +#include <pc80/mc146818rtc.h>
> Since you start using cmos in the bootblock, you might have to consider
> enabling RCBA and the upper 128 bytes of CMOS in some Intel
> southbridges' bootblock.c
The recovery code explicitely covers the first 128 bytes only - so far
the upper 128 bytes are used for suspend-to-ram data only, so this isn't
urgent yet.
RCBA, yes.. I'll take a look, but I think it worked when I tested it.
>> +static void sanitize_cmos(void)
>> +{
>> + if (cmos_error() || !cmos_chksum_valid()) {
> Is this reliably working on hardware? I remember cmos_error being flaky
> on ICH7 early on at some point.
And I vaguely remember that we managed to stabilize this by tweaking
registers.
> How does cmos recovery behave on non-CAR systems if this is removed? It
> would be nice to if we could make sure it works everywhere
The only caller of this function is bootblock_normal - so by default,
this code wasn't used at all.
The patch works on qemu, which is a "non-CAR" system, right?
Patrick
More information about the coreboot
mailing list