Re: [PATCH 1/1] configs: add mkeficapsule to tools-only_defconfig

[Tom Rini <trini@konsulko.com>]

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

On Thu, Sep 09, 2021 at 09:47:21PM +0900, AKASHI Takahiro wrote:
> On Thu, Sep 09, 2021 at 08:15:43AM -0400, Tom Rini wrote:
> > On Thu, Sep 09, 2021 at 09:10:23PM +0900, AKASHI Takahiro wrote:
> > > Tom,
> > > 
> > > On Thu, Sep 09, 2021 at 07:46:15AM -0400, Tom Rini wrote:
> > > > On Thu, Sep 09, 2021 at 05:30:36PM +0900, AKASHI Takahiro wrote:
> > > > > On Thu, Sep 09, 2021 at 09:27:50AM +0200, Heinrich Schuchardt wrote:
> > > > > > On 9/9/21 8:09 AM, AKASHI Takahiro wrote:
> > > > > > > On Thu, Sep 09, 2021 at 07:27:10AM +0200, Heinrich Schuchardt wrote:
> > > > > > > > mkeficapsule is used to create capsules for UEFI firmware update.
> > > > > > > > To ease inclusion into U-Boot tools packages of Linux distributions we
> > > > > > > > should add it to the tools-only_defconfig.
> > > > > > > > 
> > > > > > > > Provide dummy values for CONFIG_AVB_BUF_ADDR, CONFIG_AVB_BUF_SIZE to
> > > > > > > > satisfy Kconfig.
> > > > > > > > 
> > > > > > > > Suggested-by: Vagrant Cascadian <vagrant@debian.org>
> > > > > > > > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > > > > > > > ---
> > > > > > > >   configs/tools-only_defconfig | 7 ++++++-
> > > > > > > >   1 file changed, 6 insertions(+), 1 deletion(-)
> > > > > > > > 
> > > > > > > > diff --git a/configs/tools-only_defconfig b/configs/tools-only_defconfig
> > > > > > > > index f54bc1802c..8a20d3fb05 100644
> > > > > > > > --- a/configs/tools-only_defconfig
> > > > > > > > +++ b/configs/tools-only_defconfig
> > > > > > > > @@ -5,6 +5,8 @@ CONFIG_ANDROID_BOOT_IMAGE=y
> > > > > > > >   CONFIG_FIT=y
> > > > > > > >   CONFIG_FIT_SIGNATURE=y
> > > > > > > >   CONFIG_MISC_INIT_F=y
> > > > > > > > +CONFIG_AVB_BUF_ADDR=0x0
> > > > > > > > +CONFIG_AVB_BUF_SIZE=0x8192
> > > > > > > >   # CONFIG_CMD_BOOTD is not set
> > > > > > > >   # CONFIG_CMD_BOOTM is not set
> > > > > > > >   # CONFIG_CMD_ELF is not set
> > > > > > > > @@ -29,4 +31,7 @@ CONFIG_SYSRESET=y
> > > > > > > >   # CONFIG_VIRTIO_MMIO is not set
> > > > > > > >   # CONFIG_VIRTIO_PCI is not set
> > > > > > > >   # CONFIG_VIRTIO_SANDBOX is not set
> > > > > > > > -# CONFIG_EFI_LOADER is not set
> > > > > > > > +CONFIG_EFI_CAPSULE_ON_DISK=y
> > > > > > > > +CONFIG_EFI_CAPSULE_FIRMWARE_FIT=y
> > > > > > > > +CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
> > > > > > > > +CONFIG_EFI_CAPSULE_AUTHENTICATE=y
> > > > > > > 
> > > > > > > I think that we should use the way that I suggested in my patch[1].
> > > > > > > 
> > > > > > > -Takahiro Akashi
> > > > > > > 
> > > > > > > [1] https://lists.denx.de/pipermail/u-boot/2021-August/459349.html
> > > > > > 
> > > > > > Your patch [1] still requires some rework:
> > > > > > https://patchwork.ozlabs.org/project/uboot/patch/20210831024659.53464-2-takahiro.akashi@linaro.org/
> > > > > > 
> > > > > > [1] changes what mkeficapsule looks like and this patch makes it
> > > > > > available in tools-only_defconfig?
> > > > > > 
> > > > > > Aren't these two patches complementary?
> > > > > 
> > > > > With my patch applied, the only option we need to compile mkeficapsule is:
> > > > >    CONFIG_TOOLS_MKEFICAPSULE
> > > > >    (and optionally CONFIG_TOOLS_LIBCRYPTO)
> > > > > 
> > > > > There is no target-config dependency as you have expected.
> > > > 
> > > > There's two issues.  First, the general one is that when just building
> > > > host tools (typically to package up in a distribution of some sort), it
> > > > shouldn't depend on how "U-Boot" was configured (set aside the default
> > > > environment problem).
> > > 
> > > I don't get your point. CONFIG_TOOLS_MKEFICAPSULE is only for building
> > > mkeficapsule. It has nothing to do with U-Boot itself.
> > 
> > My point is that "make tools-only" must build all the tools that would
> > be packaged up in a distribution or otherwise sent to end users.  It
> > must not depend on (with the exceptions above) how we configured the
> > build.
> 
> I can see bunch of "hostprogs-$(CONFIG_...) += ...", and
> some of them are not always built.
> So what do you mean by "all the tools"?

Our emails crossed.  Yes, I was slightly wrong above, and the comment at
the top of tools/Makefile:
# A couple of simple rules:
#
# 1) Do not use target CONFIG_* options to enable or disable features in
# host
#    tools. Only use the configs from tools/Kconfig
# 2) It's okay to use target configs to disable building specific tools.
#    That's as long as the features of those tools aren't modified.

are correct.  So it's OK that the tool today depends on
CONFIG_EFI_HAVE_CAPSULE_SUPPORT.

> Which tools be packed in is totally up to a distro, isn't it?

That's the difference between building and packaging.  The best general
answer is to build everything and package individually, when fine
grained control is desired.  But no, making more work on each
distribution to get all tools built makes things harder, not easier, is
the feedback we've gotten (and with my OE guy hat on, agree with).

-- 
Tom

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