Hi Samer, Thanks for having a look at this
On Mon, Mar 01, 2021 at 08:44:00PM +0000, Samer El-Haj-Mahmoud wrote: > > > > -----Original Message----- > > From: Heinrich Schuchardt <xypron.g...@gmx.de> > > Sent: Monday, March 1, 2021 2:39 PM > > To: Ilias Apalodimas <ilias.apalodi...@linaro.org>; Grant Likely > > <grant.lik...@arm.com> > > Cc: Boot Architecture Mailman List <boot-architecture@lists.linaro.org>; > > Samer > > El-Haj-Mahmoud <samer.el-haj-mahm...@arm.com>; Ard Biesheuvel > > <a...@kernel.org>; Leif Lindholm <l...@nuviainc.com> > > Subject: Re: EFI_LOAD_FILE2 for initrd standardization > > > ... > > > > > The UEFI spec knows two types of separators for device paths. Both have type > > 0x7F (End of Hardware Device Path) but differ by the sub-type: > > > > Sub-Type 0xff – End Entire Device Path > > Sub-Type 0x01 – End Instance of a Device Path > > > > Field EFI_LOAD_OPTION.FilePathList[] is described in the UEFI spec as > > follows: > > > > "A packed array of UEFI device paths. The first element of the array is a > > device > > path that describes the device and location of the Image for this load > > option." > > > > It is not immediately clear if the separators between the array elements > > are of > > sub-type 0xff or 0x01. The description in the UEFI spec should be reworked > > for > > more clarity. > > > > Agree that this is not clear, and could be interpreted either way. And yes, > agree the UEFI spec needs a clarification > > > > The current EDK II coding requires that the device path identifying the UEFI > > binary (i.e. FilePathList[0] is terminated by a sub-type 0xff end node. > > > > The EDK2 code seems to be incomplete, with a "TODO" to support the > FilePathList[]. In fact, the code calls it "FilePath" to be clear that it is > assuming a single DevicePath (which means a 0xFF sub-type termination) > https://github.com/tianocore/edk2/blob/master/MdeModulePkg%2FLibrary%2FUefiBootManagerLib%2FBmLoadOption.c#L199 > > Searching the code further, I see there is support for parsing multi-instance > device path (separated by END_INSTANCE_DEVICE_PATH_SUBTYPE, or 0x1) in things > like UefiDevicePathLib , parsing code, etc... But it does not seem to be > supported at all in the UEFI Boot Manager implemented in EDK2 > > > > The UEFI variable ConDev is decribed as "The device path of all possible > > console > > input devices". The spec does not refer to it as an array of device paths. > > > > Looking at EDK2, ConXDev are treated as multi-instance devices, with > ConPlatformDxe using AppendDevicePathInstance() and > GetNextDevicePathInstance() to construct / navigate the multiple-instances > > The spec should have defined these as a "multi-instance device path of all > possible console X devices". See my notes below on multi-instance DP. > > > > So it seems that the UEFI spec editors mean by array of device paths that an > > element of the array is separated by sub-type 0xff. Each individual array > > element > > may be a collection of device paths instances separated by 0x01 sub-type end > > nodes. > > > > I am leaning towards this conclusion as well. This is further supported by > the following evidence: > Yes that's exactly what I thought as well and that's what option (2) was trying to describe on my first mail. What I had in there was: Loaded Image device path - end node - VenMedia - Initrd DP - end instance - (repeat) - Initrd DP - end node - other DPs > > * EFI_DEVICE_PATH_UTILITIES_PROTOCOL has a function called > IsDevicePathMultiInstance() along with walker functions > AppendDevicePathInstance(), GetNextDevicePathInstance(). The description of > these functions make it clear that the intention is to treat 0x01 as a > separator between multiple instances of device paths in a multi-instance > device path structure, with 0xff as the final end of DP indicator. For > example, AppendDevicePathInstance() is defined as: > > " This function creates a new device path by appending a copy of the > specified device path instance to a copy of the specified device path in an > allocated buffer. The end-of-device-path device node is moved after the end > of the appended device node and a new end-of-device-path-instance node is > inserted between." > > > * In EFI_USER_INFO , there are user info policy types that leverage the > device path definitions (such as EFI_USER_INFO_ACCESS_FORBID_LOAD), and they > are clear on distinguishing the term "multi-instance device path" from a > "series of UEFI device paths": > > "The record is a series of normal UEFI device paths (not multi-instance > device paths)." > > > * The "multi-instance" usage also shows up in the language of > LocateDevicePath() > > So if the spec intended for FilePathList[] to use the 0x1 separator, they > should have used the term "multi-instance" device path, which is not the case. > > > > > In the device path spec a sentence could be added as follows: > > > > "A packed array of UEFI device paths. The first element of the array is a > > device > > path that describes the device and location of the Image for this load > > option. > > *Each array element is terminated by a sub-type 0xff, End Entire Device Path > > node.*" > > > > Yes. Or could simply say: > "A packed array of UEFI device paths (not multi-instance device paths). The > first element of the array is a device > path that describes the device and location of the Image for this load > option." > > Either way, we can do a "code first" ECR in the UEFI spec. I say code first > to ensure that it remains in public domain, and not blocked by UEFI Forum NDA > until the publication of the next version of the UEFI spec. If we agree, I > can get this process started and propose the language. > That's fine by me. I have most of the u-boot code ready, I'll finish it up and send patches. > > > > Adding initrd or device tree device paths could be implemented as follows: > > > > Array element [0]: > > device path of the binary (e.g. the Linux kernel) terminated by 0xff. > > > > Array element [i], i > 0: > > device paths of the different initial RAM disks separated by 0x01 instance > > end > > nodes and terminated by an 0xff entire path node. > > > > To identify the device paths with all its instances as initrds we can > > prepend a > > VenMedia() device path node with a specific GUID to the entire path. > > > > Array element [j], j > 0: > > device path of the device-tree possibly followed by instances of device > > paths of > > device-tree overlays separated by 0x01 instance end nodes and terminated by > > an > > 0xff entire path node. > > > > To identify the device paths with all its instances as device trees and > > device tree > > overlays we can prepend a VenMedia() device path node with a specific GUID > > to > > the entire path. > > > > One suggestion here is to take into consideration: whether we standardize > this proposal (for loading additional kernel files from element[i], i>0 ) or > not, the FW implementation must still be compliant with the UEFI rules around > booting from a "short form device path" (UEFI spec, section 3.1.2) Ok that's something I'll have to change since I am inserting the entire DP, but as Heinrich pointed out, u-boot already has support for short form device paths so the change shouldn't be too hard. Thanks /Ilias _______________________________________________ boot-architecture mailing list boot-architecture@lists.linaro.org https://lists.linaro.org/mailman/listinfo/boot-architecture
