Re: [edk2-devel] [PATCH v5 17/23] ArmPkg: Add Universal/Smbios/SmbiosMiscDxe/Type32

["Samer El-Haj-Mahmoud" <samer.el-haj-mahmoud@arm.com>]

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Rebecca
> Cran via groups.io
> Sent: Monday, January 4, 2021 5:58 PM
> To: devel@edk2.groups.io
> Cc: Rebecca Cran <rebecca@nuviainc.com>; Leif Lindholm
> <leif@nuviainc.com>; Ard Biesheuvel <Ard.Biesheuvel@arm.com>; Sami
> Mujawar <Sami.Mujawar@arm.com>; Liming Gao
> <gaoliming@byosoft.com.cn>; Michael D Kinney
> <michael.d.kinney@intel.com>; Zhiguang Liu <zhiguang.liu@intel.com>
> Subject: [edk2-devel] [PATCH v5 17/23] ArmPkg: Add
> Universal/Smbios/SmbiosMiscDxe/Type32
>
> This code provides information for the SMBIOS Type 32 table.
>
> Signed-off-by: Rebecca Cran <rebecca@nuviainc.com>
> ---
>
> ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type32/MiscBootInformationDat
> a.c     | 34 ++++++++++
>
> ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type32/MiscBootInformationFu
> nction.c | 66 ++++++++++++++++++++
>  2 files changed, 100 insertions(+)
>
> diff --git
> a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type32/MiscBootInformation
> Data.c
> b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type32/MiscBootInformation
> Data.c
> new file mode 100644
> index 000000000000..e72656d3b002
> --- /dev/null
> +++
> b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type32/MiscBootInformation
> Da
> +++ ta.c
> @@ -0,0 +1,34 @@
> +/** @file
> +  This driver parses the mMiscSubclassDataTable structure and reports
> +  any generated data to the DataHub.
> +

This is not true in this specific implementation. There is no DataHub involved in this code



> +  Based on files under Nt32Pkg/MiscSubClassPlatformDxe/
> +
> +  Copyright (c) 2006 - 2009, 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"
> +
> +//
> +// Static (possibly build generated) Bios Vendor data.
> +//
> +MISC_SMBIOS_TABLE_DATA(SMBIOS_TABLE_TYPE32,
> MiscBootInformation) = {
> +  {                                                     // Hdr
> +    EFI_SMBIOS_TYPE_SYSTEM_BOOT_INFORMATION,              // Type,
> +    0,                                                    // Length,
> +    0                                                     // Handle
> +  },
> +  {                                                     // Reserved[6]
> +    0,
> +    0,
> +    0,
> +    0,
> +    0,
> +    0
> +  },
> +  BootInformationStatusNoError                          // BootInformationStatus

While most implementations in edk2-platforms do indeed hard-code this value to BootInformationStatusNoError, this is an incomplete implementation. Platforms should be able to report their boot status / and specific boot errors.

Can you use a platform dynamic PCD (and default to BootInformationStatusNoError), as done for example in Features/Intel/SystemInformation/SmbiosFeaturePkg/SmbiosFeaturePkg.dec ? Or maybe add to a library function in the newly introduced OemMiscLib ?




> +};
> diff --git
> a/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type32/MiscBootInformationF
> unction.c
> b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type32/MiscBootInformationF
> unction.c
> new file mode 100644
> index 000000000000..dabecbbd3a64
> --- /dev/null
> +++
> b/ArmPkg/Universal/Smbios/SmbiosMiscDxe/Type32/MiscBootInformationF
> u
> +++ nction.c
> @@ -0,0 +1,66 @@
> +/** @file
> +  boot information boot time changes.
> +  SMBIOS type 32.
> +
> +  Based on files under Nt32Pkg/MiscSubClassPlatformDxe/
> +
> +  Copyright (c) 2009 - 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"
> +
> +/**
> +  This function makes boot time changes to the contents of the
> +  MiscBootInformation (Type 32).
> +
> +  @param  RecordData                 Pointer to copy of RecordData from the Data
> Table.
> +
> +  @retval EFI_SUCCESS                All parameters were valid.
> +  @retval EFI_UNSUPPORTED            Unexpected RecordType value.
> +  @retval EFI_INVALID_PARAMETER      Invalid parameter was found.
> +
> +**/
> +MISC_SMBIOS_TABLE_FUNCTION(MiscBootInformation)
> +{
> +  EFI_STATUS                         Status;
> +  EFI_SMBIOS_HANDLE                  SmbiosHandle;
> +  SMBIOS_TABLE_TYPE32                *SmbiosRecord;
> +  SMBIOS_TABLE_TYPE32                *InputData;
> +
> +  //
> +  // First check for invalid parameters.
> +  //
> +  if (RecordData == NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  InputData = (SMBIOS_TABLE_TYPE32 *)RecordData;
> +
> +  //
> +  // Two zeros following the last string.
> +  //
> +  SmbiosRecord = AllocateZeroPool (sizeof (SMBIOS_TABLE_TYPE32) + 1 +
> + 1);  if (SmbiosRecord == NULL) {
> +    return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  (VOID)CopyMem (SmbiosRecord, InputData, sizeof
> + (SMBIOS_TABLE_TYPE32));
> +
> +  SmbiosRecord->Hdr.Length = sizeof (SMBIOS_TABLE_TYPE32);
> +
> +  //
> +  // Now we have got the full smbios record, call smbios protocol to add this
> record.
> +  //
> +  Status = LogSmbiosData ((UINT8*)SmbiosRecord, &SmbiosHandle);  if
> + (EFI_ERROR (Status)) {
> +    DEBUG ((DEBUG_ERROR, "[%a]:[%dL] Smbios Type32 Table Log Failed! %r
> \n",
> +            __FUNCTION__, __LINE__, Status));  }
> +
> +  FreePool (SmbiosRecord);
> +  return Status;
> +}
> --
> 2.26.2
>
>
>
> 
>

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.