From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tom Rini Subject: Re: [PATCH] libfdt: Add FDT alignment check to fdt_check_header() Date: Wed, 7 Apr 2021 14:39:50 -0400 Message-ID: <20210407183950.GV1310@bill-the-cat> References: <20210406190712.2118098-1-robh@kernel.org> <20210407180913.GU1310@bill-the-cat> Mime-Version: 1.0 Return-path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=konsulko.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=LDxPFWf8JiOL5+doXb2TbTEQhh7s9dnsjRSYIOEYkDY=; b=TtVvlEuVlgue9h+jvRH3Pc8KUYMnW2kwr2+Avp8kuQiPRrDKPR+jVKGvVZbwwtyKbo G0WRIqQcFM8eosFWcn+vne+Uj9CfWYPiFoUITCnoXMireWSyZ914oba5rEbLvihdV48u Mr0LMN1Eyms62gNbK7a7UsrwX0DdVuXrhWaZ0= Content-Disposition: inline In-Reply-To: List-ID: Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Simon Glass Cc: Rob Herring , Devicetree Compiler , Frank Rowand On Thu, Apr 08, 2021 at 06:36:53AM +1200, Simon Glass wrote: > Hi Tom & Rob, > > On Thu, 8 Apr 2021 at 06:09, Tom Rini wrote: > > > > On Thu, Apr 08, 2021 at 03:35:35AM +1200, Simon Glass wrote: > > > Hi Rob, > > > > > > On Wed, 7 Apr 2021 at 07:07, Rob Herring wrote: > > > > > > > > Only checking the FDT alignment in fdt_ro_probe_() means that > > > > fdt_check_header() can pass, but then subsequent API calls fail on > > > > alignment checks. Let's add an alignment check to fdt_check_header() so > > > > alignment errors are found up front. > > > > > > > > Cc: Tom Rini > > > > Cc: Frank Rowand > > > > Signed-off-by: Rob Herring > > > > --- > > > > For background, the new alignment check triggered a crash in the > > > > linux kernel. Yes, we should fix the error handling, but > > > > fdt_check_header() shouldn't tell us the FDT is valid only to fail > > > > later on. > > > > > > > > Maybe we should move the check instead, but fdt_ro_probe_() and > > > > fdt_check_header() already have a lot of the same checks. > > > > > > > > libfdt/fdt.c | 4 ++++ > > > > 1 file changed, 4 insertions(+) > > > > > > At present U-Boot uses a 4-byte alignment, so far as I know, so this > > > will break things. > > > > It's 8 byte, not 4 byte and I have nothing good to say about places that > > get by with 4-and-not-8 alignment. > > I am thinking of this in arch/arm/cpu/u-boot.lds : > > . = ALIGN(4); > > .image_copy_end : { > > At least for now, we use 4-byte alignment on 32-bit ARM, for example. Lets move that over to the U-Boot list and see what needs whacking where then, keeping in mind that it's where we use the DTB once it's in memory that needs 8 byte alignment, not where it might be placed in a blob for storage. -- Tom