[coreboot] Intel Eagle Height evaluation board support

[Myles Watson]

> >>      /* The memory is now setup, use it */
> >> +#if USE_DCACHE_RAM == 0
> >>      cache_lbmem(MTRR_TYPE_WRBACK);
> >> +#endif
> >>  }
> >
> > Could you explain why you had to add this?
> 
> This board uses CAR, like the kontron. When we initialize the memory,
> we are still in CAR, and cache_lbmem will setup MTRR its ways. From
> what I understand, this will break CAR and we'll get stuck here. So
> cache_lbmem is OK for boards using ROMCC (mtarvon, truxton...), but is
> bad with CAR.
OK.

> > I've refactored two of your patches so that they use svn cp from kontron
> and
> > model_6fx sources to show the differences.  If you based your work on
> > different sources, let me know.
> 
> I'm really I wanted to do that but I did not find the time (lot of
> work right now). I used a mix of 2 board : for the CAR skeleton I used
> the Kontron 986LCDM mainboard, for the 3100 stuff the mt arvon board.
Great.

> > My comments:
> > There is some #if 0 code in acpi_tables.  Will it ever be enabled?  If
> not,
> > remove it.
> 
> Yes indeed. As I said I started from the kontron mainboard but in my
> case I didn't fill the OEMB table so they can safely be removed. We
> can still fill table later if needed. I also found another #if 0 /
> #endif pair in auto.c. Its about the MSR_THERM2_CTL. I remember I add
> to disable it on a core 2 duo L7400 otherwise I was stuck. I'll check
> it today if we can re-enable this one.
Thanks.

 
> > I'm confused why we need eagleheights_fixups.  Can we remove it?
> 
> Yes indeed. I always add it in case but here I did not had to use it, so
> trash.
Will do.

> > Index: svn/src/cpu/intel/model_1067x/cache_as_ram_post.c
> > ===================================================================
> > --- svn.orig/src/cpu/intel/model_1067x/cache_as_ram_post.c
> > +++ svn/src/cpu/intel/model_1067x/cache_as_ram_post.c
> > @@ -50,9 +50,9 @@
> >      "wrmsr\n"
> >      "movl    $MTRRphysMask_MSR(1), %ecx\n"
> >      "wrmsr\n"
> > -#endif
> >
> >      "movb    $0x33, %al\noutb %al, $0x80\n"
> > +#endif
> >  #ifdef CLEAR_FIRST_1M_RAM
> >      "movb    $0x34, %al\noutb %al, $0x80\n"
> >      /* Enable Write Combining and Speculative Reads for the first 1MB
> */
> > @@ -120,7 +120,7 @@
> >      "movb    $0x3b, %al\noutb %al, $0x80\n"
> >
> >      /* Enable prefetchers */
> > -    "movl    $0x01a0, %eax\n"
> > +    "movl    $0x01a0, %ecx\n"
> >      "rdmsr\n"
> >      "andl    $~((1 << 9) | (1 << 19)), %eax\n"
> >      "andl    $~((1 << 5) | (1 << 7)), %edx\n"
> >
> > These changes were surprising.  Is there a bug in the original code?
> 
> In the code, we execute a block a instructions to setup stuffs, then
> output a post code. But if the instructions are disabled (ifdef
> CLEAR_FIRST_1M_RAM), it's useless to output the post code no ? For the
> last modifications (enable prefetchers : eax becomes ecx), there is
> indeed a bug. I already sent a patch for this one few months ago but
> it has been lost in the mailing list.
Sorry about that.

> The rdmsr instruction will read
> the msr specified by ecx into edx:eax (Intel Software Dev Manual,
> volume 2B, 4-322).

So I think we should fix the original code and not duplicate it.

Stefan:  What do you think about unifying
src/cpu/intel/model_*/cache_as_ram_post.c?

There are a couple of the files that are very similar.
 
> Thanks again for taking the time to review my patch.
No problem.  Could you send me the corrected Config.lb now that you removed
the disabled devices?

Thanks,
Myles