On Fri, 24 May 2024 at 15:17, Caleb Connolly <caleb.conno...@linaro.org> wrote:
>
>
>
> On 24/05/2024 08:37, Ilias Apalodimas wrote:
> > [...]
> >
> >
> >> config EFI_CAPSULE_FIRMWARE_MANAGEMENT
> >> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> >> index de0d49ebebda..9ef67d1b4405 100644
> >> --- a/lib/efi_loader/efi_capsule.c
> >> +++ b/lib/efi_loader/efi_capsule.c
> >> @@ -19,8 +19,9 @@
> >> #include <mapmem.h>
> >> #include <sort.h>
> >> #include <sysreset.h>
> >> #include <asm/global_data.h>
> >> +#include <uuid.h>
> >>
> >> #include <crypto/pkcs7.h>
> >> #include <crypto/pkcs7_parser.h>
> >> #include <linux/err.h>
> >> @@ -403,8 +404,40 @@ out:
> >> return status;
> >> }
> >> #endif /* CONFIG_EFI_CAPSULE_AUTHENTICATE */
> >>
> >> +#if CONFIG_IS_ENABLED(EFI_CAPSULE_DYNAMIC_UUIDS)
> >> +int efi_capsule_update_info_gen_ids(efi_guid_t *namespace, const char
> >> *soc, const char *model, const char *compatible)
> >> +{
> >> + int i;
> >
> > Perhaps irrelevant to this patch, but do we need to define the name
> > space in platform code?
> > Can't we just put it on a Kconfig and do the dynamic UUIID generation
> > in efi_capsule.c?
>
> Having the namespace be a kconfig option probably makes sense, then it
> could be the thing vendors have to populate for their production boards
> (and they could just have one for all of their products across many
> architectures). So by setting one option they'd have totally unique
> GUIDs for everything.
Exactly and you would be able to reuse the entire machinery without
having to add platform code, since the GUID population would live in
the the efi firmware parts
Cheers
/Ilias
>
> >
> > Thanks
> > /Ilias
> >> +
> >> + if (!soc || !model || !compatible) {
> >> + log_err("%s: soc, model, or compatible not defined\n",
> >> __func__);
> >> + return -EINVAL;
> >> + }
> >> +
> >> + if (!update_info.num_images) {
> >> + log_err("%s: no fw_images, make sure
> >> update_info.num_images is set\n", __func__);
> >> + return -ENODATA;
> >> + }
> >> +
> >> + for (i = 0; i < update_info.num_images; i++) {
> >> + gen_uuid_v5((struct uuid*)namespace,
> >> + (struct uuid
> >> *)&update_info.images[i].image_type_id,
> >> + soc, strlen(soc),
> >> + model, strlen(model),
> >> + compatible, strlen(compatible),
> >> + update_info.images[i].fw_name,
> >> u16_strlen(update_info.images[i].fw_name),
> >> + NULL);
> >> +
> >> + log_debug("Image %ls generated UUID %pUs\n",
> >> update_info.images[i].fw_name,
> >> + &update_info.images[i].image_type_id);
> >> + }
> >> +
> >> + return 0;
> >> +}
> >> +#endif
> >> +
> >> static __maybe_unused bool fwu_empty_capsule(struct efi_capsule_header
> >> *capsule)
> >> {
> >> return !guidcmp(&capsule->capsule_guid,
> >> &fwu_guid_os_request_fw_revert) ||
> >>
> >> --
> >> 2.44.0
> >>
>
> --
> // Caleb (they/them)
