[U-Boot] [PATCH v2 0/2] common/memsize.c: restore content of the base address

[Patrick Delaunay]

I detect a issue in get_ram_size() function when maxsize is available;
in this case, the base address is not restored.

Please, check if these 2 patch don't introduce regression on your board
because this code is really sensible.

Issue detected on my board (not yet up-streamed) with
- DDR base = 0xC0000000
- expected detected size = maxsize = 0x40000000
The loop is completely executed so the size is correctly detected,
but content of memory at address 0xC0000000 is not restored:
   I sill have 0xC0000000 = 0x0 at the end of the function.

In the correction (patch 2 of the serie), I restore this content
when no error is detected (for the last return).

NB: I also add a comment for the previous return to avoid regression
    for the case size<max_size
    because for this case, the higher bit of address are not used
    and the higher addresses are overlapped with
    the lower addresses (same physical content)
    => the original data at address 0x0 is already restore
       because *(base + size) and *base is physically the same
       physical address = ~(size-1) & logical address
       => address overlap detected with (val != ~cnt)
          *base was forced to 0
          but content for *(base + size) already restore

Test on Sandbox (v2018.01)
--------------------------

I test the issue and the correction in sandbox by
adding a debug command "test_ram_size" in cmd/mem.c which:
1/ fill memory buffer with random value
2/ use get_ram_size() on this buffer
3/ check buffer content modification


static int do_test_ram_size(cmd_tbl_t *cmdtp, int flag, int argc,
		       char * const argv[])
{
	unsigned int seed = rand();
	long *ptr = NULL;
	long size;
	unsigned int offset, expec, read;

	ptr = map_sysmem(MEM_TEST_BASE, MEM_TEST_SIZE);
	for (offset = 0; offset < MEM_TEST_SIZE / sizeof(long);
	     offset ++)
		*(ptr + offset) = rand();

	size = get_ram_size(ptr, MEM_TEST_SIZE);

	srand(seed);

	for (offset = 0; offset < MEM_TEST_SIZE / sizeof(long);
	     offset ++) {
		expec = rand();
		read = *(ptr + offset);
		if (read != expec)
			printf(" ERROR: %p <= %lx, %x, expected %x\n",
			       ptr + offset,
			       MEM_TEST_BASE + (offset * sizeof(long)),
			       read, expec);
	}
	unmap_sysmem(ptr);
	printf("get_ram_size(%x,%x)=%lx\n", MEM_TEST_BASE, MEM_TEST_SIZE, size);
	return 0;
}

U_BOOT_CMD(
	test_ram_size,	1,	1,	do_test_ram_size,
	"test memory",
	""
);

after the first patch: the error is detected at address 0x0,
the value become 0x0

  => test_ram_size
    ERROR: 00007f6b0018d008 <= 0, 0, expected 4080601
    get_ram_size(0,100000)=100000

  => test_ram_size
    ERROR: 00007f6b0018d008 <= 0, 0, expected 9a61a0f3
    get_ram_size(0,100000)=100000

after the second patch, all the memory is correctly restored

  => test_ram_size
    get_ram_size(0,100000)=100000

Test on real ARM target
-----------------------

My target is a ARMv7 Cortex A7 board not yet upstreamed with DDR.

I also use the same test by patching SPL:
+ initialize the DDR controller
+ write random content in all the DDR
+ use get_ram_size(0xC0000000, 0x40000000) on all the possible DDR range
+ check content of the DDR

Test 1, 1GB DDR (2 chipsx16bits => 32 bits access) @ 0xC0000000
  => detected size is 0x40000000 (1GB)
  => test OK memory is restored (but test failed without patch 2)

=> the memory is correctly restored for last case (size=max_size)

Test 2, I change the controller parameter to reduce the used bits for DDR
  (using one chip => 16 bits access and only 512MB)
  => detected size is 0x20000000 (overlap detected for 0xE0000000 address:
     same physical address 0x0 in DDR than 0xC0000000)
  => test OK memory is restored (test OK with or without patch 2)

=> the memory is already correctly restored for the overlap case
   (where I just add comment)


Changes in v2:
- Split patch in 2 part to more easily understood regression
- Solve issue for overlap case: don't restore saved value (invalid)
  and add comment to avoid to have this issue again

Patrick Delaunay (2):
  common/memsize.c: prepare get_ram_size update
  common/memsize.c: restore content of the base address

 common/memsize.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

-- 
2.7.4