On 30.11.2007 01:19, Marc Jones wrote: > Carl-Daniel Hailfinger wrote: > >> On 29.11.2007 23:58, Marc Jones wrote: >> >>> Carl-Daniel Hailfinger wrote: >>> >>>> Everything is set up correctly until now. >>>> >>>> >>>>>>> /* enable caching for 16K/8K/4K using fixed mtrr */ >>>>>>> movl $0x269, %ecx /* fix4k_cc000*/ >>>>>>> #if CacheSize == 0x4000 >>>>>>> movl $0x06060606, %edx /* WB IO type */ >>>>>>> #endif >>>>>>> #if CacheSize == 0x2000 >>>>>>> movl $0x06060000, %edx /* WB IO type */ >>>>>>> #endif >>>>>>> #if CacheSize == 0x1000 >>>>>>> movl $0x06000000, %edx /* WB IO type */ >>>>>>> #endif >>>>>>> xorl %eax, %eax >>>>>>> wrmsr >>>>>>> >>>> This is where the bug is. I'm speaking of the two commands above, >>>> executed unconditionally. %ecx is 0x269, %eax is zeroed, %edx keeps its >>>> value ($0x06060606 in case of CacheSize>=32k). wrmsr is issued. Is there >>>> any reason to assume that this will not disable CAR again between 16k >>>> and 32k? And no, that code is not protected by an #ifdef. >>>> >>> Ah! You are right. This is a bad bug. >>> >> Fix multiple bugs in CAR setup for non-Geode x86. These bugs have been >> sitting there for years... >> The first bug unconditionally disabled CAR between 16k and 32k, even if >> 32k or 64k CAR size was specified. >> The second bug completely disabled CAR if a non-power-of-2-size or a >> size above 64k or below 4k was specified. >> Restructure and shrink the code a bit and fail the build if unsupported >> CAR sizes are requested. >> >> Found by thorough reading through the code. >> >> Signed-off-by: Carl-Daniel Hailfinger <[EMAIL PROTECTED]> >> >> Index: LinuxBIOSv3-CAR/arch/x86/stage0_i586.S >> =================================================================== >> --- LinuxBIOSv3-CAR/arch/x86/stage0_i586.S (Revision 532) >> +++ LinuxBIOSv3-CAR/arch/x86/stage0_i586.S (Arbeitskopie) >> @@ -301,37 +301,44 @@ >> jmp clear_fixed_var_mtrr >> clear_fixed_var_mtrr_out: >> >> -#if CacheSize == 0x10000 >> - /* enable caching for 64K using fixed mtrr */ >> - movl $0x268, %ecx /* fix4k_c0000*/ >> - movl $0x06060606, %eax /* WB IO type */ >> - movl %eax, %edx >> - wrmsr >> - movl $0x269, %ecx >> - wrmsr >> +#if (CacheSize & (CacheSize - 1)) >> +#error Invalid CAR size, is not a power of two! This is a code limitation. >> #endif >> +#if CacheSize > 0x10000 >> +#error Invalid CAR size, must be at most 64k. >> +#endif >> +#if CacheSize < 0x1000 >> +#error Invalid CAR size, must be at least 4k. This is a processor >> limitation. >> +#endif >> >> -#if CacheSize == 0x8000 >> + /* We round down CAR size to the next power of 2 */ >> > > > Fix comment. No longer rounding. >
OK. > >> + movl $0x269, %ecx /* fix4k_c8000*/ >> + >> +#if CacheSize >= 0x8000 >> > > Since you added the check above and there is no rounding, all the > CachSize #ifs can == . > OK. However, I have code pending which enables compile-time CacheSize granularity of 4K. It depends on the preprocessor quite a bit, but it will work fine. If there is a way to retrieve the maximum supported CacheSize of the processor at runtime, even better: I have code which will dynamically set up CAR to exactly that size. > I will get setup and test this on v2 tomorrow. > Thanks! Regards, Carl-Daniel -- linuxbios mailing list linuxbios@linuxbios.org http://www.linuxbios.org/mailman/listinfo/linuxbios