[PATCH] Reduce duplicate definition in CAR code.

[Warren Turkal]

Here is a patch that I feel improves the readability of the CAR code a
little bit. What do you all think?

For the record, the intel and amd implementations have the same lists.
These could probably be pulled into a common macro used by all three
implementations.

Thanks,
wt
8<------------------
Macros for the register addresses for the MTRR MSRs are already defined
in include/cpu/x86/car.h. This patch uses those macros instead of
creating a second instance of that same data.

Signed-off-by: Warren Turkal 
---
 src/cpu/via/car/cache_as_ram.inc |   35 ++++++++++++++++++++++++-----------
 1 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/src/cpu/via/car/cache_as_ram.inc b/src/cpu/via/car/cache_as_ram.inc
index d6df4a9..bbd4420 100644
--- a/src/cpu/via/car/cache_as_ram.inc
+++ b/src/cpu/via/car/cache_as_ram.inc
@@ -47,7 +47,7 @@ CacheAsRam:

/* Clear all MTRRs. */
    xorl	%edx, %edx
-	movl	$fixed_mtrr_msr, %esi
+	movl	$all_mtrr_msrs, %esi

clear_fixed_var_mtrr:
    lodsl	(%esi), %eax
@@ -60,17 +60,30 @@ clear_fixed_var_mtrr:

jmp	clear_fixed_var_mtrr

-fixed_mtrr_msr:
-	.long	0x250, 0x258, 0x259
-	.long	0x268, 0x269, 0x26A
-	.long	0x26B, 0x26C, 0x26D
-	.long	0x26E, 0x26F
-
-var_mtrr_msr:
-	.long	0x200, 0x201, 0x202, 0x203
-	.long	0x204, 0x205, 0x206, 0x207
-	.long	0x208, 0x209, 0x20A, 0x20B
-	.long	0x20C, 0x20D, 0x20E, 0x20F
+all_mtrr_msrs:
+	/* fixed mtrr MSRs */
+	.long	MTRRfix64K_00000_MSR
+	.long	MTRRfix16K_80000_MSR
+	.long	MTRRfix16K_A0000_MSR
+	.long	MTRRfix4K_C0000_MSR
+	.long	MTRRfix4K_C8000_MSR
+	.long	MTRRfix4K_D0000_MSR
+	.long	MTRRfix4K_D8000_MSR
+	.long	MTRRfix4K_E0000_MSR
+	.long	MTRRfix4K_E8000_MSR
+	.long	MTRRfix4K_F0000_MSR
+	.long	MTRRfix4K_F8000_MSR
+
+	/* var mtrr MSRs */
+	.long	MTRRphysBase_MSR(0), MTRRphysBase_MSR(1)
+	.long	MTRRphysBase_MSR(2), MTRRphysBase_MSR(3)
+	.long	MTRRphysBase_MSR(4), MTRRphysBase_MSR(5)
+	.long	MTRRphysBase_MSR(6), MTRRphysBase_MSR(7)
+	.long	MTRRphysBase_MSR(8), MTRRphysBase_MSR(9)
+	.long	MTRRphysBase_MSR(10), MTRRphysBase_MSR(11)
+	.long	MTRRphysBase_MSR(12), MTRRphysBase_MSR(13)
+	.long	MTRRphysBase_MSR(14), MTRRphysBase_MSR(15)
+
    .long	0x000 /* NULL, end of table */

clear_fixed_var_mtrr_out:

[ron minnich]

On Tue, Oct 5, 2010 at 3:00 AM, Warren Turkal  wrote:
> Here is a patch that I feel improves the readability of the CAR code a
> little bit. What do you all think?

I think it's quite nice, as you replace mysterious numbers with a
meaningful name.

ron

[Warren Turkal]

Is that an Ack? If so, I will work on a more serious patch that does
this for all car implementations and moves the lists to a header.

I am thinking of using a macro like MTRR_ADDR_LIST_ASM in the header.
What do you think about that? I don't want it to look like a function
call. What do you think about that?

Thanks,
wt

On Tue, Oct 5, 2010 at 8:36 AM, ron minnich  wrote:
> On Tue, Oct 5, 2010 at 3:00 AM, Warren Turkal  wrote:
> > Here is a patch that I feel improves the readability of the CAR code a
> > little bit. What do you all think?
>
> I think it's quite nice, as you replace mysterious numbers with a
> meaningful name.
>
> ron

[ron minnich]

I think I'm happier if other newer commiters ack this stuff. I don't
work with the code frequently enough to feel comfortable acking some
of these things.

I'm very happy with macros that make data definitions clearer. I'm
very worried about assembly code macros because this is tricky
assembly and people fall into certain habits in terms of thinking
macros are side-effect-free, which they are not in this case.

thanks!

ron

[Warren Turkal]

On Tue, Oct 5, 2010 at 1:15 PM, ron minnich  wrote:
> I think I'm happier if other newer commiters ack this stuff. I don't
> work with the code frequently enough to feel comfortable acking some
> of these things.

Fair enough. Anyone else have any comments?
> I'm very happy with macros that make data definitions clearer. I'm
> very worried about assembly code macros because this is tricky
> assembly and people fall into certain habits in terms of thinking
> macros are side-effect-free, which they are not in this case.

Let me throw together a patch and see what you and others think.

Thanks,
wt