coreboot-gerrit April 2022

coreboot-gerrit@coreboot.org

1 participants
5879 discussions

Change in coreboot[master]: [WIP] mb/google,intel: Split chromeos.c files
by Kyösti Mälkki (Code Review) 08 Apr '22

08 Apr '22

Attention is currently required from: Jason Glenesk, Marshall Dawson, Tim Wawrzynczak, Nick Vaccaro, Julius Werner, Fred Reitberger, Felix Held. Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59011 ) Change subject: [WIP] mb/google,intel: Split chromeos.c files ...................................................................... Patch Set 8: (1 comment) File src/mainboard/google/eve/chromeos.c: https://review.coreboot.org/c/coreboot/+/59011/comment/7cad7fc6_2dcd622e PS6, Line 26: CROS_GPIO_WP_AH(GPIO_PCH_WP, CROS_GPIO_DEVICE_NAME), > Yes, that is true, I am not sure if/why these are still required anymore, because crossystem will fa […] Actually, I meant if the use of line above with CROS_GPIO_VIRTUAL was still valid, I can see recovery_mode_switch filled in vboot context, and entry in CRHW.GPIO appears to be unusable as it does not store the state. As for WP, I do not see VBSD_BOOT_FIRMWARE_WP_ENABLED being set anywhere, and only mrc_cache code calls get_write_protect_state(). -- To view, visit https://review.coreboot.org/c/coreboot/+/59011 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I71a02c5fa1b256316b86b673660bf22dfd284f7f Gerrit-Change-Number: 59011 Gerrit-PatchSet: 8 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.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: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 08 Apr 2022 01:10:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Wawrzynczak <twawrzynczak(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]: mb/google/brya/var/vell: camera LED flicker problem
by Shon Wang (Code Review) 08 Apr '22

08 Apr '22

Attention is currently required from: Shon Wang. Hello build bot (Jenkins), Tim Wawrzynczak, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/63441 to look at the new patch set (#2). Change subject: mb/google/brya/var/vell: camera LED flicker problem ...................................................................... mb/google/brya/var/vell: camera LED flicker problem BUG=b:219644184 TEST=Build and boot on vell, observe whether camera LED flicker Change-Id: I846ec4cb5c4527f5664699b31d0d561d390d938c --- M src/mainboard/google/brya/variants/vell/overridetree.cb 1 file changed, 10 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/41/63441/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/63441 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I846ec4cb5c4527f5664699b31d0d561d390d938c Gerrit-Change-Number: 63441 Gerrit-PatchSet: 2 Gerrit-Owner: Shon Wang <shon.wang(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jimmy Su <jimmy.su(a)intel.corp-partner.google.com> Gerrit-CC: Kevin Chiu <coreboot.test(a)gmail.com> Gerrit-CC: Robert Chen <robert.chen(a)quanta.corp-partner.google.com> Gerrit-Attention: Shon Wang <shon.wang(a)quanta.corp-partner.google.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: mb/google,samsung: Drop init_bootmode_straps()
by Kyösti Mälkki (Code Review) 07 Apr '22

07 Apr '22

Attention is currently required from: Julius Werner, Angel Pons, Patrick Rudolph. Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59008 ) Change subject: mb/google,samsung: Drop init_bootmode_straps() ...................................................................... Patch Set 4: (2 comments) File src/include/bootmode.h: https://review.coreboot.org/c/coreboot/+/59008/comment/6c9bc34c_66d76042 PS2, Line 19: void stash_bootmode(int flag_spi_wp, int flag_rec_mode); > IIRC our official instructions to users have always been "hold down the recovery button until you se […] Ack File src/mainboard/google/beltino/chromeos.c: https://review.coreboot.org/c/coreboot/+/59008/comment/413532be_62d6687d PS3, Line 16: !get_recovery_mode_switch(), "presence"}, > In depthcharge, for GPIO_REC_MODE we only ever cared about the value that was sampled by coreboot -- […] Well get_recovery_mode_switch() is sampled src/security/vboot/vboot_logic.c and the lb_gpio identifier string "presence" has no longer been placed there for ages. So I was hoping to have a reason to drop any remaining "presence" entries as superseded by vboot context. -- To view, visit https://review.coreboot.org/c/coreboot/+/59008 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idcaf30c622bf5dc0f1295f2639c656086d01ff7e Gerrit-Change-Number: 59008 Gerrit-PatchSet: 4 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> 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: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Thu, 07 Apr 2022 23:56:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No 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]: mb/google,samsung: Drop init_bootmode_straps()
by Julius Werner (Code Review) 07 Apr '22

07 Apr '22

Attention is currently required from: Angel Pons, Kyösti Mälkki, Patrick Rudolph. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59008 ) Change subject: mb/google,samsung: Drop init_bootmode_straps() ...................................................................... Patch Set 4: Code-Review+2 (2 comments) File src/include/bootmode.h: https://review.coreboot.org/c/coreboot/+/59008/comment/57099f3f_eb411973 PS2, Line 19: void stash_bootmode(int flag_spi_wp, int flag_rec_mode); > At least for samsung/lumpy, rec mode switch is momentary push button. […] IIRC our official instructions to users have always been "hold down the recovery button until you see the recovery screen appear", so they should really be no reason to read it in romstage vs. ramstage. File src/mainboard/google/beltino/chromeos.c: https://review.coreboot.org/c/coreboot/+/59008/comment/b919984e_11b1014a PS3, Line 16: !get_recovery_mode_switch(), "presence"}, > Does the initial state sampled late in ramstage have any significance? We're adding an actual GPIO p […] In depthcharge, for GPIO_REC_MODE we only ever cared about the value that was sampled by coreboot -- we ignored the actual GPIO pin info. Depthcharge used to differentiate between GPIOs that should get resampled at runtime (like "lid", "power", "EC in RW") and those where it just uses the value from coreboot (like "recovery" or "write protect") because they're not expected to change in the middle of a single boot. Today we don't use most of these GPIOs anymore and I believe only the former category is left in practice on the boards we still support in depthcharge ToT. However, for boards that still booted u-boot (if we still care about having ToT coreboot stay compatible to that), I don't know what it did with each pin. -- To view, visit https://review.coreboot.org/c/coreboot/+/59008 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Idcaf30c622bf5dc0f1295f2639c656086d01ff7e Gerrit-Change-Number: 59008 Gerrit-PatchSet: 4 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Thu, 07 Apr 2022 23:25:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes 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]: cpu/x86/smm_module_load: Rewrite setup_stub
by Tim Wawrzynczak (Code Review) 07 Apr '22

07 Apr '22

Attention is currently required from: Arthur Heymans. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63478 ) Change subject: cpu/x86/smm_module_load: Rewrite setup_stub ...................................................................... Patch Set 4: (1 comment) File src/cpu/x86/smm/smm_module_loader.c: https://review.coreboot.org/c/coreboot/+/63478/comment/2e06ae0e_26e90e6e PS4, Line 323: smm_stub_place_staggered_entry_points > > the bug was filling in the stub_params->c_handler before calling this function correct? […] sorry I missed the word "not" in there, "not filling it before calling" 😊 -- To view, visit https://review.coreboot.org/c/coreboot/+/63478 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I42ef9d6a30f3039f25e2cde975086a1365ca4182 Gerrit-Change-Number: 63478 Gerrit-PatchSet: 4 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Thu, 07 Apr 2022 23:16:54 +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]: cpu/x86/smm: Refactor creating a stub/save state map
by Arthur Heymans (Code Review) 07 Apr '22

07 Apr '22

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63474 ) Change subject: cpu/x86/smm: Refactor creating a stub/save state map ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/63474/comment/4e91fd67_75818512 PS2, Line 9: This code was very hard to read so rewrite in a mostly pure functional : way. fix English! -- To view, visit https://review.coreboot.org/c/coreboot/+/63474 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7a455ba45a1c92533a8ecfd1aeecf34b4a63e409 Gerrit-Change-Number: 63474 Gerrit-PatchSet: 2 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Thu, 07 Apr 2022 23:14:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: cpu/x86/smm_module_load: Rewrite setup_stub
by Arthur Heymans (Code Review) 07 Apr '22

07 Apr '22

Attention is currently required from: Tim Wawrzynczak. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63478 ) Change subject: cpu/x86/smm_module_load: Rewrite setup_stub ...................................................................... Patch Set 4: (1 comment) File src/cpu/x86/smm/smm_module_loader.c: https://review.coreboot.org/c/coreboot/+/63478/comment/d58e8f3e_8133a179 PS4, Line 323: smm_stub_place_staggered_entry_points > the bug was filling in the stub_params->c_handler before calling this function correct? the bug is filling it after actually. -- To view, visit https://review.coreboot.org/c/coreboot/+/63478 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I42ef9d6a30f3039f25e2cde975086a1365ca4182 Gerrit-Change-Number: 63478 Gerrit-PatchSet: 4 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Comment-Date: Thu, 07 Apr 2022 23:09:02 +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]: tpm: Refactor TPM Kconfig dimensions
by Julius Werner (Code Review) 07 Apr '22

07 Apr '22

Attention is currently required from: Stefan Ott, Felix Singer, Patrick Rudolph, Jonathan Zhang, Nick Vaccaro, Arthur Heymans, Andrey Petrov, Piotr Król, Jes Klinke, Nico Huber, Sean Rhodes, Michał Żygowski, Johnny Lin, Christian Walter, Werner Zeh, Alexander Couzens, Yu-Ping Wu, Tim Chu, Frans Hendriks, Tristan Corrick, Jeremy Soller, Angel Pons, Michael Niewöhner, Tim Crawford, Maxim Polyakov, Tim Wawrzynczak. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63424 ) Change subject: tpm: Refactor TPM Kconfig dimensions ...................................................................... Patch Set 6: (12 comments) Patchset: PS6: Thanks Jes, this looks great! File src/drivers/i2c/tpm/Makefile.inc: https://review.coreboot.org/c/coreboot/+/63424/comment/9224ce55_1a6b8ebe PS6, Line 3: ifeq ($(CONFIG_I2C_TPM),y) Might as well wrap all lines in the file in this? In fact, I think it would be good to wrap all this in both ifeq ($(CONFIG_TPM)$(CONFIG_I2C_TPM),yy) and do the same for the SPI, CRB and LPC drivers. CONFIG_TPM being off should ensure that none of this code ever runs, but right now there's not really much static checking for that. If we avoid even compiling it we both save build time and make it easier to see if someone accidentally calls TPM functions in the wrong config. File src/drivers/tpm/Makefile.inc: PS6: This whole Makefile should also be wrapped in a CONFIG_TPM check. File src/mainboard/google/herobrine/Kconfig: https://review.coreboot.org/c/coreboot/+/63424/comment/9c62ff21_e5d514a0 PS6, Line 28: select TPM_GOOGLE_CR50 This needs an `if !BOARD_GOOGLE_SENOR` File src/mainboard/google/volteer/mainboard.c: https://review.coreboot.org/c/coreboot/+/63424/comment/44304c26_521388e2 PS6, Line 87: if (!CONFIG(SPI_TPM)) { This doesn't seem to make a difference for the currently available configurations, but I think it would be cleaner to also check && !CR50 here. File src/mainboard/siemens/mc_apl1/variants/mc_apl2/gpio.c: https://review.coreboot.org/c/coreboot/+/63424/comment/5c4ea59b_3f6e4f08 PS6, Line 594: /* CLK_25M_MEMORY_MAPPED_TPM_CPU */ You probably don't want to replace this. File src/security/tpm/tss/vendor/cr50/Kconfig: https://review.coreboot.org/c/coreboot/+/63424/comment/79a6510c_949d3879 PS6, Line 10: depends on TPM2 These shouldn't actually depend on TPM2 now, and neither should TPM_GOOGLE. The goal is that even if the mainboard has a Cr50/Ti50 TPM, the user can still disable it by selecting NO_TPM in menuconfig. https://review.coreboot.org/c/coreboot/+/63424/comment/231f26f6_6586c305 PS6, Line 12: default n nit: you don't need to specify `default n`, but if you do it's customary to put it above the `select`s. https://review.coreboot.org/c/coreboot/+/63424/comment/a1b02d52_6b76c65a PS6, Line 20: OOGLE_CR50 || TPM_GOOGLE_TI50 Just TPM_GOOGLE? File src/soc/intel/common/Makefile.inc: https://review.coreboot.org/c/coreboot/+/63424/comment/30ac40ed_aa3d157a PS6, Line 23: all-$(CONFIG_TPM_GOOGLE) += tpm_tis.c nit: a bit off-topic, but does Ti50 actually still require waiting for the sideband interrupt before doing the next transaction? I was hoping we would have gotten rid of that requirement now... (because that's what this code here is for) File src/vendorcode/google/chromeos/Kconfig: https://review.coreboot.org/c/coreboot/+/63424/comment/426cd070_74804dfb PS6, Line 25: config TPM_GOOGLE_IMMEDIATELY_COMMIT_FW_SECDATA Also a bit off-topic but it would probably be good to move this to tss/vendor/google. It shouldn't really depend on CHROMEOS. File src/vendorcode/google/chromeos/cse_board_reset.c: https://review.coreboot.org/c/coreboot/+/63424/comment/3d623ed1_8869ecc0 PS6, Line 19: if (CONFIG(SPI_TPM) && CONFIG(TPM_GOOGLE_CR50)) { Is there a reason to specifically check SPI_TPM here? Looks to me like the important factor here was just Cr50, but the only boards affected by this all used SPI. We should probably also check if the TPM is enabled here, which should always have been done but is an edge case people tend to forget. So check (CONFIG(TPM2) && CONFIG(TPM_GOOGLE_CR50))? -- To view, visit https://review.coreboot.org/c/coreboot/+/63424 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4656b2b90363b8dfd008dc281ad591862fe2cc9e Gerrit-Change-Number: 63424 Gerrit-PatchSet: 6 Gerrit-Owner: Jes Klinke <jbk(a)chromium.org> Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Erik van den Bogaert <ebogaert(a)eltan.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Jeremy Soller <jeremy(a)system76.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Maxim Polyakov <max.senia.poliak(a)gmail.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Stefan Ott <coreboot(a)desire.ch> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: Tim Crawford <tcrawford(a)system76.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Tristan Corrick <tristan(a)corrick.kiwi> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Stefan Ott <coreboot(a)desire.ch> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Attention: Jes Klinke <jbk(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Attention: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Attention: Tristan Corrick <tristan(a)corrick.kiwi> Gerrit-Attention: Jeremy Soller <jeremy(a)system76.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Tim Crawford <tcrawford(a)system76.com> Gerrit-Attention: Maxim Polyakov <max.senia.poliak(a)gmail.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Comment-Date: Thu, 07 Apr 2022 23:05:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: cpu/x86/smm_module_load: Rewrite setup_stub
by Tim Wawrzynczak (Code Review) 07 Apr '22

07 Apr '22

Attention is currently required from: Arthur Heymans. Tim Wawrzynczak has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63478 ) Change subject: cpu/x86/smm_module_load: Rewrite setup_stub ...................................................................... Patch Set 4: (1 comment) File src/cpu/x86/smm/smm_module_loader.c: https://review.coreboot.org/c/coreboot/+/63478/comment/9c7242d0_17bf0869 PS4, Line 323: smm_stub_place_staggered_entry_points the bug was filling in the stub_params->c_handler before calling this function correct? -- To view, visit https://review.coreboot.org/c/coreboot/+/63478 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I42ef9d6a30f3039f25e2cde975086a1365ca4182 Gerrit-Change-Number: 63478 Gerrit-PatchSet: 4 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Thu, 07 Apr 2022 22:47:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common: implement ioc driver
by Wonkyu Kim (Code Review) 07 Apr '22

07 Apr '22

Attention is currently required from: Jamie Ryu, Subrata Banik, Ethan Tsao, Ravishankar Sarawadi, Tim Wawrzynczak, Paul Menzel, Angel Pons, Nick Vaccaro. Wonkyu Kim has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63198 ) Change subject: soc/intel/common: implement ioc driver ...................................................................... Patch Set 8: (9 comments) File src/soc/intel/common/block/include/intelblocks/ioc.h: https://review.coreboot.org/c/coreboot/+/63198/comment/26380ed0_26c8ea52 PS5, Line 10: <soc/ioc_reg.h> > I'll remove this option for now. […] Ack https://review.coreboot.org/c/coreboot/+/63198/comment/5645d3b0_7c3be2a7 PS5, Line 12: /* > nit: Add space after comment beginning Ack https://review.coreboot.org/c/coreboot/+/63198/comment/e2fa77bb_ed06dc2e PS5, Line 15: : void ioc_reg_write32(uint32_t offset, uint32_t value); : uint32_t ioc_reg_read32(uint32_t offset); > outside gpmr. […] updated code to use gpmr api in other drivers and ioc api will be used in gpmr driver. File src/soc/intel/common/block/include/intelblocks/ioc_reg.h: https://review.coreboot.org/c/coreboot/+/63198/comment/58a28d0c_d11424a4 PS5, Line 7: A > Please use lowercase for hex Ack https://review.coreboot.org/c/coreboot/+/63198/comment/e3668c36_6d490cfb PS5, Line 8: C > Ditto Ack https://review.coreboot.org/c/coreboot/+/63198/comment/6eb03c72_e0eab27d PS5, Line 13: FFE > Ditto Ack https://review.coreboot.org/c/coreboot/+/63198/comment/4bdd5d89_be8f12d7 PS5, Line 14: A7C > Ditto Ack https://review.coreboot.org/c/coreboot/+/63198/comment/2b27313a_f4f38351 PS5, Line 15: A > Ditto Ack File src/soc/intel/common/block/ioc/Kconfig: https://review.coreboot.org/c/coreboot/+/63198/comment/9a581d32_2a149e85 PS5, Line 1: SOC_INTEL_COMMON_BLOCK_IOC > you are depending on MCH_BASE_ADDRESS being programmed for IOC read/write, hence, required driver is […] Do we have KConfig for this? -- To view, visit https://review.coreboot.org/c/coreboot/+/63198 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I768027c2ca78310c03845f70f17df19dc8cd0982 Gerrit-Change-Number: 63198 Gerrit-PatchSet: 8 Gerrit-Owner: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: Ethan Tsao <ethan.tsao(a)intel.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Ethan Tsao <ethan.tsao(a)intel.com> Gerrit-Attention: Ravishankar Sarawadi <ravishankar.sarawadi(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Comment-Date: Thu, 07 Apr 2022 22:45:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Wonkyu Kim <wonkyu.kim(a)intel.com> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2022