From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nikita Kiryanov Date: Wed, 23 Jan 2013 12:47:49 +0200 Subject: [U-Boot] [PATCH 5/5] cm-t35: add support for loading splash image from NAND In-Reply-To: <20130122083703.45ceac28@lilith> References: <1356246228-26732-1-git-send-email-nikita@compulab.co.il> <1356246228-26732-6-git-send-email-nikita@compulab.co.il> <50D81891.1030503@myspectrum.nl> <50D96A59.2030305@compulab.co.il> <50DB0969.1020104@myspectrum.nl> <50E0520A.5090103@compulab.co.il> <20130122083703.45ceac28@lilith> Message-ID: <50FFBFD5.90800@compulab.co.il> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Albert, On 01/22/2013 09:37 AM, Albert ARIBAUD wrote: > Hi Nikita, > [...] >> Barring that, we should at least protect lcd.c from this issue by >> making some sort of check for affected targets, or maybe limit the >> possible values for splashimage... This issue makes it way too easy >> to accidentally break the boot process in a way that's hard to recover >> from. > > I suggest a few solutions: > > 1) enforce given load address alignment so that BMP header fields are > natively aligned, with a clear error message. Simple to implement, > difficut for users to understand and accept. Yes I agree that from a user point of view this looks terrible, which is why I prefer not to do something like this. > > 2) once the address provided by the user is known, if it is not > properly aligned, then the next properly aligned address should be > used, and the byte at given address should contain the offset from the > given address to the used address. This is a general solution that > works for any given load address, odd ones included: > > Given address: First bytes: Used address: > 10000000 2 x 'B' 'M' 10000002 > 10000001 1 'B' 'M' 10000002 > 10000002 'B' 'M' 10000002 > 10000003 3 x x 'B' 'M' 10000006 > 10000004 2 x 'B' 'M' 10000006 > ... > > Note that if the user address is constrained to be 4-byte-aligned, > then only the "2 x 'B' 'M'" case would apply. I think a simpler way to implement something like this is to just use modulo 4 to check alignment and fix the address dynamically; perhaps even fixing it in the environment. This is a localized approach though. We will have to do this from all the code paths that lead to a bmp header being probed in memory. I would prefer a more localized solution. > > 3) define an internal 'BMP holder' structure which contains a > two-byte prefix before the 'BMP file' header and data. This way, if > the overall structure is aligned, then the fields in the BMP header are > aligned too. > > 4) Build a time machine and tell the designers of the BMP header format, > in great inventive and colorful detail, what horrible things will happen > to them if they don't order and size their fields so that they naturally > land on aligned offsets from the header start. This solution gives the > best results IMO. The problem with 3 (and 4) is that it still doesn't protect the user from bricking their board by choosing a non-aligned address for their BMP. This might happen because the user: - is unaware of the dangers of choosing a non-aligned address - made a typo - relies on a script or program that runs under U-Boot to setup stuff - I"m sure there are other possibilities In terms of usability it's a *big* regression. If we do not actually prevent the user from setting splashimage to an unaligned address we should make sure that all usable addresses are safe. > > 5) if none the above (including 4) is feasible for some reason, then > use unaligned accessors for this BMP fields, with a Big Fat Comment > about why this is so. I think, once this feature is merged, I'll try to see if something nice can be done with an approach like this. For now I'll add a suggestion-#2-style check in lcd.c > > Note that all solutions except 2 (and 4) depend on the given address > being constrained in some way -- such a constraint does not seem > excessive to me. > > Amicalement, > -- Regards, Nikita.