
On 01/07/19 15:09, Leif Lindholm wrote:
Apologies for late reply, back from holidays today.
I'm going to snip a whole lot of context below, since I have no idea what project this is about, and/or what files in that project (no diff hunk headers in the context). Judged from the address list, is this about u-boot perhaps? And adding type definitions to u-boot so they conform to the UEFI spec, and (assuming this "and" is possible) match edk2 practice?
My understanding is this:
- The EDK2 implementation does not conform to the specification; it completely packs the EFI_HII_KEYBOARD_LAYOUT, which the specification does not mention anything about. Since this code is well in the wild, and drivers tested against the current layout need to continuw eorking, I expect the only possible solution is to update the specification to say EFI_HII_KEYBOARD_LAYOUT must be packed.
I'm going to take a pass on this. :)
- The default EDK2 definition of GUID (and hence EFI_GUID) gives it a 32-bit alignment requirement also on 64-bit architectures. In practice, I expect this would only affect (some of the) GUIDs that are members of structs used in UEFI interfaces. But that may already be too common an occurrence to audit and address in EDK2. Does the spec need to change on this also?
The UEFI spec (v2.7) explicitly requires EFI_GUID to be 64-bit aligned, unless specified otherwise. See in "Table 5. Common UEFI Data Types":
EFI_GUID -- 128-bit buffer containing a unique identifier value. Unless otherwise specified, aligned on a 64-bit boundary.
Whether edk2 satisfies that, and if so, how (by chance / by general build flags), I don't know. The code says,
/// /// 128 bit buffer containing a unique identifier value. /// Unless otherwise specified, aligned on a 64 bit boundary. /// typedef struct { UINT32 Data1; UINT16 Data2; UINT16 Data3; UINT8 Data4[8]; } GUID;
I think there may have been an expectation in "MdePkg/Include/Base.h" that the supported compilers would automatically ensure the specified alignment, given the structure definition.
Thanks Laszlo