coreboot-gerrit May 2021

coreboot-gerrit@coreboot.org

1 participants
6048 discussions

Change in coreboot[master]: chromeos/Kconfig: Add TPM20_CREATE_FWMP
by Aseda Aboagye (Code Review) 04 May '21

04 May '21

Attention is currently required from: Andrey Pronin, Julius Werner, 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 4: (2 comments) File src/security/vboot/secdata_tpm.c: https://review.coreboot.org/c/coreboot/+/52919/comment/d16033ca_c21a7348 PS1, Line 243: rv = tlcl_define_space(FWMP_NV_INDEX, VB2_SECDATA_FWMP_MAX_SIZE, > vboot will be unhappy (later, in depthcharge) when it tries to read a FWMP space that exists but isn […] My understanding was that it simply considers the space as missing since the read would be returning `TPM_E_BADINDEX`. But now going back, I think I might have been looking at the wrong tlcl code. (in coreboot vs depthcharge). Another option is to port that same behaviour to the tlcl code in depthchage. File src/vendorcode/google/chromeos/Kconfig: https://review.coreboot.org/c/coreboot/+/52919/comment/39d11d5b_9d2ca49e 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 d […] (sorry, missed this comment earlier) I had the same question myself. :) I think it could work the same on Cr50 devices. I simply chose this way to minimize the changes to the other boards. -- 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: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Comment-Date: Wed, 05 May 2021 00:37:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Comment-In-Reply-To: Aseda Aboagye <aaboagye(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: chromeos/Kconfig: Add TPM20_CREATE_FWMP
by Julius Werner (Code Review) 04 May '21

04 May '21

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 4: (1 comment) File src/security/vboot/secdata_tpm.c: https://review.coreboot.org/c/coreboot/+/52919/comment/bf08da76_ce17ba0d PS1, Line 243: rv = tlcl_define_space(FWMP_NV_INDEX, VB2_SECDATA_FWMP_MAX_SIZE, > Per cylai@, cryptohome doesn't really care if the space has been written to or not, so I had elected […] vboot will be unhappy (later, in depthcharge) when it tries to read a FWMP space that exists but isn't valid. -- 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:21:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No 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]: vc/amd/fsp/cezanne/FspGuids: add AMD_FSP_ACPI_ALIB_HOB_GUID
by Marshall Dawson (Code Review) 04 May '21

04 May '21

Attention is currently required from: Felix Held. Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52903 ) Change subject: vc/amd/fsp/cezanne/FspGuids: add AMD_FSP_ACPI_ALIB_HOB_GUID ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/52903 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I531c8e8d0ee2aa72b51cba59e09e7a7d253da4f9 Gerrit-Change-Number: 52903 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Matt Papageorge <matthewpapa07(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 05 May 2021 00:21:15 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/cezanne/agesa_acpi: add and call agesa_write_acpi_tables
by Marshall Dawson (Code Review) 04 May '21

04 May '21

Attention is currently required from: Jason Glenesk, Felix Held. Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52902 ) Change subject: soc/amd/cezanne/agesa_acpi: add and call agesa_write_acpi_tables ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/52902 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia88cb5ea483850a8659f3bae8040c82eb2735d26 Gerrit-Change-Number: 52902 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:19:55 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: chromeos/Kconfig: Add TPM20_CREATE_FWMP
by Aseda Aboagye (Code Review) 04 May '21

04 May '21

Attention is currently required from: Andrey Pronin, Julius Werner, 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 4: (4 comments) File src/security/vboot/secdata_tpm.c: https://review.coreboot.org/c/coreboot/+/52919/comment/33f3e44a_62239458 PS1, Line 111: > I'd put the attribute definition here so we have them all in one place to compare. Ack https://review.coreboot.org/c/coreboot/+/52919/comment/fe3c6e98_52aea558 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 […] I recall reading a comment in a doc that we should consider FWMP as frozen. If we did want to modify the struct, we should create a new space? Andrey may know more here. I don't see any drawback to adding the attribute though. https://review.coreboot.org/c/coreboot/+/52919/comment/c8cc88f5_77d35957 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. […] Per cylai@, cryptohome doesn't really care if the space has been written to or not, so I had elected to not initialize the space. If we think we still should, I can definitely do so. I did notice that the other spaces are initialized immediately after definition. https://review.coreboot.org/c/coreboot/+/52919/comment/cce6655e_cf1efe02 PS1, Line 244: pcr0_allowed_policy, > I don't think this has any meaning when POLICY_DELETE isn't set, unless Andrey says otherwise. […] Ack -- 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: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Comment-Date: Wed, 05 May 2021 00:19:14 +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]: soc/amd/picasso/agesa_acpi: add comment to add_agesa_fsp_acpi_table call
by Marshall Dawson (Code Review) 04 May '21

04 May '21

Attention is currently required from: Jason Glenesk, Felix Held. Marshall Dawson has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52901 ) Change subject: soc/amd/picasso/agesa_acpi: add comment to add_agesa_fsp_acpi_table call ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/52901 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I409993dcecd38bd2ad603ba467b299a6eab177ab Gerrit-Change-Number: 52901 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:18:15 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/picasso/agesa_acpi: add missing device/device.h include
by Marshall Dawson (Code Review) 04 May '21

04 May '21

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

1 0

Change in coreboot[master]: soc/amd/common/fsp/fsp-acpi: add check for maximum table size
by Marshall Dawson (Code Review) 04 May '21

04 May '21

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

1 0

Change in coreboot[master]: soc/amd/common/fsp/fsp-acpi: factor out SSDT from HOB functionality
by Marshall Dawson (Code Review) 04 May '21

04 May '21

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

1 0

Change in coreboot[master]: soc/amd/common/fsp/pci: Add helper methods for PCI IRQ table
by Karthik Ramasubramanian (Code Review) 04 May '21

04 May '21

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

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2021