coreboot-gerrit May 2021

coreboot-gerrit@coreboot.org

1 participants
6048 discussions

Change in coreboot[master]: arch/x86/include/bert_storage: introduce and use bert_generate_ssdt
by Angel Pons (Code Review) 28 May '21

28 May '21

Attention is currently required from: Arthur Heymans, Felix Held. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55024 ) Change subject: arch/x86/include/bert_storage: introduce and use bert_generate_ssdt ...................................................................... Patch Set 2: Code-Review+1 (1 comment) File src/arch/x86/include/arch/bert_storage.h: https://review.coreboot.org/c/coreboot/+/55024/comment/eb9a06c9_1cf9c231 PS2, Line 60: bert_generate_ssdt To me, the `bert_generate_ssdt` function name sounds like it's a procedure that generates the BERT SSDT. Instead, I'd name this function `bert_should_generate_ssdt` to avoid confusion. -- To view, visit https://review.coreboot.org/c/coreboot/+/55024 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia955f627c190ea38e05b5aaedc7cb2d030274e83 Gerrit-Change-Number: 55024 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 28 May 2021 15:50:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common: Update CrashLog data length tracking
by Felix Held (Code Review) 28 May '21

28 May '21

Attention is currently required from: Francois Toguo Fotso, Martin Roth, Tim Wawrzynczak, Patrick Rudolph. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55006 ) Change subject: soc/intel/common: Update CrashLog data length tracking ...................................................................... Patch Set 3: (1 comment) Patchset: PS2: > Felix, does this help unbreak your BERT flow? if i understood it correctly, this makes the intel code still work when CB:54738 is applied. tested the full patch train in the form that i pushed on amd/mandolin -- To view, visit https://review.coreboot.org/c/coreboot/+/55006 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I97ff14d62bda69389c7647fcbbf23d5cab2b36e6 Gerrit-Change-Number: 55006 Gerrit-PatchSet: 3 Gerrit-Owner: Francois Toguo Fotso <francois.toguo.fotso(a)intel.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-CC: Nikunj Dadhania <nikunj.dadhania(a)intel.com> Gerrit-Attention: Francois Toguo Fotso <francois.toguo.fotso(a)intel.com> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Fri, 28 May 2021 15:48:03 +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]: arch/x86/acpi_bert_storage: change return type of bert_errors_present
by Angel Pons (Code Review) 28 May '21

28 May '21

Attention is currently required from: Felix Held. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55023 ) Change subject: arch/x86/acpi_bert_storage: change return type of bert_errors_present ...................................................................... Patch Set 2: Code-Review+1 (2 comments) File src/arch/x86/acpi_bert_storage.c: https://review.coreboot.org/c/coreboot/+/55023/comment/96e1dc4c_5551f8f5 PS2, Line 23: int retype this too? https://review.coreboot.org/c/coreboot/+/55023/comment/17b2799a_fda44ddb PS2, Line 38: return bert_region_broken ? 0 : !!bert_region_used; How about: return !bert_region_broken && bert_region_used; -- To view, visit https://review.coreboot.org/c/coreboot/+/55023 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I13d6472deeb26ba92d257761df069e32d9b2e5d4 Gerrit-Change-Number: 55023 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 28 May 2021 15:47:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: arch/x86/include/bert_storage: introduce and use bert_generate_ssdt
by Felix Held (Code Review) 28 May '21

28 May '21

Attention is currently required from: Arthur Heymans. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55024 ) Change subject: arch/x86/include/bert_storage: introduce and use bert_generate_ssdt ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/55024/comment/182705b3_d8d3c507 PS1, Line 7: x8 > typo: x86 Done -- To view, visit https://review.coreboot.org/c/coreboot/+/55024 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia955f627c190ea38e05b5aaedc7cb2d030274e83 Gerrit-Change-Number: 55024 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Fri, 28 May 2021 15:46:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: acpi: Add support for reporting CrashLog in BERT table
by Felix Held (Code Review) 28 May '21

28 May '21

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49799 ) Change subject: acpi: Add support for reporting CrashLog in BERT table ...................................................................... Patch Set 10: (1 comment) Patchset: PS10: > Starting next week I will be out of office for about 2 months and won't be available to review patch […] i have pushed a patch train that unifies the AMD and Intel BERT flow: https://review.coreboot.org/c/coreboot/+/54149/4 is the top of that patch train; would be great if you could test that this doesn't break your platform; i'm rather confident that it won't change behaviour, since it mostly moves some bits of code around. I'm still not sure if there's something off with the raw_data_* struct elements, but at least now the linux kernel doesn't fail to decode the bert region. Not sure if/when i'll get to the bottom of that, but at least the code is in a better shape now. Enjoy your vacation! -- To view, visit https://review.coreboot.org/c/coreboot/+/49799 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I00e390d735d61beac2e89a726e39119d9b06b3df Gerrit-Change-Number: 49799 Gerrit-PatchSet: 10 Gerrit-Owner: Francois Toguo Fotso <francois.toguo.fotso(a)intel.com> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nikunj Dadhania <nikunj.dadhania(a)intel.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com> Gerrit-CC: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-CC: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Raj Astekar <raj.astekar(a)intel.com> Gerrit-Comment-Date: Fri, 28 May 2021 15:45:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Francois Toguo Fotso <francois.toguo.fotso(a)intel.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [WIP] pc80/rtc/option: Move register backup for SMI
by Angel Pons (Code Review) 28 May '21

28 May '21

Attention is currently required from: Kyösti Mälkki, Alexander Couzens, Patrick Rudolph. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/49253 ) Change subject: [WIP] pc80/rtc/option: Move register backup for SMI ...................................................................... Patch Set 6: (1 comment) File src/southbridge/intel/lynxpoint/smihandler.c: https://review.coreboot.org/c/coreboot/+/49253/comment/ea51fd6a_8fd145ae PS6, Line 85: tmp70 = inb(0x70); Datasheet says this register is write-only. -- To view, visit https://review.coreboot.org/c/coreboot/+/49253 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I025e9952a949834755f5cbc3dfe7b26ab9aaa032 Gerrit-Change-Number: 49253 Gerrit-PatchSet: 6 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> 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-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Fri, 28 May 2021 15:36:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/stoneyridge: use common BERT SSDT generation
by Felix Held (Code Review) 28 May '21

28 May '21

Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/55055 ) Change subject: soc/amd/stoneyridge: use common BERT SSDT generation ...................................................................... soc/amd/stoneyridge: use common BERT SSDT generation Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I86d4f5ef74d4d40cb93ac4a3feaf28b99022ebd5 --- M src/soc/amd/stoneyridge/northbridge.c 1 file changed, 19 insertions(+), 21 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/55/55055/1 diff --git a/src/soc/amd/stoneyridge/northbridge.c b/src/soc/amd/stoneyridge/northbridge.c index 1998e1b..fc6d39e 100644 --- a/src/soc/amd/stoneyridge/northbridge.c +++ b/src/soc/amd/stoneyridge/northbridge.c @@ -229,7 +229,6 @@ acpi_header_t *alib; acpi_header_t *ivrs; acpi_hest_t *hest; - acpi_bert_t *bert; /* HEST */ current = ALIGN(current, 8); @@ -238,26 +237,6 @@ acpi_add_table(rsdp, (void *)current); current += hest->header.length; - /* BERT */ - if (bert_generate_ssdt()) { - /* Skip the table if no errors are present. ACPI driver reports - * a table with a 0-length region: - * BERT: [Firmware Bug]: table invalid. - */ - void *rgn; - size_t size; - bert_errors_region(&rgn, &size); - if (!rgn) { - printk(BIOS_ERR, "Error: Can't find BERT storage area\n"); - } else { - current = ALIGN(current, 8); - bert = (acpi_bert_t *)current; - acpi_write_bert(bert, (uintptr_t)rgn, size); - acpi_add_table(rsdp, (void *)current); - current += bert->header.length; - } - } - current = ALIGN(current, 8); printk(BIOS_DEBUG, "ACPI: * IVRS at %lx\n", current); ivrs = agesawrapper_getlateinitptr(PICK_IVRS); @@ -347,6 +326,25 @@ .devices = pci_device_ids, }; +enum cb_err acpi_soc_get_bert_region(void **region, size_t *length) +{ + /* + * Skip the table if no errors are present. ACPI driver reports + * a table with a 0-length region: + * BERT: [Firmware Bug]: table invalid. + */ + if (!bert_generate_ssdt()) + return CB_ERR; + + bert_errors_region(region, length); + if (!region) { + printk(BIOS_ERR, "Error: Can't find BERT storage area\n"); + return CB_ERR; + } + + return CB_SUCCESS; +} + /* * Enable VGA cycles. Set memory ranges of the FCH legacy devices (TPM, HPET, * BIOS RAM, Watchdog Timer, IOAPIC and ACPI) as non-posted. Set remaining -- To view, visit https://review.coreboot.org/c/coreboot/+/55055 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I86d4f5ef74d4d40cb93ac4a3feaf28b99022ebd5 Gerrit-Change-Number: 55055 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: acpi: rework BERT SSDT generation logic
by Felix Held (Code Review) 28 May '21

28 May '21

Attention is currently required from: Lance Zhao, Tim Wawrzynczak, Patrick Rudolph. Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/55054 ) Change subject: acpi: rework BERT SSDT generation logic ...................................................................... acpi: rework BERT SSDT generation logic Check if the ACPI_BERT Kconfig option is selected and only then try to generate the SSDT. Also remove the acpi_is_boot_error_src_present weak function from the ACPI global function and use the return value of acpi_soc_get_bert_region to determine if there is a valid BERT region with logged errors. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: I2a281f5f636010ba3b2e7e097e9cf53683022aea --- M src/acpi/acpi.c M src/include/acpi/acpi.h M src/soc/intel/common/block/acpi/acpi_bert.c 3 files changed, 33 insertions(+), 35 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/54/55054/1 diff --git a/src/acpi/acpi.c b/src/acpi/acpi.c index 2326169..d7d7d3d 100644 --- a/src/acpi/acpi.c +++ b/src/acpi/acpi.c @@ -1560,11 +1560,6 @@ } /* BERT helpers */ -bool __weak acpi_is_boot_error_src_present(void) -{ - return false; -} - __weak enum cb_err acpi_soc_get_bert_region(void **region, size_t *length) { return CB_ERR; @@ -1813,18 +1808,19 @@ current = acpi_align_current(current); - if (acpi_is_boot_error_src_present()) { + if (CONFIG(ACPI_BERT)) { void *region; size_t size; - printk(BIOS_DEBUG, "ACPI: * BERT\n"); bert = (acpi_bert_t *) current; - acpi_soc_get_bert_region(&region, &size); - acpi_write_bert(bert, (uintptr_t)region, size); - if (bert->header.length >= sizeof(acpi_bert_t)) { - current += bert->header.length; - acpi_add_table(rsdp, bert); + if (acpi_soc_get_bert_region(&region, &size) == CB_SUCCESS) { + printk(BIOS_DEBUG, "ACPI: * BERT\n"); + acpi_write_bert(bert, (uintptr_t)region, size); + if (bert->header.length >= sizeof(acpi_bert_t)) { + current += bert->header.length; + acpi_add_table(rsdp, bert); + } + current = acpi_align_current(current); } - current = acpi_align_current(current); } printk(BIOS_DEBUG, "current = %lx\n", current); diff --git a/src/include/acpi/acpi.h b/src/include/acpi/acpi.h index 7b0a775..b1eb664 100644 --- a/src/include/acpi/acpi.h +++ b/src/include/acpi/acpi.h @@ -1341,7 +1341,6 @@ unsigned long acpi_create_lpi_desc_ncst(acpi_lpi_desc_ncst_t *lpi_desc, uint16_t uid); /* For crashlog. */ -bool acpi_is_boot_error_src_present(void); enum cb_err acpi_soc_get_bert_region(void **region, size_t *length); /* For ACPI S3 support. */ diff --git a/src/soc/intel/common/block/acpi/acpi_bert.c b/src/soc/intel/common/block/acpi/acpi_bert.c index 168f329..f411bb2 100644 --- a/src/soc/intel/common/block/acpi/acpi_bert.c +++ b/src/soc/intel/common/block/acpi/acpi_bert.c @@ -7,6 +7,26 @@ #include <intelblocks/acpi.h> #include <intelblocks/crashlog.h> +static bool boot_error_src_present(void) +{ + + if (!CONFIG(SOC_INTEL_CRASHLOG)) { + printk(BIOS_DEBUG, "Crashlog disabled.\n"); + return false; + } + + if (!discover_crashlog()) { + printk(BIOS_SPEW, "Crashlog discovery result: crashlog not found\n"); + return false; + } + + collect_pmc_and_cpu_crashlog_from_srams(); + + /* Discovery tables sizes can be larger than the actual valid collected data */ + u32 crashlog_size = cl_get_total_data_size(); + + return (crashlog_size > 0); +} enum cb_err acpi_soc_get_bert_region(void **region, size_t *length) { @@ -14,6 +34,10 @@ size_t cpu_record_size, pmc_record_size; void *cl_data = NULL; + if (!boot_error_src_present()) { + return CB_ERR; + } + if (!cl_get_total_data_size()) { printk(BIOS_ERR, "Error: No crashlog record present\n"); return CB_ERR; @@ -71,24 +95,3 @@ return CB_SUCCESS; } - -bool acpi_is_boot_error_src_present(void) -{ - - if (!CONFIG(SOC_INTEL_CRASHLOG)) { - printk(BIOS_DEBUG, "Crashlog disabled.\n"); - return false; - } - - if (!discover_crashlog()) { - printk(BIOS_SPEW, "Crashlog discovery result: crashlog not found\n"); - return false; - } - - collect_pmc_and_cpu_crashlog_from_srams(); - - /* Discovery tables sizes can be larger than the actual valid collected data */ - u32 crashlog_size = cl_get_total_data_size(); - - return (crashlog_size > 0); -} -- To view, visit https://review.coreboot.org/c/coreboot/+/55054 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2a281f5f636010ba3b2e7e097e9cf53683022aea Gerrit-Change-Number: 55054 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Lance Zhao Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: acpi: rename acpi_soc_fill_bert and add return value
by Felix Held (Code Review) 28 May '21

28 May '21

Attention is currently required from: Lance Zhao, Tim Wawrzynczak, Patrick Rudolph. Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/55053 ) Change subject: acpi: rename acpi_soc_fill_bert and add return value ...................................................................... acpi: rename acpi_soc_fill_bert and add return value The return value indicates if the function has found valid BERT data and wrote them to the region and length parameters. This will be used in a follow-up patch to remove the acpi_is_boot_error_src_present function call in the common code. Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Change-Id: Iaaa3eed51645e1b3bc904c6279d171e3a10d59be --- M src/acpi/acpi.c M src/include/acpi/acpi.h M src/soc/intel/common/block/acpi/acpi_bert.c 3 files changed, 14 insertions(+), 10 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/53/55053/1 diff --git a/src/acpi/acpi.c b/src/acpi/acpi.c index 185825b..2326169 100644 --- a/src/acpi/acpi.c +++ b/src/acpi/acpi.c @@ -1565,7 +1565,10 @@ return false; } -__weak void acpi_soc_fill_bert(void **region, size_t *length) {} +__weak enum cb_err acpi_soc_get_bert_region(void **region, size_t *length) +{ + return CB_ERR; +} unsigned long __weak fw_cfg_acpi_tables(unsigned long start) { @@ -1815,7 +1818,7 @@ size_t size; printk(BIOS_DEBUG, "ACPI: * BERT\n"); bert = (acpi_bert_t *) current; - acpi_soc_fill_bert(&region, &size); + acpi_soc_get_bert_region(&region, &size); acpi_write_bert(bert, (uintptr_t)region, size); if (bert->header.length >= sizeof(acpi_bert_t)) { current += bert->header.length; diff --git a/src/include/acpi/acpi.h b/src/include/acpi/acpi.h index dd00c40..7b0a775 100644 --- a/src/include/acpi/acpi.h +++ b/src/include/acpi/acpi.h @@ -1342,7 +1342,7 @@ /* For crashlog. */ bool acpi_is_boot_error_src_present(void); -void acpi_soc_fill_bert(void **region, size_t *length); +enum cb_err acpi_soc_get_bert_region(void **region, size_t *length); /* For ACPI S3 support. */ void __noreturn acpi_resume(void *wake_vec); diff --git a/src/soc/intel/common/block/acpi/acpi_bert.c b/src/soc/intel/common/block/acpi/acpi_bert.c index f94026d..168f329 100644 --- a/src/soc/intel/common/block/acpi/acpi_bert.c +++ b/src/soc/intel/common/block/acpi/acpi_bert.c @@ -8,7 +8,7 @@ #include <intelblocks/crashlog.h> -void acpi_soc_fill_bert(void **region, size_t *length) +enum cb_err acpi_soc_get_bert_region(void **region, size_t *length) { acpi_generic_error_status_t *status = NULL; size_t cpu_record_size, pmc_record_size; @@ -16,19 +16,19 @@ if (!cl_get_total_data_size()) { printk(BIOS_ERR, "Error: No crashlog record present\n"); - return; + return CB_ERR; } status = bert_new_event(&CPER_SEC_FW_ERR_REC_REF_GUID); if (!status) { printk(BIOS_ERR, "Error: unable to allocate GSB\n"); - return; + return CB_ERR; } if (cl_get_total_data_size() > bert_storage_remaining()) { printk(BIOS_ERR, "Error: Crashlog entry would exceed " "available region\n"); - return; + return CB_ERR; } cpu_record_size = cl_get_cpu_record_size(); @@ -38,7 +38,7 @@ printk(BIOS_ERR, "Error: Crashlog CPU entry(size %lu) " "would exceed available region\n", cpu_record_size); - return; + return CB_ERR; } printk(BIOS_DEBUG, "cl_data %p, cpu_record_size %lu\n", cl_data, cpu_record_size); @@ -51,7 +51,7 @@ if (cpu_record_size && !bert_append_fw_err(status)) { printk(BIOS_ERR, "Error: Crashlog PMC entry would " "exceed available region\n"); - return; + return CB_ERR; } cl_data = new_cper_fw_error_crashlog(status, pmc_record_size); @@ -59,7 +59,7 @@ printk(BIOS_ERR, "Error: Crashlog PMC entry(size %lu) " "would exceed available region\n", pmc_record_size); - return; + return CB_ERR; } printk(BIOS_DEBUG, "cl_data %p, pmc_record_size %lu\n", cl_data, pmc_record_size); @@ -69,6 +69,7 @@ *length = status->raw_data_length; *region = (void *)status; + return CB_SUCCESS; } bool acpi_is_boot_error_src_present(void) -- To view, visit https://review.coreboot.org/c/coreboot/+/55053 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iaaa3eed51645e1b3bc904c6279d171e3a10d59be Gerrit-Change-Number: 55053 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Lance Zhao Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Lance Zhao Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: acpi: drop unused parameter from acpi_soc_fill_bert
by Felix Held (Code Review) 28 May '21

28 May '21

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2021