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

[Heinrich Schuchardt]

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.

> 
>> +               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.

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
>>