On Tue, 28 May 2024 at 19:08, Heinrich Schuchardt
<heinrich.schucha...@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.apalodi...@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
> >>
>
