On Tue, Oct 20, 2020 at 09:38:52AM -0500, Alex G. wrote:
> On 10/20/20 9:32 AM, Tom Rini wrote:
> > On Tue, Oct 20, 2020 at 04:29:36PM +0200, Marek Vasut wrote:
> > > On 10/20/20 4:07 PM, Tom Rini wrote:
> > > > On Tue, Oct 20, 2020 at 11:05:40AM +0200, Marek Vasut wrote:
> > > > > On 10/20/20 2:27 AM, Reuben Dowle wrote:
> > > > > > > > What assumptions? Any code that assumes 4 byte alignment will
> > > > > > > > also work
> > > > > > > on 8 byte alignment.
> > > > > > > >
> > > > > > > > Reverting is not the same as assuming ALIGN(...4) if incoming
> > > > > > > > data is not
> > > > > > > already aligned to 4 bytes (as was the case when I saw crashes).
> > > > > > >
> > > > > > > Can the incoming data _not_ be 4 byte aligned ?
> > > > > > > How can this be replicated ?
> > > > > >
> > > > > > In my case I have an offline signing process (separate from build
> > > > > > server to keep secure boot keys safe), and this runs a script which
> > > > > > also patches the main uboot device tree with some extra properties,
> > > > > > then updates main uboot dtb with kernel signature, then finally
> > > > > > updates the spl dtb with the uboot signature. I think when mkimage
> > > > > > patches the dtb with the signatures, this results in the alignment
> > > > > > issues (the unsigned bootloader direct from the uboot make process
> > > > > > does not experience this issue).
> > > > > >
> > > > > > Possibly using mkimage to add padding would also fix the alignment
> > > > > > issue I see at boot time.
> > > > > >
> > > > > > > > Interesting. I had not noticed the -B parameter previously. I
> > > > > > > > had originally
> > > > > > > fixed this issue on an older version of uboot that did not have
> > > > > > > that option,
> > > > > > > and later rebased the fix to newer uboot. I would need to do some
> > > > > > > testing to
> > > > > > > see if this would fix it as well.
> > > > > > >
> > > > > > > I believe this is the way to handle this if you are building a
> > > > > > > custom DT for U-
> > > > > > > Boot -- just make sure it has the correct parameters. I think
> > > > > > > this is also related
> > > > > > > to:
> > > > > > > 20a154f95b ("mkimage: fit: Do not tail-pad fitImage with external
> > > > > > > data")
> > > > > >
> > > > > > I will look into this, although unfortunately I am very busy with
> > > > > > other projects right now so can't retest th
> > > > >
> > > > > In that case, I would propose to revert this to fix existing boards in
> > > > > mainline, and if your tests are not successful, revisit this issue.
> > > > >
> > > > > I am rather sure what you are hitting is related to the mkimage patch
> > > > > above, there was a lengthy discussion on that topic before.
> > > >
> > > > This gets back to what I was saying earlier in this thread. But to
> > > > expand on it, we have been, but cannot, use the same variable for both
> > > > "this is where we have the DTB at runtime to use" and "this is where the
> > > > DTB happens to exist when we get here". For the case of "we copy the
> > > > device tree to $address", $address must be 8 bytes aligned. For the
> > > > case of "we use an externally provided DTB in place" I don't like the
> > > > idea of, and worry a lot about, assuming it's going to be 8 byte
> > > > aligned. But I can set that aside for the moment. That said, in that
> > > > second case we need to set $address to where the device tree is.
> > >
> > > I don't think I understand this paragraph at all ?
> >
> > OK. Maybe I can better explain it after you walk through how changing
> > the "copy the DTB to this address" to be 8 byte aligned is leading to
> > failure, as I ask below.
>
> (gdb) print gd->fdt_blob
> $13 = (const void *) 0xc01cf85c
> (gdb) x/4x gd->fdt_blob
> 0xc01cf85c: 0xc01b814c 0xedfe0dd0 0xa0670100 0x38000000
>
> Notice how u-boot thinks fdt_blob is at 0xc01cf85c. However it was actually
> loaded at 0xc01cf860, as evidenced by the doodfeed signature. This is with
> the 8-byte alignment.
Ah, thanks. So we're in this part of the code (conditions changed to results):
if (TRUE) {
debug("%s: cannot find FDT node\n", __func__);
/*
* U-Boot did not find a device tree inside the FIT image. Use
* the U-Boot device tree instead.
*/
if (TRUE)
memcpy((void *)image_info.load_addr, gd->fdt_blob,
fdt_totalsize(gd->fdt_blob));
So, our destination is what changed but gd->fdt_blob is 4 lower than the
correct value. Isn't that the problem? Or am I missing something
still? Thanks!
--
Tom
signature.asc
Description: PGP signature
