coreboot-gerrit March 2018

coreboot-gerrit@coreboot.org

1 participants
1399 discussions

Change in coreboot[master]: x86: Increase time out for parking APs to 250ms
by build bot (Jenkins) (Code Review) 29 Mar '18

29 Mar '18

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/25422 ) Change subject: x86: Increase time out for parking APs to 250ms ...................................................................... Patch Set 1: Verified+1 Build Successful https://qa.coreboot.org/job/coreboot-gerrit/69184/ : SUCCESS https://qa.coreboot.org/job/coreboot-checkpatch/23533/ : SUCCESS -- To view, visit https://review.coreboot.org/25422 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id3e59db4fe7a5a2fb60357b05565bba89be1e00e Gerrit-Change-Number: 25422 Gerrit-PatchSet: 1 Gerrit-Owner: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 29 Mar 2018 07:23:00 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in coreboot[master]: x86: Increase time out for parking APs to 250ms
by Furquan Shaikh (Code Review) 29 Mar '18

29 Mar '18

Furquan Shaikh has uploaded this change for review. ( https://review.coreboot.org/25422 Change subject: x86: Increase time out for parking APs to 250ms ...................................................................... x86: Increase time out for parking APs to 250ms Change f43adf0 (intel/common/block/cpu: Change post_cpus_init after BS_DEV RESOURCES) moved post_cpus_init to BS_OS_RESUME for S3 path. This results in BSP timing out waiting for APs to be parked. This change increases the time out value for APs to be parked to 250ms. This value was chosen after running suspend-resume stress test and capturing the maximum time taken for APs to be parked for 100 iterations. Typical values observed were ~150ms. Maximum value observed was 152ms. BUG=b:76442753 TEST=Verified for 100 iterations that suspend-resume does not run into any AP park time out. Change-Id: Id3e59db4fe7a5a2fb60357b05565bba89be1e00e Signed-off-by: Furquan Shaikh <furquan(a)google.com> --- M src/arch/x86/cpu.c M src/cpu/x86/mp_init.c 2 files changed, 19 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/22/25422/1 diff --git a/src/arch/x86/cpu.c b/src/arch/x86/cpu.c index cfcfdbd..142f2b1 100644 --- a/src/arch/x86/cpu.c +++ b/src/arch/x86/cpu.c @@ -309,6 +309,5 @@ return; /* APs are waiting for work. Last thing to do is park them. */ - if (mp_park_aps()) - printk(BIOS_ERR, "Parking APs failed.\n"); + mp_park_aps(); } diff --git a/src/cpu/x86/mp_init.c b/src/cpu/x86/mp_init.c index 409caa5..f99abaf 100644 --- a/src/cpu/x86/mp_init.c +++ b/src/cpu/x86/mp_init.c @@ -962,7 +962,24 @@ int mp_park_aps(void) { - return mp_run_on_aps(park_this_cpu, 10 * USECS_PER_MSEC); + struct stopwatch sw; + int ret; + long duration_msecs; + + stopwatch_init(&sw); + + ret = mp_run_on_aps(park_this_cpu, 250 * USECS_PER_MSEC); + + duration_msecs = stopwatch_duration_msecs(&sw); + + if (!ret) + printk(BIOS_DEBUG, "%s done after %ld msecs.\n", __func__, + duration_msecs); + else + printk(BIOS_ERR, "%s failed after %ld msecs.\n", __func__, + duration_msecs); + + return ret; } static struct mp_flight_record mp_steps[] = { -- To view, visit https://review.coreboot.org/25422 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Id3e59db4fe7a5a2fb60357b05565bba89be1e00e Gerrit-Change-Number: 25422 Gerrit-PatchSet: 1 Gerrit-Owner: Furquan Shaikh <furquan(a)google.com>

1 0

Change in coreboot[master]: soc/amd/common/block/pi/amd_late_init.c: Fix coverity issue CID 1387031
by Martin Roth (Code Review) 29 Mar '18

29 Mar '18

Martin Roth has posted comments on this change. ( https://review.coreboot.org/25419 ) Change subject: soc/amd/common/block/pi/amd_late_init.c: Fix coverity issue CID 1387031 ...................................................................... Patch Set 1: Do you need to update the size of dimm->module_part_number for 19 bytes from the SPD and 1 byte of termination? -- To view, visit https://review.coreboot.org/25419 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I18fa5e9c73401575441b6810f1db80d11666368c Gerrit-Change-Number: 25419 Gerrit-PatchSet: 1 Gerrit-Owner: Richard Spiegel <richard.spiegel(a)silverbackltd.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin Roth <martinroth(a)google.com> Gerrit-Comment-Date: Thu, 29 Mar 2018 06:46:27 +0000 Gerrit-HasComments: No Gerrit-HasLabels: No

1 0

Change in coreboot[master]: soc/amd/common/block/pi/amd_late_init.c: Fix coverity issue CID 1387031
by Martin Roth (Code Review) 29 Mar '18

29 Mar '18

Martin Roth has posted comments on this change. ( https://review.coreboot.org/25419 ) Change subject: soc/amd/common/block/pi/amd_late_init.c: Fix coverity issue CID 1387031 ...................................................................... Patch Set 1: (1 comment) https://review.coreboot.org/#/c/25419/1/src/soc/amd/common/block/pi/amd_lat… File src/soc/amd/common/block/pi/amd_late_init.c: https://review.coreboot.org/#/c/25419/1/src/soc/amd/common/block/pi/amd_lat… PS1, Line 53: sizeof(dimm->module_part_number) - 1) Is this the correct solution here? Isn't this going to truncate the last byte of the spd's part number? Maybe use memcpy and manually terminate the string instead? -- To view, visit https://review.coreboot.org/25419 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I18fa5e9c73401575441b6810f1db80d11666368c Gerrit-Change-Number: 25419 Gerrit-PatchSet: 1 Gerrit-Owner: Richard Spiegel <richard.spiegel(a)silverbackltd.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Martin Roth <martinroth(a)google.com> Gerrit-Comment-Date: Thu, 29 Mar 2018 06:43:30 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in coreboot[master]: soc/intel: Change in timeout value for mp_init
by build bot (Jenkins) (Code Review) 29 Mar '18

29 Mar '18

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/25420 ) Change subject: soc/intel: Change in timeout value for mp_init ...................................................................... Patch Set 1: Verified+1 Build Successful https://qa.coreboot.org/job/coreboot-gerrit/69183/ : SUCCESS https://qa.coreboot.org/job/coreboot-checkpatch/23532/ : SUCCESS -- To view, visit https://review.coreboot.org/25420 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7f09c1407ccbcc3bae90fb3806f1c754b1d9ac09 Gerrit-Change-Number: 25420 Gerrit-PatchSet: 1 Gerrit-Owner: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 29 Mar 2018 05:23:03 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in coreboot[master]: soc/intel: Change in timeout value for mp_init
by Srinidhi N Kaushik (Code Review) 29 Mar '18

29 Mar '18

Srinidhi N Kaushik has uploaded this change for review. ( https://review.coreboot.org/25420 Change subject: soc/intel: Change in timeout value for mp_init ...................................................................... soc/intel: Change in timeout value for mp_init This patch chnages the timeout for all the mp_run_on_all_cpus calls to fix the Parking AP error. BUG=76442753 BRANCH=none TEST=Build and boot, perform Suspend and during resume should not see "Parking APs failed" error. Change-Id: I7f09c1407ccbcc3bae90fb3806f1c754b1d9ac09 Signed-off-by: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com> --- M src/soc/intel/apollolake/chip.c M src/soc/intel/apollolake/cpu.c M src/soc/intel/common/block/cpu/mp_init.c 3 files changed, 3 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/20/25420/1 diff --git a/src/soc/intel/apollolake/chip.c b/src/soc/intel/apollolake/chip.c index 1dd6daf..242ce37 100644 --- a/src/soc/intel/apollolake/chip.c +++ b/src/soc/intel/apollolake/chip.c @@ -604,7 +604,7 @@ static void drop_privilege_all(void) { /* Drop privilege level on all the CPUs */ - if (mp_run_on_all_cpus(&cpu_enable_untrusted_mode, 1000) < 0) + if (mp_run_on_all_cpus(&cpu_enable_untrusted_mode, 3000) < 0) printk(BIOS_ERR, "failed to enable untrusted mode\n"); } diff --git a/src/soc/intel/apollolake/cpu.c b/src/soc/intel/apollolake/cpu.c index d093acc..cadf50a 100644 --- a/src/soc/intel/apollolake/cpu.c +++ b/src/soc/intel/apollolake/cpu.c @@ -241,7 +241,7 @@ smm_southbridge_enable(PWRBTN_EN | GBL_EN); if (IS_ENABLED(CONFIG_SOC_INTEL_COMMON_BLOCK_SGX)) - mp_run_on_all_cpus(sgx_configure, 2000); + mp_run_on_all_cpus(sgx_configure, 3000); } static const struct mp_ops mp_ops = { diff --git a/src/soc/intel/common/block/cpu/mp_init.c b/src/soc/intel/common/block/cpu/mp_init.c index 085a340..462c01a 100644 --- a/src/soc/intel/common/block/cpu/mp_init.c +++ b/src/soc/intel/common/block/cpu/mp_init.c @@ -132,7 +132,7 @@ /* Ensure to re-program all MTRRs based on DRAM resource settings */ static void post_cpus_init(void *unused) { - if (mp_run_on_all_cpus(&x86_setup_mtrrs_with_detect, 1000) < 0) + if (mp_run_on_all_cpus(&x86_setup_mtrrs_with_detect, 2000) < 0) printk(BIOS_ERR, "MTRR programming failure\n"); x86_mtrr_check(); -- To view, visit https://review.coreboot.org/25420 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I7f09c1407ccbcc3bae90fb3806f1c754b1d9ac09 Gerrit-Change-Number: 25420 Gerrit-PatchSet: 1 Gerrit-Owner: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com>

1 0

Change in coreboot[master]: src/lib/dimm_info_util.c: Add methods to convert from SMBIOS to SPD
by Richard Spiegel (Code Review) 29 Mar '18

29 Mar '18

Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/25412 ) Change subject: src/lib/dimm_info_util.c: Add methods to convert from SMBIOS to SPD ...................................................................... Patch Set 1: (7 comments) https://review.coreboot.org/#/c/25412/1//COMMIT_MSG Commit Message: https://review.coreboot.org/#/c/25412/1//COMMIT_MSG@14 PS1, Line 14: Tested on grunt Say something better. How did you tested? Using code not present in the commit? Will this code be committed separately? The reader needs to understand how it was tested. https://review.coreboot.org/#/c/25412/1/src/include/dimm_info_util.h File src/include/dimm_info_util.h: https://review.coreboot.org/#/c/25412/1/src/include/dimm_info_util.h@4 PS1, Line 4: Inc LLC https://review.coreboot.org/#/c/25412/1/src/include/dimm_info_util.h@38 PS1, Line 38: /** Empty line before a comment. It's ok to place prototypes together if they don't have a comment describing them. https://review.coreboot.org/#/c/25412/1/src/lib/dimm_info_util.c File src/lib/dimm_info_util.c: https://review.coreboot.org/#/c/25412/1/src/lib/dimm_info_util.c@4 PS1, Line 4: Inc LLC https://review.coreboot.org/#/c/25412/1/src/lib/dimm_info_util.c@74 PS1, Line 74: else if Just an "if" is enough because if there is a match in the previous "if" it returns and never reaches the else. https://review.coreboot.org/#/c/25412/1/src/lib/dimm_info_util.c@77 PS1, Line 77: else if Just an "if" is enough because if there is a match in the previous "if" it returns and never reaches the else. https://review.coreboot.org/#/c/25412/1/src/lib/dimm_info_util.c@80 PS1, Line 80: else No need of "else". It will only reach here if there was no previous match. -- To view, visit https://review.coreboot.org/25412 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Id9fa98e9aad83dfd0a86f45e18b3c312665dce9b Gerrit-Change-Number: 25412 Gerrit-PatchSet: 1 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Richard Spiegel <richard.spiegel(a)silverbackltd.com> Gerrit-Comment-Date: Thu, 29 Mar 2018 00:24:58 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in coreboot[master]: src/include/hexutil.h: Add converters for working with hex strings
by Richard Spiegel (Code Review) 29 Mar '18

29 Mar '18

Richard Spiegel has posted comments on this change. ( https://review.coreboot.org/25411 ) Change subject: src/include/hexutil.h: Add converters for working with hex strings ...................................................................... Patch Set 1: (5 comments) https://review.coreboot.org/#/c/25411/1//COMMIT_MSG Commit Message: https://review.coreboot.org/#/c/25411/1//COMMIT_MSG@12 PS1, Line 12: I have a bunch of unit tests for these functions outside of this commit. Is : there somewhere I can commit them? This does not belong here. That said, you can start the title with [Not-for-merge] and commit them to coreboot. https://review.coreboot.org/#/c/25411/1//COMMIT_MSG@16 PS1, Line 16: tested on grunt Say something like: "Build and boot grunt. Tested with functions that are part of a separate commit." or "Build and boot grunt. Used debug tests to test the functions, removed before committing." https://review.coreboot.org/#/c/25411/1/src/include/hexutil.h File src/include/hexutil.h: https://review.coreboot.org/#/c/25411/1/src/include/hexutil.h@26 PS1, Line 26: * */ */ https://review.coreboot.org/#/c/25411/1/src/lib/hexutil.c File src/lib/hexutil.c: https://review.coreboot.org/#/c/25411/1/src/lib/hexutil.c@35 PS1, Line 35: What would happen if (src_size - 1) > (dest_size * 2)? You should add some code to trim src_size to fall within the limits imposed by dest_size. https://review.coreboot.org/#/c/25411/1/src/lib/hexutil.c@59 PS1, Line 59: What would happen if dest_size <= (src_size * 2)? You should add some code to trim src_size to fall within the limits imposed by dest_size. -- To view, visit https://review.coreboot.org/25411 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifcabf09be4dc053f8b6e132b4014380cdb5c521b Gerrit-Change-Number: 25411 Gerrit-PatchSet: 1 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Richard Spiegel <richard.spiegel(a)silverbackltd.com> Gerrit-Comment-Date: Thu, 29 Mar 2018 00:06:58 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in coreboot[master]: soc/intel/cannonlake: Add VT-d and VMX programming
by build bot (Jenkins) (Code Review) 29 Mar '18

29 Mar '18

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/25118 ) Change subject: soc/intel/cannonlake: Add VT-d and VMX programming ...................................................................... Patch Set 11: Verified+1 Build Successful https://qa.coreboot.org/job/coreboot-gerrit/69182/ : SUCCESS https://qa.coreboot.org/job/coreboot-checkpatch/23531/ : SUCCESS -- To view, visit https://review.coreboot.org/25118 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I589590450aa4b9302ee2f9bb7b879a332f50b73e Gerrit-Change-Number: 25118 Gerrit-PatchSet: 11 Gerrit-Owner: Lijian Zhao <lijian.zhao(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Caveh Jalali <caveh(a)google.com> Gerrit-Reviewer: Duncan Laurie <dlaurie(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Reviewer: Lijian Zhao <lijian.zhao(a)intel.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Pratikkumar V Prajapati <pratikkumar.v.prajapati(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 28 Mar 2018 22:15:03 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in coreboot[master]: intel/fsp: Update cannonlake fsp header
by build bot (Jenkins) (Code Review) 29 Mar '18

29 Mar '18

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/25378 ) Change subject: intel/fsp: Update cannonlake fsp header ...................................................................... Patch Set 3: Build Successful https://qa.coreboot.org/job/coreboot-gerrit/69180/ : SUCCESS https://qa.coreboot.org/job/coreboot-checkpatch/23529/ : SUCCESS -- To view, visit https://review.coreboot.org/25378 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie1c4df162e75535ad458709452a76de01e31907e Gerrit-Change-Number: 25378 Gerrit-PatchSet: 3 Gerrit-Owner: Lijian Zhao <lijian.zhao(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Caveh Jalali <caveh(a)google.com> Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Wed, 28 Mar 2018 22:13:19 +0000 Gerrit-HasComments: No Gerrit-HasLabels: No

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit March 2018