From: Siarhei Siamashka <siarhei.siamashka@gmail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 11/14] sunxi: Fill memory before comparing it when doing dram init on sun6i
Date: Mon, 22 Dec 2014 09:39:04 +0200 [thread overview]
Message-ID: <20141222093904.6a14dede@i7> (raw)
In-Reply-To: <549458C8.8040506@redhat.com>
On Fri, 19 Dec 2014 17:56:40 +0100
Hans de Goede <hdegoede@redhat.com> wrote:
> Hi,
>
> On 19-12-14 11:08, Siarhei Siamashka wrote:
> > On Thu, 18 Dec 2014 19:12:13 +0000
> > Ian Campbell <ijc@hellion.org.uk> wrote:
> >
> >> On Tue, 2014-12-16 at 21:31 +0100, Hans de Goede wrote:
> >>> The sun8i boot0 code fills the DRAM with a "random" pattern before comparing
> >>> it at different offsets to do columns, etc. detection. The sun6i boot0 code
> >>> does not do it, but it seems like a good idea to do this regardless.
> >>
> >> Is this the right way round? The existing sun6i code (which you are
> >> moving here) seems to _rely_ on something having written a useful
> >> pattern, which I would have assumed to have been boot0. Or else how does
> >> it work now? Chance?
> >
> > It appears that this code is just trying to find the first address line,
> > which is not connected anywhere. If the address line is not connected,
> > then having the corresponding bit set or clear in the memory address
> > does not matter and we are effectively accessing the same location.
>
> Correct.
>
> > Both sun6i and sun8i code is incorrect and only works because of relying
> > on luck.
> >
> > The sun6i code is incorrect because it is just reading memory without
> > initializing it at all. Probably relying on having some unique garbage
> > there in a natural way.
> >
> > The newer sun8i code is still incorrect because it is not safeguarding
> > against accidentally encountering the same test pattern at an unrelated
> > memory location. Yes, the probability is extremely low, but still not
> > zero.
>
> The probability is so low that it really does not matter, so I've no intention
> to fix this.
"If anything bad can happen, it probably will" :-)
There are many real life examples, when something supposedly almost
impossible has in fact happened. Like the following story:
http://ixbtlabs.com/articles2/magia-chisel/
In a nutshell, the designers of some old CD/CD-RW drives just did not
bother to ensure that the written user data can be never confused
with the service information (sync headers) on disk. The probability
of encountering something that resembles a sync header was considered
to be way too low. Well, until somebody has encountered a hardware
glitch and a sync header fragment got mistakenly read from the drive
as part of some file. Now this corrupted file can wreak havoc if
written to a CD disk, because the false sync header in it is getting
interpreted as an actual sync header. Oops.
Anyway, this is a bug in the sun6i/sun8i dram code. And the submitted
patch appears to be only a partial/incomplete fix. A strong NAK from
me.
--
Best regards,
Siarhei Siamashka
next prev parent reply other threads:[~2014-12-22 7:39 UTC|newest]
Thread overview: 54+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-12-16 20:31 [U-Boot] [PATCH 01/14] sun6i: s/SUNXI_GPL0_R_P2WI/SUN6I_GPL0_R_P2WI/ Hans de Goede
2014-12-16 20:31 ` [U-Boot] [PATCH 02/14] sunxi: Add support for the rsb (Reduced Serial Bus) Hans de Goede
2014-12-17 2:22 ` Chen-Yu Tsai
2014-12-17 12:56 ` Hans de Goede
2014-12-18 18:57 ` Ian Campbell
2014-12-18 18:59 ` Ian Campbell
2014-12-19 15:26 ` Hans de Goede
2014-12-16 20:31 ` [U-Boot] [PATCH 03/14] sunxi: axp221: Add axp223 support Hans de Goede
2014-12-17 2:34 ` Chen-Yu Tsai
2014-12-18 10:44 ` Hans de Goede
2014-12-18 19:00 ` Ian Campbell
2014-12-16 20:31 ` [U-Boot] [PATCH 04/14] sunxi: axp221: Add Kconfig help and sane defaults for typical ldo usage Hans de Goede
2014-12-18 19:02 ` Ian Campbell
2014-12-16 20:31 ` [U-Boot] [PATCH 05/14] sunxi: axp221: Make dcdc1 voltage configurable Hans de Goede
2014-12-18 19:03 ` Ian Campbell
2014-12-16 20:31 ` [U-Boot] [PATCH 06/14] sunxi: axp221: Explicitly turn off unused voltages Hans de Goede
2014-12-18 19:04 ` Ian Campbell
2014-12-16 20:31 ` [U-Boot] [PATCH 07/14] sunxi: axp221: Disable dcdc4 on sun8i (A23) Hans de Goede
2014-12-18 19:05 ` Ian Campbell
2014-12-16 20:31 ` [U-Boot] [PATCH 08/14] sun6i: Add a sigma_delta_enable paramter to clock_set_pll5() Hans de Goede
2014-12-18 19:07 ` Ian Campbell
2014-12-19 10:02 ` Siarhei Siamashka
2014-12-16 20:31 ` [U-Boot] [PATCH 09/14] sun6i: Add k and m parameters " Hans de Goede
2014-12-18 19:07 ` Ian Campbell
2014-12-19 10:03 ` Siarhei Siamashka
2014-12-19 16:40 ` Hans de Goede
2014-12-22 7:25 ` Siarhei Siamashka
2014-12-16 20:31 ` [U-Boot] [PATCH 10/14] sunxi: Move await_completion dram helper to dram.h Hans de Goede
2014-12-18 19:08 ` Ian Campbell
2014-12-19 10:06 ` Siarhei Siamashka
2014-12-19 16:42 ` Hans de Goede
2014-12-22 7:28 ` Siarhei Siamashka
2014-12-16 20:31 ` [U-Boot] [PATCH 11/14] sunxi: Fill memory before comparing it when doing dram init on sun6i Hans de Goede
2014-12-18 19:12 ` Ian Campbell
2014-12-19 10:08 ` Siarhei Siamashka
2014-12-19 16:56 ` Hans de Goede
2014-12-22 7:39 ` Siarhei Siamashka [this message]
2014-12-19 16:55 ` Hans de Goede
2014-12-22 14:19 ` Ian Campbell
2014-12-22 14:32 ` Siarhei Siamashka
2014-12-22 14:34 ` Ian Campbell
2014-12-22 15:47 ` Hans de Goede
2014-12-16 20:31 ` [U-Boot] [PATCH 12/14] sunxi: Use memcmp for mctl_mem_matches Hans de Goede
2014-12-18 19:13 ` Ian Campbell
2014-12-16 20:31 ` [U-Boot] [PATCH 13/14] sun8i: Add dram initialization support Hans de Goede
2014-12-18 11:12 ` Chen-Yu Tsai
2014-12-18 19:17 ` Ian Campbell
2014-12-19 16:51 ` Hans de Goede
2014-12-19 10:20 ` Siarhei Siamashka
2014-12-19 17:05 ` Hans de Goede
2014-12-22 7:43 ` Siarhei Siamashka
2014-12-16 20:31 ` [U-Boot] [PATCH 14/14] sun8i: Add defconfig for Ippo_q8h v1.2 Hans de Goede
2014-12-18 19:19 ` Ian Campbell
2014-12-18 18:56 ` [U-Boot] [PATCH 01/14] sun6i: s/SUNXI_GPL0_R_P2WI/SUN6I_GPL0_R_P2WI/ Ian Campbell
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=20141222093904.6a14dede@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox