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 c-d.hailfinger.devel.2006@gmx.net
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 */ + movl $0x269, %ecx /* fix4k_c8000*/ + +#if CacheSize >= 0x8000 /* enable caching for 32K using fixed mtrr */ - movl $0x269, %ecx /* fix4k_c8000*/ movl $0x06060606, %eax /* WB IO type */ movl %eax, %edx wrmsr #endif
+#if CacheSize >= 0x10000 + /* enable caching for 32K-64K using fixed mtrr */ + movl $0x268, %ecx /* fix4k_c0000*/ + wrmsr +#endif + +#if CacheSize < 0x10000 /* enable caching for 16K/8K/4K using fixed mtrr */ - movl $0x269, %ecx /* fix4k_cc000*/ -#if CacheSize == 0x4000 +#if CacheSize >= 0x4000 movl $0x06060606, %edx /* WB IO type */ -#endif -#if CacheSize == 0x2000 +#elif CacheSize >= 0x2000 movl $0x06060000, %edx /* WB IO type */ -#endif -#if CacheSize == 0x1000 +#elif CacheSize >= 0x1000 movl $0x06000000, %edx /* WB IO type */ #endif xorl %eax, %eax wrmsr +#endif
#if defined(CONFIG_XIP_ROM_SIZE) && defined(CONFIG_XIP_ROM_BASE) /* enable write base caching so we can do execute in place