coreboot-gerrit November 2022

coreboot-gerrit@coreboot.org

1 participants
7286 discussions

[M] Change in coreboot[master]: device/pciexp: add pcie_find_dsn()
by Marc Jones (Code Review) 18 Nov '22

18 Nov '22

Attention is currently required from: Jonathan Zhang, Stefan Reinauer, David Hendricks, Angel Pons, Subrata Banik, ron minnich. Marc Jones has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/54510 ) Change subject: device/pciexp: add pcie_find_dsn() ...................................................................... Patch Set 14: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/54510 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I54b6dc42c8da47cd7b4447ab23a6a21562c7618b Gerrit-Change-Number: 54510 Gerrit-PatchSet: 14 Gerrit-Owner: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Angel Pons <angel.pons(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: David Hendricks Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Marc Jones <marc(a)marcjonesconsulting.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: ron minnich <rminnich(a)gmail.com> Gerrit-CC: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: David Hendricks Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Angel Pons <angel.pons(a)9elements.com> Gerrit-Attention: ron minnich <rminnich(a)gmail.com> Gerrit-Comment-Date: Fri, 18 Nov 2022 17:48:54 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: acpi: Add initial support for CEDT
by Marc Jones (Code Review) 18 Nov '22

18 Nov '22

Attention is currently required from: Arthur Heymans, Jonathan Zhang, Tim Wawrzynczak, Paul Menzel, Angel Pons. Marc Jones has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/64263 ) Change subject: acpi: Add initial support for CEDT ...................................................................... Patch Set 9: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/64263 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4fbce78efc86ad9f2468c37b4827a6dadbdc6802 Gerrit-Change-Number: 64263 Gerrit-PatchSet: 9 Gerrit-Owner: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Marc Jones <marc(a)marcjonesconsulting.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Fri, 18 Nov 2022 17:42:09 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: soc/amd/common/acpi: Implement DTTS Proposal
by EricKY Cheng (Code Review) 18 Nov '22

18 Nov '22

Attention is currently required from: Raul Rangel, Jason Nien, Matt DeVillier, Paul Menzel, Eric Lai, Martin Roth, Tim Van Patten, Eric Peers, LeilaCY Lin, Jason Glenesk, Dtrain Hsu, Caveh Jalali, Tim Wawrzynczak, Fred Reitberger, Karthikeyan Ramasubramanian, Boris Mittelberg, Felix Held. EricKY Cheng has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68471 ) Change subject: soc/amd/common/acpi: Implement DTTS Proposal ...................................................................... Patch Set 57: (3 comments) Patchset: PS30: > I found that I will get 3 body detection events continuously during boot without body status change. […] Done File src/mainboard/google/skyrim/dsdt.asl: https://review.coreboot.org/c/coreboot/+/68471/comment/23b31ac6_18d2830c PS30, Line 18: variants/winterhold/DTTS.asl > I move DTTS. […] Done File src/soc/amd/common/acpi/dptc.asl: https://review.coreboot.org/c/coreboot/+/68471/comment/381bf476_2237303b PS21, Line 46: // AMB sensor trigger point > I update the commit description with table translation rule and new code change following these rule […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/68471 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I866e5e497e2936984e713029b5f0b6d54cbc9622 Gerrit-Change-Number: 68471 Gerrit-PatchSet: 57 Gerrit-Owner: EricKY Cheng <ericky_cheng(a)compal.corp-partner.google.com> Gerrit-Reviewer: Boris Mittelberg <bmbm(a)google.com> Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org> Gerrit-Reviewer: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> Gerrit-Reviewer: Eric Peers <epeers(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Reviewer: Karthikeyan Ramasubramanian <kramasub(a)chromium.org> Gerrit-Reviewer: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Tim Van Patten <timvp(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-CC: LeilaCY Lin <leilacy_lin(a)compal.corp-partner.google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Jason Nien <jason.nien(a)amd.corp-partner.google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Attention: Martin Roth <martin.roth(a)amd.corp-partner.google.com> Gerrit-Attention: Tim Van Patten <timvp(a)google.com> Gerrit-Attention: Eric Peers <epeers(a)google.com> Gerrit-Attention: LeilaCY Lin <leilacy_lin(a)compal.corp-partner.google.com> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Dtrain Hsu <dtrain_hsu(a)compal.corp-partner.google.com> Gerrit-Attention: Caveh Jalali <caveh(a)chromium.org> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Karthikeyan Ramasubramanian <kramasub(a)chromium.org> Gerrit-Attention: Boris Mittelberg <bmbm(a)google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 18 Nov 2022 17:36:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org> Comment-In-Reply-To: EricKY Cheng <ericky_cheng(a)compal.corp-partner.google.com> Comment-In-Reply-To: Tim Van Patten <timvp(a)google.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: cpu/intel/socket_*: Clean up Kconfig files
by Arthur Heymans (Code Review) 18 Nov '22

18 Nov '22

Attention is currently required from: Stefan Ott, Nico Huber, Kyösti Mälkki, Alexander Couzens, Elyes Haouas. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69447 ) Change subject: cpu/intel/socket_*: Clean up Kconfig files ...................................................................... Patch Set 4: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/69447/comment/8bc3fcf8_dcede6eb PS3, Line 10: Move MAX_CPUS to mainboards. > The problem with having the defaults for `MAX_CPUS` in `cpu/intel/model_*/Kconfig` is that sockets that support multiple CPU models (e.g. LGA775) could end up using an incorrect default depending on the order in which the CPU models' Kconfig files are included. This happens because of the way Kconfig handles defaults (it uses the first applicable value it finds). > > It's true that the status quo for `MAX_CPUS` defaults in pre-SoC Intel platforms isn't very consistent. There's some defaults in CPU-model Kconfig (model_2065x, model_206ax, haswell: the corresponding socket Kconfigs got dropped for the sake of simplicity), others in CPU-socket Kconfig (socket_BGA956, socket_FCBGA559, socket_p) and others in northbridge Kconfig (i945, x4x). > > If you'd like to clean some things up, you can start with moving `MAX_CPUS` defaults from the northbridges to the corresponding CPU sockets. i945 is paired with LGA775 and socket_441 (both sockets support up to 4 CPUs), and x4x is only paired with LGA775 in coreboot. > Not true. i945 only supports dual core CPUs. Quad cores fail to boot regardless of them actually fitting in the socket and this is a hardware limitation afaik. Putting that limitation in the northbridge code is appropriate. > Note: Boards that pair x4x with socket_p exist (e.g. Dell Precision M6400), but they're rare and none of them has coreboot support yet. While both socket_p and x4x provide the same default for `MAX_CPUS` (4), moving the values to sockets would remove the redundancy. -- To view, visit https://review.coreboot.org/c/coreboot/+/69447 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If2265ac716e90720e7ccc550239737d40c2f7a0a Gerrit-Change-Number: 69447 Gerrit-PatchSet: 4 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Stefan Ott <coreboot(a)desire.ch> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Stefan Ott <coreboot(a)desire.ch> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Comment-Date: Fri, 18 Nov 2022 17:32:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Comment-In-Reply-To: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: cpu/intel/socket_*: Clean up Kconfig files
by Angel Pons (Code Review) 18 Nov '22

18 Nov '22

Attention is currently required from: Stefan Ott, Nico Huber, Arthur Heymans, Kyösti Mälkki, Alexander Couzens, Elyes Haouas. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69447 ) Change subject: cpu/intel/socket_*: Clean up Kconfig files ...................................................................... Patch Set 4: (1 comment) File src/cpu/intel/socket_BGA956/Kconfig: https://review.coreboot.org/c/coreboot/+/69447/comment/166c44f1_47277841 PS4, Line 9: select MMX > It's not even used. You can remove CONFIG_MMX, and we can probably assume all x86 CPUs have MMX. Ignore, MMX is used by the top-level Makefile. If anything, making it enabled by default and disabling it from Quark (seems to be the only x86 that doesn't support MMX) would be tidier. -- To view, visit https://review.coreboot.org/c/coreboot/+/69447 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If2265ac716e90720e7ccc550239737d40c2f7a0a Gerrit-Change-Number: 69447 Gerrit-PatchSet: 4 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Stefan Ott <coreboot(a)desire.ch> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Stefan Ott <coreboot(a)desire.ch> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Comment-Date: Fri, 18 Nov 2022 17:31:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: Documentation/measured_boot.md: document new TPM options
by Sergii Dmytruk (Code Review) 18 Nov '22

18 Nov '22

Attention is currently required from: Michał Żygowski, Daniel P. Smith, Maciej Pijanowski, Krystian Hebel. Sergii Dmytruk has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68752 ) Change subject: Documentation/measured_boot.md: document new TPM options ...................................................................... Patch Set 10: (2 comments) File Documentation/security/vboot/measured_boot.md: https://review.coreboot.org/c/coreboot/+/68752/comment/fa7142bb_5817854a PS9, Line 84: [TPM12]: https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClientImplementa… : [TPM20]: https://trustedcomputinggroup.org/wp-content/uploads/TCG_PCClient_PFP_r1p05… : > Please move links to the bottom of the file Done https://review.coreboot.org/c/coreboot/+/68752/comment/e7bd845d_d87f2529 PS9, Line 254: > So what will be the PCR0 usage here? Specified explicitly that it's unused in this case. -- To view, visit https://review.coreboot.org/c/coreboot/+/68752 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6dae8e95c59b440c75e13473eefc4c2cf4fd369b Gerrit-Change-Number: 68752 Gerrit-PatchSet: 10 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Daniel P. Smith Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Daniel P. Smith Gerrit-Attention: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Comment-Date: Fri, 18 Nov 2022 17:27:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: Documentation/measured_boot.md: fix SRTM/DRTM explanations
by Sergii Dmytruk (Code Review) 18 Nov '22

18 Nov '22

Attention is currently required from: Michał Żygowski, Daniel P. Smith, Maciej Pijanowski, Krystian Hebel. Sergii Dmytruk has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68751 ) Change subject: Documentation/measured_boot.md: fix SRTM/DRTM explanations ...................................................................... Patch Set 8: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/68751/comment/0c20ddf7_a41f9ec1 PS7, Line 11: Signed-off-by: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> > > Unrecognized email address: 'Daniel P. Smith' […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/68751 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If224dc0cf3c0515dbd18daca544c22275e96b459 Gerrit-Change-Number: 68751 Gerrit-PatchSet: 8 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Daniel P. Smith Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Daniel P. Smith Gerrit-Attention: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Comment-Date: Fri, 18 Nov 2022 17:27:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-MessageType: comment

1 0

[L] Change in coreboot[master]: util/cbmem: add parsing of TPM logs per standard
by Sergii Dmytruk (Code Review) 18 Nov '22

18 Nov '22

Attention is currently required from: Michał Żygowski, Maciej Pijanowski, Christian Walter, Krystian Hebel. Sergii Dmytruk has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68749 ) Change subject: util/cbmem: add parsing of TPM logs per standard ...................................................................... Patch Set 9: (1 comment) File src/lib/coreboot_table.c: https://review.coreboot.org/c/coreboot/+/68749/comment/709466e7_800bac5b PS8, Line 298: const struct tpm_log_ref *range = cbmem_find(CBMEM_ID_TPM_LOG_REF); > I don't get it why do we need to define yet another CBMEM ID. […] Thanks for noticing this, it was necessary at some point, but not anymore. -- To view, visit https://review.coreboot.org/c/coreboot/+/68749 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib76dc7dec56dd1789a219539a1ac05a958f47a5c Gerrit-Change-Number: 68749 Gerrit-PatchSet: 9 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Comment-Date: Fri, 18 Nov 2022 17:27:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-MessageType: comment

1 0

[L] Change in coreboot[master]: security/tpm: add TPM log format as per 2.0 spec
by Sergii Dmytruk (Code Review) 18 Nov '22

18 Nov '22

1 0

[L] Change in coreboot[master]: security/tpm: add TPM log format as per 1.2 spec
by Sergii Dmytruk (Code Review) 18 Nov '22

18 Nov '22

Attention is currently required from: Michał Żygowski, Maciej Pijanowski, Christian Walter, Julius Werner, Krystian Hebel. Sergii Dmytruk has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/68747 ) Change subject: security/tpm: add TPM log format as per 1.2 spec ...................................................................... Patch Set 8: (6 comments) File src/commonlib/bsd/include/commonlib/bsd/cbmem_id.h: https://review.coreboot.org/c/coreboot/+/68747/comment/f685f234_d2c41e7c PS7, Line 143: { CBMEM_ID_TPM_LOG, "TPM STD LOG" }, \ > Why do we create a new cbmem ID? We already have IDs TPM logs: […] Because existing code makes assumptions about format of data based on cbmem IDs. Reusing the ID will break the code, it's not always possible to determine the log format based on data and existing code isn't even trying to. E.g. `CBMEM_ID_TPM2_TCG_LOG` is used by `src/acpi/acpi.c` and is not compatible with a regular TPM log. File src/security/tpm/Kconfig: https://review.coreboot.org/c/coreboot/+/68747/comment/be947940_5b53f672 PS5, Line 108: bool "TPM 1.2 format" > Exactly, TPM2 should have different format. […] Done File src/security/tpm/Kconfig: https://review.coreboot.org/c/coreboot/+/68747/comment/9292b49c_2c554eaf PS7, Line 100: default TPM_LOG_TPM1 if TPM1 Well, https://ticket.coreboot.org/issues/422 says: > New entries should default to one of TCG formats, depending on selected TPM family. Choice of other format shouldn't be restricted. I thought it means log version should be chosen by the user. File src/security/tpm/tpm1_log_serialized.h: https://review.coreboot.org/c/coreboot/+/68747/comment/dac58786_b324878e PS7, Line 1: /* SPDX-License-Identifier: GPL-2.0-only */ > This file could be commonlib with BSD license isn't it? […] `commonlib/tpm_log_serialized.h` is licensed as `GPL-2.0-only`. I did try having a single header, it's a bit messy, but the much bigger problem is that it can't work for TPM2 log because it uses `CONFIG()` macro to which `cbmem` has no access. https://review.coreboot.org/c/coreboot/+/68747/comment/ba692641_9be9d19f PS7, Line 13: #define TPM_1_LOG_VI_MAGIC 0x31544243 /* "CBT1" in LE */ > For ACPI table we use "COREv4" as OEM ID and "CORE" as ASL compiler. […] And use the same ID for TPM1 and TPM2 logs? File src/security/tpm/tspi/log-tpm1.c: https://review.coreboot.org/c/coreboot/+/68747/comment/49f6b205_70e6aada PS7, Line 16: void *tpm_log_cbmem_init(void) > If it is source file for TPM1. […] These functions are called from shared code which doesn't know what log format is used. I can change names only by adding one more function to pick the implementation based on `CONFIG()` options, which can also be done later. -- To view, visit https://review.coreboot.org/c/coreboot/+/68747 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I89720615a75573d44dd0a39ad3d7faa78f125843 Gerrit-Change-Number: 68747 Gerrit-PatchSet: 8 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Maciej Pijanowski <maciej.pijanowski(a)3mdeb.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Comment-Date: Fri, 18 Nov 2022 17:27:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Michał Żygowski <michal.zygowski(a)3mdeb.com> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Comment-In-Reply-To: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit November 2022