Re: [SPAM] Re: Replace make-fit-atf.py with binman. Was: migrate u-boot-rockchip.bin to binman and generate an image for SPI

Simon Glass
2022-08-07T18:51:05Z

Hi Xavier,

On Tue, 2 Aug 2022 at 11:19, Xavier Drudis Ferran <xdrudis@tinet.cat> wrote:
>
> El Tue, Aug 02, 2022 at 06:41:40AM -0600, Simon Glass deia:
> >
> > It seems we need a lot more guidance here. I can help write more into
> > the packaging docs, perhaps:
> >
> > https://u-boot.readthedocs.io/en/latest/develop/package/binman.html#motivation
> >
> > Can you please suggest a few questions to answer?
> >
>
> I can try. But let me start by saying thank you for the guidance that was already there.
> I had read it and I still don't understand it together with some of what you said on the list.
> So how about...:
>
> - Exact criteria for what is a binary you take from external sources
> and and what is a packaged image that's binman responsability. So,
> TF-A is compiled externally, and the U-Boot build system just gets
> a BL31 env variable pointing at an bl31.elf or so. But u-boot.itb,
> despite being a FIT that includes parts of TF-A, maybe parts of
> tee.bin, some dtbs and binaries built by U-Boot itself and who
> knows if public keys or extra dtb overlays provided directly by the
> user, or something I can't think of now, it's not a binary from
> external sources but a U-Boot image that needs to be packaged by
> binman? For me there's some grey zone. Since make_fit_atf.py is in
> U-Boot then u-boot.itb is a binary, but since part of the contents
> are external, and mixed with U-Boot's own code then it's a packaged
> image. But then it's just part of the final packaged image that
> one can store in SPI, SD or wherever.
>
> - Scope ? how customizable needs the description of the image to be ?
>
> If a board meeds different images for booting from sd, spi and nand,
> should the *-u-boot.dts* provide the 3 images or one or ...?
>
> Must the image description recognize all possible kconfig options ?
> So should it look for splash source = spi and put a copy of some
> bmp or bmp.gz at some offset if the user opted for splash image in spi?
>
> Or that's a stretch and it's basically it should work for the
> default config for each board and users who change things should
> change the dts or package manually following the various READMEs ?
> (taking into account that any manual packaging risks the image
> description in dts no longer matching the image in case some soft is
> reading that)
>
> - Use ? Visibility ?
>
> From the motivation text one would think that users run
> make boardX_defconfig
> make
> BL31=bl31.elf binman
>
> But in fact currently it is make who calls binman. Is it just a temporary
> situation and the intent is for make just to build binaries and then
> binman will be called afterwards to package images ? Because binman not
> only packages what make built, it changes some properties of the dtb that
> was built by make, if I've understood it. In that sense it seems to be
> part of the build system not just a packager that comes afterwards.
>
> And the different parameters of binman are supposed to be set by make,
> and then more technical or is binman supposed to be used by the user
> to build some custom image too?
>
> - What about reuse ?
>
> the binman recipes go into u-boot.dts* files . But into which ? The
> SOC .dtsi ? the mach .dtsi ? the board .dtsi ? For example many
> boards include rk3399-u-boot.dtsi. Rk3399 can boot from spi, sd, or
> emmc (the same image can be used for emmc and sd). That's the
> bootrom. In theory SPL (a fatter SPL) could load U-Boot from nvme, I
> think, or USB, or serial, or network but I don't think that's
> implemented?
>
> So if you put the binman node in rk3399-u-boot.dtsi you could have
> it generate two images, one for sd/emmc and one for spi (plus
> intermediate images that people asked for to customize things). If
> you put it in rockchip-u-boot.dtsi then it can be more reused, but
> maybe then you need a format for nand too? And what if a board
> doesn't have emmc nor SD ? or doesn't have spi NOR? Should it modify
> the inherited binman node to disable one of the images, or just generate
> some unused image and call it a day ? Or you should put your binman
> subnodes in independent .dtsi files and carefully include the needed
> ones in each board -u-boot.dts ? Or you put it all in the most
> generic u-boot.dtsi you can and #ifdef away the parts that apply to
> each board/configuration ? So far we seem to prefer this last
> approach, but I don't know whether it was a plan or what.
>
>
> >
> > Because we have lots of scripts with no tests and it will proliferate
> > even more. Binman is data-driven, has tests and allows us to build
> > common functionality used by all SoCs.
> >
>
> Makes sense, tests are necessary, but some functionality may be common
> and some may be very specific for some arch or SOC or vendor. If all
> specific kirks also go into binman it can get a little complex. It can
> still be worth it, it's just that when some external entity decides
> things work some way (because they design a SOC or write a bootrom, or
> whatever) it'll be easier for them to give a shell script that
> implements what's needed than to understand and adapt binman. So
> that's a job for others to do possibly. So until someone does (like
> Quentin just did for rkspi) then we'll still have binman and whatever
> the external vendor came up with.
>
> One option would be to use binman just for generic things (padding,
> alignment, extraction, concatenation, calling external tools). And
> resign ourselves to keep having some external tools that make calls
> before calling binman, or that binman itself calls. I don't know.
>
> >
> > Have you looked at osfc binman talk from a few years ago?
> >
>
> No, or maybe so long ago that I forgot. I read what I could from
> doc/. I generally prefer text than video. But I may take a look, now
> that you mention it.
>
> > Binman is not to replace make. The dependency we are talking about is
> > between different images generated by Binman. Part of the problem is
> > that you know what you want, but it is a bit foggy to me.
> >
>
> We are currently building images that include images, so some files
> are both binaries and images, and that is not supported. If you
> support that in a general way (because binman seems to be intended to
> generalize a bit packaging) then it can become a little complex
> because new dependencies between images or sections arise. If you want
> to keep binman simpler, you could offload that to make. But you don't
> want it, I still don't know why.
>
> > To move forward, can I suggest you send a patch with a binman
> > description file containing what you want. Obviously it won't work,
> > due to the dependencies, but I can then figure out how to enable that
> > in binman. Can you try that?
> >
>
> I can help explaining my doubts because they're big and candid, and I
> could try to explain what is needed for Rock Pi 4, but I don't think I
> could explain it better than what Quentin and Jerome already did.
>
> I sent a dtsi with a binman description file
>
> https://lists.denx.de/pipermail/u-boot/2022-July/490068.html
>
> (you replied to that message, but maybe didn't read until the end ?)
>
> Your suggestion was to add ordering properties to binman. I guess we
> could, but I'm not sure that's very scalable or even very data driven.
> Its simpler than managing dependencies, I guess, so that's good.
>
> But I'm no longer sure that's what I want. Well I'm sure that's not
> what I want since Jerome pointed out that that would mistreat
> tee.bin. Maybe what I want is to keep using make_fit_atf.py to build
> u-boot.itb and then consider u-boot.itb a binary for binman, that's
> what I think Quentin already sent, before I messed it up by mentioning
> make_fit_atf.py at all. Maybe just remove the message make outputs
> saying CONFIG_USE_SPL_FIT_GENERATOR looks ugly ? Because that's what
> lead me to the rabbit hole...
>

I think there is a bit much agonising over things here. I'll see if I
can create a patch that addresses these points and send it out. Thank
you for writing it all up.

Regards,
Simon