| Message ID | 20230814090309.1548310-3-sughosh.ganu@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | capsule: Embed the public key ESL as part of build | expand |
On Mon, Aug 14, 2023 at 02:33:07PM +0530, Sughosh Ganu wrote: > The EFI capsule authentication logic in u-boot expects the public key > in the form of an EFI Signature List(ESL) to be provided as part of > the platform's dtb. Currently, the embedding of the ESL file into the > dtb needs to be done manually. > > Add a target for generating a dtsi file which contains the signature > node with the ESL file included as a property under the signature > node. Include the dtsi file in the dtb. This brings the embedding of > the ESL in the dtb into the U-Boot build flow. > > The path to the ESL file is specified through the > CONFIG_EFI_CAPSULE_ESL_FILE symbol. > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > --- > lib/efi_loader/Kconfig | 9 +++++++++ > lib/efi_loader/capsule_esl.dtsi.in | 11 +++++++++++ > scripts/Makefile.lib | 17 +++++++++++++++++ > 3 files changed, 37 insertions(+) > create mode 100644 lib/efi_loader/capsule_esl.dtsi.in > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > index 9989e3f384..f40a62a0ba 100644 > --- a/lib/efi_loader/Kconfig > +++ b/lib/efi_loader/Kconfig > @@ -272,6 +272,15 @@ config EFI_CAPSULE_MAX > Select the max capsule index value used for capsule report > variables. This value is used to create CapsuleMax variable. > > +config EFI_CAPSULE_ESL_FILE > + string "Path to the EFI Signature List File" > + default "" No default. [snip] > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > index f41b16781d..cf4eef0b05 100644 > --- a/scripts/Makefile.lib > +++ b/scripts/Makefile.lib > @@ -334,8 +334,25 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ > ; \ > sed "s:$(pre-tmp):$(<):" $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile) > > +ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE > +quiet_cmd_capsule_esl_gen = CAPSULE_ESL_GEN $@ > +cmd_capsule_esl_gen = \ > + $(shell sed "s:ESL_BIN_FILE:$(capsule_esl_path):" $(capsule_esl_input_file) > $@) > + > +$(obj)/.capsule_esl.dtsi: > + $(call cmd_capsule_esl_gen) > + > +capsule_esl_input_file=$(srctree)/lib/efi_loader/capsule_esl.dtsi.in > +capsule_esl_dtsi = .capsule_esl.dtsi > +capsule_esl_path=$(abspath $(srctree)/$(subst $(quote),,$(CONFIG_EFI_CAPSULE_ESL_FILE))) This seems right. > +include_files += $(capsule_esl_dtsi) > + > +$(obj)/%.dtb: $(src)/%.dts $(DTC) $(obj)/.capsule_esl.dtsi FORCE > + $(call if_changed_dep,dtc) > +else > $(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE > $(call if_changed_dep,dtc) > +endif I think this implies to me that we should have been depending on $(include_files) (once renamed to be less vague) here already ?
hi Tom, On Mon, 14 Aug 2023 at 20:34, Tom Rini <trini@konsulko.com> wrote: > > On Mon, Aug 14, 2023 at 02:33:07PM +0530, Sughosh Ganu wrote: > > The EFI capsule authentication logic in u-boot expects the public key > > in the form of an EFI Signature List(ESL) to be provided as part of > > the platform's dtb. Currently, the embedding of the ESL file into the > > dtb needs to be done manually. > > > > Add a target for generating a dtsi file which contains the signature > > node with the ESL file included as a property under the signature > > node. Include the dtsi file in the dtb. This brings the embedding of > > the ESL in the dtb into the U-Boot build flow. > > > > The path to the ESL file is specified through the > > CONFIG_EFI_CAPSULE_ESL_FILE symbol. > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > --- > > lib/efi_loader/Kconfig | 9 +++++++++ > > lib/efi_loader/capsule_esl.dtsi.in | 11 +++++++++++ > > scripts/Makefile.lib | 17 +++++++++++++++++ > > 3 files changed, 37 insertions(+) > > create mode 100644 lib/efi_loader/capsule_esl.dtsi.in > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > index 9989e3f384..f40a62a0ba 100644 > > --- a/lib/efi_loader/Kconfig > > +++ b/lib/efi_loader/Kconfig > > @@ -272,6 +272,15 @@ config EFI_CAPSULE_MAX > > Select the max capsule index value used for capsule report > > variables. This value is used to create CapsuleMax variable. > > > > +config EFI_CAPSULE_ESL_FILE > > + string "Path to the EFI Signature List File" > > + default "" > > No default. I think Simon wanted to keep this so that it would not break the build if no option was provided. > > [snip] > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > > index f41b16781d..cf4eef0b05 100644 > > --- a/scripts/Makefile.lib > > +++ b/scripts/Makefile.lib > > @@ -334,8 +334,25 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ > > ; \ > > sed "s:$(pre-tmp):$(<):" $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile) > > > > +ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE > > +quiet_cmd_capsule_esl_gen = CAPSULE_ESL_GEN $@ > > +cmd_capsule_esl_gen = \ > > + $(shell sed "s:ESL_BIN_FILE:$(capsule_esl_path):" $(capsule_esl_input_file) > $@) > > + > > +$(obj)/.capsule_esl.dtsi: > > + $(call cmd_capsule_esl_gen) > > + > > +capsule_esl_input_file=$(srctree)/lib/efi_loader/capsule_esl.dtsi.in > > +capsule_esl_dtsi = .capsule_esl.dtsi > > +capsule_esl_path=$(abspath $(srctree)/$(subst $(quote),,$(CONFIG_EFI_CAPSULE_ESL_FILE))) > > This seems right. > > > +include_files += $(capsule_esl_dtsi) > > + > > +$(obj)/%.dtb: $(src)/%.dts $(DTC) $(obj)/.capsule_esl.dtsi FORCE > > + $(call if_changed_dep,dtc) > > +else > > $(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE > > $(call if_changed_dep,dtc) > > +endif > > I think this implies to me that we should have been depending on > $(include_files) (once renamed to be less vague) here already ? Okay. That can be done. I had kept the .capsule_esl.dtsi as a dependency primarily because that file needs to be generated before it can be included. I suppose the same can apply to some other dtsi as well. -sughosh
On Tue, Aug 15, 2023 at 12:19:36AM +0530, Sughosh Ganu wrote: > hi Tom, > > On Mon, 14 Aug 2023 at 20:34, Tom Rini <trini@konsulko.com> wrote: > > > > On Mon, Aug 14, 2023 at 02:33:07PM +0530, Sughosh Ganu wrote: > > > The EFI capsule authentication logic in u-boot expects the public key > > > in the form of an EFI Signature List(ESL) to be provided as part of > > > the platform's dtb. Currently, the embedding of the ESL file into the > > > dtb needs to be done manually. > > > > > > Add a target for generating a dtsi file which contains the signature > > > node with the ESL file included as a property under the signature > > > node. Include the dtsi file in the dtb. This brings the embedding of > > > the ESL in the dtb into the U-Boot build flow. > > > > > > The path to the ESL file is specified through the > > > CONFIG_EFI_CAPSULE_ESL_FILE symbol. > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > > --- > > > lib/efi_loader/Kconfig | 9 +++++++++ > > > lib/efi_loader/capsule_esl.dtsi.in | 11 +++++++++++ > > > scripts/Makefile.lib | 17 +++++++++++++++++ > > > 3 files changed, 37 insertions(+) > > > create mode 100644 lib/efi_loader/capsule_esl.dtsi.in > > > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > > index 9989e3f384..f40a62a0ba 100644 > > > --- a/lib/efi_loader/Kconfig > > > +++ b/lib/efi_loader/Kconfig > > > @@ -272,6 +272,15 @@ config EFI_CAPSULE_MAX > > > Select the max capsule index value used for capsule report > > > variables. This value is used to create CapsuleMax variable. > > > > > > +config EFI_CAPSULE_ESL_FILE > > > + string "Path to the EFI Signature List File" > > > + default "" > > > > No default. > > I think Simon wanted to keep this so that it would not break the build > if no option was provided. No, that means that there's a missing dependency. Blank defaults are almost never right. And "so that we don't break configs" means that something else is wrong, generally a missing dependency. And maybe I cut out too much context here as yes, this option needs to depend on some part of the capsule framework. > > [snip] > > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > > > index f41b16781d..cf4eef0b05 100644 > > > --- a/scripts/Makefile.lib > > > +++ b/scripts/Makefile.lib > > > @@ -334,8 +334,25 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ > > > ; \ > > > sed "s:$(pre-tmp):$(<):" $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile) > > > > > > +ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE > > > +quiet_cmd_capsule_esl_gen = CAPSULE_ESL_GEN $@ > > > +cmd_capsule_esl_gen = \ > > > + $(shell sed "s:ESL_BIN_FILE:$(capsule_esl_path):" $(capsule_esl_input_file) > $@) > > > + > > > +$(obj)/.capsule_esl.dtsi: > > > + $(call cmd_capsule_esl_gen) > > > + > > > +capsule_esl_input_file=$(srctree)/lib/efi_loader/capsule_esl.dtsi.in > > > +capsule_esl_dtsi = .capsule_esl.dtsi > > > +capsule_esl_path=$(abspath $(srctree)/$(subst $(quote),,$(CONFIG_EFI_CAPSULE_ESL_FILE))) > > > > This seems right. > > > > > +include_files += $(capsule_esl_dtsi) > > > + > > > +$(obj)/%.dtb: $(src)/%.dts $(DTC) $(obj)/.capsule_esl.dtsi FORCE > > > + $(call if_changed_dep,dtc) > > > +else > > > $(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE > > > $(call if_changed_dep,dtc) > > > +endif > > > > I think this implies to me that we should have been depending on > > $(include_files) (once renamed to be less vague) here already ? > > Okay. That can be done. I had kept the .capsule_esl.dtsi as a > dependency primarily because that file needs to be generated before it > can be included. I suppose the same can apply to some other dtsi as > well. You shouldn't need to have it be explicit, if it's in dtsi_include_list as there's a rule so make can resolve how to satisfy the dependency. But since we dynamically #include the other files, I don't know that they will be caught with the normal dependency logic. But, it will still be cleaner to have the dtb rules depend on the automagic #include files too, rather than special casing the rule here I believe.
hi Tom, On Tue, 15 Aug 2023 at 00:48, Tom Rini <trini@konsulko.com> wrote: > > On Tue, Aug 15, 2023 at 12:19:36AM +0530, Sughosh Ganu wrote: > > hi Tom, > > > > On Mon, 14 Aug 2023 at 20:34, Tom Rini <trini@konsulko.com> wrote: > > > > > > On Mon, Aug 14, 2023 at 02:33:07PM +0530, Sughosh Ganu wrote: > > > > The EFI capsule authentication logic in u-boot expects the public key > > > > in the form of an EFI Signature List(ESL) to be provided as part of > > > > the platform's dtb. Currently, the embedding of the ESL file into the > > > > dtb needs to be done manually. > > > > > > > > Add a target for generating a dtsi file which contains the signature > > > > node with the ESL file included as a property under the signature > > > > node. Include the dtsi file in the dtb. This brings the embedding of > > > > the ESL in the dtb into the U-Boot build flow. > > > > > > > > The path to the ESL file is specified through the > > > > CONFIG_EFI_CAPSULE_ESL_FILE symbol. > > > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > > > --- > > > > lib/efi_loader/Kconfig | 9 +++++++++ > > > > lib/efi_loader/capsule_esl.dtsi.in | 11 +++++++++++ > > > > scripts/Makefile.lib | 17 +++++++++++++++++ > > > > 3 files changed, 37 insertions(+) > > > > create mode 100644 lib/efi_loader/capsule_esl.dtsi.in > > > > > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > > > index 9989e3f384..f40a62a0ba 100644 > > > > --- a/lib/efi_loader/Kconfig > > > > +++ b/lib/efi_loader/Kconfig > > > > @@ -272,6 +272,15 @@ config EFI_CAPSULE_MAX > > > > Select the max capsule index value used for capsule report > > > > variables. This value is used to create CapsuleMax variable. > > > > > > > > +config EFI_CAPSULE_ESL_FILE > > > > + string "Path to the EFI Signature List File" > > > > + default "" > > > > > > No default. > > > > I think Simon wanted to keep this so that it would not break the build > > if no option was provided. > > No, that means that there's a missing dependency. Blank defaults are > almost never right. And "so that we don't break configs" means that > something else is wrong, generally a missing dependency. And maybe I > cut out too much context here as yes, this option needs to depend on > some part of the capsule framework. Okay. Yes, the above config does actually depend on EFI_CAPSULE_AUTHENTICATE. > > > > [snip] > > > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > > > > index f41b16781d..cf4eef0b05 100644 > > > > --- a/scripts/Makefile.lib > > > > +++ b/scripts/Makefile.lib > > > > @@ -334,8 +334,25 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ > > > > ; \ > > > > sed "s:$(pre-tmp):$(<):" $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile) > > > > > > > > +ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE > > > > +quiet_cmd_capsule_esl_gen = CAPSULE_ESL_GEN $@ > > > > +cmd_capsule_esl_gen = \ > > > > + $(shell sed "s:ESL_BIN_FILE:$(capsule_esl_path):" $(capsule_esl_input_file) > $@) > > > > + > > > > +$(obj)/.capsule_esl.dtsi: > > > > + $(call cmd_capsule_esl_gen) > > > > + > > > > +capsule_esl_input_file=$(srctree)/lib/efi_loader/capsule_esl.dtsi.in > > > > +capsule_esl_dtsi = .capsule_esl.dtsi > > > > +capsule_esl_path=$(abspath $(srctree)/$(subst $(quote),,$(CONFIG_EFI_CAPSULE_ESL_FILE))) > > > > > > This seems right. > > > > > > > +include_files += $(capsule_esl_dtsi) > > > > + > > > > +$(obj)/%.dtb: $(src)/%.dts $(DTC) $(obj)/.capsule_esl.dtsi FORCE > > > > + $(call if_changed_dep,dtc) > > > > +else > > > > $(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE > > > > $(call if_changed_dep,dtc) > > > > +endif > > > > > > I think this implies to me that we should have been depending on > > > $(include_files) (once renamed to be less vague) here already ? > > > > Okay. That can be done. I had kept the .capsule_esl.dtsi as a > > dependency primarily because that file needs to be generated before it > > can be included. I suppose the same can apply to some other dtsi as > > well. > > You shouldn't need to have it be explicit, if it's in dtsi_include_list > as there's a rule so make can resolve how to satisfy the dependency. > But since we dynamically #include the other files, I don't know that > they will be caught with the normal dependency logic. But, it will > still be cleaner to have the dtb rules depend on the automagic #include > files too, rather than special casing the rule here I believe. Okay, in that case, there can be a single rule for making the dtb. Only the logic for generating the .capsule_esl.dtsi can then be put under the ifdef. -sughosh
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index 9989e3f384..f40a62a0ba 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -272,6 +272,15 @@ config EFI_CAPSULE_MAX Select the max capsule index value used for capsule report variables. This value is used to create CapsuleMax variable. +config EFI_CAPSULE_ESL_FILE + string "Path to the EFI Signature List File" + default "" + depends on EFI_CAPSULE_AUTHENTICATE + help + Provides the path to the EFI Signature List file which will + be embedded in the platform's device tree and used for + capsule authentication at the time of capsule update. + config EFI_DEVICE_PATH_TO_TEXT bool "Device path to text protocol" default y diff --git a/lib/efi_loader/capsule_esl.dtsi.in b/lib/efi_loader/capsule_esl.dtsi.in new file mode 100644 index 0000000000..61a9f2b25e --- /dev/null +++ b/lib/efi_loader/capsule_esl.dtsi.in @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: GPL-2.0+ +/** + * Devicetree file with the public key EFI Signature List(ESL) + * node. This file is used to generate the dtsi file to be + * included into the DTB. +*/ +/ { + signature { + capsule-key = /incbin/("ESL_BIN_FILE"); + }; +}; diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index f41b16781d..cf4eef0b05 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -334,8 +334,25 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ ; \ sed "s:$(pre-tmp):$(<):" $(depfile).pre.tmp $(depfile).dtc.tmp > $(depfile) +ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE +quiet_cmd_capsule_esl_gen = CAPSULE_ESL_GEN $@ +cmd_capsule_esl_gen = \ + $(shell sed "s:ESL_BIN_FILE:$(capsule_esl_path):" $(capsule_esl_input_file) > $@) + +$(obj)/.capsule_esl.dtsi: + $(call cmd_capsule_esl_gen) + +capsule_esl_input_file=$(srctree)/lib/efi_loader/capsule_esl.dtsi.in +capsule_esl_dtsi = .capsule_esl.dtsi +capsule_esl_path=$(abspath $(srctree)/$(subst $(quote),,$(CONFIG_EFI_CAPSULE_ESL_FILE))) +include_files += $(capsule_esl_dtsi) + +$(obj)/%.dtb: $(src)/%.dts $(DTC) $(obj)/.capsule_esl.dtsi FORCE + $(call if_changed_dep,dtc) +else $(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE $(call if_changed_dep,dtc) +endif pre-tmp = $(subst $(comma),_,$(dot-target).pre.tmp) dtc-tmp = $(subst $(comma),_,$(dot-target).dts.tmp)
The EFI capsule authentication logic in u-boot expects the public key in the form of an EFI Signature List(ESL) to be provided as part of the platform's dtb. Currently, the embedding of the ESL file into the dtb needs to be done manually. Add a target for generating a dtsi file which contains the signature node with the ESL file included as a property under the signature node. Include the dtsi file in the dtb. This brings the embedding of the ESL in the dtb into the U-Boot build flow. The path to the ESL file is specified through the CONFIG_EFI_CAPSULE_ESL_FILE symbol. Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> --- lib/efi_loader/Kconfig | 9 +++++++++ lib/efi_loader/capsule_esl.dtsi.in | 11 +++++++++++ scripts/Makefile.lib | 17 +++++++++++++++++ 3 files changed, 37 insertions(+) create mode 100644 lib/efi_loader/capsule_esl.dtsi.in