Attention is currently required from: Aseda Aboagye, Aaron Durbin.
Aseda Aboagye has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52919 )
Change subject: chromeos/Kconfig: Add TPM20_CREATE_FWMP
......................................................................
Patch Set 3:
(1 comment)
File src/security/vboot/secdata_tpm.c:
https://review.coreboot.org/c/coreboot/+/52919/comment/2671eb19_0a808720
PS1, Line 246: if (rv == TPM_E_NV_DEFINED) {
> Sorry, I missed this was in _factory_initialized_tpm(). […]
Ah, I see what you mean now. Yes, sorry, we want to continue if we're successful.
--
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: 3
Gerrit-Owner: Aseda Aboagye <aaboagye(a)google.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(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: Tue, 04 May 2021 23:57:52 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Aseda Aboagye <aaboagye(a)google.com>
Comment-In-Reply-To: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Aseda Aboagye.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52919 )
Change subject: chromeos/Kconfig: Add TPM20_CREATE_FWMP
......................................................................
Patch Set 3:
(1 comment)
File src/security/vboot/secdata_tpm.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-118445):
https://review.coreboot.org/c/coreboot/+/52919/comment/5dd7b43d_a4f6dea9
PS3, Line 249: * This scenario is rare as it should only occur if we succeded in
'succeded' may be misspelled - perhaps 'succeeded'?
--
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: 3
Gerrit-Owner: Aseda Aboagye <aaboagye(a)google.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Aseda Aboagye <aaboagye(a)google.com>
Gerrit-Comment-Date: Tue, 04 May 2021 23:56:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Aseda Aboagye.
Hello build bot (Jenkins), Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/52919
to look at the new patch set (#3).
Change subject: chromeos/Kconfig: Add TPM20_CREATE_FWMP
......................................................................
chromeos/Kconfig: Add TPM20_CREATE_FWMP
This commit adds a new config option, TPM20_CREATE_FWMP which
instructs coreboot to create the Chrome OS Firmware Management
Parameters space in the TPM. When selected, coreboot will simply
define this space, but not initialize the contents. The space can be
written to by the TPM owner from userspace.
BUG=b:184677625
BRANCH=None
TEST=emerge-keeby coreboot
Signed-off-by: Aseda Aboagye <aaboagye(a)google.com>
Change-Id: I1f566e00f11046ff9a9891c65660af50fbb83675
---
M src/security/vboot/secdata_tpm.c
M src/vendorcode/google/chromeos/Kconfig
2 files changed, 38 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/52919/3
--
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: 3
Gerrit-Owner: Aseda Aboagye <aaboagye(a)google.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Aseda Aboagye <aaboagye(a)google.com>
Gerrit-MessageType: newpatchset
Attention is currently required from: Aseda Aboagye.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52919 )
Change subject: chromeos/Kconfig: Add TPM20_CREATE_FWMP
......................................................................
Patch Set 2:
(1 comment)
File src/security/vboot/secdata_tpm.c:
https://review.coreboot.org/c/coreboot/+/52919/comment/07a50f55_d4d85120
PS1, Line 246: if (rv == TPM_E_NV_DEFINED) {
> Negative. This scenario should be rare (e.g. […]
Sorry, I missed this was in _factory_initialized_tpm(). It might be more self explanatory if I was more perceptive.
My next question is if it wasn't defined we'd return from the function w/o calling set_firmware_space()? Basically if rv == TPM_SUCCESS or (rv != TPM_E_NV_DEFINED) we're returning early. Should the 'else' be 'else if (rv != TPM_SUCCESS)' or whatever the successful return value?
--
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: 2
Gerrit-Owner: Aseda Aboagye <aaboagye(a)google.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Aseda Aboagye <aaboagye(a)google.com>
Gerrit-Comment-Date: Tue, 04 May 2021 23:53:01 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Aseda Aboagye <aaboagye(a)google.com>
Comment-In-Reply-To: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Aaron Durbin.
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52919 )
Change subject: chromeos/Kconfig: Add TPM20_CREATE_FWMP
......................................................................
Patch Set 2:
(1 comment)
File src/security/vboot/secdata_tpm.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-118443):
https://review.coreboot.org/c/coreboot/+/52919/comment/10415da8_6cbafb0f
PS2, Line 249: * This scenario is rare as it should only occur if we succeded in
'succeded' may be misspelled - perhaps 'succeeded'?
--
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: 2
Gerrit-Owner: Aseda Aboagye <aaboagye(a)google.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Comment-Date: Tue, 04 May 2021 23:51:24 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Aseda Aboagye, Aaron Durbin.
Hello build bot (Jenkins), Aaron Durbin,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/52919
to look at the new patch set (#2).
Change subject: chromeos/Kconfig: Add TPM20_CREATE_FWMP
......................................................................
chromeos/Kconfig: Add TPM20_CREATE_FWMP
This commit adds a new config option, TPM20_CREATE_FWMP which
instructs coreboot to create the Chrome OS Firmware Management
Parameters space in the TPM. When selected, coreboot will simply
define this space, but not initialize the contents. The space can be
written to by the TPM owner from userspace.
BUG=b:184677625
BRANCH=None
TEST=emerge-keeby coreboot
Signed-off-by: Aseda Aboagye <aaboagye(a)google.com>
Change-Id: I1f566e00f11046ff9a9891c65660af50fbb83675
---
M src/security/vboot/secdata_tpm.c
M src/vendorcode/google/chromeos/Kconfig
2 files changed, 38 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/19/52919/2
--
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: 2
Gerrit-Owner: Aseda Aboagye <aaboagye(a)google.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(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
Attention is currently required from: Aseda Aboagye.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52920 )
Change subject: mb/google/dedede: Select TPM20_CREATE_FWMP for discrete TPM boards
......................................................................
Patch Set 1:
(1 comment)
File src/mainboard/google/dedede/Kconfig:
https://review.coreboot.org/c/coreboot/+/52920/comment/563f39fb_88e46be6
PS1, Line 167: select TPM20_CREATE_FWMP
Doesn't this selection need to be under another config? Basically move this to line 41 or 42. Btw, I *think* Andrey wanted to do this for all Chrome OS devices going forward. So we may want to just do it unconditionally for all Chrome OS devices going forward.
--
To view, visit https://review.coreboot.org/c/coreboot/+/52920
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I6c49f2018baa17df8c932b7a7d7f1aec28749ff2
Gerrit-Change-Number: 52920
Gerrit-PatchSet: 1
Gerrit-Owner: Aseda Aboagye <aaboagye(a)google.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Attention: Aseda Aboagye <aaboagye(a)google.com>
Gerrit-Comment-Date: Tue, 04 May 2021 23:49:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Aaron Durbin.
Aseda Aboagye has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52919 )
Change subject: chromeos/Kconfig: Add TPM20_CREATE_FWMP
......................................................................
Patch Set 1:
(1 comment)
File src/security/vboot/secdata_tpm.c:
https://review.coreboot.org/c/coreboot/+/52919/comment/c40b1729_84b6db5d
PS1, Line 246: if (rv == TPM_E_NV_DEFINED) {
> Is this our expected return value? If so, could you please add a comment indicating expectations an […]
Negative. This scenario should be rare (e.g. we define the FWMP space, but lose power before setting the firmware space). However, I can add this brief description to this portion.
--
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: 1
Gerrit-Owner: Aseda Aboagye <aaboagye(a)google.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Attention: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Comment-Date: Tue, 04 May 2021 23:46:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-MessageType: comment
Attention is currently required from: Aseda Aboagye.
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52919 )
Change subject: chromeos/Kconfig: Add TPM20_CREATE_FWMP
......................................................................
Patch Set 1:
(1 comment)
File src/security/vboot/secdata_tpm.c:
https://review.coreboot.org/c/coreboot/+/52919/comment/b094ab81_134e3b97
PS1, Line 246: if (rv == TPM_E_NV_DEFINED) {
Is this our expected return value? If so, could you please add a comment indicating expectations and flow of the creation of this space?
--
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: 1
Gerrit-Owner: Aseda Aboagye <aaboagye(a)google.com>
Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org>
Gerrit-Attention: Aseda Aboagye <aaboagye(a)google.com>
Gerrit-Comment-Date: Tue, 04 May 2021 23:43:19 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment