Re: [edk2-devel][edk2-platforms][PATCH V1 3/9] WhitleyOpenBoardPkg/BaseCrcLib: Add library for CRC16

["Pedro Falcato" <pedro.falcato@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 7200 bytes --]

Hi,

I've just noticed this patch adds CRC16, which I've already added to my
Ext4Pkg (
https://github.com/tianocore/edk2-platforms/blob/master/Features/Ext4Pkg/Ext4Dxe/Crc16.c
).
I suggest we add CRC16 (and possibly CRC32C, which I already have in my
package as well) to MdePkg, as to de-duplicate code which might be useful
in other places.
What do you think? If it sounds good to you, I'll open a bugzilla and work
on that.

Best regards,
Pedro

On Thu, Mar 10, 2022 at 10:41 PM Oram, Isaac W <isaac.w.oram@intel.com>
wrote:

> Core only supports CRC32, this library adds CRC16 support.
>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Signed-off-by: Isaac Oram <isaac.w.oram@intel.com>
> ---
>  Platform/Intel/WhitleyOpenBoardPkg/Include/Library/CrcLib.h          | 42
> ++++++++++++
>  Platform/Intel/WhitleyOpenBoardPkg/Library/BaseCrcLib/BaseCrcLib.c   | 71
> ++++++++++++++++++++
>  Platform/Intel/WhitleyOpenBoardPkg/Library/BaseCrcLib/BaseCrcLib.inf | 23
> +++++++
>  Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc                   |  1
> +
>  4 files changed, 137 insertions(+)
>
> diff --git a/Platform/Intel/WhitleyOpenBoardPkg/Include/Library/CrcLib.h
> b/Platform/Intel/WhitleyOpenBoardPkg/Include/Library/CrcLib.h
> new file mode 100644
> index 0000000000..7ca3b7cabb
> --- /dev/null
> +++ b/Platform/Intel/WhitleyOpenBoardPkg/Include/Library/CrcLib.h
> @@ -0,0 +1,42 @@
> +/** @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/BaseCrcLib/BaseCrcLib.c
> b/Platform/Intel/WhitleyOpenBoardPkg/Library/BaseCrcLib/BaseCrcLib.c
> new file mode 100644
> index 0000000000..3e8fa402ad
> --- /dev/null
> +++ b/Platform/Intel/WhitleyOpenBoardPkg/Library/BaseCrcLib/BaseCrcLib.c
> @@ -0,0 +1,71 @@
> +/** @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
> new file mode 100644
> index 0000000000..6b404e1259
> --- /dev/null
> +++ b/Platform/Intel/WhitleyOpenBoardPkg/Library/BaseCrcLib/BaseCrcLib.inf
> @@ -0,0 +1,23 @@
> +## @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 e78a104004..9cdb5bc2f6 100644
> --- a/Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc
> +++ b/Platform/Intel/WhitleyOpenBoardPkg/PlatformPkg.dsc
> @@ -618,6 +618,7 @@
>
>  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
>
>  [LibraryClasses.Common.SEC, LibraryClasses.Common.PEI_CORE,
> LibraryClasses.Common.PEIM]
>
>  FspWrapperApiLib|IntelFsp2WrapperPkg/Library/BaseFspWrapperApiLib/BaseFspWrapperApiLib.inf
> --
> 2.27.0.windows.1
>
>
>
> 
>
>
>

-- 
Pedro Falcato

[-- Attachment #2: Type: text/html, Size: 8816 bytes --]