On 08.01.2008 01:40, Marc Jones wrote:
Carl-Daniel Hailfinger wrote:
While the patch is against the generic x86 CAR code, it can also be easily modified to work with AMD K8/K9/K10 CAR code. Especially the recent K10 commit made AMD CAR an #ifdef mess which could be sorted out nicely.
Yes, It would be good to clean them all up.
OK, will prepare a patch to do so after we have finalized the generic x86 CAR patch.
This patch is part of my quest to clean up those v2 code parts which will someday end up in v3.
A good cause :)
-#if CacheSize == 0x10000 - /* enable caching for 64K using fixed mtrr */ +/* 0x06 is the WB IO type for a given 4k segment.
- segs is the number of 4k segments we want to use for CAR.
- subpart is the nth 32-bit window into IO type configuration.
- reg is the register where the IO type should be stored.
+.macro extractmask segs, subpart, reg +.if \segs - (\subpart * 4) <= 0
- xorl \reg, \reg
+.elseif \segs - (\subpart * 4) == 1
- movl $0x06000000, \reg
+.elseif \segs - (\subpart * 4) == 2
- movl $0x06060000, \reg
+.elseif \segs - (\subpart * 4) == 3
- movl $0x06060600, \reg
+.elseif \segs - (\subpart * 4) >= 4
- movl $0x06060606, \reg
Why not just pass in the number of 4k pieces, \segs - (\subpart * 4)?
To simplify understanding the formula for readers of the code. But I agree moving calculations makes the code more readable in the function above.
+.macro simplemask_helper segs, subpart +.if \subpart & 0x1
- extractmask \segs, \subpart, %eax
- extractmask \segs, \subpart, %edx
+.endif +.endm +/* size is the cache size in bytes we want to use for CAR.
- part is the nth 64-bit window into IO type configuration.
+.macro simplemask size, part
- simplemask_helper (\size / 0x1000), (\part * 2 + 1)
- simplemask_helper (\size / 0x1000), (\part * 2)
The \part stuff isn't really intuitive. I think an .if size > 32K would be better and then the caller doesn't have to know \part. Building on that the macro could fill in ecx as well.
I'll rewrite the patch a bit to make it more intuitive. I disagree with the ".if size > 32k" part, though. But once you see my new code, it may be elegant enough to not worry about the 32k boundary anymore.