coreboot-gerrit November 2023

coreboot-gerrit@coreboot.org

1 participants
3037 discussions

[S] Change in coreboot[main]: mb/google/brya/var/marasov: Enable Wi-Fi sar table for Intel module
by Subrata Banik (Code Review) 03 Nov '23

03 Nov '23

Attention is currently required from: Daniel Peng, Eric Lai, Karthik Ramasubramanian, Kyle Lin, Nick Vaccaro, Paul Menzel. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77789?usp=email ) Change subject: mb/google/brya/var/marasov: Enable Wi-Fi sar table for Intel module ...................................................................... Patch Set 7: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/77789?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I5b5c6bea6c2c916fb682044218ec7b3a5d2659f6 Gerrit-Change-Number: 77789 Gerrit-PatchSet: 7 Gerrit-Owner: Daniel Peng <daniel_peng(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Daniel Peng <daniel_peng(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Kyle Lin <kylelinck(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Eric Lai <ericllai(a)google.com> Gerrit-Attention: Kyle Lin <kylelinck(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Daniel Peng <daniel_peng(a)pegatron.corp-partner.google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Fri, 03 Nov 2023 05:44:34 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: mb/google/brya/var/marasov: Enable wifi sar table for Intel module
by Subrata Banik (Code Review) 03 Nov '23

03 Nov '23

Attention is currently required from: Daniel Peng, Nick Vaccaro, Paul Menzel. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78310?usp=email ) Change subject: mb/google/brya/var/marasov: Enable wifi sar table for Intel module ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: > my bad, i just overlooked it please submit this cl into the main branch -- To view, visit https://review.coreboot.org/c/coreboot/+/78310?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5b5c6bea6c2c916fb682044218ec7b3a5d2659f6 Gerrit-Change-Number: 78310 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Peng <daniel_peng(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Daniel Peng <daniel_peng(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Kyle Lin <kylelinck(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Daniel Peng <daniel_peng(a)pegatron.corp-partner.google.com> Gerrit-Comment-Date: Fri, 03 Nov 2023 05:44:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[main]: soc/intel/cmn/gfx: Join MBUS while FSP-S performs GFX init
by Subrata Banik (Code Review) 03 Nov '23

03 Nov '23

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/78802?usp=email ) Change subject: soc/intel/cmn/gfx: Join MBUS while FSP-S performs GFX init ...................................................................... soc/intel/cmn/gfx: Join MBUS while FSP-S performs GFX init This patch calls into the function to join the MBUS if the GFX PEIM module inside the FSP binary is taking care of graphics initialization based on the RUN_FSP_GOP config option. The FW skips joining the MBUS in case of a non-FSP solution and/or SOC_INTEL_GFX_MBUS_JOIN config is not enabled. BUG=b:284799726 TEST=MBUS joining is only applicable for google/rex while using GFX PEIM. Change-Id: I50d719a286722f5aafbad48ab4ca60500c836dd6 Signed-off-by: Subrata Banik <subratabanik(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/78802 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Nick Vaccaro <nvaccaro(a)google.com> --- M src/soc/intel/common/block/graphics/graphics.c 1 file changed, 32 insertions(+), 21 deletions(-) Approvals: Nick Vaccaro: Looks good to me, approved build bot (Jenkins): Verified diff --git a/src/soc/intel/common/block/graphics/graphics.c b/src/soc/intel/common/block/graphics/graphics.c index e744ca7..9b780da 100644 --- a/src/soc/intel/common/block/graphics/graphics.c +++ b/src/soc/intel/common/block/graphics/graphics.c @@ -280,31 +280,42 @@ } } +static void graphics_join_mbus(void) +{ + enum display_type type = get_external_display_status(); + uint32_t hashing_mode = 0; /* 2x2 */ + if (type == INTERNAL_DISPLAY_ONLY) { + hashing_mode = GFX_MBUS_HASHING_MODE; /* 1x4 */ + /* Only eDP pipes is joining the MBUS */ + graphics_gtt_rmw(GFX_MBUS_SEL(PIPE_A), PIPE_A, GFX_MBUS_JOIN | hashing_mode); + } else if (type == DUAL_DISPLAY) { + /* All pipes are joining the MBUS */ + graphics_gtt_rmw(GFX_MBUS_SEL(PIPE_A), PIPE_A, GFX_MBUS_JOIN | hashing_mode); + graphics_gtt_rmw(GFX_MBUS_SEL(PIPE_B), PIPE_B, GFX_MBUS_JOIN | hashing_mode); + graphics_gtt_rmw(GFX_MBUS_SEL(PIPE_C), PIPE_C, GFX_MBUS_JOIN | hashing_mode); +#if CONFIG(INTEL_GMA_VERSION_2) + graphics_gtt_rmw(GFX_MBUS_SEL(PIPE_D), PIPE_D, GFX_MBUS_JOIN | hashing_mode); +#endif + } else { + /* No pipe joins the MBUS */ + graphics_gtt_rmw(GFX_MBUS_CTL, GFX_MBUS_JOIN_PIPE_SEL, + GFX_MBUS_JOIN | hashing_mode); + } +} + static void graphics_dev_final(struct device *dev) { pci_dev_request_bus_master(dev); - if (CONFIG(SOC_INTEL_GFX_MBUS_JOIN)) { - enum display_type type = get_external_display_status(); - uint32_t hashing_mode = 0; /* 2x2 */ - if (type == INTERNAL_DISPLAY_ONLY) { - hashing_mode = GFX_MBUS_HASHING_MODE; /* 1x4 */ - /* Only eDP pipes is joining the MBUS */ - graphics_gtt_rmw(GFX_MBUS_SEL(PIPE_A), PIPE_A, GFX_MBUS_JOIN | hashing_mode); - } else if (type == DUAL_DISPLAY) { - /* All pipes are joining the MBUS */ - graphics_gtt_rmw(GFX_MBUS_SEL(PIPE_A), PIPE_A, GFX_MBUS_JOIN | hashing_mode); - graphics_gtt_rmw(GFX_MBUS_SEL(PIPE_B), PIPE_B, GFX_MBUS_JOIN | hashing_mode); - graphics_gtt_rmw(GFX_MBUS_SEL(PIPE_C), PIPE_C, GFX_MBUS_JOIN | hashing_mode); -#if CONFIG(INTEL_GMA_VERSION_2) - graphics_gtt_rmw(GFX_MBUS_SEL(PIPE_D), PIPE_D, GFX_MBUS_JOIN | hashing_mode); -#endif - } else { - /* No pipe joins the MBUS */ - graphics_gtt_rmw(GFX_MBUS_CTL, GFX_MBUS_JOIN_PIPE_SEL, - GFX_MBUS_JOIN | hashing_mode); - } - } + /* + * Call function to join the MBUS if GFX PEIM module inside FSP + * binary is taking care of graphics initialization based on + * RUN_FSP_GOP config option. + * + * Skip FW joining the MBUS in case of non-FSP solution. + */ + if (CONFIG(RUN_FSP_GOP) && CONFIG(SOC_INTEL_GFX_MBUS_JOIN) && display_init_required()) + graphics_join_mbus(); } const struct device_operations graphics_ops = { -- To view, visit https://review.coreboot.org/c/coreboot/+/78802?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I50d719a286722f5aafbad48ab4ca60500c836dd6 Gerrit-Change-Number: 78802 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in coreboot[main]: doc/soc/intel/xeon_sp: Add Xeon-SP coreboot community preview guide
by David Hendricks (Code Review) 03 Nov '23

03 Nov '23

Attention is currently required from: Anjaneya "Reddy" Chagam, Jonathan Zhang, Shuo Liu. David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78787?usp=email ) Change subject: doc/soc/intel/xeon_sp: Add Xeon-SP coreboot community preview guide ...................................................................... Patch Set 7: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/78787?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ie2ffc1722a4a26d6039b6642efa95bf072d896ad Gerrit-Change-Number: 78787 Gerrit-PatchSet: 7 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-CC: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Attention: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Comment-Date: Fri, 03 Nov 2023 04:17:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: pc80/rtc: Clear CMOS on errors on S3 resume too
by Bill XIE (Code Review) 03 Nov '23

03 Nov '23

Attention is currently required from: Kyösti Mälkki, Nico Huber. Bill XIE has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78881?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: pc80/rtc: Clear CMOS on errors on S3 resume too ...................................................................... Patch Set 1: (1 comment) File src/drivers/pc80/rtc/option.c: https://review.coreboot.org/c/coreboot/+/78881/comment/1dbce815_43bd4613 : PS1, Line 204: (CONFIG(STATIC_OPTION_TABLE) && !acpi_is_wakeup_s3()) During CB:78288 I Have considered this, and decided to defer CMOS reset on error to the next boot, for CMOS reset during s3 on GM45 platforms will erase their DRAM training data, making s3 resume fail. Do we really need to reset CMOS on error during s3 resume? @Jonathon Hall The ultimate solution fur this may be your proposal in the comment of CB:78288 to exclude the range of memory training data from CMOS checksum. -- To view, visit https://review.coreboot.org/c/coreboot/+/78881?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I1533875466f6eb7b409bb19ef98f6dcaf2d115c4 Gerrit-Change-Number: 78881 Gerrit-PatchSet: 1 Gerrit-Owner: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Bill XIE <persmule(a)hardenedlinux.org> Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Comment-Date: Fri, 03 Nov 2023 03:52:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: soc/mediatek/mt8188: Support loading BL32 image from rootfs
by Yidi Lin (Code Review) 03 Nov '23

03 Nov '23

Attention is currently required from: Hung-Te Lin, Kiwi Liu, Yu-Ping Wu. Yidi Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78801?usp=email ) Change subject: soc/mediatek/mt8188: Support loading BL32 image from rootfs ...................................................................... Patch Set 13: (2 comments) Patchset: PS13: Please resolve the comments File src/soc/mediatek/mt8188/Makefile.inc: https://review.coreboot.org/c/coreboot/+/78801/comment/2001eb94_3ab68064 : PS13, Line 61: PLAT=mt8188 SPD=opteed OPTEE_ALLOW_SMC_LOAD=1 \ : PLAT_XLAT_TABLES_DYNAMIC=1 CROS_WIDEVINE_SMC=1 Is there any reason that the parameters should be in the same line ? -- To view, visit https://review.coreboot.org/c/coreboot/+/78801?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ic38c8beb59c090ae56c5be6821dd8625435609e9 Gerrit-Change-Number: 78801 Gerrit-PatchSet: 13 Gerrit-Owner: Kiwi Liu <kiwi.liu(a)mediatek.corp-partner.google.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Attention: Kiwi Liu <kiwi.liu(a)mediatek.corp-partner.google.com> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Fri, 03 Nov 2023 03:42:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: mb/google/corsola: Enable FW_CONFIG and FW_CONFIG_SOURCE_CHROMEEC_CBI
by Yidi Lin (Code Review) 03 Nov '23

03 Nov '23

Attention is currently required from: Yu-Ping Wu. Yidi Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78905?usp=email ) Change subject: mb/google/corsola: Enable FW_CONFIG and FW_CONFIG_SOURCE_CHROMEEC_CBI ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/78905?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I6c12041d3666907c884f5a50a12c1433c2085961 Gerrit-Change-Number: 78905 Gerrit-PatchSet: 1 Gerrit-Owner: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Comment-Date: Fri, 03 Nov 2023 03:38:42 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: mb/google/brya/var/marasov: Enable wifi sar table for Intel module
by Daniel Peng (Code Review) 03 Nov '23

03 Nov '23

Attention is currently required from: Nick Vaccaro, Paul Menzel. Daniel Peng has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78310?usp=email ) Change subject: mb/google/brya/var/marasov: Enable wifi sar table for Intel module ...................................................................... Patch Set 3: (5 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/78310/comment/9a8b0c2a_f3e80da7 : PS3, Line 2: Daniel_Peng > Please update your git configuration to remove the underscore in the name: […] Done https://review.coreboot.org/c/coreboot/+/78310/comment/b039f7ee_eb79aa6b : PS3, Line 8: > Please motivate the change, by stating the problem. Maybe: […] Done Patchset: PS3: Move the branch to main. Please see CL:https://review.coreboot.org/c/coreboot/+/77789. File src/mainboard/google/brya/variants/marasov/variant.c: https://review.coreboot.org/c/coreboot/+/78310/comment/8f12a8f7_3606617c : PS3, Line 13: printk(BIOS_INFO, "Use wifi_sar_1.hex.\n"); > Info messages should be understandable to “normal” users. […] Done https://review.coreboot.org/c/coreboot/+/78310/comment/bd8439c3_f73d9fb4 : PS3, Line 17: printk(BIOS_INFO, "Intel Wi-Fi SAR not used, return NULL!\n"); > It’d be great, if you rephrased this too. Done. Thanks for the suggestion. -- To view, visit https://review.coreboot.org/c/coreboot/+/78310?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5b5c6bea6c2c916fb682044218ec7b3a5d2659f6 Gerrit-Change-Number: 78310 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Peng <daniel_peng(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Daniel Peng <daniel_peng(a)pegatron.corp-partner.google.com> Gerrit-Reviewer: Eric Lai <ericllai(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Kyle Lin <kylelinck(a)google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Comment-Date: Fri, 03 Nov 2023 02:56:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: libpayload/libc/time: Fix possible overflow in multiplication
by Yu-Ping Wu (Code Review) 03 Nov '23

03 Nov '23

Attention is currently required from: Julius Werner, Yidi Lin. Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78888?usp=email ) Change subject: libpayload/libc/time: Fix possible overflow in multiplication ...................................................................... Patch Set 4: Code-Review+2 (1 comment) File payloads/libpayload/libc/time.c: https://review.coreboot.org/c/coreboot/+/78888/comment/76a92f5d_2a9b0f3e : PS4, Line 181: 1000000 > nit: might look better to initialize `mult` earlier and use it here. […] There's also `MHz`. -- To view, visit https://review.coreboot.org/c/coreboot/+/78888?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Ia55532490651fcf47128b83a8554751f050bcc89 Gerrit-Change-Number: 78888 Gerrit-PatchSet: 4 Gerrit-Owner: Yidi Lin <yidilin(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Yidi Lin <yidilin(a)google.com> Gerrit-Comment-Date: Fri, 03 Nov 2023 02:44:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: security/vboot: Avoid using invalid vb2_context pointer
by Yu-Ping Wu (Code Review) 03 Nov '23

03 Nov '23

Attention is currently required from: Julius Werner. Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/78808?usp=email ) Change subject: security/vboot: Avoid using invalid vb2_context pointer ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: The main point of this patch is to prevent the device continuing to boot when `vb2api_reinit()` fails (for example when the vboot shared data version isn't the same in RO and RW). In that case, `vboot_ctx` won't be properly initialized. Our vboot code heavily relies on `vboot_ctx`. If the boot flow continues with an invalid `vboot_ctx`, weird security issues may happen (such as entering manual recovery in RW due to incorrect `vboot_ctx->boot_mode`?), which I think we'll want to avoid. Therefore what I want here is just an always-fatal version of assertion (similar to `VERIFY` in Visual C++). > In general, assert() is the check you're supposed to use when you "know" that something must be true (because of the way the rest of the code is written) I totally agree with you about that. I think the debate here is about the definition of "the rest of code". I'd probably interpret it as "the rest of code within the same coreboot stage", instead of "the rest of code within the firmware image". On a quick look, most of the other `assert` calls in coreboot seem to be simple checks for input arguments. So I think the cases here are quite different. -- To view, visit https://review.coreboot.org/c/coreboot/+/78808?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I4ff5ef1202bba2384c71634ec5ba12db1b784607 Gerrit-Change-Number: 78808 Gerrit-PatchSet: 1 Gerrit-Owner: Yu-Ping Wu <yupingso(a)google.com> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Comment-Date: Fri, 03 Nov 2023 02:40:45 +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 2023