Hi,
v2 and v3 have almost identical CAR setup code with identical bugs for CAR sizes != {4k,8k,16k}. In v3, the erroneous code paths are not triggered and the bug is latent, but we have at a few boards in v2 which trigger these bugs, resulting in holes and/or smaller size of the CAR area.
Let's look at the v3 code:
#ifndef CONFIG_CARSIZE #define CacheSize 4096 #else #define CacheSize CONFIG_CARSIZE #endif
v2 uses DCACHE_RAM_SIZE instead of CONFIG_CARSIZE [...]
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 #endif
OK, 64k are working.
#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
OK, 32k are working.
/* 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
Disable CAR between 16k and 32k unconditionally. Even if CacheSize is 32k or 64k. Not nice. This affects the following boards: ./src/mainboard/amd/db800/Options.lb:default DCACHE_RAM_SIZE=0x08000 ./src/mainboard/amd/norwich/Options.lb:default DCACHE_RAM_SIZE=0x08000 ./src/mainboard/amd/serengeti_cheetah/Options.lb:default DCACHE_RAM_SIZE=0x08000 ./src/mainboard/msi/ms7260/Options.lb:default DCACHE_RAM_SIZE = 0x08000 ./src/mainboard/asus/a8n_e/Options.lb:default DCACHE_RAM_SIZE=0x08000 ./src/mainboard/tyan/s2912/Options.lb:default DCACHE_RAM_SIZE=0x08000 ./src/mainboard/artecgroup/dbe61/Options.lb:default DCACHE_RAM_SIZE=0x08000 ./src/mainboard/supermicro/h8dmr/Options.lb:default DCACHE_RAM_SIZE=0x08000 ./src/mainboard/pcengines/alix1c/Options.lb:default DCACHE_RAM_SIZE=0x08000 ./src/mainboard/nvidia/l1_2pvv/Options.lb:default DCACHE_RAM_SIZE=0x08000 ./src/mainboard/digitallogic/msm800sev/Options.lb:default DCACHE_RAM_SIZE=0x08000 ./src/mainboard/gigabyte/ga_2761gxdk/Options.lb:default DCACHE_RAM_SIZE=0x08000 ./src/mainboard/gigabyte/m57sli/Options.lb:default DCACHE_RAM_SIZE=0x08000
If CacheSize is not one of 4k,8k,16k,32k,64k, we will not set up CAR at all. This affects the following board: ./src/mainboard/iwill/dk8_htx/Options.lb:default DCACHE_RAM_SIZE=0x0c000
---------------------------------------------------------------------------------------------
Clean up handling different CAR sizes and round down CAR size to the nearest power of two. 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 510) +++ LinuxBIOSv3-CAR/arch/x86/stage0_i586.S (Arbeitskopie) @@ -301,37 +301,36 @@ 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 -#endif + /* We round down CAR size to the next power of 2 */ + movl $0x269, %ecx /* fix4k_c8000*/
-#if CacheSize == 0x8000 +#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 */ +#else +#error Invalid CAR size, must be at least 4k. #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
Marc? This has been sitting in my tree for a while now.
On 16.11.2007 16:00, Carl-Daniel Hailfinger wrote:
Hi,
v2 and v3 have almost identical CAR setup code with identical bugs for CAR sizes != {4k,8k,16k}. In v3, the erroneous code paths are not triggered and the bug is latent, but we have at a few boards in v2 which trigger these bugs, resulting in holes and/or smaller size of the CAR area.
Let's look at the v3 code:
#ifndef CONFIG_CARSIZE #define CacheSize 4096 #else #define CacheSize CONFIG_CARSIZE #endif
v2 uses DCACHE_RAM_SIZE instead of CONFIG_CARSIZE [...]
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 #endif
OK, 64k are working.
#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
OK, 32k are working.
/* 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
Disable CAR between 16k and 32k unconditionally. Even if CacheSize is 32k or 64k. Not nice. This affects the following boards: ./src/mainboard/amd/db800/Options.lb:default DCACHE_RAM_SIZE=0x08000 ./src/mainboard/amd/norwich/Options.lb:default DCACHE_RAM_SIZE=0x08000 ./src/mainboard/amd/serengeti_cheetah/Options.lb:default DCACHE_RAM_SIZE=0x08000 ./src/mainboard/msi/ms7260/Options.lb:default DCACHE_RAM_SIZE = 0x08000 ./src/mainboard/asus/a8n_e/Options.lb:default DCACHE_RAM_SIZE=0x08000 ./src/mainboard/tyan/s2912/Options.lb:default DCACHE_RAM_SIZE=0x08000 ./src/mainboard/artecgroup/dbe61/Options.lb:default DCACHE_RAM_SIZE=0x08000 ./src/mainboard/supermicro/h8dmr/Options.lb:default DCACHE_RAM_SIZE=0x08000 ./src/mainboard/pcengines/alix1c/Options.lb:default DCACHE_RAM_SIZE=0x08000 ./src/mainboard/nvidia/l1_2pvv/Options.lb:default DCACHE_RAM_SIZE=0x08000 ./src/mainboard/digitallogic/msm800sev/Options.lb:default DCACHE_RAM_SIZE=0x08000 ./src/mainboard/gigabyte/ga_2761gxdk/Options.lb:default DCACHE_RAM_SIZE=0x08000 ./src/mainboard/gigabyte/m57sli/Options.lb:default DCACHE_RAM_SIZE=0x08000
If CacheSize is not one of 4k,8k,16k,32k,64k, we will not set up CAR at all. This affects the following board: ./src/mainboard/iwill/dk8_htx/Options.lb:default DCACHE_RAM_SIZE=0x0c000
Clean up handling different CAR sizes and round down CAR size to the nearest power of two. 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 510) +++ LinuxBIOSv3-CAR/arch/x86/stage0_i586.S (Arbeitskopie) @@ -301,37 +301,36 @@ 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
-#endif
- /* We round down CAR size to the next power of 2 */
movl $0x269, %ecx /* fix4k_c8000*/-#if CacheSize == 0x8000 +#if CacheSize >= 0x8000 /* enable caching for 32K using fixed mtrr */
movl $0x269, %ecx /* fix4k_c8000*/ movl $0x06060606, %eax /* WB IO type */#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 */ +#else +#error Invalid CAR size, must be at least 4k. #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
Carl-Daniel Hailfinger wrote:
Marc? This has been sitting in my tree for a while now.
On 16.11.2007 16:00, Carl-Daniel Hailfinger wrote:
Hi,
v2 and v3 have almost identical CAR setup code with identical bugs for CAR sizes != {4k,8k,16k}. In v3, the erroneous code paths are not triggered and the bug is latent, but we have at a few boards in v2 which trigger these bugs, resulting in holes and/or smaller size of the CAR area.
Sorry, I have been very busy and I have been putting this off.
I think you are correct that CAR expects power of 2 cache sizes. How about just error if the size isn't power of 2 between 4K and 64K? If you wanted to support non power of 2 you should round up otherwise you might write off the end.
Note that Geode platforms don't use the same car file as K8 and has a max cache size of 32K. See arch\x86\geodelx\stage0.S
Marc
On 28.11.2007 23:52, Marc Jones wrote:
Carl-Daniel Hailfinger wrote:
Marc? This has been sitting in my tree for a while now.
On 16.11.2007 16:00, Carl-Daniel Hailfinger wrote:
Hi,
v2 and v3 have almost identical CAR setup code with identical bugs for CAR sizes != {4k,8k,16k}. In v3, the erroneous code paths are not triggered and the bug is latent, but we have at a few boards in v2 which trigger these bugs, resulting in holes and/or smaller size of the CAR area.
Sorry, I have been very busy and I have been putting this off.
I think you are correct that CAR expects power of 2 cache sizes. How about just error if the size isn't power of 2 between 4K and 64K? If you wanted to support non power of 2 you should round up otherwise you might write off the end.
OK, will prepare an updated patch.
What about the bugs which cause 32k CAR to end up as 16k and 64k CAR to have a hole between 16k and 32k?
Note that Geode platforms don't use the same car file as K8 and has a max cache size of 32K. See arch\x86\geodelx\stage0.S
Yes, indeed. Will take a look at that code soon.
Regards, Carl-Daniel
Carl-Daniel Hailfinger wrote:
On 28.11.2007 23:52, Marc Jones wrote:
Carl-Daniel Hailfinger wrote:
Marc? This has been sitting in my tree for a while now.
On 16.11.2007 16:00, Carl-Daniel Hailfinger wrote:
Hi,
v2 and v3 have almost identical CAR setup code with identical bugs for CAR sizes != {4k,8k,16k}. In v3, the erroneous code paths are not triggered and the bug is latent, but we have at a few boards in v2 which trigger these bugs, resulting in holes and/or smaller size of the CAR area.
Sorry, I have been very busy and I have been putting this off.
I think you are correct that CAR expects power of 2 cache sizes. How about just error if the size isn't power of 2 between 4K and 64K? If you wanted to support non power of 2 you should round up otherwise you might write off the end.
OK, will prepare an updated patch.
What about the bugs which cause 32k CAR to end up as 16k and 64k CAR to have a hole between 16k and 32k?
I am not very familiar with the code but it looks like the size is growing from 0xCFFFF down to 0xC0000. I don't see a gap. The movl %eax, %edx make the entire MSR 0x0606060606060606 which would be 32K and then setting that in both 0x269 and 0x268 would be 64K. But I could be misunderstanding the code.
Marc
On 29.11.2007 00:50, Marc Jones wrote:
Carl-Daniel Hailfinger wrote:
On 28.11.2007 23:52, Marc Jones wrote:
Carl-Daniel Hailfinger wrote:
Marc? This has been sitting in my tree for a while now.
On 16.11.2007 16:00, Carl-Daniel Hailfinger wrote:
Hi,
v2 and v3 have almost identical CAR setup code with identical bugs for CAR sizes != {4k,8k,16k}. In v3, the erroneous code paths are not triggered and the bug is latent, but we have at a few boards in v2 which trigger these bugs, resulting in holes and/or smaller size of the CAR area.
Sorry, I have been very busy and I have been putting this off.
I think you are correct that CAR expects power of 2 cache sizes. How about just error if the size isn't power of 2 between 4K and 64K? If you wanted to support non power of 2 you should round up otherwise you might write off the end.
OK, will prepare an updated patch.
What about the bugs which cause 32k CAR to end up as 16k and 64k CAR to have a hole between 16k and 32k?
I am not very familiar with the code but it looks like the size is growing from 0xCFFFF down to 0xC0000. I don't see a gap. The movl %eax, %edx make the entire MSR 0x0606060606060606 which would be 32K and then setting that in both 0x269 and 0x268 would be 64K. But I could be misunderstanding the code.
Quoting from my original mail:
[...]
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 #endif
OK, 64k are working.
#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
OK, 32k are working.
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.
Disable CAR between 16k and 32k unconditionally. Even if CacheSize is 32k or 64k. Not nice.
Regards, Carl-Daniel
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. Marc
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
Has this been tested on hardware? CAR is black magic ;-)
ron
On 30.11.2007 01:12, ron minnich wrote:
Has this been tested on hardware? CAR is black magic ;-)
I have no hardware capable of running LinuxBIOS (except a few OLPC machines with Geode GX, but they use a different CAR setup). Besides that, testing this code in v3 is rather difficult because no machine uses it (yet). Testing this code in v2 requires a few adaptations, depending on whether you test K8 or generic x86.
So, no, sorry, that code change has not been tested at all.
Marc: The AMD Family 10h BKDG says CAR should be set up with a size of exactly 48k. Is that a SHOULD in the RFC sense or just a suggestion?
Regards, Carl-Daniel
my own tree have
#if CacheSize < 0x8000 /* 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 #endif
maybe i didn't post sth...
normally we can use 4k only at first.
run_time decide is not good. for example REV E, you could use 64k. for for REV F you can only use 48K.
Family 10h need more tricks.
YH
yhlu wrote:
my own tree have
#if CacheSize < 0x8000 /* 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 #endif
maybe i didn't post sth...
This is in the v2 code but not in v3.
normally we can use 4k only at first.
run_time decide is not good. for example REV E, you could use 64k. for for REV F you can only use 48K.
So this is still a problem since 48K isn't an option and it isn't a power of 2 so I was incorrect on that point.
It would be good to have an automatic runtime setting for each CPU that could be adjusted by a mainboard override.
Family 10h need more tricks.
These changes are coming soon.
Marc
Marc Jones wrote:
yhlu wrote:
my own tree have
#if CacheSize < 0x8000 /* 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 #endif
maybe i didn't post sth...
This is in the v2 code but not in v3.
normally we can use 4k only at first.
run_time decide is not good. for example REV E, you could use 64k. for for REV F you can only use 48K.
So this is still a problem since 48K isn't an option and it isn't a power of 2 so I was incorrect on that point.
It would be good to have an automatic runtime setting for each CPU that could be adjusted by a mainboard override.
Family 10h need more tricks.
These changes are coming soon.
Marc
Also, 48K is an option in v2. It seems that v3 should be brought up to date with v2 first. Marc
On 30.11.2007 18:36, Marc Jones wrote:
Marc Jones wrote:
yhlu wrote:
my own tree have
#if CacheSize < 0x8000 /* 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 #endif
maybe i didn't post sth...
This is in the v2 code but not in v3.
normally we can use 4k only at first.
run_time decide is not good. for example REV E, you could use 64k. for for REV F you can only use 48K.
So this is still a problem since 48K isn't an option and it isn't a power of 2 so I was incorrect on that point.
It would be good to have an automatic runtime setting for each CPU that could be adjusted by a mainboard override.
Family 10h need more tricks.
These changes are coming soon.
Marc
Also, 48K is an option in v2. It seems that v3 should be brought up to date with v2 first.
48K is not an option for the v2 generic x86 code, only for the v2 K8 code (which is arguably less buggy, but still fails for some non-power-of-2 settings).
Regards, Carl-Daniel
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 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 */
Fix comment. No longer rounding.
movl $0x269, %ecx /* fix4k_c8000*/+#if CacheSize >= 0x8000
Since you added the check above and there is no rounding, all the CachSize #ifs can == .
I will get setup and test this on v2 tomorrow.
Marc
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 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 */
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
-
Carl-Daniel Hailfinger -
Marc Jones -
ron minnich -
yhlu