On Mon, Jun 11, 2018 at 03:42:54PM -0700, Cyril Novikov wrote: > On 6/11/2018 2:58 AM, David Gibson wrote: > > > 1181ec: f94004e1 ldr x1, [x7, #8] > > > The first instruction crashes because x7 is 4-byte aligned, but not 8-byte > > > aligned. > > > > > > I'm not sure where the bug is. Is fdt64_to_cpu() supposed to convert the > > > access to non-aligned and not doing its job properly? > > > > No, it's not supposed to do that, because it can't do that: the memory > > access happens *before* the value is passed to fdt64_to_cpu(). At the > > source level, anyway, obviously it's likely they'll get optimized > > together by the compiler. > > > > What's odd here, is that the memory reserve section should be aligned > > w.r.t the beginning othe device tree blob. So if you're hitting an > > unaligned access here, it suggests that either > > > > 1) your blob doesn't have an aligned memrsv section > > > > Which would definitely violate the dtb spec, and neither dtc nor > > libfdt should generate such a tree. > > > > 2) The code that loads your blob isn't aligning it to 64-bits > > > > That's not within the scope of the dtb spec as such, but aligning the > > load is definitely a best practice. > > Yes, it appears that our customer is using the "fdt_high" environment > variable in U-Boot, which makes U-Boot use the FDT in-place, and it's not > 8-byte aligned in the FIT image. We didn't even know that that variable > existed until now. > > So, I believe the bug is in U-Boot, which doesn't error out when someone > loads an unaligned FDT in-place. I would also blame their mkimage tool, if > we weren't generating the FIT image ourselves :) But I think mkimage has the > same bug. The problem is, it's hard to align the FIT image "data" property > value at 8 bytes, because libfdt doesn't provide any standard means to do > so. So maybe something to look at for libfdt people. Ah.. this is kind of a fundamental problem with the way FIT embeds whole device tree blobs as properties within other device tree blobs. Ultimately we want the embedded blob 8-byte aligned, but properties are only 4-byte aligned. It would be possible to use FDT_NOP tags to align those properties to an 8-byte boundary. But that could be fragile, because something altering the (outer) FIT as a general dtb might not preserve that alignment. So, I guess it would be useful to make libfdt more robust against unaligned blobs. -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson
Attachment:
signature.asc
Description: PGP signature
