On 10.12.2008 20:39, Corey Osgood 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? >
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/ -- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
