coreboot-gerrit July 2021

coreboot-gerrit@coreboot.org

1 participants
5824 discussions

Change in coreboot[master]: soc/intel/alderlake: Switch to runtime generation of Intel Power Engine
by Furquan Shaikh (Code Review) 15 Jul '21

15 Jul '21

Attention is currently required from: Tim Wawrzynczak, Patrick Rudolph. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56006 ) Change subject: soc/intel/alderlake: Switch to runtime generation of Intel Power Engine ...................................................................... Patch Set 2: (1 comment) File src/soc/intel/alderlake/pmc.c: https://review.coreboot.org/c/coreboot/+/56006/comment/3ea84f03_bb703307 PS2, Line 127: if (CONFIG(SOC_INTEL_COMMON_BLOCK_ACPI_PEP)) Is this required? -- To view, visit https://review.coreboot.org/c/coreboot/+/56006 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I617bc3d1c3cf4ac6b6cbbd790dcf62e731024834 Gerrit-Change-Number: 56006 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Furquan Shaikh <furquan(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Comment-Date: Wed, 14 Jul 2021 22:23:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/intel/common/block/acpi: Add LPM requirements support to PEPD _DSM
by Furquan Shaikh (Code Review) 15 Jul '21

15 Jul '21

Attention is currently required from: Tim Wawrzynczak, Michael Niewöhner, Patrick Rudolph, Karthik Ramasubramanian. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56005 ) Change subject: soc/intel/common/block/acpi: Add LPM requirements support to PEPD _DSM ...................................................................... Patch Set 3: (4 comments) File src/soc/intel/common/block/acpi/pep.c: https://review.coreboot.org/c/coreboot/+/56005/comment/ac4be9c9_f7b34c2e PS3, Line 37: LPM_STATES If I understand this correctly, the LPM_STATES here refer to the S0ix states (S0i2.0, S0i2.1...S0i3.4) as outlined in `LPM_EN` register in PCH EDS vol2. A few questions: 1. On TGL, we had set FSP UPD to decide what S0ix sub-states should be enabled. For ADL, we are leaving that UPD at default. Don't we need to configure those for brya? 2. Depending upon what S0ix sub-states are actually enabled, can we skip the requirements for any sub-states that are disabled by default? We can probably check LPM_EN state to determine which states need to collect the requirements. 3. Looks like this requirement collection is dependent on S0ix sub-states supported and enabled by the SoC and platform. In that case, shouldn't there be appropriate callbacks to get this information from the SoC rather than assuming the LPM information? https://review.coreboot.org/c/coreboot/+/56005/comment/b3de49b5_db33cde3 PS3, Line 39: PMC_LPM_REQ_BASE + i * LPM_REQ_REG_STRIDE + : j * sizeof(uint32_t) I wonder if we need something like a `struct pmc_lpm` which provides information about: 1. num_substates /* Num of S0ix sub-states */ 2. num_status_regs /* Num of LPM status registers for each sub-state */ 3. lpm_ipc_offset /* Offset of LPM registers for PMC IPC */ 4. status_reg_stride /* Register stride for each sub-state */ This will allow it to be defined differently for each SoC. https://review.coreboot.org/c/coreboot/+/56005/comment/2e52e17f_a4e7184f PS3, Line 45: pmc_send_ipc_cmd Just curious: How much time does this function take since it is making multiple calls to the PMC? Also, have we ensured that we skip this in FSP so that we don't have to bear the penalty of doing the operations twice? https://review.coreboot.org/c/coreboot/+/56005/comment/cb3905e4_dec7217f PS3, Line 197: DSM_UUID(PEP_S0IX_UUID, pep_s0ix, ARRAY_SIZE(pep_s0ix), (void *)reg), nit: Is there value in exposing this _DSM with 0 supported functions if !SOC_INTEL_COMMON_BLOCK_ACPI_PEP_LPM_REQ? What do you think about replacing `acpigen_write_dsm_uuid_arr` below with: ``` acpigen_write_dsm(LPI_S0_HELPER_UUID, lpi_s0_helpers, ARRAY_SIZE(lpi_s0_helpers), NULL); if (CONFIG(SOC_INTEL_COMMON_BLOCK_ACPI_PEP_LPM_REQ)) { void *reg = read_pmc_lpm_requirements(); acpigen_write_dsm(PEP_S0IX_UUID, pep_s0ix, ARRAY_SIZE(pep_s0ix), reg); } ``` -- To view, visit https://review.coreboot.org/c/coreboot/+/56005 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I542290bd5490aa6580a5ae2b266da3d78bc17e6b Gerrit-Change-Number: 56005 Gerrit-PatchSet: 3 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Comment-Date: Wed, 14 Jul 2021 22:22:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: lib/thread: Add thread_handle
by build bot (Jenkins) (Code Review) 15 Jul '21

15 Jul '21

Attention is currently required from: Julius Werner, Eric Peers, Felix Held. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56229 ) Change subject: lib/thread: Add thread_handle ...................................................................... Patch Set 5: (1 comment) File src/include/thread.h: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-124252): https://review.coreboot.org/c/coreboot/+/56229/comment/ebc95246_b27ddfc5 PS5, Line 33: enum cb_err (*entry)(void *); function definition argument 'void *' should also have an identifier name -- To view, visit https://review.coreboot.org/c/coreboot/+/56229 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie6f64d0c5a5acad4431a605f0b0b5100dc5358ff Gerrit-Change-Number: 56229 Gerrit-PatchSet: 5 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Eric Peers <epeers(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Eric Peers <epeers(a)google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 14 Jul 2021 22:21:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/cezanne/mca: add empty mca_check_all_banks function
by Felix Held (Code Review) 14 Jul '21

14 Jul '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/56280 ) Change subject: soc/amd/cezanne/mca: add empty mca_check_all_banks function ...................................................................... soc/amd/cezanne/mca: add empty mca_check_all_banks function This will allow factoring out and moving check_mca() to soc/amd/common. Change-Id: I92c7657baef17c248a5aef1eda268e9647502837 Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/56280 Reviewed-by: Raul Rangel <rrangel(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/amd/cezanne/mca.c 1 file changed, 6 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved diff --git a/src/soc/amd/cezanne/mca.c b/src/soc/amd/cezanne/mca.c index 24a7788..a8d9702 100644 --- a/src/soc/amd/cezanne/mca.c +++ b/src/soc/amd/cezanne/mca.c @@ -3,11 +3,15 @@ #include <amdblocks/mca.h> #include <cpu/x86/msr.h> +static void mca_check_all_banks(void) +{ + /* TODO: Implement MCAX register checking and BERT table generation. */ +} + /* Check the Machine Check Architecture Extension registers */ void check_mca(void) { - /* TODO: Implement MCAX register checking and BERT table generation. */ - + mca_check_all_banks(); /* mca_clear_status uses the MCA registers and not the MCAX ones. Since they are aliases, we can use either set of registers. */ mca_clear_status(); -- To view, visit https://review.coreboot.org/c/coreboot/+/56280 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I92c7657baef17c248a5aef1eda268e9647502837 Gerrit-Change-Number: 56280 Gerrit-PatchSet: 4 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: soc/amd: move check_mca prototype to soc/amd/common/blocks/include
by Felix Held (Code Review) 14 Jul '21

14 Jul '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/56279 ) Change subject: soc/amd: move check_mca prototype to soc/amd/common/blocks/include ...................................................................... soc/amd: move check_mca prototype to soc/amd/common/blocks/include Change-Id: Ia489dbfba59c334cf29f96a4000cef73b9b797d4 Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/56279 Reviewed-by: Raul Rangel <rrangel(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/amd/cezanne/cpu.c M src/soc/amd/cezanne/include/soc/cpu.h M src/soc/amd/cezanne/mca.c A src/soc/amd/common/block/include/amdblocks/mca.h M src/soc/amd/picasso/cpu.c M src/soc/amd/picasso/include/soc/cpu.h M src/soc/amd/picasso/mca.c M src/soc/amd/stoneyridge/cpu.c M src/soc/amd/stoneyridge/include/soc/cpu.h M src/soc/amd/stoneyridge/mca.c 10 files changed, 14 insertions(+), 9 deletions(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved diff --git a/src/soc/amd/cezanne/cpu.c b/src/soc/amd/cezanne/cpu.c index 5f0a5a1..2ac30b6 100644 --- a/src/soc/amd/cezanne/cpu.c +++ b/src/soc/amd/cezanne/cpu.c @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-only */ #include <amdblocks/cpu.h> +#include <amdblocks/mca.h> #include <amdblocks/reset.h> #include <amdblocks/smm.h> #include <assert.h> diff --git a/src/soc/amd/cezanne/include/soc/cpu.h b/src/soc/amd/cezanne/include/soc/cpu.h index ec83349..27647ad 100644 --- a/src/soc/amd/cezanne/include/soc/cpu.h +++ b/src/soc/amd/cezanne/include/soc/cpu.h @@ -5,6 +5,4 @@ #define CEZANNE_A0_CPUID 0x00a50f00 -void check_mca(void); - #endif /* AMD_CEZANNE_CPU_H */ diff --git a/src/soc/amd/cezanne/mca.c b/src/soc/amd/cezanne/mca.c index 7160256..24a7788 100644 --- a/src/soc/amd/cezanne/mca.c +++ b/src/soc/amd/cezanne/mca.c @@ -1,7 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-only */ +#include <amdblocks/mca.h> #include <cpu/x86/msr.h> -#include <soc/cpu.h> /* Check the Machine Check Architecture Extension registers */ void check_mca(void) diff --git a/src/soc/amd/common/block/include/amdblocks/mca.h b/src/soc/amd/common/block/include/amdblocks/mca.h new file mode 100644 index 0000000..9ea29e8 --- /dev/null +++ b/src/soc/amd/common/block/include/amdblocks/mca.h @@ -0,0 +1,8 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef AMD_BLOCK_MCA_H +#define AMD_BLOCK_MCA_H + +void check_mca(void); + +#endif /* AMD_BLOCK_MCA_H */ diff --git a/src/soc/amd/picasso/cpu.c b/src/soc/amd/picasso/cpu.c index 4e9c9a9..b04d004 100644 --- a/src/soc/amd/picasso/cpu.c +++ b/src/soc/amd/picasso/cpu.c @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-only */ #include <amdblocks/cpu.h> +#include <amdblocks/mca.h> #include <amdblocks/reset.h> #include <amdblocks/smm.h> #include <assert.h> diff --git a/src/soc/amd/picasso/include/soc/cpu.h b/src/soc/amd/picasso/include/soc/cpu.h index 79d5cc6..7051bb8 100644 --- a/src/soc/amd/picasso/include/soc/cpu.h +++ b/src/soc/amd/picasso/include/soc/cpu.h @@ -3,8 +3,6 @@ #ifndef AMD_PICASSO_CPU_H #define AMD_PICASSO_CPU_H -void check_mca(void); - #define RAVEN1_B0_CPUID 0x00810f10 #define PICASSO_B0_CPUID 0x00810f80 #define PICASSO_B1_CPUID 0x00810f81 diff --git a/src/soc/amd/picasso/mca.c b/src/soc/amd/picasso/mca.c index 6787b75..24a921a 100644 --- a/src/soc/amd/picasso/mca.c +++ b/src/soc/amd/picasso/mca.c @@ -1,11 +1,11 @@ /* SPDX-License-Identifier: GPL-2.0-only */ +#include <amdblocks/mca.h> #include <amdblocks/msr_zen.h> #include <amdblocks/reset.h> #include <cpu/x86/lapic.h> #include <cpu/x86/msr.h> #include <acpi/acpi.h> -#include <soc/cpu.h> #include <console/console.h> #include <arch/bert_storage.h> #include <cper.h> diff --git a/src/soc/amd/stoneyridge/cpu.c b/src/soc/amd/stoneyridge/cpu.c index e007637..9283ff7 100644 --- a/src/soc/amd/stoneyridge/cpu.c +++ b/src/soc/amd/stoneyridge/cpu.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */ +#include <amdblocks/mca.h> #include <amdblocks/reset.h> #include <amdblocks/smm.h> #include <cpu/amd/msr.h> diff --git a/src/soc/amd/stoneyridge/include/soc/cpu.h b/src/soc/amd/stoneyridge/include/soc/cpu.h index 528daa4..cb6bb52 100644 --- a/src/soc/amd/stoneyridge/include/soc/cpu.h +++ b/src/soc/amd/stoneyridge/include/soc/cpu.h @@ -14,6 +14,4 @@ #define SOC_EARLY_VMTRR_CAR_HEAP 2 #define SOC_EARLY_VMTRR_TEMPRAM 3 -void check_mca(void); - #endif /* AMD_STONEYRIDGE_CPU_H */ diff --git a/src/soc/amd/stoneyridge/mca.c b/src/soc/amd/stoneyridge/mca.c index bdb06cc..b3e6682 100644 --- a/src/soc/amd/stoneyridge/mca.c +++ b/src/soc/amd/stoneyridge/mca.c @@ -1,11 +1,11 @@ /* SPDX-License-Identifier: GPL-2.0-only */ +#include <amdblocks/mca.h> #include <amdblocks/reset.h> #include <cpu/amd/msr.h> #include <cpu/x86/lapic.h> #include <cpu/x86/msr.h> #include <acpi/acpi.h> -#include <soc/cpu.h> #include <console/console.h> #include <arch/bert_storage.h> #include <cper.h> -- To view, visit https://review.coreboot.org/c/coreboot/+/56279 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia489dbfba59c334cf29f96a4000cef73b9b797d4 Gerrit-Change-Number: 56279 Gerrit-PatchSet: 4 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: soc/amd/stoneyridge: add and use mca_is_valid_bank & mca_get_bank_name
by Felix Held (Code Review) 14 Jul '21

14 Jul '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/56277 ) Change subject: soc/amd/stoneyridge: add and use mca_is_valid_bank & mca_get_bank_name ...................................................................... soc/amd/stoneyridge: add and use mca_is_valid_bank & mca_get_bank_name This patch changes the way how the not implemented MCA bank 3 gets skipped. For the not implemented bank 3 the name gets set to NULL resulting in mca_is_valid_bank returning false causing the bank to get skipped. This is a preparation for commonizing the MCA(X) handing in the soc/amd sub-tree. Change-Id: I40d6a6752504d804c45b445fce7e763e80161211 Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/56277 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Martin Roth <martinroth(a)google.com> --- M src/soc/amd/stoneyridge/mca.c 1 file changed, 18 insertions(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified Martin Roth: Looks good to me, approved diff --git a/src/soc/amd/stoneyridge/mca.c b/src/soc/amd/stoneyridge/mca.c index 50e7fdd..bdb06cc 100644 --- a/src/soc/amd/stoneyridge/mca.c +++ b/src/soc/amd/stoneyridge/mca.c @@ -139,18 +139,33 @@ [0] = "Load-store unit", [1] = "Instruction fetch unit", [2] = "Combined unit", - [3] = "Reserved", + /* Bank 3 is reserved and not all corresponding MSRs are implemented in Family 15h. + Accessing non-existing MSRs causes a general protection fault. */ + [3] = NULL, [4] = "Northbridge", [5] = "Execution unit", [6] = "Floating point unit" }; +static bool mca_is_valid_bank(unsigned int bank) +{ + return (bank < ARRAY_SIZE(mca_bank_name) && mca_bank_name[bank] != NULL); +} + +static const char *mca_get_bank_name(unsigned int bank) +{ + if (mca_is_valid_bank(bank)) + return mca_bank_name[bank]; + else + return ""; +} + static void mca_print_error(unsigned int bank) { msr_t msr; printk(BIOS_WARNING, "#MC Error: core %u, bank %u %s\n", initial_lapicid(), bank, - mca_bank_name[bank]); + mca_get_bank_name(bank)); msr = rdmsr(IA32_MC_STATUS(bank)); printk(BIOS_WARNING, " MC%u_STATUS = %08x_%08x\n", bank, msr.hi, msr.lo); @@ -173,7 +188,7 @@ return; for (unsigned int i = 0 ; i < num_banks ; i++) { - if (i == 3) /* Reserved in Family 15h */ + if (!mca_is_valid_bank(i)) continue; mci.bank = i; -- To view, visit https://review.coreboot.org/c/coreboot/+/56277 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I40d6a6752504d804c45b445fce7e763e80161211 Gerrit-Change-Number: 56277 Gerrit-PatchSet: 5 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: soc/amd/picasso: implement and use mca_get_bank_name
by Felix Held (Code Review) 14 Jul '21

14 Jul '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/56276 ) Change subject: soc/amd/picasso: implement and use mca_get_bank_name ...................................................................... soc/amd/picasso: implement and use mca_get_bank_name Change-Id: I42abff5efcd7c85d2932a7aaacc736d0376cfaa0 Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/56276 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Raul Rangel <rrangel(a)chromium.org> --- M src/soc/amd/picasso/mca.c 1 file changed, 9 insertions(+), 1 deletion(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved diff --git a/src/soc/amd/picasso/mca.c b/src/soc/amd/picasso/mca.c index b03df90..6787b75 100644 --- a/src/soc/amd/picasso/mca.c +++ b/src/soc/amd/picasso/mca.c @@ -165,12 +165,20 @@ return (bank < ARRAY_SIZE(mca_bank_name) && mca_bank_name[bank] != NULL); } +static const char *mca_get_bank_name(unsigned int bank) +{ + if (mca_is_valid_bank(bank)) + return mca_bank_name[bank]; + else + return ""; +} + static void mca_print_error(unsigned int bank) { msr_t msr; printk(BIOS_WARNING, "#MC Error: core %u, bank %u %s\n", initial_lapicid(), bank, - bank < ARRAY_SIZE(mca_bank_name) ? mca_bank_name[bank] : ""); + mca_get_bank_name(bank)); msr = rdmsr(MCAX_STATUS_MSR(bank)); printk(BIOS_WARNING, " MC%u_STATUS = %08x_%08x\n", bank, msr.hi, msr.lo); -- To view, visit https://review.coreboot.org/c/coreboot/+/56276 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I42abff5efcd7c85d2932a7aaacc736d0376cfaa0 Gerrit-Change-Number: 56276 Gerrit-PatchSet: 4 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: soc/amd/picasso: implement and use mca_is_valid_bank
by Felix Held (Code Review) 14 Jul '21

14 Jul '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/56275 ) Change subject: soc/amd/picasso: implement and use mca_is_valid_bank ...................................................................... soc/amd/picasso: implement and use mca_is_valid_bank In mca_check_all_banks only check valid MCA banks for errors. This aligns the Picasso code a bit more with the Stoneyridge code base which will be updated in a follow-up patch. This is a preparation for commonizing the MCA(X) handing in the soc/amd sub-tree. Change-Id: I0c7f3066afd220e6b8bf8308a321189d7a2679f6 Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/56275 Reviewed-by: Raul Rangel <rrangel(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/amd/picasso/mca.c 1 file changed, 8 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved diff --git a/src/soc/amd/picasso/mca.c b/src/soc/amd/picasso/mca.c index ade8c50..b03df90 100644 --- a/src/soc/amd/picasso/mca.c +++ b/src/soc/amd/picasso/mca.c @@ -160,6 +160,11 @@ [22] = "PIE", }; +static bool mca_is_valid_bank(unsigned int bank) +{ + return (bank < ARRAY_SIZE(mca_bank_name) && mca_bank_name[bank] != NULL); +} + static void mca_print_error(unsigned int bank) { msr_t msr; @@ -188,6 +193,9 @@ printk(BIOS_WARNING, "CPU has an unexpected number of MCA banks!\n"); for (unsigned int i = 0 ; i < num_banks ; i++) { + if (!mca_is_valid_bank(i)) + continue; + mci.bank = i; mci.sts = rdmsr(MCAX_STATUS_MSR(i)); if (mci.sts.hi || mci.sts.lo) { -- To view, visit https://review.coreboot.org/c/coreboot/+/56275 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0c7f3066afd220e6b8bf8308a321189d7a2679f6 Gerrit-Change-Number: 56275 Gerrit-PatchSet: 4 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: soc/amd/picasso: check length of mca_bank_name array
by Felix Held (Code Review) 14 Jul '21

14 Jul '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/56274 ) Change subject: soc/amd/picasso: check length of mca_bank_name array ...................................................................... soc/amd/picasso: check length of mca_bank_name array The length of mca_bank_name should match the return value of mca_get_bank_count which gets the number of MCA banks from an MSR. TEST=No error message on serial console on amd/mandolin Change-Id: Ibdad51a7ef27266e110dfbb43188361952618342 Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/56274 Reviewed-by: Raul Rangel <rrangel(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/amd/picasso/mca.c 1 file changed, 3 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved diff --git a/src/soc/amd/picasso/mca.c b/src/soc/amd/picasso/mca.c index fcc9d3e..ade8c50 100644 --- a/src/soc/amd/picasso/mca.c +++ b/src/soc/amd/picasso/mca.c @@ -184,6 +184,9 @@ struct mca_bank_status mci; const unsigned int num_banks = mca_get_bank_count(); + if (ARRAY_SIZE(mca_bank_name) != num_banks) + printk(BIOS_WARNING, "CPU has an unexpected number of MCA banks!\n"); + for (unsigned int i = 0 ; i < num_banks ; i++) { mci.bank = i; mci.sts = rdmsr(MCAX_STATUS_MSR(i)); -- To view, visit https://review.coreboot.org/c/coreboot/+/56274 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibdad51a7ef27266e110dfbb43188361952618342 Gerrit-Change-Number: 56274 Gerrit-PatchSet: 4 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Change in coreboot[master]: soc/amd/common/block/lpc/spi_dma: Implement SPI DMA functionality
by build bot (Jenkins) (Code Review) 14 Jul '21

14 Jul '21

Attention is currently required from: Jason Glenesk, Marshall Dawson, Eric Peers, Karthik Ramasubramanian, Felix Held. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56228 ) Change subject: soc/amd/common/block/lpc/spi_dma: Implement SPI DMA functionality ...................................................................... Patch Set 5: (1 comment) File src/soc/amd/common/block/lpc/spi_dma.c: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-124249): https://review.coreboot.org/c/coreboot/+/56228/comment/d4052859_7958ab64 PS5, Line 71: * it easier to debug why the SPI DMA wasn't used. 'wasn' may be misspelled - perhaps 'was'? -- To view, visit https://review.coreboot.org/c/coreboot/+/56228 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0be555956581fd82bbe1482d8afa8828c61aaa01 Gerrit-Change-Number: 56228 Gerrit-PatchSet: 5 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Eric Peers <epeers(a)google.com> Gerrit-CC: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Eric Peers <epeers(a)google.com> Gerrit-Attention: Karthik Ramasubramanian <kramasub(a)google.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Wed, 14 Jul 2021 21:55:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit July 2021