coreboot-gerrit November 2022

coreboot-gerrit@coreboot.org

1 participants
7286 discussions

[S] Change in coreboot[master]: ec/google/chromeec: Simplify error handling for GET_VERSION
by Martin L Roth (Code Review) 13 Nov '22

13 Nov '22

Martin L Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/69372 ) Change subject: ec/google/chromeec: Simplify error handling for GET_VERSION ...................................................................... ec/google/chromeec: Simplify error handling for GET_VERSION We don't need to check the lower level error code to determine if an EC call succeeded. Simply check the return value of the call. BUG=b:258126464 BRANCH=none TEST=none Change-Id: Iaf0795b0c1a2df0d3f44e6098ad02b82e33c5710 Signed-off-by: Caveh Jalali <caveh(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/69372 Reviewed-by: Boris Mittelberg <bmbm(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> --- M src/ec/google/chromeec/ec.c 1 file changed, 24 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Boris Mittelberg: Looks good to me, but someone else must approve Eric Lai: Looks good to me, approved diff --git a/src/ec/google/chromeec/ec.c b/src/ec/google/chromeec/ec.c index 906b5f8..9c78404 100644 --- a/src/ec/google/chromeec/ec.c +++ b/src/ec/google/chromeec/ec.c @@ -1356,10 +1356,11 @@ .cmd_size_out = sizeof(resp), .cmd_dev_index = 0, }; + int rv; - google_chromeec_command(&cmd); + rv = google_chromeec_command(&cmd); - if (cmd.cmd_code) { + if (rv != 0) { printk(BIOS_DEBUG, "Google Chrome EC: version command failed!\n"); } else { -- To view, visit https://review.coreboot.org/c/coreboot/+/69372 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iaf0795b0c1a2df0d3f44e6098ad02b82e33c5710 Gerrit-Change-Number: 69372 Gerrit-PatchSet: 4 Gerrit-Owner: Caveh Jalali <caveh(a)chromium.org> Gerrit-Reviewer: Boris Mittelberg <bmbm(a)google.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[master]: ec/google/chromeec: Simplify get_uptime_info error handling
by Martin L Roth (Code Review) 12 Nov '22

12 Nov '22

Martin L Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/69371 ) Change subject: ec/google/chromeec: Simplify get_uptime_info error handling ...................................................................... ec/google/chromeec: Simplify get_uptime_info error handling google_chromeec_get_uptime_info() doesn't need to return an error code from the lower level calls for the caller to interpret. It is more appropriate to return a success/failure boolean. BUG=b:258126464 BRANCH=none TEST=none Change-Id: I3e27b8b4eed9d23e6330eda863e43ca78bb174a3 Signed-off-by: Caveh Jalali <caveh(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/69371 Reviewed-by: Boris Mittelberg <bmbm(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> --- M src/ec/google/chromeec/ec.c 1 file changed, 26 insertions(+), 5 deletions(-) Approvals: build bot (Jenkins): Verified Boris Mittelberg: Looks good to me, but someone else must approve Eric Lai: Looks good to me, approved diff --git a/src/ec/google/chromeec/ec.c b/src/ec/google/chromeec/ec.c index 01f9ff4..906b5f8 100644 --- a/src/ec/google/chromeec/ec.c +++ b/src/ec/google/chromeec/ec.c @@ -948,7 +948,7 @@ return resp.sku_id; } -static uint16_t google_chromeec_get_uptime_info( +static bool google_chromeec_get_uptime_info( struct ec_response_uptime_info *resp) { struct chromeec_command cmd = { @@ -961,8 +961,7 @@ .cmd_dev_index = 0, }; - google_chromeec_command(&cmd); - return cmd.cmd_code; + return google_chromeec_command(&cmd) == 0; } bool google_chromeec_get_ap_watchdog_flag(void) @@ -970,7 +969,7 @@ int i; struct ec_response_uptime_info resp; - if (google_chromeec_get_uptime_info(&resp)) + if (!google_chromeec_get_uptime_info(&resp)) return false; if (resp.ec_reset_flags & EC_RESET_FLAG_AP_WATCHDOG) @@ -1297,7 +1296,7 @@ struct ec_response_uptime_info cmd_resp; int i, flag, flag_count; - if (google_chromeec_get_uptime_info(&cmd_resp)) { + if (!google_chromeec_get_uptime_info(&cmd_resp)) { /* * Deliberately say nothing for EC's that don't support this * command -- To view, visit https://review.coreboot.org/c/coreboot/+/69371 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3e27b8b4eed9d23e6330eda863e43ca78bb174a3 Gerrit-Change-Number: 69371 Gerrit-PatchSet: 4 Gerrit-Owner: Caveh Jalali <caveh(a)chromium.org> Gerrit-Reviewer: Boris Mittelberg <bmbm(a)google.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[master]: lib/ramtest.c: Update ram failure post code
by Martin L Roth (Code Review) 12 Nov '22

12 Nov '22

Martin L Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/69202 ) Change subject: lib/ramtest.c: Update ram failure post code ...................................................................... lib/ramtest.c: Update ram failure post code coreboot already has a ram failure post code defined, but the ram test functions weren't using it, and were using 0xea instead. This changes those failures to display 0xe3, the value defined in post_codes.h by POST_RAM_FAILURE. Signed-off-by: Martin Roth <gaumless(a)gmail.com> Change-Id: I21ef196e48ff37ffe320b575d6de66b43997e7eb Reviewed-on: https://review.coreboot.org/c/coreboot/+/69202 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Matt DeVillier <matt.devillier(a)gmail.com> --- M src/lib/ramtest.c 1 file changed, 21 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Matt DeVillier: Looks good to me, approved Angel Pons: Looks good to me, but someone else must approve diff --git a/src/lib/ramtest.c b/src/lib/ramtest.c index d6f958f..7ac141a 100644 --- a/src/lib/ramtest.c +++ b/src/lib/ramtest.c @@ -110,7 +110,7 @@ } } if (failures) { - post_code(0xea); + post_code(POST_RAM_FAILURE); printk(BIOS_DEBUG, "\nDRAM did _NOT_ verify!\n"); return 1; } @@ -200,7 +200,7 @@ write_phys(dst, backup); if (fail) { - post_code(0xea); + post_code(POST_RAM_FAILURE); die("RAM INIT FAILURE!\n"); } phys_memory_barrier(); -- To view, visit https://review.coreboot.org/c/coreboot/+/69202 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I21ef196e48ff37ffe320b575d6de66b43997e7eb Gerrit-Change-Number: 69202 Gerrit-PatchSet: 2 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[master]: device & commonlib: Update pci_scan_bus postcodes
by Martin L Roth (Code Review) 12 Nov '22

12 Nov '22

Martin L Roth has submitted this change. ( https://review.coreboot.org/c/coreboot/+/69201 ) Change subject: device & commonlib: Update pci_scan_bus postcodes ...................................................................... device & commonlib: Update pci_scan_bus postcodes The function pci_scan_bus had 3 post codes in it: 0x24 - beginning 0x25 - middle 0x55 - end I got rid of the middle postcode and used 0x25 for the code signifying the end of the function. I don't think all three are needed. 0x24 & 0x25 postcodes are currently also used in intel cache-as-ram code. Those postcodes should be adjusted to avoid conflicting. Signed-off-by: Martin Roth <gaumless(a)gmail.com> Change-Id: I19c9d5e256505b64234919a99f73a71efbbfdae3 Reviewed-on: https://review.coreboot.org/c/coreboot/+/69201 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Elyes Haouas <ehaouas(a)noos.fr> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> --- M src/commonlib/include/commonlib/console/post_codes.h M src/device/pci_device.c 2 files changed, 41 insertions(+), 4 deletions(-) Approvals: build bot (Jenkins): Verified Elyes Haouas: Looks good to me, approved Angel Pons: Looks good to me, approved diff --git a/src/commonlib/include/commonlib/console/post_codes.h b/src/commonlib/include/commonlib/console/post_codes.h index 8ab069b..d838815 100644 --- a/src/commonlib/include/commonlib/console/post_codes.h +++ b/src/commonlib/include/commonlib/console/post_codes.h @@ -66,6 +66,20 @@ #define POST_ENTRY_C_START 0x13 /** + * \brief Entry into pci_scan_bus + * + * Entered pci_scan_bus() + */ +#define POST_ENTER_PCI_SCAN_BUS 0x24 + +/** + * \brief Entry into pci_scan_bus + * + * Entered pci_scan_bus() + */ +#define POST_EXIT_PCI_SCAN_BUS 0x25 + +/** * \brief Pre-memory init preparation start * * Post code emitted in romstage before making callbacks to allow SoC/mainboard diff --git a/src/device/pci_device.c b/src/device/pci_device.c index 16c31ea..8651586 100644 --- a/src/device/pci_device.c +++ b/src/device/pci_device.c @@ -1423,7 +1423,7 @@ max_devfn=0xff; } - post_code(0x24); + post_code(POST_ENTER_PCI_SCAN_BUS); if (pci_bus_only_one_child(bus)) max_devfn = MIN(max_devfn, 0x07); @@ -1464,8 +1464,6 @@ } } - post_code(0x25); - /* * Warn if any leftover static devices are found. * There's probably a problem in devicetree.cb. @@ -1516,7 +1514,7 @@ * side of any bridges that may be on this bus plus any devices. * Return how far we've got finding sub-buses. */ - post_code(0x55); + post_code(POST_EXIT_PCI_SCAN_BUS); } typedef enum { -- To view, visit https://review.coreboot.org/c/coreboot/+/69201 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I19c9d5e256505b64234919a99f73a71efbbfdae3 Gerrit-Change-Number: 69201 Gerrit-PatchSet: 3 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in coreboot[master]: device & commonlib: Update pci_scan_bus postcodes
by Martin L Roth (Code Review) 12 Nov '22

12 Nov '22

Attention is currently required from: Felix Singer. Martin L Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69201 ) Change subject: device & commonlib: Update pci_scan_bus postcodes ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/69201/comment/5939b2f1_cec8f38c PS2, Line 18: code. Those postcodes should be adjusted to avoid conflicting. > Hmmm, not sure if we have a list of postcodes anywhere other than the commonlib header. Right. I'm about to adjust all of the postcodes into various different ranges and organize things. That should follow very shortly. -- To view, visit https://review.coreboot.org/c/coreboot/+/69201 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I19c9d5e256505b64234919a99f73a71efbbfdae3 Gerrit-Change-Number: 69201 Gerrit-PatchSet: 2 Gerrit-Owner: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Comment-Date: Sat, 12 Nov 2022 22:52:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Martin L Roth <gaumless(a)gmail.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: security/tpm: support compiling in multiple TPM drivers
by build bot (Jenkins) (Code Review) 12 Nov '22

12 Nov '22

Attention is currently required from: Michał Żygowski, Christian Walter, Julius Werner, Krystian Hebel. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69162 ) Change subject: security/tpm: support compiling in multiple TPM drivers ...................................................................... Patch Set 2: (2 comments) File src/security/tpm/tis.h: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-163463): https://review.coreboot.org/c/coreboot/+/69162/comment/a28d6027_428f8361 PS2, Line 110: #define __tis_driver __attribute__((used, section(".rodata.tis_driver"))) Prefer __used over __attribute__((used)) Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-163463): https://review.coreboot.org/c/coreboot/+/69162/comment/bb6a8589_da9d7711 PS2, Line 110: #define __tis_driver __attribute__((used, section(".rodata.tis_driver"))) Prefer __section(".rodata.tis_driver") over __attribute__((section(".rodata.tis_driver"))) -- To view, visit https://review.coreboot.org/c/coreboot/+/69162 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I44c5a1d825afe414c2f5c2c90f4cfe41ba9bef5f Gerrit-Change-Number: 69162 Gerrit-PatchSet: 2 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Michał Żygowski <michal.zygowski(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: Sat, 12 Nov 2022 22:52:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: security/tpm: replace CONFIG(TPMx) checks with runtime check
by Sergii Dmytruk (Code Review) 12 Nov '22

12 Nov '22

Attention is currently required from: Lance Zhao, Michał Żygowski, Christian Walter, Tim Wawrzynczak, Julius Werner, Krystian Hebel, Yu-Ping Wu. Sergii Dmytruk has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69161 ) Change subject: security/tpm: replace CONFIG(TPMx) checks with runtime check ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/69161/comment/e7afab8b_0e90f8dc PS1, Line 8: > Please explain why this is needed. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/69161 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id9cc25aad8d1d7bfad12b7a92059b1b3641bbfa9 Gerrit-Change-Number: 69161 Gerrit-PatchSet: 2 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Lance Zhao Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Sat, 12 Nov 2022 22:51:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com> Gerrit-MessageType: comment

1 0

[L] Change in coreboot[master]: security/tpm: resolve conflicts in TSS implementations
by Sergii Dmytruk (Code Review) 12 Nov '22

12 Nov '22

Attention is currently required from: Jason Glenesk, Raul Rangel, Michał Żygowski, Frans Hendriks, Matt DeVillier, Christian Walter, Julius Werner, Krystian Hebel, Fred Reitberger, Yu-Ping Wu, Felix Held. Sergii Dmytruk has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69160 ) Change subject: security/tpm: resolve conflicts in TSS implementations ...................................................................... Patch Set 5: (6 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/69160/comment/ac60b42d_2bd6dd5f PS4, Line 8: > I suppose the goal is compile both TPM1 and TPM2 code at the same time (CB:69162). […] Sure, I'll update the message. Same platform can be used with different TPMs and hard-coding TPM version at compile-time currently requires updating firmware in order to switch a TPM device. File src/security/tpm/tss.h: https://review.coreboot.org/c/coreboot/+/69160/comment/393b17c6_20539b53 PS4, Line 26: * tlcl_lib_init(). > I think you should use tlcl1_ and tlcl2_ as prefixes... mixing tlcl12_ and tlcl2_ is just confusing. Done https://review.coreboot.org/c/coreboot/+/69160/comment/30fdb622_5f6259cd PS4, Line 30: * TPM1.2-specific > I think this would be a good opportunity to factor this out into more files, since this one is getti […] Done File src/security/tpm/tss/tss.c: https://review.coreboot.org/c/coreboot/+/69160/comment/65054ba0_5ab06a4f PS4, Line 32: #if CONFIG(TPM1) > These don't need to be preprocessor #if, you can just write `if (CONFIG(TPM1) && tpm_family == 1)`. […] I remember getting undefined symbol errors with that. Maybe something was off with the build at that point. Updated. https://review.coreboot.org/c/coreboot/+/69160/comment/c1582b0e_da076f54 PS4, Line 35: return VB2_SUCCESS; > Why don't you just make tis_sendrecv an exported global in this file that can be directly accessed f […] Didn't want to use a global variable. Added one. https://review.coreboot.org/c/coreboot/+/69160/comment/99940a6e_bf667ce2 PS4, Line 62: die("%s: can't be used with TPM %d\n", __func__, tpm_family); > also, die() here and elsewhere in the file seems inappropriate. […] I thought that using a `__noreturn` function is better than returning some semi-random `TPM_E_*` error precisely because this shouldn't happen. Changed to `return TPM_E_INTERNAL_INCONSISTENCY;`. -- To view, visit https://review.coreboot.org/c/coreboot/+/69160 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia0ea5a917c46ada9fc3274f17240e12bca98db6a Gerrit-Change-Number: 69160 Gerrit-PatchSet: 5 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Erik van den Bogaert <ebogaert(a)eltan.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.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-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Sat, 12 Nov 2022 22:51:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Comment-In-Reply-To: Yu-Ping Wu <yupingso(a)google.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: security/tpm: make tis_probe() return tpm_family
by Sergii Dmytruk (Code Review) 12 Nov '22

12 Nov '22

Attention is currently required from: Michał Żygowski, Christian Walter, Julius Werner, Krystian Hebel. Sergii Dmytruk has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69159 ) Change subject: security/tpm: make tis_probe() return tpm_family ...................................................................... Patch Set 2: (1 comment) File src/drivers/i2c/tpm/tpm.c: https://review.coreboot.org/c/coreboot/+/69159/comment/104a3535_2a0ee7af PS1, Line 461: *tpm_family = 1; > TPM 2.0 for I2C does exist and it should be detected in the same way as for SPI/LPC. […] I don't think registers are the same for different interfaces. `TPM_INTF_CAPABILITY` seems to contain `tpmFamily` in case of I2C. And TPM1 had no I2C specification? Look at p. 24 in https://www.trustedcomputinggroup.org/wp-content/uploads/TCG-TPM-I2C-Interf…, those register addresses are very different from what the driver uses, including how locality is encoded in the address. It seems to use "Infineon I2C Protocol Stack Specification v0.20", which I haven't seen. -- To view, visit https://review.coreboot.org/c/coreboot/+/69159 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5006e0cdfef76ff79ce9e1cf280fcd5515ae01b0 Gerrit-Change-Number: 69159 Gerrit-PatchSet: 2 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(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: 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: Sat, 12 Nov 2022 22:51:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Comment-In-Reply-To: Krystian Hebel <krystian.hebel(a)3mdeb.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: drivers/pc80/tpm: probe for TPM family of a device
by Sergii Dmytruk (Code Review) 12 Nov '22

12 Nov '22

Attention is currently required from: Michał Żygowski, Christian Walter, Julius Werner, Krystian Hebel. Sergii Dmytruk has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/69023 ) Change subject: drivers/pc80/tpm: probe for TPM family of a device ...................................................................... Patch Set 3: (3 comments) File src/drivers/pc80/tpm/tis.c: https://review.coreboot.org/c/coreboot/+/69023/comment/0091d4bd_0e70cc2e PS2, Line 97: int tpm_family; > I think it might be nice to make an enum for this (especially since you start using it more widely i […] Done https://review.coreboot.org/c/coreboot/+/69023/comment/4834e535_06b53ce2 PS2, Line 392: const char *vendor_name = device_name; > Why not just initialize these to NULL now so you don't have to strcmp? Done https://review.coreboot.org/c/coreboot/+/69023/comment/1dc59bf8_a7b72b82 PS2, Line 435: return TPM_DRIVER_ERR; > This is a pretty big change in behavior (refusing to support anything not in the list). […] It is a big change and I might have got it wrong. Michał wrote in https://ticket.coreboot.org/issues/433 about multiple probe functions succeeding in which case there might be an ambiguity with regard to TPM version, so I made the check stricter here. However, since the next change returns TPM family, shouldn't that be enough to know which TSS implementation to use? -- To view, visit https://review.coreboot.org/c/coreboot/+/69023 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5464771836c66bcc441efb7189ded416b8f53827 Gerrit-Change-Number: 69023 Gerrit-PatchSet: 3 Gerrit-Owner: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Krystian Hebel <krystian.hebel(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: 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: Sat, 12 Nov 2022 22:51:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No 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 November 2022