coreboot-gerrit May 2021

coreboot-gerrit@coreboot.org

1 participants
6048 discussions

Change in coreboot[master]: soc/intel/common: Add function to lpc_lib to return PIRQ routing
by Tim Wawrzynczak (Code Review) 05 May '21

05 May '21

Attention is currently required from: Furquan Shaikh, Angel Pons. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50858 ) Change subject: soc/intel/common: Add function to lpc_lib to return PIRQ routing ...................................................................... Patch Set 15: (1 comment) File src/soc/intel/common/block/include/intelblocks/itss.h: https://review.coreboot.org/c/coreboot/+/50858/comment/69130eb0_aed80001 PS14, Line 7: MAX_PXRC_CONFIG > I think this can be dropped now and we can use `PIRQ_COUNT` which was added in CB:50857. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/50858 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib215fba54573c50a88aa4584442bd8d27ae017be Gerrit-Change-Number: 50858 Gerrit-PatchSet: 15 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Wed, 05 May 2021 22:35:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/tigerlake: Enable support for common IRQ block
by Tim Wawrzynczak (Code Review) 05 May '21

05 May '21

Hello build bot (Jenkins), Furquan Shaikh, Duncan Laurie, Nick Vaccaro, Aamir Bohra, Srinidhi N Kaushik, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/49409 to look at the new patch set (#21). Change subject: soc/intel/tigerlake: Enable support for common IRQ block ...................................................................... soc/intel/tigerlake: Enable support for common IRQ block Since GPIO IO-APIC IRQs are fixed in hardware (RO registers), this patch allows tigerlake boards to dynamically assign PCI IRQs. This means not relying on FSP defaults, which eliminates the problem of PCI IRQs interfering with GPIO IRQs routed to the same IRQ, when both have selected IO-APIC routing. Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Change-Id: Ieb241f2b91af52a7e2d0efe997d35732882ac463 --- M src/soc/intel/tigerlake/Kconfig D src/soc/intel/tigerlake/acpi/pci_irqs.asl M src/soc/intel/tigerlake/acpi/southbridge.asl M src/soc/intel/tigerlake/chip.c M src/soc/intel/tigerlake/fsp_params.c 5 files changed, 152 insertions(+), 164 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/09/49409/21 -- To view, visit https://review.coreboot.org/c/coreboot/+/49409 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ieb241f2b91af52a7e2d0efe997d35732882ac463 Gerrit-Change-Number: 49409 Gerrit-PatchSet: 21 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Duncan Laurie Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/intel/common/block/irq: Add support for intel_write_pci0_PRT
by Tim Wawrzynczak (Code Review) 05 May '21

05 May '21

Attention is currently required from: Nico Huber, Furquan Shaikh, Patrick Rudolph. Hello build bot (Jenkins), Nico Huber, Furquan Shaikh, Nick Vaccaro, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/51159 to look at the new patch set (#11). Change subject: soc/intel/common/block/irq: Add support for intel_write_pci0_PRT ...................................................................... soc/intel/common/block/irq: Add support for intel_write_pci0_PRT Add a new function to fill out the data structures necessary to generate a _PRT table. Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Change-Id: I21a4835890ca03bff83ed0e8791441b3af54cb62 --- M src/soc/intel/common/block/include/intelblocks/irq.h M src/soc/intel/common/block/irq/irq.c 2 files changed, 50 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/59/51159/11 -- To view, visit https://review.coreboot.org/c/coreboot/+/51159 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I21a4835890ca03bff83ed0e8791441b3af54cb62 Gerrit-Change-Number: 51159 Gerrit-PatchSet: 11 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/intel/common: Add new IRQ module
by Tim Wawrzynczak (Code Review) 05 May '21

05 May '21

Attention is currently required from: Nico Huber. Hello build bot (Jenkins), Furquan Shaikh, Patrick Georgi, Martin Roth, Duncan Laurie, Nick Vaccaro, Arthur Heymans, Aamir Bohra, Srinidhi N Kaushik, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/49408 to look at the new patch set (#27). Change subject: soc/intel/common: Add new IRQ module ...................................................................... soc/intel/common: Add new IRQ module The Intel FSP provides a default set of IO-APIC IRQs for PCI devices, if the DevIntConfigPtr UPD is not filled in. However, the FSP has a list of rules that the input IRQ table must conform to: 1) One entry per slot/function 2) Functions using PIRQs must use IOxAPIC IRQs 16-23 3) Single-function devices must use INTA 4) Each slot must have consistent INTx<->PIRQy mappings 5) Some functions have special interrupt pin requirements 6) PCI Express RPs must be assigned in a special way (FIXED_INT_PIN) 7) Some functions require a unique IRQ number 8) PCI functions must avoid sharing an IRQ with a GPIO pad which routes its IRQ through IO-APIC. Since the FSP has no visibility into the actual GPIOs used on the board when GpioOverride is selected, IRQ conflicts can occur between PCI devices and GPIOs. This patch gives SoC code the ability to generate a table of PCI IRQs that will meet the BWG/FSP rules and also not conflict with GPIO IRQs. BUG=b:171580862 TEST=Boot with patch series on volteer, verify IO-APIC IRQs in `/proc/interrupts` match what is expected. No `GSI INT` or `could not derive routing` messages seen in `dmesg` output. Verified TPM, touchpad, touchscreen IRQs all function as expected. Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Change-Id: I0c22a08ce589fa80d0bb1e637422304a3af2045c --- A src/soc/intel/common/block/include/intelblocks/irq.h A src/soc/intel/common/block/irq/Kconfig A src/soc/intel/common/block/irq/Makefile.inc A src/soc/intel/common/block/irq/irq.c 4 files changed, 402 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/08/49408/27 -- To view, visit https://review.coreboot.org/c/coreboot/+/49408 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0c22a08ce589fa80d0bb1e637422304a3af2045c Gerrit-Change-Number: 49408 Gerrit-PatchSet: 27 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Aamir Bohra <aamir.bohra(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Duncan Laurie Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Srinidhi N Kaushik <srinidhi.n.kaushik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: soc/intel/common: Add function to lpc_lib to return PIRQ routing
by Tim Wawrzynczak (Code Review) 05 May '21

05 May '21

Attention is currently required from: Tim Wawrzynczak, Angel Pons. Hello build bot (Jenkins), Nico Huber, Furquan Shaikh, Angel Pons, Nick Vaccaro, Michael Niewöhner, Patrick Rudolph, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/50858 to look at the new patch set (#15). Change subject: soc/intel/common: Add function to lpc_lib to return PIRQ routing ...................................................................... soc/intel/common: Add function to lpc_lib to return PIRQ routing In order to fill out static entries for a _PRT table for soc/intel/common, the PIRQ<->IRQ mapping is required. This patch adds a function lpc_get_pch_pirq_routing() which returns this mapping. Signed-off-by: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Change-Id: Ib215fba54573c50a88aa4584442bd8d27ae017be --- M src/soc/intel/common/block/include/intelblocks/itss.h M src/soc/intel/common/block/include/intelblocks/lpc_lib.h M src/soc/intel/common/block/itss/itss.c M src/soc/intel/common/block/lpc/lpc_lib.c 4 files changed, 24 insertions(+), 18 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/58/50858/15 -- To view, visit https://review.coreboot.org/c/coreboot/+/50858 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib215fba54573c50a88aa4584442bd8d27ae017be Gerrit-Change-Number: 50858 Gerrit-PatchSet: 15 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: tests: Enable config override for tests
by Julius Werner (Code Review) 05 May '21

05 May '21

Attention is currently required from: Jakub Czapiga, Paul Fagerburg, Jan Dabros. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52937 ) Change subject: tests: Enable config override for tests ...................................................................... Patch Set 1: (5 comments) Patchset: PS1: Whoops, I actually wrote the same thing up a few weeks ago but then got distracted and didn't get around to uploading it. Oh well. Good to see we came up with almost the same implementation, at least. File tests/Makefile.inc: https://review.coreboot.org/c/coreboot/+/52937/comment/38b1af23_46e1afd4 PS1, Line 88: $(eval $(1)-config-file := $(obj)/$(1)/config.h) I'm confused why this needs to be an eval? I think you can just do $(1)-config-file := ... here, but then you should use $$($(1)-config-file) to refer to it later. https://review.coreboot.org/c/coreboot/+/52937/comment/39e825fc_1bc10618 PS1, Line 89: $($(1)-config-file): $(TEST_KCONFIG_AUTOHEADER) nit: Doesn't really need to be a separate line on its own. https://review.coreboot.org/c/coreboot/+/52937/comment/36651acd_ec6559f5 PS1, Line 97: printf '#ifdef %s\n' "$$$$key" >> $$@; \ nit: don't really need the #ifdef wrapper around the #undef https://review.coreboot.org/c/coreboot/+/52937/comment/ea60253d_f46d9c7c PS1, Line 107: $(TEST_KCONFIG_AUTOHEADER) Just replace this with $(1)-config-file, then you don't need the extra line above -- To view, visit https://review.coreboot.org/c/coreboot/+/52937 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1aeb78362c2609fbefbfd91c0f58ec19ed258ee1 Gerrit-Change-Number: 52937 Gerrit-PatchSet: 1 Gerrit-Owner: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Reviewer: Jan Dabros <jsd(a)semihalf.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Paul Fagerburg <pfagerburg(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Attention: Paul Fagerburg <pfagerburg(a)chromium.org> Gerrit-Attention: Jan Dabros <jsd(a)semihalf.com> Gerrit-Comment-Date: Wed, 05 May 2021 22:30:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Documentation: Add suggestion to document flag days
by Patrick Georgi (Code Review) 05 May '21

05 May '21

Attention is currently required from: Martin Roth, Julius Werner, Felix Held. Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52576 ) Change subject: Documentation: Add suggestion to document flag days ...................................................................... Patch Set 1: (1 comment) File Documentation/getting_started/gerrit_guidelines.md: https://review.coreboot.org/c/coreboot/+/52576/comment/fda4055f_e116a27f PS1, Line 332: requires a change in all mainboards using it) needs to be documented. > I understand what this proposal is trying to get at, but as written this is completely impractical. […] Would "the code has shipped on a product" be a good break-off point? At that point the quick iterations should be mostly over, and if there are multiple teams working on the same pre-production SoC[1] they better collaborate, for which upstream is a great place - any team trying to opt out of that should bear the costs themselves instead of expecting others to accommodate them. However, once stuff is shipping and development for that platform has slowed down, any kind of rework of the code should be documented so that upstream changes can be brought into local stabilization branches (e.g. ChromeOS' firmware branches!) more easily, and local development branches have an easier time when it comes to upstreaming. In Felix' patchset level comment I proposed "two different vendors" but that's actually not a good benchmark because even if a chipset is exclusively used by one vendor (e.g. Google) at the time, once it's settled down, I think we should document changes sufficiently so that external development can be brought in with relative ease. Consider: Even if a vendor doesn't intend to upstream, they still have to provide their sources to their customers (thanks to the GPL). Helping those customers upstream such custom development can go a long way to help bring separate lines of development back "home". [1] or whatever component they share -- To view, visit https://review.coreboot.org/c/coreboot/+/52576 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ic7c01a0f1008821ebbda43a426ba1e3d6410e861 Gerrit-Change-Number: 52576 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 05 May 2021 22:27:46 +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/intel/alderlake: Add CrashLog implementation for Intel ADL
by Francois Toguo Fotso (Code Review) 05 May '21

05 May '21

Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Paul Menzel. Francois Toguo Fotso has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52454 ) Change subject: soc/intel/alderlake: Add CrashLog implementation for Intel ADL ...................................................................... Patch Set 7: (1 comment) Patchset: PS7: Hey Furquan, All the comments in this review have been resolved. Could we please get help merge this CL? Regards, Francois -- To view, visit https://review.coreboot.org/c/coreboot/+/52454 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I15ba0b41f73c1772f09584f13bcf5585caa90782 Gerrit-Change-Number: 52454 Gerrit-PatchSet: 7 Gerrit-Owner: Francois Toguo Fotso <francois.toguo.fotso(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Raj Astekar <raj.astekar(a)intel.com> Gerrit-CC: Selma Bensaid <selma.bensaid(a)intel.com> Gerrit-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Comment-Date: Wed, 05 May 2021 22:16:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: lib/romstage_handoff.c: Initialize using a cbmem hook
by Julius Werner (Code Review) 05 May '21

05 May '21

Attention is currently required from: Damien Zammit, Lee Leahy, Huang Jin, Arthur Heymans, Kyösti Mälkki, Andrey Petrov. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52933 ) Change subject: lib/romstage_handoff.c: Initialize using a cbmem hook ...................................................................... Patch Set 3: (1 comment) File src/lib/romstage_handoff.c: https://review.coreboot.org/c/coreboot/+/52933/comment/8e22bb65_99b2196f PS3, Line 56: ROMSTAGE_CBMEM_INIT_HOOK(romstage_handoff_init); Since this file is compiled unconditionally, adding an INIT_HOOK() expands this to a lot of platforms which never used it. Please add a Kconfig to limit compilation of the file to the right set of boards if you want to do this (I'm not quite sure what exactly this is or what the right set of boards for it would be, but I know the Arm boards I work on don't use this, for example). -- To view, visit https://review.coreboot.org/c/coreboot/+/52933 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I93abf33a6eb055eeaba1fed5042387822de1aa20 Gerrit-Change-Number: 52933 Gerrit-PatchSet: 3 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Damien Zammit Gerrit-Reviewer: Huang Jin <huang.jin(a)intel.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Lee Leahy <leroy.p.leahy(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Damien Zammit Gerrit-Attention: Lee Leahy <leroy.p.leahy(a)intel.com> Gerrit-Attention: Huang Jin <huang.jin(a)intel.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Comment-Date: Wed, 05 May 2021 22:13:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

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

05 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 5: (4 comments) Patchset: PS5: > I added a new API in vboot_reference. […] You need to uprev the submodule. First, the vboot change needs to get merged. Then, go into your coreboot checkout, cd into 3rdparty/vboot, fetch and checkout the new updated main branch (you may need to add it separately as a remote first), then go back into the coreboot toplevel directory and you can check that in like a commit. See older patches like CB:47784 File src/security/vboot/secdata_tpm.c: https://review.coreboot.org/c/coreboot/+/52919/comment/196abef7_ba4dca5f PS1, Line 243: rv = tlcl_define_space(FWMP_NV_INDEX, VB2_SECDATA_FWMP_MAX_SIZE, > I'm initializing the space now. Well, reading through the TPM spec I guess it should actually return TPM_RC_NV_UNINITIALIZED, but that's not good either. We have special consideration for the FMWP not existing at all (BADINDEX) in depthcharge, but not for any other error code. See depthcharge/src/vboot/secdata_tpm.c#secdata_fwmp_read() File src/security/vboot/secdata_tpm.c: https://review.coreboot.org/c/coreboot/+/52919/comment/a69f1641_a0397001 PS5, Line 200: VB2_SECDATA_FWMP_MAX_SIZE Actually, thinking it through again this needs to be the size of the currently used FWMP structure, not MAX_SIZE (see explanation in CL:2875533 for the difference). That size should really come from the result of the create() function. Maybe we should reshuffle these set_xxx_space() functions a bit so they take the whole context pointer and call the respective create() function themselves so they can use that result right away (and maybe they should be called setup_xxx_space() or something? Just "set" sounds more like (over-)writing than creating...). File src/vendorcode/google/chromeos/Kconfig: https://review.coreboot.org/c/coreboot/+/52919/comment/1db82b80_2d24598a PS1, Line 95: config TPM20_CREATE_FWMP > (sorry, missed this comment earlier) I had the same question myself. […] Let's just switch all boards over at once and avoid more fragmentation. -- 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: 5 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 22:05:40 +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

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2021