coreboot-gerrit July 2021

coreboot-gerrit@coreboot.org

1 participants
5824 discussions

Change in coreboot[master]: src: use mca_clear_status function instead of open coding
by Felix Held (Code Review) 14 Jul '21

14 Jul '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/56259 ) Change subject: src: use mca_clear_status function instead of open coding ...................................................................... src: use mca_clear_status function instead of open coding Change-Id: I53413b4051b79d7c2f24b1191ce877155e654400 Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/56259 Reviewed-by: Raul Rangel <rrangel(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/cpu/amd/agesa/family14/model_14_init.c M src/cpu/amd/agesa/family15tn/model_15_init.c M src/cpu/amd/agesa/family16kb/model_16_init.c M src/cpu/amd/pi/00730F01/model_16_init.c M src/cpu/intel/haswell/haswell_init.c M src/cpu/intel/model_2065x/model_2065x_init.c M src/cpu/intel/model_206ax/model_206ax_init.c M src/soc/amd/stoneyridge/mca.c M src/soc/intel/common/block/cpu/cpulib.c 9 files changed, 11 insertions(+), 72 deletions(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved diff --git a/src/cpu/amd/agesa/family14/model_14_init.c b/src/cpu/amd/agesa/family14/model_14_init.c index 76c8521..9539c3d 100644 --- a/src/cpu/amd/agesa/family14/model_14_init.c +++ b/src/cpu/amd/agesa/family14/model_14_init.c @@ -15,9 +15,7 @@ static void model_14_init(struct device *dev) { - u8 i; msr_t msr; - unsigned int num_banks; int msrno; #if CONFIG(LOGICAL_CPUS) u32 siblings; @@ -59,11 +57,7 @@ x86_enable_cache(); /* zero the machine check error status registers */ - num_banks = mca_get_bank_count(); - msr.lo = 0; - msr.hi = 0; - for (i = 0; i < num_banks; i++) - wrmsr(IA32_MC_STATUS(i), msr); + mca_clear_status(); /* Enable the local CPU APICs */ setup_lapic(); diff --git a/src/cpu/amd/agesa/family15tn/model_15_init.c b/src/cpu/amd/agesa/family15tn/model_15_init.c index 883bd59..9d4da76 100644 --- a/src/cpu/amd/agesa/family15tn/model_15_init.c +++ b/src/cpu/amd/agesa/family15tn/model_15_init.c @@ -18,9 +18,7 @@ { printk(BIOS_DEBUG, "Model 15 Init.\n"); - u8 i; msr_t msr; - unsigned int num_banks; int msrno; unsigned int cpu_idx; #if CONFIG(LOGICAL_CPUS) @@ -58,11 +56,7 @@ x86_enable_cache(); /* zero the machine check error status registers */ - num_banks = mca_get_bank_count(); - msr.lo = 0; - msr.hi = 0; - for (i = 0; i < num_banks; i++) - wrmsr(IA32_MC_STATUS(i), msr); + mca_clear_status(); /* Enable the local CPU APICs */ setup_lapic(); diff --git a/src/cpu/amd/agesa/family16kb/model_16_init.c b/src/cpu/amd/agesa/family16kb/model_16_init.c index f945f80..9fadc7e 100644 --- a/src/cpu/amd/agesa/family16kb/model_16_init.c +++ b/src/cpu/amd/agesa/family16kb/model_16_init.c @@ -17,9 +17,7 @@ { printk(BIOS_DEBUG, "Model 16 Init.\n"); - u8 i; msr_t msr; - unsigned int num_banks; int msrno; #if CONFIG(LOGICAL_CPUS) u32 siblings; @@ -56,11 +54,7 @@ x86_enable_cache(); /* zero the machine check error status registers */ - num_banks = mca_get_bank_count(); - msr.lo = 0; - msr.hi = 0; - for (i = 0; i < num_banks; i++) - wrmsr(IA32_MC_STATUS(i), msr); + mca_clear_status(); /* Enable the local CPU APICs */ setup_lapic(); diff --git a/src/cpu/amd/pi/00730F01/model_16_init.c b/src/cpu/amd/pi/00730F01/model_16_init.c index 3c78c09..a5a8064 100644 --- a/src/cpu/amd/pi/00730F01/model_16_init.c +++ b/src/cpu/amd/pi/00730F01/model_16_init.c @@ -20,9 +20,7 @@ { printk(BIOS_DEBUG, "Model 16 Init.\n"); - u8 i; msr_t msr; - unsigned int num_banks; u32 siblings; /* @@ -41,11 +39,7 @@ x86_mtrr_check(); /* zero the machine check error status registers */ - num_banks = mca_get_bank_count(); - msr.lo = 0; - msr.hi = 0; - for (i = 0; i < num_banks; i++) - wrmsr(IA32_MC_STATUS(i), msr); + mca_clear_status(); /* Enable the local CPU APICs */ setup_lapic(); diff --git a/src/cpu/intel/haswell/haswell_init.c b/src/cpu/intel/haswell/haswell_init.c index 29c663e..2c6384c 100644 --- a/src/cpu/intel/haswell/haswell_init.c +++ b/src/cpu/intel/haswell/haswell_init.c @@ -527,12 +527,10 @@ for (i = 0; i < num_banks; i++) wrmsr(IA32_MC_CTL(i), msr); - msr.lo = msr.hi = 0; /* TODO(adurbin): This should only be done on a cold boot. Also, some * of these banks are core vs package scope. For now every CPU clears * every bank. */ - for (i = 0; i < num_banks; i++) - wrmsr(IA32_MC_STATUS(i), msr); + mca_clear_status(); } /* All CPUs including BSP will run the following function. */ diff --git a/src/cpu/intel/model_2065x/model_2065x_init.c b/src/cpu/intel/model_2065x/model_2065x_init.c index fe5ac56..f70d7b2 100644 --- a/src/cpu/intel/model_2065x/model_2065x_init.c +++ b/src/cpu/intel/model_2065x/model_2065x_init.c @@ -73,24 +73,13 @@ ((perf_ctl.lo >> 8) & 0xff) * IRONLAKE_BCLK); } -static void configure_mca(void) -{ - msr_t msr; - int i; - const unsigned int num_banks = mca_get_bank_count(); - - msr.lo = msr.hi = 0; - /* This should only be done on a cold boot */ - for (i = 0; i < num_banks; i++) - wrmsr(IA32_MC_STATUS(i), msr); -} - static void model_2065x_init(struct device *cpu) { char processor_name[49]; /* Clear out pending MCEs */ - configure_mca(); + /* This should only be done on a cold boot */ + mca_clear_status(); /* Print processor name */ fill_processor_name(processor_name); diff --git a/src/cpu/intel/model_206ax/model_206ax_init.c b/src/cpu/intel/model_206ax/model_206ax_init.c index 541cb3b..09cad24 100644 --- a/src/cpu/intel/model_206ax/model_206ax_init.c +++ b/src/cpu/intel/model_206ax/model_206ax_init.c @@ -297,18 +297,6 @@ return SANDYBRIDGE_BCLK; } -static void configure_mca(void) -{ - msr_t msr; - int i; - const unsigned int num_banks = mca_get_bank_count(); - - msr.lo = msr.hi = 0; - /* This should only be done on a cold boot */ - for (i = 0; i < num_banks; i++) - wrmsr(IA32_MC_STATUS(i), msr); -} - static void model_206ax_report(void) { static const char *const mode[] = {"NOT ", ""}; @@ -340,7 +328,8 @@ { /* Clear out pending MCEs */ - configure_mca(); + /* This should only be done on a cold boot */ + mca_clear_status(); /* Print infos */ model_206ax_report(); diff --git a/src/soc/amd/stoneyridge/mca.c b/src/soc/amd/stoneyridge/mca.c index 0773101..e0c2791 100644 --- a/src/soc/amd/stoneyridge/mca.c +++ b/src/soc/amd/stoneyridge/mca.c @@ -186,18 +186,8 @@ } } -static void mca_clear_errors(void) -{ - const unsigned int num_banks = mca_get_bank_count(); - const msr_t msr = {.lo = 0, .hi = 0}; - - /* Zero all machine check error status registers */ - for (unsigned int i = 0 ; i < num_banks ; i++) - wrmsr(IA32_MC_STATUS(i), msr); -} - void check_mca(void) { mca_check_all_banks(); - mca_clear_errors(); + mca_clear_status(); } diff --git a/src/soc/intel/common/block/cpu/cpulib.c b/src/soc/intel/common/block/cpu/cpulib.c index a245824..9372513 100644 --- a/src/soc/intel/common/block/cpu/cpulib.c +++ b/src/soc/intel/common/block/cpu/cpulib.c @@ -336,17 +336,14 @@ void mca_configure(void) { - msr_t msr; int i; const unsigned int num_banks = mca_get_bank_count(); printk(BIOS_DEBUG, "Clearing out pending MCEs\n"); - msr.lo = msr.hi = 0; + mca_clear_status(); for (i = 0; i < num_banks; i++) { - /* Clear the machine check status */ - wrmsr(IA32_MC_STATUS(i), msr); /* Initialize machine checks */ wrmsr(IA32_MC_CTL(i), (msr_t) {.lo = 0xffffffff, .hi = 0xffffffff}); -- To view, visit https://review.coreboot.org/c/coreboot/+/56259 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I53413b4051b79d7c2f24b1191ce877155e654400 Gerrit-Change-Number: 56259 Gerrit-PatchSet: 4 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> 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]: include/cpu/x86/msr: add mca_clear_status function
by Felix Held (Code Review) 14 Jul '21

14 Jul '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/56258 ) Change subject: include/cpu/x86/msr: add mca_clear_status function ...................................................................... include/cpu/x86/msr: add mca_clear_status function In multiple locations within the coreboot tree all available IA32_MC_STATUS registers are cleared, so add this to the common code to avoid duplication of code. Change-Id: I04af23c16021b0ce90f7105f7a3856bd26ffa736 Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/56258 Reviewed-by: Raul Rangel <rrangel(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/include/cpu/x86/msr.h 1 file changed, 10 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved diff --git a/src/include/cpu/x86/msr.h b/src/include/cpu/x86/msr.h index dd969d7..51ef2e0 100644 --- a/src/include/cpu/x86/msr.h +++ b/src/include/cpu/x86/msr.h @@ -166,6 +166,16 @@ return msr.lo & MCA_BANKS_MASK; } +/* Clear all MCA status registers */ +static inline void mca_clear_status(void) +{ + const unsigned int num_banks = mca_get_bank_count(); + const msr_t msr = {.lo = 0, .hi = 0}; + + for (unsigned int i = 0 ; i < num_banks ; i++) + wrmsr(IA32_MC_STATUS(i), msr); +} + /* Helpers for interpreting MC[i]_STATUS */ static inline int mca_valid(msr_t msr) -- To view, visit https://review.coreboot.org/c/coreboot/+/56258 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I04af23c16021b0ce90f7105f7a3856bd26ffa736 Gerrit-Change-Number: 56258 Gerrit-PatchSet: 3 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> 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/cpu/mca: commonize mca_check_all_banks
by Felix Held (Code Review) 14 Jul '21

14 Jul '21

Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson. Hello build bot (Jenkins), Jason Glenesk, Raul Rangel, Marshall Dawson, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/56313 to look at the new patch set (#2). Change subject: soc/amd/common/block/cpu/mca: commonize mca_check_all_banks ...................................................................... soc/amd/common/block/cpu/mca: commonize mca_check_all_banks Since we don't need to skip the MCA check on cold boot on MCAX capable systems, add a mca_skip_check implementation that always returns false. Change-Id: Id8fc4b6f02b6c02b03172fe11f0451a9893e514d Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> --- M src/soc/amd/common/block/cpu/mca/mca.c M src/soc/amd/common/block/cpu/mca/mca_common.c M src/soc/amd/common/block/cpu/mca/mca_common_defs.h M src/soc/amd/common/block/cpu/mca/mcax.c 4 files changed, 43 insertions(+), 55 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/56313/2 -- To view, visit https://review.coreboot.org/c/coreboot/+/56313 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id8fc4b6f02b6c02b03172fe11f0451a9893e514d Gerrit-Change-Number: 56313 Gerrit-PatchSet: 2 Gerrit-Owner: 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-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: payloads/external/tianocore: Add build argument for 4G Decode
by Sean Rhodes (Code Review) 14 Jul '21

14 Jul '21

Attention is currently required from: Nico Huber, Martin Roth, Stefan Reinauer. Sean Rhodes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56223 ) Change subject: payloads/external/tianocore: Add build argument for 4G Decode ...................................................................... Patch Set 9: (1 comment) File payloads/external/tianocore/Kconfig: https://review.coreboot.org/c/coreboot/+/56223/comment/9fff58e7_1e5be7c5 PS7, Line 99: for Qubes (Xen). > Mapping would probably be a better way of putting it, but descriptions aren't my strong suit! […] final version: https://github.com/StarLabsLtd/edk2/commit/cec52e8872beded7dcc6cf35ce2c8c47… -- To view, visit https://review.coreboot.org/c/coreboot/+/56223 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia3b1d15196c0ec611431af29031682fea626d19d Gerrit-Change-Number: 56223 Gerrit-PatchSet: 9 Gerrit-Owner: Sean Rhodes <admin(a)starlabs.systems> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Andy Pont <andy.pont(a)sdcsystems.com> Gerrit-CC: Nico Huber <nico.h(a)gmx.de> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Comment-Date: Wed, 14 Jul 2021 17:18:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Sean Rhodes <admin(a)starlabs.systems> Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: src/device: Remove DEVICE_PATH_ESPI & DEVICE_PATH_LPC
by Felix Held (Code Review) 14 Jul '21

14 Jul '21

Attention is currently required from: Martin Roth. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56278 ) Change subject: src/device: Remove DEVICE_PATH_ESPI & DEVICE_PATH_LPC ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/56278 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9ae7817bb63d69ee272103b2d1186f125e188950 Gerrit-Change-Number: 56278 Gerrit-PatchSet: 1 Gerrit-Owner: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Comment-Date: Wed, 14 Jul 2021 17:16:22 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: util/sconfig: Remove unused devicetree keywords ESPI & LPC
by Felix Held (Code Review) 14 Jul '21

14 Jul '21

Attention is currently required from: Martin Roth. Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56197 ) Change subject: util/sconfig: Remove unused devicetree keywords ESPI & LPC ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/56197 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3a78afc55477d62eac8056e2ca4bcdd3ab12ea47 Gerrit-Change-Number: 56197 Gerrit-PatchSet: 2 Gerrit-Owner: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Comment-Date: Wed, 14 Jul 2021 17:16:08 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: soc/amd/common/block/cpu/mca/mcax: print all MCAX registers
by Felix Held (Code Review) 14 Jul '21

14 Jul '21

Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/56315 ) Change subject: soc/amd/common/block/cpu/mca/mcax: print all MCAX registers ...................................................................... soc/amd/common/block/cpu/mca/mcax: print all MCAX registers Also move the registers in the order they are in the hardware. Change-Id: If018e746e58c14475caeda76feb8b5281d7732f1 Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> --- M src/soc/amd/common/block/cpu/mca/mcax.c 1 file changed, 22 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/15/56315/1 diff --git a/src/soc/amd/common/block/cpu/mca/mcax.c b/src/soc/amd/common/block/cpu/mca/mcax.c index b8fa0c0..a4b5c59 100644 --- a/src/soc/amd/common/block/cpu/mca/mcax.c +++ b/src/soc/amd/common/block/cpu/mca/mcax.c @@ -14,6 +14,7 @@ return false; } +/* Print the contents of the MCAX registers for a given bank */ void mca_print_error(unsigned int bank) { msr_t msr; @@ -21,14 +22,32 @@ printk(BIOS_WARNING, "#MC Error: core %u, bank %u %s\n", initial_lapicid(), bank, mca_get_bank_name(bank)); + msr = rdmsr(MCAX_CTL_MSR(bank)); + printk(BIOS_WARNING, " MC%u_CTL = %08x_%08x\n", bank, msr.hi, msr.lo); msr = rdmsr(MCAX_STATUS_MSR(bank)); printk(BIOS_WARNING, " MC%u_STATUS = %08x_%08x\n", bank, msr.hi, msr.lo); msr = rdmsr(MCAX_ADDR_MSR(bank)); printk(BIOS_WARNING, " MC%u_ADDR = %08x_%08x\n", bank, msr.hi, msr.lo); msr = rdmsr(MCAX_MISC0_MSR(bank)); - printk(BIOS_WARNING, " MC%u_MISC = %08x_%08x\n", bank, msr.hi, msr.lo); - msr = rdmsr(MCAX_CTL_MSR(bank)); - printk(BIOS_WARNING, " MC%u_CTL = %08x_%08x\n", bank, msr.hi, msr.lo); + printk(BIOS_WARNING, " MC%u_MISC0 = %08x_%08x\n", bank, msr.hi, msr.lo); + msr = rdmsr(MCAX_CONFIG_MSR(bank)); + printk(BIOS_WARNING, " MC%u_CONFIG = %08x_%08x\n", bank, msr.hi, msr.lo); + msr = rdmsr(MCAX_IPID_MSR(bank)); + printk(BIOS_WARNING, " MC%u_IPID = %08x_%08x\n", bank, msr.hi, msr.lo); + msr = rdmsr(MCAX_SYND_MSR(bank)); + printk(BIOS_WARNING, " MC%u_SYND = %08x_%08x\n", bank, msr.hi, msr.lo); + msr = rdmsr(MCAX_DESTAT_MSR(bank)); + printk(BIOS_WARNING, " MC%u_DESTAT = %08x_%08x\n", bank, msr.hi, msr.lo); + msr = rdmsr(MCAX_DEADDR_MSR(bank)); + printk(BIOS_WARNING, " MC%u_DEADDR = %08x_%08x\n", bank, msr.hi, msr.lo); + msr = rdmsr(MCAX_MISC1_MSR(bank)); + printk(BIOS_WARNING, " MC%u_MISC1 = %08x_%08x\n", bank, msr.hi, msr.lo); + msr = rdmsr(MCAX_MISC2_MSR(bank)); + printk(BIOS_WARNING, " MC%u_MISC2 = %08x_%08x\n", bank, msr.hi, msr.lo); + msr = rdmsr(MCAX_MISC3_MSR(bank)); + printk(BIOS_WARNING, " MC%u_MISC3 = %08x_%08x\n", bank, msr.hi, msr.lo); + msr = rdmsr(MCAX_MISC4_MSR(bank)); + printk(BIOS_WARNING, " MC%u_MISC4 = %08x_%08x\n", bank, msr.hi, msr.lo); msr = rdmsr(MCA_CTL_MASK_MSR(bank)); printk(BIOS_WARNING, " MC%u_CTL_MASK = %08x_%08x\n", bank, msr.hi, msr.lo); } -- To view, visit https://review.coreboot.org/c/coreboot/+/56315 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: If018e746e58c14475caeda76feb8b5281d7732f1 Gerrit-Change-Number: 56315 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: soc/amd/common/block/cpu/mca: make building the BERT support conditional
by Felix Held (Code Review) 14 Jul '21

14 Jul '21

1 0

Change in coreboot[master]: soc/amd/common/block/cpu/mca: commonize mca_check_all_banks
by Felix Held (Code Review) 14 Jul '21

14 Jul '21

Attention is currently required from: Jason Glenesk, Raul Rangel, Marshall Dawson. Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/56313 ) Change subject: soc/amd/common/block/cpu/mca: commonize mca_check_all_banks ...................................................................... soc/amd/common/block/cpu/mca: commonize mca_check_all_banks Since we don't need to skip the MCA check on cold boot on MCAX capable systems, add a mca_skip_check implementation that always returns false. Change-Id: Id8fc4b6f02b6c02b03172fe11f0451a9893e514d Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> --- M src/soc/amd/common/block/cpu/mca/mca.c M src/soc/amd/common/block/cpu/mca/mca_common.c M src/soc/amd/common/block/cpu/mca/mca_common_defs.h M src/soc/amd/common/block/cpu/mca/mcax.c 4 files changed, 42 insertions(+), 55 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/56313/1 diff --git a/src/soc/amd/common/block/cpu/mca/mca.c b/src/soc/amd/common/block/cpu/mca/mca.c index 1c3a30c..48c0286 100644 --- a/src/soc/amd/common/block/cpu/mca/mca.c +++ b/src/soc/amd/common/block/cpu/mca/mca.c @@ -1,6 +1,5 @@ /* 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> @@ -9,12 +8,12 @@ #include <types.h> #include "mca_common_defs.h" -static bool mca_skip_check(void) +bool mca_skip_check(void) { return !is_warm_reset(); } -static void mca_print_error(unsigned int bank) +void mca_print_error(unsigned int bank) { msr_t msr; @@ -32,29 +31,3 @@ msr = rdmsr(MC_CTL_MASK(bank)); printk(BIOS_WARNING, " MC%u_CTL_MASK = %08x_%08x\n", bank, msr.hi, msr.lo); } - -void mca_check_all_banks(void) -{ - struct mca_bank_status mci; - const unsigned int num_banks = mca_get_bank_count(); - - if (!mca_has_expected_bank_count()) - printk(BIOS_WARNING, "CPU has an unexpected number of MCA banks!\n"); - - if (mca_skip_check()) - return; - - for (unsigned int i = 0 ; i < num_banks ; i++) { - if (!mca_is_valid_bank(i)) - continue; - - mci.bank = i; - mci.sts = rdmsr(IA32_MC_STATUS(i)); - if (mci.sts.hi || mci.sts.lo) { - mca_print_error(i); - - if (CONFIG(ACPI_BERT) && mca_valid(mci.sts)) - build_bert_mca_error(&mci); - } - } -} diff --git a/src/soc/amd/common/block/cpu/mca/mca_common.c b/src/soc/amd/common/block/cpu/mca/mca_common.c index f029966..b357691 100644 --- a/src/soc/amd/common/block/cpu/mca/mca_common.c +++ b/src/soc/amd/common/block/cpu/mca/mca_common.c @@ -2,8 +2,38 @@ #include <amdblocks/mca.h> #include <cpu/x86/msr.h> +#include <console/console.h> +#include <types.h> #include "mca_common_defs.h" +static void mca_check_all_banks(void) +{ + struct mca_bank_status mci; + const unsigned int num_banks = mca_get_bank_count(); + + if (!mca_has_expected_bank_count()) + printk(BIOS_WARNING, "CPU has an unexpected number of MCA banks!\n"); + + if (mca_skip_check()) + return; + + for (unsigned int i = 0 ; i < num_banks ; i++) { + if (!mca_is_valid_bank(i)) + continue; + + mci.bank = i; + /* The MCA status register can be used in both the MCA and MCAX case */ + mci.sts = rdmsr(IA32_MC_STATUS(i)); + if (mci.sts.hi || mci.sts.lo) { + mca_print_error(i); + + if (CONFIG(ACPI_BERT) && mca_valid(mci.sts)) + build_bert_mca_error(&mci); + } + } +} + + void check_mca(void) { mca_check_all_banks(); diff --git a/src/soc/amd/common/block/cpu/mca/mca_common_defs.h b/src/soc/amd/common/block/cpu/mca/mca_common_defs.h index 8142371..7d53e75 100644 --- a/src/soc/amd/common/block/cpu/mca/mca_common_defs.h +++ b/src/soc/amd/common/block/cpu/mca/mca_common_defs.h @@ -6,7 +6,8 @@ #include <amdblocks/mca.h> #include <cper.h> -void mca_check_all_banks(void); +bool mca_skip_check(void); +void mca_print_error(unsigned int bank); void build_bert_mca_error(struct mca_bank_status *mci); enum cper_x86_check_type error_to_chktype(struct mca_bank_status *mci); void fill_generic_section(cper_proc_generic_error_section_t *sec, struct mca_bank_status *mci); diff --git a/src/soc/amd/common/block/cpu/mca/mcax.c b/src/soc/amd/common/block/cpu/mca/mcax.c index fecac0a..b8fa0c0 100644 --- a/src/soc/amd/common/block/cpu/mca/mcax.c +++ b/src/soc/amd/common/block/cpu/mca/mcax.c @@ -1,6 +1,5 @@ /* SPDX-License-Identifier: GPL-2.0-only */ -#include <amdblocks/mca.h> #include <amdblocks/msr_zen.h> #include <cpu/x86/lapic.h> #include <cpu/x86/msr.h> @@ -8,7 +7,14 @@ #include <types.h> #include "mca_common_defs.h" -static void mca_print_error(unsigned int bank) +bool mca_skip_check(void) +{ + /* On Zen-based CPUs/APUs the MCA(X) status register have a defined state even in the + cold boot path, so no need to skip the check */ + return false; +} + +void mca_print_error(unsigned int bank) { msr_t msr; @@ -26,26 +32,3 @@ msr = rdmsr(MCA_CTL_MASK_MSR(bank)); printk(BIOS_WARNING, " MC%u_CTL_MASK = %08x_%08x\n", bank, msr.hi, msr.lo); } - -void mca_check_all_banks(void) -{ - struct mca_bank_status mci; - const unsigned int num_banks = mca_get_bank_count(); - - if (!mca_has_expected_bank_count()) - 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) { - mca_print_error(i); - - if (CONFIG(ACPI_BERT) && mca_valid(mci.sts)) - build_bert_mca_error(&mci); - } - } -} -- To view, visit https://review.coreboot.org/c/coreboot/+/56313 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id8fc4b6f02b6c02b03172fe11f0451a9893e514d Gerrit-Change-Number: 56313 Gerrit-PatchSet: 1 Gerrit-Owner: 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-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-MessageType: newchange

1 0

Change in coreboot[master]: soc/amd/common/block/cpu/mca/mca: factor out mca_skip_check
by Felix Held (Code Review) 14 Jul '21

14 Jul '21

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit July 2021