Re: [U-Boot] [PATCH] sunxi: Fix buggy sun6i/sun8i DRAM size detection logic

Hi,

On 24-12-14 17:58, Siarhei Siamashka wrote:

After reboot, reset or even short power off, DRAM typically retains the old stale data for some period of time (for this type of memory, the bits of data are stored in slowly discharging capacitors).

The current sun6i/sun8i DRAM size detection logic, which is inherited from the Allwinner code, relies on using a large magic signature with the hope that it is unique enough and unlikely to ever accidentally match this leftover garbage data in RAM. But this approach is inherently unsafe, as can be demonstrated using the following test program:

/***** A testcase for reproducing the problem ******/ #include <stdlib.h> #include <stdio.h> #include <stdint.h>

void main(int argc, char *argv[]) { size_t size, i; uint32_t *buf; /* Allocate the buffer */ if (argc < 2 || !(size = (size_t)atoi(argv[1]) * 1048576) || !(buf = malloc(size))) { printf("Need buffer size in MiB as a cmdline argument\n"); exit(1); } /* Fill it with the Allwinner DRAM "magic" values */ for (i = 0; i < size / 4; i++) buf[i] = 0xaa55aa55 + ((uintptr_t)&buf[i] / 4) % 64; /* Try to reboot */ system("reboot"); /* And wait */ for (;;) {} } /***************************************************/

If this test program is run on the device (giving it a large chunk of memory), then the DRAM size detection logic in u-boot gets confused after reboot and fails to initialize DRAM properly.

A better approach is not to rely on luck and abstain from making any assumptions about the properties of the leftover garbage data in RAM. Instead just use a more reliable code for testing whether two different addresses refer to the same memory location.

Signed-off-by: Siarhei Siamashka siarhei.siamashka@gmail.com

Thanks for the patch. I've run various tests and it works well, queued up for merging in u-boot-sunxi/next .

Regards,

Hans

---

Done only very limited testing on MSI Primo81 tablet (Allwinner A31s), which is currently a rather impractical device for doing any sun6i code development due to having no access to the serial console, USB or other convenient ways to interact with the device.

It might be a good idea to backup/restore the data in RAM when doing this check in the code.

Using the standard u-boot 'get_ram_size' function could be also an option to replace the loops and simplify the sun6i/sun8i dram code in the future. The only inconvenience is that 'get_ram_size' returns 'size' instead of 'log2(size)'. This could be probably resolved by introducing a new 'get_ram_size_log2' common function. But since I don't have any Allwinner A23/A31 devboard for testing/debugging, don't expect me to do this work any time soon, or ever at all.

arch/arm/cpu/armv7/sunxi/dram_sun6i.c | 1 - arch/arm/cpu/armv7/sunxi/dram_sun8i.c | 1 - arch/arm/include/asm/arch-sunxi/dram.h | 22 ++++++---------------- 3 files changed, 6 insertions(+), 18 deletions(-)

diff --git a/arch/arm/cpu/armv7/sunxi/dram_sun6i.c b/arch/arm/cpu/armv7/sunxi/dram_sun6i.c index 4675c48..4518b80 100644 --- a/arch/arm/cpu/armv7/sunxi/dram_sun6i.c +++ b/arch/arm/cpu/armv7/sunxi/dram_sun6i.c @@ -377,7 +377,6 @@ unsigned long sunxi_dram_init(void) MCTL_CR_BANK(1) | MCTL_CR_RANK(1));

/* Detect and set page size */

• mctl_mem_fill(); for (columns = 7; columns < 20; columns++) { if (mctl_mem_matches(1 << columns)) break;

diff --git a/arch/arm/cpu/armv7/sunxi/dram_sun8i.c b/arch/arm/cpu/armv7/sunxi/dram_sun8i.c index df9ff1f..ebba53b 100644 --- a/arch/arm/cpu/armv7/sunxi/dram_sun8i.c +++ b/arch/arm/cpu/armv7/sunxi/dram_sun8i.c @@ -289,7 +289,6 @@ unsigned long sunxi_dram_init(void) writel(0x000310f4 | MCTL_CR_PAGE_SIZE(page_size), &mctl_com->cr); setbits_le32(&mctl_com->swonr, 0x0003ffff);

• mctl_mem_fill();
  

  for (rows = 11; rows < 16; rows++) { offset = 1 << (rows + columns + bus); if (mctl_mem_matches(offset))

diff --git a/arch/arm/include/asm/arch-sunxi/dram.h b/arch/arm/include/asm/arch-sunxi/dram.h index 8d78029..7ff43e6 100644 --- a/arch/arm/include/asm/arch-sunxi/dram.h +++ b/arch/arm/include/asm/arch-sunxi/dram.h @@ -24,8 +24,6 @@ #include <asm/arch/dram_sun4i.h> #endif

-#define MCTL_MEM_FILL_MATCH_COUNT 64

• unsigned long sunxi_dram_init(void);

  /*

@@ -42,24 +40,16 @@ static inline void mctl_await_completion(u32 *reg, u32 mask, u32 val) }

/*

• • Fill beginning of DRAM with "random" data for mctl_mem_matches()
• */

-static inline void mctl_mem_fill(void) -{

• int i;
• for (i = 0; i < MCTL_MEM_FILL_MATCH_COUNT; i++)

• writel(0xaa55aa55 + i, CONFIG_SYS_SDRAM_BASE + i * 4);
  

-}

-/*

• Test if memory at offset offset matches memory at begin of DRAM

*/ static inline bool mctl_mem_matches(u32 offset) {

• return memcmp((u32 *)CONFIG_SYS_SDRAM_BASE,

•       (u32 *)(CONFIG_SYS_SDRAM_BASE + offset),
  

•       MCTL_MEM_FILL_MATCH_COUNT * 4) == 0;
  

• /* Try to write different values to RAM at two addresses */

• writel(0, CONFIG_SYS_SDRAM_BASE);

• writel(0xaa55aa55, CONFIG_SYS_SDRAM_BASE + offset);

• /* Check if the same value is actually observed when reading back */

• return readl(CONFIG_SYS_SDRAM_BASE) ==

•       readl(CONFIG_SYS_SDRAM_BASE + offset);
  

  }

  #endif /* _SUNXI_DRAM_H */