[PATCH v7 09/11] sandbox: capsule: Generate capsule related files through binman

[Simon Glass]

Hi Sughosh,

On Sat, 5 Aug 2023 at 12:01, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
>
> hi Simon,
>
> On Sat, 5 Aug 2023 at 20:34, Simon Glass <sjg at chromium.org> wrote:
> >
> > Hi Sughosh,
> >
> > On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> > >
> > > The EFI capsule files can now be generated as part of u-boot
> > > build through binman. Add capsule entry nodes in the u-boot.dtsi for
> > > the sandbox architecture for generating the capsules. These capsules
> > > are then used for testing the EFI capsule update functionality on the
> > > sandbox platforms. Also add binman nodes for generating the input
> > > files used for these capsules.
> > >
> > > Remove the corresponding logic which was used for generation of these
> > > input files which is now superfluous.
> > >
> > > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> > > ---
> > > Changes since V6:
> > > * Use macros defined in sandbox_efi_capsule for GUIDs and capsule
> > >   input filenames.
> > > * Generate the capsule input files through binman text entries.
> > >
> > >  arch/sandbox/dts/u-boot.dtsi                  | 363 ++++++++++++++++++
> > >  include/sandbox_efi_capsule.h                 |  11 +
> > >  test/py/tests/test_efi_capsule/conftest.py    | 168 +-------
> > >  test/py/tests/test_efi_capsule/signature.dts  |  10 -
> > >  .../tests/test_efi_capsule/uboot_bin_env.its  |  36 --
> > >  5 files changed, 385 insertions(+), 203 deletions(-)
> > >  delete mode 100644 test/py/tests/test_efi_capsule/signature.dts
> > >  delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its
> >
> > I think you are still getting confused with using filenames when you
> > don't need to. See below...
>
> No, I don't think so. This is being done on purpose, since I want
> these files to be created. These are then copied to the efi capsule
> update tests' data directory, and are then used in testing the
> feature. Maybe if you want, I can put a comment to that effect.

I just traced through this code. I really think this needs to be
simplified. You can do it in a patch at the end if you like.

But you have the 'u-boot.bin.old' and 'Old' strings in
test_efi_capsule_auth2, for example.

In the binman world you define UBOOT_BIN_IMAGE_OLD as
"u-boot.bin.old", then use that in the sandbox.dtsi

Then binman creates the u-boot.bin.old file (unnecessarily in my view)
Then in efi_capsule_data() you copy the file to the test directory.

So for the last step, you could just create the file again, rather
than copying it from the U-Boot build directory. After all, you know
the contents. If you like you could put the different contents as
binary strings in capsule_defs.py

Then you don't need to create the files.

There is a lot more I can say about the EFI capsule tests. For now I
think we'll have to live with it creating 19 different binman images
on sandbox. I think we could put them in an efi_capsules subdir, but
that would need to be created somewhere, since binman doesn't do it. I
suppose we could make binman automatically create a directory if an
entry filename needs it?

Anyway, for tests, primarily we need to split things up, so we have,
for example:

process_capsule_file()

which processes the capsule in U-Boot, e.g. using an 'efi' command,
then outputs what it did. Then the test can plant the capsule, run
that function and check that the output is as expected. This can
actually be a unit test, i.e. written in C.

Most of the tests can do this. Then we can have one test that checks
the whole flow, but it doesn't need to be done for every case, as now.

Regards,
Simon