[coreboot] [PATCH] Factor out common CAR asm snippets
kevin at koconnor.net
Sun Oct 3 01:07:40 CEST 2010
On Sat, Oct 02, 2010 at 10:21:38PM +0200, Uwe Hermann wrote:
> Yes, it scares me too (well, sort of :) It's one of the most complex and
> non-obvious pieces of code we have in coreboot. Which makes it even more
> important to make this code as clean and easily readable and
> understandable as we can. Every little bit helps here, e.g. not open-coding
> various asm snippets (in 3 or more slightly different variants) all over
> the place is one very good measure to make it less confusing for people
> trying to read the code. Less lines in cache_as_ram.inc is good.
> Readable macros such as "enable_l2_cache" instead of some open-coded,
> harder to understand assembler instructions is good.
I think the three lines of assembler is easier to understand than
"enable_l2_cache". Assembler isn't C - the macros defined aren't free
abstractions. (In particular, it's not clear they clobber %eax.)
Again, just my 2ct.
> High-level "look, here we enable cache"-style code is much better than
> "look, here we set bit 2 in CR0, and clear bits 5-6 in some MSR and
> write magic value 456 to magic address 123, now please go find the right
> datasheet and look up what it actually does".
I think the bit definitions, msr addresses, port numbers, and special
addresses should use definitions. For an example of this from
More information about the coreboot