Hi,

Just wanted to note that this patch set is not mergeable/testable right now. I'll submit a correct v2 as soon as possible.

Thanks for the understanding,
Pedro

On Sun, Mar 27, 2022 at 12:59 AM Pedro Falcato <pedro.falcato@gmail.com> wrote:
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3871

Delete BaseCrcLib and slightly adjust each CalculateCrc16 call
for the BaseLib CalculateCrc16() interface, as part of an effort to
unify CRC16 implementations.

Cc: Isaac Oram <isaac.w.oram@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>

Signed-off-by: Pedro Falcato <pedro.falcato@gmail.com>
---
 .../Include/Library/CrcLib.h                  | 42 -----------
 .../AcpiPlatformLibBdat.c                     | 46 ++++--------
 .../Library/BaseCrcLib/BaseCrcLib.c           | 71 -------------------
 .../Library/BaseCrcLib/BaseCrcLib.inf         | 23 ------
 .../Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc |  1 -
 5 files changed, 12 insertions(+), 171 deletions(-)
 delete mode 100644 Platform/Intel/WhitleyOpenBoardPkg/Include/Library/CrcLib.h
 delete mode 100644 Platform/Intel/WhitleyOpenBoardPkg/Library/BaseCrcLib/BaseCrcLib.c
 delete mode 100644 Platform/Intel/WhitleyOpenBoardPkg/Library/BaseCrcLib/BaseCrcLib.inf

diff --git a/Platform/Intel/WhitleyOpenBoardPkg/Include/Library/CrcLib.h b/Platform/Intel/WhitleyOpenBoardPkg/Include/Library/CrcLib.h
deleted file mode 100644
index 7ca3b7cabb14..000000000000
--- a/Platform/Intel/WhitleyOpenBoardPkg/Include/Library/CrcLib.h
+++ /dev/null
@@ -1,42 +0,0 @@
-/** @file
-  Interface header file for the CRC library class.
-
-  @copyright
-  Copyright 2016 - 2018 Intel Corporation. <BR>
-
-  SPDX-License-Identifier: BSD-2-Clause-Patent
-**/
-
-#ifndef _CRC_LIB_H_
-#define _CRC_LIB_H_
-
-#include <Uefi.h>
-
-/**
-  Calculate a 16-bit CRC.
-
-  The algorithm used is MSB-first form of the ITU-T Recommendation V.41, which
-  uses an initial value of 0x0000 and a polynomial of 0x1021. It is the same
-  algorithm used by XMODEM.
-
-  The output CRC location is not updated until the calculation is finished, so
-  it is possible to pass a structure as the data, and the CRC field of the same
-  structure as the output location for the calculated CRC. The CRC field should
-  be set to zero before calling this function. Once the CRC field is updated by
-  this function, running it again over the structure produces a CRC of zero.
-
-  @param[in]  Data              A pointer to the target data.
-  @param[in]  DataSize          The target data size.
-  @param[out] CrcOut            A pointer to the return location of the CRC.
-
-  @retval EFI_SUCCESS           The CRC was calculated successfully.
-  @retval EFI_INVALID_PARAMETER A null pointer was provided.
-**/
-EFI_STATUS
-CalculateCrc16 (
-  IN  VOID    *Data,
-  IN  UINTN   DataSize,
-  OUT UINT16  *CrcOut
-  );
-
-#endif  // _CRC_LIB_H_
diff --git a/Platform/Intel/WhitleyOpenBoardPkg/Library/AcpiPlatformTableLib/AcpiPlatformLibBdat.c b/Platform/Intel/WhitleyOpenBoardPkg/Library/AcpiPlatformTableLib/AcpiPlatformLibBdat.c
index 09464b4a11d1..a1780ced39aa 100644
--- a/Platform/Intel/WhitleyOpenBoardPkg/Library/AcpiPlatformTableLib/AcpiPlatformLibBdat.c
+++ b/Platform/Intel/WhitleyOpenBoardPkg/Library/AcpiPlatformTableLib/AcpiPlatformLibBdat.c
@@ -11,9 +11,9 @@
 // Statements that include other files
 //
 #include "AcpiPlatformLibLocal.h"
-#include <Library/CrcLib.h>
 #include <BdatSchema.h>
 #include <Guid/MemoryMapData.h>
+#include <Library/BaseLib.h>
 #include <Library/CompressedVariableLib.h>
 #include <Protocol/DynamicSiLibraryProtocol2.h>
 #include <Protocol/DynamicSiLibraryProtocol2.h>
@@ -287,15 +287,12 @@ CreateBdatHeader (
   // CRC16 value of the BDAT_STRUCTURE
   //
   (*BdatHeaderStructPtr)->BdatHeader.Crc16 = 0;
-  Status = CalculateCrc16 (
+  (*BdatHeaderStructPtr)->BdatHeader.Crc16 = CalculateCrc16 (
     (VOID *)(*BdatHeaderStructPtr),
     BdatSize,
-    &(*BdatHeaderStructPtr)->BdatHeader.Crc16
+    0
   );
-  ASSERT_EFI_ERROR (Status);
-  if (EFI_ERROR (Status)) {
-    (*BdatHeaderStructPtr)->BdatHeader.Crc16 = 0xFFFF;
-  }
+
   (*BdatHeaderStructPtr)->BdatSchemas.SchemaListLength = NumberOfSchema;
   (*BdatHeaderStructPtr)->BdatSchemas.Reserved = 0;
   (*BdatHeaderStructPtr)->BdatSchemas.Reserved1 = 0;
@@ -1088,15 +1085,11 @@ SaveBssaResultsToBdat (
       // CRC16 value of the BDAT_SCHEMA_HEADER_STRUCTURE
       //
       BssaSchemaHeaderPtr->Crc16 = 0;
-      Status = CalculateCrc16 (
+      BssaSchemaHeaderPtr->Crc16 = CalculateCrc16 (
         (VOID *) BssaSchemaHeaderPtr,
         sizeof (BDAT_SCHEMA_HEADER_STRUCTURE),
-        &BssaSchemaHeaderPtr->Crc16
+        0
         );
-      ASSERT_EFI_ERROR (Status);
-      if (EFI_ERROR (Status)) {
-        BssaSchemaHeaderPtr->Crc16 = 0xFFFF;
-      }

       if (RemainingHobSizeBssaSchema < CurrentHobSize) {
         DEBUG ((DEBUG_WARN, "Not enough space to add complete BIOS SSA result\n"));
@@ -1217,17 +1210,12 @@ SaveEwlToBdat (
   // CRC16 value of the BDAT_SCHEMA_HEADER_STRUCTURE
   //
   EwlSchemaHeaderPtr->Crc16 = 0;
-  Status = CalculateCrc16 (
+  EwlSchemaHeaderPtr->Crc16 = CalculateCrc16 (
     (VOID *)EwlSchemaHeaderPtr,
     sizeof(BDAT_SCHEMA_HEADER_STRUCTURE),
-    &EwlSchemaHeaderPtr->Crc16
+    0
   );

-  ASSERT_EFI_ERROR (Status);
-  if (EFI_ERROR (Status)) {
-    EwlSchemaHeaderPtr->Crc16 = 0xFFFF;
-  }
-
   GuidHob = GetFirstGuidHob (&EWLDataGuid);
   EwlPrivateData = GET_GUID_HOB_DATA (GuidHob);

@@ -1334,17 +1322,12 @@ SaveSpdToBdat (
   // CRC16 value of the BDAT_SCHEMA_HEADER_STRUCTURE
   //
   SpdSchemaHeaderPtr->Crc16 = 0;
-  Status = CalculateCrc16 (
+  SpdSchemaHeaderPtr->Crc16 = CalculateCrc16 (
     (VOID *)SpdSchemaHeaderPtr,
     sizeof(BDAT_SCHEMA_HEADER_STRUCTURE),
-    &SpdSchemaHeaderPtr->Crc16
+    0
   );

-  ASSERT_EFI_ERROR (Status);
-  if (EFI_ERROR (Status)) {
-    SpdSchemaHeaderPtr->Crc16 = 0xFFFF;
-  }
-
   Address = Address + sizeof(BDAT_SCHEMA_HEADER_STRUCTURE);
   *SchemaSpaceUsed = *SchemaSpaceUsed + sizeof(BDAT_SCHEMA_HEADER_STRUCTURE);

@@ -1483,17 +1466,12 @@ SaveTrainingDataToBdat (
   // CRC16 value of the BDAT_SCHEMA_HEADER_STRUCTURE
   //
   SchemaHeaderPtr->Crc16 = 0;
-  Status = CalculateCrc16 (
+  SchemaHeaderPtr->Crc16 = CalculateCrc16 (
     (VOID *)SchemaHeaderPtr,
     sizeof(BDAT_SCHEMA_HEADER_STRUCTURE),
-    &SchemaHeaderPtr->Crc16
+    0
   );

-  ASSERT_EFI_ERROR (Status);
-  if (EFI_ERROR (Status)) {
-    SchemaHeaderPtr->Crc16 = 0xFFFF;
-  }
-
   GuidHob = GetFirstGuidHob (&TrainingDataGuid);

   ASSERT (GuidHob != NULL);
diff --git a/Platform/Intel/WhitleyOpenBoardPkg/Library/BaseCrcLib/BaseCrcLib.c b/Platform/Intel/WhitleyOpenBoardPkg/Library/BaseCrcLib/BaseCrcLib.c
deleted file mode 100644
index 3e8fa402add3..000000000000
--- a/Platform/Intel/WhitleyOpenBoardPkg/Library/BaseCrcLib/BaseCrcLib.c
+++ /dev/null
@@ -1,71 +0,0 @@
-/** @file
-  Base implementation of the CRC library class.
-
-  @copyright
-  Copyright 2016 - 2018 Intel Corporation. <BR>
-
-  SPDX-License-Identifier: BSD-2-Clause-Patent
-**/
-
-#include <Base.h>
-#include <Library/CrcLib.h>
-
-/**
-  Calculate a 16-bit CRC.
-
-  The algorithm used is MSB-first form of the ITU-T Recommendation V.41, which
-  uses an initial value of 0x0000 and a polynomial of 0x1021. It is the same
-  algorithm used by XMODEM.
-
-  The output CRC location is not updated until the calculation is finished, so
-  it is possible to pass a structure as the data, and the CRC field of the same
-  structure as the output location for the calculated CRC. The CRC field should
-  be set to zero before calling this function. Once the CRC field is updated by
-  this function, running it again over the structure produces a CRC of zero.
-
-  @param[in]  Data              A pointer to the target data.
-  @param[in]  DataSize          The target data size.
-  @param[out] CrcOut            A pointer to the return location of the CRC.
-
-  @retval EFI_SUCCESS           The CRC was calculated successfully.
-  @retval EFI_INVALID_PARAMETER A null pointer was provided.
-**/
-EFI_STATUS
-CalculateCrc16 (
-  IN  VOID    *Data,
-  IN  UINTN   DataSize,
-  OUT UINT16  *CrcOut
-  )
-{
-  UINT32  Crc;
-  UINTN   Index;
-  UINT8   *Byte;
-
-  if (Data == NULL || CrcOut == NULL) {
-    return EFI_INVALID_PARAMETER;
-  }
-
-  Crc = 0x0000;
-  for (Byte = (UINT8 *) Data; Byte < (UINT8 *) Data + DataSize; Byte++) {
-    //
-    // XOR the next data byte into the CRC.
-    //
-    Crc ^= (UINT16) *Byte << 8;
-    //
-    // Shift out eight bits, feeding back based on the polynomial whenever a
-    // 1 is shifted out of bit 15.
-    //
-    for (Index = 0; Index < 8; Index++) {
-      Crc <<= 1;
-      if (Crc & BIT16) {
-        Crc ^= 0x1021;
-      }
-    }
-  }
-
-  //
-  // Mask and return the 16-bit CRC.
-  //
-  *CrcOut = (UINT16) (Crc & 0xFFFF);
-  return EFI_SUCCESS;
-}
diff --git a/Platform/Intel/WhitleyOpenBoardPkg/Library/BaseCrcLib/BaseCrcLib.inf b/Platform/Intel/WhitleyOpenBoardPkg/Library/BaseCrcLib/BaseCrcLib.inf
deleted file mode 100644
index 6b404e125959..000000000000
--- a/Platform/Intel/WhitleyOpenBoardPkg/Library/BaseCrcLib/BaseCrcLib.inf
+++ /dev/null
@@ -1,23 +0,0 @@
-## @file
-# Base implementation of the CRC library class.
-#
-# @copyright
-# Copyright 2016 Intel Corporation. <BR>
-#
-# SPDX-License-Identifier: BSD-2-Clause-Patent
-##
-
-[Defines]
-  INF_VERSION                   = 0x00010019
-  BASE_NAME                     = BaseCrcLib
-  FILE_GUID                     = F3BE9A28-78A2-4B02-AB26-D27EE85D9256
-  MODULE_TYPE                   = BASE
-  VERSION_STRING                = 1.0
-  LIBRARY_CLASS                 = CrcLib
-
-[Sources]
-  BaseCrcLib.c
-
-[Packages]
-  MdePkg/MdePkg.dec
-  WhitleyOpenBoardPkg/PlatformPkg.dec
diff --git a/Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc b/Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc
index 8c4b9cf6ce28..5b9b08feca46 100644
--- a/Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc
+++ b/Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc
@@ -618,7 +618,6 @@
   PciSegmentInfoLib|$(PLATFORM_PKG)/Pci/Library/PciSegmentInfoLibSimple/PciSegmentInfoLibSimple.inf
   PlatformOpromPolicyLib|$(RP_PKG)/Library/PlatformOpromPolicyLibNull/PlatformOpromPolicyLibNull.inf
   VmgExitLib|UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.inf
-  CrcLib|WhitleyOpenBoardPkg/Library/BaseCrcLib/BaseCrcLib.inf
   PlatformSpecificAcpiTableLib|WhitleyOpenBoardPkg/Library/PlatformSpecificAcpiTableLibNull/PlatformSpecificAcpiTableLibNull.inf
   BuildAcpiTablesLib|WhitleyOpenBoardPkg/Library/BuildAcpiTablesLib/DxeBuildAcpiTablesLib.inf
   AcpiPlatformTableLib|WhitleyOpenBoardPkg/Library/AcpiPlatformTableLib/AcpiPlatformLib.inf
--
2.35.1



--
Pedro Falcato