* [PATCH 1/1] configs: add mkeficapsule to tools-only_defconfig @ 2021-09-09 5:27 Heinrich Schuchardt 2021-09-09 6:09 ` AKASHI Takahiro 0 siblings, 1 reply; 13+ messages in thread From: Heinrich Schuchardt @ 2021-09-09 5:27 UTC (permalink / raw) To: Tom Rini Cc: Simon Glass, Vagrant Cascadian, AKASHI Takahiro, Sughosh Ganu, u-boot, 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/1] configs: add mkeficapsule to tools-only_defconfig 2021-09-09 5:27 [PATCH 1/1] configs: add mkeficapsule to tools-only_defconfig Heinrich Schuchardt @ 2021-09-09 6:09 ` AKASHI Takahiro 2021-09-09 7:27 ` Heinrich Schuchardt 0 siblings, 1 reply; 13+ messages in thread From: AKASHI Takahiro @ 2021-09-09 6:09 UTC (permalink / raw) To: Heinrich Schuchardt Cc: Tom Rini, Simon Glass, Vagrant Cascadian, Sughosh Ganu, u-boot 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 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/1] configs: add mkeficapsule to tools-only_defconfig 2021-09-09 6:09 ` AKASHI Takahiro @ 2021-09-09 7:27 ` Heinrich Schuchardt 2021-09-09 8:30 ` AKASHI Takahiro 0 siblings, 1 reply; 13+ messages in thread From: Heinrich Schuchardt @ 2021-09-09 7:27 UTC (permalink / raw) To: AKASHI Takahiro Cc: Tom Rini, Simon Glass, Vagrant Cascadian, u-boot, Sughosh Ganu 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 >> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/1] configs: add mkeficapsule to tools-only_defconfig 2021-09-09 7:27 ` Heinrich Schuchardt @ 2021-09-09 8:30 ` AKASHI Takahiro 2021-09-09 11:46 ` Tom Rini 0 siblings, 1 reply; 13+ messages in thread From: AKASHI Takahiro @ 2021-09-09 8:30 UTC (permalink / raw) To: Heinrich Schuchardt Cc: Tom Rini, Simon Glass, Vagrant Cascadian, u-boot, Sughosh Ganu 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 > > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/1] configs: add mkeficapsule to tools-only_defconfig 2021-09-09 8:30 ` AKASHI Takahiro @ 2021-09-09 11:46 ` Tom Rini 2021-09-09 12:10 ` AKASHI Takahiro 2021-09-09 12:31 ` Heinrich Schuchardt 0 siblings, 2 replies; 13+ messages in thread From: Tom Rini @ 2021-09-09 11:46 UTC (permalink / raw) To: AKASHI Takahiro, Heinrich Schuchardt, Simon Glass, Vagrant Cascadian, u-boot, Sughosh Ganu [-- 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 --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/1] configs: add mkeficapsule to tools-only_defconfig 2021-09-09 11:46 ` Tom Rini @ 2021-09-09 12:10 ` AKASHI Takahiro 2021-09-09 12:15 ` Tom Rini 2021-09-09 12:31 ` Heinrich Schuchardt 1 sibling, 1 reply; 13+ messages in thread From: AKASHI Takahiro @ 2021-09-09 12:10 UTC (permalink / raw) To: Tom Rini Cc: Heinrich Schuchardt, Simon Glass, Vagrant Cascadian, u-boot, Sughosh Ganu 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/1] configs: add mkeficapsule to tools-only_defconfig 2021-09-09 12:10 ` AKASHI Takahiro @ 2021-09-09 12:15 ` Tom Rini 2021-09-09 12:47 ` AKASHI Takahiro 0 siblings, 1 reply; 13+ messages in thread From: Tom Rini @ 2021-09-09 12:15 UTC (permalink / raw) To: AKASHI Takahiro, Heinrich Schuchardt, Simon Glass, Vagrant Cascadian, u-boot, Sughosh Ganu [-- 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 --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/1] configs: add mkeficapsule to tools-only_defconfig 2021-09-09 12:15 ` Tom Rini @ 2021-09-09 12:47 ` AKASHI Takahiro 2021-09-09 13:02 ` Tom Rini 0 siblings, 1 reply; 13+ messages in thread From: AKASHI Takahiro @ 2021-09-09 12:47 UTC (permalink / raw) To: Tom Rini Cc: Heinrich Schuchardt, Simon Glass, Vagrant Cascadian, u-boot, Sughosh Ganu 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/1] configs: add mkeficapsule to tools-only_defconfig 2021-09-09 12:47 ` AKASHI Takahiro @ 2021-09-09 13:02 ` Tom Rini 2021-09-10 2:17 ` AKASHI Takahiro 0 siblings, 1 reply; 13+ messages in thread From: Tom Rini @ 2021-09-09 13:02 UTC (permalink / raw) To: AKASHI Takahiro, Heinrich Schuchardt, Simon Glass, Vagrant Cascadian, u-boot, Sughosh Ganu [-- 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 --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/1] configs: add mkeficapsule to tools-only_defconfig 2021-09-09 13:02 ` Tom Rini @ 2021-09-10 2:17 ` AKASHI Takahiro 2021-09-10 3:14 ` Tom Rini 0 siblings, 1 reply; 13+ messages in thread From: AKASHI Takahiro @ 2021-09-10 2:17 UTC (permalink / raw) To: Tom Rini Cc: Heinrich Schuchardt, Simon Glass, Vagrant Cascadian, u-boot, Sughosh Ganu 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/1] configs: add mkeficapsule to tools-only_defconfig 2021-09-10 2:17 ` AKASHI Takahiro @ 2021-09-10 3:14 ` Tom Rini 0 siblings, 0 replies; 13+ messages in thread From: Tom Rini @ 2021-09-10 3:14 UTC (permalink / raw) To: AKASHI Takahiro, Heinrich Schuchardt, Simon Glass, Vagrant Cascadian, u-boot, Sughosh Ganu [-- 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 --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/1] configs: add mkeficapsule to tools-only_defconfig 2021-09-09 11:46 ` Tom Rini 2021-09-09 12:10 ` AKASHI Takahiro @ 2021-09-09 12:31 ` Heinrich Schuchardt 2021-09-09 12:44 ` Tom Rini 1 sibling, 1 reply; 13+ messages in thread From: Heinrich Schuchardt @ 2021-09-09 12:31 UTC (permalink / raw) To: Tom Rini Cc: AKASHI Takahiro, Vagrant Cascadian, Simon Glass, Sughosh Ganu, u-boot 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/1] configs: add mkeficapsule to tools-only_defconfig 2021-09-09 12:31 ` Heinrich Schuchardt @ 2021-09-09 12:44 ` Tom Rini 0 siblings, 0 replies; 13+ messages in thread From: Tom Rini @ 2021-09-09 12:44 UTC (permalink / raw) To: Heinrich Schuchardt Cc: AKASHI Takahiro, Vagrant Cascadian, Simon Glass, Sughosh Ganu, u-boot [-- 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 --] ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-09-10 3:14 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-09 5:27 [PATCH 1/1] configs: add mkeficapsule to tools-only_defconfig Heinrich Schuchardt 2021-09-09 6:09 ` AKASHI Takahiro 2021-09-09 7:27 ` Heinrich Schuchardt 2021-09-09 8:30 ` AKASHI Takahiro 2021-09-09 11:46 ` Tom Rini 2021-09-09 12:10 ` AKASHI Takahiro 2021-09-09 12:15 ` Tom Rini 2021-09-09 12:47 ` AKASHI Takahiro 2021-09-09 13:02 ` Tom Rini 2021-09-10 2:17 ` AKASHI Takahiro 2021-09-10 3:14 ` Tom Rini 2021-09-09 12:31 ` Heinrich Schuchardt 2021-09-09 12:44 ` Tom Rini
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.