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

[Ilias Apalodimas]

[...]

> > > > > The order of operation is supposed to be:
> > > > >
> > > > > 1. Various projects used to build their ouputs (e.g. TF-A)
> > > > > 2. Makefile used to build U-Boot:
> > > > > 2a. The build produces a set of files which serve as inputs to binman (INPUTS-y)
> > > > > 2b. Binman runs on INPUTS-y, picking up all the bits and creating the
> > > > > final firmware image(s)
> > > > > 3. If necessary, separate from the U-Boot build, binman can be used
> > > > > separately to do signing or whatever is needed on the final image(s)
> > > > >
> > > > > I understand that the public key is available in a CONFIG, so it
> > > > > should be possible to embed it in the build as input, either as a
> > > > > .dtsi build using 2a, or as a binary file pulled in by binman in 2b.
> > > >
> > > > Using a dtsi would mean that every platform which wants to enable
> > > > capsule authentication would need to add a signature node to it's
> > > > dtsi. Instead, is it not simpler to just generate a dtbo and merge it
> > > > with the dtb being generated. That is what was being done in V1 [1].
> > >
> > > Why is it simpler? The .dtsi is where we are supposed to put
> > > devicetree properties. It seems a lot harder (to me) to add it later.
> > > It could be a #include, or even just put it in the .dtsi if it is the
> > > same for all boards?
> >
> > I was referring to the solution in V1 of the patch series [1]. All
> > that was needed there was the path to the public key ESL file, which
> > was being provided through the CONFIG_EFI_CAPSULE_ESL_FILE. The
> > embed_capsule_key.sh would take care of doing what you are suggesting
> > to be done via dtsi. When adding a node to the dtsi, the user will
> > still have to add a node to the dtsi and include the contents of the
> > ESL file. All that was being automated through the script in V1.
>
> Instead of all that complexity, can you check the irc where Kwiboo
> describes how to add a .dtsi fragment containing that CONFIG
>
> BTW the format you are using in the dtb looks to be binary. Have you
> thought about using real properties and values like the existing
> public key mechanism? Or is that binary format required by EFI?
>
> >
> > >
> > > > For your suggestion to pull it in as a binary file in binman, that
> > > > still does not fix the issue of not changing INPUTS-y.
> > > >
> > > > If you ask me, the embedding of the public-key into the dtb is not a
> > > > task suitable for binman. Why? Because this task is actually changing
> > > > one of the INPUTS-y file that feeds into binman. And yes, we can
> > > > generate a different set of files, like u-boot-capsule.dtb and
> > > > u-boot-capsule.bin -- implementing that is not at all difficult. But
> > > > like I had highlighted earlier, and also explained by Ilias, we
> > > > already have platforms that use capsule updates and which use
> > > > u-boot.dtb and u-boot.bin. Also, platforms would not want a separate
> > >
> > > Those platforms should change, IMO. But how can this be, when this
> > > functionality has not yet been added to U-Boot?
> > >
> > > > set of files, one for normal boot, one when using capsules. So I think
> > > > it is imperative that we generate the same set of files irrespective
> > > > of whether a platform enables capsule updates. So a proper design
> > > > would be to add/embed the public key into the dtb as the dtb is
> > > > getting built. Again, this is what was being done in V1.
> > >
> > > I completely disagree with this. A capsule update is not the same as
> > > the vanilla board build / binary. Please can you just give up on this
> > > idea? Many platforms generate their output in separate files, e.g. see
> > > u-boot-rockchip.bin - it just does not make sense to change the built
> > > binary after it is built.
> >
> > I completely agree with your last statement. Which is why I said that
> > if we want to use the same files which are otherwise
> > generated(u-boot.dtb, u-boot.bin), binman is not the right way to add
> > the public key to the platform's dtb -- that should be done when the
> > dtb is being built.
>
> OK, so perhaps that is some progress.
>
> My second point (perhaps lost above) is that the capsule file should
> be called u-boot-capsule.bin or something like that, not u-boot.bin,
> since that is the output from the build system.

+CC Jonas who proposed the idea.

So one reasonable way to plug this was what Jonas and I discussed over IRC.
Instead of having a custom script injecting the 'signature' node in
the .dts we could add
- u-boot-cap-key.dtsi (or similar)
- if authenticated capsule updates are selected then
CONFIG_DEVICE_TREE_INCLUDES can be automatically selected to include
the .dts that contains the public portion of the key

That would take care of the public key inclusion in the final binary,
without going through binman since this is already working in
makefiles.   We need to add a few sanity checks.  E.g if the .esl file
is empty we need to pop a build error, but that's all sanely doable.

For the last part of the capsule generation, you can use binman.  You
can include the private key you need to sign the capsule in a config
file that's fed into it.  That tool would then call mkeficapsule and
generate the signed capsule update files.  You'll be missing a
makefile target 'make capsules', but given the fact you need to build
u-boot to include it in the capsule eventually, I don't think this is
a big deal.

Regards
/Ilias



>
> Regards,
> Simon
>
> >
> > -sughosh
> >
> > [1] - https://lists.denx.de/pipermail/u-boot/2023-June/520121.html
> >
> > >
> > > >
> > > >
> > > > >
> > > > > The patches I reviewed are modifying the input files (INPUTS-y) which
> > > > > is not allowed. Nor does it seem to be necessary.
> > > >
> > > > The necessity was explained by me and Ilias earlier, and above.
> > >
> > > OK, but it still is not correct.
> > >
> > > Regards,
> > > Simon
> > >
> > > >
> > > > -sughosh
> > > >
> > > > [1] - https://lists.denx.de/pipermail/u-boot/2023-June/520121.html
> > > >
> > > >
> > > >
> > > >
> > > > >
> > > > > As to having 'real' dependencies between images in Binman, I believe
> > > > > that should be possible. At the moment images are entirely
> > > > > independent, but I am looking at implementing a simple 'template'
> > > > > scheme, where you can include some or all of an image description in
> > > > > another image. See [1] common-part in case you have thoughts on that.
> > > > >
> > > > > Regards,
> > > > > Simon
> > > > >
> > > > > [1] https://patchwork.ozlabs.org/project/uboot/patch/9e905abd369f1d8b182e6fc6e3c9ef4b1526bf76.1685975993.git.jan.kiszka@siemens.com/