[PATCH 5/7] Makefile: Add a target for building capsules

[Sughosh Ganu]

hi Simon,

On Mon, 26 Jun 2023 at 17:43, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
>
> hi Simon,
>
> On Mon, 26 Jun 2023 at 14:38, Simon Glass <sjg at chromium.org> wrote:
> >
> > Hi Sughosh,
> >
> > On Wed, 21 Jun 2023 at 05:26, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> > >
> > > hi Simon,
> > >
> > > On Mon, 19 Jun 2023 at 18:07, Simon Glass <sjg at chromium.org> wrote:
> > > >
> > > > Hi Sughosh,
> > > >
> > > > On Thu, 15 Jun 2023 at 17:25, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> > > > >
> > > > > hi Simon,
> > > > >
> > > > > On Thu, 15 Jun 2023 at 14:44, Simon Glass <sjg at chromium.org> wrote:
> > > > > >
> > > > > > Hi Sughosh,
> > > > > >
> > > > > > On Tue, 13 Jun 2023 at 11:39, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> > > > > > >
> > > > > > > Add a target for building EFI capsules. The capsule parameters are
> > > > > > > specified through a config file, and the path to the config file is
> > > > > > > specified through CONFIG_EFI_CAPSULE_CFG_FILE. When the config file is
> > > > > > > not specified, the command only builds tools.
> > > > > > >
> > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> > > > > > > ---
> > > > > > >  Makefile | 9 +++++++++
> > > > > > >  1 file changed, 9 insertions(+)
> > > > > > >
> > > > > > > diff --git a/Makefile b/Makefile
> > > > > > > index 10bfaa52ad..96db29aa77 100644
> > > > > > > --- a/Makefile
> > > > > > > +++ b/Makefile
> > > > > > > @@ -1151,6 +1151,15 @@ dtbs: dts/dt.dtb
> > > > > > >  dts/dt.dtb: u-boot
> > > > > > >         $(Q)$(MAKE) $(build)=dts dtbs
> > > > > > >
> > > > > > > +quiet_cmd_mkeficapsule = MKEFICAPSULE $@
> > > > > > > +cmd_mkeficapsule = $(objtree)/tools/mkeficapsule $@
> > > > > > > +
> > > > > > > +PHONY += capsule
> > > > > > > +capsule: tools
> > > > > > > +ifneq ($(CONFIG_EFI_CAPSULE_CFG_FILE),"")
> > > > > > > +       $(call cmd,mkeficapsule)
> > > > > > > +endif
> > > > > > > +
> > > > > > >  quiet_cmd_copy = COPY    $@
> > > > > > >        cmd_copy = cp $< $@
> > > > > > >
> > > > > > > --
> > > > > > > 2.34.1
> > > > > > >
> > > > > >
> > > > > > We should be using binman to build images...you seem to be building
> > > > > > something in parallel with that. Can you please take a look at binman?
> > > > >
> > > > > Again, I had explored using binman for this task. The one issue where
> > > > > I find the above flow better is that I can simply build my payload
> > > > > image(s) followed by 'make capsule' to generate the capsules for
> > > > > earlier generated images. In it's current form, I don't see an easy
> > > > > way to enforce this dependency in binman when I want to build the
> > > > > payload followed by generation of capsules. I did see the mention of
> > > > > encapsulating an entry within another dependent entry, but I think
> > > > > that makes the implementation more complex than it ought to be.
> > > > >
> > > > > I think it is much easier to use the make flow to generate the images
> > > > > followed by capsules, instead of tweaking the binman node to first
> > > > > generate the payload images, followed by enabling the capsule node to
> > > > > build the capsules. If there is an easy way of enforcing this
> > > > > dependency, please let me know. Thanks
> > > >
> > > > Can you share your explorations? I think the capsule should be created
> > > > as part of the build, if enabled. Rather than changing the input
> > > > files, binman should produce new output files.
> > >
> > > This is an issue of handling dependencies in binman, and not changing
> > > input files. We do not have support for telling binman "build/generate
> > > this particular image first before you proceed to build the capsules
> > > using the earlier built images". I am not sure if this can be done in
> > > a generic manner in binman, so that irrespective of the image being
> > > generated, it can be specified to build capsules once the capsule
> > > input images have been generated.
> >
> > I'm just not sure what you are getting out here.
> >
> > See INPUTS-y for the input files to binman. Then binman uses these to
> > generate output files. It does not mess with the input files, nor
> > should it. Please read the top part of the Binman motivation to
> > understand how all this works.
> >
> > At the risk of repeating myself, we want the Makefile to focus on
> > building U-Boot, with Binman handling the laterprocessing of those
> > files. Binman may run as part of the U-Boot build, and/or can be run
> > later, with the input files.
> >
> > >
> > > >
> > > > We are trying to remove most of the output logic in Makefile. It
> > > > should just be producing input files for binman.
> > >
> > > I understand. However, like I mentioned above, as of now, we don't
> > > have a way of handling dependencies in binman, at least in a generic
> > > manner. Once this support gets added, I know that it would be trivial
> > > to add support for building capsules in binman.
> >
> > What dependencies do you need? Please describe it in a simple manner
> > so I can understand. It cannot involve change the binman input files.
>
> Consider the following scenarios.
>
> One board, say Board A uses fip.bin as the input file(payload) for
> generating the capsule file. The fip.bin is being generated through
> binman. A binman entry is also added for generating the capsule(say
> fip.capule). Now, binman has to generate fip.bin *and subsequently*
> fip.capsule, as the capsule file will contain fip.bin as it's
> payload(input).
>
> Second Board B, uses u-boot.itb, which is a FIT image, as the input
> file for generating the capsule. The u-boot.itb is being generated
> through binman, and so is the capsule. But binman needs to build the
> u-boot.itb *before* it generates the corresponding capsule file, which
> uses u-boot.itb as the capsule payload.
>
> There can be multiple such examples of input files being generated by
> binman, followed by the capsule getting generated by binman. How do I
> specify this dependency in binman -- build/generate the input file
> first, and then use that files in generating the capsule.

Can you confirm if the above dependencies can be handled in binman
currently. If not, I'd suggest you remove your Nak for patch 6 of this
series [1]. Like I mentioned earlier, if there is a means of
specifying dependencies for generating images in binman, moving the
capsule generation to binman will not be a difficult task.

Also, can you go through the other set of patches in V2, specifically
the ones where putting the public key ESL into the dtb is being done
through binman.

-sughosh

[1] - https://lists.denx.de/pipermail/u-boot/2023-June/521103.html