[SAMI]
+ 0, //
MemoryErrorInformationHandle
+ 0xFFFF, // TotalWidth
+ 0xFFFF, // DataWIdth
[SAMI] I need to find out how these fields should be populated, but the
Annex A, SMBIOS specification version 3.6.0 says the following:
4.8.4 Total Width is not 0FFFFh (Unknown) if the memory device is
installed. (Size is not 0.)
4.8.5 Data Width is not 0FFFFh (Unknown).
Can you explain how this field is used, please?
[/SAMI]
+ 0x7FFF, // Size (always use
Extended Size)
[SAMI] I think this field should be set based on the Size.
The spec says "If the size is 32 GB-1 MB or greater, the field value is
7FFFh and the actual size is stored in the Extended Size field."
I think it would be good to have a static function that encodes the
size in wither the Size field or the Extended Size field.
e.g. UpdateSmbiosTable17Size (SMBIOS_TABLE_TYPE17 *SmbiosRecord, UINTN
Size) {
if (Size > 32GB-1MB) {
SmbiosRecord->Size = EncodeSize (xxx);
} else {
SmbiosRecord->ExtendedSize = EncodeExtendedSize (xxx);
}
}
[/SAMI]
+ MemoryTypeUnknown, // FormFactor
+ 0xFF, // DeviceSet
+ 1, // Device Locator
+ 2, // Bank Locator
+ MemoryTypeSdram, // MemoryType
+ { // TypeDetail
+ 0
+ },
+ 0xFFFF, // Speed (Use Extended Speed)
[SAMI] Maybe we need a helper function (similar to
UpdateSmbiosTable17Size()) for this field as well.
+ 0, // Manufacturer
+ // (Unused Use
ModuleManufactuerId)
+ 3, // SerialNumber
+ 4, // AssetTag
+ 0, // PartNumber
+ // (Unused Use
ModuleProductId)
+ 0, // Attributes
+ 0, // ExtendedSize
+ 2000, // ConfiguredMemoryClockSpeed
[SAMI] Should this be provided through CM_ARM_MEMORY_DEVICE_INFO?
+ 0, // MinimumVoltage
+ 0, // MaximumVoltage
+ 0, // ConfiguredVoltage
+ MemoryTechnologyDram, // MemoryTechnology
[SAMI] Should this be provided through CM_ARM_MEMORY_DEVICE_INFO?
+ { //
MemoryOperatingModeCapability
+ .Uint16 = 0x000
+ },
[SAMI] I think the above initialisation may not be portable.
+ 5, // FirmwareVersion
+ 0, // ModuleManufacturerId
+ 0, // ModuleProductId
+ 0, //
MemorySubsystemContollerManufacturerId
+ 0, //
MemorySybsystemControllerProductId
+ 0, // NonVolatileSize
+ 0, // VolatileSize
+ 0, // CacheSize
+ 0, // LogicalSize
+ 0, // ExtendedSpeed
+ 0, //
ExtendedConfiguredMemorySpeed
+};
+
+STATIC CHAR8 UnknownStr[] = "Unknown\0";
[SAMI] Would it be possible to add the CONST qualifier, please?
+
+STATIC
+EFI_STATUS
+EFIAPI
+FreeSmbiosType17TableEx (
+ IN CONST SMBIOS_TABLE_GENERATOR *CONST This,
+ IN CONST CM_STD_OBJ_SMBIOS_TABLE_INFO *CONST
SmbiosTableInfo,
+ IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST
CfgMgrProtocol,
+ IN OUT SMBIOS_STRUCTURE ***CONST Table,
+ IN CONST UINTN
TableCount
+ )
+{
+ return EFI_SUCCESS;
+}
+
+/** Construct SMBIOS Type17 Table describing memory devices.
+
+ If this function allocates any resources then they must be freed
+ in the FreeXXXXTableResources function.
+
+ @param [in] This Pointer to the SMBIOS table generator.
+ @param [in] SmbiosTableInfo Pointer to the SMBIOS table
information.
+ @param [in] CfgMgrProtocol Pointer to the Configuration Manager
+ Protocol interface.
+ @param [out] Table Pointer to the SMBIOS table.
+
+ @retval EFI_SUCCESS Table generated successfully.
+ @retval EFI_BAD_BUFFER_SIZE The size returned by the
Configuration
+ Manager is less than the Object
size for
+ the requested object.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+ @retval EFI_NOT_FOUND Could not find information.
+ @retval EFI_OUT_OF_RESOURCES Could not allocate memory.
+ @retval EFI_UNSUPPORTED Unsupported configuration.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+BuildSmbiosType17TableEx (
+ IN CONST SMBIOS_TABLE_GENERATOR *This,
+ IN CM_STD_OBJ_SMBIOS_TABLE_INFO *CONST
SmbiosTableInfo,
+ IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST
CfgMgrProtocol,
+ OUT SMBIOS_STRUCTURE ***Table,
+ OUT UINTN *CONST TableCount
+ )
+{
+ EFI_STATUS Status;
+ UINT32 NumMemDevices;
+ SMBIOS_STRUCTURE **TableList;
+ CM_ARM_MEMORY_DEVICE_INFO *MemoryDevicesInfo;
+ UINTN Index;
+ UINTN SerialNumLen;
+ CHAR8 *SerialNum;
+ UINTN AssetTagLen;
+ CHAR8 *AssetTag;
+ UINTN DeviceLocatorLen;
+ CHAR8 *DeviceLocator;
+ UINTN BankLocatorLen;
+ CHAR8 *BankLocator;
+ UINTN FirmwareVersionLen;
+ CHAR8 *FirmwareVersion;
+ CHAR8 *OptionalStrings;
+ SMBIOS_TABLE_TYPE17 *SmbiosRecord;
+
+ ASSERT (This != NULL);
+ ASSERT (SmbiosTableInfo != NULL);
+ ASSERT (CfgMgrProtocol != NULL);
+ ASSERT (Table != NULL);
+ ASSERT (TableCount != NULL);
+ ASSERT (SmbiosTableInfo->TableGeneratorId == This->GeneratorID);
+
+ DEBUG ((DEBUG_ERROR, "%a : Start \n", __FUNCTION__));
+ *Table = NULL;
+ Status = GetEArmObjMemoryDeviceInfo (
+ CfgMgrProtocol,
+ CM_NULL_TOKEN,
+ &MemoryDevicesInfo,
+ &NumMemDevices
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "Failed to get Memory Devices CM Object %r\n",
+ Status
+ ));
+ return Status;
+ }
+
+ TableList = (SMBIOS_STRUCTURE **)AllocateZeroPool (sizeof
(SMBIOS_STRUCTURE *) * NumMemDevices);
[SAMI] The memory allocated here should be freed in
FreeSmbiosType17TableEx.
+ if (TableList == NULL) {
+ DEBUG ((DEBUG_ERROR, "Failed to alloc memory for %u devices
table\n"));
+ Status = EFI_OUT_OF_RESOURCES;
+ goto exit;
+ }
+
+ for (Index = 0; Index < NumMemDevices; Index++) {
+ if (MemoryDevicesInfo[Index].SerialNum != NULL) {
+ SerialNumLen = AsciiStrLen (MemoryDevicesInfo[Index].SerialNum);
+ SerialNum = MemoryDevicesInfo[Index].SerialNum;
+ } else {
+ SerialNumLen = AsciiStrLen (UnknownStr);
+ SerialNum = UnknownStr;
[SAMI] If the serial number is not provided, then should the string
field be set to 0?
See Section 6.1.3, SMBIOS Spec Version 3.6.0 which states
"If a string field references no string, a null (0) is placed in that
string field."
[/SAMI]
+ }
+
+ if (MemoryDevicesInfo[Index].AssetTag != NULL) {
+ AssetTagLen = AsciiStrLen (MemoryDevicesInfo[Index].AssetTag);
+ AssetTag = MemoryDevicesInfo[Index].AssetTag;
+ } else {
+ AssetTagLen = AsciiStrLen (UnknownStr);
+ AssetTag = UnknownStr;
+ }
+
+ if (MemoryDevicesInfo[Index].DeviceLocator != NULL) {
+ DeviceLocatorLen = AsciiStrLen
(MemoryDevicesInfo[Index].DeviceLocator);
+ DeviceLocator = MemoryDevicesInfo[Index].DeviceLocator;
+ } else {
+ DeviceLocatorLen = AsciiStrLen (UnknownStr);
+ DeviceLocator = UnknownStr;
+ }
+
+ if (MemoryDevicesInfo[Index].BankLocator != NULL) {
+ BankLocatorLen = AsciiStrLen
(MemoryDevicesInfo[Index].BankLocator);
+ BankLocator = MemoryDevicesInfo[Index].BankLocator;
+ } else {
+ BankLocatorLen = AsciiStrLen (UnknownStr);
+ BankLocator = UnknownStr;
+ }
+
+ if (MemoryDevicesInfo[Index].FirmwareVersion != NULL) {
+ FirmwareVersionLen = AsciiStrLen
(MemoryDevicesInfo[Index].FirmwareVersion);
+ FirmwareVersion = MemoryDevicesInfo[Index].FirmwareVersion;
+ } else {
+ FirmwareVersionLen = AsciiStrLen (UnknownStr);
+ FirmwareVersion = UnknownStr;
+ }
+
+ SmbiosRecord = (SMBIOS_TABLE_TYPE17 *)AllocateZeroPool (
+ sizeof
(SMBIOS_TABLE_TYPE17) +
+ SerialNumLen + 1 +
+ AssetTagLen + 1 +
DeviceLocatorLen + 1 +
+ BankLocatorLen + 1 +
FirmwareVersionLen + 1 + 1
+ );
[SAMI] The memory allocated here needs to be freed in
FreeSmbiosType17TableEx as SmbiosProtocol->Add () makes a copy of the
SmbiosTable, see
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Fedk2%2Fblob%2Fmaster%2FMdeModulePkg%2FUniversal%2FSmbiosDxe%2FSmbiosDxe.c%23L476&data=05%7C01%7Cgmahadevan%40nvidia.com%7C1169a52a2ad140a8d79308da94cf2762%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637985914717380634%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=EnTxn07ekjA7bGUeCg2c0BaUMVW0yU5JFjXNOcZuhQA%3D&reserved=0
and
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Fedk2%2Fblob%2Fmaster%2FMdeModulePkg%2FUniversal%2FSmbiosDxe%2FSmbiosDxe.c%23L516&data=05%7C01%7Cgmahadevan%40nvidia.com%7C1169a52a2ad140a8d79308da94cf2762%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637985914717380634%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=d6kU6CdywK4fnOdxE8CyTTM9eQHDE38FzZB7SA2FTqc%3D&reserved=0.
+ if (SmbiosRecord == NULL) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto exit;
+ }
+
+ CopyMem (SmbiosRecord, &MemDeviceInfoTemplate, sizeof
(SMBIOS_TABLE_TYPE17));
+ SmbiosRecord->ExtendedSize =
MemoryDevicesInfo[Index].Size;
[SAMI] CM_ARM_MEMORY_DEVICE_INFO.Size is documented as size in bytes,
while looking at the SMBIOS specification, section 7.18.5 for the
Extended Size Bits 30:0 represent the size of the memory device in
megabytes.
I think it will be useful to create UpdateSmbiosTable17Size() which does
the appropriate encoding and updation of the SMBIOS table fieds.
[/SAMI]
+ SmbiosRecord->DeviceSet =
MemoryDevicesInfo[Index].DeviceSet;
+ SmbiosRecord->ModuleManufacturerID =
+ MemoryDevicesInfo[Index].ModuleManufacturerId;
+ SmbiosRecord->ModuleProductID =
+ MemoryDevicesInfo[Index].ModuleProductId;
+ SmbiosRecord->Attributes =
+ MemoryDevicesInfo[Index].Attributes;
+ SmbiosRecord->ExtendedSpeed = MemoryDevicesInfo[Index].Speed;
+ OptionalStrings = (CHAR8 *)(SmbiosRecord + 1);
+ AsciiSPrint (OptionalStrings, DeviceLocatorLen + 1,
DeviceLocator);
[SAMI] I think we can simplify the publishing of the SMBIOS strings
using SmbiosStringTableLib. Please see the patch at
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F93651&data=05%7C01%7Cgmahadevan%40nvidia.com%7C1169a52a2ad140a8d79308da94cf2762%7C43083d15727340c1b7db39efd9ccc17a%7C0%7C0%7C637985914717380634%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=4xHMP2KfNlcdeBrtH%2FIT1F249uWIoz0XZreF3FSugq8%3D&reserved=0
+ OptionalStrings = OptionalStrings + DeviceLocatorLen + 1;
+ AsciiSPrint (OptionalStrings, BankLocatorLen + 1, BankLocator);
+ OptionalStrings = OptionalStrings + BankLocatorLen + 1;
+ AsciiSPrint (OptionalStrings, SerialNumLen + 1, SerialNum);
+ OptionalStrings = OptionalStrings + SerialNumLen + 1;
+ AsciiSPrint (OptionalStrings, AssetTagLen + 1, AssetTag);
+ OptionalStrings = OptionalStrings + AssetTagLen + 1;
+ AsciiSPrint (OptionalStrings, FirmwareVersionLen + 1,
FirmwareVersion);
+ OptionalStrings = OptionalStrings + FirmwareVersionLen + 1;
+ TableList[Index] = (SMBIOS_STRUCTURE *)SmbiosRecord;
+ }
+
+ *Table = TableList;
+ *TableCount = NumMemDevices;
+
+exit:
+ DEBUG ((DEBUG_ERROR, "%a : Done \n", __FUNCTION__));
+ return Status;
+}
+
+/** The interface for the SMBIOS Type17 Table Generator.
+*/
+STATIC
+CONST
+SMBIOS_TABLE_GENERATOR SmbiosType17Generator = {
+ // Generator ID
+ CREATE_STD_SMBIOS_TABLE_GEN_ID (EStdSmbiosTableIdType17),
+ // Generator Description
+ L"SMBIOS.TYPE17.GENERATOR",
+ // SMBIOS Table Type
+ EFI_SMBIOS_TYPE_MEMORY_DEVICE,
+ NULL,
+ NULL,
+ // Build table function Extended.
+ BuildSmbiosType17TableEx,
+ // Free function Extended.
+ FreeSmbiosType17TableEx
+};
+
+/** Register the Generator with the SMBIOS Table Factory.
+
+ @param [in] ImageHandle The handle to the image.
+ @param [in] SystemTable Pointer to the System Table.
+
+ @retval EFI_SUCCESS The Generator is registered.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+ @retval EFI_ALREADY_STARTED The Generator for the Table ID
+ is already registered.
+**/
+EFI_STATUS
+EFIAPI
+SmbiosType17LibConstructor (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_STATUS Status;
+
+ Status = RegisterSmbiosTableGenerator (&SmbiosType17Generator);
+ DEBUG ((
+ DEBUG_INFO,
+ "SMBIOS Type 17: Register Generator. Status = %r\n",
+ Status
+ ));
+ ASSERT_EFI_ERROR (Status);
+
+ return Status;
+}
+
+/** Deregister the Generator from the SMBIOS Table Factory.
+
+ @param [in] ImageHandle The handle to the image.
+ @param [in] SystemTable Pointer to the System Table.
+
+ @retval EFI_SUCCESS The Generator is deregistered.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+ @retval EFI_NOT_FOUND The Generator is not registered.
+**/
+EFI_STATUS
+EFIAPI
+SmbiosType17LibDestructor (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_STATUS Status;
+
+ Status = DeregisterSmbiosTableGenerator (&SmbiosType17Generator);
+ DEBUG ((
+ DEBUG_INFO,
+ "SMBIOS Type17: Deregister Generator. Status = %r\n",
+ Status
+ ));
+ ASSERT_EFI_ERROR (Status);
+ return Status;
+}
diff --git
a/DynamicTablesPkg/Library/Smbios/Arm/SmbiosType17Lib/SmbiosType17LibArm.inf
b/DynamicTablesPkg/Library/Smbios/Arm/SmbiosType17Lib/SmbiosType17LibArm.inf
new file mode 100644
index 0000000000..78a80b75f0
--- /dev/null
+++
b/DynamicTablesPkg/Library/Smbios/Arm/SmbiosType17Lib/SmbiosType17LibArm.inf
@@ -0,0 +1,32 @@
+## @file
+# SMBIOS Type17 Table Generator
+#
+# Copyright (c) 2022, NVIDIA CORPORATION & AFFILIATES. All rights
reserved.
+# Copyright (c) 2019 - 2021, Arm Limited. All rights reserved.<BR>
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+ INF_VERSION = 0x0001001B
+ BASE_NAME = SmbiosType17LibArm
+ FILE_GUID = 1f063bac-f8f1-4e08-8ffd-9aae52c75497
+ VERSION_STRING = 1.0
+ MODULE_TYPE = DXE_DRIVER
+ LIBRARY_CLASS = NULL|DXE_DRIVER
+ CONSTRUCTOR = SmbiosType17LibConstructor
+ DESTRUCTOR = SmbiosType17LibDestructor
+
+[Sources]
+ SmbiosType17Generator.c
+
+[Packages]
+ MdePkg/MdePkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ EmbeddedPkg/EmbeddedPkg.dec
+ ArmPlatformPkg/ArmPlatformPkg.dec
+ DynamicTablesPkg/DynamicTablesPkg.dec
+
+[LibraryClasses]
+ BaseLib
+ DebugLib
