coreboot-gerrit December 2024

coreboot-gerrit@coreboot.org

1 participants
3018 discussions

[XL] Change in coreboot[main]: drivers/option: Add forms in cbtables
by Angel Pons (Code Review) 17 Dec '24

17 Dec '24

Attention is currently required from: Benjamin Doron, Lean Sheng Tan, Martin L Roth, Maximilian Brune, Nico Huber, Patrick Rudolph, Sean Rhodes, Werner Zeh. Angel Pons has posted comments on this change by Angel Pons. ( https://review.coreboot.org/c/coreboot/+/74121?usp=email ) Change subject: drivers/option: Add forms in cbtables ...................................................................... Patch Set 29: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/74121?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I304de7d26d79245a2e31a6d01f6c5643b31cb772 Gerrit-Change-Number: 74121 Gerrit-PatchSet: 29 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Christian Walter <christian.walter(a)9elements.com> Gerrit-CC: Daniel Maslowski <info(a)orangecms.org> Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-CC: Martin L Roth <gaumless(a)gmail.com> Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Comment-Date: Tue, 17 Dec 2024 17:58:20 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XL] Change in coreboot[main]: drivers/option: Add forms in cbtables
by Angel Pons (Code Review) 17 Dec '24

17 Dec '24

Attention is currently required from: Benjamin Doron, Lean Sheng Tan, Martin L Roth, Maximilian Brune, Nico Huber, Patrick Rudolph, Sean Rhodes, Werner Zeh. Angel Pons has posted comments on this change by Angel Pons. ( https://review.coreboot.org/c/coreboot/+/74121?usp=email ) Change subject: drivers/option: Add forms in cbtables ...................................................................... Patch Set 29: (1 comment) File src/commonlib/include/commonlib/cfr.h: https://review.coreboot.org/c/coreboot/+/74121/comment/223f540b_75abbe6b?us… : PS27, Line 143: uint32_t tag; /* CFR_TAG_OPTION_VARCHAR */ : uint32_t size; : uint64_t object_id; /* Uniqueue ID */ : uint64_t dependency_id; /* Grayout if value of lb_cfr_numeric_option with given ID is 0. : * Ignore if field is 0. : */ : uint32_t flags; /* enum cfr_option_flags */ > Is this a nit? I am not sure how to do it, and i think current implementation is fine? It's a "I will take care of it myself in a follow-up" -- To view, visit https://review.coreboot.org/c/coreboot/+/74121?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I304de7d26d79245a2e31a6d01f6c5643b31cb772 Gerrit-Change-Number: 74121 Gerrit-PatchSet: 29 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Christian Walter <christian.walter(a)9elements.com> Gerrit-CC: Daniel Maslowski <info(a)orangecms.org> Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-CC: Martin L Roth <gaumless(a)gmail.com> Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Comment-Date: Tue, 17 Dec 2024 17:58:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Comment-In-Reply-To: Lean Sheng Tan <sheng.tan(a)9elements.com>

1 0

[XL] Change in coreboot[main]: drivers/option: Add forms in cbtables
by Lean Sheng Tan (Code Review) 17 Dec '24

17 Dec '24

Attention is currently required from: Angel Pons, Benjamin Doron, Martin L Roth, Maximilian Brune, Nico Huber, Patrick Rudolph, Sean Rhodes, Werner Zeh. Lean Sheng Tan has posted comments on this change by Angel Pons. ( https://review.coreboot.org/c/coreboot/+/74121?usp=email ) Change subject: drivers/option: Add forms in cbtables ...................................................................... Patch Set 29: (2 comments) File src/commonlib/include/commonlib/cfr.h: https://review.coreboot.org/c/coreboot/+/74121/comment/1609d39f_8613bc08?us… : PS27, Line 106: * CFR_TAG_VARCHAR_UI_HELPTEXT or CFR_TAG_VARCHAR_DEF_VALUE > nit: remove extra space […] Done https://review.coreboot.org/c/coreboot/+/74121/comment/e52c8836_b68d6136?us… : PS27, Line 143: uint32_t tag; /* CFR_TAG_OPTION_VARCHAR */ : uint32_t size; : uint64_t object_id; /* Uniqueue ID */ : uint64_t dependency_id; /* Grayout if value of lb_cfr_numeric_option with given ID is 0. : * Ignore if field is 0. : */ : uint32_t flags; /* enum cfr_option_flags */ > We could have an `option header` structure so that we can factor out filling in the header fields. Is this a nit? I am not sure how to do it, and i think current implementation is fine? -- To view, visit https://review.coreboot.org/c/coreboot/+/74121?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I304de7d26d79245a2e31a6d01f6c5643b31cb772 Gerrit-Change-Number: 74121 Gerrit-PatchSet: 29 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Christian Walter <christian.walter(a)9elements.com> Gerrit-CC: Daniel Maslowski <info(a)orangecms.org> Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-CC: Martin L Roth <gaumless(a)gmail.com> Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Comment-Date: Tue, 17 Dec 2024 17:54:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>

1 0

[XL] Change in coreboot[main]: drivers/option: Add forms in cbtables
by Lean Sheng Tan (Code Review) 17 Dec '24

17 Dec '24

Attention is currently required from: Angel Pons, Benjamin Doron, Martin L Roth, Maximilian Brune, Nico Huber, Patrick Rudolph, Sean Rhodes, Werner Zeh. Lean Sheng Tan has uploaded a new patch set (#29) to the change originally created by Angel Pons. ( https://review.coreboot.org/c/coreboot/+/74121?usp=email ) Change subject: drivers/option: Add forms in cbtables ...................................................................... drivers/option: Add forms in cbtables Introduce a mechanism so that coreboot can provide a list of options to post-coreboot code. The options are grouped together into forms and have a meaning name and optional help text. This can be used to let payloads know which options should be displayed in a setup menu, for instance. Although this system was written to be used with edk2, it has been designed with flexibility in mind so that other payloads can also make use of this mechanism. The system currently lacks a way to describe where to find option values. This information is stored in a set of data structures specifically created for this purpose. This format is known as CFR, which means "coreboot forms representation" or "cursed forms representation". Although the "forms representation" is borrowed from UEFI, CFR can be used in non-UEFI scenarios as well. The data structures are implemented as an extension of cbtables records to support nesting. It should not break backwards compatibility because the CFR root record (LB_TAG_CFR_ROOT) size includes all of its children records. The concept of record nesting is borrowed from the records for CMOS options. It is not possible to reuse the CMOS records because they are too closely coupled with CMOS options; using these structures would needlessly restrict more capable backends to what can be done with CMOS options, which is undesired. Because CFR supports variable-length components, directly transforming options into CFR structures is not a trivial process. Furthermore, CFR structures need to be written in one go. Because of this, abstractions exist to generate CFR structures from a set of "setup menu" structures that are coreboot-specific and could be integrated with the devicetree at some point. Note that `struct sm_object` is a tagged union. This is used to have lists of options in an array, as building linked lists of options at runtime is extremely impractical because options would have to be added at the end of the linked list to maintain option order. To avoid mistakes defining `struct sm_object` values, helper macros exist for supported option types. The macros also provide some type checking as they initialise specific union members. It should be possible to extend CFR support for more sophisticated options like fan curve points. Feedback about this is highly appreciated. Change-Id: I304de7d26d79245a2e31a6d01f6c5643b31cb772 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- A Documentation/drivers/cfr.md A Documentation/drivers/cfr_internal.md M Documentation/drivers/index.md A src/commonlib/include/commonlib/cfr.h M src/commonlib/include/commonlib/coreboot_tables.h A src/drivers/option/Kconfig A src/drivers/option/Makefile.mk A src/drivers/option/cfr.c A src/drivers/option/cfr_frontend.h M src/include/boot/coreboot_tables.h M src/lib/coreboot_table.c M src/lib/program.ld 12 files changed, 1,100 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/74121/29 -- To view, visit https://review.coreboot.org/c/coreboot/+/74121?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I304de7d26d79245a2e31a6d01f6c5643b31cb772 Gerrit-Change-Number: 74121 Gerrit-PatchSet: 29 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Christian Walter <christian.walter(a)9elements.com> Gerrit-CC: Daniel Maslowski <info(a)orangecms.org> Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-CC: Martin L Roth <gaumless(a)gmail.com> Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>

1 0

[XL] Change in coreboot[main]: drivers/option: Add forms in cbtables
by Lean Sheng Tan (Code Review) 17 Dec '24

17 Dec '24

Attention is currently required from: Angel Pons, Benjamin Doron, Martin L Roth, Maximilian Brune, Nico Huber, Patrick Rudolph, Sean Rhodes, Werner Zeh. Lean Sheng Tan has posted comments on this change by Angel Pons. ( https://review.coreboot.org/c/coreboot/+/74121?usp=email ) Change subject: drivers/option: Add forms in cbtables ...................................................................... Patch Set 27: (6 comments) File Documentation/drivers/cfr.md: https://review.coreboot.org/c/coreboot/+/74121/comment/7247a78a_cc46756d?us… : PS27, Line 9: S > lowercase s Done https://review.coreboot.org/c/coreboot/+/74121/comment/acb84c10_adafecc8?us… : PS27, Line 15: means > meaning? Done https://review.coreboot.org/c/coreboot/+/74121/comment/6ff91763_2706ae67?us… : PS27, Line 24: it's > its (there are multiple ocurrences) Done File src/commonlib/include/commonlib/cfr.h: https://review.coreboot.org/c/coreboot/+/74121/comment/515aeb9a_764e9695?us… : PS27, Line 1: /* SPDX-License-Identifier: GPL-2.0-only */ > We should consider moving this to commonlib/bsd for interoperability. […] Talked to Patrick as he also worked on this, he has no objection, so do i, so I changed. https://review.coreboot.org/c/coreboot/+/74121/comment/a4b628cc_af17cd26?us… : PS27, Line 6: #include <stdint.h> > Redundant, types.h already includes it. Done File src/drivers/option/cfr.c: https://review.coreboot.org/c/coreboot/+/74121/comment/7601af2f_412f3422?us… : PS27, Line 42: data = (uint8_t *)(cfr_str + 1); > Right, this is a variable length structure and we don't have a struct member for the data. […] Done -- To view, visit https://review.coreboot.org/c/coreboot/+/74121?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I304de7d26d79245a2e31a6d01f6c5643b31cb772 Gerrit-Change-Number: 74121 Gerrit-PatchSet: 27 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Christian Walter <christian.walter(a)9elements.com> Gerrit-CC: Daniel Maslowski <info(a)orangecms.org> Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-CC: Martin L Roth <gaumless(a)gmail.com> Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Comment-Date: Tue, 17 Dec 2024 17:51:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com>

1 0

[XL] Change in coreboot[main]: drivers/option: Add forms in cbtables
by Lean Sheng Tan (Code Review) 17 Dec '24

17 Dec '24

Attention is currently required from: Angel Pons, Benjamin Doron, Lean Sheng Tan, Martin L Roth, Maximilian Brune, Nico Huber, Patrick Rudolph, Sean Rhodes, Werner Zeh. Lean Sheng Tan has uploaded a new patch set (#28) to the change originally created by Angel Pons. ( https://review.coreboot.org/c/coreboot/+/74121?usp=email ) The following approvals got outdated and were removed: Code-Review+1 by Angel Pons, Verified+1 by build bot (Jenkins) Change subject: drivers/option: Add forms in cbtables ...................................................................... drivers/option: Add forms in cbtables Introduce a mechanism so that coreboot can provide a list of options to post-coreboot code. The options are grouped together into forms and have a meaning name and optional help text. This can be used to let payloads know which options should be displayed in a setup menu, for instance. Although this system was written to be used with edk2, it has been designed with flexibility in mind so that other payloads can also make use of this mechanism. The system currently lacks a way to describe where to find option values. This information is stored in a set of data structures specifically created for this purpose. This format is known as CFR, which means "coreboot forms representation" or "cursed forms representation". Although the "forms representation" is borrowed from UEFI, CFR can be used in non-UEFI scenarios as well. The data structures are implemented as an extension of cbtables records to support nesting. It should not break backwards compatibility because the CFR root record (LB_TAG_CFR_ROOT) size includes all of its children records. The concept of record nesting is borrowed from the records for CMOS options. It is not possible to reuse the CMOS records because they are too closely coupled with CMOS options; using these structures would needlessly restrict more capable backends to what can be done with CMOS options, which is undesired. Because CFR supports variable-length components, directly transforming options into CFR structures is not a trivial process. Furthermore, CFR structures need to be written in one go. Because of this, abstractions exist to generate CFR structures from a set of "setup menu" structures that are coreboot-specific and could be integrated with the devicetree at some point. Note that `struct sm_object` is a tagged union. This is used to have lists of options in an array, as building linked lists of options at runtime is extremely impractical because options would have to be added at the end of the linked list to maintain option order. To avoid mistakes defining `struct sm_object` values, helper macros exist for supported option types. The macros also provide some type checking as they initialise specific union members. It should be possible to extend CFR support for more sophisticated options like fan curve points. Feedback about this is highly appreciated. Change-Id: I304de7d26d79245a2e31a6d01f6c5643b31cb772 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- A Documentation/drivers/cfr.md A Documentation/drivers/cfr_internal.md M Documentation/drivers/index.md A src/commonlib/include/commonlib/cfr.h M src/commonlib/include/commonlib/coreboot_tables.h A src/drivers/option/Kconfig A src/drivers/option/Makefile.mk A src/drivers/option/cfr.c A src/drivers/option/cfr_frontend.h M src/include/boot/coreboot_tables.h M src/lib/coreboot_table.c M src/lib/program.ld 12 files changed, 1,100 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/21/74121/28 -- To view, visit https://review.coreboot.org/c/coreboot/+/74121?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I304de7d26d79245a2e31a6d01f6c5643b31cb772 Gerrit-Change-Number: 74121 Gerrit-PatchSet: 28 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Christian Walter <christian.walter(a)9elements.com> Gerrit-CC: Daniel Maslowski <info(a)orangecms.org> Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-CC: Martin L Roth <gaumless(a)gmail.com> Gerrit-CC: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com>

1 0

[XS] Change in coreboot[main]: cpu/qemu: Enable IDT_IN_EVERY_STAGE
by Lean Sheng Tan (Code Review) 17 Dec '24

17 Dec '24

Lean Sheng Tan has submitted this change. ( https://review.coreboot.org/c/coreboot/+/85593?usp=email ) Change subject: cpu/qemu: Enable IDT_IN_EVERY_STAGE ...................................................................... cpu/qemu: Enable IDT_IN_EVERY_STAGE Enable IDT_IN_EVERY_STAGE in qemu to easily debug HW breakpoints. TEST: Can use breakpoints in pre RAM stages to debug invalid memory accesses on QEMU. Change-Id: If4a386d1adc788ff717899027881590b7c8b3369 Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/85593 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Lean Sheng Tan <sheng.tan(a)9elements.com> --- M src/cpu/qemu-x86/Kconfig 1 file changed, 1 insertion(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Lean Sheng Tan: Looks good to me, approved diff --git a/src/cpu/qemu-x86/Kconfig b/src/cpu/qemu-x86/Kconfig index e1469d6..f564d70 100644 --- a/src/cpu/qemu-x86/Kconfig +++ b/src/cpu/qemu-x86/Kconfig @@ -8,6 +8,7 @@ select TSC_MONOTONIC_TIMER select UNKNOWN_TSC_RATE select NEED_SMALL_2MB_PAGE_TABLES # QEMU doesn't support 1GB pages + select IDT_IN_EVERY_STAGE if CPU_QEMU_X86 -- To view, visit https://review.coreboot.org/c/coreboot/+/85593?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: If4a386d1adc788ff717899027881590b7c8b3369 Gerrit-Change-Number: 85593 Gerrit-PatchSet: 3 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

[XS] Change in coreboot[main]: cpu/qemu: Enable IDT_IN_EVERY_STAGE
by Lean Sheng Tan (Code Review) 17 Dec '24

17 Dec '24

Attention is currently required from: Patrick Rudolph. Lean Sheng Tan has posted comments on this change by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/85593?usp=email ) Change subject: cpu/qemu: Enable IDT_IN_EVERY_STAGE ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/85593?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: If4a386d1adc788ff717899027881590b7c8b3369 Gerrit-Change-Number: 85593 Gerrit-PatchSet: 2 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Comment-Date: Tue, 17 Dec 2024 17:37:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XS] Change in coreboot[main]: soc/intel/xeon_sp: Enable IDT_IN_EVERY_STAGE
by Lean Sheng Tan (Code Review) 17 Dec '24

17 Dec '24

Lean Sheng Tan has submitted this change. ( https://review.coreboot.org/c/coreboot/+/85569?usp=email ) Change subject: soc/intel/xeon_sp: Enable IDT_IN_EVERY_STAGE ...................................................................... soc/intel/xeon_sp: Enable IDT_IN_EVERY_STAGE Make use of exception handling in every stage. Additionally this enables breakpoints in all stages, making NULL dereferences and stack overflows easier to detect. TEST: Stack canary exceptions are seen in romstage on ibm/sbp1. Change-Id: I8a9f12b9ae041ce47c14f2ef7f09b029d408260e Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/85569 Reviewed-by: Shuo Liu <shuo.liu(a)intel.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/intel/xeon_sp/Kconfig 1 file changed, 1 insertion(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Shuo Liu: Looks good to me, approved diff --git a/src/soc/intel/xeon_sp/Kconfig b/src/soc/intel/xeon_sp/Kconfig index e1a4ac5..f61de56 100644 --- a/src/soc/intel/xeon_sp/Kconfig +++ b/src/soc/intel/xeon_sp/Kconfig @@ -46,6 +46,7 @@ select USE_FSP_NOTIFY_PHASE_POST_PCI_ENUM select USE_FSP_NOTIFY_PHASE_READY_TO_BOOT select USE_FSP_NOTIFY_PHASE_END_OF_FIRMWARE + select IDT_IN_EVERY_STAGE if XEON_SP_COMMON_BASE -- To view, visit https://review.coreboot.org/c/coreboot/+/85569?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I8a9f12b9ae041ce47c14f2ef7f09b029d408260e Gerrit-Change-Number: 85569 Gerrit-PatchSet: 4 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Ray Ni <ray.ni(a)intel.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: yuchi.chen(a)intel.com

1 0

[XS] Change in coreboot[main]: soc/intel/xeon_sp: Enable IDT_IN_EVERY_STAGE
by Lean Sheng Tan (Code Review) 17 Dec '24

17 Dec '24

Attention is currently required from: Christian Walter, Jincheng Li, Johnny Lin, Jonathan Zhang, Patrick Rudolph, Ray Ni, Tim Chu, yuchi.chen(a)intel.com. Lean Sheng Tan has posted comments on this change by Patrick Rudolph. ( https://review.coreboot.org/c/coreboot/+/85569?usp=email ) Change subject: soc/intel/xeon_sp: Enable IDT_IN_EVERY_STAGE ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: > We need to wait till https://review.coreboot.org/c/coreboot/+/85568/3?usp=related-change merged. Done -- To view, visit https://review.coreboot.org/c/coreboot/+/85569?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I8a9f12b9ae041ce47c14f2ef7f09b029d408260e Gerrit-Change-Number: 85569 Gerrit-PatchSet: 3 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Ray Ni <ray.ni(a)intel.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Reviewer: yuchi.chen(a)intel.com Gerrit-Attention: Ray Ni <ray.ni(a)intel.com> Gerrit-Attention: yuchi.chen(a)intel.com Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Jincheng Li <jincheng.li(a)intel.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Tue, 17 Dec 2024 17:37:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com>

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit December 2024