On Wed, Dec 10, 2008 at 4:46 PM, Myles Watson <span dir="ltr"><<a href="mailto:mylesgw@gmail.com">mylesgw@gmail.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<br><br><div class="gmail_quote"><div><div></div><div class="Wj3C7c">On Wed, Dec 10, 2008 at 2:27 PM, Carl-Daniel Hailfinger <span dir="ltr"><<a href="mailto:c-d.hailfinger.devel.2006@gmx.net" target="_blank">c-d.hailfinger.devel.2006@gmx.net</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div><div></div><div>On 10.12.2008 20:39, Corey Osgood wrote:<br>
> On Wed, Dec 10, 2008 at 1:29 PM, Myles Watson <<a href="mailto:mylesgw@gmail.com" target="_blank">mylesgw@gmail.com</a>> wrote:<br>
><br>
><br>
>><br>
>>> -----Original Message-----<br>
>>> From: <a href="mailto:coreboot-bounces@coreboot.org" target="_blank">coreboot-bounces@coreboot.org</a> [mailto:<br>
>>><br>
>> <a href="mailto:coreboot-bounces@coreboot.org" target="_blank">coreboot-bounces@coreboot.org</a>]<br>
>><br>
>>> On Behalf Of Carl-Daniel Hailfinger<br>
>>> Sent: Wednesday, December 10, 2008 11:18 AM<br>
>>> To: Corey Osgood<br>
>>> Cc: Segher Boessenkool; coreboot<br>
>>> Subject: Re: [coreboot] [PATCH][v3] Check that CAR and ROM areas<br>
>>> don'tcollide<br>
>>><br>
>>> Hi Segher,<br>
>>><br>
>>> is the last test below with 0x100000000 (2^32) in the formula guaranteed<br>
>>> to work or may cpp truncate the results to 32 bit? I'd rather avoid<br>
>>> introducing a test that can never trigger.<br>
>>><br>
>> ...snip...<br>
>><br>
>><br>
>>> What you actually want is this test:<br>
>>> #if CONFIG_CARBASE + CONFIG_CARSIZE + (CONFIG_COREBOOT_ROMSIZE_KB * 1024)<br>
>>><br>
>>>> 0x100000000<br>
>>>><br>
>> To avoid that problem, maybe we should /1024 instead of *.<br>
>> #if CONFIG_CARBASE/1024 + CONFIG_CARSIZE/1024 + CONFIG_COREBOOT_ROMSIZE_KB<br>
>><br>
>> 1<<22<br>
>><br>
>> I realize that 1<<22 isn't pretty, but the rest doesn't seem too bad.<br>
>><br>
>> Thanks,<br>
>> Myles<br>
>><br>
>><br>
>><br>
> I'm probably missing something, but I'm not seeing the off-by-one error in<br>
> the original function. CONFIG_CARBASE and 0xffffffff are both addresses, you<br>
> could say they're both off by one, but by comparing them the problem is<br>
> negated. You could rearrange it to<br>
><br>
> 0xffffffff - (CONFIG_CARBASE + CONFIG_CARSIZE) < CONFIG_COREBOOT_ROMSIZE_KB<br>
> * 1024<br>
><br>
> which should, AFAIK, compare the remaining space with the ROM size. Right?<br>
><br>
<br>
</div></div>Let me demonstrate the off-by-one error for you with your original code:<br>
<br>
#define CONFIG_CARBASE 0xffef0000<br>
#define CONFIG_CARSIZE 0x00010000<br>
#define CONFIG_COREBOOT_ROMSIZE_KB 1024<br>
#if ((0xffffffff - (CONFIG_COREBOOT_ROMSIZE_KB * 1024)) < (CONFIG_CARBASE + CONFIG_CARSIZE))<br>
<div>#error Your current Cache-As-Ram base does not allow room to map the selected\<br>
 chip size to memory. Please select a different chip size or move the CAR\<br>
 base to another sane location.<br>
#endif<br>
<br>
<br>
</div>Try to compile that snippet. It will error out although the<br>
configuration is valid.<br>
CAR is from 0xffef0000-0xffefffff (size 0x10000)<br>
ROM is from 0xfff00000-0xffffffff (size 0x100000)<br>
<div><div></div><div><br>
Regards,<br>
Carl-Daniel<br>
<br>
--<br>
<a href="http://www.hailfinger.org/" target="_blank">http://www.hailfinger.org/</a></div></div></blockquote></div></div><div><br>I agree with Carl-Daniel.<br><br>How about this one: <br></div><div>#if ((0xffffffff - (CONFIG_COREBOOT_ROMSIZE_KB * 1024)) < (CONFIG_CARBASE + CONFIG_CARSIZE-1)) <br>

</div></div>if the last useable address before ROM < last CAR address (instead of next useable address after CAR)<br><br>Thanks,<br><font color="#888888">Myles<br>
</font></blockquote></div><br>Carl-Daniel, thanks, I see it now. Per Segher's email I'd prefer to go with Carl-Daniel's original suggestion of using 0x100000000.<br><br>Thanks,<br>Corey<br>