[LinuxBIOS] [PATCH] rewrite generic x86 CAR code
c-d.hailfinger.devel.2006 at gmx.net
Tue Jan 8 02:49:54 CET 2008
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
> 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
>>> +.macro simplemask_helper segs, subpart
>>> +.if \subpart & 0x1
>>> + extractmask \segs, \subpart, %eax
>>> + extractmask \segs, \subpart, %edx
>>> +/* 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.
More information about the coreboot