Change in coreboot[master]: lib: add calculate crc byte by byte

[Xiang Wang (Code Review)]

Xiang Wang has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36624 )

Change subject: lib: add calculate crc byte by byte
......................................................................

lib: add calculate crc byte by byte

Change-Id: I5cab1f90452b08a464ad7a2d7e75d97187452992
Signed-off-by: Xiang Wang 
---
A src/include/crc_byte.h
M src/lib/Makefile.inc
A src/lib/crc_byte.c
3 files changed, 140 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/36624/1

diff --git a/src/include/crc_byte.h b/src/include/crc_byte.h
new file mode 100644
index 0000000..07e2aeb
--- /dev/null
+++ b/src/include/crc_byte.h
@@ -0,0 +1,36 @@
+/*
+ * This file is part of the coreboot project.
+ *
+ * Copyright (C) 2019 HardenedLinux
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef CRC_BYTE_H
+#define CRC_BYTE_H
+
+#include <stdint.h>
+
+/* This function is used to calculate crc7 byte by byte.
+ * prev_crc: old crc result
+ * data: new byte
+ * return value: new crc result
+ */
+uint8_t crc7_byte(uint8_t prev_crc, uint8_t data);
+
+/* This function is used to calculate crc16 byte by byte.
+ * prev_crc: old crc result
+ * data: new byte
+ * return value: new crc result
+ */
+uint16_t crc16_byte(uint16_t prev_crc, uint8_t data);
+
+
+#endif /* CRC_BYTE_H */
diff --git a/src/lib/Makefile.inc b/src/lib/Makefile.inc
index e5678ff..23468c9 100644
--- a/src/lib/Makefile.inc
+++ b/src/lib/Makefile.inc
@@ -249,6 +249,14 @@
 ramstage-y += string.c
 smm-y += string.c

+decompressor-y += crc_byte.c
+bootblock-y += crc_byte.c
+verstage-y += crc_byte.c
+romstage-y += crc_byte.c
+postcar-y += crc_byte.c
+ramstage-y += crc_byte.c
+smm-y += crc_byte.c
+
 postcar-y += bootmode.c
 postcar-y += boot_device.c
 postcar-y += cbfs.c
diff --git a/src/lib/crc_byte.c b/src/lib/crc_byte.c
new file mode 100644
index 0000000..9a89f55
--- /dev/null
+++ b/src/lib/crc_byte.c
@@ -0,0 +1,96 @@
+/*
+ * This file is part of the coreboot project.
+ *
+ * Copyright (C) 2019 HardenedLinux
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <crc_byte.h>
+
+static const uint8_t crc7_table[256] = {
+	0x00, 0x09, 0x12, 0x1b, 0x24, 0x2d, 0x36, 0x3f,
+	0x48, 0x41, 0x5a, 0x53, 0x6c, 0x65, 0x7e, 0x77,
+	0x19, 0x10, 0x0b, 0x02, 0x3d, 0x34, 0x2f, 0x26,
+	0x51, 0x58, 0x43, 0x4a, 0x75, 0x7c, 0x67, 0x6e,
+	0x32, 0x3b, 0x20, 0x29, 0x16, 0x1f, 0x04, 0x0d,
+	0x7a, 0x73, 0x68, 0x61, 0x5e, 0x57, 0x4c, 0x45,
+	0x2b, 0x22, 0x39, 0x30, 0x0f, 0x06, 0x1d, 0x14,
+	0x63, 0x6a, 0x71, 0x78, 0x47, 0x4e, 0x55, 0x5c,
+	0x64, 0x6d, 0x76, 0x7f, 0x40, 0x49, 0x52, 0x5b,
+	0x2c, 0x25, 0x3e, 0x37, 0x08, 0x01, 0x1a, 0x13,
+	0x7d, 0x74, 0x6f, 0x66, 0x59, 0x50, 0x4b, 0x42,
+	0x35, 0x3c, 0x27, 0x2e, 0x11, 0x18, 0x03, 0x0a,
+	0x56, 0x5f, 0x44, 0x4d, 0x72, 0x7b, 0x60, 0x69,
+	0x1e, 0x17, 0x0c, 0x05, 0x3a, 0x33, 0x28, 0x21,
+	0x4f, 0x46, 0x5d, 0x54, 0x6b, 0x62, 0x79, 0x70,
+	0x07, 0x0e, 0x15, 0x1c, 0x23, 0x2a, 0x31, 0x38,
+	0x41, 0x48, 0x53, 0x5a, 0x65, 0x6c, 0x77, 0x7e,
+	0x09, 0x00, 0x1b, 0x12, 0x2d, 0x24, 0x3f, 0x36,
+	0x58, 0x51, 0x4a, 0x43, 0x7c, 0x75, 0x6e, 0x67,
+	0x10, 0x19, 0x02, 0x0b, 0x34, 0x3d, 0x26, 0x2f,
+	0x73, 0x7a, 0x61, 0x68, 0x57, 0x5e, 0x45, 0x4c,
+	0x3b, 0x32, 0x29, 0x20, 0x1f, 0x16, 0x0d, 0x04,
+	0x6a, 0x63, 0x78, 0x71, 0x4e, 0x47, 0x5c, 0x55,
+	0x22, 0x2b, 0x30, 0x39, 0x06, 0x0f, 0x14, 0x1d,
+	0x25, 0x2c, 0x37, 0x3e, 0x01, 0x08, 0x13, 0x1a,
+	0x6d, 0x64, 0x7f, 0x76, 0x49, 0x40, 0x5b, 0x52,
+	0x3c, 0x35, 0x2e, 0x27, 0x18, 0x11, 0x0a, 0x03,
+	0x74, 0x7d, 0x66, 0x6f, 0x50, 0x59, 0x42, 0x4b,
+	0x17, 0x1e, 0x05, 0x0c, 0x33, 0x3a, 0x21, 0x28,
+	0x5f, 0x56, 0x4d, 0x44, 0x7b, 0x72, 0x69, 0x60,
+	0x0e, 0x07, 0x1c, 0x15, 0x2a, 0x23, 0x38, 0x31,
+	0x46, 0x4f, 0x54, 0x5d, 0x62, 0x6b, 0x70, 0x79
+};
+
+uint8_t crc7_byte(uint8_t crc, uint8_t data)
+{
+	return crc7_table[(crc << 1) ^ data];
+}
+
+static const uint16_t crc16_tab[] = {
+	0x0000, 0x1021, 0x2042, 0x3063, 0x4084, 0x50a5, 0x60c6, 0x70e7,
+	0x8108, 0x9129, 0xa14a, 0xb16b, 0xc18c, 0xd1ad, 0xe1ce, 0xf1ef,
+	0x1231, 0x0210, 0x3273, 0x2252, 0x52b5, 0x4294, 0x72f7, 0x62d6,
+	0x9339, 0x8318, 0xb37b, 0xa35a, 0xd3bd, 0xc39c, 0xf3ff, 0xe3de,
+	0x2462, 0x3443, 0x0420, 0x1401, 0x64e6, 0x74c7, 0x44a4, 0x5485,
+	0xa56a, 0xb54b, 0x8528, 0x9509, 0xe5ee, 0xf5cf, 0xc5ac, 0xd58d,
+	0x3653, 0x2672, 0x1611, 0x0630, 0x76d7, 0x66f6, 0x5695, 0x46b4,
+	0xb75b, 0xa77a, 0x9719, 0x8738, 0xf7df, 0xe7fe, 0xd79d, 0xc7bc,
+	0x48c4, 0x58e5, 0x6886, 0x78a7, 0x0840, 0x1861, 0x2802, 0x3823,
+	0xc9cc, 0xd9ed, 0xe98e, 0xf9af, 0x8948, 0x9969, 0xa90a, 0xb92b,
+	0x5af5, 0x4ad4, 0x7ab7, 0x6a96, 0x1a71, 0x0a50, 0x3a33, 0x2a12,
+	0xdbfd, 0xcbdc, 0xfbbf, 0xeb9e, 0x9b79, 0x8b58, 0xbb3b, 0xab1a,
+	0x6ca6, 0x7c87, 0x4ce4, 0x5cc5, 0x2c22, 0x3c03, 0x0c60, 0x1c41,
+	0xedae, 0xfd8f, 0xcdec, 0xddcd, 0xad2a, 0xbd0b, 0x8d68, 0x9d49,
+	0x7e97, 0x6eb6, 0x5ed5, 0x4ef4, 0x3e13, 0x2e32, 0x1e51, 0x0e70,
+	0xff9f, 0xefbe, 0xdfdd, 0xcffc, 0xbf1b, 0xaf3a, 0x9f59, 0x8f78,
+	0x9188, 0x81a9, 0xb1ca, 0xa1eb, 0xd10c, 0xc12d, 0xf14e, 0xe16f,
+	0x1080, 0x00a1, 0x30c2, 0x20e3, 0x5004, 0x4025, 0x7046, 0x6067,
+	0x83b9, 0x9398, 0xa3fb, 0xb3da, 0xc33d, 0xd31c, 0xe37f, 0xf35e,
+	0x02b1, 0x1290, 0x22f3, 0x32d2, 0x4235, 0x5214, 0x6277, 0x7256,
+	0xb5ea, 0xa5cb, 0x95a8, 0x8589, 0xf56e, 0xe54f, 0xd52c, 0xc50d,
+	0x34e2, 0x24c3, 0x14a0, 0x0481, 0x7466, 0x6447, 0x5424, 0x4405,
+	0xa7db, 0xb7fa, 0x8799, 0x97b8, 0xe75f, 0xf77e, 0xc71d, 0xd73c,
+	0x26d3, 0x36f2, 0x0691, 0x16b0, 0x6657, 0x7676, 0x4615, 0x5634,
+	0xd94c, 0xc96d, 0xf90e, 0xe92f, 0x99c8, 0x89e9, 0xb98a, 0xa9ab,
+	0x5844, 0x4865, 0x7806, 0x6827, 0x18c0, 0x08e1, 0x3882, 0x28a3,
+	0xcb7d, 0xdb5c, 0xeb3f, 0xfb1e, 0x8bf9, 0x9bd8, 0xabbb, 0xbb9a,
+	0x4a75, 0x5a54, 0x6a37, 0x7a16, 0x0af1, 0x1ad0, 0x2ab3, 0x3a92,
+	0xfd2e, 0xed0f, 0xdd6c, 0xcd4d, 0xbdaa, 0xad8b, 0x9de8, 0x8dc9,
+	0x7c26, 0x6c07, 0x5c64, 0x4c45, 0x3ca2, 0x2c83, 0x1ce0, 0x0cc1,
+	0xef1f, 0xff3e, 0xcf5d, 0xdf7c, 0xaf9b, 0xbfba, 0x8fd9, 0x9ff8,
+	0x6e17, 0x7e36, 0x4e55, 0x5e74, 0x2e93, 0x3eb2, 0x0ed1, 0x1ef0,
+};
+
+uint16_t crc16_byte(uint16_t prev, uint8_t in)
+{
+	return crc16_tab[((prev >> 8) ^ in) & 0xff] ^ (prev << 8);
+}

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36624 )

Change subject: lib: add calculate crc byte by byte
......................................................................

Patch Set 1:

(3 comments)

https://review.coreboot.org/c/coreboot/+/36624/1/src/include/crc_byte.h 
File src/include/crc_byte.h:

https://review.coreboot.org/c/coreboot/+/36624/1/src/include/crc_byte.h@33 
PS1, Line 33: uint16_t crc16_byte(uint16_t prev_crc, uint8_t data);
Please document the exact polynomials implemented by these functions, there exist multiple common versions of these CRCs (e.g. see https://en.wikipedia.org/wiki/Cyclic_redundancy_check).

https://review.coreboot.org/c/coreboot/+/36624/1/src/lib/crc_byte.c 
File src/lib/crc_byte.c:

https://review.coreboot.org/c/coreboot/+/36624/1/src/lib/crc_byte.c@18 
PS1, Line 18: static const uint8_t crc7_table[256] = {
Is there a reason these need to be table-based implementations? Can you just implement the CRC algorithm bit by bit (e.g. like in src/security/vboot/vbnv.c:crc8_vbnv())? In a firmware setting, the time you waste loading all these extra bytes from flash is almost certainly worse than the tiny speedup you may gain from it.

https://review.coreboot.org/c/coreboot/+/36624/1/src/lib/crc_byte.c@53 
PS1, Line 53: uint8_t crc7_byte(uint8_t crc, uint8_t data)
Is this really a CRC-7? Looks to me like it's consuming 8 bits at once, wouldn't that make it a CRC-8?

[Xiang Wang (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/36624

to look at the new patch set (#2).

Change subject: lib: add calculate crc byte by byte
......................................................................

lib: add calculate crc byte by byte

Change-Id: I5cab1f90452b08a464ad7a2d7e75d97187452992
Signed-off-by: Xiang Wang 
---
A src/include/crc_byte.h
M src/lib/Makefile.inc
A src/lib/crc_byte.c
3 files changed, 84 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/36624/2

[Xiang Wang (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Martin Roth,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/36624

to look at the new patch set (#3).

Change subject: lib: add calculate crc byte by byte
......................................................................

lib: add calculate crc byte by byte

Change-Id: I5cab1f90452b08a464ad7a2d7e75d97187452992
Signed-off-by: Xiang Wang 
---
A src/include/crc_byte.h
M src/lib/Makefile.inc
A src/lib/crc_byte.c
3 files changed, 86 insertions(+), 0 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/24/36624/3

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36624 )

Change subject: lib: add calculate crc byte by byte
......................................................................

Patch Set 3:

(3 comments)

https://review.coreboot.org/c/coreboot/+/36624/1/src/include/crc_byte.h 
File src/include/crc_byte.h:

https://review.coreboot.org/c/coreboot/+/36624/1/src/include/crc_byte.h@33 
PS1, Line 33: uint16_t crc16_byte(uint16_t prev_crc, uint8_t data);
> Please document the exact polynomials implemented by these functions, there exist multiple common ve […]

Done

https://review.coreboot.org/c/coreboot/+/36624/1/src/lib/crc_byte.c 
File src/lib/crc_byte.c:

https://review.coreboot.org/c/coreboot/+/36624/1/src/lib/crc_byte.c@18 
PS1, Line 18: static const uint8_t crc7_table[256] = {
> Is there a reason these need to be table-based implementations? Can you just implement the CRC algor […]

Done

https://review.coreboot.org/c/coreboot/+/36624/1/src/lib/crc_byte.c@53 
PS1, Line 53: uint8_t crc7_byte(uint8_t crc, uint8_t data)
> Is this really a CRC-7? Looks to me like it's consuming 8 bits at once, wouldn't that make it a CRC- […]

The naming convention for CRC-n isn't a word width (they all operate on bitstreams, even if they're bundled up like here). Instead, n is the highest exponent in the polynomial (here: x^7+x^3+1).

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36624 )

Change subject: lib: add calculate crc byte by byte
......................................................................

Patch Set 3: Code-Review+1

[Patrick Georgi (Code Review)]

Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36624 )

Change subject: lib: add calculate crc byte by byte
......................................................................

Patch Set 3: Code-Review+2

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/36624 )

Change subject: lib: add calculate crc byte by byte
......................................................................

lib: add calculate crc byte by byte

Change-Id: I5cab1f90452b08a464ad7a2d7e75d97187452992
Signed-off-by: Xiang Wang 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/36624
Tested-by: build bot (Jenkins) 
Reviewed-by: Julius Werner 
Reviewed-by: Patrick Georgi 
---
A src/include/crc_byte.h
M src/lib/Makefile.inc
A src/lib/crc_byte.c
3 files changed, 86 insertions(+), 0 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Patrick Georgi: Looks good to me, approved
  Julius Werner: Looks good to me, but someone else must approve

diff --git a/src/include/crc_byte.h b/src/include/crc_byte.h
new file mode 100644
index 0000000..9315277
--- /dev/null
+++ b/src/include/crc_byte.h
@@ -0,0 +1,40 @@
+/*
+ * This file is part of the coreboot project.
+ *
+ * Copyright (C) 2019 HardenedLinux
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef CRC_BYTE_H
+#define CRC_BYTE_H
+
+#include <stdint.h>
+
+/* This function is used to calculate crc7 byte by byte, with polynomial
+ * x^7 + x^3 + 1.
+ *
+ * prev_crc: old crc result (0 for first)
+ * data: new byte
+ * return value: new crc result
+ */
+uint8_t crc7_byte(uint8_t prev_crc, uint8_t data);
+
+/* This function is used to calculate crc16 byte by byte, with polynomial
+ * x^16 + x^12 + x^5 + 1.
+ *
+ * prev_crc: old crc result (0 for first)
+ * data: new byte
+ * return value: new crc result
+ */
+uint16_t crc16_byte(uint16_t prev_crc, uint8_t data);
+
+
+#endif /* CRC_BYTE_H */
diff --git a/src/lib/Makefile.inc b/src/lib/Makefile.inc
index da7b4bb..3b7d57c 100644
--- a/src/lib/Makefile.inc
+++ b/src/lib/Makefile.inc
@@ -249,6 +249,14 @@
 ramstage-y += string.c
 smm-y += string.c

+decompressor-y += crc_byte.c
+bootblock-y += crc_byte.c
+verstage-y += crc_byte.c
+romstage-y += crc_byte.c
+postcar-y += crc_byte.c
+ramstage-y += crc_byte.c
+smm-y += crc_byte.c
+
 postcar-y += bootmode.c
 postcar-y += boot_device.c
 postcar-y += cbfs.c
diff --git a/src/lib/crc_byte.c b/src/lib/crc_byte.c
new file mode 100644
index 0000000..0ac0063
--- /dev/null
+++ b/src/lib/crc_byte.c
@@ -0,0 +1,38 @@
+/*
+ * This file is part of the coreboot project.
+ *
+ * Copyright (C) 2019 HardenedLinux
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <crc_byte.h>
+
+uint8_t crc7_byte(uint8_t prev_crc, uint8_t data)
+{
+	const uint8_t g = 0x89;
+	prev_crc ^= data;
+	for (int i = 0; i < 8; i++) {
+		if (prev_crc & 0x80)
+			prev_crc ^= g;
+		prev_crc <<= 1;
+	}
+	return prev_crc;
+}
+
+uint16_t crc16_byte(uint16_t prev_crc, uint8_t data)
+{
+	prev_crc  = (uint8_t)(prev_crc >> 8)|(prev_crc << 8);
+	prev_crc ^=  data;
+	prev_crc ^= (uint8_t)(prev_crc & 0xff) >> 4;
+	prev_crc ^= (prev_crc << 8) << 4;
+	prev_crc ^= ((prev_crc & 0xff) << 4) << 1;
+	return prev_crc;
+}