[edk2-devel] [PATCH v5 18/23] ArmPkg: Add Universal/Smbios/SmbiosMiscDxe
Leif Lindholm
leif at nuviainc.com
Sun Jan 10 02:03:49 UTC 2021
On Mon, Jan 04, 2021 at 15:58:25 -0700, Rebecca Cran wrote:
> SmbiosMiscDxe provides SMBIOS tables 0, 1, 2, 3, 13, and 32.
>
> Signed-off-by: Rebecca Cran <rebecca at nuviainc.com>
> ---
> ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf | 86 +++++++++
> ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMisc.h | 136 +++++++++++++++
> ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscDataTable.c | 61 +++++++
> ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscEntryPoint.c | 184 ++++++++++++++++++++
> ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscLibStrings.uni | 21 +++
> 5 files changed, 488 insertions(+)
>
> diff --git a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
> new file mode 100644
> index 000000000000..06612f02b34c
> --- /dev/null
> +++ b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscDxe.inf
> @@ -0,0 +1,86 @@
> +#/** @file
> +# Component description file for SmbiosMisc instance.
> +#
> +# Parses the MiscSubclassDataTable and reports any generated data to the DataHub.
> +# All .uni file who tagged with "ToolCode="DUMMY"" in following file list is included by
> +# MiscSubclassDriver.uni file, the StrGather tool will expand MiscSubclassDriver.uni file
> +# and parse all .uni file.
> +#
> +# Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2015, Hisilicon Limited. All rights reserved.<BR>
> +# Copyright (c) 2015, Linaro Limited. All rights reserved.<BR>
> +#
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#
> +# Based on files under Nt32Pkg/MiscSubClassPlatformDxe/
> +#**/
> +
> +
> +[Defines]
> + INF_VERSION = 1.29
> + BASE_NAME = SmbiosMiscDxe
> + FILE_GUID = 7e5e26d4-0be9-401f-b5e1-1c2bda7ca777
> + MODULE_TYPE = DXE_DRIVER
> + VERSION_STRING = 1.0
> + ENTRY_POINT = SmbiosMiscEntryPoint
> +
> +[Sources]
> + SmbiosMisc.h
> + SmbiosMiscDataTable.c
> + SmbiosMiscEntryPoint.c
> + SmbiosMiscLibStrings.uni
> + Type00/MiscBiosVendorData.c
> + Type00/MiscBiosVendorFunction.c
> + Type01/MiscSystemManufacturerData.c
> + Type01/MiscSystemManufacturerFunction.c
> + Type02/MiscBaseBoardManufacturerData.c
> + Type02/MiscBaseBoardManufacturerFunction.c
> + Type03/MiscChassisManufacturerData.c
> + Type03/MiscChassisManufacturerFunction.c
> + Type13/MiscNumberOfInstallableLanguagesData.c
> + Type13/MiscNumberOfInstallableLanguagesFunction.c
> + Type32/MiscBootInformationData.c
> + Type32/MiscBootInformationFunction.c
> +
> +[Packages]
> + ArmPkg/ArmPkg.dec
> + MdeModulePkg/MdeModulePkg.dec
> + MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> + BaseLib
> + BaseMemoryLib
> + DebugLib
> + DevicePathLib
> + PcdLib
> + HiiLib
> + HobLib
> + MemoryAllocationLib
> + OemMiscLib
> + UefiBootServicesTableLib
> + UefiDriverEntryPoint
> + UefiLib
> + UefiRuntimeServicesTableLib
> +
> +[Protocols]
> + gEfiSmbiosProtocolGuid # PROTOCOL ALWAYS_CONSUMED
> +
> +[Pcd]
> + gArmTokenSpaceGuid.PcdFdSize
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVendor
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareVersionString
> + gArmTokenSpaceGuid.PcdSystemProductName
> + gArmTokenSpaceGuid.PcdSystemVersion
> + gArmTokenSpaceGuid.PcdBaseBoardManufacturer
> + gArmTokenSpaceGuid.PcdBaseBoardProductName
> + gArmTokenSpaceGuid.PcdBaseBoardVersion
> + gArmTokenSpaceGuid.PcdFdBaseAddress
> +
> +[Guids]
> + gEfiGenericVariableGuid
> +
> +[Depex]
> + gEfiSmbiosProtocolGuid
> +
> +
> diff --git a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMisc.h b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMisc.h
> new file mode 100644
> index 000000000000..20840f40d04b
> --- /dev/null
> +++ b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMisc.h
> @@ -0,0 +1,136 @@
> +/** @file
> + Header file for the SmbiosMisc Driver.
> +
> + Based on files under Nt32Pkg/MiscSubClassPlatformDxe/
> +
> + Copyright (c) 2006 - 2011, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2015, Hisilicon Limited. All rights reserved.<BR>
> + Copyright (c) 2015, Linaro Limited. All rights reserved.<BR>
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef SMBIOS_MISC_H_
> +#define SMBIOS_MISC_H_
> +
> +#include <Protocol/Smbios.h>
> +#include <IndustryStandard/SmBios.h>
We appear to still have a discrepancy w.r.t. Smbios/SmBios capitalisation.
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/HiiLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/OemMiscLib.h>
> +#include <Library/PcdLib.h>
> +#include <Library/PrintLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiLib.h>
> +#include <Library/UefiRuntimeServicesTableLib.h>
> +#include <Guid/DebugMask.h>
This is definitely more includes than are required by this file.
Please include only those providing definitions required by this file.
> +
> +
> +//
> +// Data table entry update function.
> +//
> +typedef EFI_STATUS (EFIAPI EFI_MISC_SMBIOS_DATA_FUNCTION) (
> + IN VOID *RecordData,
> + IN EFI_SMBIOS_PROTOCOL *Smbios
> + );
> +
> +
> +//
> +// Data table entry definition.
> +//
> +typedef struct {
> + //
> + // intermediate input data for SMBIOS record
> + //
> + VOID *RecordData;
> + EFI_MISC_SMBIOS_DATA_FUNCTION *Function;
> +} EFI_MISC_SMBIOS_DATA_TABLE;
EFI_ prefix is reserved for structs defined by the UEFI specification.
SMBIOSMISC_DATA_TABLE or SMBIOS_MISC_DATA_TABLE would seem appropriate.
> +
> +
> +//
> +// SMBIOS table extern definitions
> +//
> +#define MISC_SMBIOS_TABLE_EXTERNS(NAME1, NAME2, NAME3) \
> +extern NAME1 NAME2 ## Data; \
> +extern EFI_MISC_SMBIOS_DATA_FUNCTION NAME3 ## Function;
> +
> +
> +//
> +// SMBIOS data table entries
> +//
> +// This is used to define a pair of table structure pointer and functions
> +// in order to iterate through the list of tables, populate them and add
> +// them into the system.
> +#define MISC_SMBIOS_TABLE_ENTRY_DATA_AND_FUNCTION(NAME1, NAME2) \
> +{ \
> + & NAME1 ## Data, \
> + NAME2 ## Function \
> +}
> +
> +//
> +// Global definition macros.
> +//
> +#define MISC_SMBIOS_TABLE_DATA(NAME1, NAME2) \
> + NAME1 NAME2 ## Data
> +
> +#define MISC_SMBIOS_TABLE_FUNCTION(NAME2) \
> + EFI_STATUS EFIAPI NAME2 ## Function( \
> + IN VOID *RecordData, \
> + IN EFI_SMBIOS_PROTOCOL *Smbios \
> + )
> +
> +//
> +// Data Table Array Entries
> +//
> +extern EFI_HII_HANDLE mHiiHandle;
While the m prefix indicates this as a module-local variable, the
actual linkage visibility is global - so a more unique name would be
preferable. (e.g, mSmbiosMiscHiiHandle)
> +
> +typedef struct _EFI_TYPE13_BIOS_LANGUAGE_INFORMATION_STRING{
> + UINT8 *LanguageSignature;
> + EFI_STRING_ID InstallableLanguageLongString;
> + EFI_STRING_ID InstallableLanguageAbbreviateString;
> +} EFI_TYPE13_BIOS_LANGUAGE_INFORMATION_STRING;
EFI_ prefix.
> +
> +
> +/**
> + Logs SMBIOS record.
> +
> + @param [in] Buffer Pointer to the data buffer.
> + @param [in] SmbiosHandle Pointer for retrieve handle.
> +
> +**/
> +EFI_STATUS
> +LogSmbiosData (
Similarly, global visibility:
SmbiosMiscLogData?
> + IN UINT8 *Buffer,
> + IN OUT EFI_SMBIOS_HANDLE *SmbiosHandle
> + );
> +
> +/**
> + Get Link Type Handle.
> +
> + @param [in] SmbiosType Get this Type from SMBIOS table
> + @param [out] HandleArray Pointer to handle array which will be freed by caller
> + @param [out] HandleCount Pointer to handle count
> +
> +**/
> +VOID
> +GetLinkTypeHandle(
SmbiosMiscGetLinkTypeHandle?
> + IN UINT8 SmbiosType,
> + OUT UINT16 **HandleArray,
> + OUT UINTN *HandleCount
> + );
> +
> +//
> +// Data Table Array
> +//
> +extern EFI_MISC_SMBIOS_DATA_TABLE mSmbiosMiscDataTable[];
> +
> +//
> +// Data Table Array Entries
> +//
> +extern UINTN mSmbiosMiscDataTableEntries;
> +extern UINT8 SmbiosMiscDxeStrings[];
Also needs m prefix.
> +
> +#endif // SMBIOS_MISC_H_
> diff --git a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscDataTable.c b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscDataTable.c
> new file mode 100644
> index 000000000000..c9f460f1d5a8
> --- /dev/null
> +++ b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscDataTable.c
> @@ -0,0 +1,61 @@
> +/** @file
> + This file provides SMBIOS Misc Type.
> +
> + Based on files under Nt32Pkg/MiscSubClassPlatformDxe/
> +
> + Copyright (c) 2006 - 2011, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2015, Hisilicon Limited. All rights reserved.<BR>
> + Copyright (c) 2015, Linaro Limited. All rights reserved.<BR>
> + SPDX-License-Identifier: BSD-2-Clause-Patent\
> +
> +**/
> +
> +#include "SmbiosMisc.h"
> +
> +MISC_SMBIOS_TABLE_EXTERNS (SMBIOS_TABLE_TYPE0,
> + MiscBiosVendor,
> + MiscBiosVendor)
> +MISC_SMBIOS_TABLE_EXTERNS (SMBIOS_TABLE_TYPE1,
> + MiscSystemManufacturer,
> + MiscSystemManufacturer)
> +MISC_SMBIOS_TABLE_EXTERNS (SMBIOS_TABLE_TYPE3,
> + MiscChassisManufacturer,
> + MiscChassisManufacturer)
> +MISC_SMBIOS_TABLE_EXTERNS (SMBIOS_TABLE_TYPE2,
> + MiscBaseBoardManufacturer,
> + MiscBaseBoardManufacturer)
> +MISC_SMBIOS_TABLE_EXTERNS (SMBIOS_TABLE_TYPE13,
> + MiscNumberOfInstallableLanguages,
> + MiscNumberOfInstallableLanguages)
> +MISC_SMBIOS_TABLE_EXTERNS (SMBIOS_TABLE_TYPE32,
> + MiscBootInformation,
> + MiscBootInformation)
> +
> +
> +EFI_MISC_SMBIOS_DATA_TABLE mSmbiosMiscDataTable[] = {
> + // Type0
> + MISC_SMBIOS_TABLE_ENTRY_DATA_AND_FUNCTION (MiscBiosVendor,
> + MiscBiosVendor),
> + // Type1
> + MISC_SMBIOS_TABLE_ENTRY_DATA_AND_FUNCTION (MiscSystemManufacturer,
> + MiscSystemManufacturer),
> + // Type3
> + MISC_SMBIOS_TABLE_ENTRY_DATA_AND_FUNCTION (MiscChassisManufacturer,
> + MiscChassisManufacturer),
> + // Type2
> + MISC_SMBIOS_TABLE_ENTRY_DATA_AND_FUNCTION (MiscBaseBoardManufacturer,
> + MiscBaseBoardManufacturer),
> + // Type13
> + MISC_SMBIOS_TABLE_ENTRY_DATA_AND_FUNCTION (MiscNumberOfInstallableLanguages,
> + MiscNumberOfInstallableLanguages),
> + // Type32
> + MISC_SMBIOS_TABLE_ENTRY_DATA_AND_FUNCTION (MiscBootInformation,
> + MiscBootInformation),
> +};
> +
> +
> +//
> +// Number of Data Table entries.
> +//
> +UINTN mSmbiosMiscDataTableEntries =
> + (sizeof (mSmbiosMiscDataTable)) / sizeof (EFI_MISC_SMBIOS_DATA_TABLE);
> diff --git a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscEntryPoint.c b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscEntryPoint.c
> new file mode 100644
> index 000000000000..afd96476a843
> --- /dev/null
> +++ b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscEntryPoint.c
> @@ -0,0 +1,184 @@
> +/** @file
> + This driver parses the mSmbiosMiscDataTable structure and reports
> + any generated data using SMBIOS protocol.
> +
> + Based on files under Nt32Pkg/MiscSubClassPlatformDxe/
> +
> + Copyright (c) 2006 - 2011, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2015, Hisilicon Limited. All rights reserved.<BR>
> + Copyright (c) 2015, Linaro Limited. All rights reserved.<BR>
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include "SmbiosMisc.h"
> +
> +
> +EFI_HANDLE mImageHandle;
> +EFI_HII_HANDLE mHiiHandle;
> +EFI_SMBIOS_PROTOCOL *mSmbios = NULL;
mImageHandle and mSmbios are also a bit too generically named.
> +
> +/**
> + Standard EFI driver point. This driver parses the mSmbiosMiscDataTable
> + structure and reports any generated data using SMBIOS protocol.
> +
> + @param ImageHandle Handle for the image of this driver
> + @param SystemTable Pointer to the EFI System Table
> +
> + @retval EFI_SUCCESS The data was successfully stored.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +SmbiosMiscEntryPoint(
> + IN EFI_HANDLE ImageHandle,
> + IN EFI_SYSTEM_TABLE *SystemTable
> + )
> +{
> + UINTN Index;
> + EFI_STATUS EfiStatus;
> + EFI_SMBIOS_PROTOCOL *Smbios;
> +
> + mImageHandle = ImageHandle;
> +
> + EfiStatus = gBS->LocateProtocol (&gEfiSmbiosProtocolGuid, NULL, (VOID**)&Smbios);
> + if (EFI_ERROR (EfiStatus)) {
> + DEBUG ((DEBUG_ERROR, "Could not locate SMBIOS protocol. %r\n", EfiStatus));
> + return EfiStatus;
> + }
> +
> + mSmbios = Smbios;
If the call fails, *Smbios will contain NULL.
So is it really needed as a separate variable, or could this use
mSmbios directly?
> +
> + mHiiHandle = HiiAddPackages (
> + &gEfiCallerIdGuid,
> + mImageHandle,
> + SmbiosMiscDxeStrings,
> + NULL
> + );
> + if (mHiiHandle == NULL) {
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + for (Index = 0; Index < mSmbiosMiscDataTableEntries; ++Index) {
> + //
> + // If the entry have a function pointer, just log the data.
> + //
> + if (mSmbiosMiscDataTable[Index].Function != NULL) {
> + EfiStatus = (*mSmbiosMiscDataTable[Index].Function)(
> + mSmbiosMiscDataTable[Index].RecordData,
> + Smbios
> + );
> +
> + if (EFI_ERROR(EfiStatus)) {
> + DEBUG ((DEBUG_ERROR, "Misc smbios store error. Index=%d, ReturnStatus=%r\n", Index, EfiStatus));
Please wrap.
> + return EfiStatus;
> + }
> + }
> + }
> +
> + return EfiStatus;
> +}
> +
> +
> +/**
> + Logs SMBIOS record.
> +
> + @param Buffer The data for the fixed portion of the SMBIOS record. The format of the record is
> + determined by EFI_SMBIOS_TABLE_HEADER.Type. The size of the formatted area is defined
> + by EFI_SMBIOS_TABLE_HEADER.Length and either followed by a double-null (0x0000) or
> + a set of null terminated strings and a null.
> + @param SmbiosHandle A unique handle will be assigned to the SMBIOS record.
> +
> + @retval EFI_SUCCESS Record was added.
> + @retval EFI_OUT_OF_RESOURCES Record was not added due to lack of system resources.
Please wrap long lines.
> +
> +**/
> +EFI_STATUS
> +LogSmbiosData (
> + IN UINT8 *Buffer,
> + IN OUT EFI_SMBIOS_HANDLE *SmbiosHandle
> + )
> +{
> + EFI_STATUS Status;
> +
> + *SmbiosHandle = SMBIOS_HANDLE_PI_RESERVED;
> +
> + Status = mSmbios->Add (
> + mSmbios,
Indentation.
> + NULL,
> + SmbiosHandle,
> + (EFI_SMBIOS_TABLE_HEADER *)Buffer
> + );
> +
> + return Status;
> +}
> +
> +/**
> + Fetches a list of the specified SMBIOS table types.
> +
> + @param[in] SmbiosType The type of table to fetch
> + @param[out] **HandleArray The array of handles
> + @param[out] *HandleCount Number of handles in the array
> +**/
> +VOID
> +GetLinkTypeHandle(
> + IN UINT8 SmbiosType,
> + OUT SMBIOS_HANDLE **HandleArray,
> + OUT UINTN *HandleCount
> + )
> +{
> + UINTN Index;
> + EFI_STATUS Status;
> + EFI_SMBIOS_HANDLE SmbiosHandle;
> + EFI_SMBIOS_TABLE_HEADER *Record;
> +
> + if (mSmbios == NULL) {
> + return;
> + }
> +
> + SmbiosHandle = SMBIOS_HANDLE_PI_RESERVED;
> + *HandleCount = 0;
> +
> + // Iterate through entries to get the number
> + while (TRUE) {
> + Status = mSmbios->GetNext (
> + mSmbios,
> + &SmbiosHandle,
> + &SmbiosType,
> + &Record,
> + NULL
> + );
> +
> + if (!EFI_ERROR (Status)) {
> + (*HandleCount)++;
> + } else {
> + break;
> + }
> + }
Would this look neater as
do {
...
if (Status == EFI_SUCCESS) {
(*HandleCount)++;
}
} while (!EFI_ERROR (Status));
?
But actually, could you break the count out into a static helper function?
/
Leif
> +
> + *HandleArray = AllocateZeroPool (sizeof (SMBIOS_HANDLE) * (*HandleCount));
> + if (*HandleArray == NULL) {
> + DEBUG ((DEBUG_ERROR, "HandleArray allocate memory resource failed.\n"));
> + *HandleCount = 0;
> + return;
> + }
> +
> + SmbiosHandle = SMBIOS_HANDLE_PI_RESERVED;
> +
> + for (Index = 0; Index < (*HandleCount); Index++) {
> + Status = mSmbios->GetNext (
> + mSmbios,
> + &SmbiosHandle,
> + &SmbiosType,
> + &Record,
> + NULL
> + );
> +
> + if (!EFI_ERROR (Status)) {
> + (*HandleArray)[Index] = Record->Handle;
> + } else {
> + break;
> + }
> + }
> +}
> +
> diff --git a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscLibStrings.uni b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscLibStrings.uni
> new file mode 100644
> index 000000000000..32f30b41566d
> --- /dev/null
> +++ b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/SmbiosMiscLibStrings.uni
> @@ -0,0 +1,21 @@
> +/** @file
> + * Based on files under Nt32Pkg/MiscSubClassPlatformDxe/
> + *
> + * Copyright (c) 2006 - 2010, Intel Corporation. All rights reserved.<BR>
> + * Copyright (c) 2015, Hisilicon Limited. All rights reserved.<BR>
> + * Copyright (c) 2015, Linaro Limited. All rights reserved.<BR>
> + * SPDX-License-Identifier: BSD-2-Clause-Patent
> + *
> + *
> +**/
> +
> +
> +/=#
> +
> +#langdef en-US "English"
> +
> +#include "Type00/MiscBiosVendor.uni"
> +#include "Type01/MiscSystemManufacturer.uni"
> +#include "Type02/MiscBaseBoardManufacturer.uni"
> +#include "Type03/MiscChassisManufacturer.uni"
> +#include "Type13/MiscNumberOfInstallableLanguages.uni"
> --
> 2.26.2
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#70083): https://edk2.groups.io/g/devel/message/70083
Mute This Topic: https://groups.io/mt/79438423/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list