diff mbox

[U-Boot,v3] fix get_ram_size memory corruption

Message ID 1413894504-26255-1-git-send-email-kraxel@redhat.com
State Rejected
Delegated to: Tom Rini
Headers show

Commit Message

Gerd Hoffmann Oct. 21, 2014, 12:28 p.m. UTC
base[0] is saved, but only restored in case the ram test failed.

Test case:  Start u-boot in qemu, using vexpress-a9 emulation.
qemu places the fdt at the start of ram, as a service for the
guest.  Trying to pick it up there by setting fdt_addr
accordingly fails because the fdt magic cookie is gone (zeroed
out) after calling get_ram_size.

Cc: Wolfgang Denk <wd@denx.de>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 common/memsize.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Wolfgang Denk Oct. 21, 2014, 2:04 p.m. UTC | #1
Dear Gerd,

In message <1413894504-26255-1-git-send-email-kraxel@redhat.com> you wrote:
> base[0] is saved, but only restored in case the ram test failed.

When posting a new version of a patch please always make sure to add a
change log!

I'm afraid your patch breaks at least some boards;  tested on TQM5200
board...

Before (v2014.10):

	U-Boot 2014.10 (Oct 21 2014 - 15:59:47)

	CPU:   MPC5200B v2.2, Core v1.4 at 396 MHz
	       Bus 132 MHz, IPB 132 MHz, PCI 66 MHz
	Board: TQM5200S (TQ-Components GmbH)
	       on a STK52xx carrier board
	I2C:   85 kHz, ready
	DRAM:  64 MiB
	Flash: 32 MiB
	In:    serial
	Out:   serial
	Err:   serial
	Net:   FEC
	IDE:   Bus 0: OK 
	...

With your patch:

	U-Boot 2014.10-00001-g27030e7 (Oct 21 2014 - 15:45:12)

	CPU:   MPC5200B v2.2, Core v1.4 at 396 MHz
	       Bus 132 MHz, IPB 132 MHz, PCI 66 MHz
	Board: TQM5200S (TQ-Components GmbH)
	       on a STK52xx carrier board
	I2C:   85 kHz, ready
	DRAM:  1 GiB
[board hangs here]

Note also the incorrect RAM size.


Sorry, but this is not a correct fix.  NAK.

Best regards,

Wolfgang Denk
diff mbox

Patch

diff --git a/common/memsize.c b/common/memsize.c
index 589400d..dac1368 100644
--- a/common/memsize.c
+++ b/common/memsize.c
@@ -48,11 +48,12 @@  long get_ram_size(long *base, long maxsize)
 	*addr = 0;
 
 	sync ();
-	if ((val = *addr) != 0) {
+	val = *addr;
+	*addr = save[i];
+	if (val != 0) {
 		/* Restore the original data before leaving the function.
 		 */
 		sync ();
-		*addr = save[i];
 		for (cnt = 1; cnt < maxsize / sizeof(long); cnt <<= 1) {
 			addr  = base + cnt;
 			sync ();