coreboot-gerrit May 2021

coreboot-gerrit@coreboot.org

1 participants
6048 discussions

Change in coreboot[master]: vboot/secdata_tpm: Create FWMP space in coreboot
by Andrey Pronin (Code Review) 14 May '21

14 May '21

Attention is currently required from: Joel Kitching, Aaron Durbin. Andrey Pronin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52919 ) Change subject: vboot/secdata_tpm: Create FWMP space in coreboot ...................................................................... Patch Set 7: Code-Review+1 (3 comments) File src/security/vboot/secdata_tpm.c: https://review.coreboot.org/c/coreboot/+/52919/comment/2e1fc2fb_75f6dfb6 PS1, Line 237: .TPMA_NV_OWNERWRITE = 1, > Done as discussed on the doc, f/w can already modify this space by undefining and re-defining. so, PPWRITE doesn't add a new capability for it, so ok to have it. if we wanted to prevent f/w and recovery mode from modifying FWMP, we'd also need an unsatisfiable POLICY_DELETE https://review.coreboot.org/c/coreboot/+/52919/comment/a7d713c7_f8d52aee PS1, Line 420: /* : * Set initial values of secdata_firmware space. : * kernel space is created in _factory_initialize_tpm(). : */ : vb2api_secdata_firmware_create(ctx); > No, shouldn't be. I think this was just put here to minimize duplication between TPM1/TPM2 parts. to have some defaults if tlcl_self_test_full() or _factory_initialize_tpm() fail? File src/security/vboot/secdata_tpm.c: https://review.coreboot.org/c/coreboot/+/52919/comment/dcc62bdc_7988eea9 PS7, Line 13: <security/tpm/tss/tcg-2.0/tss_structures.h> curious: how did it work before this? we were already pulling TPMA_NV_* from somewhere... the only new thing added here is TPMA_NV_OWNERWRITE, other TPMA_NV_* were already present in the code -- To view, visit https://review.coreboot.org/c/coreboot/+/52919 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1f566e00f11046ff9a9891c65660af50fbb83675 Gerrit-Change-Number: 52919 Gerrit-PatchSet: 7 Gerrit-Owner: Aseda Aboagye <aaboagye(a)google.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Andrey Pronin <apronin(a)chromium.org> Gerrit-Reviewer: Joel Kitching <kitching(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Joel Kitching <kitching(a)google.com> Gerrit-Attention: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Comment-Date: Sat, 15 May 2021 01:00:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Aseda Aboagye <aaboagye(a)google.com> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: vboot/secdata_tpm: Rename set_space()
by Aseda Aboagye (Code Review) 14 May '21

14 May '21

Attention is currently required from: Julius Werner, Aaron Durbin. Aseda Aboagye has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54308 ) Change subject: vboot/secdata_tpm: Rename set_space() ...................................................................... Patch Set 3: (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/54308/comment/d79b8c6a_10388396 PS2, Line 9: The name `set_space()` seems to imply that it's writing to a TPM space when > nit: are these 72 characters? No, they were not; forgot to wrap. File src/security/vboot/secdata_tpm.c: https://review.coreboot.org/c/coreboot/+/54308/comment/07676a5b_5016b141 PS2, Line 203: ro_space_attributes, pcr0_allowed_policy, > From Jenkins: […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/54308 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I80a8342c51d7bfaa0cb2eb3fd37240425d5901be Gerrit-Change-Number: 54308 Gerrit-PatchSet: 3 Gerrit-Owner: Aseda Aboagye <aaboagye(a)google.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Comment-Date: Sat, 15 May 2021 00:53:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: vboot/secdata_tpm: Rename set_space()
by Aseda Aboagye (Code Review) 14 May '21

14 May '21

Attention is currently required from: Aseda Aboagye, Aaron Durbin. Hello build bot (Jenkins), Julius Werner, Aaron Durbin, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/54308 to look at the new patch set (#3). Change subject: vboot/secdata_tpm: Rename set_space() ...................................................................... vboot/secdata_tpm: Rename set_space() The name `set_space()` seems to imply that it's writing to a TPM space when actually, the function can create a space and write to it. This commit attempts to make that a bit more clear. Additionally, in order to use the correct sizes when creating the space, this commit also refactors the functions slightly to incorporate the vboot context object such that the correct sizes are used. The various vboot APIs will return the size of the created object that we can then create the space with. BUG=b:184677625 BRANCH=None TEST=`emerge-keeby coreboot` Signed-off-by: Aseda Aboagye <aaboagye(a)google.com> Change-Id: I80a8342c51d7bfaa0cb2eb3fd37240425d5901be --- M src/security/vboot/secdata_tpm.c 1 file changed, 21 insertions(+), 18 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/54308/3 -- To view, visit https://review.coreboot.org/c/coreboot/+/54308 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I80a8342c51d7bfaa0cb2eb3fd37240425d5901be Gerrit-Change-Number: 54308 Gerrit-PatchSet: 3 Gerrit-Owner: Aseda Aboagye <aaboagye(a)google.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Aseda Aboagye <aaboagye(a)google.com> Gerrit-Attention: Aaron Durbin <adurbin(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: amdfwtool:cezanne: use correct bootloader binary for whitelist support
by Nikolai Vyssotski (Code Review) 14 May '21

14 May '21

Attention is currently required from: Raul Rangel, Martin Roth, Marshall Dawson. Nikolai Vyssotski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/53892 ) Change subject: amdfwtool:cezanne: use correct bootloader binary for whitelist support ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: Verified it working on Guybrush. Majolica worked with it like this (broken) because it did not seem to mind stage2 BL copied into stage1 (Type0x01) entry. This was not the case for Guybrush. -- To view, visit https://review.coreboot.org/c/coreboot/+/53892 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I71539a2065546547edc8a2621474cd1388b6434b Gerrit-Change-Number: 53892 Gerrit-PatchSet: 1 Gerrit-Owner: Nikolai Vyssotski <nikolai.vyssotski(a)amd.corp-partner.google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.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: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Comment-Date: Sat, 15 May 2021 00:51:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: amdfwtool:cezanne: use correct bootloader binary for whitelist support
by Nikolai Vyssotski (Code Review) 14 May '21

14 May '21

Attention is currently required from: Raul Rangel, Martin Roth, Marshall Dawson. Nikolai Vyssotski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/53892 ) Change subject: amdfwtool:cezanne: use correct bootloader binary for whitelist support ...................................................................... Set Ready For Review -- To view, visit https://review.coreboot.org/c/coreboot/+/53892 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I71539a2065546547edc8a2621474cd1388b6434b Gerrit-Change-Number: 53892 Gerrit-PatchSet: 1 Gerrit-Owner: Nikolai Vyssotski <nikolai.vyssotski(a)amd.corp-partner.google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.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: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Comment-Date: Sat, 15 May 2021 00:48:18 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: vboot/secdata_mock: Make v0 kernel secdata context
by Aseda Aboagye (Code Review) 14 May '21

14 May '21

Attention is currently required from: Furquan Shaikh, Julius Werner. Aseda Aboagye has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54304 ) Change subject: vboot/secdata_mock: Make v0 kernel secdata context ...................................................................... Patch Set 2: (3 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/54304/comment/7b361192_868707a2 PS1, Line 10: supported on systems which do _NOT_ have a v0 secdata kernel context. > Like I mentioned on the bug, it doesn't. It just assumes that secdata is stored between reboots. Done File src/security/vboot/secdata_mock.c: https://review.coreboot.org/c/coreboot/+/54304/comment/fd269090_c11251f2 PS1, Line 31: /* Vboot implicitly assumes that EFS2 (Early Firmware Selection v2) is > coreboot multi-line comment styles are either […] Thank you. I'd never seen the latter before so I got confused. https://review.coreboot.org/c/coreboot/+/54304/comment/a10cf8f5_9fe66921 PS1, Line 32: * supported on systems which do _NOT_ have a v0 secdata kernel context. > (here too) Done -- To view, visit https://review.coreboot.org/c/coreboot/+/54304 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id8e81afcddadf27d9eec274f7f85ff1520315aaa Gerrit-Change-Number: 54304 Gerrit-PatchSet: 2 Gerrit-Owner: Aseda Aboagye <aaboagye(a)google.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Comment-Date: Sat, 15 May 2021 00:39:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: vboot/secdata_mock: Make v0 kernel secdata context
by Aseda Aboagye (Code Review) 14 May '21

14 May '21

Attention is currently required from: Furquan Shaikh, Aseda Aboagye. Hello build bot (Jenkins), Furquan Shaikh, Julius Werner, Aaron Durbin, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/54304 to look at the new patch set (#2). Change subject: vboot/secdata_mock: Make v0 kernel secdata context ...................................................................... vboot/secdata_mock: Make v0 kernel secdata context Vboot implicitly assumes that for EFS2 (Early Firmware Selection v2) systems, secdata is stored between reboots. For MOCK_SECDATA, we cannot retain data across a reboot (which is what EFS2 needs in order to use Hmir, the mirrored EC hash). Therefore, in order for vboot to skip the Hmir sync while using MOCK_SECDATA, we need to have MOCK_SECDATA create a v0 secdata kernel context. Otherwise, this would result in a reboot loop where vboot attempts to set Hmir and retrieve it after a reboot, but the value is not expected. This was encountered on using a firmware built with MOCK_SECDATA but had EC software sync enabled. BUG=b:187843114 BRANCH=None TEST=`USE=mocktpm cros build-ap -b keeby`; Flash keeby device, verify that DUT does not continuously reboot with EC software sync enabled. Signed-off-by: Aseda Aboagye <aaboagye(a)google.com> Change-Id: Id8e81afcddadf27d9eec274f7f85ff1520315aaa --- M src/security/vboot/secdata_mock.c 1 file changed, 11 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/54304/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/54304 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id8e81afcddadf27d9eec274f7f85ff1520315aaa Gerrit-Change-Number: 54304 Gerrit-PatchSet: 2 Gerrit-Owner: Aseda Aboagye <aaboagye(a)google.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Aseda Aboagye <aaboagye(a)google.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: vboot/secdata_tpm: Create FWMP space in coreboot
by Julius Werner (Code Review) 14 May '21

14 May '21

Attention is currently required from: Joel Kitching, Andrey Pronin, Aseda Aboagye, Aaron Durbin. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52919 ) Change subject: vboot/secdata_tpm: Create FWMP space in coreboot ...................................................................... Patch Set 7: Code-Review+2 (1 comment) File src/security/vboot/secdata_tpm.c: https://review.coreboot.org/c/coreboot/+/52919/comment/d894f8e6_4e2cd975 PS1, Line 420: /* : * Set initial values of secdata_firmware space. : * kernel space is created in _factory_initialize_tpm(). : */ : vb2api_secdata_firmware_create(ctx); > Does anyone know if there's any particular reason this is done prior to _factory_initialize_tpm()? I […] No, shouldn't be. I think this was just put here to minimize duplication between TPM1/TPM2 parts. -- To view, visit https://review.coreboot.org/c/coreboot/+/52919 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1f566e00f11046ff9a9891c65660af50fbb83675 Gerrit-Change-Number: 52919 Gerrit-PatchSet: 7 Gerrit-Owner: Aseda Aboagye <aaboagye(a)google.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Andrey Pronin <apronin(a)chromium.org> Gerrit-Reviewer: Joel Kitching <kitching(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Joel Kitching <kitching(a)google.com> Gerrit-Attention: Andrey Pronin <apronin(a)chromium.org> Gerrit-Attention: Aseda Aboagye <aaboagye(a)google.com> Gerrit-Attention: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Comment-Date: Sat, 15 May 2021 00:28:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Aseda Aboagye <aaboagye(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Update vboot submodule to upstream/main (e681c37)
by Julius Werner (Code Review) 14 May '21

14 May '21

Attention is currently required from: Aseda Aboagye. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54307 ) Change subject: Update vboot submodule to upstream/main (e681c37) ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/54307 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifd130e3f66f1819f59f00703f0ad0c2278b544bf Gerrit-Change-Number: 54307 Gerrit-PatchSet: 2 Gerrit-Owner: Aseda Aboagye <aaboagye(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Aseda Aboagye <aaboagye(a)google.com> Gerrit-Comment-Date: Sat, 15 May 2021 00:25:51 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: vboot/secdata_tpm: Rename set_space()
by Julius Werner (Code Review) 14 May '21

14 May '21

Attention is currently required from: Aseda Aboagye, Aaron Durbin. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54308 ) Change subject: vboot/secdata_tpm: Rename set_space() ...................................................................... Patch Set 2: (1 comment) File src/security/vboot/secdata_tpm.c: https://review.coreboot.org/c/coreboot/+/54308/comment/3a5f7f09_7e9217f0 PS2, Line 203: ro_space_attributes, pcr0_allowed_policy, From Jenkins: src/security/vboot/secdata_tpm.c: In function 'set_mrc_hash_space': src/security/vboot/secdata_tpm.c:202:10: error: implicit declaration of function 'set_space'; did you mean 'setup_space'? [-Werror=implicit-function-declaration] return set_space("RO MRC Hash", index, data, HASH_NV_SIZE, ^~~~~~~~~ setup_space -- To view, visit https://review.coreboot.org/c/coreboot/+/54308 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I80a8342c51d7bfaa0cb2eb3fd37240425d5901be Gerrit-Change-Number: 54308 Gerrit-PatchSet: 2 Gerrit-Owner: Aseda Aboagye <aaboagye(a)google.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Aseda Aboagye <aaboagye(a)google.com> Gerrit-Attention: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Comment-Date: Sat, 15 May 2021 00:24:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2021