On Wed, Dec 10, 2008 at 2:27 PM, Carl-Daniel Hailfinger < [EMAIL PROTECTED]> wrote:
> 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/ > I agree with Carl-Daniel. How about this one: #if ((0xffffffff - (CONFIG_COREBOOT_ROMSIZE_KB * 1024)) < (CONFIG_CARBASE + CONFIG_CARSIZE-1)) if the last useable address before ROM < last CAR address (instead of next useable address after CAR) Thanks, Myles
-- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
