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.
On 10.12.2008 17:53, Corey Osgood wrote: > include/arch/x86/cpu.h is probably not the best place for this, but AFAIK it > should be included by and work for all possible targets. Another possible > solution is something like this, I don't think I like it as well, and it > fails the KISS test IMO: > > mainboard/Kconfig: > config COREBOOT_ROMSIZE_KB_128 > bool "128 KB" > depends (0xffffffff - (128 * 1024) > (CONFIG_CARBASE + CONFIG_CARSIZE))) > help > Choose this option if you have a 128 KB ROM chip. > > Yes, the Kconfig test is ugly beyond belief. > Check that the CAR and ROM areas don't collide. > > Signed-off-by: Corey Osgood <[EMAIL PROTECTED]> > > Index: include/arch/x86/cpu.h > =================================================================== > --- include/arch/x86/cpu.h (revision 1066) > +++ include/arch/x86/cpu.h (working copy) > @@ -26,7 +26,15 @@ > #include <device/device.h> > #include <shared.h> > #include <mtrr.h> > +#include <config.h> > > +/* Check that the CAR and ROM areas aren't going to collide */ > +#if ((0xffffffff - (CONFIG_COREBOOT_ROMSIZE_KB * 1024)) < (CONFIG_CARBASE + > CONFIG_CARSIZE)) > Actually, that formula is off by one. A few equivalence transformations will show that: #if (0xffffffff - (CONFIG_COREBOOT_ROMSIZE_KB * 1024)) < (CONFIG_CARBASE + CONFIG_CARSIZE) is equivalent to #if 0xffffffff < CONFIG_CARBASE + CONFIG_CARSIZE + (CONFIG_COREBOOT_ROMSIZE_KB * 1024) is equivalent to #if CONFIG_CARBASE + CONFIG_CARSIZE + (CONFIG_COREBOOT_ROMSIZE_KB * 1024) > 0xffffffff What you actually want is this test: #if CONFIG_CARBASE + CONFIG_CARSIZE + (CONFIG_COREBOOT_ROMSIZE_KB * 1024) > 0x100000000 > +#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 > + > #define X86_VENDOR_INTEL 0 > #define X86_VENDOR_CYRIX 1 > #define X86_VENDOR_AMD 2 > If you fix the off-by-one, the patch is Acked-by: Carl-Daniel Hailfinger <[EMAIL PROTECTED]> Regards, Carl-Daniel -- http://www.hailfinger.org/ -- coreboot mailing list: [email protected] http://www.coreboot.org/mailman/listinfo/coreboot
