* [U-Boot] [PATCH] mpc86xx: adjust the DDR BATs after calculating true DDR size
@ 2010-03-29 17:51 Timur Tabi
2010-03-29 22:09 ` Becky Bruce
0 siblings, 1 reply; 4+ messages in thread
From: Timur Tabi @ 2010-03-29 17:51 UTC (permalink / raw)
To: u-boot
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@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);
+
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.
+ *
+ * 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 */
/* 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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH] mpc86xx: adjust the DDR BATs after calculating true DDR size
2010-03-29 17:51 [U-Boot] [PATCH] mpc86xx: adjust the DDR BATs after calculating true DDR size Timur Tabi
@ 2010-03-29 22:09 ` Becky Bruce
2010-03-29 23:07 ` Timur Tabi
0 siblings, 1 reply; 4+ messages in thread
From: Becky Bruce @ 2010-03-29 22:09 UTC (permalink / raw)
To: u-boot
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@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
^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH] mpc86xx: adjust the DDR BATs after calculating true DDR size
2010-03-29 22:09 ` Becky Bruce
@ 2010-03-29 23:07 ` Timur Tabi
2010-03-30 0:28 ` Becky Bruce
0 siblings, 1 reply; 4+ messages in thread
From: Timur Tabi @ 2010-03-29 23:07 UTC (permalink / raw)
To: u-boot
On Mon, Mar 29, 2010 at 5:09 PM, Becky Bruce <beckyb@kernel.crashing.org> wrote:
>> --- 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.
U-Boot hasn't relocated yet, so it's not using any of the DDR. I
don't see how there could be a problem.
>?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
Ok, I get it now. It will catch anyone trying to write to DDR before
it's really set up. I can do that.
> 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.
Ok.
> 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
Ok.
> I think you need to adjust how much usable ram u-boot thinks it has if
> you can't map it all.
I tried that. The problem is that U-Boot uses this number to tell
Linux how much DDR there is. The code doesn't really support U-Boot
and Linux seeing different amounts of DDR.
>?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.
AFAIK, we always have only one BAT for DDR. I wasn't planning on
expanding the scope of this patch to add support for multiple BATs.
I don't know how to handle !2^X sizes of DDR.
>> +#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.
I used KB to keep the sizes of numbers small. The smallest value is
128KB, so it's not *that* arbitrary.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH] mpc86xx: adjust the DDR BATs after calculating true DDR size
2010-03-29 23:07 ` Timur Tabi
@ 2010-03-30 0:28 ` Becky Bruce
0 siblings, 0 replies; 4+ messages in thread
From: Becky Bruce @ 2010-03-30 0:28 UTC (permalink / raw)
To: u-boot
On Mar 29, 2010, at 6:07 PM, Timur Tabi wrote:
> On Mon, Mar 29, 2010 at 5:09 PM, Becky Bruce <beckyb@kernel.crashing.org
> > wrote:
<snip>
>
>
>> I think you need to adjust how much usable ram u-boot thinks it has
>> if
>> you can't map it all.
>
> I tried that. The problem is that U-Boot uses this number to tell
> Linux how much DDR there is. The code doesn't really support U-Boot
> and Linux seeing different amounts of DDR.
>> 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.
>
> AFAIK, we always have only one BAT for DDR. I wasn't planning on
> expanding the scope of this patch to add support for multiple BATs.
I'm not suggesting that - you really can't use multiple BATs as there
are only 8 of them and they're a hot resource.
>
> I don't know how to handle !2^X sizes of DDR.
You can't map !2^x sizes of DDR with one BAT, so in this case you're
always going to end up with a mapping that's smaller than the actual
size of DDR, which is a *new* scenario for 86xx. IIRC, before, we
always set the BAT up for 2G, so we either 1) had *less* RAM than we
had BAT mappings for or 2) pretended that we had 2G of memory and
ignored the rest. Have you looked at 83xx or 85xx to see if they do
anything interesting here? It's possible this problem has just been
ignored other places, and we haven't had an issue yet. It should't be
too complicated to get get_effective_memsize to report something
reasonable.
>
>>> +#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.
>
> I used KB to keep the sizes of numbers small. The smallest value is
> 128KB, so it's not *that* arbitrary.
The other stuff used bytes. It's confusing. Just use bytes.....
-B
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-03-30 0:28 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-03-29 17:51 [U-Boot] [PATCH] mpc86xx: adjust the DDR BATs after calculating true DDR size Timur Tabi
2010-03-29 22:09 ` Becky Bruce
2010-03-29 23:07 ` Timur Tabi
2010-03-30 0:28 ` Becky Bruce
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.