Re: [coreboot] [PATCH v2] CAR for C7, newest version

[Uwe Hermann]

On Fri, Oct 03, 2008 at 04:31:01AM +0200, Carl-Daniel Hailfinger wrote:
> Thanks to Jason Zhao we got a skeleton CAR code for VIA C7. I have tried
> to clean it up a bit and find justifications for every difference from
> x86 and AMD CAR code. I believe this is mostly merge-ready. Although I'd
> have preferred to do this for v3 first, we can fix v2 boards with this
> change and then move them to v3.
> Thanks to Bari Ari for getting the code to me for rewrite/review.
>
> CONFIG_CARTEST shall not be enabled (breaks the build).
>
> Signed-off-by: Carl-Daniel Hailfinger 
> Signed-off-by: Jason Zhao

Very nice, thanks!

Acked-by: Uwe Hermann

Can't test here either, but see comments below:
> Index: LinuxBIOSv2-via_CAR/src/cpu/via/car/cache_as_ram.lds
>
> --- LinuxBIOSv2-via_CAR/src/cpu/via/car/cache_as_ram.lds	(Revision 0)
> +++ LinuxBIOSv2-via_CAR/src/cpu/via/car/cache_as_ram.lds	(Revision 0)
> @@ -0,0 +1,11 @@
> +SECTIONS {
> .init . : {
> _init = .;
> *(.init.text);
> *(.init.rodata);
> *(.init.rodata.*);
> . = ALIGN(16);
> _einit = .;
> }
>
> +}

This is an exact copy of
 cpu/x86/car/cache_as_ram.lds (no license header)
and
 cpu/amd/car/cache_as_ram.lds (this one says
 (C) Stefan Reinauer  but doesn't mention a license)

Can we use only one of the files for all CAR implementations (probably
requires a small fix in the build system, not sure)?

Or, at least, all of them should have a proper GPL header.

This shouldn't hold up a commit, but we should definately fix
it (now or later).
> Index: LinuxBIOSv2-via_CAR/src/cpu/via/car/cache_as_ram.inc
>
> --- LinuxBIOSv2-via_CAR/src/cpu/via/car/cache_as_ram.inc	(Revision 0)
> +++ LinuxBIOSv2-via_CAR/src/cpu/via/car/cache_as_ram.inc	(Revision 0)

[...]
> +#define	CacheSize DCACHE_RAM_SIZE
> +#define	CacheBase DCACHE_RAM_BASE
>
> +#include	<cpu/x86/mtrr.h>
> /* Save the BIST result */
> movl    %eax, %ebp
>
> +CacheAsRam:
> /* disable cache */
> movl    %cr0, %eax
> orl    $(0x1<<30),%eax
> movl    %eax,%cr0
> invd
> /* Set the default memory type and enable fixed and variable MTRRs */
> /* FIXME: The block below should not be commented out. Carl-Daniel */

Please also explain why.
> /*
> movl    $MTRRdefType_MSR, %ecx
> xorl    %edx, %edx
> /* Enable Variable and Fixed MTRRs */
> movl    $0x00000c00, %eax
> wrmsr
> */
> /* Clear all MTRRs */
> xorl    %edx, %edx
> movl    $fixed_mtrr_msr, %esi
>
> +clear_fixed_var_mtrr:
> lodsl   (%esi), %eax
> testl   %eax, %eax
> jz      clear_fixed_var_mtrr_out
> movl    %eax, %ecx
> xorl    %eax, %eax
> wrmsr

Uwe.