coreboot-gerrit June 2023

coreboot-gerrit@coreboot.org

1 participants
5387 discussions

[XS] Change in coreboot[master]: Update qc_blobs submodule to upstream master
by Felix Singer (Code Review) 12 Jun '23

12 Jun '23

Felix Singer has submitted this change. ( https://review.coreboot.org/c/coreboot/+/75795?usp=email ) Change subject: Update qc_blobs submodule to upstream master ...................................................................... Update qc_blobs submodule to upstream master Updating from commit id 33cc4f2: 2022-10-26 14:21:20 +0530 - (sc7280/qtiseclib: Update qtiseclib blobs binaries and release notes from 63 to 69) to commit id a252198: 2023-05-23 11:00:31 +0000 - (sc7180/boot: Update qclib blobs binaries from 50 to 55) This brings in 4 new commits: a252198 sc7180/boot: Update qclib blobs binaries from 50 to 55 3fbd986 sc7180/qtiseclib: Update qtiseclib blobs binaries and release notes from 50 to 69 7a3f064 sc7280/boot,shrm: Update qclib blobs binaries from 35 to 52 9884189 sc7280: Update AOP firmware to version 454 Change-Id: I938b768318d31d5e105d7c98823947cf8c02b195 Signed-off-by: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/75795 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Reviewed-by: Elyes Haouas <ehaouas(a)noos.fr> --- M 3rdparty/qc_blobs 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Felix Singer: Looks good to me, approved build bot (Jenkins): Verified Elyes Haouas: Looks good to me, approved diff --git a/3rdparty/qc_blobs b/3rdparty/qc_blobs index 33cc4f2..a252198 160000 --- a/3rdparty/qc_blobs +++ b/3rdparty/qc_blobs @@ -1 +1 @@ -Subproject commit 33cc4f2fd8d9529c4231564d7a30a00c06b213de +Subproject commit a252198ec6544e13904cfe831cec3e784aaa715d -- To view, visit https://review.coreboot.org/c/coreboot/+/75795?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I938b768318d31d5e105d7c98823947cf8c02b195 Gerrit-Change-Number: 75795 Gerrit-PatchSet: 3 Gerrit-Owner: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in coreboot[master]: arch/x86/tables.c: Drop placing ACPI and SMBIOS in lower memory
by Maximilian Brune (Code Review) 11 Jun '23

11 Jun '23

Attention is currently required from: Arthur Heymans, Benjamin Doron, Nico Huber, Paul Menzel. Maximilian Brune has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75712?usp=email ) Change subject: arch/x86/tables.c: Drop placing ACPI and SMBIOS in lower memory ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/75712?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I216e7c713d112ad1012b0beaf26196967988b951 Gerrit-Change-Number: 75712 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Sun, 11 Jun 2023 21:32:32 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: mb/google/skyrim: Use CMOS bit to toggle ABL WA for Hynix DRAM
by Matt DeVillier (Code Review) 11 Jun '23

11 Jun '23

Attention is currently required from: Felix Held, Jason Glenesk, Jason Nien, Karthik Ramasubramanian, Martin Roth, Paul Menzel. Matt DeVillier has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75698?usp=email ) Change subject: mb/google/skyrim: Use CMOS bit to toggle ABL WA for Hynix DRAM ...................................................................... Patch Set 3: (5 comments) File src/mainboard/google/skyrim/bootblock.c: https://review.coreboot.org/c/coreboot/+/75698/comment/c93a78b6_aa7874d8 : PS2, Line 17: 18 > Agreed - I was more thinking about creating the cbi_part_number array though. I'm using the full length allowed the SPD spec for the cbi_part_number array, which is 33 bytes, so no concern there https://review.coreboot.org/c/coreboot/+/75698/comment/188732d8_ae32f282 : PS2, Line 43: /* If problematic Hynix module, ensure bit set */ > I think if we're worried about it, we should get rid of most of the log messages. […] they're all filtered on SPEW level already, so do we really care if there are too many? https://review.coreboot.org/c/coreboot/+/75698/comment/1253eae1_4627e8ea : PS2, Line 94: hynix_dram_cmos_check(); > YAGNI. […] I'm perfectly happy to just restict this to just FF, adding another Kconfig seems unnecessary File src/mainboard/google/skyrim/bootblock.c: https://review.coreboot.org/c/coreboot/+/75698/comment/558ffd93_082cdaf7 : PS3, Line 32: google_chromeec_cbi_get_dram_part_num > Part of the reason I was asking for the termination byte for the string is just because I don't have […] since it's passed directly to the EC via a host cmd, it appears it would overflow if too short. cbi_get_string() ensures it's null-terminated https://review.coreboot.org/c/coreboot/+/75698/comment/21d6f62f_2c9d2d4c : PS3, Line 35: return; > Should we clear the cmos bit in this case? Can we assume that if there's an error here, the bad hyni […] I think there are reasonable arguments for leaving it and for clearing it. I don't have a preference either way -- To view, visit https://review.coreboot.org/c/coreboot/+/75698?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibb6e145f6cdba7270e0a322ef414bf1cb09c5eaa Gerrit-Change-Number: 75698 Gerrit-PatchSet: 3 Gerrit-Owner: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)amd.corp-partner.google.com> Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)amd.corp-partner.google.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Sun, 11 Jun 2023 21:14:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Comment-In-Reply-To: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: [WIP] util/abuild: Add parser for statistics
by Kyösti Mälkki (Code Review) 11 Jun '23

11 Jun '23

Attention is currently required from: Martin L Roth. Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/75804?usp=email ) Change subject: [WIP] util/abuild: Add parser for statistics ...................................................................... [WIP] util/abuild: Add parser for statistics Change-Id: I01b976bfffc32d2b2835c697403a29f3630e6ad5 Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- A util/abuild/showstats.py 1 file changed, 242 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/75804/1 diff --git a/util/abuild/showstats.py b/util/abuild/showstats.py new file mode 100755 index 0000000..4aa0c39 --- /dev/null +++ b/util/abuild/showstats.py @@ -0,0 +1,242 @@ +#! /usr/bin/python3 + +import os +import re +import subprocess +import tarfile +import tempfile + +debug = 0 + +abuild_timestamps_name ='abuild.timestamps' +abuild_timestamps_pattern = re.compile("TS_(\d+) (\d+)$") + +ccache_bin = os.getenv("CCACHE", default="ccache") +ccache_statslog_name = 'ccache.stats' +ccache_statslog_pattern = re.compile(" *(.*): +(\d*) / (\d*) $.*$$") + +def print_hit_ratio(a, b): + ratio = float(a) / float(b) * 100 + print(f'{a:d} {b} ({ratio:2.2f})') + +def ccache_hit_rates(ccache): + totals = 0 + rates = [[]] * len(ccache) + rates_str = [[]] * len(ccache) + + for i in range(0, len(ccache)): + totals += ccache[i] + if totals == 0: + return -1 + + for i in range(0, len(ccache)): + rates[i] = float(ccache[i]) / float(totals) * 100 + rates_str[i] = f'{rates[i]:2.2f}%' + + return rates_str + +def parse_ccache_log_stats(content): + state = 0 + for line in content.splitlines(): + + parsed = ccache_statslog_pattern.match(line) + if not parsed: + continue + + if debug: + print(parsed.groups()) + + title = parsed.groups()[0] + value = (int(parsed.groups()[1]), int(parsed.groups()[2])) + + if title == "Cacheable calls": + cacheable_1 = value[0] + total_calls_1 = value[1] + state = 1 + continue + + if title == "Hits": + if state == 1: + hits_1 = value[0] + cacheable_2 = value[1] + state = 2 + continue + + if title == "Direct": + if state == 2: + direct = value[0] + hits_2 = value[1] + continue + + if title == "Preprocessed": + if state == 2: + preprocessed = value[0] + hits_3 = value[1] + continue + + if title == "Misses": + misses = value[0] + cacheable_3 = value[1] + state = 3 + continue + + if title == "Uncacheable calls": + uncacheable = value[0] + total_calls_2 = value[1] + state = 4 + continue + + # Sanity check input + if cacheable_1 != cacheable_2 or cacheable_2 != cacheable_3: + return -1 + if hits_1 != hits_2 or hits_2 != hits_3: + return -1 + if total_calls_1 != total_calls_2: + return -1 + if preprocessed + direct != hits_1: + return -1 + if misses + hits_1 != cacheable_1: + return -1 + if uncacheable + cacheable_1 != total_calls_1: + return -1 + + if debug: + print_hit_ratio(uncacheable, total_calls_1) + print_hit_ratio(cacheable_1, total_calls_1) + print_hit_ratio(misses, cacheable_1) + print_hit_ratio(preprocessed, hits_1) + print_hit_ratio(direct, hits_1) + + return [uncacheable, misses, preprocessed, direct] + + +def extract_ccache_stats(tar, orig_dir, board): + path = orig_dir + '/' + board + '/' + ccache_statslog_name + + f = tar.extractfile(path) + ccache_tmp_statslog = tempfile.NamedTemporaryFile() + ccache_tmp_statslog.write(f.read()) + f.close() + + if debug: + print(ccache_tmp_statslog.name) + + # ccache takes the filename in environment var + ccache_env = {**os.environ, 'CCACHE_STATSLOG': ccache_tmp_statslog.name} + log = subprocess.run([ccache_bin, "--show-log-stats"], capture_output=True, \ + text=True, env=ccache_env) + + ccache_tmp_statslog.close() + + return parse_ccache_log_stats(log.stdout) + + +def print_elapsed(usecs): + minutes = usecs // (60 * 1000 * 1000) + usecs = usecs % (60 * 1000 * 1000) + seconds = usecs / (1000 * 1000) + + if minutes > 0 : + elapsed=f'{minutes} min {seconds:2.0f} sec' + else: + elapsed=f'{seconds:2.3f} seconds' + + return elapsed + +def parse_timestamps(content): + ts = [[]] * 3 + elapsed_us = [[]] * 2 + + for line in content.splitlines(): + + parsed = abuild_timestamps_pattern.match(line) + if not parsed: + continue + + idx = int(parsed.groups()[0]) + ts[idx] = int(parsed.groups()[1]) + + elapsed_us[0] = ts[1] - ts[0] + elapsed_us[1] = ts[2] - ts[1] + + if debug: + print(f'{elapsed_us}') + + return elapsed_us + + +def extract_timestamps(tar, orig_dir, board): + path = orig_dir + '/' + board + '/' + abuild_timestamps_name + f = tar.extractfile(path) + content = f.read().decode() + f.close() + if debug: + print(content) + return parse_timestamps(content) + + +sum_of_ccache = None +sum_of_ts = None + +def parse_stats_from_tar(tar, orig_dir, board): + global sum_of_ccache + global sum_of_ts + + timestamps = extract_timestamps(tar, orig_dir, board) + ccache = extract_ccache_stats(tar, orig_dir, board) + + if (timestamps != -1): + if sum_of_ts == None: + sum_of_ts = timestamps + else: + for i in range(0, len(sum_of_ts)): + sum_of_ts[i] += timestamps[i] + + if (ccache != -1): + if sum_of_ccache == None: + sum_of_ccache = ccache + else: + for i in range(0, len(sum_of_ccache)): + sum_of_ccache[i] += ccache[i] + + if (timestamps != -1) and (ccache != -1): + elapsed = print_elapsed(timestamps[1]) + hits = ccache_hit_rates(ccache) + print(f'{board} ts: {timestamps} ccache: {ccache} {hits} {elapsed}') + elif (timestamps != -1): + elapsed = print_elapsed(timestamps[1]) + print(f'{board} ts: {timestamps} ccache-disabled: {elapsed}') + + +def collect_stats(statistics_tar): + boards = 0 + stats_files_pattern = re.compile("(.+)/(.+)/(.*)$") + + tar = tarfile.open(statistics_tar, 'r') + for line in tar.getnames(): + parsed = stats_files_pattern.match(line); + if not parsed: + continue + if debug: + print(parsed.groups()) + + orig_dir = parsed.groups()[0] + board = parsed.groups()[1] + basename = parsed.groups()[2] + if basename == abuild_timestamps_name: + parse_stats_from_tar(tar, orig_dir, board) + boards += 1 + + tar.close() + + cfg_total = print_elapsed(sum_of_ts[0]) + cfg_avg = print_elapsed(sum_of_ts[0] / boards) + print(f'Total (config): {cfg_total} avg: {cfg_avg}') + + ccache_total = print_elapsed(sum_of_ts[1]) + ccache_avg = print_elapsed(sum_of_ts[1] / boards) + hits = ccache_hit_rates(sum_of_ccache) + print(f'Total (ccache): {sum_of_ccache} {hits} {ccache_total}') + + +collect_stats('coreboot-builds/statistics.tar') -- To view, visit https://review.coreboot.org/c/coreboot/+/75804?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I01b976bfffc32d2b2835c697403a29f3630e6ad5 Gerrit-Change-Number: 75804 Gerrit-PatchSet: 1 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[master]: util/abuild: Add per-build statistics tarfile
by Kyösti Mälkki (Code Review) 11 Jun '23

11 Jun '23

Attention is currently required from: Martin L Roth. Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/75803?usp=email ) Change subject: util/abuild: Add per-build statistics tarfile ...................................................................... util/abuild: Add per-build statistics tarfile Change-Id: Icb9a5bdf94013fe493dc8ec634cf3094bcff2838 Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- M util/abuild/abuild 1 file changed, 19 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/75803/1 diff --git a/util/abuild/abuild b/util/abuild/abuild index fb2fbaf..3b527ec 100755 --- a/util/abuild/abuild +++ b/util/abuild/abuild @@ -420,9 +420,15 @@ cd "${build_dir}" || return $? + timestamps="abuild.timestamps" + printf "Build started %s\n" "${ts_basetime_str}" > "${timestamps}" + printf "BASETIME_SECONDS %d\n" $ts_exec_shell >> "${timestamps}" + printf "TS_0 %d\n" $ts_0 >> "${timestamps}" + printf "TS_1 %d\n" $ts_1 >> "${timestamps}" + printf "TS_2 %d\n" $ts_2 >> "${timestamps}" + duration=$(ts_delta_seconds $ts_0 $ts_2) duration_str=$(ts_delta_string $ts_0 $ts_2) - junit " <testcase classname='${TESTRUN}${testclass/#/.}' name='$BUILD_NAME' time='$duration' >" if [ $MAKE_FAILED -eq 0 ]; then @@ -452,6 +458,13 @@ sort "${build_dir}/config.h" | grep CONFIG_ > "${build_dir}/config.h.sorted" sha256sum "${build_dir}/config.h.sorted" >> "${checksum_file}_config" fi + + stats_files="${build_dir}/${timestamps}" + if [ -f ${build_dir}/ccache.stats ]; then + stats_files="${stats_files} ${build_dir}/ccache.stats" + fi + flock -F -w 0.1 $TARGET/.statslock tar -rf ${stats_archive} ${stats_files} + if [ "$clean_work" = "true" ]; then rm -rf "${build_dir}" fi @@ -914,6 +927,8 @@ FAILED_BOARDS="$(realpath ${TARGET}/failed_boards)" PASSED_BOARDS="$(realpath ${TARGET}/passing_boards)" +stats_archive="$TARGET/statistics.tar" + # Generate a single xcompile for all boards export xcompile="${TARGET}/xcompile" @@ -921,6 +936,9 @@ rm -f "${xcompile}" $MAKE -C"${ROOT}" obj="$TARGET/temp" objutil="$TARGET/sharedutils" UPDATED_SUBMODULES=1 "${xcompile}" || exit 1 rm -f "$FAILED_BOARDS" "$PASSED_BOARDS" + + # Initialize empty statistics archive + tar -cf "${stats_archive}" "${xcompile}" fi USE_XARGS=0 -- To view, visit https://review.coreboot.org/c/coreboot/+/75803?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Icb9a5bdf94013fe493dc8ec634cf3094bcff2838 Gerrit-Change-Number: 75803 Gerrit-PatchSet: 1 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-MessageType: newchange

1 0

[M] Change in coreboot[master]: util/abuild: Improve elapsed time measurement
by Kyösti Mälkki (Code Review) 11 Jun '23

11 Jun '23

Kyösti Mälkki has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/75802?usp=email ) Change subject: util/abuild: Improve elapsed time measurement ...................................................................... util/abuild: Improve elapsed time measurement Time elapsed for a single board build with ccache typically measures well below 10 seconds. Improve the measurements to milliseconds resolution using bash EPOCHREALTIME (pseudo) environment variable. Change-Id: Iaedc470bb45cf9bb6f14ff8b37cd6f7ae3818a08 Signed-off-by: Kyösti Mälkki <kyosti.malkki(a)gmail.com> --- M util/abuild/abuild 1 file changed, 54 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/02/75802/1 diff --git a/util/abuild/abuild b/util/abuild/abuild index d103fce..fb2fbaf 100755 --- a/util/abuild/abuild +++ b/util/abuild/abuild @@ -94,6 +94,10 @@ # Skip builds with this Kconfig value notset skipconfig_unset="" +# EPOCHTIME +ts_exec_shell=$(printf "%(%s)T" -2) +ts_basetime_str=$(date -u --date=@${ts_exec_shell}) + trap interrupt INT function interrupt @@ -364,6 +368,42 @@ return 0 } +# Counting microseconds since start of shell +function add_timestamp +{ + local now=${EPOCHREALTIME} + local seconds=$(echo $now | cut -f 1 -d '.') + local usecs=$(echo $now | cut -f 2 -d '.') + seconds=$(( seconds - ts_exec_shell )) + usecs=$(( seconds * 1000 * 1000 + 10#$usecs )) + printf "%s" $usecs +} + +function ts_delta_seconds +{ + local delta=$(( ($2 - $1) / (1000 * 1000) )) + printf "%s" $delta +} + +function ts_delta_string +{ + local ts_minutes + local ts_seconds + local delta + + delta=$(( ($2 - $1) / 1000 )) + ts_minutes=$(( delta / (60 * 1000) )) + delta=$(( delta % (60 * 1000) )) + ts_seconds=$(( delta / 1000)) + delta=$(( delta % 1000 )) + + if [ $ts_minutes -ne 0 ] ; then + printf "%d min %d sec" $ts_minutes $ts_seconds + else + printf "%d.%03d seconds" $ts_seconds $delta + fi +} + function compile_target { local BUILD_NAME=$1 @@ -371,14 +411,18 @@ if [ "$quiet" == "false" ]; then echo " Compiling $MAINBOARD image$cpuconfig..."; fi CURR=$( pwd ) - #stime=`perl -e 'print time();' 2>/dev/null || date +%s` + + ts_1=$(add_timestamp) eval "$BUILDPREFIX" "$MAKE" "$verboseopt" DOTCONFIG="${build_dir}/config.build" obj="${build_dir}" objutil="$TARGET/sharedutils" BUILD_TIMELESS=$TIMELESS \ &> "${build_dir}/make.log" ; \ MAKE_FAILED=$? + ts_2=$(add_timestamp) + cd "${build_dir}" || return $? - etime=$(perl -e 'print time();' 2>/dev/null || date +%s) - duration=$(( etime - stime )) + duration=$(ts_delta_seconds $ts_0 $ts_2) + duration_str=$(ts_delta_string $ts_0 $ts_2) + junit " <testcase classname='${TESTRUN}${testclass/#/.}' name='$BUILD_NAME' time='$duration' >" if [ $MAKE_FAILED -eq 0 ]; then @@ -386,14 +430,14 @@ junitfile make.log junit "</system-out>" printf "ok\n" > compile.status - printf "%s built successfully. (took %ss)\n" "$BUILD_NAME" "${duration}" + printf "%s built successfully. (took %s)\n" "$BUILD_NAME" "${duration_str}" echo "$BUILD_NAME" >> "$PASSED_BOARDS" else junit "<failure type='BuildFailed'>" junitfile make.log junit "</failure>" printf "failed\n" > compile.status - printf "%s build FAILED after %ss!\nLog excerpt:\n" "$BUILD_NAME" "${duration}" + printf "%s build FAILED after %s!\nLog excerpt:\n" "$BUILD_NAME" "${duration_str}" tail -n $CONTEXT make.log 2> /dev/null || tail -$CONTEXT make.log if [ "$clean_work" = "true" ]; then echo "$BUILD_NAME" >> "$FAILED_BOARDS" @@ -448,7 +492,8 @@ XMLFILE="$ABSPATH/${BUILD_NAME}.xml" rm -f "${XMLFILE}" - stime=$(perl -e 'print time();' 2>/dev/null || date +%s) + ts_0=$(add_timestamp) + create_buildenv "$BUILD_NAME" "$build_dir" "$config_file" local BUILDENV_CREATED=$? @@ -925,11 +970,11 @@ ABSPATH="$(cd "$TARGET/abuild" && pwd)" local XMLFILE="$ABSPATH/__util.xml" rm -f "${XMLFILE}" - stime=$(perl -e 'print time();' 2>/dev/null || date +%s) + stime=$(add_timestamp) $BUILDPREFIX "$MAKE" -j "$cpus" DOTCONFIG="$TMPCFG" obj="$TARGET/temp" objutil="$TARGET/sharedutils" tools > "$TARGET/sharedutils/make.log" 2>&1 local ret=$? - etime=$(perl -e 'print time();' 2>/dev/null || date +%s) - local duration=$(( etime - stime )) + etime=$(add_timestamp) + local duration=$(ts_delta_seconds $stime $etime) junit " <testcase classname='util' name='all' time='$duration' >" if [ $ret -eq 0 ]; then -- To view, visit https://review.coreboot.org/c/coreboot/+/75802?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iaedc470bb45cf9bb6f14ff8b37cd6f7ae3818a08 Gerrit-Change-Number: 75802 Gerrit-PatchSet: 1 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: newchange

1 0

[M] Change in coreboot[master]: mb/google/skyrim: Use CMOS bit to toggle ABL WA for Hynix DRAM
by Martin Roth (Code Review) 11 Jun '23

11 Jun '23

Attention is currently required from: Felix Held, Jason Glenesk, Jason Nien, Karthik Ramasubramanian, Matt DeVillier, Paul Menzel. Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75698?usp=email ) Change subject: mb/google/skyrim: Use CMOS bit to toggle ABL WA for Hynix DRAM ...................................................................... Patch Set 3: (6 comments) File src/mainboard/google/skyrim/bootblock.c: https://review.coreboot.org/c/coreboot/+/75698/comment/8412a6a3_bbfd6d1b : PS2, Line 17: 18 > I'm not sure that matters for strncmp Agreed - I was more thinking about creating the cbi_part_number array though. https://review.coreboot.org/c/coreboot/+/75698/comment/788154f9_d0a6d28d : PS2, Line 41: strncmp > we don't seem to have the case-insensitive version in coreboot currently Acknowledged https://review.coreboot.org/c/coreboot/+/75698/comment/00521551_8a41fb3e : PS2, Line 43: /* If problematic Hynix module, ensure bit set */ > Most of the comments could be dropped, as they are redundant to the code or the same thing is set by […] I think if we're worried about it, we should get rid of most of the log messages. If we want to know the DRAM part number, this isn't the right place to print it - that'd be in something that's run on every chromeos platform that stores the part number in CBI. Other than that, the only log message we really need is when the bit is set or cleared - the no-change messages are implied if you don't see one of the others. https://review.coreboot.org/c/coreboot/+/75698/comment/32fb858a_973d7f09 : PS2, Line 94: hynix_dram_cmos_check(); > Karthik requested that we not restrict to FF, in case another variant picked up the same memory part […] YAGNI. So let's add a Kconfig value stating whether to check or not. It's a waste of time to check whether the system is using that memory on boards that simply don't use it. We can use that same Kconfig to see which RAMID that value is on if it's supported. -1 = don't check. 0-15 = ram ID to check Something like this example code: ``` config CHECK_FOR_H9JCNNNCP3MLYR-N6E int default 2 if BOARD_FROSTFLOW default -1 help Enable checking for H9JCNNNCP3MLYR-N6E DRAM ``` If there's still concern, we can add a makefile function that greps through each boards' memory config for the faulty memory and throws a build-time error if it's there, but this value is set to -1. File src/mainboard/google/skyrim/bootblock.c: https://review.coreboot.org/c/coreboot/+/75698/comment/8a7de190_d7af776f : PS3, Line 32: google_chromeec_cbi_get_dram_part_num Part of the reason I was asking for the termination byte for the string is just because I don't have a good idea of what this function does. Does this truncate the part number, or return an error if the supplied buffer isn't large enough? Is the termination part of the size or not? This seems like a function where a pointer should be returned instead of having the calling function make a buffer. It's very ambiguous this way. https://review.coreboot.org/c/coreboot/+/75698/comment/c3031c59_c71cf37e : PS3, Line 35: return; Should we clear the cmos bit in this case? Can we assume that if there's an error here, the bad hynix memory is not being used? Clearing it seems better than leaving it in an unknown state. -- To view, visit https://review.coreboot.org/c/coreboot/+/75698?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibb6e145f6cdba7270e0a322ef414bf1cb09c5eaa Gerrit-Change-Number: 75698 Gerrit-PatchSet: 3 Gerrit-Owner: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)amd.corp-partner.google.com> Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)amd.corp-partner.google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Sun, 11 Jun 2023 20:57:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Comment-In-Reply-To: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[master]: Switch gitconfig.sh over to use main branch
by Felix Singer (Code Review) 11 Jun '23

11 Jun '23

Attention is currently required from: Stefan Reinauer. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75783?usp=email ) Change subject: Switch gitconfig.sh over to use main branch ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/75783?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iea1a7e61b60c4bf04be2fed9c503eaf7e20fe462 Gerrit-Change-Number: 75783 Gerrit-PatchSet: 1 Gerrit-Owner: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Sun, 11 Jun 2023 20:53:29 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[master]: Switch scripts over to use main branch
by Felix Singer (Code Review) 11 Jun '23

11 Jun '23

Attention is currently required from: Stefan Reinauer. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75782?usp=email ) Change subject: Switch scripts over to use main branch ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/75782?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I90fadf2352d56074ce8b58d559a73b0c53fac14b Gerrit-Change-Number: 75782 Gerrit-PatchSet: 1 Gerrit-Owner: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Sun, 11 Jun 2023 20:52:38 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[master]: Switch jenkins node over to use encapsulate main branch
by Felix Singer (Code Review) 11 Jun '23

11 Jun '23

Attention is currently required from: Stefan Reinauer. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75781?usp=email ) Change subject: Switch jenkins node over to use encapsulate main branch ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/75781?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I493acb4de615508b08826f814ef6ac1b37cbdf0c Gerrit-Change-Number: 75781 Gerrit-PatchSet: 1 Gerrit-Owner: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Sun, 11 Jun 2023 20:50:55 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit June 2023