Felix, I agree with your points and agree that the C structure definition from UEFI 2.5 should be added to UefiSpec.h.
I would like to see the comment block above the structure state that some of the variable length structure fields are commented out on purpose. Mike -----Original Message----- From: Felix Poludov [mailto:[email protected]] Sent: Thursday, May 07, 2015 11:50 AM To: [email protected]; Andrew Fish; Kinney, Michael D; Ni, Ruiyu Subject: RE: [edk2] [Patch] MdePkg: Add EFI_LOAD_OPTION definition. The EFI_LOAD_OPTION was not defined in the UEFI specification, but it is now (starting from UEFI 2.5). The situation with EFI_LOAD_OPTION was somewhat confusing. The type was mentioned in the specification, but was not explicitly defined. I think having the type explicitly defined has some value. Specifically: - Having a type that partially describes a data structure is, in my opinion, better that usage of a void pointer. It gives a chance to the compiler to do some error checking. It also makes the code more readable. - Boot option format can be looked up in the header such as UefiSpec.h, which is much faster than opening the specification - Explicit type definition adds EFI_LOAD_OPTION identifier to a domain of UEFI reserved types, which prevents others from defining a type with the same name. For example ArmPkg defines EFI_LOAD_OPTION as a pointer to UINT8 -----Original Message----- From: Ni, Ruiyu [mailto:[email protected]] Sent: Tuesday, May 05, 2015 9:39 PM To: [email protected]; Andrew Fish; Kinney, Michael D Subject: Re: [edk2] [Patch] MdePkg: Add EFI_LOAD_OPTION definition. Mike, Andrew, I fully agree with your comments that the C structure is not useful in code. I will not add this C structure. All, Any concerns? Thanks, Ray -----Original Message----- From: Andrew Fish [mailto:[email protected]] Sent: Saturday, May 2, 2015 1:49 AM To: [email protected] Cc: Kinney, Michael D; Ni, Ruiyu Subject: Re: [edk2] [Patch] MdePkg: Add EFI_LOAD_OPTION definition. > On May 1, 2015, at 10:40 AM, Kinney, Michael D <[email protected]> > wrote: > > Ray, > > The idea of adding EFI_LOAD_OPTION structure to MdePkg .h files has been > considered before. > > This is not defined as a C data structure in the UEFI Specification. > Instead, it is described as a binary buffer that must be parsed from > beginning to end. > > I recommend we do not add this C data structure to UefiSpec.h. > I agree as most of the this C struct is just a comment, that follows the spec. Given it only has two elements it is not going to be that useful in code. Thanks, Andrew Fish > Best regards, > > Mike > > > -----Original Message----- > From: Ruiyu Ni [mailto:[email protected]] > Sent: Monday, April 27, 2015 11:17 PM > To: [email protected] > Subject: [edk2] [Patch] MdePkg: Add EFI_LOAD_OPTION definition. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ruiyu Ni <[email protected]> > Reviewed-by: Liming Gao <[email protected]> > --- > MdePkg/Include/Uefi/UefiSpec.h | 42 ++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 42 insertions(+) > > diff --git a/MdePkg/Include/Uefi/UefiSpec.h b/MdePkg/Include/Uefi/UefiSpec.h > index 64105c2..5e23694 100644 > --- a/MdePkg/Include/Uefi/UefiSpec.h > +++ b/MdePkg/Include/Uefi/UefiSpec.h > @@ -2026,6 +2026,48 @@ EFI_STATUS > ); > > // > +// EFI Load Option > +// > +typedef struct _EFI_LOAD_OPTION { > + /// > + /// The attributes for this load option entry. All unused bits must be zero > + /// and are reserved by the UEFI specification for future growth. > + /// > + UINT32 Attributes; > + /// > + /// Length in bytes of the FilePathList. OptionalData starts at offset > + /// sizeof(UINT32) + sizeof(UINT16) + StrSize(Description) + > FilePathListLength > + /// of the EFI_LOAD_OPTION descriptor. > + /// > + UINT16 FilePathListLength; > + /// > + /// The user readable description for the load option. > + /// This field ends with a Null character. > + /// > + // CHAR16 Description[]; > + /// > + /// 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. The FilePathList[0] is specific to the device type. Other > + /// device paths may optionally exist in the FilePathList, but their usage > is > + /// OSV specific. Each element in the array is variable length, and ends at > + /// the device path end structure. Because the size of Description is > + /// arbitrary, this data structure is not guaranteed to be aligned on a > + /// natural boundary. This data structure may have to be copied to an > aligned > + /// natural boundary before it is used. > + /// > + // EFI_DEVICE_PATH_PROTOCOL FilePathList[]; > + /// > + /// The remaining bytes in the load option descriptor are a binary data > buffer > + /// that is passed to the loaded image. If the field is zero bytes long, a > + /// NULL pointer is passed to the loaded image. The number of bytes in > + /// OptionalData can be computed by subtracting the starting offset of > + /// OptionalData from total size in bytes of the EFI_LOAD_OPTION. > + /// > + // UINT8 OptionalData[]; > +} EFI_LOAD_OPTION; > + > +// > // EFI Load Options Attributes > // > #define LOAD_OPTION_ACTIVE 0x00000001 > -- > 1.9.5.msysgit.1 > > > ------------------------------------------------------------------------------ > One dashboard for servers and applications across Physical-Virtual-Cloud > Widest out-of-the-box monitoring support with 50+ applications > Performance metrics, stats and reports that give you Actionable Insights > Deep dive visibility with transaction tracing using APM Insight. > http://ad.doubleclick.net/ddm/clk/290420510;117567292;y > _______________________________________________ > edk2-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/edk2-devel > > ------------------------------------------------------------------------------ > One dashboard for servers and applications across Physical-Virtual-Cloud > Widest out-of-the-box monitoring support with 50+ applications > Performance metrics, stats and reports that give you Actionable Insights > Deep dive visibility with transaction tracing using APM Insight. > http://ad.doubleclick.net/ddm/clk/290420510;117567292;y > _______________________________________________ > edk2-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/edk2-devel ------------------------------------------------------------------------------ One dashboard for servers and applications across Physical-Virtual-Cloud Widest out-of-the-box monitoring support with 50+ applications Performance metrics, stats and reports that give you Actionable Insights Deep dive visibility with transaction tracing using APM Insight. http://ad.doubleclick.net/ddm/clk/290420510;117567292;y _______________________________________________ edk2-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/edk2-devel The information contained in this message may be confidential and proprietary to American Megatrends, Inc. This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission. ------------------------------------------------------------------------------ One dashboard for servers and applications across Physical-Virtual-Cloud Widest out-of-the-box monitoring support with 50+ applications Performance metrics, stats and reports that give you Actionable Insights Deep dive visibility with transaction tracing using APM Insight. http://ad.doubleclick.net/ddm/clk/290420510;117567292;y _______________________________________________ edk2-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/edk2-devel
