On Friday 30 December 2022 12:51:03 Simon Glass wrote:
Hi Pali,
On Fri, 30 Dec 2022 at 11:55, Pali Rohár pali@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 pali@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 pali@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-t...
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:
- 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?
- 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?
- Enable binman by default
- 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)
- 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...