On Wed, Dec 10, 2008 at 12:39 PM, Corey Osgood <[EMAIL PROTECTED]>wrote:
> On Wed, Dec 10, 2008 at 1:29 PM, Myles Watson <[EMAIL PROTECTED]> wrote: > >> >> >> > -----Original Message----- >> > From: [EMAIL PROTECTED] [mailto: >> [EMAIL PROTECTED] >> > 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? I think your original works fine. It was just confusing at first because of the mix of addresses and sizes. > +#if ((0xffffffff - (CONFIG_COREBOOT_ROMSIZE_KB * 1024)) < (CONFIG_CARBASE + CONFIG_CARSIZE)) If you write out the meaning in words it makes good sense. If the last usable address before the ROM is less than the first usable address after CAR. Looks good to me. Thanks, Myles
-- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
