coreboot-gerrit April 2023

coreboot-gerrit@coreboot.org

1 participants
5761 discussions

[M] Change in coreboot[master]: mb/google/myst: Configure WLAN
by Jon Murphy (Code Review) 03 Apr '23

03 Apr '23

Attention is currently required from: Raul Rangel, Tim Van Patten, Karthik Ramasubramanian, Mark Hasemeyer. Jon Murphy has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74113 ) Change subject: mb/google/myst: Configure WLAN ...................................................................... Patch Set 9: (4 comments) File src/mainboard/google/myst/variants/baseboard/Makefile.inc: https://review.coreboot.org/c/coreboot/+/74113/comment/7246e9c0_aaa6450b PS9, Line 5: romstage-y += gpio.c > nit: Move above `ramstage` to match previous boards. This was done intentionally to make the variable assignment alphabetical. Let me know if you have a strong preference. File src/mainboard/google/myst/variants/baseboard/gpio.c: https://review.coreboot.org/c/coreboot/+/74113/comment/743a184d_eb507af3 PS9, Line 193: static const struct soc_amd_gpio early_gpio_table[] = { > Where did these GPIO values come from? go/myst-gpios https://review.coreboot.org/c/coreboot/+/74113/comment/01b44400_bc2f8a9d PS9, Line 202: pcie_gpio_table > Skyrim defines this as `romstage_gpio_table`, which is used in `baseboard_romstage_gpio_table()`. […] This is how it was named originally on Skyrim. It was changed by AMD, I can apply the same change. https://review.coreboot.org/c/coreboot/+/74113/comment/be44cce1_eddc089f PS9, Line 228: __weak void variant_pcie_gpio_table(const struct soc_amd_gpio **gpio, size_t *size) > This isn't defined for any other board. It's also not called anywhere. […] It's called in mb_pre_fspm in romstage.c in this board. We can align naming with other boards -- To view, visit https://review.coreboot.org/c/coreboot/+/74113 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5059be0bc011978e74ab4245e6ae037aa177ef9b Gerrit-Change-Number: 74113 Gerrit-PatchSet: 9 Gerrit-Owner: Jon Murphy <jpmurphy(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Mark Hasemeyer <markhas(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Tim Van Patten <timvp(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Tim Van Patten <timvp(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Mark Hasemeyer <markhas(a)google.com> Gerrit-Comment-Date: Mon, 03 Apr 2023 22:03:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Van Patten <timvp(a)google.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: acpi: Add warning message when timeout in write_delay_until function
by Cliff Huang (Code Review) 03 Apr '23

03 Apr '23

Attention is currently required from: Lance Zhao, Cliff Huang, Tim Wawrzynczak. Hello Lance Zhao, build bot (Jenkins), Jérémy Compostella, Tim Wawrzynczak, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/73348 to look at the new patch set (#4). Change subject: acpi: Add warning message when timeout in write_delay_until function ...................................................................... acpi: Add warning message when timeout in write_delay_until function The ACPI should print warning message to kernel when timeout during the loop waiting for the condition to occur This timeout message won't be displayed when this function are used as delay loop, (ie. without checking variable condition) To enable this print to kernel log: echo 1 > /sys/module/acpi/parameters/aml_debug_output ex: Waiting for variable L23E = 0; the max loop count( Local7) = 8 will generate ACPI code: Local7 = 0x08 While ((Local7 > Zero)) { If ((L23E == Zero)) { Break } Sleep (0x10) Local7-- If ((Local7 == Zero)) { Debug = "WARN: Wait loop Timeout! variable = L23E" } } BRANCH=firmware-brya-14505.B TEST=Boot to OS and check that the Debug print is added to the function. Signed-off-by: Cliff Huang <cliff.huang(a)intel.com> Change-Id: I3843e51988527e99822017d1b5f653ff2eaa7958 --- M src/acpi/acpigen.c 1 file changed, 47 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/48/73348/4 -- To view, visit https://review.coreboot.org/c/coreboot/+/73348 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3843e51988527e99822017d1b5f653ff2eaa7958 Gerrit-Change-Number: 73348 Gerrit-PatchSet: 4 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anil Kumar K <anil.kumar.k(a)intel.com> Gerrit-CC: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-CC: Hannah Williams <hannah.williams(a)intel.com> Gerrit-CC: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[master]: acpigen: Add print-like function for debug string
by Cliff Huang (Code Review) 03 Apr '23

03 Apr '23

Attention is currently required from: Lance Zhao, Bora Guvendik, Hannah Williams, Jakub Czapiga, Paul Menzel, Tim Wawrzynczak, Angel Pons, Nick Vaccaro. Cliff Huang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/73113 ) Change subject: acpigen: Add print-like function for debug string ...................................................................... Patch Set 8: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/73113/comment/2d78ff3d_4af2dd9e PS7, Line 7: acpi: Add print-like function for debug string > > acpigen: Add printf-like function for debug string got it! done. -- To view, visit https://review.coreboot.org/c/coreboot/+/73113 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id4a42e5854516a22b7bc4559c2ed08680722c5ba Gerrit-Change-Number: 73113 Gerrit-PatchSet: 8 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Comment-Date: Mon, 03 Apr 2023 21:55:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Cliff Huang <cliff.huang(a)intel.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: acpigen: Add print-like function for debug string
by Cliff Huang (Code Review) 03 Apr '23

03 Apr '23

Attention is currently required from: Lance Zhao, Bora Guvendik, Hannah Williams, Cliff Huang, Jakub Czapiga, Paul Menzel, Tim Wawrzynczak, Nick Vaccaro. Hello Bora Guvendik, Lance Zhao, build bot (Jenkins), Hannah Williams, Jérémy Compostella, Tim Wawrzynczak, Nick Vaccaro, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/73113 to look at the new patch set (#9). Change subject: acpigen: Add print-like function for debug string ...................................................................... acpigen: Add print-like function for debug string Generate formatted string and generate code for printing this string in ACPI. ex: pcie_rp = 1 acpigen_write_debug_sprintf("calling _ON for RP: %u", pcie_rp); will generate ACPI code: Debug = "calling _ON for RP: 1" With this new function, the following functions won't be needed and therefore to be removed: acpigen_concatenate_string_string() acpigen_concatenate_string_int() acpigen_write_debug_concatenate_string_string() acpigen_write_debug_concatenate_string_int() BRANCH=firmware-brya-14505.B TEST=Add above functions in the acpigen code and check the generated SSDT table after OS boot. Check the debug messages is in the kernel log when /sys/modules/acpi/parameters/aml_debug_output is set to '1'. Signed-off-by: Cliff Huang <cliff.huang(a)intel.com> Change-Id: Id4a42e5854516a22b7bc4559c2ed08680722c5ba --- M src/acpi/acpigen.c M src/include/acpi/acpigen.h 2 files changed, 51 insertions(+), 42 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/73113/9 -- To view, visit https://review.coreboot.org/c/coreboot/+/73113 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id4a42e5854516a22b7bc4559c2ed08680722c5ba Gerrit-Change-Number: 73113 Gerrit-PatchSet: 9 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[master]: mb/google/myst: Enable chromeOS EC
by Raul Rangel (Code Review) 03 Apr '23

03 Apr '23

Attention is currently required from: Jon Murphy, Eric Lai, Tim Van Patten, Karthik Ramasubramanian, Mark Hasemeyer. Raul Rangel has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74099 ) Change subject: mb/google/myst: Enable chromeOS EC ...................................................................... Patch Set 5: (1 comment) File src/mainboard/google/myst/Kconfig: https://review.coreboot.org/c/coreboot/+/74099/comment/bd457127_bb502547 PS5, Line 19: : config CHROMEOS : select EC_GOOGLE_CHROMEEC_SWITCHES > We should match Skyrim. Skyrim started this way too but it caused non-chromeos builds to fail. Are we targeting non-chromeos builds? IMO we should be using verstage after bootblock for bringup instead of trying to get a non-chromeos build working. -- To view, visit https://review.coreboot.org/c/coreboot/+/74099 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id18a311097d575973087eb92fd446a5c511f570e Gerrit-Change-Number: 74099 Gerrit-PatchSet: 5 Gerrit-Owner: Jon Murphy <jpmurphy(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Mark Hasemeyer <markhas(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Tim Van Patten <timvp(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Attention: Jon Murphy <jpmurphy(a)google.com> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Attention: Tim Van Patten <timvp(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Mark Hasemeyer <markhas(a)google.com> Gerrit-Comment-Date: Mon, 03 Apr 2023 21:45:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Jon Murphy <jpmurphy(a)google.com> Comment-In-Reply-To: Tim Van Patten <timvp(a)google.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: mb/google/myst: Add eSPI configuration
by Jon Murphy (Code Review) 03 Apr '23

03 Apr '23

1 0

[S] Change in coreboot[master]: mb/google/myst: Enable eSPI SCI events
by Jon Murphy (Code Review) 03 Apr '23

03 Apr '23

Attention is currently required from: Raul Rangel, Tim Van Patten, Karthik Ramasubramanian, Mark Hasemeyer. Jon Murphy has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74101 ) Change subject: mb/google/myst: Enable eSPI SCI events ...................................................................... Patch Set 7: (1 comment) File src/mainboard/google/myst/ec.c: https://review.coreboot.org/c/coreboot/+/74101/comment/230ee278_988b53e2 PS6, Line 14: GEVENT_3 > We use a #define on guybrush to make it easier to understand: https://review.coreboot. […] Ack, forgot that we went through iterations on this on Skyrim. Updated to reflect those changes. -- To view, visit https://review.coreboot.org/c/coreboot/+/74101 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8fd858c484f6fcf952bcb4f756ba2e4728091d8b Gerrit-Change-Number: 74101 Gerrit-PatchSet: 7 Gerrit-Owner: Jon Murphy <jpmurphy(a)google.com> Gerrit-Reviewer: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Mark Hasemeyer <markhas(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Tim Van Patten <timvp(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Tim Van Patten <timvp(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Mark Hasemeyer <markhas(a)google.com> Gerrit-Comment-Date: Mon, 03 Apr 2023 21:38:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org> Comment-In-Reply-To: Jon Murphy <jpmurphy(a)google.com> Comment-In-Reply-To: Tim Van Patten <timvp(a)google.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: mb/google/myst: Enable chromeOS EC
by Jon Murphy (Code Review) 03 Apr '23

03 Apr '23

Attention is currently required from: Raul Rangel, Eric Lai, Tim Van Patten, Karthik Ramasubramanian, Mark Hasemeyer. Jon Murphy has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74099 ) Change subject: mb/google/myst: Enable chromeOS EC ...................................................................... Patch Set 5: (6 comments) File src/mainboard/google/myst/Kconfig: https://review.coreboot.org/c/coreboot/+/74099/comment/e67df7c7_4bbf925b PS5, Line 19: : config CHROMEOS : select EC_GOOGLE_CHROMEEC_SWITCHES > Why is this different from Skyrim, which doesn't have `config CHROMEOS`? […] We should match Skyrim. Skyrim started this way too but it caused non-chromeos builds to fail. https://review.coreboot.org/c/coreboot/+/74099/comment/d1d6f50a_2fb8a242 PS5, Line 29: config IGNORE_IASL_MISSING_DEPENDENCY : def_bool y > What is this? I don't see it in code search. Didn't realize we had dropped this: https://review.coreboot.org/c/coreboot/+/63525. Updated to remove it. File src/mainboard/google/myst/variants/baseboard/include/baseboard/ec.h: https://review.coreboot.org/c/coreboot/+/74099/comment/0b4c2510_aaca24ec PS5, Line 26: | EC_HOST_EVENT_MASK(EC_HOST_EVENT_PANIC)) > Missing: `EC_HOST_EVENT_BODY_DETECT_CHANGE` Done https://review.coreboot.org/c/coreboot/+/74099/comment/9904e617_673f0cf8 PS5, Line 58: #define EC_SCI_GPI GEVENT_24 /* eSPI system event -> GPE 24 */ > GPE11? […] This event is correct. Raul clarified in a previous comment https://review.coreboot.org/c/coreboot/+/74099/comment/6a79baf0_880c34f2 PS5, Line 62: #define EC_ENABLE_WAKE_PIN GEVENT_5 /* AGPIO 11 -> GPE 5 */ > `GEVENT_5` is not listed, but neither is the lid switch, unless there's some abreviation I'm missing […] It should be GPE 13. It corresponds to EC_SOC_WAKE_L. Good catch, updated the CL. https://review.coreboot.org/c/coreboot/+/74099/comment/97656ea0_b7384664 PS5, Line 76: #define EC_SYNC_IRQ GPIO_84 > I'm not sure what the "sync" IRQ is, but GPIO 74 is `FANIN0`. It should correspond to EC_SOC_INT_ODL. This should be GPIO90. Updated the CL. -- To view, visit https://review.coreboot.org/c/coreboot/+/74099 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id18a311097d575973087eb92fd446a5c511f570e Gerrit-Change-Number: 74099 Gerrit-PatchSet: 5 Gerrit-Owner: Jon Murphy <jpmurphy(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Mark Hasemeyer <markhas(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Tim Van Patten <timvp(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Eric Lai <eric_lai(a)quanta.corp-partner.google.com> Gerrit-Attention: Tim Van Patten <timvp(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Mark Hasemeyer <markhas(a)google.com> Gerrit-Comment-Date: Mon, 03 Apr 2023 21:38:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tim Van Patten <timvp(a)google.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: acpi/acpigen: Add print-like function for debug string
by Angel Pons (Code Review) 03 Apr '23

03 Apr '23

Attention is currently required from: Lance Zhao, Bora Guvendik, Hannah Williams, Cliff Huang, Jakub Czapiga, Paul Menzel, Tim Wawrzynczak, Nick Vaccaro. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/73113 ) Change subject: acpi/acpigen: Add print-like function for debug string ...................................................................... Patch Set 8: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/73113/comment/bdc50369_663ced59 PS7, Line 7: acpi: Add print-like function for debug string > acpigen: Add printf-like function for debug string -- To view, visit https://review.coreboot.org/c/coreboot/+/73113 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id4a42e5854516a22b7bc4559c2ed08680722c5ba Gerrit-Change-Number: 73113 Gerrit-PatchSet: 8 Gerrit-Owner: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Lance Zhao <lance.zhao(a)gmail.com> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Jakub Czapiga <jacz(a)semihalf.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Tim Wawrzynczak <inforichland(a)gmail.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Comment-Date: Mon, 03 Apr 2023 21:26:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Cliff Huang <cliff.huang(a)intel.com> Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: option: Allow to use the EFI variable driver as option backend
by Lean Sheng Tan (Code Review) 03 Apr '23

03 Apr '23

Lean Sheng Tan has submitted this change. ( https://review.coreboot.org/c/coreboot/+/62562 ) ( 20 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )Change subject: option: Allow to use the EFI variable driver as option backend ...................................................................... option: Allow to use the EFI variable driver as option backend Use the introduced EFI variable store driver on top of the SMMSTORE region in SPI flash to read/write options. Change-Id: I520eca96bcd573f825ed35a29bf8f750e313a02d Signed-off-by: Patrick Rudolph <patrick.rudolph(a)9elements.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/62562 Reviewed-by: Lean Sheng Tan <sheng.tan(a)9elements.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/Kconfig A src/drivers/efi/option.c 2 files changed, 70 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Lean Sheng Tan: Looks good to me, approved diff --git a/src/Kconfig b/src/Kconfig index 16f7881..e35df43 100644 --- a/src/Kconfig +++ b/src/Kconfig @@ -142,6 +142,8 @@ prompt "Option backend to use" default USE_MAINBOARD_SPECIFIC_OPTION_BACKEND if HAVE_MAINBOARD_SPECIFIC_OPTION_BACKEND default USE_OPTION_TABLE if NVRAMCUI_SECONDARY_PAYLOAD + default USE_UEFI_VARIABLE_STORE if DRIVERS_EFI_VARIABLE_STORE && \ + PAYLOAD_EDK2 && SMMSTORE_V2 config OPTION_BACKEND_NONE bool "None" @@ -153,6 +155,15 @@ Enable this option if coreboot shall read options from the "CMOS" NVRAM instead of using hard-coded values. +config USE_UEFI_VARIABLE_STORE + bool "Use UEFI variable-store in SPI flash as option backend" + depends on DRIVERS_EFI_VARIABLE_STORE + depends on SMMSTORE_V2 + help + Enable this option if coreboot shall read/write options from the + SMMSTORE region within the SPI flash. The region must be formatted + by the payload first before it can be used. + config USE_MAINBOARD_SPECIFIC_OPTION_BACKEND bool "Use mainboard-specific option backend" depends on HAVE_MAINBOARD_SPECIFIC_OPTION_BACKEND diff --git a/src/drivers/efi/option.c b/src/drivers/efi/option.c new file mode 100644 index 0000000..3960cfc --- /dev/null +++ b/src/drivers/efi/option.c @@ -0,0 +1,43 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#include <stdlib.h> +#include <string.h> +#include <option.h> +#include <smmstore.h> + +#include <Uefi/UefiBaseType.h> + +#include "efivars.h" + +static const EFI_GUID EficorebootNvDataGuid = { + 0xceae4c1d, 0x335b, 0x4685, { 0xa4, 0xa0, 0xfc, 0x4a, 0x94, 0xee, 0xa0, 0x85 } }; + +unsigned int get_uint_option(const char *name, const unsigned int fallback) +{ + struct region_device rdev; + enum cb_err ret; + uint32_t var; + uint32_t size; + + if (smmstore_lookup_region(&rdev)) + return fallback; + + var = 0; + size = sizeof(var); + ret = efi_fv_get_option(&rdev, &EficorebootNvDataGuid, name, &var, &size); + if (ret != CB_SUCCESS) + return fallback; + + return var; +} + +enum cb_err set_uint_option(const char *name, unsigned int value) +{ + struct region_device rdev; + uint32_t var = value; + + if (smmstore_lookup_region(&rdev)) + return CB_CMOS_OTABLE_DISABLED; + + return efi_fv_set_option(&rdev, &EficorebootNvDataGuid, name, &var, sizeof(var)); +} -- To view, visit https://review.coreboot.org/c/coreboot/+/62562 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I520eca96bcd573f825ed35a29bf8f750e313a02d Gerrit-Change-Number: 62562 Gerrit-PatchSet: 24 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Elias Souza <eliascontato(a)protonmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Tim Crawford <tcrawford(a)system76.com> Gerrit-MessageType: merged

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2023