Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions DynamicTablesPkg/DynamicTables.dsc.inc
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@

DynamicTablesPkg/Library/Smbios/SmbiosType4Lib/SmbiosType4Lib.inf
DynamicTablesPkg/Library/Smbios/SmbiosType7Lib/SmbiosType7Lib.inf
DynamicTablesPkg/Library/Smbios/SmbiosType0Lib/SmbiosType0Lib.inf
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: would it be possible to order the generators ?
Same comment below


# AML Fixup (Arm specific)
DynamicTablesPkg/Library/Acpi/Arm/AcpiSsdtCmn600LibArm/SsdtCmn600LibArm.inf
Expand Down Expand Up @@ -151,6 +152,7 @@

NULL|DynamicTablesPkg/Library/Smbios/SmbiosType4Lib/SmbiosType4Lib.inf
NULL|DynamicTablesPkg/Library/Smbios/SmbiosType7Lib/SmbiosType7Lib.inf
NULL|DynamicTablesPkg/Library/Smbios/SmbiosType0Lib/SmbiosType0Lib.inf
}

[Components.RISCV64]
Expand Down
36 changes: 36 additions & 0 deletions DynamicTablesPkg/Include/ArchCommonNameSpaceObjects.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ typedef enum ArchCommonObjectID {
EArchCommonObjTpm2DeviceInfo, ///< 46 - TPM2 Device Info
EArchCommonObjMcfgPciConfigSpaceInfo, ///< 47 - MCFG PCI Configuration Space Info
EArchCommonObjPciRootPortInfo, ///< 48 - PCI root port configuration Info
EArchCommonObjBiosInfo = 52, ///< 52 - BIOS Info
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume the assignment was done to avoid colliding with new objects.
Would it be possible to use the automatically allocated ID ?

EArchCommonObjMax
} EARCH_COMMON_OBJECT_ID;

Expand Down Expand Up @@ -1132,4 +1133,39 @@ typedef struct CmArchCommonObjSpcrInfo {
UINT8 TerminalType;
} CM_ARCH_COMMON_SPCR_INFO;

/** A structure that describes BIOS Information.
SMBIOS Specification v3.6.0 Type 0
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK, the latest version is v3.9.0. What about updating comment with the latest version?

ID: EArchCommonObjBiosInfo
**/
typedef struct CmArchCommonBiosInfo {
/// BIOS vendor name string.
CHAR8 *BiosVendor;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to change these strings to an array of SMBIOS_MAX_STRING_SIZE ?

/// BIOS version string.
CHAR8 *BiosVersion;
/// BIOS starting address segment.
UINT16 BiosSegment;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this field wouldn't reequire since the spec say for "BIOS Starting Address Segments":

...
When not applicable, such as on UEFI-based systems, this value is set to 0000h

/// BIOS release date string.
CHAR8 *BiosReleaseDate;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spec seems to use "Firmware" terminology rather than "BIOS".
Would it be better to change "BIOS" prefix to "Firmware"?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it seems the table was written with BiosReleaseDate in SMBIOS_TABLE_TYPE0.
It is indeed not ideal, but it should be ok as it follows what was done in SMBIOS_TABLE_TYPE0.

/// BIOS ROM size (in 64 KB blocks minus one).
UINT8 BiosSize;
/// Bit field of supported BIOS functions.
MISC_BIOS_CHARACTERISTICS BiosCharacteristics;
/// Optional set of functions that BIOS supports (bytes 0 and 1).
UINT8 BIOSCharacteristicsExtensionBytes[2];
/// System BIOS firmware major version.
UINT8 SystemBiosMajorRelease;
/// System BIOS firmware minor version.
UINT8 SystemBiosMinorRelease;
/// Embedded Controller firmware major release.
UINT8 ECFirmwareMajorRelease;
/// Embedded Controller firmware minor release.
UINT8 ECFirmwareMinorRelease;
/// Extended BIOS ROM size.
EXTENDED_BIOS_ROM_SIZE ExtendedBiosSize;
/// CM Object Token uniquely identifying this BIOS info entry.
CM_OBJECT_TOKEN BiosInfoToken;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to move this field to the beginning.
In case one day a new field is added, it will be strange to have this token in the middle of the useful information.

Also it doesn't seem that any other SMBIOS table relies on this table handle, so this field is normally not needed (or maybe do you use it in another way ?).
@samimujawar do you think it should be kept ?

} CM_ARCH_COMMON_BIOS_INFO;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name seems really generic. What about CM_ARCH_COMMON_PLATFORM_FW_INFO maybe ?
Or if there is another suggestion ?


#pragma pack()
297 changes: 297 additions & 0 deletions DynamicTablesPkg/Library/Smbios/SmbiosType0Lib/SmbiosType0Generator.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,297 @@
/** @file
SMBIOS Type0 Table Generator.

Copyright (c) 2024 - 2026, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
Copyright (c) 2020 - 2021, Arm Limited. All rights reserved.<BR>

SPDX-License-Identifier: BSD-2-Clause-Patent
**/

#include <Library/BaseLib.h>
#include <Library/BaseMemoryLib.h>
#include <Library/DebugLib.h>
#include <Library/MemoryAllocationLib.h>
#include <Library/SmbiosStringTableLib.h>

// Module specific include files.
#include <ConfigurationManagerObject.h>
#include <ConfigurationManagerHelper.h>
#include <Protocol/ConfigurationManagerProtocol.h>
#include <Protocol/DynamicTableFactoryProtocol.h>
#include <Protocol/Smbios.h>
#include <IndustryStandard/SmBios.h>

/** This macro expands to a function that retrieves the BIOS Information
from the Configuration Manager.
*/
GET_OBJECT_LIST (
EObjNameSpaceArchCommon,
EArchCommonObjBiosInfo,
CM_ARCH_COMMON_BIOS_INFO
)

/**
Free any resources allocated when installing SMBIOS Type0 table.

@param [in] This Pointer to the SMBIOS table generator.
@param [in] TableFactoryProtocol Pointer to the SMBIOS Table Factory
Protocol interface.
@param [in] SmbiosTableInfo Pointer to the SMBIOS table information.
@param [in] CfgMgrProtocol Pointer to the Configuration Manager
Protocol interface.
@param [in] Table Pointer to the SMBIOS table.

@retval EFI_SUCCESS Table freed successfully.
**/
STATIC
EFI_STATUS
FreeSmbiosType0Table (
IN CONST SMBIOS_TABLE_GENERATOR *CONST This,
IN CONST EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL *CONST TableFactoryProtocol,
IN CONST CM_STD_OBJ_SMBIOS_TABLE_INFO *CONST SmbiosTableInfo,
IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol,
IN SMBIOS_STRUCTURE **CONST Table
)
{
if (*Table != NULL) {
FreePool (*Table);
}

return EFI_SUCCESS;
}

/** Construct SMBIOS Type0 Table describing BIOS information.

If this function allocates any resources then they must be freed
in the FreeSmbiosType0Table function.

@param [in] This Pointer to the SMBIOS table generator.
@param [in] TableFactoryProtocol Pointer to the SMBIOS Table Factory
Protocol interface.
@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.
@param [out] CmObjectToken Pointer to the CM Object Token.

@retval EFI_SUCCESS Table generated successfully.
@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.
**/
STATIC
EFI_STATUS
BuildSmbiosType0Table (
IN CONST SMBIOS_TABLE_GENERATOR *This,
IN CONST EDKII_DYNAMIC_TABLE_FACTORY_PROTOCOL *CONST TableFactoryProtocol,
IN CM_STD_OBJ_SMBIOS_TABLE_INFO *CONST SmbiosTableInfo,
IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST CfgMgrProtocol,
OUT SMBIOS_STRUCTURE **Table,
OUT CM_OBJECT_TOKEN *CmObjectToken
)
{
EFI_STATUS Status;
CM_OBJECT_TOKEN CmObject;
UINT8 VendorRef;
UINT8 VersionRef;
UINT8 ReleaseDateRef;
SMBIOS_TABLE_TYPE0 *SmbiosRecord;
UINTN SmbiosRecordSize;
STRING_TABLE StrTable;
CHAR8 *OptionalStrings;
CM_ARCH_COMMON_BIOS_INFO *BiosInfo;

ASSERT (This != NULL);
ASSERT (SmbiosTableInfo != NULL);
ASSERT (CfgMgrProtocol != NULL);
ASSERT (Table != NULL);
ASSERT (SmbiosTableInfo->TableGeneratorId == This->GeneratorID);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having these ASSERT is nice, but would it be possible to also return EFI_INVALID_PARAMETER in case these are not valid ?


//
// Retrieve BIOS info from CM object
//
*Table = NULL;
Status = GetEArchCommonObjBiosInfo (
CfgMgrProtocol,
CM_NULL_TOKEN,
&BiosInfo,
NULL
);
if (EFI_ERROR (Status)) {
DEBUG ((
DEBUG_ERROR,
"%a: Failed to get Bios Info CM Object %r\n",
__func__,
Status
));
return Status;
}

//
// Copy strings to SMBIOS table
//
Status = StringTableInitialize (&StrTable, 3);
if (EFI_ERROR (Status)) {
DEBUG ((
DEBUG_ERROR,
"%a: Failed to allocate string table for CM Object, %r\n",
__func__,
Status
));
goto exit;
}

VendorRef = 0;
VersionRef = 0;
ReleaseDateRef = 0;

if ((BiosInfo->BiosVendor != NULL) && (BiosInfo->BiosVendor[0] != '\0')) {
Status = StringTableAddString (&StrTable, BiosInfo->BiosVendor, &VendorRef);
if (EFI_ERROR (Status)) {
DEBUG ((DEBUG_ERROR, "%a: Failed to add BiosVendor string %r\n", __func__, Status));
goto exit;
}
}

if ((BiosInfo->BiosVersion != NULL) && (BiosInfo->BiosVersion[0] != '\0')) {
Status = StringTableAddString (&StrTable, BiosInfo->BiosVersion, &VersionRef);
if (EFI_ERROR (Status)) {
DEBUG ((DEBUG_ERROR, "%a: Failed to add BiosVersion string %r\n", __func__, Status));
goto exit;
}
}

if ((BiosInfo->BiosReleaseDate != NULL) && (BiosInfo->BiosReleaseDate[0] != '\0')) {
Status = StringTableAddString (&StrTable, BiosInfo->BiosReleaseDate, &ReleaseDateRef);
if (EFI_ERROR (Status)) {
DEBUG ((DEBUG_ERROR, "%a: Failed to add BiosReleaseDate string %r\n", __func__, Status));
goto exit;
}
}

SmbiosRecordSize = sizeof (SMBIOS_TABLE_TYPE0) +
StringTableGetStringSetSize (&StrTable);
SmbiosRecord = (SMBIOS_TABLE_TYPE0 *)AllocateZeroPool (SmbiosRecordSize);
if (SmbiosRecord == NULL) {
DEBUG ((DEBUG_ERROR, "%a: memory allocation failed for smbios type0 record\n", __func__));
Status = EFI_OUT_OF_RESOURCES;
goto exit;
}

SmbiosRecord->BiosSegment = BiosInfo->BiosSegment;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should SmbiosRecord->BiosSegment be always 0x00?

SmbiosRecord->BiosSize = BiosInfo->BiosSize;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to add some checks against the Extended size, cf.

FFh - size is 16MiB or greater, see Extended Firmware ROM Size for actual size


SmbiosRecord->BiosCharacteristics = BiosInfo->BiosCharacteristics;

SmbiosRecord->BIOSCharacteristicsExtensionBytes[0] = BiosInfo->BIOSCharacteristicsExtensionBytes[0];
SmbiosRecord->BIOSCharacteristicsExtensionBytes[1] = BiosInfo->BIOSCharacteristicsExtensionBytes[1];

SmbiosRecord->SystemBiosMajorRelease = BiosInfo->SystemBiosMajorRelease;
SmbiosRecord->SystemBiosMinorRelease = BiosInfo->SystemBiosMinorRelease;

SmbiosRecord->EmbeddedControllerFirmwareMajorRelease = BiosInfo->ECFirmwareMajorRelease;
SmbiosRecord->EmbeddedControllerFirmwareMinorRelease = BiosInfo->ECFirmwareMinorRelease;

SmbiosRecord->ExtendedBiosSize = BiosInfo->ExtendedBiosSize;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to add some checks for the reserved bits, cf:

Bits 15:14
Unit
00b - mebibytes
01b - gibibytes
10b - reserved
11b - reserved


SmbiosRecord->Vendor = VendorRef;
SmbiosRecord->BiosVersion = VersionRef;
SmbiosRecord->BiosReleaseDate = ReleaseDateRef;

OptionalStrings = (CHAR8 *)(SmbiosRecord + 1);
// publish the string set
StringTablePublishStringSet (
&StrTable,
OptionalStrings,
(SmbiosRecordSize - sizeof (SMBIOS_TABLE_TYPE0))
);
// setup the header
SmbiosRecord->Hdr.Type = EFI_SMBIOS_TYPE_BIOS_INFORMATION;
SmbiosRecord->Hdr.Length = sizeof (SMBIOS_TABLE_TYPE0);
CmObject = BiosInfo->BiosInfoToken;

*Table = (SMBIOS_STRUCTURE *)SmbiosRecord;
*CmObjectToken = CmObject;

exit:
// free string table
StringTableFree (&StrTable);
return Status;
}

/** The interface for the SMBIOS Type0 Table Generator.
*/
STATIC
CONST
SMBIOS_TABLE_GENERATOR SmbiosType0Generator = {
// Generator ID
CREATE_STD_SMBIOS_TABLE_GEN_ID (EStdSmbiosTableIdType00),
// Generator Description
L"SMBIOS.TYPE0.GENERATOR",
// SMBIOS Table Type
EFI_SMBIOS_TYPE_BIOS_INFORMATION,
// Build table function
BuildSmbiosType0Table,
// Free function
FreeSmbiosType0Table,
NULL,
NULL
};

/** 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
SmbiosType0LibConstructor (
IN EFI_HANDLE ImageHandle,
IN EFI_SYSTEM_TABLE *SystemTable
)
{
EFI_STATUS Status;

Status = RegisterSmbiosTableGenerator (&SmbiosType0Generator);
DEBUG ((
DEBUG_INFO,
"SMBIOS Type 0: 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
SmbiosType0LibDestructor (
IN EFI_HANDLE ImageHandle,
IN EFI_SYSTEM_TABLE *SystemTable
)
{
EFI_STATUS Status;

Status = DeregisterSmbiosTableGenerator (&SmbiosType0Generator);
DEBUG ((
DEBUG_INFO,
"SMBIOS Type0: Deregister Generator. Status = %r\n",
Status
));
ASSERT_EFI_ERROR (Status);
return Status;
}
37 changes: 37 additions & 0 deletions DynamicTablesPkg/Library/Smbios/SmbiosType0Lib/SmbiosType0Lib.inf
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
## @file
# SMBIOS Type0 Table Generator
#
# Copyright (c) 2024 - 2026, 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 = SmbiosType0LibArm
FILE_GUID = 8f95040b-4846-4c7e-b5c1-e4565bfcfa4b
VERSION_STRING = 1.0
MODULE_TYPE = DXE_DRIVER
LIBRARY_CLASS = NULL|DXE_DRIVER
CONSTRUCTOR = SmbiosType0LibConstructor
DESTRUCTOR = SmbiosType0LibDestructor

[Sources]
SmbiosType0Generator.c

[Packages]
MdePkg/MdePkg.dec
MdeModulePkg/MdeModulePkg.dec
EmbeddedPkg/EmbeddedPkg.dec
ArmPlatformPkg/ArmPlatformPkg.dec
DynamicTablesPkg/DynamicTablesPkg.dec

[Protocols]
gEfiSmbiosProtocolGuid # PROTOCOL ALWAYS_CONSUMED

[LibraryClasses]
BaseLib
DebugLib
SmbiosStringTableLib
MemoryAllocationLib
Loading