[coreboot] [PATCH] Factor out common CAR asm snippets

[Kevin O'Connor]

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
seabios, see:

http://git.linuxtogo.org/?p=kevin/seabios.git;a=blob;f=src/romlayout.S;h=a4695963cd4b10d1369273aef36562eb7f00dd65;hb=94dc9c49c283cd576c25692d17567035557a2505#l231

-Kevin