coreboot-gerrit July 2021

coreboot-gerrit@coreboot.org

1 participants
5824 discussions

Change in coreboot[master]: cpu/intel: Add dedicated file to grow Intel CPUIDs
by Furquan Shaikh (Code Review) 15 Jul '21

15 Jul '21

Attention is currently required from: Mariusz Szafrański, Jonathan Zhang, Subrata Banik, Angel Pons, Arthur Heymans, Patrick Rudolph, Anjaneya "Reddy" Chagam, Johnny Lin, Tim Wawrzynczak, Suresh Bellampalli, Vanessa Eusebio, Michal Motyl, Morgan Jang. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56333 ) Change subject: cpu/intel: Add dedicated file to grow Intel CPUIDs ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/56333 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id88e038c5d8b1ae077c822554582410de6f4a7ca Gerrit-Change-Number: 56333 Gerrit-PatchSet: 2 Gerrit-Owner: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Reviewer: Mariusz Szafrański <mariuszx.szafranski(a)intel.com> Gerrit-Reviewer: Michal Motyl <michalx.motyl(a)intel.com> Gerrit-Reviewer: Morgan Jang <Morgan_Jang(a)wiwynn.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Suresh Bellampalli <suresh.bellampalli(a)intel.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Mariusz Szafrański <mariuszx.szafranski(a)intel.com> Gerrit-Attention: Jonathan Zhang <jonzhang(a)fb.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Anjaneya "Reddy" Chagam <anjaneya.chagam(a)intel.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Suresh Bellampalli <suresh.bellampalli(a)intel.com> Gerrit-Attention: Vanessa Eusebio <vanessa.f.eusebio(a)intel.com> Gerrit-Attention: Michal Motyl <michalx.motyl(a)intel.com> Gerrit-Attention: Morgan Jang <Morgan_Jang(a)wiwynn.com> Gerrit-Comment-Date: Thu, 15 Jul 2021 20:25:58 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mainboard/starlabs: Add Star Labs labtop series
by Sean Rhodes (Code Review) 15 Jul '21

15 Jul '21

Attention is currently required from: Nico Huber, Tim Wawrzynczak. Hello build bot (Jenkins), Nico Huber, Tim Wawrzynczak, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/56088 to look at the new patch set (#43). Change subject: mainboard/starlabs: Add Star Labs labtop series ...................................................................... mainboard/starlabs: Add Star Labs labtop series Add support for: LabTop Mk III (kbl-r) LabTop Mk IV (cml) StarBook Mk V (tgl) Signed-off-by: Sean Rhodes <sean(a)starlabs.systems> Change-Id: I090971a9e8d2be5b08be886d00d304607304b645 --- M Documentation/distributions.md M Documentation/mainboard/index.md A Documentation/mainboard/starlabs/labtop.md M MAINTAINERS A src/mainboard/starlabs/Kconfig A src/mainboard/starlabs/Kconfig.name A src/mainboard/starlabs/labtop/Kconfig A src/mainboard/starlabs/labtop/Kconfig.name A src/mainboard/starlabs/labtop/Makefile.inc A src/mainboard/starlabs/labtop/acpi/ec.asl A src/mainboard/starlabs/labtop/acpi/mainboard.asl A src/mainboard/starlabs/labtop/acpi/sleep.asl A src/mainboard/starlabs/labtop/acpi/superio.asl A src/mainboard/starlabs/labtop/board_info.txt A src/mainboard/starlabs/labtop/bootblock.c A src/mainboard/starlabs/labtop/cmos.default A src/mainboard/starlabs/labtop/cmos.layout A src/mainboard/starlabs/labtop/dsdt.asl A src/mainboard/starlabs/labtop/hda_verb.c A src/mainboard/starlabs/labtop/mainboard.c A src/mainboard/starlabs/labtop/ramstage.c A src/mainboard/starlabs/labtop/spd/Makefile.inc A src/mainboard/starlabs/labtop/spd/empty_ddr4.spd.hex A src/mainboard/starlabs/labtop/spd/micron-MT40A1G16KD-062E-E.spd.hex A src/mainboard/starlabs/labtop/spd/samsung-K4A8G165WB-BCRC.spd.hex A src/mainboard/starlabs/labtop/spd/spd.h A src/mainboard/starlabs/labtop/spd/spd_util.c A src/mainboard/starlabs/labtop/variants/baseboard/include/baseboard/memory.h A src/mainboard/starlabs/labtop/variants/baseboard/include/baseboard/romstage.h A src/mainboard/starlabs/labtop/variants/baseboard/include/baseboard/variants.h A src/mainboard/starlabs/labtop/variants/cml/Makefile.inc A src/mainboard/starlabs/labtop/variants/cml/board.fmd A src/mainboard/starlabs/labtop/variants/cml/data.vbt A src/mainboard/starlabs/labtop/variants/cml/devicetree.cb A src/mainboard/starlabs/labtop/variants/cml/devtree.c A src/mainboard/starlabs/labtop/variants/cml/gma-mainboard.ads A src/mainboard/starlabs/labtop/variants/cml/gpio.c A src/mainboard/starlabs/labtop/variants/cml/hda_verb.c A src/mainboard/starlabs/labtop/variants/cml/include/variant/ec.h A src/mainboard/starlabs/labtop/variants/cml/romstage.c A src/mainboard/starlabs/labtop/variants/kbl/Makefile.inc A src/mainboard/starlabs/labtop/variants/kbl/board.fmd A src/mainboard/starlabs/labtop/variants/kbl/data.vbt A src/mainboard/starlabs/labtop/variants/kbl/devicetree.cb A src/mainboard/starlabs/labtop/variants/kbl/devtree.c A src/mainboard/starlabs/labtop/variants/kbl/gma-mainboard.ads A src/mainboard/starlabs/labtop/variants/kbl/gpio.c A src/mainboard/starlabs/labtop/variants/kbl/hda_verb.c A src/mainboard/starlabs/labtop/variants/kbl/include/variant/ec.h A src/mainboard/starlabs/labtop/variants/kbl/include/variant/gpio.c A src/mainboard/starlabs/labtop/variants/kbl/include/variant/hda_verb.c A src/mainboard/starlabs/labtop/variants/kbl/romstage.c A src/mainboard/starlabs/labtop/variants/tgl/Makefile.inc A src/mainboard/starlabs/labtop/variants/tgl/board.fmd A src/mainboard/starlabs/labtop/variants/tgl/data.vbt A src/mainboard/starlabs/labtop/variants/tgl/devicetree.cb A src/mainboard/starlabs/labtop/variants/tgl/devtree.c A src/mainboard/starlabs/labtop/variants/tgl/gpio.c A src/mainboard/starlabs/labtop/variants/tgl/hda_verb.c A src/mainboard/starlabs/labtop/variants/tgl/include/variant/ec.h A src/mainboard/starlabs/labtop/variants/tgl/romstage.c 61 files changed, 3,302 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/88/56088/43 -- To view, visit https://review.coreboot.org/c/coreboot/+/56088 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I090971a9e8d2be5b08be886d00d304607304b645 Gerrit-Change-Number: 56088 Gerrit-PatchSet: 43 Gerrit-Owner: Sean Rhodes <admin(a)starlabs.systems> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Andy Pont <andy.pont(a)sdcsystems.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

Change in coreboot[master]: console: Handle verstage as "first non-bootblock stage"
by Julius Werner (Code Review) 15 Jul '21

15 Jul '21

Attention is currently required from: Nico Huber. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56210 ) Change subject: console: Handle verstage as "first non-bootblock stage" ...................................................................... Patch Set 1: (1 comment) File src/console/init.c: https://review.coreboot.org/c/coreboot/+/56210/comment/5e7b495a_c37632ad PS1, Line 15: #endif > Given the back and forth here I'm thinking of making all this explicit, even though I dislike option […] No, please, let's not overkill it. The arch/arm requirement is more of a non-x86 (or non-BOOT_DEVICE_MEMORY_MAPPED) requirement anyway. And I think for x86 there are other good reasons not to rely on this in the bootblock too, more similar to what Nico mentioned, because CBFS *is* actually a pretty complicated system where it would be good to have output if something goes wrong (e.g. with CBFS_VERIFICATION, it will print an error and die on a hash mismatch... would be good if that was always visible). The thing is just that for CBFS, of course it's going to get used anyway whether BOOTBLOCK_CONSOLE is enabled or not, so if you have a CBFS problem and NO_BOOTBLOCK_CONSOLE you're not gonna see anything either way. For get_option(), I guess that's not necessarily true (but it might). I would prefer to use ENV_INITIAL_STAGE to guard the CBFS stuff, and then whether get_option() is also guarded by that or not at all I don't care too much about. I'd assume the "using get_option() + using NO_BOOTBLOCK_CONSOLE + error in get_option() + get_option() not already used anyway by other bootblock stuff" intersection is probably not really big enough to worry that much about. -- To view, visit https://review.coreboot.org/c/coreboot/+/56210 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I494a33483e306a049aa0c8137a118644fc28177e Gerrit-Change-Number: 56210 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Thu, 15 Jul 2021 19:02:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Patrick Georgi <pgeorgi(a)google.com> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: mb/google/brya/var/gimble: Include SPD for MT53E1G32D2NP-046 WT:A and...
by Caveh Jalali (Code Review) 15 Jul '21

15 Jul '21

Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Nick Vaccaro, Mark Hsieh. Caveh Jalali has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56329 ) Change subject: mb/google/brya/var/gimble: Include SPD for MT53E1G32D2NP-046 WT:A and K4U6E3S4AA-MGCR ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/56329 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4bfc18fd42c6ff2675e6f836c2ecc9617fac3aff Gerrit-Change-Number: 56329 Gerrit-PatchSet: 1 Gerrit-Owner: Mark Hsieh <mark_hsieh(a)wistron.corp-partner.google.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Caveh Jalali <caveh(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Mark Hsieh <mark_hsieh(a)wistron.corp-partner.google.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anfernee Chen <anfernee_chen(a)wistron.corp-partner.google.com> Gerrit-CC: Ariel Fang <ariel_fang(a)wistron.corp-partner.google.com> Gerrit-CC: Casper Chang <casper_chang(a)wistron.corp-partner.google.com> Gerrit-CC: Malik Hsu <malik_hsu(a)wistron.corp-partner.google.com> Gerrit-CC: Scott Chao <scott_chao(a)wistron.corp-partner.google.com> Gerrit-CC: Terry Chen <terry_chen(a)wistron.corp-partner.google.com> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Attention: Mark Hsieh <mark_hsieh(a)wistron.corp-partner.google.com> Gerrit-Comment-Date: Thu, 15 Jul 2021 18:39:57 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in coreboot[master]: Revert "mb/google/brya: Enable south XHCI ports 1 and 2"
by Furquan Shaikh (Code Review) 15 Jul '21

15 Jul '21

Attention is currently required from: Tim Wawrzynczak, Ronak Kanabar. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56132 ) Change subject: Revert "mb/google/brya: Enable south XHCI ports 1 and 2" ...................................................................... Patch Set 2: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/56132/comment/e2228ddc_969844d2 PS2, Line 12: BUG=? TEST=? -- To view, visit https://review.coreboot.org/c/coreboot/+/56132 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I82bae21d185247bc0f3580fd6f92abb8eece6732 Gerrit-Change-Number: 56132 Gerrit-PatchSet: 2 Gerrit-Owner: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Reviewer: EricR Lai <ericr_lai(a)compal.corp-partner.google.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.corp-partner.google.com> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Tim Wawrzynczak <twawrzynczak(a)chromium.org> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.corp-partner.google.com> Gerrit-Comment-Date: Thu, 15 Jul 2021 18:25:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No 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) 15 Jul '21

15 Jul '21

Felix Held has submitted this change. ( 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> Reviewed-on: https://review.coreboot.org/c/coreboot/+/56315 Reviewed-by: Martin Roth <martinroth(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/amd/common/block/cpu/mca/mcax.c 1 file changed, 22 insertions(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified Martin Roth: Looks good to me, approved 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: 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: 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-MessageType: merged

1 0

Change in coreboot[master]: src/drivers/intel/fsp2_0: allow larger FSP 2.0 header
by Furquan Shaikh (Code Review) 15 Jul '21

15 Jul '21

Attention is currently required from: Martin Roth, Michał Żygowski, Marshall Dawson, Subrata Banik, Nikolai Vyssotski, Andrey Petrov, Patrick Rudolph, Nathaniel L Desimone. Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56190 ) Change subject: src/drivers/intel/fsp2_0: allow larger FSP 2.0 header ...................................................................... Patch Set 5: (4 comments) File src/drivers/intel/fsp2_0/util.c: https://review.coreboot.org/c/coreboot/+/56190/comment/e136103c_5fe0a376 PS3, Line 14: looks_like_fsp_header > i believe upper boundary check is always prescribed isn't it. we don't know what for those additional bytes are being introduced? Prescribed by? If there are additional bytes, it shouldn't really matter for the consumer unless there were assumptions that the payload starts right after the header. But that is not true. There are pointers to get to the required components within the payload. Given that having additional bytes in header which remain unused should be fine. > Assume for this case, if we really want to discard those additional 4 bytes of MultiSi API then can't we check if FSP_INFO_HEADER.HeaderRevision < 5 then FSP_INFO_HEADER..HeaderLength - 4 == 72 would help to find the integrity of the FSP header with FSP 2.0 isn't it ? (in this process we actually knew what we are discarding) vs a minimal boundary check? But, how would you verify the integrity? There is nothing in the header that provides checksum. So, there is no way for coreboot to perform integrity check. Also, there is no way to know what version of EDK2 the header was built with. So, subtracting 4 works in this case but not for other cases. > If FSP_INFO_HEADER.HeaderRevision >= 5 then we are expecting MultiSi is added and in that case FSP_INFO_HEADER..HeaderLength should be 76 with FSP 2.2 spec? Are you saying that instead of relying on FSP version, we should check Header version? That should work. But, it would still be the same checks. Currently, FSP version and header version are upreved at the same time. Thus, each header version has a required length field. As long as the provided header is at least as long as the required length, it shouldn't be a problem. > Sorry may be I'm thinking a loud about how someone can explode the situation and introduced few more APIs or some data fields without bootloader knowledge (may be an imaginary situation unless its actually appears) If someone adds new APIs or new fields to the header without bootloader knowledge, then those fields would be considered invalid. The consumer (bootloader) has no way to know what that means and would simply discard those. File src/drivers/intel/fsp2_0/util.c: https://review.coreboot.org/c/coreboot/+/56190/comment/e31a6c28_78223084 PS5, Line 17: if (CONFIG(PLATFORM_USES_FSP2_2)) > Somewhat unrelated to this patch, but does it make sense to switch this from a config to a runtime o […] I think we should evaluate that separately. For now, let's continue with this static selection. https://review.coreboot.org/c/coreboot/+/56190/comment/eae589cb_55ac6fa8 PS5, Line 30: > Should we check the bytes 10 & 11 as well? […] Doesn't hurt to check the FSP version against the version that is advertised to ensure Kconfig selection in coreboot is correct. But, like you said it should be done as a follow-up if required. https://review.coreboot.org/c/coreboot/+/56190/comment/910b2f16_f08eb0a0 PS5, Line 36: FSP 2.0 header can actually have two lengths: 72 and 76. I think this can be dropped since this is the condition today, but might change in the future. Probably say: "It is possible to build FSP with any version of EDK2 which could have introduced new fields in FSP_INFO_HEADER. The new fields will be ignored based on the reported FSP version. This check ensures that the reported header length is at least what the reported FSP version requires so that we do not access any out-of-bound bytes." -- To view, visit https://review.coreboot.org/c/coreboot/+/56190 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie8422447b2cff0a6c536e13014905ffa15c70586 Gerrit-Change-Number: 56190 Gerrit-PatchSet: 5 Gerrit-Owner: Nikolai Vyssotski <nikolai.vyssotski(a)amd.corp-partner.google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Nathaniel L Desimone <nathaniel.l.desimone(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-CC: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Martin Roth <martinroth(a)google.com> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Attention: Subrata Banik <subrata.banik(a)intel.com> Gerrit-Attention: Nikolai Vyssotski <nikolai.vyssotski(a)amd.corp-partner.google.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Attention: Nathaniel L Desimone <nathaniel.l.desimone(a)intel.com> Gerrit-Comment-Date: Thu, 15 Jul 2021 17:52:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Comment-In-Reply-To: Martin Roth <martinroth(a)google.com> Comment-In-Reply-To: Subrata Banik <subrata.banik(a)intel.com> Comment-In-Reply-To: Nikolai Vyssotski <nikolai.vyssotski(a)amd.corp-partner.google.com> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: x86/smp/spinlock: Disable thread coop when taking spinlock
by Julius Werner (Code Review) 15 Jul '21

15 Jul '21

Attention is currently required from: Raul Rangel, Furquan Shaikh. Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/56320 ) Change subject: x86/smp/spinlock: Disable thread coop when taking spinlock ...................................................................... Patch Set 1: (1 comment) File src/arch/x86/include/arch/smp/spinlock.h: https://review.coreboot.org/c/coreboot/+/56320/comment/14bfe7c3_cd536bb8 PS1, Line 61: lock->coop = thread_coop_enabled(); > So I had the same thought as your did about storing it in the thread structure, but there is only 1 […] Oh... hmm... yeah okay, that's a fair point. I still think the semaphore approach is much better though, because like I said there will probably be other situations with the same fundamental problem, and it's better to just solve them all in one go. -- To view, visit https://review.coreboot.org/c/coreboot/+/56320 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1b929070b7f175965d4f37be693462fef26be052 Gerrit-Change-Number: 56320 Gerrit-PatchSet: 1 Gerrit-Owner: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Julius Werner <jwerner(a)chromium.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Furquan Shaikh <furquan(a)google.com> Gerrit-Comment-Date: Thu, 15 Jul 2021 17:52:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Raul Rangel <rrangel(a)chromium.org> Comment-In-Reply-To: Furquan Shaikh <furquan(a)google.com> Comment-In-Reply-To: Julius Werner <jwerner(a)chromium.org> Gerrit-MessageType: comment

1 0

Change in coreboot[master]: [WIP] soc/amd/cezanne/mca: add and use mca_bank_name[]
by Felix Held (Code Review) 15 Jul '21

15 Jul '21

Felix Held has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/56345 ) Change subject: [WIP] soc/amd/cezanne/mca: add and use mca_bank_name[] ...................................................................... [WIP] soc/amd/cezanne/mca: add and use mca_bank_name[] This enables the MCAX checking and BERT entry generation for Cezanne. TODO: verification Change-Id: Ibe8047ce5bb5e7136a8786693bcced4d2225b1fd Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> --- M src/soc/amd/cezanne/mca.c 1 file changed, 38 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/45/56345/1 diff --git a/src/soc/amd/cezanne/mca.c b/src/soc/amd/cezanne/mca.c index 77006a7..778b8e7 100644 --- a/src/soc/amd/cezanne/mca.c +++ b/src/soc/amd/cezanne/mca.c @@ -1,19 +1,54 @@ /* SPDX-License-Identifier: GPL-2.0-only */ #include <amdblocks/mca.h> +#include <cpu/x86/msr.h> #include <types.h> +static const char *const mca_bank_name[] = { + [0] = "Load-store unit", + [1] = "Instruction fetch unit", + [2] = "L2 cache unit", + [3] = "Decode unit", + [4] = "", + [5] = "Execution unit", + [6] = "Floating point unit", + [7] = "L3 cache unit", + [8] = "L3 cache unit", + [9] = "L3 cache unit", + [10] = "L3 cache unit", + [11] = "L3 cache unit", + [12] = "L3 cache unit", + [13] = "L3 cache unit", + [14] = "L3 cache unit", + [15] = "", + [16] = "", + [17] = "UMC", + [18] = "UMC", + [19] = "CS", + [20] = "CS", + [21] = "", + [22] = "", + [23] = "", + [24] = "", + [25] = "", + [26] = "", + [27] = "PIE", +}; + bool mca_has_expected_bank_count(void) { - return true; + return ARRAY_SIZE(mca_bank_name) == mca_get_bank_count(); } bool mca_is_valid_bank(unsigned int bank) { - return false; + return (bank < ARRAY_SIZE(mca_bank_name) && mca_bank_name[bank] != NULL); } const char *mca_get_bank_name(unsigned int bank) { - return ""; + if (mca_is_valid_bank(bank)) + return mca_bank_name[bank]; + else + return ""; } -- To view, visit https://review.coreboot.org/c/coreboot/+/56345 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibe8047ce5bb5e7136a8786693bcced4d2225b1fd Gerrit-Change-Number: 56345 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) 15 Jul '21

15 Jul '21

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/56314 ) Change subject: soc/amd/common/block/cpu/mca: make building the BERT support conditional ...................................................................... soc/amd/common/block/cpu/mca: make building the BERT support conditional Only when ACPI_BERT is selected the BERT functionality needs to be included in the build. Change-Id: I8a21562f4535fb0ea3c53f2ea8df50f66cc6a64c Signed-off-by: Felix Held <felix-coreboot(a)felixheld.de> Reviewed-on: https://review.coreboot.org/c/coreboot/+/56314 Reviewed-by: Martin Roth <martinroth(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/amd/common/block/cpu/mca/Makefile.inc 1 file changed, 3 insertions(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified Martin Roth: Looks good to me, approved diff --git a/src/soc/amd/common/block/cpu/mca/Makefile.inc b/src/soc/amd/common/block/cpu/mca/Makefile.inc index 0232805..fd9e573 100644 --- a/src/soc/amd/common/block/cpu/mca/Makefile.inc +++ b/src/soc/amd/common/block/cpu/mca/Makefile.inc @@ -1,14 +1,14 @@ ifeq ($(CONFIG_SOC_AMD_COMMON_BLOCK_MCA_COMMON),y) ramstage-y += mca_common.c -ramstage-y += mca_common_bert.c +ramstage-$(CONFIG_ACPI_BERT) += mca_common_bert.c endif # CONFIG_SOC_AMD_COMMON_BLOCK_MCA_COMMON ifeq ($(CONFIG_SOC_AMD_COMMON_BLOCK_MCA),y) -ramstage-y += mca_bert.c +ramstage-$(CONFIG_ACPI_BERT) += mca_bert.c ramstage-y += mca.c endif # CONFIG_SOC_AMD_COMMON_BLOCK_MCA ifeq ($(CONFIG_SOC_AMD_COMMON_BLOCK_MCAX),y) -ramstage-y += mcax_bert.c +ramstage-$(CONFIG_ACPI_BERT) += mcax_bert.c ramstage-y += mcax.c endif # CONFIG_SOC_AMD_COMMON_BLOCK_MCAX -- To view, visit https://review.coreboot.org/c/coreboot/+/56314 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8a21562f4535fb0ea3c53f2ea8df50f66cc6a64c Gerrit-Change-Number: 56314 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: 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-MessageType: merged

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit July 2021