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(-)