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. > > That all said, I'm still not quite sure how you're ending up in the > > place you're ending up. Which if/else paths in spl_fit_append_fdt() is > > your exact platform hitting, and where is what in memory? > > Is this a question for me or for Reuben ? For you, the person with the current failure. Please walk me through how / where that function is now failing. With address values (approximate if you can't get the exact ones). -- Tom
signature.asc
Description: PGP signature