[coreboot] Intel Eagle Height evaluation board support
mylesgw at gmail.com
Tue Jun 30 15:55:03 CEST 2009
> >> /* 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.
> > I've refactored two of your patches so that they use svn cp from kontron
> > 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.
> > My comments:
> > There is some #if 0 code in acpi_tables. Will it ever be enabled? If
> > 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.
> > 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
> > 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
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?
More information about the coreboot