Attention is currently required from: Jason Glenesk, Felix Held.
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52899 )
Change subject: soc/amd/picasso/agesa_acpi: add missing device/device.h include
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/52899
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7892cf680661253f74c3e291f5e9fb372e1d4ce3
Gerrit-Change-Number: 52899
Gerrit-PatchSet: 1
Gerrit-Owner: 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-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Wed, 05 May 2021 00:17:53 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Jason Glenesk, Felix Held.
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52897 )
Change subject: soc/amd/common/fsp/fsp-acpi: add check for maximum table size
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/52897
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I965c01bd9ab66b14d6f77b6f23c28479ae6d6a50
Gerrit-Change-Number: 52897
Gerrit-PatchSet: 1
Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)amd.corp-partner.google.com>
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-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Wed, 05 May 2021 00:14:51 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Jason Glenesk, Felix Held.
Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52896 )
Change subject: soc/amd/common/fsp/fsp-acpi: factor out SSDT from HOB functionality
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://review.coreboot.org/c/coreboot/+/52896
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I7b256de712fe60d1c021cb875aaadec1d331584b
Gerrit-Change-Number: 52896
Gerrit-PatchSet: 1
Gerrit-Owner: 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-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com>
Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de>
Gerrit-Comment-Date: Wed, 05 May 2021 00:12:14 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson, Felix Held.
Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52911 )
Change subject: soc/amd/common/fsp/pci: Add helper methods for PCI IRQ table
......................................................................
Patch Set 1:
(1 comment)
File src/soc/amd/common/fsp/pci/pci_routing_info.c:
https://review.coreboot.org/c/coreboot/+/52911/comment/cfb9f599_de190b19
PS1, Line 29: routing_table
Can this be a static variable, so that we can lookup the HOB once and then re-use it for subsequent calls of get_pci_routing_table?
--
To view, visit https://review.coreboot.org/c/coreboot/+/52911
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: Id03d0b74ca12e7bcee11f8d13b0e802861c13923
Gerrit-Change-Number: 52911
Gerrit-PatchSet: 1
Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org>
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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Karthik Ramasubramanian <kramasub(a)google.com>
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: Wed, 05 May 2021 00:02:33 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Andrey Pronin, Aseda Aboagye, 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 4:
(1 comment)
File src/security/vboot/secdata_tpm.c:
Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-118447):
https://review.coreboot.org/c/coreboot/+/52919/comment/276fd449_f7e6344e
PS4, 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: 4
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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
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: Wed, 05 May 2021 00:00:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Andrey Pronin, Aseda Aboagye, Aaron Durbin.
Hello build bot (Jenkins), Andrey Pronin, 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 (#4).
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/4
--
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: 4
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: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
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-MessageType: newpatchset
Attention is currently required from: Andrey Pronin, Aseda Aboagye, Aaron Durbin.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52919 )
Change subject: chromeos/Kconfig: Add TPM20_CREATE_FWMP
......................................................................
Patch Set 3:
(5 comments)
File src/security/vboot/secdata_tpm.c:
https://review.coreboot.org/c/coreboot/+/52919/comment/739bf9ef_8b1c1449
PS1, Line 111:
I'd put the attribute definition here so we have them all in one place to compare.
https://review.coreboot.org/c/coreboot/+/52919/comment/d7633e7f_01f35a81
PS1, Line 237: .TPMA_NV_OWNERWRITE = 1,
Should we also add PPWRITE just in case the RW firmware ever needs to make modifications there? (Not currently needed but may be useful to keep open as an option for unexpected future requirements.)
https://review.coreboot.org/c/coreboot/+/52919/comment/1292c9a6_6cbefd73
PS1, Line 243: rv = tlcl_define_space(FWMP_NV_INDEX, VB2_SECDATA_FWMP_MAX_SIZE,
Why not reuse set_space()? We should initialize the FWMP with valid data too. You can add a vb2_secdata_fwmp_create() function to vboot to do that. (Also, should probably wrap the set_space() invocation in another set_fwmp_space() just for consistency with the others.)
https://review.coreboot.org/c/coreboot/+/52919/comment/b9eda9a6_ff8d6cbd
PS1, Line 244: pcr0_allowed_policy,
I don't think this has any meaning when POLICY_DELETE isn't set, unless Andrey says otherwise. I think you just want (NULL, 0) here like for set_kernel_space().
File src/vendorcode/google/chromeos/Kconfig:
https://review.coreboot.org/c/coreboot/+/52919/comment/fbc28c9d_bb60bfc1
PS1, Line 95: config TPM20_CREATE_FWMP
Not sure why this should be a Kconfig? Don't we just want to do this unconditionally on all future devices? (I remember discussing this with Andrey but can't find it anymore... basically, I think it would be good to just get off the custom cr50 hacks and do things this way on all devices going forward.)
--
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: Andrey Pronin <apronin(a)chromium.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Julius Werner <jwerner(a)chromium.org>
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: Tue, 04 May 2021 23:59:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
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