[PATCH 1/4] tools: mkeficapsule: Add support for parsing capsule params from config file

Sughosh Ganu sughosh.ganu at linaro.org
Mon Dec 4 08:15:44 CET 2023 

hi Simon,

On Sat, 2 Dec 2023 at 00:02, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Sughosh,
>
> On Thu, 30 Nov 2023 at 23:39, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> >
> > hi Simon,
> >
> > On Thu, 30 Nov 2023 at 08:16, Simon Glass <sjg at chromium.org> wrote:
> > >
> > > Hi Sughosh,
> > >
> > > On Wed, 22 Nov 2023 at 00:40, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> > > >
> > > > hi Ilias,
> > > >
> > > > On Wed, 22 Nov 2023 at 13:06, Ilias Apalodimas
> > > > <ilias.apalodimas at linaro.org> wrote:
> > > > >
> > > > > Hi all,
> > > > >
> > > > > On Wed, 22 Nov 2023 at 07:23, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> > > > > >
> > > > > > hi Simon,
> > > > > >
> > > > > > On Wed, 22 Nov 2023 at 03:42, Simon Glass <sjg at chromium.org> wrote:
> > > > > > >
> > > > > > > Hi Sughosh,
> > > > > > >
> > > > > > > On Tue, 21 Nov 2023 at 00:02, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> > > > > > > >
> > > > > > > > Add support for specifying the parameters needed for capsule
> > > > > > > > generation through a config file, instead of passing them through
> > > > > > > > command-line. Parameters for more than a single capsule file can be
> > > > > > > > specified, resulting in generation of multiple capsules through a
> > > > > > > > single invocation of the command.
> > > > > > > >
> > > > > > > > The config file can then be passed to the mkeficapsule tool in such
> > > > > > > > manner
> > > > > > > >
> > > > > > > >  $ ./tools/mkeficapsule -f <path/to/the/config/file>
> > > > > > > >
> > > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> > > > > > > > ---
> > > > > > > >  tools/Kconfig              |  15 ++
> > > > > > > >  tools/Makefile             |   1 +
> > > > > > > >  tools/eficapsule.h         | 114 ++++++++++++
> > > > > > > >  tools/mkeficapsule.c       |  87 +++++----
> > > > > > > >  tools/mkeficapsule_parse.c | 352 +++++++++++++++++++++++++++++++++++++
> > > > > > > >  5 files changed, 538 insertions(+), 31 deletions(-)
> > > > > > > >  create mode 100644 tools/mkeficapsule_parse.c
> > > > > > >
> > > > > > > This patch keeps coming back :-)
> > > > > > >
> > > > > > > Can we not add multiple capsules in the binman description? Why do we
> > > > > > > need a new file format? How can binman decode images produced in this
> > > > > > > way?
> > > > > >
> > > > > > So as Tom mentions, this brings parity with respect to the other
> > > > > > capsule generation tool in EDKII that generates capsules. IIRC, this
> > > > > > is something which even Xilix was interested in, and Michal had kind
> > > > > > of gone through these patches earlier. Lastly, it would be good to
> > > > > > have support in U-Boot's mkeficapsule tool for generating a single
> > > > > > capsule file with multiple payloads, and having support for this
> > > > > > functionality helps in that goal.
> > > > > >
> > > > > > Also, you might have noticed that, since your objection to the last
> > > > > > series, I have removed putting this in binman. So now, this aspect of
> > > > > > the capsule generation would only be supported through the
> > > > > > command-line invocation of the tool.
> > > > >
> > > > > I think that overall the approach is sane. mkeficapsule is currently
> > > > > supported and compiled for distros, so the multiple payload support is
> > > > > useful. If we want to add support to binman, instead of rewriting this
> > > > > in python, we could just call that tool for parsing and creating
> > > > > capsules
> > > >
> > > > Given the amount of time these patches have been under review(also
> > > > number of iterations), I would request that this series be reviewed
> > > > and merged first. I think there is general consensus that there is
> > > > value to have this functionality in the mkeficapsule tool. If it is
> > > > deemed fit to support this through binman as well, that task can be
> > > > taken up separately. Thanks.
> > >
> > > The point you are missing is that it is the entire goal of 'skirting
> > > around' binman which is suspect.
> > >
> > > If there is a need to generate an output file from the build, we
> > > should support it in binman. If people start creating configuration
> > > files all over the place, then they are not using binman, right?
> > >
> > > I understand that there are pre-existing vendor-specific config files,
> > > etc. that the EFI thing is a grey area, but I cannot imagine that this
> > > patch would lead to a good outcome.
> > >
> > > The goal of binman is to bring order to the chaos of firmware
> > > packaging...we cannot do that if it is not actually used.
> > >
> > > So let's figure out what is missing from binman's capsule generation
> > > (multiple capsules? accept/reject capsules?) and how best to add it.
> >
> > I think I need to jog your memory back a bit. For context, I have
> > jotted down the points.
> >
> > * The mkeficapsule tool generates capsules in U-Boot.
> > * Currently, when the tool is invoked from the command-line, the
> > capsules are generated by passing the capsule parameters as cmd-line
> > options.
> > * I had earlier added support for generating the capsules as part of
> > U-Boot build, through binman. This support has been merged.
> > * I had followed these patches up with another series [1] which
> > generates capsules by parsing the capsule parameters through a config
> > file instead of cmd-line options.
> > * This series also had patches which were attempting to integrate this
> > functionality into binman [2].
> > * As part of reviewing the patch series, you had objected to adding
> > this support in binman, primarily because this way of specifying the
> > capsule parameters goes against the normal way of image description in
> > binman [3].
> > * I have described in this mail thread about why we need to have
> > support for generating capsules through config files [4].
>
> Thanks for that. Yes this fits with my memory, such as it is.
>
> I regard the tool as a binman tool, though, in the sense that binman
> is responsible for generating the images. For example, binman calls
> mkimage to handle FIT images. People are free to write a script to
> call mkimage manually, but the in-tree use of mkimage is supposed to
> be with binman.

No issues with supporting the functionality through binman. However, I
think that if binman is to be used as the encompassing packaging tool
which in turn calls other tools like mkimage or mkeficapsule, it
should make allowances for accepting formats, or ways of describing
payloads that might not sit with it's core design of "each image is a
DT node".

>
> >
> > So, in essence, this functionality is needed to be added to the tool.
> > I have earlier tried integrating this in binman, but that was
> > rejected. So, the way I see this moving ahead is to first add support
> > for this feature in the tool, and then see if this can also be added
> > in binman. As I mentioned earlier, I am fine with this functionality
> > not getting integrated with binman, if this contravenes the idea of
> > describing images in binman, and if no exceptions can be made on that
> > regard. But please do understand that no attempt is being made at
> > 'skirting around binman'. In fact, I had worked on your earlier
> > objection of using absolute paths in testing this functionality in
> > binman. But then you had put the other objection of how this does not
> > follow the idea or concept of image description in binman. Hence this
> > approach.
>
> It would be easier to talk this through f2f, but I will make an attempt here.
>
> The goal here is for people to be able to create an EFI capsule (or
> more than one?) automatically as part of the build.

That is one of the goals, yes. But the other goal is also to add
functionality to the base capsule generation tool which extends the
tool to support generating capsules through the config file. I would
be more than happy if this can be plumbed in as part of the build.


>  Since the config
> is in the binman description, putting it somewhere else doesn't help
> anyone. It will lead to .cfg files in U-Boot and Binman will just be a
> side show. That is my concern.

Okay. But I don't think that is the main issue here. For example, we
can require the .cfg files to be placed in the board dir, similar to
how the capsule keys are placed. The issue here is that can binman
make allowances(IMO it should) to have the capsule payloads be
specified through a .cfg file parameter, instead of the usual way of
specifying it as a DT node. I think if this allowance can be made, it
solves all the issues here.

>
> I would really like to see a capsule generated for sandbox so I can
> understand this better.

I will work on the capsule generation for sandbox. I will generate
signed capsules, as you suggested in this mail thread as part of the
sandbox U-Boot build. However, this will be using the currently merged
capsule generation logic, where all the capsule parameters, it's
payload included, are specified in binman nodes. I suspect this won't
give you a picture of capsule generation through config files.

>
> Anyway, I hope that makes sense? If not, I think we should discuss it
> f2f or VC or somehow.

Sure. I will be happy to have a VC anytime if you think that would
help. Let me know and we can sync up over IRC. Thanks.

-sughosh

>
> Regards,
> Simon
>
> >
> > -sughosh
> >
> > [1] - https://lore.kernel.org/u-boot/20230908120002.29851-1-sughosh.ganu@linaro.org/
> > [2] - https://lore.kernel.org/u-boot/20230908120002.29851-4-sughosh.ganu@linaro.org/
> > [3] - https://lore.kernel.org/u-boot/CAPnjgZ3Zi3hmQg7d5+0461hRK+5O_V89PY=QxMwQarhhqUW6VQ@mail.gmail.com/
> > [4] - https://lore.kernel.org/u-boot/CADg8p94EvJc6HNKvk2_XWaDb97u28vGfZasnv2je4yJw1k860Q@mail.gmail.com/
> >
> > >
> > > >
> > > > -sughosh
> > > >
> > > > >
> > > > > Thanks
> > > > > /Ilias
> > > > > >
> > > > > > >
> > > > > > > Also, could we get sandbox to produce one EFI capsule as part of the
> > > > > > > normal build? I think that discussion trailed off.
> > > > > >
> > > > > > Yes, apologies for missing out on this. Slipped my mind. Would you
> > > > > > want, say, all the non-signed capsules to be generated as part of the
> > > > > > sandbox build?
> > > > > >
> > > > > > -sughosh
> > >
> > > Regards,
> > > Simon

More information about the U-Boot mailing list