coreboot-gerrit February 2024

coreboot-gerrit@coreboot.org

1 participants
3066 discussions

[XS] Change in coreboot[main]: util/cbfstool: Fallback to linear search
by Keith Short (Code Review) 02 Feb '24

02 Feb '24

Keith Short has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/80308?usp=email ) Change subject: util/cbfstool: Fallback to linear search ...................................................................... util/cbfstool: Fallback to linear search When the image size is a power of 2, cbfstool attempts to find the FMAP region using a binary search. However, if the FMAP location isn't aligned on a 16 byte boundary the binary search fails. Fall back to linear search if binary search fails. TEST=cbfstool ec.bin layout Change-Id: Ifa9f6f57d1c59114fd1d4ace2d82b62e953cf6a8 Signed-off-by: Keith Short <keithshort(a)chromium.org> --- M util/cbfstool/flashmap/fmap.c 1 file changed, 6 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/80308/1 diff --git a/util/cbfstool/flashmap/fmap.c b/util/cbfstool/flashmap/fmap.c index 46c31bb..cf36bbb 100644 --- a/util/cbfstool/flashmap/fmap.c +++ b/util/cbfstool/flashmap/fmap.c @@ -149,10 +149,13 @@ if ((image == NULL) || (image_len == 0)) return -1; - if (popcnt(image_len) == 1) + if (popcnt(image_len) == 1) { ret = fmap_bsearch(image, image_len); - else - ret = fmap_lsearch(image, image_len); + if (ret >= 0) + return ret; + } + + ret = fmap_lsearch(image, image_len); return ret; } -- To view, visit https://review.coreboot.org/c/coreboot/+/80308?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ifa9f6f57d1c59114fd1d4ace2d82b62e953cf6a8 Gerrit-Change-Number: 80308 Gerrit-PatchSet: 1 Gerrit-Owner: Keith Short <keithshort(a)chromium.org> Gerrit-MessageType: newchange

1 0

[M] Change in coreboot[main]: device/azalia: Rework the verb table
by Nicholas Sudsgaard (Code Review) 02 Feb '24

02 Feb '24

Hello build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/80333?usp=email to look at the new patch set (#3). The following approvals got outdated and were removed: Verified-1 by build bot (Jenkins) Change subject: device/azalia: Rework the verb table ...................................................................... device/azalia: Rework the verb table This is a very experimental change and I would appreciate anyone's opinion on this. :) My intentions are to make verb tables cleaner on the both the configuration and implementation side. On the configuration side (i.e. hda_verbs.c) this method removes most superfluous comments (e.g. /* Subsystem ID */) and makes it more "structured". On the implementation side I tried to make it easier to read (and hopefully maintain). For example, compare azalia_find_verb() and azalia_find_verb_table(). I also got rid of global variables (but introduced including c source files directly instead...). Change-Id: If8b672e4fd800b34e5ba39fad174fcf1154b0a54 Signed-off-by: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> --- M src/device/azalia_device.c M src/include/device/azalia_device.h A src/include/device/azalia_table_exporter.c M src/mainboard/hp/snb_ivb_laptops/variants/2560p/hda_verb.c 4 files changed, 114 insertions(+), 103 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/80333/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/80333?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: If8b672e4fd800b34e5ba39fad174fcf1154b0a54 Gerrit-Change-Number: 80333 Gerrit-PatchSet: 3 Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[main]: util/cbmem: Use commonlib ipchksum() algorithm
by Julius Werner (Code Review) 02 Feb '24

02 Feb '24

Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80256?usp=email ) ( 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: util/cbmem: Use commonlib ipchksum() algorithm ...................................................................... util/cbmem: Use commonlib ipchksum() algorithm This patch switches the cbmem utility from its own IP checksum implementation to the commonlib version (which is good because the old one had a couple of bugs: doesn't work on odd sizes and may overflow its carry accumulator with input larger than 64K). Change-Id: I0bef2c85c37ddd3438b7ac6389e9daa3e4955b31 Signed-off-by: Julius Werner <jwerner(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/80256 Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz> Reviewed-by: Yidi Lin <yidilin(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M util/cbmem/Makefile M util/cbmem/cbmem.c 2 files changed, 6 insertions(+), 23 deletions(-) Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved Yidi Lin: Looks good to me, approved diff --git a/util/cbmem/Makefile b/util/cbmem/Makefile index 9219a08..0c88aeb 100644 --- a/util/cbmem/Makefile +++ b/util/cbmem/Makefile @@ -4,6 +4,7 @@ PROGRAM = cbmem TOP ?= $(abspath ../..) ROOT = $(TOP)/src +COMMONLIB = $(ROOT)/commonlib CC ?= $(CROSS_COMPILE)gcc INSTALL ?= /usr/bin/env install PREFIX ?= /usr/local @@ -13,14 +14,14 @@ CPPFLAGS += -I . -I $(ROOT)/commonlib/include -I $(ROOT)/commonlib/bsd/include CPPFLAGS += -include $(ROOT)/commonlib/bsd/include/commonlib/bsd/compiler.h -OBJS = $(PROGRAM).o +OBJS = $(PROGRAM).o $(COMMONLIB)/bsd/ipchksum.o all: $(PROGRAM) $(PROGRAM): $(OBJS) clean: - rm -f $(PROGRAM) *.o .dependencies *~ junit.xml + rm -f $(PROGRAM) $(OBJS:.c=.o) .dependencies *~ junit.xml install: $(PROGRAM) $(INSTALL) -d $(DESTDIR)$(PREFIX)/sbin/ diff --git a/util/cbmem/cbmem.c b/util/cbmem/cbmem.c index d33b714..15b6770 100644 --- a/util/cbmem/cbmem.c +++ b/util/cbmem/cbmem.c @@ -19,6 +19,7 @@ #include <assert.h> #include <regex.h> #include <commonlib/bsd/cbmem_id.h> +#include <commonlib/bsd/ipchksum.h> #include <commonlib/bsd/tpm_log_defs.h> #include <commonlib/loglevel.h> #include <commonlib/timestamp_serialized.h> @@ -199,25 +200,6 @@ return dest; } -/* - * calculate ip checksum (16 bit quantities) on a passed in buffer. In case - * the buffer length is odd last byte is excluded from the calculation - */ -static u16 ipchcksum(const void *addr, unsigned size) -{ - const u16 *p = addr; - unsigned i, n = size / 2; /* don't expect odd sized blocks */ - u32 sum = 0; - - for (i = 0; i < n; i++) - sum += p[i]; - - sum = (sum >> 16) + (sum & 0xffff); - sum += (sum >> 16); - sum = ~sum & 0xffff; - return (u16) sum; -} - /* Find the first cbmem entry filling in the details. */ static int find_cbmem_entry(uint32_t id, uint64_t *addr, size_t *size) { @@ -400,7 +382,7 @@ lbh = buf + i; if (memcmp(lbh->signature, "LBIO", sizeof(lbh->signature)) || !lbh->header_bytes || - ipchcksum(lbh, sizeof(*lbh))) { + ipchksum(lbh, sizeof(*lbh))) { continue; } @@ -411,7 +393,7 @@ continue; } - if (ipchcksum(mapping_virt(&table_mapping), lbh->table_bytes) != + if (ipchksum(mapping_virt(&table_mapping), lbh->table_bytes) != lbh->table_checksum) { debug("Signature found, but wrong checksum.\n"); unmap_memory(&table_mapping); -- To view, visit https://review.coreboot.org/c/coreboot/+/80256?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I0bef2c85c37ddd3438b7ac6389e9daa3e4955b31 Gerrit-Change-Number: 80256 Gerrit-PatchSet: 3 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[main]: commonlib: Add assembly optimization for ipchksum() on x86
by Julius Werner (Code Review) 02 Feb '24

02 Feb '24

Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80255?usp=email ) ( 2 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: commonlib: Add assembly optimization for ipchksum() on x86 ...................................................................... commonlib: Add assembly optimization for ipchksum() on x86 This patch adds a bit of optimized assembly code to the ipchksum() algorithm for x86 targets in order to take advantage of larger load sizes and the add-with-carry instruction. The same assembly (with one minor manual tweak) works for both 32 and 64 bit mode (with most of the work being done by GCC which automatically inserts `rax` or `eax` in the inline assembly depending on the build target). Change-Id: I484620dc14679ff5ca02b2ced2f84650730a6efc Signed-off-by: Julius Werner <jwerner(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/80255 Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/commonlib/bsd/ipchksum.c 1 file changed, 24 insertions(+), 1 deletion(-) Approvals: Arthur Heymans: Looks good to me, approved build bot (Jenkins): Verified diff --git a/src/commonlib/bsd/ipchksum.c b/src/commonlib/bsd/ipchksum.c index 89d261f..b7434e5 100644 --- a/src/commonlib/bsd/ipchksum.c +++ b/src/commonlib/bsd/ipchksum.c @@ -34,7 +34,30 @@ :: "cc" ); } -#endif +#elif defined(__i386__) || defined(__x86_64__) + size_t size8 = size / 8; + const uint64_t *p8 = data; + i = size8 * 8; + asm ( + "clc\n\t" + "1:\n\t" + "jecxz 2f\n\t" /* technically RCX on 64, but not gonna be that big */ + "adc (%[p8]), %[wsum]\n\t" +#if defined(__i386__) + "adc 4(%[p8]), %[wsum]\n\t" +#endif /* __i386__ */ + "lea -1(%[size8]), %[size8]\n\t" /* Use LEA as a makeshift ADD that */ + "lea 8(%[p8]), %[p8]\n\t" /* doesn't modify the carry flag. */ + "jmp 1b\n\t" + "2:\n\t" + "setc %b[size8]\n\t" /* reuse size register to save last carry */ + "add %[size8], %[wsum]\n\t" + : [wsum] "+r" (wide_sum), + [p8] "+r" (p8), + [size8] "+c" (size8) /* put size in ECX so we can JECXZ */ + :: "cc" + ); +#endif /* __i386__ || __x86_64__ */ while (wide_sum) { sum += wide_sum & 0xFFFF; -- To view, visit https://review.coreboot.org/c/coreboot/+/80255?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I484620dc14679ff5ca02b2ced2f84650730a6efc Gerrit-Change-Number: 80255 Gerrit-PatchSet: 4 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[main]: commonlib: Add assembly optimization for ipchksum() on arm64
by Julius Werner (Code Review) 02 Feb '24

02 Feb '24

Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80254?usp=email ) ( 2 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: commonlib: Add assembly optimization for ipchksum() on arm64 ...................................................................... commonlib: Add assembly optimization for ipchksum() on arm64 This patch adds a bit of optimized assembly code to the ipchksum() algorithm for arm64 targets in order to take advantage of larger load sizes and the add-with-carry instruction. This improves execution speed on a Cortex-A75 by more than 20x. Change-Id: I9c7bbc9d7a1cd083ced62fe9222592243a796077 Signed-off-by: Julius Werner <jwerner(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/80254 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz> Reviewed-by: Yidi Lin <yidilin(a)google.com> --- M src/commonlib/bsd/ipchksum.c 1 file changed, 25 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved Yidi Lin: Looks good to me, approved diff --git a/src/commonlib/bsd/ipchksum.c b/src/commonlib/bsd/ipchksum.c index a40b86c..89d261f 100644 --- a/src/commonlib/bsd/ipchksum.c +++ b/src/commonlib/bsd/ipchksum.c @@ -11,6 +11,31 @@ uint32_t sum = 0; size_t i = 0; +#if defined(__aarch64__) + size_t size16 = size / 16; + const uint64_t *p8 = data; + if (size16) { + unsigned long tmp1, tmp2; + i = size16 * 16; + asm ( + "adds xzr, xzr, xzr\n\t" /* init carry flag for addition */ + "1:\n\t" + "ldp %[v1], %[v2], [%[p8]], #16\n\t" + "adcs %[wsum], %[wsum], %[v1]\n\t" + "adcs %[wsum], %[wsum], %[v2]\n\t" + "sub %[size16], %[size16], #1\n\t" + "cbnz %[size16], 1b\n\t" + "adcs %[wsum], %[wsum], xzr\n\t" /* use up last carry */ + : [v1] "=r" (tmp1), + [v2] "=r" (tmp2), + [wsum] "+r" (wide_sum), + [p8] "+r" (p8), + [size16] "+r" (size16) + :: "cc" + ); + } +#endif + while (wide_sum) { sum += wide_sum & 0xFFFF; wide_sum >>= 16; -- To view, visit https://review.coreboot.org/c/coreboot/+/80254?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I9c7bbc9d7a1cd083ced62fe9222592243a796077 Gerrit-Change-Number: 80254 Gerrit-PatchSet: 4 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

[M] Change in coreboot[main]: libpayload: Switch to commonlib ipchksum() algorithm
by Julius Werner (Code Review) 02 Feb '24

02 Feb '24

Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80253?usp=email ) ( 2 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: libpayload: Switch to commonlib ipchksum() algorithm ...................................................................... libpayload: Switch to commonlib ipchksum() algorithm This patch moves libpayload over to the commonlib implementation for calculating the IP checksum. Change-Id: Ie8d323ce9f8d946758619761b4b22d54bce222b6 Signed-off-by: Julius Werner <jwerner(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/80253 Reviewed-by: Jakub Czapiga <czapiga(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz> Reviewed-by: Yidi Lin <yidilin(a)google.com> --- M payloads/libpayload/include/coreboot_tables.h D payloads/libpayload/include/ipchksum.h M payloads/libpayload/include/libpayload.h M payloads/libpayload/libc/Makefile.mk D payloads/libpayload/libc/ipchecksum.c 5 files changed, 4 insertions(+), 94 deletions(-) Approvals: Yidi Lin: Looks good to me, approved Arthur Heymans: Looks good to me, approved Jakub Czapiga: Looks good to me, approved build bot (Jenkins): Verified diff --git a/payloads/libpayload/include/coreboot_tables.h b/payloads/libpayload/include/coreboot_tables.h index 5c3f0c4..5b19ca8 100644 --- a/payloads/libpayload/include/coreboot_tables.h +++ b/payloads/libpayload/include/coreboot_tables.h @@ -30,7 +30,7 @@ #define _COREBOOT_TABLES_H #include <arch/types.h> -#include <ipchksum.h> +#include <commonlib/bsd/ipchksum.h> #include <stdint.h> enum { diff --git a/payloads/libpayload/include/ipchksum.h b/payloads/libpayload/include/ipchksum.h deleted file mode 100644 index 391fefb..0000000 --- a/payloads/libpayload/include/ipchksum.h +++ /dev/null @@ -1,39 +0,0 @@ -/* - * - * Copyright (c) 2012 The ChromiumOS Authors. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: - * 1. Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in the - * documentation and/or other materials provided with the distribution. - * 3. The name of the author may not be used to endorse or promote products - * derived from this software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND - * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE - * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL - * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS - * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) - * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT - * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY - * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF - * SUCH DAMAGE. - */ - -#ifndef __IPCHKSUM_H__ -#define __IPCHKSUM_H__ - -/** - * @defgroup ipchecksum IP checksum functions - * @{ - */ -unsigned short ipchksum(const void *ptr, unsigned long nbytes); -/** @} */ - -#endif diff --git a/payloads/libpayload/include/libpayload.h b/payloads/libpayload/include/libpayload.h index 06c6de4..6da4564 100644 --- a/payloads/libpayload/include/libpayload.h +++ b/payloads/libpayload/include/libpayload.h @@ -48,12 +48,12 @@ #include <commonlib/bsd/elog.h> #include <commonlib/bsd/fmap_serialized.h> #include <commonlib/bsd/helpers.h> +#include <commonlib/bsd/ipchksum.h> #include <commonlib/bsd/mem_chip_info.h> #include <ctype.h> #include <die.h> #include <endian.h> #include <fmap.h> -#include <ipchksum.h> #include <kconfig.h> #include <stddef.h> #include <stdio.h> diff --git a/payloads/libpayload/libc/Makefile.mk b/payloads/libpayload/libc/Makefile.mk index 2d277da..306bebf 100644 --- a/payloads/libpayload/libc/Makefile.mk +++ b/payloads/libpayload/libc/Makefile.mk @@ -28,7 +28,7 @@ ## libc-$(CONFIG_LP_LIBC) += malloc.c printf.c console.c string.c -libc-$(CONFIG_LP_LIBC) += memory.c ctype.c ipchecksum.c lib.c libgcc.c +libc-$(CONFIG_LP_LIBC) += memory.c ctype.c lib.c libgcc.c libc-$(CONFIG_LP_LIBC) += rand.c time.c exec.c libc-$(CONFIG_LP_LIBC) += readline.c getopt_long.c sysinfo.c libc-$(CONFIG_LP_LIBC) += args.c @@ -47,4 +47,5 @@ ifeq ($(CONFIG_LP_LIBC),y) libc-srcs += $(coreboottop)/src/commonlib/bsd/elog.c libc-srcs += $(coreboottop)/src/commonlib/bsd/gcd.c +libc-srcs += $(coreboottop)/src/commonlib/bsd/ipchksum.c endif diff --git a/payloads/libpayload/libc/ipchecksum.c b/payloads/libpayload/libc/ipchecksum.c deleted file mode 100644 index 118a694..0000000 --- a/payloads/libpayload/libc/ipchecksum.c +++ /dev/null @@ -1,52 +0,0 @@ -/* - * - * It has originally been taken from the FreeBSD project. - * - * Copyright (c) 2001 Charles Mott <cm(a)linktel.net> - * Copyright (c) 2008 coresystems GmbH - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions - * are met: - * 1. Redistributions of source code must retain the above copyright - * notice, this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright - * notice, this list of conditions and the following disclaimer in the - * documentation and/or other materials provided with the distribution. - * - * THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND - * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE - * ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE - * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL - * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS - * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) - * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT - * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY - * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF - * SUCH DAMAGE. - */ - -#include <libpayload.h> - -unsigned short ipchksum(const void *vptr, unsigned long nbytes) -{ - int sum, oddbyte; - const unsigned short *ptr = vptr; - - sum = 0; - while (nbytes > 1) { - sum += *ptr++; - nbytes -= 2; - } - if (nbytes == 1) { - oddbyte = 0; - ((u8 *) & oddbyte)[0] = *(u8 *) ptr; - ((u8 *) & oddbyte)[1] = 0; - sum += oddbyte; - } - sum = (sum >> 16) + (sum & 0xffff); - sum += (sum >> 16); - return (~sum); -} -- To view, visit https://review.coreboot.org/c/coreboot/+/80253?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ie8d323ce9f8d946758619761b4b22d54bce222b6 Gerrit-Change-Number: 80253 Gerrit-PatchSet: 4 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[main]: tests: Add some more ipchksum() test cases
by Julius Werner (Code Review) 02 Feb '24

02 Feb '24

Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80252?usp=email ) ( 2 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: tests: Add some more ipchksum() test cases ...................................................................... tests: Add some more ipchksum() test cases This patch adds a few more test cases for the IP checksum algorithm to catch more possible corner cases (large data with more than 64K carries, unaligned data, checksum addition with offset, etc.). Change-Id: I39b4d3f1bb833894985649872329eec88a02a22c Signed-off-by: Julius Werner <jwerner(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/80252 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Arthur Heymans <arthur(a)aheymans.xyz> Reviewed-by: Jakub Czapiga <czapiga(a)google.com> --- M tests/commonlib/bsd/ipchksum-test.c 1 file changed, 30 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved Jakub Czapiga: Looks good to me, approved diff --git a/tests/commonlib/bsd/ipchksum-test.c b/tests/commonlib/bsd/ipchksum-test.c index 2aed94c..3231e21 100644 --- a/tests/commonlib/bsd/ipchksum-test.c +++ b/tests/commonlib/bsd/ipchksum-test.c @@ -63,6 +63,23 @@ free(helper_buffer); } +static void test_ipchksum_80kff(void **state) +{ + /* 64K is an important boundary since naive 32-bit sum implementations that accumulate + carries may run over after that point. */ + size_t buffer_sz = 80 * 1024; + char *buffer = malloc(buffer_sz); + + memset(buffer, 0xff, buffer_sz); + assert_int_equal(ipchksum(buffer, buffer_sz), 0); + + /* Make things a bit more interesting... */ + memcpy(buffer + 0x6789, test_data_simple, test_data_simple_sz); + assert_int_equal(ipchksum(buffer, buffer_sz), 0x6742); + + free(buffer); +} + static void test_ipchksum_add_empty_values(void **state) { uint16_t res; @@ -81,7 +98,19 @@ test_data_simple_sz / 2); uint16_t res_sum = ipchksum_add(test_data_simple_sz / 2, res_1, res_2); + assert_int_equal(0xb62e, res_1); + assert_int_equal(0x8c38, res_2); assert_int_equal(test_data_simple_checksum, res_sum); + + /* Test some unaligned sums */ + res_1 = ipchksum(test_data_simple, 17); + res_2 = ipchksum(test_data_simple + 17, test_data_simple_sz - 17); + res_sum = ipchksum_add(17, res_1, res_2); + + assert_int_equal(0x2198, res_1); + assert_int_equal(0xcf20, res_2); + assert_int_equal(test_data_simple_checksum, res_sum); + } int main(void) @@ -90,6 +119,7 @@ cmocka_unit_test(test_ipchksum_zero_length), cmocka_unit_test(test_ipchksum_zero_buffer), cmocka_unit_test(test_ipchksum_simple_data), + cmocka_unit_test(test_ipchksum_80kff), cmocka_unit_test(test_ipchksum_add_empty_values), cmocka_unit_test(test_ipchksum_add), -- To view, visit https://review.coreboot.org/c/coreboot/+/80252?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I39b4d3f1bb833894985649872329eec88a02a22c Gerrit-Change-Number: 80252 Gerrit-PatchSet: 4 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[L] Change in coreboot[main]: lib: Move IP checksum to commonlib
by Julius Werner (Code Review) 02 Feb '24

02 Feb '24

Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80251?usp=email ) Change subject: lib: Move IP checksum to commonlib ...................................................................... lib: Move IP checksum to commonlib This patch moves the IP checksum algorithm into commonlib to prepare for it being shared with libpayload. The current implementation is ancient and pretty hard to read (and does some unnecessary questionable things like the type-punning stuff which leads to suboptimal code generation), so this reimplements it from scratch (that also helps with the licensing). This algorithm is prepared to take in a pre-calculated "wide" checksum in a machine-register-sized data type which is then narrowed down to 16 bits (see RFC 1071 for why that's valid). This isn't used yet (and the code will get optimized out), but will be used later in this patch series for architecture-specific optimization. Change-Id: Ic04c714c00439a17fc04a8a6e730cc2aa19b8e68 Signed-off-by: Julius Werner <jwerner(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/80251 Reviewed-by: Yidi Lin <yidilin(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Jakub Czapiga <czapiga(a)google.com> --- M src/arch/x86/boot.c M src/commonlib/Makefile.mk A src/commonlib/bsd/include/commonlib/bsd/ipchksum.h A src/commonlib/bsd/ipchksum.c M src/drivers/elog/boot_count.c M src/drivers/intel/fsp1_1/hob.c M src/drivers/net/ne2k.c D src/include/ip_checksum.h M src/lib/Makefile.mk D src/lib/compute_ip_checksum.c M src/lib/coreboot_table.c M src/northbridge/intel/haswell/broadwell_mrc/raminit.c M src/northbridge/intel/haswell/haswell_mrc/raminit.c M src/northbridge/intel/ironlake/raminit.c M src/northbridge/intel/sandybridge/raminit_mrc.c M src/soc/intel/common/basecode/ramtop/ramtop.c M src/soc/intel/common/block/cse/cse_lite_cmos.c M src/soc/mediatek/common/memory.c M src/soc/mediatek/mt8183/memory.c M src/southbridge/intel/bd82x6x/early_pch.c M tests/commonlib/bsd/Makefile.mk A tests/commonlib/bsd/ipchksum-test.c M tests/lib/Makefile.mk D tests/lib/compute_ip_checksum-test.c 24 files changed, 193 insertions(+), 202 deletions(-) Approvals: build bot (Jenkins): Verified Yidi Lin: Looks good to me, approved Jakub Czapiga: Looks good to me, approved diff --git a/src/arch/x86/boot.c b/src/arch/x86/boot.c index c50ec0e..90af84f 100644 --- a/src/arch/x86/boot.c +++ b/src/arch/x86/boot.c @@ -5,7 +5,6 @@ #include <commonlib/helpers.h> #include <console/console.h> #include <program_loading.h> -#include <ip_checksum.h> #include <symbols.h> #include <assert.h> diff --git a/src/commonlib/Makefile.mk b/src/commonlib/Makefile.mk index 70e731d..7ec4de9 100644 --- a/src/commonlib/Makefile.mk +++ b/src/commonlib/Makefile.mk @@ -61,3 +61,5 @@ decompressor-y += bsd/gcd.c all-y += bsd/gcd.c + +all-y += bsd/ipchksum.c diff --git a/src/commonlib/bsd/include/commonlib/bsd/ipchksum.h b/src/commonlib/bsd/include/commonlib/bsd/ipchksum.h new file mode 100644 index 0000000..91d6872 --- /dev/null +++ b/src/commonlib/bsd/include/commonlib/bsd/ipchksum.h @@ -0,0 +1,12 @@ +/* SPDX-License-Identifier: BSD-3-Clause */ + +#ifndef _COMMONLIB_BSD_IPCHKSUM_H_ +#define _COMMONLIB_BSD_IPCHKSUM_H_ + +#include <stddef.h> +#include <stdint.h> + +uint16_t ipchksum(const void *data, size_t size); +uint16_t ipchksum_add(size_t offset, uint16_t first, uint16_t second); + +#endif /* _COMMONLIB_BSD_IPCHKSUM_H_ */ diff --git a/src/commonlib/bsd/ipchksum.c b/src/commonlib/bsd/ipchksum.c new file mode 100644 index 0000000..a40b86c --- /dev/null +++ b/src/commonlib/bsd/ipchksum.c @@ -0,0 +1,52 @@ +/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0-or-later */ + +#include <commonlib/bsd/ipchksum.h> + +/* See RFC 1071 for mathematical explanations of why we can first sum in a larger register and + then narrow down, why we don't need to worry about endianness, etc. */ +uint16_t ipchksum(const void *data, size_t size) +{ + const uint8_t *p1 = data; + unsigned long wide_sum = 0; + uint32_t sum = 0; + size_t i = 0; + + while (wide_sum) { + sum += wide_sum & 0xFFFF; + wide_sum >>= 16; + } + sum = (sum & 0xFFFF) + (sum >> 16); + + for (; i < size; i++) { + uint32_t v = p1[i]; + if (i % 2) + v <<= 8; + sum += v; + + /* Doing this unconditionally seems to be faster. */ + sum = (sum & 0xFFFF) + (sum >> 16); + } + + return (uint16_t)~sum; +} + +uint16_t ipchksum_add(size_t offset, uint16_t first, uint16_t second) +{ + first = ~first; + second = ~second; + + /* + * Since the checksum is calculated in 16-bit chunks, if the offset at which + * the data covered by the second checksum would start (if both data streams + * came one after the other) is odd, that means the second stream starts in + * the middle of a 16-bit chunk. This means the second checksum is byte + * swapped compared to what we need it to be, and we must swap it back. + */ + if (offset % 2) + second = (second >> 8) | (second << 8); + + uint32_t sum = first + second; + sum = (sum & 0xFFFF) + (sum >> 16); + + return (uint16_t)~sum; +} diff --git a/src/drivers/elog/boot_count.c b/src/drivers/elog/boot_count.c index 17d928a..9d618f9 100644 --- a/src/drivers/elog/boot_count.c +++ b/src/drivers/elog/boot_count.c @@ -1,7 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-only */ +#include <commonlib/bsd/ipchksum.h> #include <console/console.h> -#include <ip_checksum.h> #include <pc80/mc146818rtc.h> #include <stdint.h> #include <elog.h> @@ -47,7 +47,7 @@ } /* Verify checksum over signature and counter only */ - csum = compute_ip_checksum(bc, offsetof(struct boot_count, checksum)); + csum = ipchksum(bc, offsetof(struct boot_count, checksum)); if (csum != bc->checksum) { printk(BIOS_DEBUG, "Boot Count checksum mismatch\n"); @@ -63,7 +63,7 @@ u8 i, *p; /* Checksum over signature and counter only */ - bc->checksum = compute_ip_checksum( + bc->checksum = ipchksum( bc, offsetof(struct boot_count, checksum)); for (p = (u8 *)bc, i = 0; i < sizeof(*bc); i++, p++) diff --git a/src/drivers/intel/fsp1_1/hob.c b/src/drivers/intel/fsp1_1/hob.c index e7c5fb4..7522df1 100644 --- a/src/drivers/intel/fsp1_1/hob.c +++ b/src/drivers/intel/fsp1_1/hob.c @@ -3,7 +3,6 @@ #include <arch/hlt.h> #include <console/console.h> #include <fsp/util.h> -#include <ip_checksum.h> #include <string.h> /* Compares two EFI GUIDs. Returns true of the GUIDs match, false otherwise. */ diff --git a/src/drivers/net/ne2k.c b/src/drivers/net/ne2k.c index 4479a76..70ef0ef 100644 --- a/src/drivers/net/ne2k.c +++ b/src/drivers/net/ne2k.c @@ -28,11 +28,11 @@ */ #include <arch/io.h> +#include <commonlib/bsd/ipchksum.h> #include <console/ne2k.h> #include <device/device.h> #include <device/pci.h> #include <device/pci_ops.h> -#include <ip_checksum.h> #include "ns8390.h" @@ -184,7 +184,7 @@ hdr[38] = (8 + pktlen) >> 8; hdr[39] = 8 + pktlen; - chksum = compute_ip_checksum(&hdr[14], 20); + chksum = ipchksum(&hdr[14], 20); hdr[25] = chksum >> 8; hdr[24] = chksum; diff --git a/src/include/ip_checksum.h b/src/include/ip_checksum.h deleted file mode 100644 index 74a2f48..0000000 --- a/src/include/ip_checksum.h +++ /dev/null @@ -1,8 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#ifndef IP_CHECKSUM_H -#define IP_CHECKSUM_H -unsigned long compute_ip_checksum(const void *addr, unsigned long length); -unsigned long add_ip_checksums(unsigned long offset, unsigned long sum, - unsigned long new); -#endif /* IP_CHECKSUM_H */ diff --git a/src/lib/Makefile.mk b/src/lib/Makefile.mk index fd3f464..2a95be9 100644 --- a/src/lib/Makefile.mk +++ b/src/lib/Makefile.mk @@ -116,8 +116,6 @@ romstage-$(CONFIG_COLLECT_TIMESTAMPS) += timestamp.c romstage-$(CONFIG_CONSOLE_CBMEM) += cbmem_console.c -bootblock-y += compute_ip_checksum.c -romstage-y += compute_ip_checksum.c romstage-y += dimm_info_util.c ifeq ($(CONFIG_COMPILER_GCC),y) bootblock-$(CONFIG_ARCH_BOOTBLOCK_X86_32) += gcc.c @@ -145,7 +143,6 @@ ramstage-y += dimm_info_util.c ramstage-y += delay.c ramstage-y += fallback_boot.c -ramstage-y += compute_ip_checksum.c ramstage-y += cbfs.c ramstage-y += lzma.c lzmadecode.c ramstage-y += stack.c diff --git a/src/lib/compute_ip_checksum.c b/src/lib/compute_ip_checksum.c deleted file mode 100644 index 913f25c..0000000 --- a/src/lib/compute_ip_checksum.c +++ /dev/null @@ -1,53 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#include <stdint.h> -#include <ip_checksum.h> - -unsigned long compute_ip_checksum(const void *addr, unsigned long length) -{ - const uint8_t *ptr; - volatile union { - uint8_t byte[2]; - uint16_t word; - } value; - unsigned long sum; - unsigned long i; - /* In the most straight forward way possible, - * compute an ip style checksum. - */ - sum = 0; - ptr = addr; - for (i = 0; i < length; i++) { - unsigned long v; - v = ptr[i]; - if (i & 1) - v <<= 8; - /* Add the new value */ - sum += v; - /* Wrap around the carry */ - if (sum > 0xFFFF) - sum = (sum + (sum >> 16)) & 0xFFFF; - } - value.byte[0] = sum & 0xff; - value.byte[1] = (sum >> 8) & 0xff; - return (~value.word) & 0xFFFF; -} - -unsigned long add_ip_checksums(unsigned long offset, unsigned long sum, - unsigned long new) -{ - unsigned long checksum; - sum = ~sum & 0xFFFF; - new = ~new & 0xFFFF; - if (offset & 1) { - /* byte swap the sum if it came from an odd offset - * since the computation is endian independent this - * works. - */ - new = ((new >> 8) & 0xff) | ((new << 8) & 0xff00); - } - checksum = sum + new; - if (checksum > 0xFFFF) - checksum -= 0xFFFF; - return (~checksum) & 0xFFFF; -} diff --git a/src/lib/coreboot_table.c b/src/lib/coreboot_table.c index 800d2d4..d93ba01 100644 --- a/src/lib/coreboot_table.c +++ b/src/lib/coreboot_table.c @@ -2,10 +2,10 @@ #include <acpi/acpi.h> #include <arch/cbconfig.h> +#include <commonlib/bsd/ipchksum.h> #include <console/console.h> #include <console/uart.h> #include <identity.h> -#include <ip_checksum.h> #include <boot/coreboot_tables.h> #include <boot/tables.h> #include <boot_device.h> @@ -432,10 +432,9 @@ } first_rec = lb_first_record(head); - head->table_checksum = compute_ip_checksum(first_rec, - head->table_bytes); + head->table_checksum = ipchksum(first_rec, head->table_bytes); head->header_checksum = 0; - head->header_checksum = compute_ip_checksum(head, sizeof(*head)); + head->header_checksum = ipchksum(head, sizeof(*head)); printk(BIOS_DEBUG, "Wrote coreboot table at: %p, 0x%x bytes, checksum %x\n", head, head->table_bytes, head->table_checksum); diff --git a/src/northbridge/intel/haswell/broadwell_mrc/raminit.c b/src/northbridge/intel/haswell/broadwell_mrc/raminit.c index c00e59a..b8a9416 100644 --- a/src/northbridge/intel/haswell/broadwell_mrc/raminit.c +++ b/src/northbridge/intel/haswell/broadwell_mrc/raminit.c @@ -8,7 +8,6 @@ #include <cbmem.h> #include <cbfs.h> #include <cf9_reset.h> -#include <ip_checksum.h> #include <memory_info.h> #include <mrc_cache.h> #include <device/device.h> diff --git a/src/northbridge/intel/haswell/haswell_mrc/raminit.c b/src/northbridge/intel/haswell/haswell_mrc/raminit.c index bf072fa..f23e40d 100644 --- a/src/northbridge/intel/haswell/haswell_mrc/raminit.c +++ b/src/northbridge/intel/haswell/haswell_mrc/raminit.c @@ -7,7 +7,6 @@ #include <cbmem.h> #include <cbfs.h> #include <cf9_reset.h> -#include <ip_checksum.h> #include <memory_info.h> #include <mrc_cache.h> #include <device/device.h> diff --git a/src/northbridge/intel/ironlake/raminit.c b/src/northbridge/intel/ironlake/raminit.c index 8a934ed..b84461a 100644 --- a/src/northbridge/intel/ironlake/raminit.c +++ b/src/northbridge/intel/ironlake/raminit.c @@ -11,7 +11,6 @@ #include <cpu/x86/cache.h> #include <cbmem.h> #include <cf9_reset.h> -#include <ip_checksum.h> #include <option.h> #include <device/pci_def.h> #include <device/device.h> diff --git a/src/northbridge/intel/sandybridge/raminit_mrc.c b/src/northbridge/intel/sandybridge/raminit_mrc.c index dde5742..82e3e82 100644 --- a/src/northbridge/intel/sandybridge/raminit_mrc.c +++ b/src/northbridge/intel/sandybridge/raminit_mrc.c @@ -11,7 +11,7 @@ #include <arch/cpu.h> #include <cbmem.h> #include <cbfs.h> -#include <ip_checksum.h> +#include <commonlib/bsd/ipchksum.h> #include <pc80/mc146818rtc.h> #include <device/pci_def.h> #include <lib.h> @@ -73,9 +73,9 @@ pei_data->scrambler_seed_s3, CMOS_OFFSET_MRC_SEED_S3); /* Save a simple checksum of the seed values */ - c1 = compute_ip_checksum((u8 *)&pei_data->scrambler_seed, sizeof(u32)); - c2 = compute_ip_checksum((u8 *)&pei_data->scrambler_seed_s3, sizeof(u32)); - checksum = add_ip_checksums(sizeof(u32), c1, c2); + c1 = ipchksum((u8 *)&pei_data->scrambler_seed, sizeof(u32)); + c2 = ipchksum((u8 *)&pei_data->scrambler_seed_s3, sizeof(u32)); + checksum = ipchksum_add(sizeof(u32), c1, c2); cmos_write((checksum >> 0) & 0xff, CMOS_OFFSET_MRC_SEED_CHK); cmos_write((checksum >> 8) & 0xff, CMOS_OFFSET_MRC_SEED_CHK + 1); @@ -100,9 +100,9 @@ pei_data->scrambler_seed_s3, CMOS_OFFSET_MRC_SEED_S3); /* Compute seed checksum and compare */ - c1 = compute_ip_checksum((u8 *)&pei_data->scrambler_seed, sizeof(u32)); - c2 = compute_ip_checksum((u8 *)&pei_data->scrambler_seed_s3, sizeof(u32)); - checksum = add_ip_checksums(sizeof(u32), c1, c2); + c1 = ipchksum((u8 *)&pei_data->scrambler_seed, sizeof(u32)); + c2 = ipchksum((u8 *)&pei_data->scrambler_seed_s3, sizeof(u32)); + checksum = ipchksum_add(sizeof(u32), c1, c2); seed_checksum = cmos_read(CMOS_OFFSET_MRC_SEED_CHK); seed_checksum |= cmos_read(CMOS_OFFSET_MRC_SEED_CHK + 1) << 8; diff --git a/src/soc/intel/common/basecode/ramtop/ramtop.c b/src/soc/intel/common/basecode/ramtop/ramtop.c index 90717e0..ec326bb 100644 --- a/src/soc/intel/common/basecode/ramtop/ramtop.c +++ b/src/soc/intel/common/basecode/ramtop/ramtop.c @@ -1,8 +1,8 @@ /* SPDX-License-Identifier: GPL-2.0-or-later */ +#include <commonlib/bsd/ipchksum.h> #include <console/console.h> #include <cpu/x86/mtrr.h> -#include <ip_checksum.h> #include <intelbasecode/ramtop.h> #include <pc80/mc146818rtc.h> #include <stdint.h> @@ -57,7 +57,7 @@ } /* Verify checksum over signature and counter only */ - csum = compute_ip_checksum(ramtop, offsetof(struct ramtop_table, checksum)); + csum = ipchksum(ramtop, offsetof(struct ramtop_table, checksum)); if (csum != ramtop->checksum) { printk(BIOS_DEBUG, "ramtop_table checksum mismatch\n"); @@ -73,8 +73,7 @@ u8 i, *p; /* Checksum over signature and counter only */ - ramtop->checksum = compute_ip_checksum( - ramtop, offsetof(struct ramtop_table, checksum)); + ramtop->checksum = ipchksum(ramtop, offsetof(struct ramtop_table, checksum)); for (p = (u8 *)ramtop, i = 0; i < sizeof(*ramtop); i++, p++) cmos_write(*p, (CMOS_VSTART_ramtop / 8) + i); diff --git a/src/soc/intel/common/block/cse/cse_lite_cmos.c b/src/soc/intel/common/block/cse/cse_lite_cmos.c index 02d0232..5c26c01 100644 --- a/src/soc/intel/common/block/cse/cse_lite_cmos.c +++ b/src/soc/intel/common/block/cse/cse_lite_cmos.c @@ -1,7 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-only */ +#include <commonlib/bsd/ipchksum.h> #include <console/console.h> -#include <ip_checksum.h> #include <pc80/mc146818rtc.h> #include "cse_lite_cmos.h" @@ -66,7 +66,7 @@ } /* Verify checksum over signature and backup_status only */ - uint16_t csum = compute_ip_checksum(psr, offsetof(struct psr_backup_status, checksum)); + uint16_t csum = ipchksum(psr, offsetof(struct psr_backup_status, checksum)); if (csum != psr->checksum) { printk(BIOS_ERR, "PSR backup status checksum mismatch\n"); @@ -80,8 +80,7 @@ static void psr_backup_status_cmos_write(struct psr_backup_status *psr) { /* Checksum over signature and backup_status only */ - psr->checksum = compute_ip_checksum( - psr, offsetof(struct psr_backup_status, checksum)); + psr->checksum = ipchksum(psr, offsetof(struct psr_backup_status, checksum)); for (uint8_t *p = (uint8_t *)psr, i = 0; i < sizeof(*psr); i++, p++) cmos_write(*p, PARTITION_FW_CMOS_OFFSET + sizeof(struct cse_specific_info) + i); diff --git a/src/soc/mediatek/common/memory.c b/src/soc/mediatek/common/memory.c index 1a627dd..be72023 100644 --- a/src/soc/mediatek/common/memory.c +++ b/src/soc/mediatek/common/memory.c @@ -6,7 +6,6 @@ #include <commonlib/bsd/mem_chip_info.h> #include <console/console.h> #include <soc/dramc_common.h> -#include <ip_checksum.h> #include <mrc_cache.h> #include <soc/dramc_param.h> #include <soc/emi.h> diff --git a/src/soc/mediatek/mt8183/memory.c b/src/soc/mediatek/mt8183/memory.c index a43cdea..ca6f7a1 100644 --- a/src/soc/mediatek/mt8183/memory.c +++ b/src/soc/mediatek/mt8183/memory.c @@ -3,8 +3,8 @@ #include <assert.h> #include <bootmode.h> #include <cbfs.h> +#include <commonlib/bsd/ipchksum.h> #include <console/console.h> -#include <ip_checksum.h> #include <security/vboot/vboot_common.h> #include <soc/dramc_param.h> #include <soc/dramc_pi_api.h> @@ -63,8 +63,7 @@ static u32 compute_checksum(const struct dramc_param *dparam) { - return (u32)compute_ip_checksum(dparam->freq_params, - sizeof(dparam->freq_params)); + return (u32)ipchksum(dparam->freq_params, sizeof(dparam->freq_params)); } static int dram_run_fast_calibration(const struct dramc_param *dparam, diff --git a/src/southbridge/intel/bd82x6x/early_pch.c b/src/southbridge/intel/bd82x6x/early_pch.c index 8433bb2..414b238 100644 --- a/src/southbridge/intel/bd82x6x/early_pch.c +++ b/src/southbridge/intel/bd82x6x/early_pch.c @@ -3,7 +3,6 @@ #include <device/mmio.h> #include <device/pci_ops.h> #include <cf9_reset.h> -#include <ip_checksum.h> #include <device/pci_def.h> #include <device/smbus_host.h> #include <southbridge/intel/common/gpio.h> diff --git a/tests/commonlib/bsd/Makefile.mk b/tests/commonlib/bsd/Makefile.mk index bf17b6d..3de223e 100644 --- a/tests/commonlib/bsd/Makefile.mk +++ b/tests/commonlib/bsd/Makefile.mk @@ -2,8 +2,12 @@ tests-y += helpers-test tests-y += gcd-test +tests-y += ipchksum-test helpers-test-srcs += tests/commonlib/bsd/helpers-test.c gcd-test-srcs += tests/commonlib/bsd/gcd-test.c gcd-test-srcs += src/commonlib/bsd/gcd.c + +ipchksum-test-srcs += tests/commonlib/bsd/ipchksum-test.c +ipchksum-test-srcs += src/commonlib/bsd/ipchksum.c diff --git a/tests/commonlib/bsd/ipchksum-test.c b/tests/commonlib/bsd/ipchksum-test.c new file mode 100644 index 0000000..2aed94c --- /dev/null +++ b/tests/commonlib/bsd/ipchksum-test.c @@ -0,0 +1,99 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <commonlib/bsd/ipchksum.h> +#include <tests/test.h> +#include <string.h> +#include <stdlib.h> +#include <types.h> + +static const uint8_t test_data_simple[] = { + 0x64, 0x3b, 0x33, 0x17, 0x34, 0x74, 0x62, 0x30, 0x75, 0x73, 0xf3, 0x11, 0x30, 0x2c, + 0x34, 0x35, 0x6d, 0x39, 0x69, 0x32, 0x23, 0x24, 0x76, 0x71, 0x77, 0x30, 0x39, 0x75, + 0x76, 0x35, 0x71, 0x32, 0x40, 0x46, 0x34, 0x34, 0xBB, 0x03, 0x66, 0x52}; +static const size_t test_data_simple_sz = ARRAY_SIZE(test_data_simple); +static const uint16_t test_data_simple_checksum = 0x4267; + +static uint8_t test_data_zeros[1024]; +static const size_t test_data_zeros_sz = ARRAY_SIZE(test_data_zeros); +static const uint16_t test_data_zeros_checksum = 0xFFFF; + +static int setup_test_group(void **state) +{ + memset(test_data_zeros, 0, test_data_zeros_sz); + + return 0; +} + +static void test_ipchksum_zero_length(void **state) +{ + uint16_t res = ipchksum(test_data_simple, 0); + + /* Expect checksum to be in initial state as there are were no data provided. */ + assert_int_equal(0xFFFF, res); +} + +static void test_ipchksum_zero_buffer(void **state) +{ + uint16_t res = ipchksum(test_data_zeros, test_data_zeros_sz); + assert_int_equal(test_data_zeros_checksum, res); +} + +static void test_ipchksum_simple_data(void **state) +{ + uint16_t res; + uint16_t check_res; + const size_t helper_buffer_size = sizeof(uint8_t) * (test_data_simple_sz + 2); + char *helper_buffer = malloc(helper_buffer_size); + + /* Self test */ + assert_non_null(helper_buffer); + + /* Expect function to generate the same checksum as stored in */ + res = ipchksum(test_data_simple, test_data_simple_sz); + assert_int_equal(test_data_simple_checksum, res); + + /* Copy test data and checksum to new buffer. Expect computed checksum to be zero, + as it proves the data and the checksum are correct. */ + memcpy(helper_buffer, test_data_simple, test_data_simple_sz); + helper_buffer[helper_buffer_size - 2] = res & 0xFF; + helper_buffer[helper_buffer_size - 1] = (res >> 8) & 0xFF; + check_res = ipchksum(helper_buffer, helper_buffer_size); + assert_int_equal(0, check_res); + + free(helper_buffer); +} + +static void test_ipchksum_add_empty_values(void **state) +{ + uint16_t res; + + res = ipchksum_add(0, 0xFFFF, 0xFFFF); + assert_int_equal(0xFFFF, res); + + res = ipchksum_add(1, 0xFFFF, 0xFFFF); + assert_int_equal(0xFFFF, res); +} + +static void test_ipchksum_add(void **state) +{ + uint16_t res_1 = ipchksum(test_data_simple, test_data_simple_sz / 2); + uint16_t res_2 = ipchksum(test_data_simple + test_data_simple_sz / 2, + test_data_simple_sz / 2); + uint16_t res_sum = ipchksum_add(test_data_simple_sz / 2, res_1, res_2); + + assert_int_equal(test_data_simple_checksum, res_sum); +} + +int main(void) +{ + const struct CMUnitTest tests[] = { + cmocka_unit_test(test_ipchksum_zero_length), + cmocka_unit_test(test_ipchksum_zero_buffer), + cmocka_unit_test(test_ipchksum_simple_data), + + cmocka_unit_test(test_ipchksum_add_empty_values), + cmocka_unit_test(test_ipchksum_add), + }; + + return cb_run_group_tests(tests, setup_test_group, NULL); +} diff --git a/tests/lib/Makefile.mk b/tests/lib/Makefile.mk index 0cab1ba..b8c4c71 100644 --- a/tests/lib/Makefile.mk +++ b/tests/lib/Makefile.mk @@ -22,7 +22,6 @@ tests-y += malloc-test tests-y += memmove-test tests-y += crc_byte-test -tests-y += compute_ip_checksum-test tests-y += memrange-test tests-y += uuid-test tests-y += bootmem-test @@ -123,9 +122,6 @@ crc_byte-test-srcs += tests/lib/crc_byte-test.c crc_byte-test-srcs += src/lib/crc_byte.c -compute_ip_checksum-test-srcs += tests/lib/compute_ip_checksum-test.c -compute_ip_checksum-test-srcs += src/lib/compute_ip_checksum.c - memrange-test-srcs += tests/lib/memrange-test.c memrange-test-srcs += src/lib/memrange.c memrange-test-srcs += tests/stubs/console.c @@ -148,7 +144,7 @@ coreboot_table-test-srcs += tests/lib/coreboot_table-test.c coreboot_table-test-srcs += tests/stubs/console.c -coreboot_table-test-srcs += src/lib/compute_ip_checksum.c +coreboot_table-test-srcs += src/commonlib/bsd/ipchksum.c coreboot_table-test-srcs += src/lib/coreboot_table.c coreboot_table-test-srcs += src/lib/imd_cbmem.c coreboot_table-test-srcs += src/lib/imd.c diff --git a/tests/lib/compute_ip_checksum-test.c b/tests/lib/compute_ip_checksum-test.c deleted file mode 100644 index d465bfc..0000000 --- a/tests/lib/compute_ip_checksum-test.c +++ /dev/null @@ -1,99 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-only */ - -#include <tests/test.h> -#include <string.h> -#include <stdlib.h> -#include <types.h> -#include <ip_checksum.h> - -static const uint8_t test_data_simple[] = { - 0x64, 0x3b, 0x33, 0x17, 0x34, 0x74, 0x62, 0x30, 0x75, 0x73, 0xf3, 0x11, 0x30, 0x2c, - 0x34, 0x35, 0x6d, 0x39, 0x69, 0x32, 0x23, 0x24, 0x76, 0x71, 0x77, 0x30, 0x39, 0x75, - 0x76, 0x35, 0x71, 0x32, 0x40, 0x46, 0x34, 0x34, 0xBB, 0x03, 0x66, 0x52}; -static const size_t test_data_simple_sz = ARRAY_SIZE(test_data_simple); -static const unsigned long test_data_simple_checksum = 0x4267; - -static uint8_t test_data_zeros[1024]; -static const size_t test_data_zeros_sz = ARRAY_SIZE(test_data_zeros); -static const unsigned long test_data_zeros_checksum = 0xFFFF; - -static int setup_test_group(void **state) -{ - memset(test_data_zeros, 0, test_data_zeros_sz); - - return 0; -} - -static void test_compute_ip_checksum_zero_length(void **state) -{ - unsigned long res = compute_ip_checksum(test_data_simple, 0); - - /* Expect checksum to be in initial state as there are were no data provided. */ - assert_int_equal(0xFFFF, res); -} - -static void test_compute_ip_checksum_zero_buffer(void **state) -{ - unsigned long res = compute_ip_checksum(test_data_zeros, test_data_zeros_sz); - assert_int_equal(test_data_zeros_checksum, res); -} - -static void test_compute_ip_checksum_simple_data(void **state) -{ - unsigned long res; - unsigned long check_res; - const size_t helper_buffer_size = sizeof(uint8_t) * (test_data_simple_sz + 2); - char *helper_buffer = malloc(helper_buffer_size); - - /* Self test */ - assert_non_null(helper_buffer); - - /* Expect function to generate the same checksum as stored in */ - res = compute_ip_checksum(test_data_simple, test_data_simple_sz); - assert_int_equal(test_data_simple_checksum, res); - - /* Copy test data and checksum to new buffer. Expect computed checksum to be zero, - as it proves the data and the checksum are correct. */ - memcpy(helper_buffer, test_data_simple, test_data_simple_sz); - helper_buffer[helper_buffer_size - 2] = res & 0xFF; - helper_buffer[helper_buffer_size - 1] = (res >> 8) & 0xFF; - check_res = compute_ip_checksum(helper_buffer, helper_buffer_size); - assert_int_equal(0, check_res); - - free(helper_buffer); -} - -static void test_add_ip_checksums_empty_values(void **state) -{ - unsigned long res; - - res = add_ip_checksums(0, 0xFFFF, 0xFFFF); - assert_int_equal(0xFFFF, res); - - res = add_ip_checksums(1, 0xFFFF, 0xFFFF); - assert_int_equal(0xFFFF, res); -} - -static void test_add_ip_checksums(void **state) -{ - unsigned long res_1 = compute_ip_checksum(test_data_simple, test_data_simple_sz / 2); - unsigned long res_2 = compute_ip_checksum(test_data_simple + test_data_simple_sz / 2, - test_data_simple_sz / 2); - unsigned long res_sum = add_ip_checksums(test_data_simple_sz / 2, res_1, res_2); - - assert_int_equal(test_data_simple_checksum, res_sum); -} - -int main(void) -{ - const struct CMUnitTest tests[] = { - cmocka_unit_test(test_compute_ip_checksum_zero_length), - cmocka_unit_test(test_compute_ip_checksum_zero_buffer), - cmocka_unit_test(test_compute_ip_checksum_simple_data), - - cmocka_unit_test(test_add_ip_checksums_empty_values), - cmocka_unit_test(test_add_ip_checksums), - }; - - return cb_run_group_tests(tests, setup_test_group, NULL); -} -- To view, visit https://review.coreboot.org/c/coreboot/+/80251?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ic04c714c00439a17fc04a8a6e730cc2aa19b8e68 Gerrit-Change-Number: 80251 Gerrit-PatchSet: 4 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

[M] Change in coreboot[main]: soc/intel/common/block: Add support for watchdog
by Marek Maślanka (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: Jakub Czapiga, Michał Żygowski, Subrata Banik. Hello Jakub Czapiga, Subrata Banik, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/79909?usp=email to look at the new patch set (#14). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: soc/intel/common/block: Add support for watchdog ...................................................................... soc/intel/common/block: Add support for watchdog Implement watchdog for intel based platform by filling ACPI Watchdog Action Table (WDAT) table. The WDAT ACPI table encompasses essential watchdog functions, including: - Setting and retrieving countdown/timeout values - Starting and stopping the watchdog - Pinging the watchdog - Retrieving the cause of the last reboot, whether it was triggered by the watchdog or another reason The general purpose register TCO_MESSAGE1 stores the reason for the most recent reboot rather than the original register TCO2_STS. This is because the firmware must clear TCO2_STS, and it can't be reused for storing this information for the operating system. The watchdog is designed for use by the OS through certain defined actions in the WDAT table. It relies on the ACPI Power Management Timer, which may result in an increase in power consumption. BUG=b:314260167 TEST=Enable CONFIG_ACPI_WDAT_WDT and CONFIG_USE_PM_ACPI_TIMER in the config. Enable CONFIG_WDAT_WDT in the kernel config. Build and deploy both firmware and kernel to the device. Trigger the watchdog by performing the command: “cat > /dev/watchdog”. Wait approximately 30 seconds for the watchdog to reset the device. Change-Id: Iaf7971f8407920a553fd91d2ed04193c882e08f1 Signed-off-by: Marek Maslanka <mmaslanka(a)google.com> --- M src/soc/intel/common/block/acpi/acpi.c M src/soc/intel/common/block/include/intelblocks/tco.h M src/soc/intel/common/block/smbus/tco.c 3 files changed, 169 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/79909/14 -- To view, visit https://review.coreboot.org/c/coreboot/+/79909?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Iaf7971f8407920a553fd91d2ed04193c882e08f1 Gerrit-Change-Number: 79909 Gerrit-PatchSet: 14 Gerrit-Owner: Marek Maślanka <mmaslanka(a)google.com> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[main]: drivers/intel/fsp2_0: Add FSP_DOES_NOT_NEED_TEMP_RAM
by Martin L Roth (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: Andrey Petrov, Chen, Gang C, David Hendricks, Johnny Lin, Lean Sheng Tan, Patrick Rudolph, Paul Menzel, Ronak Kanabar, Shuo Liu. Martin L Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80328?usp=email ) Change subject: drivers/intel/fsp2_0: Add FSP_DOES_NOT_NEED_TEMP_RAM ...................................................................... Patch Set 3: (1 comment) File src/drivers/intel/fsp2_0/Kconfig: https://review.coreboot.org/c/coreboot/+/80328/comment/b226f5e5_8f8faf1f : PS3, Line 42: FSP_DOES_NOT_NEED_TEMP_RAM > Personally I’d prefer non-negative names, that means without `NOT`. Maybe "FSP_ALLOCATES_TEMP_RAM"? The FSP still needs temp_ram, it's just allocated differently, right? -- To view, visit https://review.coreboot.org/c/coreboot/+/80328?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I17a73e4f627c91808ed17c712b72937662ae9293 Gerrit-Change-Number: 80328 Gerrit-PatchSet: 3 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin L Roth <gaumless(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Comment-Date: Fri, 02 Feb 2024 21:30:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2024