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

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

[Heinrich Schuchardt]

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
--
2.30.2

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

[AKASHI Takahiro]

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

> --
> 2.30.2
> 

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

[Heinrich Schuchardt]

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?

Where do you see the conflict?

Best regards

Heinrich

>
>> --
>> 2.30.2
>>

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

[AKASHI Takahiro]

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.

-Takahiro Akashi


> Where do you see the conflict?
> 
> Best regards
> 
> Heinrich
> 
> > 
> > > --
> > > 2.30.2
> > > 
> 

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

[Tom Rini]

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

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).  CONFIG_TOOLS_LIBCRYPTO is the exception here as
it's how we make things reproducible at least, with respect to libcrypto
related requirements.  The second is that "tools-only_defconfig" is
what's used when configuring U-Boot (as tools care about
CONFIG_TOOLS_LIBCRYPTO but also LOCALVERSION).

That said, I would like to know why AVB stuff comes in for building
mkeficapsule.  Is there shared code?  If so, are these dummy variables
OK and not going to cause a problem?

-- 
Tom

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

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

[AKASHI Takahiro]

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.

> CONFIG_TOOLS_LIBCRYPTO is the exception here as
> it's how we make things reproducible at least, with respect to libcrypto
> related requirements.  The second is that "tools-only_defconfig" is
> what's used when configuring U-Boot (as tools care about
> CONFIG_TOOLS_LIBCRYPTO but also LOCALVERSION).

Again, I don't get your point.
Do you mean that we don't need "make tools-only_defconfig" for
"make tools"?

> That said, I would like to know why AVB stuff comes in for building
> mkeficapsule.  Is there shared code?  If so, are these dummy variables
> OK and not going to cause a problem?

What does AVB mean?
openssl(libssl)? If so, please take a look at my patch[1].

-Takahiro Akashi

[1] https://lists.denx.de/pipermail/u-boot/2021-August/459349.html

> -- 
> Tom

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

[Tom Rini]

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

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.

> > CONFIG_TOOLS_LIBCRYPTO is the exception here as
> > it's how we make things reproducible at least, with respect to libcrypto
> > related requirements.  The second is that "tools-only_defconfig" is
> > what's used when configuring U-Boot (as tools care about
> > CONFIG_TOOLS_LIBCRYPTO but also LOCALVERSION).
> 
> Again, I don't get your point.
> Do you mean that we don't need "make tools-only_defconfig" for
> "make tools"?

I mean that for all real configurations of U-Boot, "make tools" (or,
"make tools-only") is expected to work.  And also that
"tools-only_defconfig" isn't a real U-Boot config, but a dummy config to
support distributions being able to build and package the host tools,
and use the "tools-only" build target.

> > That said, I would like to know why AVB stuff comes in for building
> > mkeficapsule.  Is there shared code?  If so, are these dummy variables
> > OK and not going to cause a problem?
> 
> What does AVB mean?

My question here was to Heinrich, as he's adding dummy values for AVB,
which is Android Verified Boot, to the tools-only_defconfig and I want
to know what's pulling that in, and it matters for the host tools
themselves in some way.  I hope not, in which case the dummy values are
fine.

-- 
Tom

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

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

[Heinrich Schuchardt]

On 9/9/21 1:46 PM, 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).  CONFIG_TOOLS_LIBCRYPTO is the exception here as

Agreed. That is why in response to [1] I asked Takahiro to change the
patch such that it covers both signed and unsigned capsules. I don't
want two different versions.

Currently the tool is not build at all if
CONFIG_EFI_HAVE_CAPSULE_SUPPORT is not selected. Do I understand you
right that this dependency should be lifted?

> it's how we make things reproducible at least, with respect to libcrypto
> related requirements.  The second is that "tools-only_defconfig" is
> what's used when configuring U-Boot (as tools care about
> CONFIG_TOOLS_LIBCRYPTO but also LOCALVERSION).
>
> That said, I would like to know why AVB stuff comes in for building
> mkeficapsule.  Is there shared code?  If so, are these dummy variables
> OK and not going to cause a problem?
>

AVB_VERIFY is implied by SANDBOX and depends on PARTITION_UUIDS.
CONFIG_EFI_HAVE_CAPSULE_SUPPORT requires EFI_LOADER.
EFI_LOADER selects PARTITION_UUIDS.

Best regards

Heinrich

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

[Tom Rini]

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

On Thu, Sep 09, 2021 at 02:31:18PM +0200, Heinrich Schuchardt wrote:
> On 9/9/21 1:46 PM, 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).  CONFIG_TOOLS_LIBCRYPTO is the exception here as
> 
> Agreed. That is why in response to [1] I asked Takahiro to change the
> patch such that it covers both signed and unsigned capsules. I don't
> want two different versions.
> 
> Currently the tool is not build at all if
> CONFIG_EFI_HAVE_CAPSULE_SUPPORT is not selected. Do I understand you
> right that this dependency should be lifted?

I went and re-read the rules on how we enable host tools.  I think it's
fine to leave that part as-is (and then yes, I've been mistaken in what
I've said above, a few more options are also relevant).

> > it's how we make things reproducible at least, with respect to libcrypto
> > related requirements.  The second is that "tools-only_defconfig" is
> > what's used when configuring U-Boot (as tools care about
> > CONFIG_TOOLS_LIBCRYPTO but also LOCALVERSION).
> > 
> > That said, I would like to know why AVB stuff comes in for building
> > mkeficapsule.  Is there shared code?  If so, are these dummy variables
> > OK and not going to cause a problem?
> 
> AVB_VERIFY is implied by SANDBOX and depends on PARTITION_UUIDS.
> CONFIG_EFI_HAVE_CAPSULE_SUPPORT requires EFI_LOADER.
> EFI_LOADER selects PARTITION_UUIDS.

Ah, OK.  I might have gone with turning off AVB in tools-only_defconfig
instead, but it's not a big deal.  I'm going to take a quick poke at
something now in fact.

-- 
Tom

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

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

[AKASHI Takahiro]

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"?

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

-Takahiro Akashi

> > > CONFIG_TOOLS_LIBCRYPTO is the exception here as
> > > it's how we make things reproducible at least, with respect to libcrypto
> > > related requirements.  The second is that "tools-only_defconfig" is
> > > what's used when configuring U-Boot (as tools care about
> > > CONFIG_TOOLS_LIBCRYPTO but also LOCALVERSION).
> > 
> > Again, I don't get your point.
> > Do you mean that we don't need "make tools-only_defconfig" for
> > "make tools"?
> 
> I mean that for all real configurations of U-Boot, "make tools" (or,
> "make tools-only") is expected to work.  And also that
> "tools-only_defconfig" isn't a real U-Boot config, but a dummy config to
> support distributions being able to build and package the host tools,
> and use the "tools-only" build target.
> 
> > > That said, I would like to know why AVB stuff comes in for building
> > > mkeficapsule.  Is there shared code?  If so, are these dummy variables
> > > OK and not going to cause a problem?
> > 
> > What does AVB mean?
> 
> My question here was to Heinrich, as he's adding dummy values for AVB,
> which is Android Verified Boot, to the tools-only_defconfig and I want
> to know what's pulling that in, and it matters for the host tools
> themselves in some way.  I hope not, in which case the dummy values are
> fine.
> 
> -- 
> Tom

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

[Tom Rini]

[-- 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 --]

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

[AKASHI Takahiro]

Simon, Heinrich,

On Thu, Sep 09, 2021 at 09:02:23AM -0400, Tom Rini wrote:
> 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).

@Simon,
Even so, I would like you to clarify what "all (the tools)" and
"tools-only" means.
Looking at tools/Makefile, "hostprogs-y" can be affected by several
different types of kconfigs:

1) the tools is necessary to build the u-boot binary for a specific
   target or board.
2) the tool is not needed for building, but is used for a specific
   feature of U-Boot (later by users).

tools-only_defconfig addresses (2), plus 
3) all the kconfigs which will allows for building "full-featured" tools
which can be implemented with such kconfigs on or off.

@Heinrich,
Regarding CONFIG_EFI_HAVE_CAPSULE_SUPPORT, I prefer having a dedicated
kconfig, CONFIG_TOOLS_MKEFICAPSULE for some reasons:

* Difference btw (2) and (3) is not trivial to users, and
  if CONFIG_EFI_HAVE_CAPSULE_SUPPORT is used, even much so.
* On the other hand,
  ===>
  config TOOLS_MKEFICAPSULE
  	bool "Build efimkcapsule command"
  	default y if EFI_CAPSULE_ON_DISK
  <===
  would make this clear.
  (We might better use 'imply' here.)
* CONFIG_EFI_HAVE_CAPSULE_SUPPORT is an internal kconfig, while
  CONFIG_TOOL_MAKEFICAPSULE is an external one and you can turn it
  on and off with "make menuconfig".
  (There is a small possibility that people may want to use another
  tool instead, like one from EDK-II's script? Who knows.)

Finally,
> Currently the tool is not build at all if
> CONFIG_EFI_HAVE_CAPSULE_SUPPORT is not selected. Do I understand you
> right that this dependency should be lifted?

I'm not sure this statement targeted me or not.
As a matter of fact, the implementation of mkeficapsule even in
the current form doesn't depend on none of
CONFIG_EFI_HAVE_CAPSULE_SUPPORT
CONFIG_EFI_CAPSULE_ON_DISK
CONFIG_EFI_CAPSULE_FIRMWARE_FIT
CONFIG_EFI_CAPSULE_FIRMWARE_RAW
CONFIG_EFI_CAPSULE_AUTHENTICATE

-Takahiro Akashi

> -- 
> Tom

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

[Tom Rini]

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

On Fri, Sep 10, 2021 at 11:17:16AM +0900, AKASHI Takahiro wrote:
> Simon, Heinrich,
> 
> On Thu, Sep 09, 2021 at 09:02:23AM -0400, Tom Rini wrote:
> > 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).
> 
> @Simon,
> Even so, I would like you to clarify what "all (the tools)" and
> "tools-only" means.
> Looking at tools/Makefile, "hostprogs-y" can be affected by several
> different types of kconfigs:
> 
> 1) the tools is necessary to build the u-boot binary for a specific
>    target or board.
> 2) the tool is not needed for building, but is used for a specific
>    feature of U-Boot (later by users).
> 
> tools-only_defconfig addresses (2), plus 
> 3) all the kconfigs which will allows for building "full-featured" tools
> which can be implemented with such kconfigs on or off.

Functionally, "tools-only_defconfig" is what distributions use to build
the host tools, which are packaged up and are not board specific.  The
only practical difference, with that defconfig, between "tools-only" and
"tools-all" make targets is envcrc.  The "tools-only" make target is
intended to build all of the host tools that are generically useful.

> @Heinrich,
> Regarding CONFIG_EFI_HAVE_CAPSULE_SUPPORT, I prefer having a dedicated
> kconfig, CONFIG_TOOLS_MKEFICAPSULE for some reasons:
> 
> * Difference btw (2) and (3) is not trivial to users, and
>   if CONFIG_EFI_HAVE_CAPSULE_SUPPORT is used, even much so.
> * On the other hand,
>   ===>
>   config TOOLS_MKEFICAPSULE
>   	bool "Build efimkcapsule command"
>   	default y if EFI_CAPSULE_ON_DISK
>   <===
>   would make this clear.
>   (We might better use 'imply' here.)
> * CONFIG_EFI_HAVE_CAPSULE_SUPPORT is an internal kconfig, while
>   CONFIG_TOOL_MAKEFICAPSULE is an external one and you can turn it
>   on and off with "make menuconfig".
>   (There is a small possibility that people may want to use another
>   tool instead, like one from EDK-II's script? Who knows.)

No, if we're building a generically useful tool, we don't want another
symbol for it.  The worst case here of building something the user then
doesn't use is a micro-optimization.  If there's going to be multiple
public tools to create and package an EFI capsule update, we shouldn't
even have the tool in our tree without a really good reason and instead
instruct users (and update CI to) grab the tool from the upstream
location.

-- 
Tom

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