[U-Boot] [PATCH] mpc86xx: adjust the DDR BATs after calculating true DDR size

Becky Bruce beckyb at kernel.crashing.org
Tue Mar 30 00:09:39 CEST 2010


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 <timur at 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 at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot



More information about the U-Boot mailing list