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

Sughosh Ganu
2023-06-26T12:14:19Z

hi Simon,

On Mon, 26 Jun 2023 at 14:38, Simon Glass <sjg@chromium.org> wrote:
>
> Hi Sughosh,
>
> On Wed, 21 Jun 2023 at 05:26, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> >
> > hi Simon,
> >
> > On Mon, 19 Jun 2023 at 18:07, Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Sughosh,
> > >
> > > On Thu, 15 Jun 2023 at 17:25, Sughosh Ganu <sughosh.ganu@linaro.org> wrote:
> > > >
> > > > hi Simon,
> > > >
> > > > On Thu, 15 Jun 2023 at 14:44, Simon Glass <sjg@chromium.org> wrote:
> > > > >
> > > > > Hi Sughosh,
> > > > >
> > > > > On Tue, 13 Jun 2023 at 11:39, Sughosh Ganu <sughosh.ganu@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@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.

-sughosh