[coreboot] [PATCH][v3] Check that CAR and ROM areas don'tcollide

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Wed Dec 10 22:27:22 CET 2008


On 10.12.2008 20:39, Corey Osgood wrote:
> On Wed, Dec 10, 2008 at 1:29 PM, Myles Watson <mylesgw at gmail.com> wrote:
>
>   
>>     
>>> -----Original Message-----
>>> From: coreboot-bounces at coreboot.org [mailto:
>>>       
>> coreboot-bounces at coreboot.org]
>>     
>>> On Behalf Of Carl-Daniel Hailfinger
>>> Sent: Wednesday, December 10, 2008 11:18 AM
>>> To: Corey Osgood
>>> Cc: Segher Boessenkool; coreboot
>>> Subject: Re: [coreboot] [PATCH][v3] Check that CAR and ROM areas
>>> don'tcollide
>>>
>>> Hi Segher,
>>>
>>> is the last test below with 0x100000000 (2^32) in the formula guaranteed
>>> to work or may cpp truncate the results to 32 bit? I'd rather avoid
>>> introducing a test that can never trigger.
>>>       
>> ...snip...
>>
>>     
>>> What you actually want is this test:
>>> #if CONFIG_CARBASE + CONFIG_CARSIZE + (CONFIG_COREBOOT_ROMSIZE_KB * 1024)
>>>       
>>>> 0x100000000
>>>>         
>> To avoid that problem, maybe we should /1024 instead of *.
>> #if CONFIG_CARBASE/1024 + CONFIG_CARSIZE/1024 + CONFIG_COREBOOT_ROMSIZE_KB
>>     
>> 1<<22
>>
>> I realize that 1<<22 isn't pretty, but the rest doesn't seem too bad.
>>
>> Thanks,
>> Myles
>>
>>
>>     
> I'm probably missing something, but I'm not seeing the off-by-one error in
> the original function. CONFIG_CARBASE and 0xffffffff are both addresses, you
> could say they're both off by one, but by comparing them the problem is
> negated. You could rearrange it to
>
> 0xffffffff - (CONFIG_CARBASE + CONFIG_CARSIZE) < CONFIG_COREBOOT_ROMSIZE_KB
> * 1024
>
> which should, AFAIK, compare the remaining space with the ROM size. Right?
>   

Let me demonstrate the off-by-one error for you with your original code:

#define CONFIG_CARBASE 0xffef0000
#define CONFIG_CARSIZE 0x00010000
#define CONFIG_COREBOOT_ROMSIZE_KB 1024
#if ((0xffffffff - (CONFIG_COREBOOT_ROMSIZE_KB * 1024)) < (CONFIG_CARBASE + CONFIG_CARSIZE))
#error Your current Cache-As-Ram base does not allow room to map the selected\
 chip size to memory. Please select a different chip size or move the CAR\
 base to another sane location.
#endif


Try to compile that snippet. It will error out although the
configuration is valid.
CAR is from 0xffef0000-0xffefffff (size 0x10000)
ROM is from 0xfff00000-0xffffffff (size 0x100000)

Regards,
Carl-Daniel

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





More information about the coreboot mailing list