coreboot-gerrit January 2024

coreboot-gerrit@coreboot.org

1 participants
3043 discussions

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

31 Jan '24

Julius Werner has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/80253?usp=email ) 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> --- 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(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/53/80253/1 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: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: newchange

1 0

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

31 Jan '24

Attention is currently required from: Jakub Czapiga. Julius Werner has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/80252?usp=email ) 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> --- M tests/commonlib/bsd/ipchksum-test.c 1 file changed, 30 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/52/80252/1 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: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-MessageType: newchange

1 0

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

31 Jan '24

Julius Werner has uploaded this change for review. ( 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-calculatted "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> --- 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/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/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/mt8183/memory.c M tests/commonlib/bsd/Makefile.inc A tests/commonlib/bsd/ipchksum-test.c M tests/lib/Makefile.inc D tests/lib/compute_ip_checksum-test.c 17 files changed, 193 insertions(+), 195 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/80251/1 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/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/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/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/tests/commonlib/bsd/Makefile.inc b/tests/commonlib/bsd/Makefile.inc index bf17b6d..3de223e 100644 --- a/tests/commonlib/bsd/Makefile.inc +++ b/tests/commonlib/bsd/Makefile.inc @@ -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.inc b/tests/lib/Makefile.inc index 0cab1ba..b8c4c71 100644 --- a/tests/lib/Makefile.inc +++ b/tests/lib/Makefile.inc @@ -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: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[main]: util/kconfig: Uprev to Linux 6.7's kconfig
by Patrick Georgi (Code Review) 30 Jan '24

30 Jan '24

Patrick Georgi has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/80263?usp=email ) Change subject: util/kconfig: Uprev to Linux 6.7's kconfig ...................................................................... util/kconfig: Uprev to Linux 6.7's kconfig Just a memory leak fix in Linux 6.7. Change-Id: I1ff302dafa01e78429a30ff18e21ffe0b45ce46e Signed-off-by: Patrick Georgi <patrick(a)coreboot.org> --- M util/kconfig/patches/0007-kconfig-Allow-KCONFIG_STRICT-outside-of-confdata.c.patch M util/kconfig/patches/0008-kconfig-Add-wildcard-support-for-source.patch M util/kconfig/symbol.c 3 files changed, 21 insertions(+), 23 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/80263/1 diff --git a/util/kconfig/patches/0007-kconfig-Allow-KCONFIG_STRICT-outside-of-confdata.c.patch b/util/kconfig/patches/0007-kconfig-Allow-KCONFIG_STRICT-outside-of-confdata.c.patch index f98729e..a9f3218 100644 --- a/util/kconfig/patches/0007-kconfig-Allow-KCONFIG_STRICT-outside-of-confdata.c.patch +++ b/util/kconfig/patches/0007-kconfig-Allow-KCONFIG_STRICT-outside-of-confdata.c.patch @@ -26,43 +26,43 @@ --- kconfig.orig/symbol.c +++ kconfig/symbol.c @@ -37,6 +37,7 @@ static struct symbol symbol_empty = { - + struct symbol *modules_sym; static tristate modules_val; +static int sym_warnings; - + enum symbol_type sym_get_type(struct symbol *sym) { -@@ -319,12 +320,14 @@ static void sym_warn_unmet_dep(struct sy +@@ -317,12 +318,14 @@ static void sym_warn_unmet_dep(struct sy " Selected by [m]:\n"); - + fputs(str_get(&gs), stderr); + sym_warnings++; } - + void sym_calc_value(struct symbol *sym) { struct symbol_value newval, oldval; struct property *prop; + const char *werror; struct expr *e; - + if (!sym) -@@ -340,8 +343,9 @@ void sym_calc_value(struct symbol *sym) +@@ -338,8 +341,9 @@ void sym_calc_value(struct symbol *sym) sym_calc_value(prop_get_symbol(prop)); } - + + werror = getenv("KCONFIG_WERROR"); + sym_warnings = 0; sym->flags |= SYMBOL_VALID; - oldval = sym->curr; - + switch (sym->type) { -@@ -432,6 +436,9 @@ void sym_calc_value(struct symbol *sym) +@@ -430,6 +434,9 @@ void sym_calc_value(struct symbol *sym) ; } - + + if (sym_warnings && werror) + exit(1); + diff --git a/util/kconfig/patches/0008-kconfig-Add-wildcard-support-for-source.patch b/util/kconfig/patches/0008-kconfig-Add-wildcard-support-for-source.patch index b476bc1..2e6a24f 100644 --- a/util/kconfig/patches/0008-kconfig-Add-wildcard-support-for-source.patch +++ b/util/kconfig/patches/0008-kconfig-Add-wildcard-support-for-source.patch @@ -28,7 +28,7 @@ +++ kconfig/lexer.l @@ -8,6 +8,7 @@ %{ - + #include <assert.h> +#include <glob.h> #include <limits.h> @@ -37,7 +37,7 @@ @@ -439,6 +440,32 @@ void zconf_nextfile(const char *name) current_file = file; } - + +void zconf_nextfiles(const char *wildcard) +{ + glob_t g; @@ -78,7 +78,7 @@ +void zconf_nextfiles(const char *name); int zconf_lineno(void); const char *zconf_curname(void); - + Index: kconfig/parser.y =================================================================== --- kconfig.orig/parser.y @@ -91,4 +91,4 @@ + zconf_nextfiles($2); free($2); }; - + diff --git a/util/kconfig/symbol.c b/util/kconfig/symbol.c index a50f131..34fc66e 100644 --- a/util/kconfig/symbol.c +++ b/util/kconfig/symbol.c @@ -123,9 +123,9 @@ static void sym_validate_range(struct symbol *sym) { struct property *prop; + struct symbol *range_sym; int base; long long val, val2; - char str[64]; switch (sym->type) { case S_INT: @@ -141,17 +141,15 @@ if (!prop) return; val = strtoll(sym->curr.val, NULL, base); - val2 = sym_get_range_val(prop->expr->left.sym, base); + range_sym = prop->expr->left.sym; + val2 = sym_get_range_val(range_sym, base); if (val >= val2) { - val2 = sym_get_range_val(prop->expr->right.sym, base); + range_sym = prop->expr->right.sym; + val2 = sym_get_range_val(range_sym, base); if (val <= val2) return; } - if (sym->type == S_INT) - sprintf(str, "%lld", val2); - else - sprintf(str, "0x%llx", val2); - sym->curr.val = xstrdup(str); + sym->curr.val = range_sym->curr.val; } static void sym_set_changed(struct symbol *sym) -- To view, visit https://review.coreboot.org/c/coreboot/+/80263?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: I1ff302dafa01e78429a30ff18e21ffe0b45ce46e Gerrit-Change-Number: 80263 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Georgi <patrick(a)coreboot.org> Gerrit-MessageType: newchange

1 0

[XS] Change in coreboot[main]: mb/google/rex/variants/deku: Add H58G66BK7BX067 to RAM ID table
by Felix Held (Code Review) 30 Jan '24

30 Jan '24

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/80230?usp=email ) Change subject: mb/google/rex/variants/deku: Add H58G66BK7BX067 to RAM ID table ...................................................................... mb/google/rex/variants/deku: Add H58G66BK7BX067 to RAM ID table Add RAM ID for H58G66BK7BX067 0 (0000) BUG=b:322528721 BRANCH=firmware-rex-15709.B TEST=Run part_id_gen tool without any errors Change-Id: I31538988d1329d9e2f45d862eb0ae05c0d6a179e Signed-off-by: Tony Huang <tony-huang(a)quanta.corp-partner.google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/80230 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Kevin Chiu <kevin.chiu.17802(a)gmail.com> Reviewed-by: Kapil Porwal <kapilporwal(a)google.com> Reviewed-by: Eran Mitrani <mitrani(a)google.com> Reviewed-by: Subrata Banik <subratabanik(a)google.com> --- M src/mainboard/google/rex/variants/deku/memory/Makefile.mk M src/mainboard/google/rex/variants/deku/memory/dram_id.generated.txt M src/mainboard/google/rex/variants/deku/memory/mem_parts_used.txt 3 files changed, 5 insertions(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified Kevin Chiu: Looks good to me, approved Subrata Banik: Looks good to me, approved Eran Mitrani: Looks good to me, approved Kapil Porwal: Looks good to me, approved diff --git a/src/mainboard/google/rex/variants/deku/memory/Makefile.mk b/src/mainboard/google/rex/variants/deku/memory/Makefile.mk index 9620b9c..e7518ed 100644 --- a/src/mainboard/google/rex/variants/deku/memory/Makefile.mk +++ b/src/mainboard/google/rex/variants/deku/memory/Makefile.mk @@ -1,7 +1,7 @@ # SPDX-License-Identifier: GPL-2.0-or-later # This is an auto-generated file. Do not edit!! # Generated by: -# /tmp/go-build2560620659/b001/exe/part_id_gen MTL lp5 src/mainboard/google/rex/variants/deku/memory src/mainboard/google/rex/variants/deku/memory/mem_parts_used.txt +# ./util/spd_tools/bin/part_id_gen MTL lp5 src/mainboard/google/rex/variants/deku/memory src/mainboard/google/rex/variants/deku/memory/mem_parts_used.txt SPD_SOURCES = -SPD_SOURCES += spd/lp5/set-0/spd-8.hex # ID = 0(0b0000) Parts = MT62F2G32D4DS-026 WT:B +SPD_SOURCES += spd/lp5/set-0/spd-8.hex # ID = 0(0b0000) Parts = MT62F2G32D4DS-026 WT:B, H58G66BK7BX067 diff --git a/src/mainboard/google/rex/variants/deku/memory/dram_id.generated.txt b/src/mainboard/google/rex/variants/deku/memory/dram_id.generated.txt index 21b3f69..0e3a06a 100644 --- a/src/mainboard/google/rex/variants/deku/memory/dram_id.generated.txt +++ b/src/mainboard/google/rex/variants/deku/memory/dram_id.generated.txt @@ -1,7 +1,8 @@ # SPDX-License-Identifier: GPL-2.0-or-later # This is an auto-generated file. Do not edit!! # Generated by: -# /tmp/go-build2560620659/b001/exe/part_id_gen MTL lp5 src/mainboard/google/rex/variants/deku/memory src/mainboard/google/rex/variants/deku/memory/mem_parts_used.txt +# ./util/spd_tools/bin/part_id_gen MTL lp5 src/mainboard/google/rex/variants/deku/memory src/mainboard/google/rex/variants/deku/memory/mem_parts_used.txt DRAM Part Name ID to assign MT62F2G32D4DS-026 WT:B 0 (0000) +H58G66BK7BX067 0 (0000) diff --git a/src/mainboard/google/rex/variants/deku/memory/mem_parts_used.txt b/src/mainboard/google/rex/variants/deku/memory/mem_parts_used.txt index 53ec721..0871f3c 100644 --- a/src/mainboard/google/rex/variants/deku/memory/mem_parts_used.txt +++ b/src/mainboard/google/rex/variants/deku/memory/mem_parts_used.txt @@ -10,3 +10,4 @@ # Part Name MT62F2G32D4DS-026 WT:B +H58G66BK7BX067 -- To view, visit https://review.coreboot.org/c/coreboot/+/80230?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: I31538988d1329d9e2f45d862eb0ae05c0d6a179e Gerrit-Change-Number: 80230 Gerrit-PatchSet: 3 Gerrit-Owner: Tony Huang <tony-huang(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Derek Huang <derekhuang(a)google.com> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Kevin Chiu <kevin.chiu.17802(a)gmail.com> Gerrit-Reviewer: Kevin Chiu <kevin.chiu(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun <tstuli(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[XS] Change in coreboot[main]: MAINTAINERS: Update maintainers list for Xeon SP
by Lean Sheng Tan (Code Review) 30 Jan '24

30 Jan '24

Attention is currently required from: Arthur Heymans, Felix Held, Felix Singer, Patrick Rudolph, Paul Menzel, Shuo Liu. Lean Sheng Tan has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80240?usp=email ) Change subject: MAINTAINERS: Update maintainers list for Xeon SP ...................................................................... Patch Set 2: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/80240/comment/dd9d6045_e47e1626 : PS2, Line 7: Update maintainers list for Xeon SP : > The title is not appropriate for the names. please use the commit summary instead. it is always this way as Paul suggested: https://review.coreboot.org/c/coreboot/+/78989 -- To view, visit https://review.coreboot.org/c/coreboot/+/80240?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: Ide3aa87fca69be6b0f1ffe0b18d7ffb410e5c563 Gerrit-Change-Number: 80240 Gerrit-PatchSet: 2 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Tue, 30 Jan 2024 17:56:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: MAINTAINERS: Update maintainers list for Xeon SP
by Felix Singer (Code Review) 30 Jan '24

30 Jan '24

Attention is currently required from: Arthur Heymans, Felix Held, Patrick Rudolph, Paul Menzel, Shuo Liu. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80240?usp=email ) Change subject: MAINTAINERS: Update maintainers list for Xeon SP ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/80240/comment/8cefb1b8_72b6ed24 : PS2, Line 7: Update maintainers list for Xeon SP : > Please be more specific. Maybe: […] The title is not appropriate for the names. please use the commit summary instead. -- To view, visit https://review.coreboot.org/c/coreboot/+/80240?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: Ide3aa87fca69be6b0f1ffe0b18d7ffb410e5c563 Gerrit-Change-Number: 80240 Gerrit-PatchSet: 2 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Tue, 30 Jan 2024 17:50:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: cpu/x86: Add 1GiB pages for memory access up to 512GiB
by Arthur Heymans (Code Review) 30 Jan '24

30 Jan '24

Attention is currently required from: Ashish Kumar Mishra, Jérémy Compostella, Paul Menzel, Subrata Banik, Wonkyu Kim. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80088?usp=email ) Change subject: cpu/x86: Add 1GiB pages for memory access up to 512GiB ...................................................................... Patch Set 6: Code-Review-1 (3 comments) Patchset: PS5: > This patch is for access upto 512GB only and verified on 64bit mode with 38-bit and 42-bit hardware. Are you using any different mode? It was on a 36-bit physical address cpu, but it has been some time so my memory could be false. I used the following. /* SPDX-License-Identifier: GPL-2.0-only */ /* * For reference see "AMD64 Architecture Programmer's Manual Volume 2", * Document 24593-Rev. 3.31-July 2019 Chapter 5.3.4 * * Page table attributes: WB, User+Supervisor, Present, Writeable, Accessed, Dirty */ .section .rodata #define _PRES (1ULL << 0) #define _RW (1ULL << 1) #define _US (1ULL << 2) #define _A (1ULL << 5) #define _D (1ULL << 6) #define _PS (1ULL << 7) #define _GEN_DIR(a) (_PRES + _RW + _US + _A + (a)) #define _GEN_PAGE(a) (_PRES + _RW + _US + _PS + _A + _D + (a)) .global PM4LE .align 32 PM4LE: .quad _GEN_DIR(PDPE_table) .align 4096 PDPE_table: /* identity map 1G pages */ .rept 32 .quad _GEN_PAGE(0x40000000 * ((. - PDPE_table) >> 3)) .endr File src/cpu/x86/64bit/pt1G.S: https://review.coreboot.org/c/coreboot/+/80088/comment/ebea3bc3_e6b844d5 : PS6, Line 19: PM align to 32 https://review.coreboot.org/c/coreboot/+/80088/comment/fed948cc_acbc837f : PS6, Line 23: 16 needs to align to 4096 -- To view, visit https://review.coreboot.org/c/coreboot/+/80088?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: Id569ae5b50abf5b72e4db33b5e4cd802399e76ec Gerrit-Change-Number: 80088 Gerrit-PatchSet: 6 Gerrit-Owner: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Haribalaraman Ramasubramanian <haribalaraman.r(a)intel.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Tue, 30 Jan 2024 17:40:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: cpu/x86: Add 1GiB pages for memory access up to 512GiB
by Ashish Kumar Mishra (Code Review) 30 Jan '24

30 Jan '24

Attention is currently required from: Arthur Heymans, Jérémy Compostella, Paul Menzel, Subrata Banik, Wonkyu Kim. Ashish Kumar Mishra has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80088?usp=email ) Change subject: cpu/x86: Add 1GiB pages for memory access up to 512GiB ...................................................................... Patch Set 6: (1 comment) Patchset: PS5: > This assumes a physical address space of 41 bits which is not always the case on all hardware suppor […] This patch is for access upto 512GB only and verified on 64bit mode with 38-bit and 42-bit hardware. Are you using any different mode? -- To view, visit https://review.coreboot.org/c/coreboot/+/80088?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: Id569ae5b50abf5b72e4db33b5e4cd802399e76ec Gerrit-Change-Number: 80088 Gerrit-PatchSet: 6 Gerrit-Owner: Ashish Kumar Mishra <ashish.k.mishra(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Haribalaraman Ramasubramanian <haribalaraman.r(a)intel.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Tue, 30 Jan 2024 17:24:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

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

30 Jan '24

Attention is currently required from: Jakub Czapiga, Michał Żygowski, Subrata Banik. Marek Maślanka has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79909?usp=email ) Change subject: soc/intel/common/block: Add support for watchdog ...................................................................... Patch Set 13: (8 comments) File src/soc/intel/common/block/acpi/acpi.c: https://review.coreboot.org/c/coreboot/+/79909/comment/41488cb1_b0fe0436 : PS12, Line 395: wdat > missed the NULL check in the code ? I added checking against the NULL, just in case https://review.coreboot.org/c/coreboot/+/79909/comment/cf3d2b27_f67f0ce3 : PS12, Line 397: tcobase > what if this address is zero like TCO is not enabled ? Now, it's checked and handled https://review.coreboot.org/c/coreboot/+/79909/comment/6018259d_b431745b : PS12, Line 410: // write countdown > ` […] Done https://review.coreboot.org/c/coreboot/+/79909/comment/826fd3e1_2829571b : PS12, Line 461: 11 > what is the meaning of the hardcoded value ? I replaced it with a function that takes the offset from a given field register definition to make it more clear and less error-prone. https://review.coreboot.org/c/coreboot/+/79909/comment/3eb694a1_4b948f99 : PS12, Line 468: entry->action = ACPI_WDAT_SET_RUNNING_STATE; : entry->instruction = ACPI_WDAT_WRITE_VALUE | ACPI_WDAT_PRESERVE_REGISTER; : entry->mask = 0x01; : entry->register_region.space_id = ACPI_ADDRESS_SPACE_IO; : entry->register_region.addrl = tcobase + TCO1_CNT; : entry->register_region.access_size = ACPI_WDAT_ACCESS_SIZE_WORD; : entry->register_region.bit_offset = 11; > can this be a helper function like […] I would prefer to keep it consistent with filling in other entries instead of creating separate functions for that. File src/soc/intel/common/block/smbus/tco.c: https://review.coreboot.org/c/coreboot/+/79909/comment/dfb2d8fd_f232529f : PS12, Line 79: { > single statement, don't need braces Done https://review.coreboot.org/c/coreboot/+/79909/comment/4501edc8_52629f8a : PS12, Line 148: __weak > why weak ? right, needless https://review.coreboot.org/c/coreboot/+/79909/comment/70de1e51_7a23ec02 : PS12, Line 149: u32 > uint32_t Done -- 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: 13 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-Comment-Date: Tue, 30 Jan 2024 16:50:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit January 2024