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

Sughosh Ganu
2023-06-21T04:26:50Z

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.

>
> 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.

-sughosh