Re: [PATCH v8 4/9] sandbox: Build the mkeficapsule tool for the sandbox variants

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 5355 bytes --]

On Sat, Aug 12, 2023 at 07:08:44AM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Fri, 11 Aug 2023 at 09:56, Tom Rini <trini@konsulko.com> wrote:
> >
> > On Fri, Aug 11, 2023 at 08:26:36AM -0600, Simon Glass wrote:
> > > Hi Sughosh,
> > >
> > > On Fri, 11 Aug 2023 at 08:23, Sughosh Ganu <sughosh.ganu@linaro.org>
> wrote:
> > > >
> > > > On Fri, 11 Aug 2023 at 19:28, Tom Rini <trini@konsulko.com> wrote:
> > > > >
> > > > > On Fri, Aug 11, 2023 at 04:29:37PM +0530, Sughosh Ganu wrote:
> > > > > > On Thu, 10 Aug 2023 at 22:47, Tom Rini <trini@konsulko.com> wrote:
> > > > > > >
> > > > > > > On Thu, Aug 10, 2023 at 10:39:06PM +0530, Sughosh Ganu wrote:
> > > > > > > > On Thu, 10 Aug 2023 at 21:22, Tom Rini <trini@konsulko.com>
> wrote:
> > > > > > > > >
> > > > > > > > > On Thu, Aug 10, 2023 at 07:53:33PM +0530, Sughosh Ganu
> wrote:
> > > > > > > > >
> > > > > > > > > > Build the mkeficapsule tool for all the sandbox variants.
> This tool
> > > > > > > > > > will be used subsequently for testing capsule generation
> in binman.
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> > > > > > > > > > ---
> > > > > > > > > > Changes since V7: None
> > > > > > > > > >
> > > > > > > > > >  tools/Kconfig | 6 +++---
> > > > > > > > > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > > > > > > > >
> > > > > > > > > > diff --git a/tools/Kconfig b/tools/Kconfig
> > > > > > > > > > index 6e23f44d55..353a855243 100644
> > > > > > > > > > --- a/tools/Kconfig
> > > > > > > > > > +++ b/tools/Kconfig
> > > > > > > > > > @@ -91,10 +91,10 @@ config TOOLS_SHA512
> > > > > > > > > >         Enable SHA512 support in the tools builds
> > > > > > > > > >
> > > > > > > > > >  config TOOLS_MKEFICAPSULE
> > > > > > > > > > -     bool "Build efimkcapsule command"
> > > > > > > > > > -     default y if EFI_CAPSULE_ON_DISK
> > > > > > > > > > +     bool "Build mkeficapsule tool"
> > > > > > > > > > +     default y if EFI_CAPSULE_ON_DISK || SANDBOX
> > > > > > > > > >       help
> > > > > > > > > > -       This command allows users to create a UEFI
> capsule file and,
> > > > > > > > > > +       This tool allows users to create a UEFI capsule
> file and,
> > > > > > > > > >         optionally sign that file. If you want to enable
> UEFI capsule
> > > > > > > > > >         update feature on your target, you certainly need
> this.
> > > > > > > > >
> > > > > > > > > Sorry, what is this fixing exactly?
> > > > > > > >
> > > > > > > > The tool is required to be supported on the sandbox_spl
> variant, since
> > > > > > > > that is used for the binman tests in CI. Simon had then asked
> me to
> > > > > > > > add support for the tool on all sandbox variants. I missed
> putting his
> > > > > > > > R-b on this patch.
> > > > > > >
> > > > > > > OK, moving forward just depend on:
> > > > > > >
> https://patchwork.ozlabs.org/project/uboot/patch/20230810165224.514772-1-trini@konsulko.com/
> > > > > > > instead please, thanks.
> > > > > >
> > > > > > I will base my changes on top of your patch. However, we would
> still
> > > > > > need this patch as part of the series, since Simon wants the
> capsules
> > > > > > to be generated for all the sandbox variants. Thanks.
> > > > >
> > > > > No, this isn't needed.  Any sandbox variant that needs capsules has
> > > > > EFI_CAPSULE_ON_DISK enabled.
> > > >
> > > > Simon wants the capsules to be generated on all sandbox variants,
> > > > including those that do not have the EFI_CAPSULE_ON_DISK enabled.
> > > > Which is why we need to have the tool enabled for all sandbox
> > > > variants.
> > >
> > > I want to avoid #ifdefs in the sandbox .dts so far as possible.
> > >
> > > Tom, I'll let you make the final decision.
> > >
> > > In any case, the multiple-images thing needs to be fixed.
> >
> > Sughosh, please update the other sandbox defconfigs to just enable
> > EFI_CAPSULE_ON_DISK.
> >
> > Simon, this I think is an example of where re-working
> > configs/sandbox64_defconfig
> > configs/sandbox_defconfig
> > configs/sandbox_flattree_defconfig
> > configs/sandbox_noinst_defconfig
> > configs/sandbox_spl_defconfig
> > configs/sandbox_vpl_defconfig
> >
> > To be configs/sandbox_defconfig + boards/sandbox/flattree.config,
> > noinst.config, spl.config, vpl.config would be helpful.  There's the
> > sandbox config itself where EFI_CAPSULE_ON_DISK=y and then every other
> > variant just gets that, and we don't have to tweak N configs.
> 
> You mean split configs? So far I am unable to build those...

I don't know what you mean by split configs.  I mean that I think the
only intentional difference between configs/sandbox_defconfig and
configs/sandbox64_defconfig is:
CONFIG_SANDBOX64=y
CONFIG_DEFAULT_DEVICE_TREE="sandbox64"

And everything else is unintentional.  And there's lots of other deltas
like that between each of the other variants, and sandbox.  And that
this isn't the first, nor likely the last, time where we need to enable
some option on other sandbox config files too, so that CI passes.  This
would all be avoided by using the config fragments mechanism so that
we captured only the intentional delta of a fragment rather than
maintaining N nearly identical, but not quite, files.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]