[PATCH v2 2/8] cmd: eficonfig: add support for setting fdt

[Ilias Apalodimas]

On Tue, 28 May 2024 at 19:08, Heinrich Schuchardt
<heinrich.schuchardt at canonical.com> wrote:
>
> On 28.05.24 17:16, Ilias Apalodimas wrote:
> > Hi Heinrich,
> >
> > [...]
> >
> >>
> >> -       const struct efi_initrd_dp id_dp = {
> >> +       const struct efi_initrd_dp initrd_prefix = {
> >>                  .vendor = {
> >>                          {
> >>                          DEVICE_PATH_TYPE_MEDIA_DEVICE,
> >>                          DEVICE_PATH_SUB_TYPE_VENDOR_PATH,
> >> -                       sizeof(id_dp.vendor),
> >> +                       sizeof(initrd_prefix.vendor),
> >>                          },
> >>                          EFI_INITRD_MEDIA_GUID,
> >>                  },
> >>                  .end = {
> >>                          DEVICE_PATH_TYPE_END,
> >>                          DEVICE_PATH_SUB_TYPE_END,
> >> -                       sizeof(id_dp.end),
> >> +                       sizeof(initrd_prefix.end),
> >> +               }
> >> +       };
> >> +
> >> +       const struct efi_initrd_dp fdt_prefix = {
> >
> > We need to rename efi_initrd_dp to something more generic in the future
> >
> >> +               .vendor = {
> >> +                       {
> >> +                       DEVICE_PATH_TYPE_MEDIA_DEVICE,
> >> +                       DEVICE_PATH_SUB_TYPE_VENDOR_PATH,
> >> +                       sizeof(fdt_prefix.vendor),
> >> +                       },
> >> +                       EFI_FDT_GUID,
> >> +               },
> >> +               .end = {
> >> +                       DEVICE_PATH_TYPE_END,
> >> +                       DEVICE_PATH_SUB_TYPE_END,
> >> +                       sizeof(initrd_prefix.end),
> >>                  }
> >>          };
> >>
> >
> > [...]
> >
> >> +/**
> >> + * efi_load_option_dp_join() - join device-paths for load option
> >> + *
> >> + * @dp:                in: binary device-path, out: joined device-path
> >> + * @dp_size:   size of joined device-path
> >> + * @initrd_dp: initrd device-path or NULL
> >> + * @fdt_dp:    device-tree device-path or NULL
> >> + * Return:     status_code
> >> + */
> >> +efi_status_t efi_load_option_dp_join(struct efi_device_path **dp,
> >> +                                    size_t *dp_size,
> >> +                                    struct efi_device_path *initrd_dp,
> >> +                                    struct efi_device_path *fdt_dp)
> >> +{
> >> +       if (!dp)
> >
> > Should we add && !*dp here?
>
> efi_dp_concat() handles the case of one or both device-paths being NULL.

Fair enough

>
> >
> >> +               return EFI_INVALID_PARAMETER;
> >> +
> >> +       *dp_size = efi_dp_size(*dp);
> >> +
> >> +       if (initrd_dp) {
> >> +               struct efi_device_path *tmp_dp = *dp;
> >> +
> >> +               *dp = efi_dp_concat(tmp_dp, initrd_dp, *dp_size);
> >> +               efi_free_pool(tmp_dp);
> >> +               if (!*dp)
> >> +                       return EFI_OUT_OF_RESOURCES;
> >> +               *dp_size += efi_dp_size(initrd_dp) + sizeof(END);
> >> +       }
> >> +
> >> +       if (fdt_dp) {
> >> +               struct efi_device_path *tmp_dp = *dp;
> >> +
> >> +               *dp = efi_dp_concat(tmp_dp, fdt_dp, *dp_size);
> >> +               efi_free_pool(tmp_dp);
> >> +               if (!dp)
> >> +                       return EFI_OUT_OF_RESOURCES;
> >> +               *dp_size += efi_dp_size(fdt_dp) + sizeof(END);
> >> +       }
> >> +
> >> +       *dp_size += sizeof(END);
> >
> > Why do we have to account for the end node twice if either fdt_dp or
> > initrd_dp are found?
>
> This is the length of the END node of the binary.

ah yes correct.

Feel free to add
Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
but we need to fix the naming at some point

>
> Best regards
>
> Heinrich
>
> >
> > Thanks
> > /Ilias
> >> +
> >> +       return EFI_SUCCESS;
> >> +}
> >> +
> >>   const struct guid_to_hash_map {
> >>          efi_guid_t guid;
> >>          const char algo[32];
> >> --
> >> 2.43.0
> >>
>