[PATCH v6 9/9] sandbox: capsule: Generate capsule related files through binman

[Sughosh Ganu]

hi Simon,

On Fri, 4 Aug 2023 at 08:32, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Sughosh,
>
> On Thu, 3 Aug 2023 at 05:18, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> >
> > hi Simon,
> >
> > On Wed, 2 Aug 2023 at 18:23, Simon Glass <sjg at chromium.org> wrote:
> > >
> > > Hi Sughosh,
> > >
> > > On Tue, 1 Aug 2023 at 11:41, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> > > >
> > > > The EFI capsule files can now be generated as part of u-boot
> > > > build. This is done through binman. Add capsule entry nodes in the
> > > > u-boot.dtsi for the sandbox architecture for generating the
> > > > capsules. Remove the corresponding generation of capsules from the
> > > > capsule update conftest file.
> > > >
> > > > The capsules are generated through the config file for the sandbox
> > > > variant, and through explicit parameters for the sandbox_flattree
> > > > variant.
> > > >
> > > > Also generate the FIT image used for testing the capsule update
> > > > feature on the sandbox_flattree variant through binman. Remove the now
> > > > superfluous its file which was used for generating this FIT image.
> > > >
> > > > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> > > > ---
> > > > Changes since V5:
> > > > * Use the public key ESL file and other input files from the tree
> > > >   instead of the /tmp/capsules/ directory being used in previous
> > > >   version.
> > > > * Use macros for other input files and certs.
> > > >
> > > >  arch/sandbox/dts/u-boot.dtsi                  | 347 ++++++++++++++++++
> > > >  test/py/tests/test_efi_capsule/conftest.py    | 128 +------
> > > >  .../tests/test_efi_capsule/uboot_bin_env.its  |  36 --
> > > >  3 files changed, 348 insertions(+), 163 deletions(-)
> > > >  delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its
> > > >
> > >
> > > I want to get the binman stuff right before diving into this, but the
> > > binman stuff seems fairly close, so I'll just mention...do you really
> > > need all these combinations of tests? It seems to me that one test is
> > > enough. You know that the binman tests will protect the code there, so
> > > why test it all over again here?
> >
> > These are capsules that are needed for testing the EFI capsule update
> > functionality. Currently, the capsules used for testing the feature
> > are generated after u-boot has been built. Same for embedding the
> > public key in the dtb. I think it is better to have the same flow of
> > generating capsules and the associated logic(public key embedding)
> > that is being supported in u-boot rather than having two divergent
> > flows. This also serves as an example for potential users who would
> > want to generate capsules as part of the build flow.
>
> But my question was why you need more than one test here? Are you
> testing that U-Boot can decode a capsule file of various types? That
> should be done in unit tests.

The tests are the same. They are not being changed. What is changed is
the stage at which the capsules are being generated. Currently, the
capsules get generated only when the tests are invoked, as part of the
test setup. Same for embedding of the public key cert EFI Signature
List(ESL) file. This patch results in the capsules getting generated
as part of the u-boot build. Same for embedding of the public key ESL.
If we don't follow this flow, we would have support for generating
capsules as part of the u-boot build, but that flow would not be used
at all. I understand that binman tests the generation of capsules, but
we would then have this divergence between the flow that is supported,
and what is actually used in the tests.

One alternative, which I think is a middle ground for this would be to
add a Kconfig symbol and use that for generating capsules. We can then
use that symbol in CI. This is similar to how the trace testing
happens in CI on the sandbox platform. In that scenario, we would not
have the capsules getting generated during normal builds.

>
> Now I see the tests you are referring to in
> test_capsule_firmware_signed_raw.py  (please shorten the name!)
>
> These tests all have the reboot problem we need to fix, but anyway, at
> least I understand it.
>
> It looks like you are writing the test files into the source tree?
> They should be written to the output tree.

If we are to generate the capsules, and embed the key as part of the
u-boot build, these input files are needed. Btw, I do see a few places
which have input files in the source, including inside binman. What
issue do you see having these in the source?

I had discussed this with Tom over irc and he had suggested this
location for the files.

-sughosh