[coreboot] [PATCH] v3: fix bug in ad-hoc archive finding code in mc146818rtc.c

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Sun Feb 10 15:22:34 CET 2008


On 10.02.2008 14:27, Stefan Reinauer wrote:
> * Stefan Reinauer <stepan at coresystems.de> [080210 14:15]:
>   
>> * Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> [080210 13:02]:
>>     
>>> Use the existing init_archive function to find the LAR in memory.
>>> This fixes the case where the option table was not found depending
>>> on a few unrelated parameters.
>>>       
>
> The parameters are obviously very related. The functionality was just
> moved into the lar utility ;-)
>
>   
>> LAR: Start 0xfff00000 len 0x100000                                                                     
>> LAR: Start 0xfff00000 len 0x100000                                                                     
>> LAR: Start 0xfff00000 len 0x100000                                                                     
>> LAR: Start 0xfff00000 len 0x100000                                                                     
>> LAR: Start 0xfff00000 len 0x100000                                                                     
>> LAR: Start 0xfff00000 len 0x100000                                                                     
>> LAR: Start 0xfffc0000 len 0x3c000 <------BUG!                                                          
>> LAR: Start 0xfff00000 len 0x100000                                                                     
>> LAR: Start 0xfff00000 len 0x100000     
>>     
>
> Your init_archive function is pretty broken, so the above line with the
> "BUG" mentioned is the only one that is half ways correct.
>   

init_archive() is just moved code which was initially committed in r427.
Changelog follows:
> r427 | stepan | 2007-07-01 00:03:39 +0200 (So, 01 Jul 2007) | 13 lines
>
> this patch puts the lar size in the bootblock and reads it from there.
> Why? This way we don't need to recompile the image when the size of the
> LinuxBIOS image changes. This alows building images for 50 motherboards
> and equipping each with 10 payloads, resulting in 500 images while you
> only have to build each payload once and each motherboard, too.


> You mentioned this is a default build, made with:
>   
>> 2. 256 KB (COREBOOT_ROMSIZE_KB_256) (NEW)
>>     
>
> So what I don't understand is why only the BUG line really assumes a
> 256k ROM while all others assume a 1M ROM.
>   

Ouch. arch/x86/stage1.c:init_archive() tries to read the data written by
util/lar/bootblock.c:fixup_bootblock().
Let's find out where the garbage is introduced.

> Maybe this introduced all the access speed problems we were seeing
> lately? Scanning 4x the needed area is pretty overkill.
>   

That would indeed be bad.

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/





More information about the coreboot mailing list