coreboot-gerrit July 2021

coreboot-gerrit@coreboot.org

1 participants
5824 discussions

Change in coreboot[master]: include/cpu/x86/msr: introduce IA32_MC_*(x) macros
by Felix Held (Code Review) 14 Jul '21

14 Jul '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/56243 ) Change subject: include/cpu/x86/msr: introduce IA32_MC_*(x) macros ...................................................................... include/cpu/x86/msr: introduce IA32_MC_*(x) macros When accessing the MCA MSRs, the MCA bank number gets multiplied by 4 and added to the IA32_MC0_* define to get the MSR number. Add a macro that already does this calculation to avoid open coding this repeatedly. Change-Id: I2de753b8c8ac8dcff5a94d5bba43aa13bbf94b99 Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/56243 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/include/cpu/x86/msr.h M src/security/intel/txt/common.c M src/soc/amd/stoneyridge/mca.c M src/soc/intel/common/block/cpu/cpulib.c 11 files changed, 22 insertions(+), 19 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 1bcbc34..76c8521 100644 --- a/src/cpu/amd/agesa/family14/model_14_init.c +++ b/src/cpu/amd/agesa/family14/model_14_init.c @@ -63,7 +63,7 @@ msr.lo = 0; msr.hi = 0; for (i = 0; i < num_banks; i++) - wrmsr(IA32_MC0_STATUS + (i * 4), msr); + wrmsr(IA32_MC_STATUS(i), msr); /* 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 c7fcb36..883bd59 100644 --- a/src/cpu/amd/agesa/family15tn/model_15_init.c +++ b/src/cpu/amd/agesa/family15tn/model_15_init.c @@ -62,7 +62,7 @@ msr.lo = 0; msr.hi = 0; for (i = 0; i < num_banks; i++) - wrmsr(IA32_MC0_STATUS + (i * 4), msr); + wrmsr(IA32_MC_STATUS(i), msr); /* 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 28c3e78..f945f80 100644 --- a/src/cpu/amd/agesa/family16kb/model_16_init.c +++ b/src/cpu/amd/agesa/family16kb/model_16_init.c @@ -60,7 +60,7 @@ msr.lo = 0; msr.hi = 0; for (i = 0; i < num_banks; i++) - wrmsr(IA32_MC0_STATUS + (i * 4), msr); + wrmsr(IA32_MC_STATUS(i), msr); /* 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 7266a2c..3c78c09 100644 --- a/src/cpu/amd/pi/00730F01/model_16_init.c +++ b/src/cpu/amd/pi/00730F01/model_16_init.c @@ -45,7 +45,7 @@ msr.lo = 0; msr.hi = 0; for (i = 0; i < num_banks; i++) - wrmsr(IA32_MC0_STATUS + (i * 4), msr); + wrmsr(IA32_MC_STATUS(i), msr); /* 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 9fcb527..2d8bd15 100644 --- a/src/cpu/intel/haswell/haswell_init.c +++ b/src/cpu/intel/haswell/haswell_init.c @@ -528,14 +528,14 @@ /* Enable all error reporting */ msr.lo = msr.hi = ~0; for (i = 0; i < num_banks; i++) - wrmsr(IA32_MC0_CTL + (i * 4), msr); + 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_MC0_STATUS + (i * 4), msr); + wrmsr(IA32_MC_STATUS(i), msr); } /* 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 dd2aeef..abf8f61 100644 --- a/src/cpu/intel/model_2065x/model_2065x_init.c +++ b/src/cpu/intel/model_2065x/model_2065x_init.c @@ -81,7 +81,7 @@ msr.lo = msr.hi = 0; /* This should only be done on a cold boot */ for (i = 0; i < 7; i++) - wrmsr(IA32_MC0_STATUS + (i * 4), msr); + wrmsr(IA32_MC_STATUS(i), msr); } static void model_2065x_init(struct device *cpu) diff --git a/src/cpu/intel/model_206ax/model_206ax_init.c b/src/cpu/intel/model_206ax/model_206ax_init.c index 7a40644..bfe1fa5 100644 --- a/src/cpu/intel/model_206ax/model_206ax_init.c +++ b/src/cpu/intel/model_206ax/model_206ax_init.c @@ -309,7 +309,7 @@ msr.lo = msr.hi = 0; /* This should only be done on a cold boot */ for (i = 0; i < num_banks; i++) - wrmsr(IA32_MC0_STATUS + (i * 4), msr); + wrmsr(IA32_MC_STATUS(i), msr); } static void model_206ax_report(void) diff --git a/src/include/cpu/x86/msr.h b/src/include/cpu/x86/msr.h index da1932c..dd969d7 100644 --- a/src/include/cpu/x86/msr.h +++ b/src/include/cpu/x86/msr.h @@ -56,7 +56,9 @@ #define DCA_TYPE0_EN (1 << 0) #define IA32_PAT 0x277 #define IA32_MC0_CTL 0x400 +#define IA32_MC_CTL(bank) (IA32_MC0_CTL + 4 * (bank)) #define IA32_MC0_STATUS 0x401 +#define IA32_MC_STATUS(bank) (IA32_MC0_STATUS + 4 * (bank)) #define MCA_STATUS_HI_VAL (1UL << (63 - 32)) #define MCA_STATUS_HI_OVERFLOW (1UL << (62 - 32)) #define MCA_STATUS_HI_UC (1UL << (61 - 32)) @@ -75,7 +77,9 @@ #define MCA_STATUS_LO_ERRCODE_EXT_MASK (0x3f << MCA_STATUS_LO_ERRCODE_EXT_SH) #define MCA_STATUS_LO_ERRCODE_MASK (0xffff << 0) #define IA32_MC0_ADDR 0x402 +#define IA32_MC_ADDR(bank) (IA32_MC0_ADDR + 4 * (bank)) #define IA32_MC0_MISC 0x403 +#define IA32_MC_MISC(bank) (IA32_MC0_MISC + 4 * (bank)) #define IA32_VMX_BASIC_MSR 0x480 #define VMX_BASIC_HI_DUAL_MONITOR (1UL << (49 - 32)) #define IA32_VMX_MISC_MSR 0x485 diff --git a/src/security/intel/txt/common.c b/src/security/intel/txt/common.c index 011e8cc..2b7d926 100644 --- a/src/security/intel/txt/common.c +++ b/src/security/intel/txt/common.c @@ -483,7 +483,7 @@ */ size_t max_mc_msr = mca_get_bank_count(); for (size_t i = 0; i < max_mc_msr; i++) { - msr = rdmsr(IA32_MC0_STATUS + 4 * i); + msr = rdmsr(IA32_MC_STATUS(i)); if (!(msr.hi & MCA_STATUS_HI_UC)) continue; diff --git a/src/soc/amd/stoneyridge/mca.c b/src/soc/amd/stoneyridge/mca.c index 920ee7d..0773101 100644 --- a/src/soc/amd/stoneyridge/mca.c +++ b/src/soc/amd/stoneyridge/mca.c @@ -119,8 +119,7 @@ ctx = cper_new_ia32x64_context_msr(status, x86_sec, IA32_MCG_CAP, 3); if (!ctx) goto failed; - ctx = cper_new_ia32x64_context_msr(status, x86_sec, - IA32_MC0_CTL + (mci->bank * 4), 4); + ctx = cper_new_ia32x64_context_msr(status, x86_sec, IA32_MC_CTL(mci->bank), 4); if (!ctx) goto failed; ctx = cper_new_ia32x64_context_msr(status, x86_sec, @@ -152,13 +151,13 @@ printk(BIOS_WARNING, "#MC Error: core %u, bank %u %s\n", initial_lapicid(), bank, mca_bank_name[bank]); - msr = rdmsr(IA32_MC0_STATUS + (bank * 4)); + msr = rdmsr(IA32_MC_STATUS(bank)); printk(BIOS_WARNING, " MC%u_STATUS = %08x_%08x\n", bank, msr.hi, msr.lo); - msr = rdmsr(IA32_MC0_ADDR + (bank * 4)); + msr = rdmsr(IA32_MC_ADDR(bank)); printk(BIOS_WARNING, " MC%u_ADDR = %08x_%08x\n", bank, msr.hi, msr.lo); - msr = rdmsr(IA32_MC0_MISC + (bank * 4)); + msr = rdmsr(IA32_MC_MISC(bank)); printk(BIOS_WARNING, " MC%u_MISC = %08x_%08x\n", bank, msr.hi, msr.lo); - msr = rdmsr(IA32_MC0_CTL + (bank * 4)); + msr = rdmsr(IA32_MC_CTL(bank)); printk(BIOS_WARNING, " MC%u_CTL = %08x_%08x\n", bank, msr.hi, msr.lo); msr = rdmsr(MC0_CTL_MASK + bank); printk(BIOS_WARNING, " MC%u_CTL_MASK = %08x_%08x\n", bank, msr.hi, msr.lo); @@ -177,7 +176,7 @@ continue; mci.bank = i; - mci.sts = rdmsr(IA32_MC0_STATUS + (i * 4)); + mci.sts = rdmsr(IA32_MC_STATUS(i)); if (mci.sts.hi || mci.sts.lo) { mca_print_error(i); @@ -194,7 +193,7 @@ /* Zero all machine check error status registers */ for (unsigned int i = 0 ; i < num_banks ; i++) - wrmsr(IA32_MC0_STATUS + (i * 4), msr); + wrmsr(IA32_MC_STATUS(i), msr); } void check_mca(void) diff --git a/src/soc/intel/common/block/cpu/cpulib.c b/src/soc/intel/common/block/cpu/cpulib.c index 81e42ae..9e95b0f 100644 --- a/src/soc/intel/common/block/cpu/cpulib.c +++ b/src/soc/intel/common/block/cpu/cpulib.c @@ -348,9 +348,9 @@ for (i = 0; i < num_banks; i++) { /* Clear the machine check status */ - wrmsr(IA32_MC0_STATUS + (i * 4), msr); + wrmsr(IA32_MC_STATUS(i), msr); /* Initialize machine checks */ - wrmsr(IA32_MC0_CTL + i * 4, + wrmsr(IA32_MC_CTL(i), (msr_t) {.lo = 0xffffffff, .hi = 0xffffffff}); } } -- To view, visit https://review.coreboot.org/c/coreboot/+/56243 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2de753b8c8ac8dcff5a94d5bba43aa13bbf94b99 Gerrit-Change-Number: 56243 Gerrit-PatchSet: 3 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-MessageType: merged

1 0

Change in coreboot[master]: soc/amd/stoneyridge/mca: refactor warm boot check in mca_check_all_banks
by Felix Held (Code Review) 14 Jul '21

14 Jul '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/56242 ) Change subject: soc/amd/stoneyridge/mca: refactor warm boot check in mca_check_all_banks ...................................................................... soc/amd/stoneyridge/mca: refactor warm boot check in mca_check_all_banks Change-Id: Id0cf8269d1b695e05c55f33af92978b8244090fa Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/56242 Reviewed-by: Raul Rangel <rrangel(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/amd/stoneyridge/mca.c 1 file changed, 12 insertions(+), 11 deletions(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved diff --git a/src/soc/amd/stoneyridge/mca.c b/src/soc/amd/stoneyridge/mca.c index 9ba201e..920ee7d 100644 --- a/src/soc/amd/stoneyridge/mca.c +++ b/src/soc/amd/stoneyridge/mca.c @@ -169,19 +169,20 @@ struct mca_bank_status mci; const unsigned int num_banks = mca_get_bank_count(); - if (is_warm_reset()) { - for (unsigned int i = 0 ; i < num_banks ; i++) { - if (i == 3) /* Reserved in Family 15h */ - continue; + if (!is_warm_reset()) + return; - mci.bank = i; - mci.sts = rdmsr(IA32_MC0_STATUS + (i * 4)); - if (mci.sts.hi || mci.sts.lo) { - mca_print_error(i); + for (unsigned int i = 0 ; i < num_banks ; i++) { + if (i == 3) /* Reserved in Family 15h */ + continue; - if (CONFIG(ACPI_BERT) && mca_valid(mci.sts)) - build_bert_mca_error(&mci); - } + mci.bank = i; + mci.sts = rdmsr(IA32_MC0_STATUS + (i * 4)); + 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/+/56242 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id0cf8269d1b695e05c55f33af92978b8244090fa Gerrit-Change-Number: 56242 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-MessageType: merged

1 0

Change in coreboot[master]: soc/amd/picasso,stoneyridge/mca: factor out mca_check_all_banks
by Felix Held (Code Review) 14 Jul '21

14 Jul '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/56241 ) Change subject: soc/amd/picasso,stoneyridge/mca: factor out mca_check_all_banks ...................................................................... soc/amd/picasso,stoneyridge/mca: factor out mca_check_all_banks Change-Id: I5496fd27f5c56d35ab95a5e02ea313b5b5536668 Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/56241 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 M src/soc/amd/stoneyridge/mca.c 2 files changed, 34 insertions(+), 28 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 7561a56..973b3b5 100644 --- a/src/soc/amd/picasso/mca.c +++ b/src/soc/amd/picasso/mca.c @@ -164,6 +164,23 @@ printk(BIOS_WARNING, " MC%u_CTL_MASK = %08x_%08x\n", bank, msr.hi, msr.lo); } +static void mca_check_all_banks(void) +{ + struct mca_bank_status mci; + const unsigned int num_banks = mca_get_bank_count(); + + for (unsigned int i = 0 ; i < num_banks ; i++) { + 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); + } + } +} + static void mca_clear_errors(void) { const unsigned int num_banks = mca_get_bank_count(); @@ -177,20 +194,6 @@ /* Check the Machine Check Architecture Extension registers */ void check_mca(void) { - unsigned int i; - struct mca_bank_status mci; - const unsigned int num_banks = mca_get_bank_count(); - - for (i = 0 ; i < num_banks ; i++) { - 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); - } - } - + mca_check_all_banks(); mca_clear_errors(); } diff --git a/src/soc/amd/stoneyridge/mca.c b/src/soc/amd/stoneyridge/mca.c index a64b01d..9ba201e 100644 --- a/src/soc/amd/stoneyridge/mca.c +++ b/src/soc/amd/stoneyridge/mca.c @@ -164,24 +164,13 @@ printk(BIOS_WARNING, " MC%u_CTL_MASK = %08x_%08x\n", bank, msr.hi, msr.lo); } -static void mca_clear_errors(void) +static void mca_check_all_banks(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_MC0_STATUS + (i * 4), msr); -} - -void check_mca(void) -{ - unsigned int i; struct mca_bank_status mci; const unsigned int num_banks = mca_get_bank_count(); if (is_warm_reset()) { - for (i = 0 ; i < num_banks ; i++) { + for (unsigned int i = 0 ; i < num_banks ; i++) { if (i == 3) /* Reserved in Family 15h */ continue; @@ -195,6 +184,20 @@ } } } +} +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_MC0_STATUS + (i * 4), msr); +} + +void check_mca(void) +{ + mca_check_all_banks(); mca_clear_errors(); } -- To view, visit https://review.coreboot.org/c/coreboot/+/56241 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I5496fd27f5c56d35ab95a5e02ea313b5b5536668 Gerrit-Change-Number: 56241 Gerrit-PatchSet: 3 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-MessageType: merged

1 0

Change in coreboot[master]: soc/amd/picasso,stoneyridge/mca: factor out mca_clear_errors
by Felix Held (Code Review) 14 Jul '21

14 Jul '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/56240 ) Change subject: soc/amd/picasso,stoneyridge/mca: factor out mca_clear_errors ...................................................................... soc/amd/picasso,stoneyridge/mca: factor out mca_clear_errors Change-Id: Id7a716a2598a6a7bea2d2d56898ea6329b5a3bec Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/56240 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 M src/soc/amd/stoneyridge/mca.c 2 files changed, 22 insertions(+), 12 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 e525d03..7561a56 100644 --- a/src/soc/amd/picasso/mca.c +++ b/src/soc/amd/picasso/mca.c @@ -164,12 +164,21 @@ printk(BIOS_WARNING, " MC%u_CTL_MASK = %08x_%08x\n", bank, msr.hi, msr.lo); } +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(MCAX_STATUS_MSR(i), msr); +} + /* Check the Machine Check Architecture Extension registers */ void check_mca(void) { unsigned int i; struct mca_bank_status mci; - msr_t msr; const unsigned int num_banks = mca_get_bank_count(); for (i = 0 ; i < num_banks ; i++) { @@ -183,9 +192,5 @@ } } - /* zero the machine check error status registers */ - msr.lo = 0; - msr.hi = 0; - for (i = 0 ; i < num_banks ; i++) - wrmsr(MCAX_STATUS_MSR(i), msr); + mca_clear_errors(); } diff --git a/src/soc/amd/stoneyridge/mca.c b/src/soc/amd/stoneyridge/mca.c index 06b35bb..a64b01d 100644 --- a/src/soc/amd/stoneyridge/mca.c +++ b/src/soc/amd/stoneyridge/mca.c @@ -164,11 +164,20 @@ printk(BIOS_WARNING, " MC%u_CTL_MASK = %08x_%08x\n", bank, msr.hi, msr.lo); } +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_MC0_STATUS + (i * 4), msr); +} + void check_mca(void) { unsigned int i; struct mca_bank_status mci; - msr_t msr; const unsigned int num_banks = mca_get_bank_count(); if (is_warm_reset()) { @@ -187,9 +196,5 @@ } } - /* zero the machine check error status registers */ - msr.lo = 0; - msr.hi = 0; - for (i = 0 ; i < num_banks ; i++) - wrmsr(IA32_MC0_STATUS + (i * 4), msr); + mca_clear_errors(); } -- To view, visit https://review.coreboot.org/c/coreboot/+/56240 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id7a716a2598a6a7bea2d2d56898ea6329b5a3bec Gerrit-Change-Number: 56240 Gerrit-PatchSet: 3 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-MessageType: merged

1 0

Change in coreboot[master]: soc/amd/picasso,stoneyridge/mca: factor out mca_print_error()
by Felix Held (Code Review) 14 Jul '21

14 Jul '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/56239 ) Change subject: soc/amd/picasso,stoneyridge/mca: factor out mca_print_error() ...................................................................... soc/amd/picasso,stoneyridge/mca: factor out mca_print_error() Change-Id: I7cd05a389c34c2e5f3d0ab4cd06d60a7e3e5cad9 Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/56239 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 M src/soc/amd/stoneyridge/mca.c 2 files changed, 42 insertions(+), 37 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 07e700e..e525d03 100644 --- a/src/soc/amd/picasso/mca.c +++ b/src/soc/amd/picasso/mca.c @@ -145,6 +145,25 @@ "L3 cache unit" }; +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] : ""); + + 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); + msr = rdmsr(MCA_CTL_MASK_MSR(bank)); + printk(BIOS_WARNING, " MC%u_CTL_MASK = %08x_%08x\n", bank, msr.hi, msr.lo); +} + /* Check the Machine Check Architecture Extension registers */ void check_mca(void) { @@ -154,28 +173,11 @@ const unsigned int num_banks = mca_get_bank_count(); for (i = 0 ; i < num_banks ; i++) { + mci.bank = i; mci.sts = rdmsr(MCAX_STATUS_MSR(i)); if (mci.sts.hi || mci.sts.lo) { - printk(BIOS_WARNING, "#MC Error: core %u, bank %u %s\n", - initial_lapicid(), i, - i < ARRAY_SIZE(mca_bank_name) ? mca_bank_name[i] : ""); + mca_print_error(i); - printk(BIOS_WARNING, " MC%u_STATUS = %08x_%08x\n", - i, mci.sts.hi, mci.sts.lo); - msr = rdmsr(MCAX_ADDR_MSR(i)); - printk(BIOS_WARNING, " MC%u_ADDR = %08x_%08x\n", - i, msr.hi, msr.lo); - msr = rdmsr(MCAX_MISC0_MSR(i)); - printk(BIOS_WARNING, " MC%u_MISC = %08x_%08x\n", - i, msr.hi, msr.lo); - msr = rdmsr(MCAX_CTL_MSR(i)); - printk(BIOS_WARNING, " MC%u_CTL = %08x_%08x\n", - i, msr.hi, msr.lo); - msr = rdmsr(MCA_CTL_MASK_MSR(i)); - printk(BIOS_WARNING, " MC%u_CTL_MASK = %08x_%08x\n", - i, msr.hi, msr.lo); - - mci.bank = i; if (CONFIG(ACPI_BERT) && mca_valid(mci.sts)) build_bert_mca_error(&mci); } diff --git a/src/soc/amd/stoneyridge/mca.c b/src/soc/amd/stoneyridge/mca.c index 9425089..06b35bb 100644 --- a/src/soc/amd/stoneyridge/mca.c +++ b/src/soc/amd/stoneyridge/mca.c @@ -145,6 +145,25 @@ "Floating point unit" }; +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]); + + msr = rdmsr(IA32_MC0_STATUS + (bank * 4)); + printk(BIOS_WARNING, " MC%u_STATUS = %08x_%08x\n", bank, msr.hi, msr.lo); + msr = rdmsr(IA32_MC0_ADDR + (bank * 4)); + printk(BIOS_WARNING, " MC%u_ADDR = %08x_%08x\n", bank, msr.hi, msr.lo); + msr = rdmsr(IA32_MC0_MISC + (bank * 4)); + printk(BIOS_WARNING, " MC%u_MISC = %08x_%08x\n", bank, msr.hi, msr.lo); + msr = rdmsr(IA32_MC0_CTL + (bank * 4)); + printk(BIOS_WARNING, " MC%u_CTL = %08x_%08x\n", bank, msr.hi, msr.lo); + msr = rdmsr(MC0_CTL_MASK + bank); + printk(BIOS_WARNING, " MC%u_CTL_MASK = %08x_%08x\n", bank, msr.hi, msr.lo); +} + void check_mca(void) { unsigned int i; @@ -157,27 +176,11 @@ if (i == 3) /* Reserved in Family 15h */ continue; + mci.bank = i; mci.sts = rdmsr(IA32_MC0_STATUS + (i * 4)); if (mci.sts.hi || mci.sts.lo) { - printk(BIOS_WARNING, "#MC Error: core %u, bank %u %s\n", - initial_lapicid(), i, mca_bank_name[i]); + mca_print_error(i); - printk(BIOS_WARNING, " MC%u_STATUS = %08x_%08x\n", - i, mci.sts.hi, mci.sts.lo); - msr = rdmsr(IA32_MC0_ADDR + (i * 4)); - printk(BIOS_WARNING, " MC%u_ADDR = %08x_%08x\n", - i, msr.hi, msr.lo); - msr = rdmsr(IA32_MC0_MISC + (i * 4)); - printk(BIOS_WARNING, " MC%u_MISC = %08x_%08x\n", - i, msr.hi, msr.lo); - msr = rdmsr(IA32_MC0_CTL + (i * 4)); - printk(BIOS_WARNING, " MC%u_CTL = %08x_%08x\n", - i, msr.hi, msr.lo); - msr = rdmsr(MC0_CTL_MASK + i); - printk(BIOS_WARNING, " MC%u_CTL_MASK = %08x_%08x\n", - i, msr.hi, msr.lo); - - mci.bank = i; if (CONFIG(ACPI_BERT) && mca_valid(mci.sts)) build_bert_mca_error(&mci); } -- To view, visit https://review.coreboot.org/c/coreboot/+/56239 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7cd05a389c34c2e5f3d0ab4cd06d60a7e3e5cad9 Gerrit-Change-Number: 56239 Gerrit-PatchSet: 3 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-MessageType: merged

1 0

Change in coreboot[master]: soc/amd/picasso,stoneyridge/mca: use unsigned int for MCA bank number
by Felix Held (Code Review) 14 Jul '21

14 Jul '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/56238 ) Change subject: soc/amd/picasso,stoneyridge/mca: use unsigned int for MCA bank number ...................................................................... soc/amd/picasso,stoneyridge/mca: use unsigned int for MCA bank number Change-Id: Ib31075fd615eaa8492ce0179b3b21317554f1c80 Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/56238 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 M src/soc/amd/stoneyridge/mca.c 2 files changed, 16 insertions(+), 16 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 84f3ead..07e700e 100644 --- a/src/soc/amd/picasso/mca.c +++ b/src/soc/amd/picasso/mca.c @@ -14,7 +14,7 @@ #define MCAX_USED_REGISTERS_PER_BANK (MCAX_MISC4_OFFSET + 1) struct mca_bank_status { - int bank; + unsigned int bank; msr_t sts; }; @@ -148,7 +148,7 @@ /* Check the Machine Check Architecture Extension registers */ void check_mca(void) { - int i; + unsigned int i; struct mca_bank_status mci; msr_t msr; const unsigned int num_banks = mca_get_bank_count(); @@ -156,23 +156,23 @@ for (i = 0 ; i < num_banks ; i++) { mci.sts = rdmsr(MCAX_STATUS_MSR(i)); if (mci.sts.hi || mci.sts.lo) { - printk(BIOS_WARNING, "#MC Error: core %u, bank %d %s\n", + printk(BIOS_WARNING, "#MC Error: core %u, bank %u %s\n", initial_lapicid(), i, i < ARRAY_SIZE(mca_bank_name) ? mca_bank_name[i] : ""); - printk(BIOS_WARNING, " MC%d_STATUS = %08x_%08x\n", + printk(BIOS_WARNING, " MC%u_STATUS = %08x_%08x\n", i, mci.sts.hi, mci.sts.lo); msr = rdmsr(MCAX_ADDR_MSR(i)); - printk(BIOS_WARNING, " MC%d_ADDR = %08x_%08x\n", + printk(BIOS_WARNING, " MC%u_ADDR = %08x_%08x\n", i, msr.hi, msr.lo); msr = rdmsr(MCAX_MISC0_MSR(i)); - printk(BIOS_WARNING, " MC%d_MISC = %08x_%08x\n", + printk(BIOS_WARNING, " MC%u_MISC = %08x_%08x\n", i, msr.hi, msr.lo); msr = rdmsr(MCAX_CTL_MSR(i)); - printk(BIOS_WARNING, " MC%d_CTL = %08x_%08x\n", + printk(BIOS_WARNING, " MC%u_CTL = %08x_%08x\n", i, msr.hi, msr.lo); msr = rdmsr(MCA_CTL_MASK_MSR(i)); - printk(BIOS_WARNING, " MC%d_CTL_MASK = %08x_%08x\n", + printk(BIOS_WARNING, " MC%u_CTL_MASK = %08x_%08x\n", i, msr.hi, msr.lo); mci.bank = i; diff --git a/src/soc/amd/stoneyridge/mca.c b/src/soc/amd/stoneyridge/mca.c index 4c9d9b1..9425089 100644 --- a/src/soc/amd/stoneyridge/mca.c +++ b/src/soc/amd/stoneyridge/mca.c @@ -10,7 +10,7 @@ #include <cper.h> struct mca_bank_status { - int bank; + unsigned int bank; msr_t sts; }; @@ -147,7 +147,7 @@ void check_mca(void) { - int i; + unsigned int i; struct mca_bank_status mci; msr_t msr; const unsigned int num_banks = mca_get_bank_count(); @@ -159,22 +159,22 @@ mci.sts = rdmsr(IA32_MC0_STATUS + (i * 4)); if (mci.sts.hi || mci.sts.lo) { - printk(BIOS_WARNING, "#MC Error: core %u, bank %d %s\n", + printk(BIOS_WARNING, "#MC Error: core %u, bank %u %s\n", initial_lapicid(), i, mca_bank_name[i]); - printk(BIOS_WARNING, " MC%d_STATUS = %08x_%08x\n", + printk(BIOS_WARNING, " MC%u_STATUS = %08x_%08x\n", i, mci.sts.hi, mci.sts.lo); msr = rdmsr(IA32_MC0_ADDR + (i * 4)); - printk(BIOS_WARNING, " MC%d_ADDR = %08x_%08x\n", + printk(BIOS_WARNING, " MC%u_ADDR = %08x_%08x\n", i, msr.hi, msr.lo); msr = rdmsr(IA32_MC0_MISC + (i * 4)); - printk(BIOS_WARNING, " MC%d_MISC = %08x_%08x\n", + printk(BIOS_WARNING, " MC%u_MISC = %08x_%08x\n", i, msr.hi, msr.lo); msr = rdmsr(IA32_MC0_CTL + (i * 4)); - printk(BIOS_WARNING, " MC%d_CTL = %08x_%08x\n", + printk(BIOS_WARNING, " MC%u_CTL = %08x_%08x\n", i, msr.hi, msr.lo); msr = rdmsr(MC0_CTL_MASK + i); - printk(BIOS_WARNING, " MC%d_CTL_MASK = %08x_%08x\n", + printk(BIOS_WARNING, " MC%u_CTL_MASK = %08x_%08x\n", i, msr.hi, msr.lo); mci.bank = i; -- To view, visit https://review.coreboot.org/c/coreboot/+/56238 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib31075fd615eaa8492ce0179b3b21317554f1c80 Gerrit-Change-Number: 56238 Gerrit-PatchSet: 3 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-MessageType: merged

1 0

Change in coreboot[master]: soc/amd/picasso,stoneyridge/mca: clean up mca_bank struct
by Felix Held (Code Review) 14 Jul '21

14 Jul '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/56237 ) Change subject: soc/amd/picasso,stoneyridge/mca: clean up mca_bank struct ...................................................................... soc/amd/picasso,stoneyridge/mca: clean up mca_bank struct Only the fields bank and sts from the mca_bank struct were used outside a local scope, so remove the rest. Also rename the struct that now only contains the bank number and the status MSR content to mca_bank_status. Change-Id: I925347dff950ac2bd021635ca988c02fba48df7f Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/56237 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 M src/soc/amd/stoneyridge/mca.c 2 files changed, 34 insertions(+), 40 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 3c5dd10..84f3ead 100644 --- a/src/soc/amd/picasso/mca.c +++ b/src/soc/amd/picasso/mca.c @@ -13,13 +13,9 @@ /* MISC4 is the last used register in the MCAX banks of Picasso */ #define MCAX_USED_REGISTERS_PER_BANK (MCAX_MISC4_OFFSET + 1) -struct mca_bank { +struct mca_bank_status { int bank; - msr_t ctl; msr_t sts; - msr_t addr; - msr_t misc; - msr_t cmask; }; static inline size_t mca_report_size_reqd(void) @@ -50,7 +46,7 @@ return size; } -static enum cper_x86_check_type error_to_chktype(struct mca_bank *mci) +static enum cper_x86_check_type error_to_chktype(struct mca_bank_status *mci) { int error = mca_err_type(mci->sts); @@ -68,7 +64,7 @@ /* Fill additional information in the Generic Processor Error Section. */ static void fill_generic_section(cper_proc_generic_error_section_t *sec, - struct mca_bank *mci) + struct mca_bank_status *mci) { int type = mca_err_type(mci->sts); @@ -91,7 +87,7 @@ * structures: A "processor generic error" that is parsed, and an IA32/X64 one * to capture complete information. */ -static void build_bert_mca_error(struct mca_bank *mci) +static void build_bert_mca_error(struct mca_bank_status *mci) { acpi_generic_error_status_t *status; acpi_hest_generic_data_v300_t *gen_entry; @@ -153,7 +149,8 @@ void check_mca(void) { int i; - struct mca_bank mci; + struct mca_bank_status mci; + msr_t msr; const unsigned int num_banks = mca_get_bank_count(); for (i = 0 ; i < num_banks ; i++) { @@ -165,18 +162,18 @@ printk(BIOS_WARNING, " MC%d_STATUS = %08x_%08x\n", i, mci.sts.hi, mci.sts.lo); - mci.addr = rdmsr(MCAX_ADDR_MSR(i)); + msr = rdmsr(MCAX_ADDR_MSR(i)); printk(BIOS_WARNING, " MC%d_ADDR = %08x_%08x\n", - i, mci.addr.hi, mci.addr.lo); - mci.misc = rdmsr(MCAX_MISC0_MSR(i)); + i, msr.hi, msr.lo); + msr = rdmsr(MCAX_MISC0_MSR(i)); printk(BIOS_WARNING, " MC%d_MISC = %08x_%08x\n", - i, mci.misc.hi, mci.misc.lo); - mci.ctl = rdmsr(MCAX_CTL_MSR(i)); + i, msr.hi, msr.lo); + msr = rdmsr(MCAX_CTL_MSR(i)); printk(BIOS_WARNING, " MC%d_CTL = %08x_%08x\n", - i, mci.ctl.hi, mci.ctl.lo); - mci.cmask = rdmsr(MCA_CTL_MASK_MSR(i)); + i, msr.hi, msr.lo); + msr = rdmsr(MCA_CTL_MASK_MSR(i)); printk(BIOS_WARNING, " MC%d_CTL_MASK = %08x_%08x\n", - i, mci.cmask.hi, mci.cmask.lo); + i, msr.hi, msr.lo); mci.bank = i; if (CONFIG(ACPI_BERT) && mca_valid(mci.sts)) @@ -185,8 +182,8 @@ } /* zero the machine check error status registers */ - mci.sts.lo = 0; - mci.sts.hi = 0; + msr.lo = 0; + msr.hi = 0; for (i = 0 ; i < num_banks ; i++) - wrmsr(MCAX_STATUS_MSR(i), mci.sts); + wrmsr(MCAX_STATUS_MSR(i), msr); } diff --git a/src/soc/amd/stoneyridge/mca.c b/src/soc/amd/stoneyridge/mca.c index 6432ff4..4c9d9b1 100644 --- a/src/soc/amd/stoneyridge/mca.c +++ b/src/soc/amd/stoneyridge/mca.c @@ -9,13 +9,9 @@ #include <arch/bert_storage.h> #include <cper.h> -struct mca_bank { +struct mca_bank_status { int bank; - msr_t ctl; msr_t sts; - msr_t addr; - msr_t misc; - msr_t cmask; }; static inline size_t mca_report_size_reqd(void) @@ -45,7 +41,7 @@ return size; } -static enum cper_x86_check_type error_to_chktype(struct mca_bank *mci) +static enum cper_x86_check_type error_to_chktype(struct mca_bank_status *mci) { int error = mca_err_type(mci->sts); @@ -63,7 +59,7 @@ /* Fill additional information in the Generic Processor Error Section. */ static void fill_generic_section(cper_proc_generic_error_section_t *sec, - struct mca_bank *mci) + struct mca_bank_status *mci) { int type = mca_err_type(mci->sts); @@ -91,7 +87,7 @@ * Processor Generic section and the failing error/check added to the * IA32/X64 section. */ -static void build_bert_mca_error(struct mca_bank *mci) +static void build_bert_mca_error(struct mca_bank_status *mci) { acpi_generic_error_status_t *status; acpi_hest_generic_data_v300_t *gen_entry; @@ -152,7 +148,8 @@ void check_mca(void) { int i; - struct mca_bank mci; + struct mca_bank_status mci; + msr_t msr; const unsigned int num_banks = mca_get_bank_count(); if (is_warm_reset()) { @@ -167,18 +164,18 @@ printk(BIOS_WARNING, " MC%d_STATUS = %08x_%08x\n", i, mci.sts.hi, mci.sts.lo); - mci.addr = rdmsr(IA32_MC0_ADDR + (i * 4)); + msr = rdmsr(IA32_MC0_ADDR + (i * 4)); printk(BIOS_WARNING, " MC%d_ADDR = %08x_%08x\n", - i, mci.addr.hi, mci.addr.lo); - mci.misc = rdmsr(IA32_MC0_MISC + (i * 4)); + i, msr.hi, msr.lo); + msr = rdmsr(IA32_MC0_MISC + (i * 4)); printk(BIOS_WARNING, " MC%d_MISC = %08x_%08x\n", - i, mci.misc.hi, mci.misc.lo); - mci.ctl = rdmsr(IA32_MC0_CTL + (i * 4)); + i, msr.hi, msr.lo); + msr = rdmsr(IA32_MC0_CTL + (i * 4)); printk(BIOS_WARNING, " MC%d_CTL = %08x_%08x\n", - i, mci.ctl.hi, mci.ctl.lo); - mci.cmask = rdmsr(MC0_CTL_MASK + i); + i, msr.hi, msr.lo); + msr = rdmsr(MC0_CTL_MASK + i); printk(BIOS_WARNING, " MC%d_CTL_MASK = %08x_%08x\n", - i, mci.cmask.hi, mci.cmask.lo); + i, msr.hi, msr.lo); mci.bank = i; if (CONFIG(ACPI_BERT) && mca_valid(mci.sts)) @@ -188,8 +185,8 @@ } /* zero the machine check error status registers */ - mci.sts.lo = 0; - mci.sts.hi = 0; + msr.lo = 0; + msr.hi = 0; for (i = 0 ; i < num_banks ; i++) - wrmsr(IA32_MC0_STATUS + (i * 4), mci.sts); + wrmsr(IA32_MC0_STATUS + (i * 4), msr); } -- To view, visit https://review.coreboot.org/c/coreboot/+/56237 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I925347dff950ac2bd021635ca988c02fba48df7f Gerrit-Change-Number: 56237 Gerrit-PatchSet: 3 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-MessageType: merged

1 0

Change in coreboot[master]: soc/amd/picasso,stoneyridge/mca: mark num_banks as constant
by Felix Held (Code Review) 14 Jul '21

14 Jul '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/56236 ) Change subject: soc/amd/picasso,stoneyridge/mca: mark num_banks as constant ...................................................................... soc/amd/picasso,stoneyridge/mca: mark num_banks as constant Change-Id: I23aa4d36d4e6d4c7ed66800c2e7963c4ed03c393 Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/56236 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 M src/soc/amd/stoneyridge/mca.c 2 files changed, 2 insertions(+), 6 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 65b1916..3c5dd10 100644 --- a/src/soc/amd/picasso/mca.c +++ b/src/soc/amd/picasso/mca.c @@ -154,9 +154,7 @@ { int i; struct mca_bank mci; - unsigned int num_banks; - - num_banks = mca_get_bank_count(); + const unsigned int num_banks = mca_get_bank_count(); for (i = 0 ; i < num_banks ; i++) { mci.sts = rdmsr(MCAX_STATUS_MSR(i)); diff --git a/src/soc/amd/stoneyridge/mca.c b/src/soc/amd/stoneyridge/mca.c index c2ec402..6432ff4 100644 --- a/src/soc/amd/stoneyridge/mca.c +++ b/src/soc/amd/stoneyridge/mca.c @@ -153,9 +153,7 @@ { int i; struct mca_bank mci; - unsigned int num_banks; - - num_banks = mca_get_bank_count(); + const unsigned int num_banks = mca_get_bank_count(); if (is_warm_reset()) { for (i = 0 ; i < num_banks ; i++) { -- To view, visit https://review.coreboot.org/c/coreboot/+/56236 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I23aa4d36d4e6d4c7ed66800c2e7963c4ed03c393 Gerrit-Change-Number: 56236 Gerrit-PatchSet: 3 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-MessageType: merged

1 0

Change in coreboot[master]: include/cpu/x86/msr: add IA32_ prefix to MC0_ADDR and MC0_MISC
by Felix Held (Code Review) 14 Jul '21

14 Jul '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/56235 ) Change subject: include/cpu/x86/msr: add IA32_ prefix to MC0_ADDR and MC0_MISC ...................................................................... include/cpu/x86/msr: add IA32_ prefix to MC0_ADDR and MC0_MISC Those registers are architectural MSR and this also gets them in line with IA32_MC0_CTL and IA32_MC0_STATUS. Also move them below the definitions for IA32_MC0_STATUS, so that the numbers of the MSRs are ascending. Change-Id: Icef6526c896720248f5b648ddf1a271bdf46917c Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/56235 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 M src/soc/amd/stoneyridge/mca.c 2 files changed, 4 insertions(+), 4 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 748aed5..da1932c 100644 --- a/src/include/cpu/x86/msr.h +++ b/src/include/cpu/x86/msr.h @@ -74,11 +74,11 @@ #define MCA_STATUS_LO_ERRCODE_EXT_SH 16 #define MCA_STATUS_LO_ERRCODE_EXT_MASK (0x3f << MCA_STATUS_LO_ERRCODE_EXT_SH) #define MCA_STATUS_LO_ERRCODE_MASK (0xffff << 0) +#define IA32_MC0_ADDR 0x402 +#define IA32_MC0_MISC 0x403 #define IA32_VMX_BASIC_MSR 0x480 #define VMX_BASIC_HI_DUAL_MONITOR (1UL << (49 - 32)) #define IA32_VMX_MISC_MSR 0x485 -#define MC0_ADDR 0x402 -#define MC0_MISC 0x403 #define MC0_CTL_MASK 0xC0010044 #define IA32_PM_ENABLE 0x770 diff --git a/src/soc/amd/stoneyridge/mca.c b/src/soc/amd/stoneyridge/mca.c index 1523563..c2ec402 100644 --- a/src/soc/amd/stoneyridge/mca.c +++ b/src/soc/amd/stoneyridge/mca.c @@ -169,10 +169,10 @@ printk(BIOS_WARNING, " MC%d_STATUS = %08x_%08x\n", i, mci.sts.hi, mci.sts.lo); - mci.addr = rdmsr(MC0_ADDR + (i * 4)); + mci.addr = rdmsr(IA32_MC0_ADDR + (i * 4)); printk(BIOS_WARNING, " MC%d_ADDR = %08x_%08x\n", i, mci.addr.hi, mci.addr.lo); - mci.misc = rdmsr(MC0_MISC + (i * 4)); + mci.misc = rdmsr(IA32_MC0_MISC + (i * 4)); printk(BIOS_WARNING, " MC%d_MISC = %08x_%08x\n", i, mci.misc.hi, mci.misc.lo); mci.ctl = rdmsr(IA32_MC0_CTL + (i * 4)); -- To view, visit https://review.coreboot.org/c/coreboot/+/56235 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Icef6526c896720248f5b648ddf1a271bdf46917c Gerrit-Change-Number: 56235 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-MessageType: merged

1 0

Change in coreboot[master]: src: Use initial_lapicid() instead of open coding it
by Felix Held (Code Review) 14 Jul '21

14 Jul '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/55063 ) Change subject: src: Use initial_lapicid() instead of open coding it ...................................................................... src: Use initial_lapicid() instead of open coding it Since initial_lapicid() returns an unsigned int, change the type of the local variables the return value gets assigned to to unsigned int as well if applicable. Also change the printk format strings for printing the variable's contents to %u where it was %d before. Change-Id: I289015b81b2a9d915c4cab9b0544fc19b85df7a3 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/55063 Reviewed-by: Raul Rangel <rrangel(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/drivers/amd/agesa/eventlog.c M src/drivers/amd/agesa/romstage.c M src/soc/amd/picasso/mca.c M src/soc/amd/stoneyridge/mca.c M src/soc/intel/xeon_sp/smmrelocate.c 5 files changed, 13 insertions(+), 14 deletions(-) Approvals: build bot (Jenkins): Verified Raul Rangel: Looks good to me, approved diff --git a/src/drivers/amd/agesa/eventlog.c b/src/drivers/amd/agesa/eventlog.c index 49ab4ce..126a2ee 100644 --- a/src/drivers/amd/agesa/eventlog.c +++ b/src/drivers/amd/agesa/eventlog.c @@ -1,5 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0-only */ +#include <cpu/x86/lapic.h> #include <console/console.h> #include <stdint.h> #include <string.h> @@ -97,7 +98,7 @@ { int i; - task->apic_id = (u8) (cpuid_ebx(1) >> 24); + task->apic_id = (u8)initial_lapicid(); task->func = func; task->function_name = undefined; diff --git a/src/drivers/amd/agesa/romstage.c b/src/drivers/amd/agesa/romstage.c index 29423ef..b8f38ce 100644 --- a/src/drivers/amd/agesa/romstage.c +++ b/src/drivers/amd/agesa/romstage.c @@ -6,6 +6,7 @@ #include <arch/romstage.h> #include <cbmem.h> #include <console/console.h> +#include <cpu/x86/lapic.h> #include <halt.h> #include <program_loading.h> #include <romstage_handoff.h> @@ -35,7 +36,7 @@ struct postcar_frame pcf; struct sysinfo romstage_state; struct sysinfo *cb = &romstage_state; - u8 initial_apic_id = (u8) (cpuid_ebx(1) >> 24); + unsigned int initial_apic_id = initial_lapicid(); int cbmem_initted = 0; fill_sysinfo(cb); @@ -49,7 +50,7 @@ console_init(); } - printk(BIOS_DEBUG, "APIC %02d: CPU Family_Model = %08x\n", + printk(BIOS_DEBUG, "APIC %02u: CPU Family_Model = %08x\n", initial_apic_id, cpuid_eax(1)); set_ap_entry_ptr(ap_romstage_main); diff --git a/src/soc/amd/picasso/mca.c b/src/soc/amd/picasso/mca.c index acb248d..65b1916 100644 --- a/src/soc/amd/picasso/mca.c +++ b/src/soc/amd/picasso/mca.c @@ -2,6 +2,7 @@ #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> @@ -160,10 +161,8 @@ for (i = 0 ; i < num_banks ; i++) { mci.sts = rdmsr(MCAX_STATUS_MSR(i)); if (mci.sts.hi || mci.sts.lo) { - int core = cpuid_ebx(1) >> 24; - - printk(BIOS_WARNING, "#MC Error: core %d, bank %d %s\n", - core, i, + printk(BIOS_WARNING, "#MC Error: core %u, bank %d %s\n", + initial_lapicid(), i, i < ARRAY_SIZE(mca_bank_name) ? mca_bank_name[i] : ""); printk(BIOS_WARNING, " MC%d_STATUS = %08x_%08x\n", diff --git a/src/soc/amd/stoneyridge/mca.c b/src/soc/amd/stoneyridge/mca.c index db5fabc3..1523563 100644 --- a/src/soc/amd/stoneyridge/mca.c +++ b/src/soc/amd/stoneyridge/mca.c @@ -1,6 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0-only */ #include <amdblocks/reset.h> +#include <cpu/x86/lapic.h> #include <cpu/x86/msr.h> #include <acpi/acpi.h> #include <soc/cpu.h> @@ -163,10 +164,8 @@ mci.sts = rdmsr(IA32_MC0_STATUS + (i * 4)); if (mci.sts.hi || mci.sts.lo) { - int core = cpuid_ebx(1) >> 24; - - printk(BIOS_WARNING, "#MC Error: core %d, bank %d %s\n", - core, i, mca_bank_name[i]); + printk(BIOS_WARNING, "#MC Error: core %u, bank %d %s\n", + initial_lapicid(), i, mca_bank_name[i]); printk(BIOS_WARNING, " MC%d_STATUS = %08x_%08x\n", i, mci.sts.hi, mci.sts.lo); diff --git a/src/soc/intel/xeon_sp/smmrelocate.c b/src/soc/intel/xeon_sp/smmrelocate.c index dc4b511..f44fc62 100644 --- a/src/soc/intel/xeon_sp/smmrelocate.c +++ b/src/soc/intel/xeon_sp/smmrelocate.c @@ -2,6 +2,7 @@ #include <assert.h> #include <string.h> +#include <cpu/x86/lapic.h> #include <cpu/x86/mp.h> #include <cpu/intel/em64t101_save_state.h> #include <cpu/intel/smm_reloc.h> @@ -86,7 +87,6 @@ { u32 smbase; u32 iedbase; - int apic_id; em64t101_smm_state_save_area_t *save_state; /* * The relocated handler runs with all CPUs concurrently. Therefore @@ -96,9 +96,8 @@ smbase = staggered_smbase; iedbase = relo_params->ied_base; - apic_id = cpuid_ebx(1) >> 24; printk(BIOS_DEBUG, "New SMBASE=0x%08x IEDBASE=0x%08x\n apic_id=0x%x\n", - smbase, iedbase, apic_id); + smbase, iedbase, initial_lapicid()); save_state = (void *)(curr_smbase + SMM_DEFAULT_SIZE - sizeof(*save_state)); -- To view, visit https://review.coreboot.org/c/coreboot/+/55063 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I289015b81b2a9d915c4cab9b0544fc19b85df7a3 Gerrit-Change-Number: 55063 Gerrit-PatchSet: 6 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Morgan Jang <Morgan_Jang(a)wiwynn.com> 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: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: merged

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit July 2021