Hi Sughosh, On Fri, 9 Apr 2021 at 23:27, Sughosh Ganu <sughosh.g...@linaro.org> wrote: > > hi Simon, > > On Fri, 9 Apr 2021 at 05:26, Simon Glass <s...@chromium.org> wrote: >> >> Hi Sughosh, >> >> On Thu, 8 Apr 2021 at 18:53, Sughosh Ganu <sughosh.g...@linaro.org> wrote: >> > >> > hi Simon, >> > >> > On Wed, 7 Apr 2021 at 21:44, Simon Glass <s...@chromium.org> wrote: >> >> >> >> Hi, >> >> >> >> On Wed, 7 Apr 2021 at 23:54, Sughosh Ganu <sughosh.g...@linaro.org> wrote: >> >> > >> >> > Patch 1 fixes an issue of selection of IMAGE_SIGN_INFO config option >> >> > when capsule authentication is enabled. >> >> > >> >> > Patch 2 add two config symbols, EFI_PKEY_DTB_EMBED and EFI_PKEY_FILE >> >> > which are used for enabling embedding of the public key in the dtb, >> >> > and specifying the esl file name. >> >> > >> >> > Patch 3 moves efi_capsule_auth_enabled as a weak function, which can >> >> > be used as a default mechanism for checking if capsule authentication >> >> > has been enabled. >> >> > >> >> > Patch 4 adds a default weak function for retrieving the public key >> >> > from the platform's dtb. >> >> > >> >> > Patch 5 adds the functionality to embed the esl file into the >> >> > platform's dtb during the platform build. >> >> > >> >> > I have tested this functionality on the STM32MP157C DK2 board. >> >> > >> >> > [1] - https://lists.denx.de/pipermail/u-boot/2021-March/442867.html >> >> > >> >> > Sughosh Ganu (5): >> >> > efi_loader: Kconfig: Select IMAGE_SIGN_INFO when capsule >> >> > authentication is enabled >> >> > efi_loader: Kconfig: Add symbols for embedding the public key into the >> >> > platform's dtb >> >> > efi_capsule: Add a weak function to check whether capsule >> >> > authentication is enabled >> >> > efi_capsule: Add a weak function to get the public key needed for >> >> > capsule authentication >> >> > Makefile: Add provision for embedding public key in platform's dtb >> >> > >> >> > Makefile | 10 ++++++ >> >> > board/emulation/common/qemu_capsule.c | 6 ---- >> >> > lib/efi_loader/Kconfig | 16 ++++++++++ >> >> > lib/efi_loader/efi_capsule.c | 44 ++++++++++++++++++++++++--- >> >> > 4 files changed, 66 insertions(+), 10 deletions(-) >> >> > >> >> > -- >> >> > 2.17.1 >> >> > >> >> >> >> We need to rethink the use of weak functions for this sort of thing, >> >> or we will end up with an unnavigable mess at some point. If we need >> >> to adjust the flow of boot, let's adjust the flow rather than adding >> >> hooks everywhere. >> > >> > >> > There are two weak functions being added. One is for retrieving the public >> > key to be used for the capsule authentication, and the other is for >> > checking for whether capsule authentication has been enabled. The reason >> > why a weak function is needed is because platforms can have other >> > mechanisms for retrieval of the public key or for testing if capsule >> > authentication has been enabled. >> > >> > If we consider the case of public key retrieval, the majority of platforms >> > would be built with the device tree concatenated with the u-boot binary. >> > The weak function would cater to all of those platforms -- having a weak >> > function would mean that we are not required to repeat the same >> > functionality for every platform that uses the same mechanism for >> > extracting the public key. However, there would be platforms where the >> > public key is obtained through a different mechanism which is platform >> > specific. Those platforms would have to define their own function to get >> > the public key. Same for checking whether capsule authentication feature >> > has been enabled or not. >> > >> > -sughosh >> >> Yes, I get it, but if this is to be a critical feature and part of the >> grand new design for verified boot using UEFI, surely we should be >> building a new front door, not digging a tunnel in the bathroom :-) >> >> We can either use drivers with driver model, or perhaps have a Kconfig >> that enables calling the function (so we get a link error if not >> provided). Or if there will be more than one handler, a linker_list. > > > I will use the Kconfig symbol for selecting the function to use for > retrieving the public key. So, in the current scenario, the function would be > under the CONFIG_EFI_PKEY_DTB_EMBED symbol. This should cater to the majority > of the cases. If some platform wants to use a different method to get the > public key, it would need to define it's own function.
I wonder why another method would be needed, but if it, then someone can turn your thing into a choice, I suppose. Regards, Simon