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

[Simon Glass]

Hi Tom,

On Mon, 7 Aug 2023 at 12:08, Tom Rini <trini at konsulko.com> wrote:
>
> On Sun, Aug 06, 2023 at 04:43:06PM +0530, Sughosh Ganu wrote:
> > On Sun, 6 Aug 2023 at 03:48, Tom Rini <trini at konsulko.com> wrote:
> > >
> > > On Sat, Aug 05, 2023 at 09:04:00AM -0600, Simon Glass 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...
> > > >
> > > >
> > > > >
> > > > > diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi
> > > > > index 60bd004937..f1b16b41c2 100644
> > > > > --- a/arch/sandbox/dts/u-boot.dtsi
> > > > > +++ b/arch/sandbox/dts/u-boot.dtsi
> >
> > <snip>
> >
> > > >
> > > > > +               };
> > > > > +       };
> > > > > +
> > > > > +       capsule2 {
> > > > > +               filename = "Test02";
> > > > > +               capsule {
> > > > > +                       type = "efi-capsule";
> > > > > +                       image-index = <0x2>;
> > > > > +                       image-type-id = SANDBOX_UBOOT_ENV_IMAGE_GUID;
> > > > > +
> > > > > +                       blob {
> > > > > +                               filename = UBOOT_ENV_IMAGE_NEW;
> > > > > +                       };
> > > > > +               };
> > > > > +       };
> > > > > +
> > > > > +       capsule3 {
> > > > > +               filename = "Test03";
> > > > > +               capsule {
> > > > > +                       type = "efi-capsule";
> > > > > +                       image-index = <0x1>;
> > > > > +                       image-type-id = SANDBOX_INCORRECT_GUID;
> > > > > +
> > > > > +                       blob {
> > > > > +                               filename = UBOOT_BIN_IMAGE_NEW;
> > > > > +                       };
> > > > > +               };
> > > > > +       };
> > > > > +
> > > > > +       capsule4 {
> > > > > +               filename = "Test04";
> > > > > +               capsule {
> > > > > +                       type = "efi-capsule";
> > > > > +                       image-index = <0x1>;
> > > > > +                       image-type-id = SANDBOX_FIT_IMAGE_GUID;
> > > > > +
> > > > > +                       blob {
> > > > > +                               filename =
> > > > [..]
> > > >
> > > > > +       capsule19 {
> > > > > +               filename = "Test115";
> > > >
> > > > [..]
> > > >
> > > > We really need to talk about these tests. So many cases! Can you not
> > > > reduce this?
> > >
> > > And why are these tests in the generic looking file and not one of the
> > > test dts files? This looks like something that would make implementation
> > > in real life confusing and non-obvious.
> >
> > These are not capsules that are being generated for binman tests.
> > Those dts files are indeed under tools/binman/test/. These capsules
> > are the ones used for testing the capsule update feature in the CI
> > run. The idea of having these capsule nodes defined in this file is to
> > have them generated as part of the standard build.
>
> The high level concern I have (so it applies to a few of these patches)
> is that it's not going to be clear and straightforward on how to use
> this regularly (for example, if I follow all of this right, I should be
> able to do a capsule update to push a build to one of my boards, and
> then run our pytest suite on it, instead of playing games with SD mux
> boards and so forth) or production (configure product-board so that we
> can deliver an updated firmware via $mechanism).

Well you can do that today with VBE...

Regards,
Simon