Hi Pali, On Fri, 30 Dec 2022 at 13:12, Pali Rohár <p...@kernel.org> wrote: > > On Friday 30 December 2022 12:51:03 Simon Glass wrote: > > Hi Pali, > > > > On Fri, 30 Dec 2022 at 11:55, Pali Rohár <p...@kernel.org> wrote: > > > > > > On Friday 30 December 2022 11:43:44 Simon Glass wrote: > > > > Hi Pali, > > > > > > > > On Fri, 30 Dec 2022 at 10:06, Pali Rohár <p...@kernel.org> wrote: > > > > > > > > > > On Friday 30 December 2022 09:49:08 Simon Glass wrote: > > > > > > Hi Pali, > > > > > > > > > > > > On Fri, 30 Dec 2022 at 09:44, Pali Rohár <p...@kernel.org> wrote: > > > > > > > > > > > > > > On Friday 30 December 2022 10:41:47 Tom Rini wrote: > > > > > > > > On Fri, Dec 30, 2022 at 04:24:43PM +0100, Pali Rohár wrote: > > > > > > > > > On Friday 30 December 2022 10:21:04 Tom Rini wrote: > > > > > > > > > > > In this case it would be better to build u-boot-dts.bin > > > > > > > > > > > only by binman > > > > > > > > > > > (for all platforms) instead of cat-ing rules in Makefile. > > > > > > > > > > > > > > > > > > > > This would also be an easier path forward perhaps for > > > > > > > > > > making sure that > > > > > > > > > > the dtb is always 8 byte aligned? > > > > > > > > > > > > > > > > > > Well, no. With DTB the problem is that it is not put to the > > > > > > > > > correct > > > > > > > > > offset as can be specified in linker script. So moving this > > > > > > > > > code from > > > > > > > > > Makefile to binman also moves this problem to another > > > > > > > > > location. > > > > > > > > > 8 byte alignment is just subset of the "correct offset" > > > > > > > > > problem. > > > > > > > > > > > > > > > > Right, the high level answer is binman is intended to be the > > > > > > > > tool to > > > > > > > > assemble binaries, and has to deal with "make sure binary X is > > > > > > > > at offset > > > > > > > > Y, which also has a linker symbol for run-time references". > > > > > > > > > > > > > > Ok, if this tool has access to ELF/linker symbols (or will have in > > > > > > > future in case it does not have yet) then this problem could be > > > > > > > solved > > > > > > > here. > > > > > > > > > > > > It does have this access and already updates symbols in some cases. > > > > > > See [1]. > > > > > > [1] > > > > > > https://u-boot.readthedocs.io/en/latest/develop/package/binman.html#access-to-binman-entry-offsets-at-run-time-symbols > > > > > > > > > > I just do not see how to do it, but ok, maybe something more is > > > > > needed. > > > > > > > > > > > I am a little nervous about a complete move to binman in this > > > > > > area even for simple things like u-boot.bin, since it would set off > > > > > > yet another migration. > > > > > > > > > > Yes, it sounds like a big change. > > > > > > > > > > > But perhaps most boards don't actually use u-boot.bin anyway? > > > > > > > > > > I think that most boards _use_ u-boot.bin. Or wrap u-boot.bin into > > > > > some > > > > > own container (by mkimage). > > > > > > > > > > > Part of me thinks we should solve this in the .lds files, since > > > > > > otherwise we are blurring the line between building and packaging. > > > > > > > > > > Linker script files in any case would have to be adjusted / fixed to > > > > > align _end symbol. Without it ELF symbol would not be correct and so > > > > > obviously any solution depending on ELF symbols would not work... > > > > > > > > > > As I wrote recently, I proposed alternative solution without binman: > > > > > https://lore.kernel.org/u-boot/20221217235913.w7ihsktbplbp2j7z@pali/ > > > > > > > > I think that would work. It is a little like the BINARY_SIZE_CHECK > > > > thing we have. It is the closest thing to what we have. As you saw I > > > > was happy with your original 'trunc' solution too. > > > > > > > > After some more thought, perhaps the binman solution makes sense, so > > > > long as we do what you say above (make the _end symbol aligned). Of > > > > course binman can fix that up, but it feels more like a build thing > > > > than a packaging thing to me. > > > > > > Yes, this is build thing/issue, not packaging one. > > > > > > > It could be quite confusing for people > > > > to see a symbol change between build-time and run-time. > > > > > > Yes, symbol change is confusing. So I would propose to not change any > > > symbol between build and run time. > > > > > > > So the steps would be something like this: > > > > > > > > 1. Update the align before _end in each .lds to use a constant like > > > > #define END_ALIGN 8 > > > > 2. Update binman's dtb etypes to align to 8 bytes > > > > > > This is not enough. Some boards/platform may have stricter alignment > > > (e.g. to SD card sector size = 512 bytes). So binman should not align > > > image and instead of that, it should read _exact_ address of _end > > > symbol and put DTB etype to this address. Step 1. already ensures that > > > _end is aligned to 8 bytes. > > > > OK, we can do that I suppose, perhaps with a new binman property: > > > > u_boot: u-boot-nodtb { > > }; > > u-boot-dtb { > > align-to-sym = <&u_boot> ,"_end"; > > }; > > > > or using expanding just: > > > > u-boot { > > align-dtb-to-sym = <&u_boot> ,"_end"; > > }; > > > > (which we could perhaps make the default?) > > Something like that looks reasonable. And must be enabled by default. > > I'm not sure if calling it "align" is a good idea. Because if property > is in u-boot node then it it is "padding". And if that property is in > dtb node then it is "placing" binary to the absolute address. > > From reading binman.html documentation I see that for placing image at > absolute address there is already "offset" property. And for padding > there is "pad-after" property. What about consistency? > > So maybe 'pad-after-to-sym = "_end";' in u-boot node? > Or 'offset-from-sym = <&u_boot>, "_end";' in dtb node?
Yes that makes sense. I'll take a look at this at some point. > > > > > > > > 3. Add a common binman image for u-boot.bin (used by every board) > > > > > > It should be u-boot-dtb.bin (not u-boot.bin). At least this is the > > > current file name. (See this my patch series again, which aligns this > > > naming also for powerpc/mpc85xx). > > > > We changed this 6 years ago and I'm not keen on going back. > > > > ad1ecd2063d fdt: Build a U-Boot binary without device tree > > I just do not understand you because in that commit is exactly what I > wrote. In file u-boot-dtb.bin is u-boot binary with DTB and in file > u-boot-nodtb.bin is u-boot binary without DTB. > > So binman should take input files u-boot-nodtb.bin and DTB binary. And > should produce output file u-boot-dtb.bin. Is there any issue with it? Actually u-boot-dtb.bin is a hangover from that commit, left in to allow people to adjust. So I think we should remove creation of u-boot-dtb.bin > > > > > > > > 4. Enable binman by default > > > > 5. Drop the current 'cat' rules in Makefile, so that only u-boot and > > > > u-boot-nodtb.bin are produced > > > > > > (cat rule is only for u-boot-dtb.bin) > > > > > > > 6. Optionally consider moving .img files to binman also > > > > > > > > Regards, > > > > Simon > > > > > > With slightly modified/improved step 2. I agree that this improves > > > current situation. And should these issues. > > > > Regards, > > Simon > > > > > > [1] > > https://u-boot.readthedocs.io/en/latest/develop/package/binman.html#expanded-entries Regards, Simon