Star,
Thank you for your remark. Patch v2 has been sent to the mailing list with the changes you proposed. Regards, Marvin. Von: Zeng, Star<mailto:star.z...@intel.com> Gesendet: Dienstag, 22. März 2016 06:32 An: Marvin Häuser<mailto:marvin.haeu...@outlook.com>; edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org> Cc: michael.d.kin...@intel.com<mailto:michael.d.kin...@intel.com>; liming....@intel.com<mailto:liming....@intel.com> Betreff: Re: [edk2] [PATCH v1 1/1] MdePkg: Move SMBIOS data into the IndustryStandard header. Marvin, One comment below to the patch. On 2016/3/22 2:41, Marvin Häuser wrote: > As the SMBIOS table types belong to the SMBIOS standard, they were > moved from the SMBIOS Protocol header into the SMBIOS > IndustryStandard header without the EFI_-prefix. Defines with the > EFI_-prefix have been kept in the Protocol header for > backwards-compatibility, resolving to the IndustryStandard defines. > The same has been done with the C types. > > The SMBIOS table header structure had been duplicated - > SMBIOS_STRUCTURE in the IndustryStandard header and > EFI_SMBIOS_TABLE_HEADER in the Protocol file - and thus the > Protocol type was replaced with a typedef to the InudstryStandard's. > This doesn't only make it easier to maintain, but it also prevents > potential future issues as the Protocol type has been aligned, while > the standard and the IndustryStandard header declare it as > byte-packed. > This has worked well till now only because the members of the > structure do not require alignment yet. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Marvin Haeuser <marvin.haeu...@outlook.com> > --- > MdePkg/Include/IndustryStandard/SmBios.h | 87 +++++++++++- > MdePkg/Include/Protocol/Smbios.h | 139 ++++++++------------ > 2 files changed, 137 insertions(+), 89 deletions(-) > > diff --git a/MdePkg/Include/IndustryStandard/SmBios.h > b/MdePkg/Include/IndustryStandard/SmBios.h > index b7c54f2fac12..1bb72c053da1 100644 > --- a/MdePkg/Include/IndustryStandard/SmBios.h > +++ b/MdePkg/Include/IndustryStandard/SmBios.h > @@ -51,6 +51,53 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER > EXPRESS OR IMPLIED. > // > #define SMBIOS_3_0_TABLE_MAX_LENGTH 0xFFFFFFFF > > +// > +// SMBIOS type macros which is according to SMBIOS 2.7 specification. > +// > +#define SMBIOS_TYPE_BIOS_INFORMATION 0 > +#define SMBIOS_TYPE_SYSTEM_INFORMATION 1 > +#define SMBIOS_TYPE_BASEBOARD_INFORMATION 2 > +#define SMBIOS_TYPE_SYSTEM_ENCLOSURE 3 > +#define SMBIOS_TYPE_PROCESSOR_INFORMATION 4 > +#define SMBIOS_TYPE_MEMORY_CONTROLLER_INFORMATION 5 > +#define SMBIOS_TYPE_MEMORY_MODULE_INFORMATON 6 > +#define SMBIOS_TYPE_CACHE_INFORMATION 7 > +#define SMBIOS_TYPE_PORT_CONNECTOR_INFORMATION 8 > +#define SMBIOS_TYPE_SYSTEM_SLOTS 9 > +#define SMBIOS_TYPE_ONBOARD_DEVICE_INFORMATION 10 > +#define SMBIOS_TYPE_OEM_STRINGS 11 > +#define SMBIOS_TYPE_SYSTEM_CONFIGURATION_OPTIONS 12 > +#define SMBIOS_TYPE_BIOS_LANGUAGE_INFORMATION 13 > +#define SMBIOS_TYPE_GROUP_ASSOCIATIONS 14 > +#define SMBIOS_TYPE_SYSTEM_EVENT_LOG 15 > +#define SMBIOS_TYPE_PHYSICAL_MEMORY_ARRAY 16 > +#define SMBIOS_TYPE_MEMORY_DEVICE 17 > +#define SMBIOS_TYPE_32BIT_MEMORY_ERROR_INFORMATION 18 > +#define SMBIOS_TYPE_MEMORY_ARRAY_MAPPED_ADDRESS 19 > +#define SMBIOS_TYPE_MEMORY_DEVICE_MAPPED_ADDRESS 20 > +#define SMBIOS_TYPE_BUILT_IN_POINTING_DEVICE 21 > +#define SMBIOS_TYPE_PORTABLE_BATTERY 22 > +#define SMBIOS_TYPE_SYSTEM_RESET 23 > +#define SMBIOS_TYPE_HARDWARE_SECURITY 24 > +#define SMBIOS_TYPE_SYSTEM_POWER_CONTROLS 25 > +#define SMBIOS_TYPE_VOLTAGE_PROBE 26 > +#define SMBIOS_TYPE_COOLING_DEVICE 27 > +#define SMBIOS_TYPE_TEMPERATURE_PROBE 28 > +#define SMBIOS_TYPE_ELECTRICAL_CURRENT_PROBE 29 > +#define SMBIOS_TYPE_OUT_OF_BAND_REMOTE_ACCESS 30 > +#define SMBIOS_TYPE_BOOT_INTEGRITY_SERVICE 31 > +#define SMBIOS_TYPE_SYSTEM_BOOT_INFORMATION 32 > +#define SMBIOS_TYPE_64BIT_MEMORY_ERROR_INFORMATION 33 > +#define SMBIOS_TYPE_MANAGEMENT_DEVICE 34 > +#define SMBIOS_TYPE_MANAGEMENT_DEVICE_COMPONENT 35 > +#define SMBIOS_TYPE_MANAGEMENT_DEVICE_THRESHOLD_DATA 36 > +#define SMBIOS_TYPE_MEMORY_CHANNEL 37 > +#define SMBIOS_TYPE_IPMI_DEVICE_INFORMATION 38 > +#define SMBIOS_TYPE_SYSTEM_POWER_SUPPLY 39 > +#define SMBIOS_TYPE_ADDITIONAL_INFORMATION 40 > +#define SMBIOS_TYPE_ONBOARD_DEVICES_EXTENDED_INFORMATION 41 > +#define SMBIOS_TYPE_MANAGEMENT_CONTROLLER_HOST_INTERFACE 42 > + > /// > /// Inactive type is added from SMBIOS 2.2. Reference SMBIOS 2.6, chapter > 3.3.43. > /// Upper-level software that interprets the SMBIOS structure-table should > bypass an > @@ -64,6 +111,40 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER > EXPRESS OR IMPLIED. > /// > #define SMBIOS_TYPE_END_OF_TABLE 0x007F > > +#define SMBIOS_OEM_BEGIN 128 > +#define SMBIOS_OEM_END 255 > + > +/// > +/// Text strings associated with a given SMBIOS structure are returned in > the dmiStrucBuffer, appended directly after > +/// the formatted portion of the structure. This method of returning string > information eliminates the need for > +/// application software to deal with pointers embedded in the SMBIOS > structure. Each string is terminated with a null > +/// (00h) BYTE and the set of strings is terminated with an additional null > (00h) BYTE. When the formatted portion of > +/// a SMBIOS structure references a string, it does so by specifying a > non-zero string number within the structure's > +/// string-set. For example, if a string field contains 02h, it references > the second string following the formatted portion > +/// of the SMBIOS structure. If a string field references no string, a null > (0) is placed in that string field. If the > +/// formatted portion of the structure contains string-reference fields and > all the string fields are set to 0 (no string > +/// references), the formatted section of the structure is followed by two > null (00h) BYTES. > +/// > +typedef UINT8 SMBIOS_STRING; Suggest to move the comments to SMBIOS_TABLE_STRING, and let EFI_SMBIOS_STRING in Protocol/Smbios.h to reference SMBIOS_TABLE_STRING. Then no need to define SMBIOS_STRING, as it is duplicated with SMBIOS_TABLE_STRING. Thanks, Star > + > +/// > +/// Types 0 through 127 (7Fh) are reserved for and defined by this > +/// specification. Types 128 through 256 (80h to FFh) are available for > system- and OEM-specific information. > +/// > +typedef UINT8 SMBIOS_TYPE; > + > +/// > +/// Specifies the structure's handle, a unique 16-bit number in the range 0 > to 0FFFEh (for version > +/// 2.0) or 0 to 0FEFFh (for version 2.1 and later). The handle can be used > with the Get SMBIOS > +/// Structure function to retrieve a specific structure; the handle numbers > are not required to be > +/// contiguous. For v2.1 and later, handle values in the range 0FF00h to > 0FFFFh are reserved for > +/// use by this specification. > +/// If the system configuration changes, a previously assigned handle might > no longer exist. > +/// However once a handle has been assigned by the BIOS, the BIOS cannot > re-assign that handle > +/// number to another structure. > +/// > +typedef UINT16 SMBIOS_HANDLE; > + > /// > /// Smbios Table Entry Point Structure. > /// > @@ -102,9 +183,9 @@ typedef struct { > /// The Smbios structure header. > /// > typedef struct { > - UINT8 Type; > - UINT8 Length; > - UINT16 Handle; > + SMBIOS_TYPE Type; > + UINT8 Length; > + SMBIOS_HANDLE Handle; > } SMBIOS_STRUCTURE; > > /// > diff --git a/MdePkg/Include/Protocol/Smbios.h > b/MdePkg/Include/Protocol/Smbios.h > index 0f79e419d99a..4cdb0763f858 100644 > --- a/MdePkg/Include/Protocol/Smbios.h > +++ b/MdePkg/Include/Protocol/Smbios.h > @@ -27,96 +27,63 @@ > #ifndef __SMBIOS_PROTOCOL_H__ > #define __SMBIOS_PROTOCOL_H__ > > +#include <IndustryStandard/SmBios.h> > + > #define EFI_SMBIOS_PROTOCOL_GUID \ > { 0x3583ff6, 0xcb36, 0x4940, { 0x94, 0x7e, 0xb9, 0xb3, 0x9f, 0x4a, > 0xfa, 0xf7 }} > - > -// > -// SMBIOS type macros which is according to SMBIOS 2.7 specification. > -// > -#define EFI_SMBIOS_TYPE_BIOS_INFORMATION 0 > -#define EFI_SMBIOS_TYPE_SYSTEM_INFORMATION 1 > -#define EFI_SMBIOS_TYPE_BASEBOARD_INFORMATION 2 > -#define EFI_SMBIOS_TYPE_SYSTEM_ENCLOSURE 3 > -#define EFI_SMBIOS_TYPE_PROCESSOR_INFORMATION 4 > -#define EFI_SMBIOS_TYPE_MEMORY_CONTROLLER_INFORMATION 5 > -#define EFI_SMBIOS_TYPE_MEMORY_MODULE_INFORMATON 6 > -#define EFI_SMBIOS_TYPE_CACHE_INFORMATION 7 > -#define EFI_SMBIOS_TYPE_PORT_CONNECTOR_INFORMATION 8 > -#define EFI_SMBIOS_TYPE_SYSTEM_SLOTS 9 > -#define EFI_SMBIOS_TYPE_ONBOARD_DEVICE_INFORMATION 10 > -#define EFI_SMBIOS_TYPE_OEM_STRINGS 11 > -#define EFI_SMBIOS_TYPE_SYSTEM_CONFIGURATION_OPTIONS 12 > -#define EFI_SMBIOS_TYPE_BIOS_LANGUAGE_INFORMATION 13 > -#define EFI_SMBIOS_TYPE_GROUP_ASSOCIATIONS 14 > -#define EFI_SMBIOS_TYPE_SYSTEM_EVENT_LOG 15 > -#define EFI_SMBIOS_TYPE_PHYSICAL_MEMORY_ARRAY 16 > -#define EFI_SMBIOS_TYPE_MEMORY_DEVICE 17 > -#define EFI_SMBIOS_TYPE_32BIT_MEMORY_ERROR_INFORMATION 18 > -#define EFI_SMBIOS_TYPE_MEMORY_ARRAY_MAPPED_ADDRESS 19 > -#define EFI_SMBIOS_TYPE_MEMORY_DEVICE_MAPPED_ADDRESS 20 > -#define EFI_SMBIOS_TYPE_BUILT_IN_POINTING_DEVICE 21 > -#define EFI_SMBIOS_TYPE_PORTABLE_BATTERY 22 > -#define EFI_SMBIOS_TYPE_SYSTEM_RESET 23 > -#define EFI_SMBIOS_TYPE_HARDWARE_SECURITY 24 > -#define EFI_SMBIOS_TYPE_SYSTEM_POWER_CONTROLS 25 > -#define EFI_SMBIOS_TYPE_VOLTAGE_PROBE 26 > -#define EFI_SMBIOS_TYPE_COOLING_DEVICE 27 > -#define EFI_SMBIOS_TYPE_TEMPERATURE_PROBE 28 > -#define EFI_SMBIOS_TYPE_ELECTRICAL_CURRENT_PROBE 29 > -#define EFI_SMBIOS_TYPE_OUT_OF_BAND_REMOTE_ACCESS 30 > -#define EFI_SMBIOS_TYPE_BOOT_INTEGRITY_SERVICE 31 > -#define EFI_SMBIOS_TYPE_SYSTEM_BOOT_INFORMATION 32 > -#define EFI_SMBIOS_TYPE_64BIT_MEMORY_ERROR_INFORMATION 33 > -#define EFI_SMBIOS_TYPE_MANAGEMENT_DEVICE 34 > -#define EFI_SMBIOS_TYPE_MANAGEMENT_DEVICE_COMPONENT 35 > -#define EFI_SMBIOS_TYPE_MANAGEMENT_DEVICE_THRESHOLD_DATA 36 > -#define EFI_SMBIOS_TYPE_MEMORY_CHANNEL 37 > -#define EFI_SMBIOS_TYPE_IPMI_DEVICE_INFORMATION 38 > -#define EFI_SMBIOS_TYPE_SYSTEM_POWER_SUPPLY 39 > -#define EFI_SMBIOS_TYPE_ADDITIONAL_INFORMATION 40 > -#define EFI_SMBIOS_TYPE_ONBOARD_DEVICES_EXTENDED_INFORMATION 41 > -#define EFI_SMBIOS_TYPE_MANAGEMENT_CONTROLLER_HOST_INTERFACE 42 > -#define EFI_SMBIOS_TYPE_INACTIVE 126 > -#define EFI_SMBIOS_TYPE_END_OF_TABLE 127 > -#define EFI_SMBIOS_OEM_BEGIN 128 > -#define EFI_SMBIOS_OEM_END 255 > > -/// > -/// Text strings associated with a given SMBIOS structure are returned in > the dmiStrucBuffer, appended directly after > -/// the formatted portion of the structure. This method of returning string > information eliminates the need for > -/// application software to deal with pointers embedded in the SMBIOS > structure. Each string is terminated with a null > -/// (00h) BYTE and the set of strings is terminated with an additional null > (00h) BYTE. When the formatted portion of > -/// a SMBIOS structure references a string, it does so by specifying a > non-zero string number within the structure's > -/// string-set. For example, if a string field contains 02h, it references > the second string following the formatted portion > -/// of the SMBIOS structure. If a string field references no string, a null > (0) is placed in that string field. If the > -/// formatted portion of the structure contains string-reference fields and > all the string fields are set to 0 (no string > -/// references), the formatted section of the structure is followed by two > null (00h) BYTES. > -/// > -typedef UINT8 EFI_SMBIOS_STRING; > +#define EFI_SMBIOS_TYPE_BIOS_INFORMATION > SMBIOS_TYPE_BIOS_INFORMATION > +#define EFI_SMBIOS_TYPE_SYSTEM_INFORMATION > SMBIOS_TYPE_SYSTEM_INFORMATION > +#define EFI_SMBIOS_TYPE_BASEBOARD_INFORMATION > SMBIOS_TYPE_BASEBOARD_INFORMATION > +#define EFI_SMBIOS_TYPE_SYSTEM_ENCLOSURE > SMBIOS_TYPE_SYSTEM_ENCLOSURE > +#define EFI_SMBIOS_TYPE_PROCESSOR_INFORMATION > SMBIOS_TYPE_PROCESSOR_INFORMATION > +#define EFI_SMBIOS_TYPE_MEMORY_CONTROLLER_INFORMATION > SMBIOS_TYPE_MEMORY_CONTROLLER_INFORMATION > +#define EFI_SMBIOS_TYPE_MEMORY_MODULE_INFORMATON > SMBIOS_TYPE_MEMORY_MODULE_INFORMATON > +#define EFI_SMBIOS_TYPE_CACHE_INFORMATION > SMBIOS_TYPE_CACHE_INFORMATION > +#define EFI_SMBIOS_TYPE_PORT_CONNECTOR_INFORMATION > SMBIOS_TYPE_PORT_CONNECTOR_INFORMATION > +#define EFI_SMBIOS_TYPE_SYSTEM_SLOTS > SMBIOS_TYPE_SYSTEM_SLOTS > +#define EFI_SMBIOS_TYPE_ONBOARD_DEVICE_INFORMATION > SMBIOS_TYPE_ONBOARD_DEVICE_INFORMATION > +#define EFI_SMBIOS_TYPE_OEM_STRINGS > SMBIOS_TYPE_OEM_STRINGS > +#define EFI_SMBIOS_TYPE_SYSTEM_CONFIGURATION_OPTIONS > SMBIOS_TYPE_SYSTEM_CONFIGURATION_OPTIONS > +#define EFI_SMBIOS_TYPE_BIOS_LANGUAGE_INFORMATION > SMBIOS_TYPE_BIOS_LANGUAGE_INFORMATION > +#define EFI_SMBIOS_TYPE_GROUP_ASSOCIATIONS > SMBIOS_TYPE_GROUP_ASSOCIATIONS > +#define EFI_SMBIOS_TYPE_SYSTEM_EVENT_LOG > SMBIOS_TYPE_SYSTEM_EVENT_LOG > +#define EFI_SMBIOS_TYPE_PHYSICAL_MEMORY_ARRAY > SMBIOS_TYPE_PHYSICAL_MEMORY_ARRAY > +#define EFI_SMBIOS_TYPE_MEMORY_DEVICE > SMBIOS_TYPE_MEMORY_DEVICE > +#define EFI_SMBIOS_TYPE_32BIT_MEMORY_ERROR_INFORMATION > SMBIOS_TYPE_32BIT_MEMORY_ERROR_INFORMATION > +#define EFI_SMBIOS_TYPE_MEMORY_ARRAY_MAPPED_ADDRESS > SMBIOS_TYPE_MEMORY_ARRAY_MAPPED_ADDRESS > +#define EFI_SMBIOS_TYPE_MEMORY_DEVICE_MAPPED_ADDRESS > SMBIOS_TYPE_MEMORY_DEVICE_MAPPED_ADDRESS > +#define EFI_SMBIOS_TYPE_BUILT_IN_POINTING_DEVICE > SMBIOS_TYPE_BUILT_IN_POINTING_DEVICE > +#define EFI_SMBIOS_TYPE_PORTABLE_BATTERY > SMBIOS_TYPE_PORTABLE_BATTERY > +#define EFI_SMBIOS_TYPE_SYSTEM_RESET > SMBIOS_TYPE_SYSTEM_RESET > +#define EFI_SMBIOS_TYPE_HARDWARE_SECURITY > SMBIOS_TYPE_HARDWARE_SECURITY > +#define EFI_SMBIOS_TYPE_SYSTEM_POWER_CONTROLS > SMBIOS_TYPE_SYSTEM_POWER_CONTROLS > +#define EFI_SMBIOS_TYPE_VOLTAGE_PROBE > SMBIOS_TYPE_VOLTAGE_PROBE > +#define EFI_SMBIOS_TYPE_COOLING_DEVICE > SMBIOS_TYPE_COOLING_DEVICE > +#define EFI_SMBIOS_TYPE_TEMPERATURE_PROBE > SMBIOS_TYPE_TEMPERATURE_PROBE > +#define EFI_SMBIOS_TYPE_ELECTRICAL_CURRENT_PROBE > SMBIOS_TYPE_ELECTRICAL_CURRENT_PROBE > +#define EFI_SMBIOS_TYPE_OUT_OF_BAND_REMOTE_ACCESS > SMBIOS_TYPE_OUT_OF_BAND_REMOTE_ACCESS > +#define EFI_SMBIOS_TYPE_BOOT_INTEGRITY_SERVICE > SMBIOS_TYPE_BOOT_INTEGRITY_SERVICE > +#define EFI_SMBIOS_TYPE_SYSTEM_BOOT_INFORMATION > SMBIOS_TYPE_SYSTEM_BOOT_INFORMATION > +#define EFI_SMBIOS_TYPE_64BIT_MEMORY_ERROR_INFORMATION > SMBIOS_TYPE_64BIT_MEMORY_ERROR_INFORMATION > +#define EFI_SMBIOS_TYPE_MANAGEMENT_DEVICE > SMBIOS_TYPE_MANAGEMENT_DEVICE > +#define EFI_SMBIOS_TYPE_MANAGEMENT_DEVICE_COMPONENT > SMBIOS_TYPE_MANAGEMENT_DEVICE_COMPONENT > +#define EFI_SMBIOS_TYPE_MANAGEMENT_DEVICE_THRESHOLD_DATA > SMBIOS_TYPE_MANAGEMENT_DEVICE_THRESHOLD_DATA > +#define EFI_SMBIOS_TYPE_MEMORY_CHANNEL > SMBIOS_TYPE_MEMORY_CHANNEL > +#define EFI_SMBIOS_TYPE_IPMI_DEVICE_INFORMATION > SMBIOS_TYPE_IPMI_DEVICE_INFORMATION > +#define EFI_SMBIOS_TYPE_SYSTEM_POWER_SUPPLY > SMBIOS_TYPE_SYSTEM_POWER_SUPPLY > +#define EFI_SMBIOS_TYPE_ADDITIONAL_INFORMATION > SMBIOS_TYPE_ADDITIONAL_INFORMATION > +#define EFI_SMBIOS_TYPE_ONBOARD_DEVICES_EXTENDED_INFORMATION > SMBIOS_TYPE_ONBOARD_DEVICES_EXTENDED_INFORMATION > +#define EFI_SMBIOS_TYPE_MANAGEMENT_CONTROLLER_HOST_INTERFACE > SMBIOS_TYPE_MANAGEMENT_CONTROLLER_HOST_INTERFACE > +#define EFI_SMBIOS_TYPE_INACTIVE > SMBIOS_TYPE_INACTIVE > +#define EFI_SMBIOS_TYPE_END_OF_TABLE > SMBIOS_TYPE_END_OF_TABLE > +#define EFI_SMBIOS_OEM_BEGIN SMBIOS_OEM_BEGIN > +#define EFI_SMBIOS_OEM_END SMBIOS_OEM_END > > -/// > -/// Types 0 through 127 (7Fh) are reserved for and defined by this > -/// specification. Types 128 through 256 (80h to FFh) are available for > system- and OEM-specific information. > -/// > -typedef UINT8 EFI_SMBIOS_TYPE; > - > -/// > -/// Specifies the structure's handle, a unique 16-bit number in the range 0 > to 0FFFEh (for version > -/// 2.0) or 0 to 0FEFFh (for version 2.1 and later). The handle can be used > with the Get SMBIOS > -/// Structure function to retrieve a specific structure; the handle numbers > are not required to be > -/// contiguous. For v2.1 and later, handle values in the range 0FF00h to > 0FFFFh are reserved for > -/// use by this specification. > -/// If the system configuration changes, a previously assigned handle might > no longer exist. > -/// However once a handle has been assigned by the BIOS, the BIOS cannot > re-assign that handle > -/// number to another structure. > -/// > -typedef UINT16 EFI_SMBIOS_HANDLE; > - > -typedef struct { > - EFI_SMBIOS_TYPE Type; > - UINT8 Length; > - EFI_SMBIOS_HANDLE Handle; > -} EFI_SMBIOS_TABLE_HEADER; > +typedef SMBIOS_STRING EFI_SMBIOS_STRING; > +typedef SMBIOS_TYPE EFI_SMBIOS_TYPE; > +typedef SMBIOS_HANDLE EFI_SMBIOS_HANDLE; > +typedef SMBIOS_STRUCTURE EFI_SMBIOS_TABLE_HEADER; > > typedef struct _EFI_SMBIOS_PROTOCOL EFI_SMBIOS_PROTOCOL; > > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
