All of lore.kernel.org
 help / color / mirror / Atom feed
From: Siarhei Siamashka <siarhei.siamashka@gmail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] sunxi: Fix buggy sun6i/sun8i DRAM size detection logic
Date: Wed, 24 Dec 2014 19:39:33 +0200	[thread overview]
Message-ID: <20141224193933.49fe67d4@i7> (raw)
In-Reply-To: <1419440297-29503-1-git-send-email-siarhei.siamashka@gmail.com>

On Wed, 24 Dec 2014 18:58:17 +0200
Siarhei Siamashka <siarhei.siamashka@gmail.com> 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>
> ---
> 
> 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.

Just noticed that there is actually '__ilog2' function in u-boot. This
makes it easier to switch the sun6i/sun8i dram code to using the
standard 'get_ram_size' function.

-- 
Best regards,
Siarhei Siamashka

  reply	other threads:[~2014-12-24 17:39 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-24 16:58 [U-Boot] [PATCH] sunxi: Fix buggy sun6i/sun8i DRAM size detection logic Siarhei Siamashka
2014-12-24 17:39 ` Siarhei Siamashka [this message]
2014-12-25  2:07   ` Siarhei Siamashka
2014-12-25 10:14     ` Hans de Goede
2014-12-25 16:13       ` Siarhei Siamashka
2014-12-25 22:55         ` Hans de Goede
2014-12-27 17:05 ` Hans de Goede

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141224193933.49fe67d4@i7 \
    --to=siarhei.siamashka@gmail.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.