public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 0/2] Add CRC16 and CRC32c to MdePkg/BaseLib
@ 2022-03-27  0:57 Pedro Falcato
  2022-03-27  0:57 ` [PATCH 1/2] MdePkg/BaseLib: Add CRC16 and CRC32c implementations Pedro Falcato
  2022-03-27  0:57 ` [PATCH 2/2] SourceLevelDebugPkg/DebugAgent: Delete the CRC16 implementation Pedro Falcato
  0 siblings, 2 replies; 5+ messages in thread
From: Pedro Falcato @ 2022-03-27  0:57 UTC (permalink / raw)
  To: devel; +Cc: Michael D Kinney, Liming Gao, Zhiguang Liu, Hao A Wu

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3871

This patch-set adds CRC16 and CRC32c to BaseLib and removes
SourceLevelDebugPkg (the only place where I could find CRC16 being used in edk2)'s implementation.

Note that there's another patch-set with edk2-platforms changes that's supposed to be applied more or less
around the time this one is applied. The process may be awkward, but hopefully nothing breaks.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Hao A Wu <hao.a.wu@intel.com>

Pedro Falcato (2):
  MdePkg/BaseLib: Add CRC16 and CRC32c implementations
  SourceLevelDebugPkg/DebugAgent: Delete the CRC16 implementation

 MdePkg/Include/Library/BaseLib.h              |  35 ++++-
 MdePkg/Library/BaseLib/CheckSum.c             | 144 ++++++++++++++++++
 .../DebugAgent/DebugAgentCommon/DebugAgent.c  |  35 -----
 3 files changed, 178 insertions(+), 36 deletions(-)

-- 
2.35.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] MdePkg/BaseLib: Add CRC16 and CRC32c implementations
  2022-03-27  0:57 [PATCH 0/2] Add CRC16 and CRC32c to MdePkg/BaseLib Pedro Falcato
@ 2022-03-27  0:57 ` Pedro Falcato
  2022-03-27  0:57 ` [PATCH 2/2] SourceLevelDebugPkg/DebugAgent: Delete the CRC16 implementation Pedro Falcato
  1 sibling, 0 replies; 5+ messages in thread
From: Pedro Falcato @ 2022-03-27  0:57 UTC (permalink / raw)
  To: devel; +Cc: Michael D Kinney, Liming Gao, Zhiguang Liu

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3871

Add the crc16 and crc32c implementations previously found at
Features/Ext4Pkg/Ext4Dxe/Crc{16,32c}.c to BaseLib.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>

Signed-off-by: Pedro Falcato <pedro.falcato@gmail.com>
---
 MdePkg/Include/Library/BaseLib.h  |  35 +++++++-
 MdePkg/Library/BaseLib/CheckSum.c | 144 ++++++++++++++++++++++++++++++
 2 files changed, 178 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/Library/BaseLib.h b/MdePkg/Include/Library/BaseLib.h
index 6aa0d972186e..c2819fdfc3f0 100644
--- a/MdePkg/Include/Library/BaseLib.h
+++ b/MdePkg/Include/Library/BaseLib.h
@@ -4503,6 +4503,40 @@ CalculateCrc32 (
   IN  UINTN  Length
   );
 
+/**
+   Calculates the CRC16 checksum of the given buffer.
+
+   @param[in]      Buffer        Pointer to the buffer.
+   @param[in]      Length        Length of the buffer, in bytes.
+   @param[in]      InitialValue  Initial value of the CRC.
+
+   @return The CRC16 checksum.
+**/
+UINT16
+EFIAPI
+CalculateCrc16 (
+  IN  CONST VOID  *Buffer,
+  IN  UINTN       Length,
+  IN  UINT16      InitialValue
+  );
+
+/**
+   Calculates the CRC32c checksum of the given buffer.
+
+   @param[in]      Buffer        Pointer to the buffer.
+   @param[in]      Length        Length of the buffer, in bytes.
+   @param[in]      InitialValue  Initial value of the CRC.
+
+   @return The CRC32c checksum.
+**/
+UINT32
+EFIAPI
+CalculateCrc32c (
+  IN CONST VOID  *Buffer,
+  IN UINTN       Length,
+  IN UINT32      InitialValue
+  );
+
 //
 // Base Library CPU Functions
 //
@@ -4512,7 +4546,6 @@ CalculateCrc32 (
 
   @param  Context1        Context1 parameter passed into SwitchStack().
   @param  Context2        Context2 parameter passed into SwitchStack().
-
 **/
 typedef
 VOID
diff --git a/MdePkg/Library/BaseLib/CheckSum.c b/MdePkg/Library/BaseLib/CheckSum.c
index 28dee5ac6a15..b4ec72bbadf7 100644
--- a/MdePkg/Library/BaseLib/CheckSum.c
+++ b/MdePkg/Library/BaseLib/CheckSum.c
@@ -3,6 +3,7 @@
   algorithm.
 
   Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2022, Pedro Falcato. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -618,3 +619,146 @@ CalculateCrc32 (
 
   return Crc ^ 0xffffffff;
 }
+
+GLOBAL_REMOVE_IF_UNREFERENCED STATIC CONST UINT16  mCrc16LookupTable[256] =
+{
+  0x0000, 0x1189, 0x2312, 0x329b, 0x4624, 0x57ad, 0x6536, 0x74bf,
+  0x8c48, 0x9dc1, 0xaf5a, 0xbed3, 0xca6c, 0xdbe5, 0xe97e, 0xf8f7,
+  0x0919, 0x1890, 0x2a0b, 0x3b82, 0x4f3d, 0x5eb4, 0x6c2f, 0x7da6,
+  0x8551, 0x94d8, 0xa643, 0xb7ca, 0xc375, 0xd2fc, 0xe067, 0xf1ee,
+  0x1232, 0x03bb, 0x3120, 0x20a9, 0x5416, 0x459f, 0x7704, 0x668d,
+  0x9e7a, 0x8ff3, 0xbd68, 0xace1, 0xd85e, 0xc9d7, 0xfb4c, 0xeac5,
+  0x1b2b, 0x0aa2, 0x3839, 0x29b0, 0x5d0f, 0x4c86, 0x7e1d, 0x6f94,
+  0x9763, 0x86ea, 0xb471, 0xa5f8, 0xd147, 0xc0ce, 0xf255, 0xe3dc,
+  0x2464, 0x35ed, 0x0776, 0x16ff, 0x6240, 0x73c9, 0x4152, 0x50db,
+  0xa82c, 0xb9a5, 0x8b3e, 0x9ab7, 0xee08, 0xff81, 0xcd1a, 0xdc93,
+  0x2d7d, 0x3cf4, 0x0e6f, 0x1fe6, 0x6b59, 0x7ad0, 0x484b, 0x59c2,
+  0xa135, 0xb0bc, 0x8227, 0x93ae, 0xe711, 0xf698, 0xc403, 0xd58a,
+  0x3656, 0x27df, 0x1544, 0x04cd, 0x7072, 0x61fb, 0x5360, 0x42e9,
+  0xba1e, 0xab97, 0x990c, 0x8885, 0xfc3a, 0xedb3, 0xdf28, 0xcea1,
+  0x3f4f, 0x2ec6, 0x1c5d, 0x0dd4, 0x796b, 0x68e2, 0x5a79, 0x4bf0,
+  0xb307, 0xa28e, 0x9015, 0x819c, 0xf523, 0xe4aa, 0xd631, 0xc7b8,
+  0x48c8, 0x5941, 0x6bda, 0x7a53, 0x0eec, 0x1f65, 0x2dfe, 0x3c77,
+  0xc480, 0xd509, 0xe792, 0xf61b, 0x82a4, 0x932d, 0xa1b6, 0xb03f,
+  0x41d1, 0x5058, 0x62c3, 0x734a, 0x07f5, 0x167c, 0x24e7, 0x356e,
+  0xcd99, 0xdc10, 0xee8b, 0xff02, 0x8bbd, 0x9a34, 0xa8af, 0xb926,
+  0x5afa, 0x4b73, 0x79e8, 0x6861, 0x1cde, 0x0d57, 0x3fcc, 0x2e45,
+  0xd6b2, 0xc73b, 0xf5a0, 0xe429, 0x9096, 0x811f, 0xb384, 0xa20d,
+  0x53e3, 0x426a, 0x70f1, 0x6178, 0x15c7, 0x044e, 0x36d5, 0x275c,
+  0xdfab, 0xce22, 0xfcb9, 0xed30, 0x998f, 0x8806, 0xba9d, 0xab14,
+  0x6cac, 0x7d25, 0x4fbe, 0x5e37, 0x2a88, 0x3b01, 0x099a, 0x1813,
+  0xe0e4, 0xf16d, 0xc3f6, 0xd27f, 0xa6c0, 0xb749, 0x85d2, 0x945b,
+  0x65b5, 0x743c, 0x46a7, 0x572e, 0x2391, 0x3218, 0x0083, 0x110a,
+  0xe9fd, 0xf874, 0xcaef, 0xdb66, 0xafd9, 0xbe50, 0x8ccb, 0x9d42,
+  0x7e9e, 0x6f17, 0x5d8c, 0x4c05, 0x38ba, 0x2933, 0x1ba8, 0x0a21,
+  0xf2d6, 0xe35f, 0xd1c4, 0xc04d, 0xb4f2, 0xa57b, 0x97e0, 0x8669,
+  0x7787, 0x660e, 0x5495, 0x451c, 0x31a3, 0x202a, 0x12b1, 0x0338,
+  0xfbcf, 0xea46, 0xd8dd, 0xc954, 0xbdeb, 0xac62, 0x9ef9, 0x8f70
+};
+
+/**
+   Calculates the CRC16 checksum of the given buffer.
+
+   @param[in]      Buffer        Pointer to the buffer.
+   @param[in]      Length        Length of the buffer, in bytes.
+   @param[in]      InitialValue  Initial value of the CRC.
+
+   @return The CRC16 checksum.
+**/
+UINT16
+EFIAPI
+CalculateCrc16 (
+  IN CONST VOID  *Buffer,
+  IN UINTN       Length,
+  IN UINT16      InitialValue
+  )
+{
+  CONST UINT8  *Buf;
+  UINT16       Crc;
+
+  Buf = Buffer;
+
+  Crc = ~InitialValue;
+
+  while (Length-- != 0) {
+    Crc = mCrc16LookupTable[(Crc & 0xFF) ^ *(Buf++)] ^ (Crc >> 8);
+  }
+
+  return ~Crc;
+}
+
+GLOBAL_REMOVE_IF_UNREFERENCED STATIC CONST UINT32  mCrc32cLookupTable[256] = {
+  0x00000000, 0xf26b8303, 0xe13b70f7, 0x1350f3f4, 0xc79a971f, 0x35f1141c,
+  0x26a1e7e8, 0xd4ca64eb, 0x8ad958cf, 0x78b2dbcc, 0x6be22838, 0x9989ab3b,
+  0x4d43cfd0, 0xbf284cd3, 0xac78bf27, 0x5e133c24, 0x105ec76f, 0xe235446c,
+  0xf165b798, 0x030e349b, 0xd7c45070, 0x25afd373, 0x36ff2087, 0xc494a384,
+  0x9a879fa0, 0x68ec1ca3, 0x7bbcef57, 0x89d76c54, 0x5d1d08bf, 0xaf768bbc,
+  0xbc267848, 0x4e4dfb4b, 0x20bd8ede, 0xd2d60ddd, 0xc186fe29, 0x33ed7d2a,
+  0xe72719c1, 0x154c9ac2, 0x061c6936, 0xf477ea35, 0xaa64d611, 0x580f5512,
+  0x4b5fa6e6, 0xb93425e5, 0x6dfe410e, 0x9f95c20d, 0x8cc531f9, 0x7eaeb2fa,
+  0x30e349b1, 0xc288cab2, 0xd1d83946, 0x23b3ba45, 0xf779deae, 0x05125dad,
+  0x1642ae59, 0xe4292d5a, 0xba3a117e, 0x4851927d, 0x5b016189, 0xa96ae28a,
+  0x7da08661, 0x8fcb0562, 0x9c9bf696, 0x6ef07595, 0x417b1dbc, 0xb3109ebf,
+  0xa0406d4b, 0x522bee48, 0x86e18aa3, 0x748a09a0, 0x67dafa54, 0x95b17957,
+  0xcba24573, 0x39c9c670, 0x2a993584, 0xd8f2b687, 0x0c38d26c, 0xfe53516f,
+  0xed03a29b, 0x1f682198, 0x5125dad3, 0xa34e59d0, 0xb01eaa24, 0x42752927,
+  0x96bf4dcc, 0x64d4cecf, 0x77843d3b, 0x85efbe38, 0xdbfc821c, 0x2997011f,
+  0x3ac7f2eb, 0xc8ac71e8, 0x1c661503, 0xee0d9600, 0xfd5d65f4, 0x0f36e6f7,
+  0x61c69362, 0x93ad1061, 0x80fde395, 0x72966096, 0xa65c047d, 0x5437877e,
+  0x4767748a, 0xb50cf789, 0xeb1fcbad, 0x197448ae, 0x0a24bb5a, 0xf84f3859,
+  0x2c855cb2, 0xdeeedfb1, 0xcdbe2c45, 0x3fd5af46, 0x7198540d, 0x83f3d70e,
+  0x90a324fa, 0x62c8a7f9, 0xb602c312, 0x44694011, 0x5739b3e5, 0xa55230e6,
+  0xfb410cc2, 0x092a8fc1, 0x1a7a7c35, 0xe811ff36, 0x3cdb9bdd, 0xceb018de,
+  0xdde0eb2a, 0x2f8b6829, 0x82f63b78, 0x709db87b, 0x63cd4b8f, 0x91a6c88c,
+  0x456cac67, 0xb7072f64, 0xa457dc90, 0x563c5f93, 0x082f63b7, 0xfa44e0b4,
+  0xe9141340, 0x1b7f9043, 0xcfb5f4a8, 0x3dde77ab, 0x2e8e845f, 0xdce5075c,
+  0x92a8fc17, 0x60c37f14, 0x73938ce0, 0x81f80fe3, 0x55326b08, 0xa759e80b,
+  0xb4091bff, 0x466298fc, 0x1871a4d8, 0xea1a27db, 0xf94ad42f, 0x0b21572c,
+  0xdfeb33c7, 0x2d80b0c4, 0x3ed04330, 0xccbbc033, 0xa24bb5a6, 0x502036a5,
+  0x4370c551, 0xb11b4652, 0x65d122b9, 0x97baa1ba, 0x84ea524e, 0x7681d14d,
+  0x2892ed69, 0xdaf96e6a, 0xc9a99d9e, 0x3bc21e9d, 0xef087a76, 0x1d63f975,
+  0x0e330a81, 0xfc588982, 0xb21572c9, 0x407ef1ca, 0x532e023e, 0xa145813d,
+  0x758fe5d6, 0x87e466d5, 0x94b49521, 0x66df1622, 0x38cc2a06, 0xcaa7a905,
+  0xd9f75af1, 0x2b9cd9f2, 0xff56bd19, 0x0d3d3e1a, 0x1e6dcdee, 0xec064eed,
+  0xc38d26c4, 0x31e6a5c7, 0x22b65633, 0xd0ddd530, 0x0417b1db, 0xf67c32d8,
+  0xe52cc12c, 0x1747422f, 0x49547e0b, 0xbb3ffd08, 0xa86f0efc, 0x5a048dff,
+  0x8ecee914, 0x7ca56a17, 0x6ff599e3, 0x9d9e1ae0, 0xd3d3e1ab, 0x21b862a8,
+  0x32e8915c, 0xc083125f, 0x144976b4, 0xe622f5b7, 0xf5720643, 0x07198540,
+  0x590ab964, 0xab613a67, 0xb831c993, 0x4a5a4a90, 0x9e902e7b, 0x6cfbad78,
+  0x7fab5e8c, 0x8dc0dd8f, 0xe330a81a, 0x115b2b19, 0x020bd8ed, 0xf0605bee,
+  0x24aa3f05, 0xd6c1bc06, 0xc5914ff2, 0x37faccf1, 0x69e9f0d5, 0x9b8273d6,
+  0x88d28022, 0x7ab90321, 0xae7367ca, 0x5c18e4c9, 0x4f48173d, 0xbd23943e,
+  0xf36e6f75, 0x0105ec76, 0x12551f82, 0xe03e9c81, 0x34f4f86a, 0xc69f7b69,
+  0xd5cf889d, 0x27a40b9e, 0x79b737ba, 0x8bdcb4b9, 0x988c474d, 0x6ae7c44e,
+  0xbe2da0a5, 0x4c4623a6, 0x5f16d052, 0xad7d5351
+};
+
+/**
+   Calculates the CRC32c checksum of the given buffer.
+
+   @param[in]      Buffer        Pointer to the buffer.
+   @param[in]      Length        Length of the buffer, in bytes.
+   @param[in]      InitialValue  Initial value of the CRC.
+
+   @return The CRC32c checksum.
+**/
+UINT32
+EFIAPI
+CalculateCrc32c (
+  IN CONST VOID  *Buffer,
+  IN UINTN       Length,
+  IN UINT32      InitialValue
+  )
+{
+  CONST UINT8  *Buf;
+  UINT32       Crc;
+
+  Buf = Buffer;
+  Crc = ~InitialValue;
+
+  while (Length-- != 0) {
+    Crc = mCrc32cLookupTable[(Crc & 0xFF) ^ *(Buf++)] ^ (Crc >> 8);
+  }
+
+  return ~Crc;
+}
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] SourceLevelDebugPkg/DebugAgent: Delete the CRC16 implementation
  2022-03-27  0:57 [PATCH 0/2] Add CRC16 and CRC32c to MdePkg/BaseLib Pedro Falcato
  2022-03-27  0:57 ` [PATCH 1/2] MdePkg/BaseLib: Add CRC16 and CRC32c implementations Pedro Falcato
@ 2022-03-27  0:57 ` Pedro Falcato
  2022-03-28  0:14   ` Wu, Hao A
  1 sibling, 1 reply; 5+ messages in thread
From: Pedro Falcato @ 2022-03-27  0:57 UTC (permalink / raw)
  To: devel; +Cc: Hao A Wu

BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3871

Deletes the CRC16 implementation as CalculateCrc16() is being merged
into BaseLib.

Cc: Hao A Wu <hao.a.wu@intel.com>
Signed-off-by: Pedro Falcato <pedro.falcato@gmail.com>
---
 .../DebugAgent/DebugAgentCommon/DebugAgent.c  | 35 -------------------
 1 file changed, 35 deletions(-)

diff --git a/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgent.c b/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgent.c
index a1e61a6ef90e..62c6a235d425 100644
--- a/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgent.c
+++ b/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgent.c
@@ -126,41 +126,6 @@ GLOBAL_REMOVE_IF_UNREFERENCED EFI_VECTOR_HANDOFF_INFO  mVectorHandoffInfoDebugAg
 
 GLOBAL_REMOVE_IF_UNREFERENCED UINTN  mVectorHandoffInfoCount = sizeof (mVectorHandoffInfoDebugAgent) / sizeof (EFI_VECTOR_HANDOFF_INFO);
 
-/**
-  Calculate CRC16 for target data.
-
-  @param[in]  Data              The target data.
-  @param[in]  DataSize          The target data size.
-  @param[in]  Crc               Initial CRC.
-
-  @return UINT16     The CRC16 value.
-
-**/
-UINT16
-CalculateCrc16 (
-  IN UINT8   *Data,
-  IN UINTN   DataSize,
-  IN UINT16  Crc
-  )
-{
-  UINTN  Index;
-  UINTN  BitIndex;
-
-  for (Index = 0; Index < DataSize; Index++) {
-    Crc ^= (UINT16)Data[Index];
-    for (BitIndex = 0; BitIndex < 8; BitIndex++) {
-      if ((Crc & 0x8000) != 0) {
-        Crc <<= 1;
-        Crc  ^= 0x1021;
-      } else {
-        Crc <<= 1;
-      }
-    }
-  }
-
-  return Crc;
-}
-
 /**
   Read IDT entry to check if IDT entries are setup by Debug Agent.
 
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] SourceLevelDebugPkg/DebugAgent: Delete the CRC16 implementation
  2022-03-27  0:57 ` [PATCH 2/2] SourceLevelDebugPkg/DebugAgent: Delete the CRC16 implementation Pedro Falcato
@ 2022-03-28  0:14   ` Wu, Hao A
  2022-03-28  1:49     ` Pedro Falcato
  0 siblings, 1 reply; 5+ messages in thread
From: Wu, Hao A @ 2022-03-28  0:14 UTC (permalink / raw)
  To: Pedro Falcato, devel@edk2.groups.io

Hello,

My local test shows that the results between the old (SourceLevelDebugPkg) & new (MdePkg) CRC16 implementation are different:

UINT8  Array[16] = {1,2,3,4,5,6,7,8,8,7,6,5,4,3,2,1};
UINT16 a, b;	
a = CalculateCrc16New(Array, sizeof(Array), 0xFFFF);
b = CalculateCrc16Old(Array, sizeof(Array), 0xFFFF);

Result:
a = 0x0af5
b = 0x2778

My take is that for the SourceLevelDebugPkg case, the CRC16 result should be the same (at BIOS host side). Otherwise, the target side will have issue for the CRC check.

Best Regards,
Hao Wu

> -----Original Message-----
> From: Pedro Falcato <pedro.falcato@gmail.com>
> Sent: Sunday, March 27, 2022 8:58 AM
> To: devel@edk2.groups.io
> Cc: Wu, Hao A <hao.a.wu@intel.com>
> Subject: [PATCH 2/2] SourceLevelDebugPkg/DebugAgent: Delete the CRC16
> implementation
> 
> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3871
> 
> Deletes the CRC16 implementation as CalculateCrc16() is being merged into
> BaseLib.
> 
> Cc: Hao A Wu <hao.a.wu@intel.com>
> Signed-off-by: Pedro Falcato <pedro.falcato@gmail.com>
> ---
>  .../DebugAgent/DebugAgentCommon/DebugAgent.c  | 35 -------------------
>  1 file changed, 35 deletions(-)
> 
> diff --git
> a/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgen
> t.c
> b/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgen
> t.c
> index a1e61a6ef90e..62c6a235d425 100644
> ---
> a/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgen
> t.c
> +++
> b/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgen
> t
> +++ .c
> @@ -126,41 +126,6 @@ GLOBAL_REMOVE_IF_UNREFERENCED
> EFI_VECTOR_HANDOFF_INFO  mVectorHandoffInfoDebugAg
> 
>  GLOBAL_REMOVE_IF_UNREFERENCED UINTN  mVectorHandoffInfoCount =
> sizeof (mVectorHandoffInfoDebugAgent) / sizeof
> (EFI_VECTOR_HANDOFF_INFO);
> 
> -/**
> -  Calculate CRC16 for target data.
> -
> -  @param[in]  Data              The target data.
> -  @param[in]  DataSize          The target data size.
> -  @param[in]  Crc               Initial CRC.
> -
> -  @return UINT16     The CRC16 value.
> -
> -**/
> -UINT16
> -CalculateCrc16 (
> -  IN UINT8   *Data,
> -  IN UINTN   DataSize,
> -  IN UINT16  Crc
> -  )
> -{
> -  UINTN  Index;
> -  UINTN  BitIndex;
> -
> -  for (Index = 0; Index < DataSize; Index++) {
> -    Crc ^= (UINT16)Data[Index];
> -    for (BitIndex = 0; BitIndex < 8; BitIndex++) {
> -      if ((Crc & 0x8000) != 0) {
> -        Crc <<= 1;
> -        Crc  ^= 0x1021;
> -      } else {
> -        Crc <<= 1;
> -      }
> -    }
> -  }
> -
> -  return Crc;
> -}
> -
>  /**
>    Read IDT entry to check if IDT entries are setup by Debug Agent.
> 
> --
> 2.35.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] SourceLevelDebugPkg/DebugAgent: Delete the CRC16 implementation
  2022-03-28  0:14   ` Wu, Hao A
@ 2022-03-28  1:49     ` Pedro Falcato
  0 siblings, 0 replies; 5+ messages in thread
From: Pedro Falcato @ 2022-03-28  1:49 UTC (permalink / raw)
  To: Wu, Hao A; +Cc: devel@edk2.groups.io

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

Hi,

Hao, you are 100% correct. It seems that not only is SourceLevelDebugPkg
using a different CRC-16 algorithm, my CalculateCrc16()'s lookup table is
incorrect.

I'll re-submit a v2 that fixes these issues.
Sorry for the trouble.

Thanks,
Pedro

On Mon, Mar 28, 2022 at 1:14 AM Wu, Hao A <hao.a.wu@intel.com> wrote:

> Hello,
>
> My local test shows that the results between the old (SourceLevelDebugPkg)
> & new (MdePkg) CRC16 implementation are different:
>
> UINT8  Array[16] = {1,2,3,4,5,6,7,8,8,7,6,5,4,3,2,1};
> UINT16 a, b;
> a = CalculateCrc16New(Array, sizeof(Array), 0xFFFF);
> b = CalculateCrc16Old(Array, sizeof(Array), 0xFFFF);
>
> Result:
> a = 0x0af5
> b = 0x2778
>
> My take is that for the SourceLevelDebugPkg case, the CRC16 result should
> be the same (at BIOS host side). Otherwise, the target side will have issue
> for the CRC check.
>
> Best Regards,
> Hao Wu
>
> > -----Original Message-----
> > From: Pedro Falcato <pedro.falcato@gmail.com>
> > Sent: Sunday, March 27, 2022 8:58 AM
> > To: devel@edk2.groups.io
> > Cc: Wu, Hao A <hao.a.wu@intel.com>
> > Subject: [PATCH 2/2] SourceLevelDebugPkg/DebugAgent: Delete the CRC16
> > implementation
> >
> > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3871
> >
> > Deletes the CRC16 implementation as CalculateCrc16() is being merged into
> > BaseLib.
> >
> > Cc: Hao A Wu <hao.a.wu@intel.com>
> > Signed-off-by: Pedro Falcato <pedro.falcato@gmail.com>
> > ---
> >  .../DebugAgent/DebugAgentCommon/DebugAgent.c  | 35 -------------------
> >  1 file changed, 35 deletions(-)
> >
> > diff --git
> > a/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgen
> > t.c
> > b/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgen
> > t.c
> > index a1e61a6ef90e..62c6a235d425 100644
> > ---
> > a/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgen
> > t.c
> > +++
> > b/SourceLevelDebugPkg/Library/DebugAgent/DebugAgentCommon/DebugAgen
> > t
> > +++ .c
> > @@ -126,41 +126,6 @@ GLOBAL_REMOVE_IF_UNREFERENCED
> > EFI_VECTOR_HANDOFF_INFO  mVectorHandoffInfoDebugAg
> >
> >  GLOBAL_REMOVE_IF_UNREFERENCED UINTN  mVectorHandoffInfoCount =
> > sizeof (mVectorHandoffInfoDebugAgent) / sizeof
> > (EFI_VECTOR_HANDOFF_INFO);
> >
> > -/**
> > -  Calculate CRC16 for target data.
> > -
> > -  @param[in]  Data              The target data.
> > -  @param[in]  DataSize          The target data size.
> > -  @param[in]  Crc               Initial CRC.
> > -
> > -  @return UINT16     The CRC16 value.
> > -
> > -**/
> > -UINT16
> > -CalculateCrc16 (
> > -  IN UINT8   *Data,
> > -  IN UINTN   DataSize,
> > -  IN UINT16  Crc
> > -  )
> > -{
> > -  UINTN  Index;
> > -  UINTN  BitIndex;
> > -
> > -  for (Index = 0; Index < DataSize; Index++) {
> > -    Crc ^= (UINT16)Data[Index];
> > -    for (BitIndex = 0; BitIndex < 8; BitIndex++) {
> > -      if ((Crc & 0x8000) != 0) {
> > -        Crc <<= 1;
> > -        Crc  ^= 0x1021;
> > -      } else {
> > -        Crc <<= 1;
> > -      }
> > -    }
> > -  }
> > -
> > -  return Crc;
> > -}
> > -
> >  /**
> >    Read IDT entry to check if IDT entries are setup by Debug Agent.
> >
> > --
> > 2.35.1
>
>

-- 
Pedro Falcato

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-03-28  1:49 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-03-27  0:57 [PATCH 0/2] Add CRC16 and CRC32c to MdePkg/BaseLib Pedro Falcato
2022-03-27  0:57 ` [PATCH 1/2] MdePkg/BaseLib: Add CRC16 and CRC32c implementations Pedro Falcato
2022-03-27  0:57 ` [PATCH 2/2] SourceLevelDebugPkg/DebugAgent: Delete the CRC16 implementation Pedro Falcato
2022-03-28  0:14   ` Wu, Hao A
2022-03-28  1:49     ` Pedro Falcato

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox