coreboot-gerrit January 2024

coreboot-gerrit@coreboot.org

1 participants
3043 discussions

[M] Change in coreboot[main]: soc/intel/common/block: Add support for watchdog
by Subrata Banik (Code Review) 31 Jan '24

31 Jan '24

Attention is currently required from: Jakub Czapiga, Marek Maślanka, Michał Żygowski. Subrata Banik 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: (3 comments) File src/soc/intel/common/block/acpi/acpi.c: https://review.coreboot.org/c/coreboot/+/79909/comment/4190f296_8d6a6543 : PS12, Line 461: 11 > I replaced it with a function that takes the offset from a given field register definition to make i […] Acknowledged https://review.coreboot.org/c/coreboot/+/79909/comment/45355ca5_3ab65b64 : 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; > I would prefer to keep it consistent with filling in other entries instead of creating separate functions for that. i'm unable to understand what u mean by "consistent with filling in other entries". Having a helper function can help improving the code readability as there are redundant programming except the `action` type being different File src/soc/intel/common/block/acpi/acpi.c: https://review.coreboot.org/c/coreboot/+/79909/comment/ec56f76b_ee7f4b7b : PS13, Line 396: rightmost_bit_offset i'm unable to follow which right most bit u r considering ? may be a little meaningful name with the details about what this helper will do ? -- 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: Marek Maślanka <mmaslanka(a)google.com> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Comment-Date: Wed, 31 Jan 2024 06:01:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Marek Maślanka <mmaslanka(a)google.com> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: superio/ite/common: Add option to change clock source of WDT
by Nicholas Sudsgaard (Code Review) 31 Jan '24

31 Jan '24

Nicholas Sudsgaard has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80264?usp=email ) Change subject: superio/ite/common: Add option to change clock source of WDT ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: Sorry, something is not right. CSSWDT is bit 4; I'm setting bit 3. -- To view, visit https://review.coreboot.org/c/coreboot/+/80264?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: I64f43cad692c08d6f07eb981a990cf08db580575 Gerrit-Change-Number: 80264 Gerrit-PatchSet: 1 Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 31 Jan 2024 05:34:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

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

31 Jan '24

Attention is currently required from: Arthur Heymans, Ashish Kumar Mishra, Jérémy Compostella, Paul Menzel, Subrata Banik. Wonkyu Kim 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 patch is for access upto 512GB only and verified on 64bit mode with 38-bit and 42-bit hardwar […] It looks implementation is same but using different macro and different coding style. If we using this coding style, is it ok? I agree to use same coding style to more readable and comparable for both tables. BTW, rept 32 will not work for our case for 42bit physical address. So, We need to more pages to access. -- 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: 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: Wed, 31 Jan 2024 04:30:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No 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]: superio/ite/common: Add option to change clock source of WDT
by Nicholas Sudsgaard (Code Review) 31 Jan '24

31 Jan '24

Nicholas Sudsgaard has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/80264?usp=email ) Change subject: superio/ite/common: Add option to change clock source of WDT ...................................................................... superio/ite/common: Add option to change clock source of WDT This option is necessary to get the serial port to work on some mainboards (e.g. Lenovo ThinkCentre M710s). This option is present on all datasheets I could find: * IT8712F * IT8720F * IT8728F * IT8783EF Change-Id: I64f43cad692c08d6f07eb981a990cf08db580575 Signed-off-by: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> --- M src/superio/ite/common/early_serial.c M src/superio/ite/common/ite.h 2 files changed, 9 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/64/80264/1 diff --git a/src/superio/ite/common/early_serial.c b/src/superio/ite/common/early_serial.c index 5ff403f..4f4e342 100644 --- a/src/superio/ite/common/early_serial.c +++ b/src/superio/ite/common/early_serial.c @@ -53,10 +53,14 @@ * and pass: CLKIN_DEV * ITE_UART_CLK_PREDIVIDE_24 * ITE_UART_CLK_PREDIVIDE_48 (default) + * + * ITE_CSSWDT_EXTERNAL_CLKIN + * ITE_CSSWDT_INTERNAL_CLKIN (default) */ -void ite_conf_clkin(pnp_devfn_t dev, u8 predivide) +void ite_conf_clkin(pnp_devfn_t dev, u8 reg) { - ite_reg_write(dev, ITE_CONFIG_REG_CLOCKSEL, (0x1 & predivide)); + const u8 mask = 0x09; + ite_reg_write(dev, ITE_CONFIG_REG_CLOCKSEL, (mask & reg)); } /* Bring up early serial debugging output before the RAM is initialized. */ diff --git a/src/superio/ite/common/ite.h b/src/superio/ite/common/ite.h index 3e9b5028..9c55815 100644 --- a/src/superio/ite/common/ite.h +++ b/src/superio/ite/common/ite.h @@ -9,6 +9,9 @@ #define ITE_UART_CLK_PREDIVIDE_48 0x00 /* default */ #define ITE_UART_CLK_PREDIVIDE_24 0x01 +#define ITE_CSSWDT_INTERNAL_CLKIN 0x00 /* default */ +#define ITE_CSSWDT_EXTERNAL_CLKIN 0x08 + void ite_conf_clkin(pnp_devfn_t dev, u8 predivide); void ite_enable_serial(pnp_devfn_t dev, u16 iobase); -- To view, visit https://review.coreboot.org/c/coreboot/+/80264?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: I64f43cad692c08d6f07eb981a990cf08db580575 Gerrit-Change-Number: 80264 Gerrit-PatchSet: 1 Gerrit-Owner: Nicholas Sudsgaard <devel+coreboot(a)nsudsgaard.com> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[main]: util/cbmem: Use 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/+/80256?usp=email ) 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> --- M util/cbmem/Makefile M util/cbmem/cbmem.c 2 files changed, 6 insertions(+), 23 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/56/80256/1 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: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: newchange

1 0

[L] Change in coreboot[main]: util/ifdtool: Add new cmdline to enable GPR0 protection
by Reka Norman (Code Review) 31 Jan '24

31 Jan '24

Attention is currently required from: Stefan Reinauer, Subrata Banik. Reka Norman has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80235?usp=email ) Change subject: util/ifdtool: Add new cmdline to enable GPR0 protection ...................................................................... Patch Set 4: (7 comments) Patchset: PS4: Looks good with a few nits. File util/ifdtool/ifdtool.c: https://review.coreboot.org/c/coreboot/+/80235/comment/2b625ec6_f4e29637 : PS4, Line 47: union gprd_info nit: just `union gprd`? (I think you missed my previous comment about this) https://review.coreboot.org/c/coreboot/+/80235/comment/4aa301cd_d70634f4 : PS4, Line 50: 0-15 `0-14` (or just drop this since it's implied from the bitfield) https://review.coreboot.org/c/coreboot/+/80235/comment/0825e1e6_f478a9ef : PS4, Line 50: * Start Address: bit 0-15 of the GPRD represents the protected region start address, nit: limit to 100 chars? Not sure if there's a guideline, but the rest of the file is under 100. https://review.coreboot.org/c/coreboot/+/80235/comment/f3a41287_1d0f31d9 : PS4, Line 1676: fpt++; /* Move to the next structure which is FPT sub-partition entries */ : struct cse_fpt_sub_part *fpt_sub_part = (struct cse_fpt_sub_part *)fpt; nit: this could just be `struct cse_fpt_sub_part *fpt_sub_part = (struct cse_fpt_sub_part *)(fpt + 1)` https://review.coreboot.org/c/coreboot/+/80235/comment/e8ea5c59_5f5deaf7 : PS4, Line 1685: continue Don't need this https://review.coreboot.org/c/coreboot/+/80235/comment/ede69e06_5eaf2687 : PS4, Line 1776: nit: `gprd.data.read_protect_en = 0`. Since you're reusing gprd from above there's a chance it could be set. -- To view, visit https://review.coreboot.org/c/coreboot/+/80235?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: I27c533ae4109c79299f4e7ff75e750d7cc64280f Gerrit-Change-Number: 80235 Gerrit-PatchSet: 4 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Reka Norman <rekanorman(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Wed, 31 Jan 2024 03:17:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: mb/google/brox: Enable HDA Codec ALC256
by Shelley Chen (Code Review) 31 Jan '24

31 Jan '24

Attention is currently required from: Paul Menzel, Poornima Tom, Vamshi Krishna Gopal. Shelley Chen has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/79723?usp=email ) Change subject: mb/google/brox: Enable HDA Codec ALC256 ...................................................................... Patch Set 6: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/79723/comment/0fc96858_38381d15 : PS5, Line 14: To verify HDA on Brox > Shelly, were you able to test it? Yes, I saw the device in sysfs: cat /sys/bus/hdaudio/devices/ehdaudio0D0/chip_name ID 256 cat /sys/bus/hdaudio/devices/ehdaudio0D0/vendor_name Realtek -- To view, visit https://review.coreboot.org/c/coreboot/+/79723?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: I1edd5aee053debe39b34048266703031c088cd00 Gerrit-Change-Number: 79723 Gerrit-PatchSet: 6 Gerrit-Owner: Poornima Tom <poornima.tom(a)intel.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Shelley Chen <shchen(a)google.com> Gerrit-Reviewer: Vamshi Krishna Gopal <vamshi.krishna.gopal(a)intel.com> Gerrit-Reviewer: Vamshi Krishna Gopal <vamshi.krishna.gopal(a)intel.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Poornima Tom <poornima.tom(a)intel.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Vamshi Krishna Gopal <vamshi.krishna.gopal(a)intel.com> Gerrit-Comment-Date: Wed, 31 Jan 2024 03:01:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Poornima Tom <poornima.tom(a)intel.com> Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

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

31 Jan '24

Attention is currently required from: Angel Pons, Hung-Te Lin, Jakub Czapiga, Julius Werner, Yidi Lin, Yu-Ping Wu. Hello Angel Pons, Hung-Te Lin, Jakub Czapiga, Yidi Lin, Yu-Ping Wu, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/80251?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified-1 by build bot (Jenkins) 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/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.inc A tests/commonlib/bsd/ipchksum-test.c M tests/lib/Makefile.inc D tests/lib/compute_ip_checksum-test.c 24 files changed, 193 insertions(+), 202 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/80251/2 -- 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: 2 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Jakub Czapiga <czapiga(a)google.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-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Jakub Czapiga <czapiga(a)google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[main]: commonlib: Add assembly optimization for ipchksum() on x86
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/+/80255?usp=email ) 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> --- M src/commonlib/bsd/ipchksum.c 1 file changed, 24 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/80255/1 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: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[main]: commonlib: Add assembly optimization for ipchksum() on arm64
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/+/80254?usp=email ) 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> --- M src/commonlib/bsd/ipchksum.c 1 file changed, 25 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/80254/1 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: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: newchange

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit January 2024