diff mbox

[U-Boot,2/3] common/memsize.c: correctly restore all modified memory loations

Message ID 1413922451-29726-2-git-send-email-wd@denx.de
State Deferred
Delegated to: Tom Rini
Headers show

Commit Message

Wolfgang Denk Oct. 21, 2014, 8:14 p.m. UTC 
The original memory sizing code in get_ram_size clobbers the word
at the base address, but forgets to restore it.

Unlike the (reverted) commit b8496cced856ff411f1eb2e4eff20f5abe7080b0
we save and restore the base address value at the start resp. at the
end of the function.  It needs to stay cleared until the detection is
done, otherwise it will fail to detect the same piece of memory being
mapped multiple times into the address space.

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

Comments

Wolfgang Denk Oct. 21, 2014, 8:19 p.m. UTC | #1 
Note: there is no patch 3/3 in his series.

It was originally

	[PATCH] powerpc: TQM5200: convert to generic board

but this is actually not related to these changes here, so I decided
to post it independently.

Sorry for the confusion.

Best regards,

Wolfgang Denk
Gerd Hoffmann Oct. 21, 2014, 11:04 p.m. UTC | #2 
Hi,

> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

No, I didn't signed this patch off.
And I will not because it is broken.

>  	for (cnt = (maxsize / sizeof(long)) >> 1; cnt > 0; cnt >>= 1) {
>  		addr = base + cnt;	/* pointer arith! */
> @@ -44,6 +44,7 @@ long get_ram_size(long *base, long maxsize)
>  	addr = base;
>  	sync();
>  	save[i] = *addr;

base[0] might not be the original value any more at this point ...

> +	addr = base;
> +	sync();
> +	*addr = save[last];
>  	return (maxsize);

... so this may corrupt memory.

cheers,
  Gerd
diff mbox

Patch

diff --git a/common/memsize.c b/common/memsize.c
index 0fb9ba5..d5827dd 100644
--- a/common/memsize.c
+++ b/common/memsize.c
@@ -1,5 +1,5 @@ 
 /*
- * (C) Copyright 2004
+ * (C) Copyright 2004-2014
  * Wolfgang Denk, DENX Software Engineering, wd@denx.de.
  *
  * SPDX-License-Identifier:	GPL-2.0+
@@ -31,7 +31,7 @@  long get_ram_size(long *base, long maxsize)
 	long           cnt;
 	long           val;
 	long           size;
-	int            i = 0;
+	int            i = 0, last;
 
 	for (cnt = (maxsize / sizeof(long)) >> 1; cnt > 0; cnt >>= 1) {
 		addr = base + cnt;	/* pointer arith! */
@@ -44,6 +44,7 @@  long get_ram_size(long *base, long maxsize)
 	addr = base;
 	sync();
 	save[i] = *addr;
+	last = i;
 	sync();
 	*addr = 0;
 
@@ -51,7 +52,7 @@  long get_ram_size(long *base, long maxsize)
 	if ((val = *addr) != 0) {
 		/* Restore the original data before leaving the function. */
 		sync();
-		*addr = save[i];
+		*addr = save[last];
 		for (cnt = 1; cnt < maxsize / sizeof(long); cnt <<= 1) {
 			addr  = base + cnt;
 			sync();
@@ -62,7 +63,9 @@  long get_ram_size(long *base, long maxsize)
 
 	for (cnt = 1; cnt < maxsize / sizeof(long); cnt <<= 1) {
 		addr = base + cnt;	/* pointer arith! */
+		sync();
 		val = *addr;
+		sync();
 		*addr = save[--i];
 		if (val != ~cnt) {
 			size = cnt * sizeof(long);
@@ -74,12 +77,19 @@  long get_ram_size(long *base, long maxsize)
 			     cnt < maxsize / sizeof(long);
 			     cnt <<= 1) {
 				addr  = base + cnt;
+				sync();
 				*addr = save[--i];
 			}
+			addr = base;
+			sync();
+			*addr = save[last];
 			return (size);
 		}
 	}
 
+	addr = base;
+	sync();
+	*addr = save[last];
 	return (maxsize);
 }