coreboot-gerrit November 2021

coreboot-gerrit@coreboot.org

1 participants
6394 discussions

Change in coreboot[master]: mb/google/brya/variants/primus: update gpios for power consumption
by Tim Wawrzynczak (Code Review) 23 Nov '21

23 Nov '21

Attention is currently required from: Malik Hsu. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59563 ) Change subject: mb/google/brya/variants/primus: update gpios for power consumption ...................................................................... Patch Set 4: (2 comments) File src/mainboard/google/brya/variants/primus/variant.c: https://review.coreboot.org/c/coreboot/+/59563/comment/7466b9ec_1d6a3612 PS4, Line 46: get_sku_and_set_gpio suggestion: `disable_unused_gpios` ? https://review.coreboot.org/c/coreboot/+/59563/comment/fcc72142_a94c3a88 PS4, Line 48: int sku = gpio_get(GPP_T2); : : if (sku == 1) nit: as fars as I can tell, this GPIO is just stuffed on boards with the eMMC, so let's avoid the term `SKU` here. can you rename this to `emmc_detected` or similar? -- To view, visit https://review.coreboot.org/c/coreboot/+/59563 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I753e41dec1825299e6cd437b5f67e2d957bc6148 Gerrit-Change-Number: 59563 Gerrit-PatchSet: 4 Gerrit-Owner: Malik Hsu <malik_hsu(a)wistron.corp-partner.google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Casper Chang <casper_chang(a)wistron.corp-partner.google.com> Gerrit-Attention: Malik Hsu <malik_hsu(a)wistron.corp-partner.google.com> Gerrit-Comment-Date: Tue, 23 Nov 2021 16:42:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/brya/var/primus: Update thermal table for primus
by Tim Wawrzynczak (Code Review) 23 Nov '21

23 Nov '21

Attention is currently required from: Ariel Fang. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59182 ) Change subject: mb/google/brya/var/primus: Update thermal table for primus ...................................................................... Patch Set 8: (1 comment) File src/mainboard/google/brya/variants/primus/overridetree.cb: https://review.coreboot.org/c/coreboot/+/59182/comment/d525d692_5001403d PS8, Line 79: [0] = DPTF_PASSIVE(CPU, CPU, 95, 5000), : [1] = DPTF_PASSIVE(CPU, TEMP_SENSOR_0, 90, 5000), : [2] = DPTF_PASSIVE(CHARGER, TEMP_SENSOR_1, 90, 5000), : [3] = DPTF_PASSIVE(CPU, TEMP_SENSOR_2, 90, 5000), : [4] = DPTF_PASSIVE(CPU, TEMP_SENSOR_3, 90, 5000), can you line up each column? -- To view, visit https://review.coreboot.org/c/coreboot/+/59182 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id4d8dbe678b7f0870aeffa0a0118e65de9d5c22d Gerrit-Change-Number: 59182 Gerrit-PatchSet: 8 Gerrit-Owner: Ariel Fang <ariel_fang(a)wistron.corp-partner.google.com> Gerrit-Reviewer: Ariel Fang <ariel_fang(a)wistron.corp-partner.google.com> Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Casper Chang <casper_chang(a)wistron.corp-partner.google.com> Gerrit-CC: Malik Hsu <malik_hsu(a)wistron.corp-partner.google.com> Gerrit-CC: Mark Hsieh <mark_hsieh(a)wistron.corp-partner.google.com> Gerrit-CC: Scott Chao <scott_chao(a)wistron.corp-partner.google.com> Gerrit-CC: Terry Chen <terry_chen(a)wistron.corp-partner.google.com> Gerrit-Attention: Ariel Fang <ariel_fang(a)wistron.corp-partner.google.com> Gerrit-Comment-Date: Tue, 23 Nov 2021 16:25:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/brya/var/kano: set power limits for thermal
by Tim Wawrzynczak (Code Review) 23 Nov '21

23 Nov '21

Attention is currently required from: David Wu. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59091 ) Change subject: mb/google/brya/var/kano: set power limits for thermal ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/59091 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I25cf9be68f8981d8307b4c15ab9f65b59058fb19 Gerrit-Change-Number: 59091 Gerrit-PatchSet: 2 Gerrit-Owner: David Wu <david_wu(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Paul Yang <paul.f.yang(a)intel.corp-partner.google.com> Gerrit-Reviewer: Peter Ou <peter.ou(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: YH Lin <yueherngl(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: David Wu <david_wu(a)quanta.corp-partner.google.com> Gerrit-Comment-Date: Tue, 23 Nov 2021 16:05:20 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/brya/var/gimble: Enable DRIVERS_GENESYSLOGIC_GL9750
by Tim Wawrzynczak (Code Review) 23 Nov '21

23 Nov '21

Attention is currently required from: YH Lin, Mark Hsieh, Zhuohao Lee, Felix Held. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59579 ) Change subject: mb/google/brya/var/gimble: Enable DRIVERS_GENESYSLOGIC_GL9750 ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/59579 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifc490e6e081b6a8534656417603d2916c3edcb05 Gerrit-Change-Number: 59579 Gerrit-PatchSet: 1 Gerrit-Owner: Mark Hsieh <mark_hsieh(a)wistron.corp-partner.google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Mark Hsieh <mark_hsieh(a)wistron.corp-partner.google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: YH Lin <yueherngl(a)google.com> Gerrit-Reviewer: Zhuohao Lee <zhuohao(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anfernee Chen <anfernee_chen(a)wistron.corp-partner.google.com> Gerrit-CC: Ariel Fang <ariel_fang(a)wistron.corp-partner.google.com> Gerrit-CC: Casper Chang <casper_chang(a)wistron.corp-partner.google.com> Gerrit-CC: Malik Hsu <malik_hsu(a)wistron.corp-partner.google.com> Gerrit-CC: Scott Chao <scott_chao(a)wistron.corp-partner.google.com> Gerrit-CC: Terry Chen <terry_chen(a)wistron.corp-partner.google.com> Gerrit-CC: Will Tsai <will_tsai(a)wistron.corp-partner.google.com> Gerrit-Attention: YH Lin <yueherngl(a)google.com> Gerrit-Attention: Mark Hsieh <mark_hsieh(a)wistron.corp-partner.google.com> Gerrit-Attention: Zhuohao Lee <zhuohao(a)google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Tue, 23 Nov 2021 16:03:32 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/i2c/tpm: Fix blank default statement
by Rob Barnes (Code Review) 23 Nov '21

23 Nov '21

Attention is currently required from: Christian Walter, Karthik Ramasubramanian. Rob Barnes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59551 ) Change subject: drivers/i2c/tpm: Fix blank default statement ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/59551 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3d034cfebc8b8ae7d7024d41b4b2207cdeb083e8 Gerrit-Change-Number: 59551 Gerrit-PatchSet: 1 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Patrick Georgi <patrick(a)coreboot.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Rob Barnes <robbarnes(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Tue, 23 Nov 2021 15:43:39 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/alderlake: Trigger cse_fw_sync before DRAM Init
by Sridhar Siricilla (Code Review) 23 Nov '21

23 Nov '21

Attention is currently required from: Tim Wawrzynczak, Paul Menzel, Ravindra, Subrata Banik, Patrick Rudolph. Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55364 ) Change subject: soc/intel/alderlake: Trigger cse_fw_sync before DRAM Init ...................................................................... Patch Set 12: (1 comment) File src/soc/intel/alderlake/romstage/romstage.c: https://review.coreboot.org/c/coreboot/+/55364/comment/309bcca3_93dc0868 PS12, Line 140: > Ack Looks like list of timestamp ids(940-950: Intel ME) assigned for ME are fully utilized. I need to take new range(600-650) for ME in addition to 940-950 range. Is that ok? -- To view, visit https://review.coreboot.org/c/coreboot/+/55364 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iad7403650df8bc4e40aa6e48ccfeba95a5789a2d Gerrit-Change-Number: 55364 Gerrit-PatchSet: 12 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.corp-partner.google.com> Gerrit-CC: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Ravindra <ravindra(a)intel.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Tue, 23 Nov 2021 15:33:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com> Comment-In-Reply-To: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: utils: Add initial version of "remove_unused_code" script
by Martin Roth (Code Review) 23 Nov '21

23 Nov '21

Attention is currently required from: Raul Rangel, Piotr Król. Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59169 ) Change subject: utils: Add initial version of "remove_unused_code" script ...................................................................... Patch Set 2: (22 comments) File util/scripts/rm_unused_code: https://review.coreboot.org/c/coreboot/+/59169/comment/8cf03a6a_ef84c9f5 PS1, Line 35: show_version() { : echo : _echo_color "${BOLD}${GREEN}" "${PROGNAME} version ${VERSION}" 1 : echo : } > nit: I would move this under _echo_color Done https://review.coreboot.org/c/coreboot/+/59169/comment/d887b5e5_3d048f50 PS1, Line 41: _echo_color > Can you add a comment describing the args. Added local variables with named arguments. https://review.coreboot.org/c/coreboot/+/59169/comment/720d4442_dfa3a640 PS1, Line 50: test -n "${VERBOSE}" > You have `-e set above. […] Removed _echo_debug, it's not currently used. https://review.coreboot.org/c/coreboot/+/59169/comment/5eaa0d01_6ccd068e PS1, Line 72: BLD_DIR > Do you need to `shift` after pulling the arg off? I added the shift at the end of the loop so if there's only the argument flag, no shift is needed. All of the other shifts have been removed. https://review.coreboot.org/c/coreboot/+/59169/comment/e3a6e6b4_8675af60 PS1, Line 115: ${dir} > quote Done https://review.coreboot.org/c/coreboot/+/59169/comment/a7e4939c_4ec5c960 PS1, Line 125: ${dir} > "" Done https://review.coreboot.org/c/coreboot/+/59169/comment/19c9b89e_ed1da576 PS1, Line 189: { > You could replace the {} these with () to make the function invoke the subshell. Done https://review.coreboot.org/c/coreboot/+/59169/comment/775313ef_3004a1b3 PS1, Line 196: # find "${BLD_DIR}" -type f -exec touch -a --date="2020-01-01 00:00:00" {} \; > ? This was the old version of the code before I changed to git ls-files. Removing. https://review.coreboot.org/c/coreboot/+/59169/comment/6e433fc0_b312dc2c PS1, Line 205: KEEP_FILES > Can you declare KEEP_FILES as an array above,then use "${KEEP_FILES[@]}" Done https://review.coreboot.org/c/coreboot/+/59169/comment/32088282_9fea7646 PS1, Line 206: find > Why do we need the find? Can this be replaced with `touch "${BLD_DIR}/${file}"`? The function works on files inside directories as well as just files. The find allows us to specify a directory and mark all the files inside. If it's an actual single file, the find is transparent. https://review.coreboot.org/c/coreboot/+/59169/comment/d6a9c856_736fe376 PS1, Line 211: $1 > "" Done https://review.coreboot.org/c/coreboot/+/59169/comment/863fcbf1_9997f347 PS1, Line 214: ${CONFIG_FILE} > "" Done https://review.coreboot.org/c/coreboot/+/59169/comment/d96b66e2_f5bd6cd5 PS1, Line 221: || exit 1 > Your `-e` should take care this, but it doesn't hurt. Right. This is left over from before I had the -e. Removed. https://review.coreboot.org/c/coreboot/+/59169/comment/5e4b6898_7d420438 PS1, Line 228: MODIFIED_FILES > Could use `readarray MODIFIED_FILES < <(find ... […] Done https://review.coreboot.org/c/coreboot/+/59169/comment/5f3df398_87029775 PS1, Line 228: > I think you can add a `-path ./.git -prune` to the find command and avoid the grep. Done https://review.coreboot.org/c/coreboot/+/59169/comment/d3a0c76f_23957c0e PS1, Line 229: (echo "${MODIFIED_FILES}" | wc -l) > If you use an array you can use "${#MODIFIED_FILES[@]}" Done https://review.coreboot.org/c/coreboot/+/59169/comment/3e9b9118_18e2b8cf PS1, Line 236: -exec rm {} \; > -delete Done https://review.coreboot.org/c/coreboot/+/59169/comment/02cc9ccf_70e1f419 PS1, Line 249: CLEAN_DIR_LIST > I would make it an array: "${CLEAN_DIR_LIST[@]}" Done https://review.coreboot.org/c/coreboot/+/59169/comment/d043db29_fd3e9474 PS1, Line 252: -exec rm {} \; > -delete Done https://review.coreboot.org/c/coreboot/+/59169/comment/f85f2a05_8ad80faa PS1, Line 264: && > Shouldn't need the && with `-e` Done https://review.coreboot.org/c/coreboot/+/59169/comment/c011cb67_85f3e262 PS1, Line 279: ${HASH} > "" Done https://review.coreboot.org/c/coreboot/+/59169/comment/d381e4d6_39efdc1d PS1, Line 285: ${HASH} > "" Done -- To view, visit https://review.coreboot.org/c/coreboot/+/59169 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iec69db2ad1358846d649db627b6d60ac8c2204e4 Gerrit-Change-Number: 59169 Gerrit-PatchSet: 2 Gerrit-Owner: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Comment-Date: Tue, 23 Nov 2021 15:26:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/*: PSP verstage minbuild
by build bot (Jenkins) (Code Review) 23 Nov '21

23 Nov '21

Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Felix Held. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59585 ) Change subject: soc/amd/*: PSP verstage minbuild ...................................................................... Patch Set 1: (2 comments) File src/soc/amd/common/psp_verstage/psp_verstage.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-134127): https://review.coreboot.org/c/coreboot/+/59585/comment/501b5a24_ecc5029b PS1, Line 226: memset(_transfer_buffer,0,buffer_size); space required after that ',' (ctx:VxV) Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-134127): https://review.coreboot.org/c/coreboot/+/59585/comment/400f8c53_26432adb PS1, Line 226: memset(_transfer_buffer,0,buffer_size); space required after that ',' (ctx:VxV) -- To view, visit https://review.coreboot.org/c/coreboot/+/59585 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie506193d21207ecabe65158e84dc02eec1cf5b26 Gerrit-Change-Number: 59585 Gerrit-PatchSet: 1 Gerrit-Owner: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Tue, 23 Nov 2021 15:16:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: amdfwtool: replace strcmp to switch-case
by build bot (Jenkins) (Code Review) 23 Nov '21

23 Nov '21

build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59583 ) Change subject: amdfwtool: replace strcmp to switch-case ...................................................................... Patch Set 1: (3 comments) File util/amdfwtool/data_parse.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-134125): https://review.coreboot.org/c/coreboot/+/59583/comment/472e1d47_2649e03a PS1, Line 176: if (strcmp(fw_name, "PSPBTLDR_WL_FILE") == 0) { braces {} are not necessary for any arm of this statement Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-134125): https://review.coreboot.org/c/coreboot/+/59583/comment/6cef2917_b842666d PS1, Line 179: return (cb_config->have_whitelist); return is not a function, parentheses are not required Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-134125): https://review.coreboot.org/c/coreboot/+/59583/comment/0fa7617e_3bd451b0 PS1, Line 203: if (should_skip_entry(fw_name, fw_type, cb_config)) { braces {} are not necessary for single statement blocks -- To view, visit https://review.coreboot.org/c/coreboot/+/59583 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Icf9f3465a36d83cf79f9f4bd1ec824b546a71aaf Gerrit-Change-Number: 59583 Gerrit-PatchSet: 1 Gerrit-Owner: Kangheui Won <khwon(a)chromium.org> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 23 Nov 2021 15:10:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/*: PSP verstage minbuild
by Martin Roth (Code Review) 23 Nov '21

23 Nov '21

Martin Roth has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/59585 ) Change subject: soc/amd/*: PSP verstage minbuild ...................................................................... soc/amd/*: PSP verstage minbuild The PSP that is supplied for Picasso AMD-based chromebooks currently requires a psp_verstage to boot. This supplies a minimal psp verstage that does the initialization that's normally done there, then exits without doing any verification. Signed-off-by: Martin Roth <martin(a)coreboot.org> Change-Id: Ie506193d21207ecabe65158e84dc02eec1cf5b26 --- M src/arch/x86/car.ld M src/soc/amd/cezanne/Kconfig M src/soc/amd/common/psp_verstage/Kconfig M src/soc/amd/common/psp_verstage/psp_verstage.c M src/soc/amd/picasso/Kconfig M src/soc/amd/picasso/Makefile.inc 6 files changed, 73 insertions(+), 33 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/85/59585/1 diff --git a/src/arch/x86/car.ld b/src/arch/x86/car.ld index 5a46b8b..878a7da 100644 --- a/src/arch/x86/car.ld +++ b/src/arch/x86/car.ld @@ -13,7 +13,7 @@ * aligned when using this option. */ REGION(pagetables, ., 4K * CONFIG_NUM_CAR_PAGE_TABLE_PAGES, 4K) #endif -#if CONFIG(VBOOT_STARTS_IN_BOOTBLOCK) +#if (CONFIG(VBOOT_STARTS_IN_BOOTBLOCK) || CONFIG(MIN_PSP_VSTAGE)) /* Vboot work buffer only needs to be available when verified boot * starts in bootblock. */ VBOOT2_WORK(., 12K) diff --git a/src/soc/amd/cezanne/Kconfig b/src/soc/amd/cezanne/Kconfig index 6c8c3e5..f3e4017 100644 --- a/src/soc/amd/cezanne/Kconfig +++ b/src/soc/amd/cezanne/Kconfig @@ -388,7 +388,7 @@ config PSP_VERSTAGE_FILE string "Specify the PSP_verstage file path" - depends on VBOOT_STARTS_BEFORE_BOOTBLOCK + depends on VBOOT_STARTS_BEFORE_BOOTBLOCK || MIN_PSP_VSTAGE default "\$(obj)/psp_verstage.bin" help Add psp_verstage file to the build & PSP Directory Table diff --git a/src/soc/amd/common/psp_verstage/Kconfig b/src/soc/amd/common/psp_verstage/Kconfig index 834abaa..38ee92c 100644 --- a/src/soc/amd/common/psp_verstage/Kconfig +++ b/src/soc/amd/common/psp_verstage/Kconfig @@ -5,3 +5,13 @@ Configure PSP Verstage to use Crypto Co-processor (CCP) DMA while accessing the boot device. Select it on platforms which supports using CCP DMA to access the boot device. + +config MIN_PSP_VSTAGE + bool "Use minimal psp 'verstage' that doesn't do verification" + depends on SOC_AMD_COMMON_BLOCK_PSP_GEN2 + default y if !VBOOT && MAINBOARD_HAS_CHROMEOS + help + Build and load a minimal "verstage" that does the initialization + as normal, but skips the verification portion of verstage. This + is needed for booting Picasso/Dali based chromebooks when not using + verstage. diff --git a/src/soc/amd/common/psp_verstage/psp_verstage.c b/src/soc/amd/common/psp_verstage/psp_verstage.c index 71f07c0..5a55ac1 100644 --- a/src/soc/amd/common/psp_verstage/psp_verstage.c +++ b/src/soc/amd/common/psp_verstage/psp_verstage.c @@ -206,6 +206,35 @@ return 0; } +/* + * Save workbuf (and soon memory console and timestamps) to the bootloader to pass + * back to coreboot. + */ +static uint32_t save_min_buffers(void) +{ + uint32_t retval; + uint32_t buffer_size = MIN_TRANSFER_BUFFER_SIZE; + struct transfer_info_struct buffer_info = {0}; + + + buffer_info.magic_val = TRANSFER_MAGIC_VAL; + buffer_info.struct_bytes = sizeof(buffer_info); + buffer_info.buffer_size = buffer_size; + buffer_info.workbuf_offset = (uint32_t)((uintptr_t)_fmap_cache - + (uintptr_t)_vboot2_work); + + memset(_transfer_buffer,0,buffer_size); + memcpy(_transfer_buffer, &buffer_info, sizeof(buffer_info)); + + retval = save_uapp_data((void *)_transfer_buffer, buffer_size); + if (retval) { + printk(BIOS_ERR, "Error: Could not save workbuf. Error code 0x%08x\n", retval); + return POSTCODE_WORKBUF_SAVE_ERROR; + } + + return 0; +} + void Main(void) { uint32_t retval; @@ -226,7 +255,8 @@ svc_write_postcode(POSTCODE_EARLY_INIT); retval = verstage_soc_early_init(); - if (retval) { + + if (CONFIG(VBOOT_STARTS_BEFORE_BOOTBLOCK) && retval) { /* * If verstage_soc_early_init fails, cmos is probably not * accessible, so rebooting into recovery is not an option. @@ -266,33 +296,37 @@ post_code(POSTCODE_VERSTAGE_MAIN); - vboot_run_logic(); + if (CONFIG(VBOOT_STARTS_BEFORE_BOOTBLOCK)) { + vboot_run_logic(); - ctx = vboot_get_context(); - retval = check_cmos_recovery(); - if (retval) - reboot_into_recovery(ctx, retval); - post_code(POSTCODE_UPDATE_BOOT_REGION); + ctx = vboot_get_context(); + retval = check_cmos_recovery(); + if (retval) + reboot_into_recovery(ctx, retval); - /* - * Since psp_verstage doesn't load next stage we never call - * any cbfs API on RO path. However we still need to initialize - * RO CBFS MCACHE manually to pass it in transfer_buffer. - * In RW path, MCACHE build will be skipped for RO region since - * we already built here. - */ - cbfs_get_boot_device(true); + post_code(POSTCODE_UPDATE_BOOT_REGION); - retval = update_boot_region(ctx); - if (retval) - reboot_into_recovery(ctx, retval); + /* + * Since psp_verstage doesn't load next stage we never call + * any cbfs API on RO path. However we still need to initialize + * RO CBFS MCACHE manually to pass it in transfer_buffer. + * In RW path, MCACHE build will be skipped for RO region since + * we already built here. + */ + cbfs_get_boot_device(true); - post_code(POSTCODE_SAVE_BUFFERS); - retval = save_buffers(&ctx); - if (retval) - reboot_into_recovery(ctx, retval); + retval = update_boot_region(ctx); + if (retval) + reboot_into_recovery(ctx, retval); + post_code(POSTCODE_SAVE_BUFFERS); + retval = save_buffers(&ctx); + if (retval) + reboot_into_recovery(ctx, retval); + } else { + save_min_buffers(); + } post_code(POSTCODE_UNMAP_SPI_ROM); boot_dev_base = rdev_mmap_full(boot_device_ro()); diff --git a/src/soc/amd/picasso/Kconfig b/src/soc/amd/picasso/Kconfig index 92607a3..cf1f569 100644 --- a/src/soc/amd/picasso/Kconfig +++ b/src/soc/amd/picasso/Kconfig @@ -118,8 +118,7 @@ config PSP_SHAREDMEM_BASE hex - default 0x2011000 if VBOOT - default 0x0 + default 0x2011000 help This variable defines the base address in DRAM memory where PSP copies the vboot workbuf. This is used in the linker script to have a static @@ -128,8 +127,7 @@ config PSP_SHAREDMEM_SIZE hex - default 0x8000 if VBOOT - default 0x0 + default 0x8000 help Sets the maximum size for the PSP to pass the vboot workbuf and any logs or timestamps back to coreboot. This will be copied @@ -428,8 +426,7 @@ config PSP_VERSTAGE_FILE string "Specify the PSP_verstage file path" - depends on VBOOT_STARTS_BEFORE_BOOTBLOCK - default "\$(obj)/psp_verstage.bin" + default "./psp_verstage.bin" help Add psp_verstage file to the build & PSP Directory Table diff --git a/src/soc/amd/picasso/Makefile.inc b/src/soc/amd/picasso/Makefile.inc index 7db4719..6c4ec66 100644 --- a/src/soc/amd/picasso/Makefile.inc +++ b/src/soc/amd/picasso/Makefile.inc @@ -132,7 +132,7 @@ APOB_NV_SIZE=$(shell awk '$$2 == "FMAP_SECTION_RW_MRC_CACHE_SIZE" {print $$3}' $(obj)/fmap_config.h) APOB_NV_BASE=$(shell awk '$$2 == "FMAP_SECTION_RW_MRC_CACHE_START" {print $$3}' $(obj)/fmap_config.h) -ifeq ($(CONFIG_VBOOT_STARTS_BEFORE_BOOTBLOCK),y) +ifeq ($(CONFIG_VBOOT_STARTS_BEFORE_BOOTBLOCK)$(CONFIG_MIN_PSP_VERSTAGE),y) # type = 0x6B - PSP Shared memory location ifneq ($(CONFIG_PSP_SHAREDMEM_SIZE),0x0) PSP_SHAREDMEM_SIZE=$(CONFIG_PSP_SHAREDMEM_SIZE) @@ -142,8 +142,7 @@ # type = 0x52 - PSP Bootloader Userspace Application (verstage) PSP_VERSTAGE_FILE=$(call strip_quotes,$(CONFIG_PSP_VERSTAGE_FILE)) PSP_VERSTAGE_SIG_FILE=$(call strip_quotes,$(CONFIG_PSP_VERSTAGE_SIGNING_TOKEN)) -endif # CONFIG_VBOOT_STARTS_BEFORE_BOOTBLOCK - +endif # CONFIG_VBOOT_STARTS_BEFORE_BOOTBLOCK || CONFIG_MIN_PSP_VERSTAGE # Helper function to return a value with given bit set # Soft Fuse type = 0xb - See #55758 (NDA) for bit definitions. -- To view, visit https://review.coreboot.org/c/coreboot/+/59585 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie506193d21207ecabe65158e84dc02eec1cf5b26 Gerrit-Change-Number: 59585 Gerrit-PatchSet: 1 Gerrit-Owner: Martin Roth <martinroth(a)google.com> Gerrit-MessageType: newchange

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2021