Re: [PATCH V2] IntelSiliconPkg: 0468303 caused to breaking the SMBIOS tables generation code

Re: [PATCH V2] IntelSiliconPkg: 0468303 caused to breaking the SMBIOS tables generation code

[Yao, Jiewen]

Reviewed-by: Jiewen.yao@intel.com

> -----Original Message-----
> From: Chan, Amy
> Sent: Friday, December 16, 2016 3:11 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Zeng, Star <star.zeng@intel.com>; Chiu;
> Chiu, Chasel <chasel.chiu@intel.com>; Chan; Chan, Amy <amy.chan@intel.com>
> Subject: [PATCH V2] IntelSiliconPkg: 0468303 caused to breaking the SMBIOS
> tables generation code
>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Chiu, Chasel <chasel.chiu@intel.com>
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Chan, Amy <amy.chan@intel.com>
>
> System test:
> Smbios tables are correctly published on an intel internal platform.
>
>  .../DxeSmbiosDataHobLib/DxeSmbiosDataHobLib.c      | 134
> +++------------------
>  1 file changed, 14 insertions(+), 120 deletions(-)
>
> diff --git
> a/IntelSiliconPkg/Library/DxeSmbiosDataHobLib/DxeSmbiosDataHobLib.c
> b/IntelSiliconPkg/Library/DxeSmbiosDataHobLib/DxeSmbiosDataHobLib.c
> index 8d513e0..968d372 100644
> --- a/IntelSiliconPkg/Library/DxeSmbiosDataHobLib/DxeSmbiosDataHobLib.c
> +++ b/IntelSiliconPkg/Library/DxeSmbiosDataHobLib/DxeSmbiosDataHobLib.c
> @@ -27,95 +27,6 @@
>  #include <Library/UefiBootServicesTableLib.h>
>  #include <Protocol/Smbios.h>
>
> -/**
> -
> -  Get the full size of SMBIOS structure including optional strings that follow the
> formatted structure.
> -  @note: This function is copy from SmbiosDxe in MdeModulePkg.
> -
> -  @param[in] This                 The EFI_SMBIOS_PROTOCOL instance.
> -  @param[in] Head                 Pointer to the beginning of SMBIOS
> structure.
> -  @param[out] Size                The returned size.
> -  @param[out] NumberOfStrings     The returned number of optional strings
> that follow the formatted structure.
> -
> -  @retval EFI_SUCCESS           Size returned in Size.
> -  @retval EFI_INVALID_PARAMETER Input SMBIOS structure mal-formed or Size
> is NULL.
> -
> -**/
> -EFI_STATUS
> -EFIAPI
> -GetSmbiosStructureSize (
> -  IN   CONST EFI_SMBIOS_PROTOCOL        *This,
> -  IN   EFI_SMBIOS_TABLE_HEADER          *Head,
> -  OUT  UINTN                            *Size,
> -  OUT  UINTN                            *NumberOfStrings
> -  )
> -{
> -  UINTN  FullSize;
> -  UINTN  StrLen;
> -  UINTN  MaxLen;
> -  INT8*  CharInStr;
> -
> -  if (Size == NULL || NumberOfStrings == NULL) {
> -    return EFI_INVALID_PARAMETER;
> -  }
> -
> -  FullSize = Head->Length;
> -  CharInStr = (INT8*)Head + Head->Length;
> -  *Size = FullSize;
> -  *NumberOfStrings = 0;
> -  StrLen = 0;
> -
> -  //
> -  // look for the two consecutive zeros, check the string limit by the way.
> -  //
> -  while (*CharInStr != 0 || *(CharInStr+1) != 0) {
> -    if (*CharInStr == 0) {
> -      *Size += 1;
> -      CharInStr++;
> -    }
> -
> -    if (This->MajorVersion < 2 || (This->MajorVersion == 2 &&
> This->MinorVersion < 7)) {
> -      MaxLen = SMBIOS_STRING_MAX_LENGTH;
> -    } else if (This->MajorVersion < 3) {
> -      //
> -      // Reference SMBIOS 2.7, chapter 6.1.3, it will have no limit on the length
> of each individual text string.
> -      // However, the length of the entire structure table (including all strings)
> must be reported
> -      // in the Structure Table Length field of the SMBIOS Structure Table Entry
> Point,
> -      // which is a WORD field limited to 65,535 bytes.
> -      //
> -      MaxLen = SMBIOS_TABLE_MAX_LENGTH;
> -    } else {
> -      //
> -      // SMBIOS 3.0 defines the Structure table maximum size as DWORD field
> limited to 0xFFFFFFFF bytes.
> -      // Locate the end of string as long as possible.
> -      //
> -      MaxLen = SMBIOS_3_0_TABLE_MAX_LENGTH;
> -    }
> -
> -    for (StrLen = 0 ; StrLen < MaxLen; StrLen++) {
> -      if (*(CharInStr+StrLen) == 0) {
> -        break;
> -      }
> -    }
> -
> -    if (StrLen == MaxLen) {
> -      return EFI_INVALID_PARAMETER;
> -    }
> -
> -    //
> -    // forward the pointer
> -    //
> -    CharInStr += StrLen;
> -    *Size += StrLen;
> -    *NumberOfStrings += 1;
> -  }
> -
> -  //
> -  // count ending two zeros.
> -  //
> -  *Size += 2;
> -  return EFI_SUCCESS;
> -}
>
>  /**
>    Adds SMBIOS records to tables
> @@ -138,12 +49,8 @@ DxeSmbiosDataHobLibConstructor (
>    EFI_SMBIOS_HANDLE            SmbiosHandle;
>    EFI_SMBIOS_PROTOCOL          *Smbios;
>    EFI_STATUS                   Status;
> -  UINTN                        InstalledPayloadSize;
> -  UINTN                        MaxPayloadSize;
>    UINT8                        *RecordPtr;
>    UINT16                       RecordCount;
> -  UINTN                        StructureSize;
> -  UINTN                        NumberOfStrings;
>
>    RecordCount = 0;
>
> @@ -151,42 +58,29 @@ DxeSmbiosDataHobLibConstructor (
>
>    Status = gBS->LocateProtocol (&gEfiSmbiosProtocolGuid, NULL, (VOID
> **)&Smbios);
>    if (Smbios == NULL) {
> -    DEBUG ((DEBUG_WARN, "  Can't locate SMBIOS protocol\n"));
> +    DEBUG ((DEBUG_WARN, "Can't locate SMBIOS protocol\n"));
>      return EFI_UNSUPPORTED;
>    }
>
>    ///
> -  /// Get SMBIOS HOB data
> +  /// Get SMBIOS HOB data (each hob contains one SMBIOS record)
>    ///
>    for (Hob.Raw = GetHobList (); !END_OF_HOB_LIST(Hob); Hob.Raw =
> GET_NEXT_HOB (Hob)) {
>      if ((GET_HOB_TYPE (Hob) == EFI_HOB_TYPE_GUID_EXTENSION) &&
> (CompareGuid (&Hob.Guid->Name, &gIntelSmbiosDataHobGuid))) {
> -      RecordPtr = (UINT8 *)Hob.Raw + sizeof (EFI_HOB_GUID_TYPE);
> -      MaxPayloadSize = Hob.Guid->Header.HobLength - sizeof
> (EFI_HOB_GUID_TYPE);
> -
> -      InstalledPayloadSize = 0;
> -      do {
> -        StructureSize = 0;
> -        Status = GetSmbiosStructureSize (Smbios,
> (EFI_SMBIOS_TABLE_HEADER *)RecordPtr, &StructureSize, &NumberOfStrings);
> -        if ((Status == EFI_SUCCESS) && (InstalledPayloadSize + StructureSize
> <= MaxPayloadSize)) {
> -          InstalledPayloadSize += StructureSize;
> -
> -          ///
> -          /// Add generic SMBIOS HOB to SMBIOS table
> -          ///
> -          DEBUG ((DEBUG_VERBOSE, "  Add SMBIOS record type: %x\n",
> ((EFI_SMBIOS_TABLE_HEADER *) RecordPtr)->Type));
> -          SmbiosHandle = SMBIOS_HANDLE_PI_RESERVED;
> -          Status = Smbios->Add (Smbios, NULL, &SmbiosHandle,
> (EFI_SMBIOS_TABLE_HEADER *) RecordPtr);
> -          if (!EFI_ERROR (Status)) {
> -            RecordPtr += StructureSize;
> -            RecordCount++;
> -          }
> -        } else {
> -          break;
> -        }
> -      } while (TRUE);
> +    RecordPtr = GET_GUID_HOB_DATA (Hob.Raw);
> +
> +      ///
> +      /// Add generic SMBIOS HOB to SMBIOS table
> +      ///
> +      DEBUG ((DEBUG_VERBOSE, "Add SMBIOS record type: %x\n",
> ((EFI_SMBIOS_TABLE_HEADER *) RecordPtr)->Type));
> +      SmbiosHandle = SMBIOS_HANDLE_PI_RESERVED;
> +      Status = Smbios->Add (Smbios, NULL, &SmbiosHandle,
> (EFI_SMBIOS_TABLE_HEADER *) RecordPtr);
> +      if (!EFI_ERROR (Status)) {
> +        RecordCount++;
> +      }
>      }
>    }
> -  DEBUG ((DEBUG_INFO, "  Found %d Records and added to SMBIOS table.\n",
> RecordCount));
> +  DEBUG ((DEBUG_INFO, "Found %d Records and added to SMBIOS table.\n",
> RecordCount));
>
>    return EFI_SUCCESS;
>  }