coreboot-gerrit February 2016

coreboot-gerrit@coreboot.org

1 participants
1305 discussions

New patch to review for coreboot: Partly revert "lzma: Port size-checking ulzman() version to coreboot"
by Paul Menzel 24 Feb '16

24 Feb '16

Paul Menzel (paulepanter(a)users.sourceforge.net) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/13773 -gerrit commit ab093aa310187554e040bb1c0fe0615cc66e5873 Author: Paul Menzel <paulepanter(a)users.sourceforge.net> Date: Wed Feb 24 00:03:27 2016 +0100 Partly revert "lzma: Port size-checking ulzman() version to coreboot" This partly reverts a25b5d257dbfbff808b19bf8c48565435e6bef9d. Revert the change in `src/lib/lzmadecode.c` as it decreases the boot time to ramstage by 150 ms on the ASRock E350M1. There seems to be some special case with the flash ROM access. Until this has been figured out, revert the change. Change-Id: Icc480c41cda77526256ba1761b29a24def48400e Signed-off-by: Paul Menzel <paulepanter(a)users.sourceforge.net> --- src/lib/lzmadecode.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/lib/lzmadecode.c b/src/lib/lzmadecode.c index fbf1596..ada7226 100644 --- a/src/lib/lzmadecode.c +++ b/src/lib/lzmadecode.c @@ -29,12 +29,9 @@ #define kBitModelTotal (1 << kNumBitModelTotalBits) #define kNumMoveBits 5 -/* Use 32-bit reads whenever possible to avoid bad flash performance. Fall back - * to byte reads for last 4 bytes since RC_TEST returns an error when BufferLim - * is *reached* (not surpassed!), meaning we can't allow that to happen while - * there are still bytes to decode from the algorithm's point of view. */ +/* Use 32-bit reads whenever possible to avoid bad flash performance. */ #define RC_READ_BYTE (look_ahead_ptr < 4 ? look_ahead.raw[look_ahead_ptr++] \ - : ((((uintptr_t) Buffer & 3) || ((SizeT) (BufferLim - Buffer) <= 4)) ? (*Buffer++) \ + : ((((uintptr_t) Buffer & 3) || ((SizeT) (BufferLim - Buffer) < 4)) ? (*Buffer++) \ : ((look_ahead.dw = *(UInt32 *)Buffer), (Buffer += 4), (look_ahead_ptr = 1), look_ahead.raw[0]))) #define RC_INIT2 Code = 0; Range = 0xFFFFFFFF; \

1 0

Patch set updated for coreboot: commonlib/lz4_wrapper: Use correct casts to ensure valid calculations
by Paul Menzel 23 Feb '16

23 Feb '16

Paul Menzel (paulepanter(a)users.sourceforge.net) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/13771 -gerrit commit 1b746b2b79d1ddd30060401777ec9ded2bb224c3 Author: Paul Menzel <paulepanter(a)users.sourceforge.net> Date: Tue Feb 23 18:59:04 2016 +0100 commonlib/lz4_wrapper: Use correct casts to ensure valid calculations Commit 09f2921b (cbfs: Add LZ4 in-place decompression support for pre-RAM stages) breaks building cbfstool with gcc (Debian 4.9.2-10) 4.9.2 in Debian 8.3 (jessie) with a 32-bit user space. It works fine in a 64-bit user space. ``` /home/joey/src/coreboot/src/commonlib/lz4_wrapper.c:164:18: note: in expansion of macro 'MIN' size_t size = MIN((uint32_t)b.size, dst + dstn - out); ^ /home/joey/src/coreboot/src/commonlib/include/commonlib/helpers.h:29:35: error: signed and unsigned type in conditional expression [-Werror=sign-compare] #define MIN(a,b) ((a) < (b) ? (a) : (b)) ^ ``` The problem is arithmetic on void*, so explicitly cast to the wanted types as suggested by user *redi* in #gcc(a)irc.freenode.net. Change-Id: I85bee25a69c432ef8bb934add7fd2e2e31f03662 Signed-off-by: Paul Menzel <paulepanter(a)users.sourceforge.net> --- src/commonlib/lz4_wrapper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/commonlib/lz4_wrapper.c b/src/commonlib/lz4_wrapper.c index ea7b90d..1a765af 100644 --- a/src/commonlib/lz4_wrapper.c +++ b/src/commonlib/lz4_wrapper.c @@ -161,7 +161,7 @@ size_t ulz4fn(const void *src, size_t srcn, void *dst, size_t dstn) } if (b.not_compressed) { - size_t size = MIN((uint32_t)b.size, dst + dstn - out); + size_t size = MIN((uintptr_t)b.size, (uintptr_t)dst + dstn - (uintptr_t)out); memcpy(out, in, size); if (size < b.size) break; /* output overrun */

1 0

Patch set updated for coreboot: Revert "cbfstool: Silence LZ4 -Wsign-compare warnings"
by Paul Menzel 23 Feb '16

23 Feb '16

Paul Menzel (paulepanter(a)users.sourceforge.net) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/13772 -gerrit commit 8f9a2b7f5d77d0a0cf2fa8a90f8642dd02d97bd8 Author: Paul Menzel <paulepanter(a)users.sourceforge.net> Date: Tue Feb 23 19:19:20 2016 +0100 Revert "cbfstool: Silence LZ4 -Wsign-compare warnings" This reverts commit 17cb0370a70ccfc2301b7974bf38d44c7271afea. It’s the wrong thing to do, to just disable the warning. The code is fixed for 32-bit user space now in Change-Id I85bee25a69c432ef8bb934add7fd2e2e31f03662 (commonlib/lz4_wrapper: Use correct casts to ensure valid calculations), so enable the warning again. Change-Id: I6d1c62c7b4875da8053c25e640c03cedf0ff2916 Signed-off-by: Paul Menzel <paulepanter(a)users.sourceforge.net> --- util/cbfstool/Makefile.inc | 1 - 1 file changed, 1 deletion(-) diff --git a/util/cbfstool/Makefile.inc b/util/cbfstool/Makefile.inc index 8824ad0..f480616 100644 --- a/util/cbfstool/Makefile.inc +++ b/util/cbfstool/Makefile.inc @@ -150,7 +150,6 @@ $(objutil)/cbfstool/cbfs.o: TOOLCFLAGS += -Wno-sign-compare -Wno-cast-qual $(objutil)/cbfstool/mem_pool.o: TOOLCFLAGS += -Wno-sign-compare -Wno-cast-qual # Tolerate lz4 warnings $(objutil)/cbfstool/lz4.o: TOOLCFLAGS += -Wno-missing-prototypes -$(objutil)/cbfstool/lz4_wrapper.o: TOOLCFLAGS += -Wno-sign-compare $(objutil)/cbfstool/fmd.o: $(objutil)/cbfstool/fmd_parser.h $(objutil)/cbfstool/fmd.o: $(objutil)/cbfstool/fmd_scanner.h

1 0

New patch to review for coreboot: Revert "cbfstool: Silence LZ4 -Wsign-compare warnings"
by Paul Menzel 23 Feb '16

23 Feb '16

Paul Menzel (paulepanter(a)users.sourceforge.net) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/13772 -gerrit commit 7110e09eb1185622ed6162213d8827e4d65159a6 Author: Paul Menzel <paulepanter(a)users.sourceforge.net> Date: Tue Feb 23 19:19:20 2016 +0100 Revert "cbfstool: Silence LZ4 -Wsign-compare warnings" This reverts commit 17cb0370a70ccfc2301b7974bf38d44c7271afea. It’s the wrong thing to do, to just disable the warning. The code is fixed for 32-bit user space now in Change-Id I85bee25a69c432ef8bb934add7fd2e2e31f03662 (commonlib/lz4_wrapper: Use correct casts to ensure valid calculations), so enable the warning again. Change-Id: I6d1c62c7b4875da8053c25e640c03cedf0ff2916 Signed-off-by: Paul Menzel <paulepanter(a)users.sourceforge.net> --- util/cbfstool/Makefile.inc | 1 - 1 file changed, 1 deletion(-) diff --git a/util/cbfstool/Makefile.inc b/util/cbfstool/Makefile.inc index 8824ad0..f480616 100644 --- a/util/cbfstool/Makefile.inc +++ b/util/cbfstool/Makefile.inc @@ -150,7 +150,6 @@ $(objutil)/cbfstool/cbfs.o: TOOLCFLAGS += -Wno-sign-compare -Wno-cast-qual $(objutil)/cbfstool/mem_pool.o: TOOLCFLAGS += -Wno-sign-compare -Wno-cast-qual # Tolerate lz4 warnings $(objutil)/cbfstool/lz4.o: TOOLCFLAGS += -Wno-missing-prototypes -$(objutil)/cbfstool/lz4_wrapper.o: TOOLCFLAGS += -Wno-sign-compare $(objutil)/cbfstool/fmd.o: $(objutil)/cbfstool/fmd_parser.h $(objutil)/cbfstool/fmd.o: $(objutil)/cbfstool/fmd_scanner.h

1 0

New patch to review for coreboot: commonlib/lz4_wrapper: Use correct casts to ensure valid calculations
by Paul Menzel 23 Feb '16

23 Feb '16

Paul Menzel (paulepanter(a)users.sourceforge.net) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/13771 -gerrit commit 428383ac4627c2ec522d088779e459519a16c34d Author: Paul Menzel <paulepanter(a)users.sourceforge.net> Date: Tue Feb 23 18:59:04 2016 +0100 commonlib/lz4_wrapper: Use correct casts to ensure valid calculations Commit 09f2921b (cbfs: Add LZ4 in-place decompression support for pre-RAM stages) breaks building cbfstool with gcc (Debian 4.9.2-10) 4.9.2 in Debian 8.3 (jessie) with a 32-bit user space. It works fine in a 64-bit user space. ``` /home/joey/src/coreboot/src/commonlib/lz4_wrapper.c:164:18: note: in expansion of macro 'MIN' size_t size = MIN((uint32_t)b.size, dst + dstn - out); ^ /home/joey/src/coreboot/src/commonlib/include/commonlib/helpers.h:29:35: error: signed and unsigned type in conditional expression [-Werror=sign-compare] #define MIN(a,b) ((a) < (b) ? (a) : (b)) ^ ``` The problem is arithmetic on void*, so explicitly cast to the wanted types. Change-Id: I85bee25a69c432ef8bb934add7fd2e2e31f03662 Signed-off-by: Paul Menzel <paulepanter(a)users.sourceforge.net> --- src/commonlib/lz4_wrapper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/commonlib/lz4_wrapper.c b/src/commonlib/lz4_wrapper.c index ea7b90d..1a765af 100644 --- a/src/commonlib/lz4_wrapper.c +++ b/src/commonlib/lz4_wrapper.c @@ -161,7 +161,7 @@ size_t ulz4fn(const void *src, size_t srcn, void *dst, size_t dstn) } if (b.not_compressed) { - size_t size = MIN((uint32_t)b.size, dst + dstn - out); + size_t size = MIN((uintptr_t)b.size, (uintptr_t)dst + dstn - (uintptr_t)out); memcpy(out, in, size); if (size < b.size) break; /* output overrun */

1 0

Patch set updated for coreboot: u-boot: Make sure targets aren't duplicated
by Martin Roth 23 Feb '16

23 Feb '16

Martin Roth (martinroth(a)google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/13767 -gerrit commit 773351b0afac7ee18fe7d77f08765302209c9b6a Author: Martin Roth <martinroth(a)google.com> Date: Mon Feb 22 14:03:13 2016 -0800 u-boot: Make sure targets aren't duplicated When U-Boot isn't selected as a payload, two of the targets: $(project_dir): and $(project_dir)/$(TAG-y) evaluated to the same value, generating a make warning when running a clean. By adding additional text to the file that is created, this is avoided. Gets rid of these warnings: Makefile.inc:54: warning: overriding commands for target `u-boot' Makefile.inc:37: warning: ignoring old commands for target `u-boot' Change-Id: I4b4df753612b674b3ccde2a757338840be92d1f2 Signed-off-by: Martin Roth <martinroth(a)google.com> --- payloads/external/U-Boot/Makefile.inc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/payloads/external/U-Boot/Makefile.inc b/payloads/external/U-Boot/Makefile.inc index f0abee1..255db29 100644 --- a/payloads/external/U-Boot/Makefile.inc +++ b/payloads/external/U-Boot/Makefile.inc @@ -50,7 +50,7 @@ else touch $(project_dir)/$(STABLE_COMMIT_ID) endif -$(project_dir)/$(TAG-y): fetch +$(project_dir)/tag-$(TAG-y): fetch echo " Checking out $(project_name) revision $(TAG-y)" cd $(project_dir); git checkout master; git branch -D coreboot 2>/dev/null; git checkout -b coreboot $(TAG-y) @@ -58,7 +58,7 @@ config: $(project_dir)/$(TAG-y) rm -f $(project_config_file) ifneq ($(CONFIG_PAYLOAD_CONFIGFILE),) ifneq ("$(wildcard $(CONFIG_PAYLOAD_CONFIGFILE))","") - cat $(CONFIG_PAYLOAD_CONFIGFILE)" > $(project_config_file) + cat $(CONFIG_PAYLOAD_CONFIGFILE)" > tag-$(project_config_file) else echo "Error: File $(CONFIG_PAYLOAD_CONFIGFILE) does not exist" false

1 0

Patch set updated for coreboot: kconfig_lint: Fix checks when running in taint mode
by Martin Roth 23 Feb '16

23 Feb '16

Martin Roth (martinroth(a)google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/13751 -gerrit commit c77698ee1c5a8d46e06a6d2b6d2a9bc9399565d5 Author: Martin Roth <martinroth(a)google.com> Date: Fri Feb 19 10:24:25 2016 -0700 kconfig_lint: Fix checks when running in taint mode The builders run perl scripts in taint mode, and some of the checks that the kconfig lint script were running were tainted, causing the script to terminate early when running on the servers. This checks to see if taint mode is enabled, and untaints the path if it is. All external tools (git & grep) must be in /bin, /usr/bin, or /usr/local/bin. This also removes the check for unused kconfig files if taint mode is enabled. Change-Id: I8d1e1c32275f759d085759fb5d8a6c85d4f99539 Signed-off-by: Martin Roth <martinroth(a)google.com> --- util/lint/kconfig_lint | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/util/lint/kconfig_lint b/util/lint/kconfig_lint index b5a62d3..38fa968 100755 --- a/util/lint/kconfig_lint +++ b/util/lint/kconfig_lint @@ -26,6 +26,12 @@ use File::Find; use Getopt::Long; use Getopt::Std; +# If taint mode is enabled, Untaint the path - git and grep must be in /bin, /usr/bin or /usr/local/bin +if ( ${^TAINT} ) { + $ENV{'PATH'} = '/bin:/usr/bin:/usr/local/bin'; + delete @ENV{ 'IFS', 'CDPATH', 'ENV', 'BASH_ENV' }; +} + my $suppress_error_output = 0; # flag to prevent error text my $suppress_warning_output = 0; # flag to prevent warning text my $show_note_output = 0; # flag to show minor notes text @@ -33,7 +39,7 @@ my $print_full_output = 0; # flag to print wholeconfig output my $output_file = "-"; # filename of output - set stdout by default my $dont_use_git_grep = 0; -#globals +# Globals my $top_dir = "."; # Directory where Kconfig is run my $root_dir = "src"; # Directory of the top level Kconfig file my $errors_found = 0; # count of errors @@ -76,9 +82,6 @@ sub Main { #load the Kconfig tree, checking what we can and building up all the hash tables build_and_parse_kconfig_tree("$root_dir/Kconfig"); - #run checks based on the data that was found - find( \&check_if_file_referenced, $root_dir ); - load_config($config_file) if ($config_file); check_defaults(); @@ -91,6 +94,14 @@ sub Main { check_is_enabled(); check_selected_symbols(); + # Run checks based on the data that was found + if ( ( !$suppress_warning_output ) && ( ${^TAINT} == 0 ) ) { + + # The find function is tainted - only run it if taint checking + # is disabled and warnings are enabled. + find( \&check_if_file_referenced, $root_dir ); + } + print_wholeconfig(); if ($errors_found) {

1 0

Patch merged into coreboot/master: xcompile: Add parameter to aid in debugging
by gerrit＠coreboot.org 23 Feb '16

23 Feb '16

the following patch was just integrated into master: commit d08eb062df653735778306cba5f8cfbe3a9dd740 Author: Martin Roth <martinroth(a)google.com> Date: Tue Jan 19 08:50:23 2016 -0700 xcompile: Add parameter to aid in debugging There was a report that xcompile wasn't finding the compilers correctly, so to aid in future debugging, this adds a parameter to show what xcompile is doing as it runs. Run from the command line: ./util/xcompile/xcompile --debug Change-Id: I779cb3de7b4e3f62a2ef2a6245c3538be518870c Signed-off-by: Martin Roth <martinroth(a)google.com> Reviewed-on: https://review.coreboot.org/13047 Tested-by: build bot (Jenkins) Reviewed-by: Patrick Georgi <pgeorgi(a)google.com> Reviewed-by: Paul Menzel <paulepanter(a)users.sourceforge.net> See https://review.coreboot.org/13047 for details. -gerrit

1 0

Patch set updated for coreboot: payloads: Load coreinfo as a secondary payload
by Martin Roth 23 Feb '16

23 Feb '16

Martin Roth (martinroth(a)google.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/13728 -gerrit commit 4f421e19aa191a955c0bf2eed6b6aec4c09247fa Author: Martin Roth <martinroth(a)google.com> Date: Tue Feb 16 19:40:47 2016 -0700 payloads: Load coreinfo as a secondary payload This allows coreinfo to be added to CBFS as a 'secondary' payload on x86 systems, to be loaded by the main payload if desired. Selecting this option, which defaults to no, builds the coreinfo payload and adds it to CBFS as `img/coreinfo` which can then be loaded by for example SeaBIOS or GRUB. Change-Id: I52661d486823bc4bb215ce92dca118c9d2c2a309 Signed-off-by: Martin Roth <martinroth(a)google.com> --- Makefile.inc | 2 +- payloads/Kconfig | 9 ++++++++- payloads/Makefile.inc | 21 +++++++++++++++++++++ 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/Makefile.inc b/Makefile.inc index 1889128..68012d9 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -74,7 +74,7 @@ subdirs-y += util/cbfstool util/sconfig util/nvramtool util/broadcom subdirs-y += util/futility util/marvell subdirs-y += $(wildcard src/arch/*) subdirs-y += src/mainboard/$(MAINBOARDDIR) -subdirs-y += payloads/external +subdirs-y += payloads payloads/external subdirs-y += site-local diff --git a/payloads/Kconfig b/payloads/Kconfig index 9037b30..01bd216 100644 --- a/payloads/Kconfig +++ b/payloads/Kconfig @@ -59,5 +59,12 @@ config PAYLOAD_IS_FLAT_BINARY Add the payload to cbfs as a flat binary type instead of as an elf payload -endmenu +config COREINFO_SECONDARY_PAYLOAD + bool "Load coreinfo as a secondary payload" + default n + depends on ARCH_X86 + help + coreinfo can be loaded as a secondary payload under SeaBIOS, GRUB, + or any other payload that can load additional payloads. +endmenu diff --git a/payloads/Makefile.inc b/payloads/Makefile.inc new file mode 100644 index 0000000..1bd8cf7 --- /dev/null +++ b/payloads/Makefile.inc @@ -0,0 +1,21 @@ +## +## This file is part of the coreboot project. +## +## Copyright (C) 2016 Google Inc. +## +## This program is free software; you can redistribute it and/or modify +## it under the terms of the GNU General Public License as published by +## the Free Software Foundation; version 2 of the License. +## +## This program is distributed in the hope that it will be useful, +## but WITHOUT ANY WARRANTY; without even the implied warranty of +## MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +## GNU General Public License for more details. +## + +cbfs-files-$(CONFIG_COREINFO_SECONDARY_PAYLOAD) += img/coreinfo +img/coreinfo-file := payloads/coreinfo/build/coreinfo.elf +img/coreinfo-type := payload + +payloads/coreinfo/build/coreinfo.elf coreinfo: + $(MAKE) -C payloads/coreinfo defaultbuild

1 0

Patch merged into coreboot/master: cbfstool: Silence LZ4 -Wsign-compare warnings
by gerrit＠coreboot.org 23 Feb '16

23 Feb '16

the following patch was just integrated into master: commit 17cb0370a70ccfc2301b7974bf38d44c7271afea Author: Julius Werner <jwerner(a)chromium.org> Date: Mon Feb 22 15:30:37 2016 -0800 cbfstool: Silence LZ4 -Wsign-compare warnings It seems that the exact behavior of -Wsign-compare changes between GCC versions... some of them like the commonlib/lz4_wrapper.c code, and some don't. Since we don't have a well-defined HOSTCC toolchain this slipped through pre-commit testing. Explicitly silence the warning to ensure cbfstool still builds on all systems. Change-Id: I43f951301d3f14ce34dadbe58e885b82d21d6353 Signed-off-by: Julius Werner <jwerner(a)chromium.org> Reviewed-on: https://review.coreboot.org/13769 Tested-by: build bot (Jenkins) Reviewed-by: Aaron Durbin <adurbin(a)chromium.org> See https://review.coreboot.org/13769 for details. -gerrit

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2016