coreboot-gerrit November 2021

coreboot-gerrit@coreboot.org

1 participants
6394 discussions

Change in coreboot[master]: soc/intel/cannonlake: Fix PEG1 _PRT generation
by Tim Wawrzynczak (Code Review) 19 Nov '21

19 Nov '21

Attention is currently required from: Arthur Heymans, Angel Pons, Patrick Rudolph. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58517 ) Change subject: soc/intel/cannonlake: Fix PEG1 _PRT generation ...................................................................... Patch Set 6: (1 comment) Patchset: PS6: > > very odd, I don't recall seeing anything fishy when I implemented this but google/hatch does not u […] 👍 -- To view, visit https://review.coreboot.org/c/coreboot/+/58517 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5be6e4514f8c6a47bb887d9f9b95181c9f426a51 Gerrit-Change-Number: 58517 Gerrit-PatchSet: 6 Gerrit-Owner: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Thu, 18 Nov 2021 23:08:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: src/security/vboot: Setup secure counter space in TPM NVRAM
by Julius Werner (Code Review) 19 Nov '21

19 Nov '21

Attention is currently required from: Andrey Pronin, Raul Rangel, Yu-Ping Wu, Karthik Ramasubramanian. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59476 ) Change subject: src/security/vboot: Setup secure counter space in TPM NVRAM ...................................................................... Patch Set 1: (1 comment) File src/security/vboot/secdata_tpm.c: https://review.coreboot.org/c/coreboot/+/59476/comment/c1d7f0ab_8357bc22 PS1, Line 368: rv = tlcl_read(index, &value, SECURE_COUNTER_SIZE); > Does TPM return TPM2_RC_NV_DEFINED if the NV index already exists? If so, I can treat it similar to […] I don't think you should do anything there. This code is not supposed to recover from power loss in the middle of factory setup. It's better to not have error recovery at all and always fail hard than to only do it partially and risk missing certain edge cases in an unsafe way. (If we wanted to generally make this recover correctly from aborted factory setup for all the spaces, that's a different thing, but that would require a bit more planning and careful consideration of all the different intermediate states that it could be stuck in. Since I don't think this has ever been an issue in practice, I don't think it's worth the effort.) -- To view, visit https://review.coreboot.org/c/coreboot/+/59476 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I915fbdada60e242d911b748ad5dc28028de9b657 Gerrit-Change-Number: 59476 Gerrit-PatchSet: 1 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Andrey Pronin <apronin(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Andrey Pronin <apronin(a)chromium.org> Gerrit-Attention: Andrey Pronin <apronin(a)google.com> Gerrit-Attention: Andrey Pronin <apronin(a)chromium.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Thu, 18 Nov 2021 23:05:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Andrey Pronin <apronin(a)chromium.org> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Comment-In-Reply-To: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: ChromeOS: Add legacy mainboard_ec_running_ro()
by Julius Werner (Code Review) 18 Nov '21

18 Nov '21

Attention is currently required from: Tim Wawrzynczak, Kyösti Mälkki. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58896 ) Change subject: ChromeOS: Add legacy mainboard_ec_running_ro() ...................................................................... Patch Set 7: (1 comment) File src/vendorcode/google/chromeos/gnvs.c: https://review.coreboot.org/c/coreboot/+/58896/comment/58bdfc47_d1457dfd PS7, Line 67: } > > ... Can't we just get rid of this and related callbacks? […] Okay, great... honestly then I'd say just rip this out and leave it to depthcharge. There's no reason to worry about this here. Just leave the fields in the struct zeroed out. -- To view, visit https://review.coreboot.org/c/coreboot/+/58896 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3cb95268424dc27f8c1e26b3d34eff1a7b8eab7f Gerrit-Change-Number: 58896 Gerrit-PatchSet: 7 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> 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-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Comment-Date: Thu, 18 Nov 2021 22:57:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: timestamp: Add new helper functions
by Julius Werner (Code Review) 18 Nov '21

18 Nov '21

Attention is currently required from: Bora Guvendik, Furquan Shaikh, Tim Wawrzynczak, Subrata Banik, Angel Pons, Aaron Durbin. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/51445 ) Change subject: timestamp: Add new helper functions ...................................................................... Patch Set 8: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/51445/comment/056c970b_43a542cb PS3, Line 7: Add helper fucntions > Hi Furquan, […] Hi Bora, Not sure if you've heard yet, but Furquan isn't working on coreboot anymore. I believe +Tim is taking over some of his former work, although I'm not sure if he has context on this effort. I can also try to help although I'm not familiar with your platform in particular. You are right, of course, that the formula should be `base_delta * ts->tick_freq_mhz / base_freq_mhz`, not `base_delta * base_freq_mhz / ts->tick_freq_mhz`. However, looking at this again with a few months distance, I'm actually not so sure anymore why we even need all this rewind() stuff. Fundamentally, your reason for doing that was just that some of the new timestamps you were trying to add are chronologically *before* the base_time, right? But why is that such a problem? Alternatively, we could just redefine (struct timestamp_entry).entry_stamp from uint64_t to int64_t, and declare that it should be allowed to be negative. Then I don't think you need any API changes here and just need to modify utils/cbmem to be aware of that case, and "rebase" all of those timestamps to the lowest one in the list when displaying them. I think that would require the minimum amount of extra logic in firmware code because you can basically just use timestamp_add() and pass a (potentially) negative number. (You may still need to do frequency conversions, that's where my other suggestion would come in of just rewrite x86's timestamp_get() to immediately convert to microseconds -- all other architecture ports already do it that way anyway, and then we'll never have any of this frequency confusion anymore.) What do you think? -- To view, visit https://review.coreboot.org/c/coreboot/+/51445 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6b7065ed26e231fc898ae44bcc15cba6fb42b308 Gerrit-Change-Number: 51445 Gerrit-PatchSet: 8 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Comment-Date: Thu, 18 Nov 2021 22:55:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Bora Guvendik <bora.guvendik(a)intel.com> Comment-In-Reply-To: Furquan Shaikh <furquan.m.shaikh(a)gmail.com> Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: src/security/vboot: Setup secure counter space in TPM NVRAM
by Karthik Ramasubramanian (Code Review) 18 Nov '21

18 Nov '21

Attention is currently required from: Andrey Pronin, Raul Rangel, Julius Werner, Yu-Ping Wu. Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59476 ) Change subject: src/security/vboot: Setup secure counter space in TPM NVRAM ...................................................................... Patch Set 1: (5 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/59476/comment/27f90f29_de569d3e PS1, Line 35: 72057594037927936 > Why is the value so high? I think because of endianness reasons - The value is 0x1000000000000000. File src/security/vboot/secdata_tpm.c: https://review.coreboot.org/c/coreboot/+/59476/comment/7b2e1808_8b8b2cc6 PS1, Line 349: SECURE_COUNTER_NAME > Is this any arbitrary name? This name is all local to coreboot. I dont think it is passed to TPM. Hence chosen a generic name. https://review.coreboot.org/c/coreboot/+/59476/comment/e6fd9301_750c1a0a PS1, Line 368: rv = tlcl_read(index, &value, SECURE_COUNTER_SIZE); > But that is true for all our TPM spaces. […] Does TPM return TPM2_RC_NV_DEFINED if the NV index already exists? If so, I can treat it similar to TPM_SUCCESS and continue with defining the rest of the counters. If any other response code is returned, abort defining the counters. https://review.coreboot.org/c/coreboot/+/59476/comment/bbccaaf4_de8387f5 PS1, Line 369: != > != TPM_E_BADINDEX also makes sure that the index is created and written. […] I will handle the error accordingly based on the discussion in line 368. https://review.coreboot.org/c/coreboot/+/59476/comment/e4aaaa6a_3d13844a PS1, Line 386: * Of all NVRAM spaces defined by this function the firmware space : * must be defined last, because its existence is considered an : * indication that TPM factory initialization was successfully : * completed. > Does this comment need updating, or do you need to move setup_secure_counter_spaces before setup_fi […] Julius/Andrey confirmed that firmware_space should be the last and hence asked to move counter space further up. -- To view, visit https://review.coreboot.org/c/coreboot/+/59476 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I915fbdada60e242d911b748ad5dc28028de9b657 Gerrit-Change-Number: 59476 Gerrit-PatchSet: 1 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Andrey Pronin <apronin(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Andrey Pronin <apronin(a)chromium.org> Gerrit-Attention: Andrey Pronin <apronin(a)google.com> Gerrit-Attention: Andrey Pronin <apronin(a)chromium.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Thu, 18 Nov 2021 22:48:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org> Comment-In-Reply-To: Andrey Pronin <apronin(a)chromium.org> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/cannonlake: Fix PEG1 _PRT generation
by Arthur Heymans (Code Review) 18 Nov '21

18 Nov '21

Attention is currently required from: Arthur Heymans, Angel Pons, Patrick Rudolph. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/58517 ) Change subject: soc/intel/cannonlake: Fix PEG1 _PRT generation ...................................................................... Patch Set 6: (1 comment) Patchset: PS6: > very odd, I don't recall seeing anything fishy when I implemented this but google/hatch does not use PEG, so I guess I never could test that part. So the DSDT code had _PRT for all 4 pins. I'm not so sure what is going on, but this sort forces the same behavior. -- To view, visit https://review.coreboot.org/c/coreboot/+/58517 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5be6e4514f8c6a47bb887d9f9b95181c9f426a51 Gerrit-Change-Number: 58517 Gerrit-PatchSet: 6 Gerrit-Owner: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Thu, 18 Nov 2021 22:40:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/tpm: Add always-shutdown-on-suspend DSD property
by Rob Barnes (Code Review) 18 Nov '21

18 Nov '21

Attention is currently required from: Andrey Pronin, Raul Rangel, Christian Walter, Karthik Ramasubramanian. Rob Barnes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59479 ) Change subject: drivers/tpm: Add always-shutdown-on-suspend DSD property ...................................................................... Patch Set 2: (2 comments) Patchset: PS2: Here's the matching kernel patch(WIP): https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/… File src/drivers/i2c/tpm/chip.c: https://review.coreboot.org/c/coreboot/+/59479/comment/db70dabd_6bf523b3 PS2, Line 53: suspend > Should we name this s0ix so it's not confused with S3? Technically it can apply to both S3 and S0i3. The flag just overrides the pm_suspend_via_firmware() check in tpm_pm_suspend. Turns out pm_suspend_via_firmware is used as a proxy for is_s3_suspend, but I don't want to repeat that assumption. -- To view, visit https://review.coreboot.org/c/coreboot/+/59479 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia48ead856fc0c6e637a2e07a5ecc58423f599c5b Gerrit-Change-Number: 59479 Gerrit-PatchSet: 2 Gerrit-Owner: Rob Barnes <robbarnes(a)google.com> Gerrit-Reviewer: Andrey Pronin <apronin(a)google.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Andrey Pronin <apronin(a)google.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Thu, 18 Nov 2021 22:34:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: drivers/tpm: Add always-shutdown-on-suspend DSD property
by Karthik Ramasubramanian (Code Review) 18 Nov '21

18 Nov '21

Attention is currently required from: Andrey Pronin, Raul Rangel, Christian Walter, Rob Barnes. Karthik Ramasubramanian has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59479 ) Change subject: drivers/tpm: Add always-shutdown-on-suspend DSD property ...................................................................... Patch Set 2: (1 comment) File src/drivers/i2c/tpm/chip.c: https://review.coreboot.org/c/coreboot/+/59479/comment/b218353a_5e22a6d9 PS2, Line 53: suspend > Should we name this s0ix so it's not confused with S3? +1. -- To view, visit https://review.coreboot.org/c/coreboot/+/59479 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia48ead856fc0c6e637a2e07a5ecc58423f599c5b Gerrit-Change-Number: 59479 Gerrit-PatchSet: 2 Gerrit-Owner: Rob Barnes <robbarnes(a)google.com> Gerrit-Reviewer: Andrey Pronin <apronin(a)google.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Andrey Pronin <apronin(a)google.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Rob Barnes <robbarnes(a)google.com> Gerrit-Comment-Date: Thu, 18 Nov 2021 22:31:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/alderlake: Save/restore BAR registers when extract cpu cras...
by Tim Wawrzynczak (Code Review) 18 Nov '21

18 Nov '21

Attention is currently required from: Kane Chen, Patrick Rudolph. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59355 ) Change subject: soc/intel/alderlake: Save/restore BAR registers when extract cpu crashlog ...................................................................... Patch Set 6: (1 comment) Patchset: PS6: > Hi Tim, […] It's just a PCI device like any other that has BARs that can be assigned by coreboot's resource allocator during PCI enumeration, just like i2c, spi, lpc, pcie controllers, etc. The resource allocator will read the BARs for their size requirements and carve out space for each. All the board has to do is turn the `crashlog` device on in the devicetree and it will get a valid BAR during enumeration. -- To view, visit https://review.coreboot.org/c/coreboot/+/59355 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib7e6772be51ec4f26ef31fed6cb2bddef8ffc6be Gerrit-Change-Number: 59355 Gerrit-PatchSet: 6 Gerrit-Owner: Kane Chen <kane.chen(a)intel.corp-partner.google.com> Gerrit-Reviewer: Francois Toguo Fotso <francois.toguo.fotso(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Kane Chen <kane.chen(a)intel.corp-partner.google.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Thu, 18 Nov 2021 22:29:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Kane Chen <kane.chen(a)intel.corp-partner.google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: src/security/vboot: Setup secure counter space in TPM NVRAM
by Julius Werner (Code Review) 18 Nov '21

18 Nov '21

Attention is currently required from: Andrey Pronin, Raul Rangel, Yu-Ping Wu, Karthik Ramasubramanian. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59476 ) Change subject: src/security/vboot: Setup secure counter space in TPM NVRAM ...................................................................... Patch Set 1: (1 comment) File src/security/vboot/secdata_tpm.c: https://review.coreboot.org/c/coreboot/+/59476/comment/1a149069_a09fc72e PS1, Line 368: rv = tlcl_read(index, &value, SECURE_COUNTER_SIZE); > actually, it may exist. […] But that is true for all our TPM spaces. We have never had recovery code from an aborted partial factory init for any of them (and that would be more complicated, because some of them need to be initialized right). It has never been a problem as far as I'm aware. It would be inconsistent to now do it only for this space. -- To view, visit https://review.coreboot.org/c/coreboot/+/59476 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I915fbdada60e242d911b748ad5dc28028de9b657 Gerrit-Change-Number: 59476 Gerrit-PatchSet: 1 Gerrit-Owner: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Andrey Pronin <apronin(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Andrey Pronin <apronin(a)chromium.org> Gerrit-Attention: Andrey Pronin <apronin(a)google.com> Gerrit-Attention: Andrey Pronin <apronin(a)chromium.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Thu, 18 Nov 2021 22:29:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Andrey Pronin <apronin(a)chromium.org> 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 2021