> Date: Tue, 14 May 2024 14:49:47 +0200 > From: Heinrich Schuchardt <heinrich.schucha...@canonical.com>
Hi Heinrich, > On 4/26/24 17:47, Ilias Apalodimas wrote: > > Hi Heinrich > > > > On Fri, 26 Apr 2024 at 17:53, Heinrich Schuchardt > > <heinrich.schucha...@canonical.com> wrote: > >> > >> On 26.04.24 16:30, Ilias Apalodimas wrote: > >>> Hi Heinrich, > >>> > >>> On Fri, 26 Apr 2024 at 17:13, Heinrich Schuchardt > >>> <heinrich.schucha...@canonical.com> wrote: > >>>> > >>>> Provide a function to append a device_path to a list of device paths > >>>> that is separated by final end nodes. > >>>> > >>>> Signed-off-by: Heinrich Schuchardt <heinrich.schucha...@canonical.com> > >>>> --- > >>>> include/efi_loader.h | 3 +++ > >>>> lib/efi_loader/efi_device_path.c | 31 +++++++++++++++++++++++++++++++ > >>>> 2 files changed, 34 insertions(+) > >>>> > >>>> diff --git a/include/efi_loader.h b/include/efi_loader.h > >>>> index 9600941aa32..a7d7b8324f1 100644 > >>>> --- a/include/efi_loader.h > >>>> +++ b/include/efi_loader.h > >>>> @@ -944,6 +944,9 @@ struct efi_load_option { > >>>> > >>>> struct efi_device_path *efi_dp_from_lo(struct efi_load_option *lo, > >>>> const efi_guid_t *guid); > >>>> +struct efi_device_path *efi_dp_merge(const struct efi_device_path *dp1, > >>>> + efi_uintn_t *size, > >>>> + const struct efi_device_path *dp2); > >>>> struct efi_device_path *efi_dp_concat(const struct efi_device_path > >>>> *dp1, > >>>> const struct efi_device_path > >>>> *dp2, > >>>> bool split_end_node); > >>>> diff --git a/lib/efi_loader/efi_device_path.c > >>>> b/lib/efi_loader/efi_device_path.c > >>>> index 46aa59b9e40..16cbe41d32f 100644 > >>>> --- a/lib/efi_loader/efi_device_path.c > >>>> +++ b/lib/efi_loader/efi_device_path.c > >>>> @@ -270,6 +270,37 @@ struct efi_device_path *efi_dp_dup(const struct > >>>> efi_device_path *dp) > >>>> return ndp; > >>>> } > >>>> > >>>> +/** > >>>> + * efi_dp_merge() - Concatenate two device paths separated by final end > >>>> node > >>>> + * > >>>> + * @dp1: first device path > >>>> + * @size: pointer to length of @dp1, total size on return > >>>> + * @dp2: second device path > >>>> + * > >>>> + * Return: concatenated device path or NULL > >>>> + */ > >>>> +struct efi_device_path *efi_dp_merge(const struct efi_device_path *dp1, > >>>> + efi_uintn_t *size, > >>>> + const struct efi_device_path *dp2) > >>>> +{ > >>>> + efi_uintn_t len, len1, len2; > >>>> + struct efi_device_path *dp; > >>>> + u8 *p; > >>>> + > >>>> + len1 = *size; > >>>> + len2 = efi_dp_size(dp2) + sizeof(END); > >>>> + len = len1 + len2; > >>>> + dp = efi_alloc(len); > >>>> + if (!dp) > >>>> + return NULL; > >>>> + memcpy(dp, dp1, len1); > >>>> + p = (u8 *)dp + len1; > >>>> + memcpy(p, dp2, len2); > >>>> + *size = len; > >>> > >>> Can't we just use efi_dp_concat()? > >> > >> efi_dp_concat cannot be used to put a device-path end-node in between > >> two device-paths that are concatenated. We need that separator in the > >> load options. > > > > I am not sure I am following you. It's been a few years so please bear > > with me until I manage to re-read that code and page it back to > > memory. > > > > What I remember though is that the format of the DP looks like this: > > > > Loaded image DP - end node - initrd GUID DP (without and end node) - > > initrd - end of device path. > > So to jump from the special initrd GUID to the actual DP that contains > > the initrd you need to do an efi_dp_next(). Also, efi_dp_concat() will > > inject an end node with a DEVICE_PATH_SUB_TYPE_END type if the third > > argument is 'true'. > > > > What am I missing? > > Let us assume > > dp1 = /vmlinux/endnode/initrd/endnode > dp2 = /dtb/endnode > > and we invoke dp_concat(dp1, dp2, true): > > sz1 is calculated via the efi_dp_size() function which looks for the > *first* device-path end node. > > sz1 = efi_dp_size(dp1) = sizeof(/vmlinux) > > sz1 will not include initrd and its endnode. > > So dp_concat(/vmlinux/endnode/initrd/endnode, /dtb/endnode, true) > will return: > > /vmlinux/endnode/dtb/endnode > > but in our boot option we want > > /vmlinux/endnode/initrd/endnode/dtb/endnode That makes me realize a potential flaw with the design of this boot option. The concept of an initrd is Linux-specific, but the concept of loading a DTB to pass along to an EFI image far less so. So how would this work if I want to only pass the DTB? Would that be: /vmlinux/endnode/endnode/dtb/endnode ?