On Mar 29, 2010, at 12:51 PM, Timur Tabi wrote: > After determining how much DDR is actually in the system, adjust > DBAT0 and > IBAT0 accordingly. This ensures that the CPU won't attempt to access > (via speculation) addresses outside of actual memory. > > On 86xx systems, DBAT0 and IBAT0 (the BATs for DDR) are initialized > to 2GB and > kept that way. If the system has less than 2GB of memory (typical > for an > MPC8610 HPCD), the CPU may attempt to access this memory during > speculation. > The zlib code is notorious for generating such memory reads, and > indeed on the > MPC8610, uncompressing the Linux kernel causes a machine check > (without this > patch). > > Signed-off-by: Timur Tabi <ti...@freescale.com> > --- > board/freescale/mpc8610hpcd/mpc8610hpcd.c | 2 + > board/freescale/mpc8641hpcn/mpc8641hpcn.c | 2 + > cpu/mpc86xx/cpu.c | 44 ++++++++++++++++++++ > +++++++++ > include/asm-ppc/mmu.h | 4 ++- > include/mpc86xx.h | 2 + > 5 files changed, 53 insertions(+), 1 deletions(-) > > diff --git a/board/freescale/mpc8610hpcd/mpc8610hpcd.c b/board/ > freescale/mpc8610hpcd/mpc8610hpcd.c > index 15a5b7b..b1623ba 100644 > --- a/board/freescale/mpc8610hpcd/mpc8610hpcd.c > +++ b/board/freescale/mpc8610hpcd/mpc8610hpcd.c > @@ -125,6 +125,8 @@ initdram(int board_type) > dram_size = fixed_sdram(); > #endif > > + adjust_ddr_bat(dram_size); > + > puts(" DDR: "); > return dram_size; > } > diff --git a/board/freescale/mpc8641hpcn/mpc8641hpcn.c b/board/ > freescale/mpc8641hpcn/mpc8641hpcn.c > index 7e6aabf..1f8b717 100644 > --- a/board/freescale/mpc8641hpcn/mpc8641hpcn.c > +++ b/board/freescale/mpc8641hpcn/mpc8641hpcn.c > @@ -72,6 +72,8 @@ initdram(int board_type) > dram_size = fixed_sdram(); > #endif > > + adjust_ddr_bat(dram_size); > +
By doing this here, you still have a (small) window where the problem could occur. It's highly unlikely, but we might still have problems going forward. I think we might need to: 1) remove the write of BAT0 from setup_bats (add a comment). This way, there is no BAT setup for the DDR until right after we configure the controller 2) change the name "adjust_ddr_bat" to "setup_ddr_bat" or something similar. I haven't dug around too much to see if this causes problems, but I am able to boot my 8641 this way. 3) Change setup_ddr_bat so that it just does a write (remove the BL from the #defined values for the default BAT0 and or them in here instead, and add a comment to the config file that says BL is determined dynamically > puts(" DDR: "); > return dram_size; > } > diff --git a/cpu/mpc86xx/cpu.c b/cpu/mpc86xx/cpu.c > index f7e012d..00039d3 100644 > --- a/cpu/mpc86xx/cpu.c > +++ b/cpu/mpc86xx/cpu.c > @@ -197,3 +197,47 @@ void mpc86xx_reginfo(void) > printf("\tBR7\t0x%08X\tOR7\t0x%08X \n", in_be32(&lbc->br7), > in_be32(&lbc->or7)); > > } > + > +/* > + * Update the DDR BATs to reflect the actual size of DDR. > + * > + * On 86xx, the CONFIG_SYS_DBAT0U macro is used to specify the > initial size of > + * the BAT for DDR. After the actual size of DDR is determined > (which is > + * usually smaller than the initial size), this BAT should be > adjusted > + * accordingly. Otherwise, any inadvertent access to addresses > beyond DDR > + * (such as via speculative execution) can cause a machine check. > + * > + * dram_size is the actual size of DDR, in bytes > + * > + * Note: we assume that CONFIG_MAX_MEM_MAPPED is <= > BATU_SIZE(BATU_BL_MAX); > + * that is, the maximum amount of memory that U-Boot will ever map > will always > + * fit into one BAT. If this is not true, (e.g. > CONFIG_MAX_MEM_MAPPED is 2GB > + * but HID0_XBSEN is not defined) then we might have a situation > where U-Boot > + * will attempt to relocated itself outside of the region mapped by > DBAT0. > + * This will cause a machine check. I think you need to adjust how much usable ram u-boot thinks it has if you can't map it all. If you have one BAT, and you have an amount of memory that is ! a power of 2, then you're going to leave a chunk unmapped, which can cause problems later. > + * > + * We also assume that the XBL bits are ignored by the processor > (even if set) > + * if extended BAT addressing is disabled. > + */ > +void adjust_ddr_bat(phys_addr_t dram_size) > +{ > + unsigned long batl, batu, bl; > + > + bl = KB_TO_BATU(dram_size / 1024) & BATU_BL_MAX; > + > + if (BATU_SIZE(bl) != dram_size) { > + puts("(limiting mapped memory to "); > + print_size(BATU_SIZE(bl), ")"); > + bl = BATU_BL_MAX; > + } > + > + read_bat(DBAT0, &batu, &batl); > + batu &= ~BATU_BL_MAX; > + batu |= bl; > + write_bat(DBAT0, batu, batl); > + > + read_bat(IBAT0, &batu, &batl); > + batu &= ~BATU_BL_MAX; > + batu |= bl; > + write_bat(IBAT0, batu, batl); > +} > diff --git a/include/asm-ppc/mmu.h b/include/asm-ppc/mmu.h > index fd10249..34a292d 100644 > --- a/include/asm-ppc/mmu.h > +++ b/include/asm-ppc/mmu.h > @@ -213,7 +213,9 @@ extern void print_bats(void); > #define BATL_PADDR(x) ((phys_addr_t)((x & 0xfffe0000) \ > | ((x & 0x0e00ULL) << 24) \ > | ((x & 0x04ULL) << 30))) > -#define BATU_SIZE(x) (1UL << (fls((x & BATU_BL_MAX) >> 2) + 17)) > +#define BATU_SIZE(x) (1ULL << (fls((x & BATU_BL_MAX) >> 2) + 17)) > + > +#define KB_TO_BATU(x) ((((x)/128) - 1) * 4) /* Convert KBs to BATU > value */ It seems somewhat arbitrary that you decided to use take KB here as an arg when the BATU_SIZE macro returns bytes. I'd prefer to see symmetry here. Cheers, B > > /* Used to set up SDR1 register */ > #define HASH_TABLE_SIZE_64K 0x00010000 > diff --git a/include/mpc86xx.h b/include/mpc86xx.h > index c6f30f9..42f8b13 100644 > --- a/include/mpc86xx.h > +++ b/include/mpc86xx.h > @@ -83,5 +83,7 @@ static __inline__ unsigned long get_l2cr (void) > return l2cr_val; > } > > +void adjust_ddr_bat(phys_addr_t dram_kb); > + > #endif /* _ASMLANGUAGE */ > #endif /* __MPC86xx_H__ */ > -- > 1.6.5 > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot