coreboot-gerrit September 2023

coreboot-gerrit@coreboot.org

1 participants
3824 discussions

[S] Change in coreboot[master]: drivers/intel/fsp2_0: Introduce MRC cache store after FSP-M/S APIs
by Martin L Roth (Code Review) 03 Sep '23

03 Sep '23

Attention is currently required from: Andrey Petrov, Felix Held, Fred Reitberger, Jason Glenesk, Johnny Lin, Kapil Porwal, Lean Sheng Tan, Matt DeVillier, Nico Huber, Paul Menzel, Raul Rangel, Subrata Banik. Martin L Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77556?usp=email ) Change subject: drivers/intel/fsp2_0: Introduce MRC cache store after FSP-M/S APIs ...................................................................... Patch Set 6: (2 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/77556/comment/3796abf0_a8dfe22f : PS3, Line 43: +--------------------+--------------------------+--------------------------+-------------------------+ > > Not sure if the long tables are necessary as only a single item changes. […] Just because it's not an issue in one program's git log function doesn't mean it isn't in others. There are lots of different tools and there's a reason for the rule to wrap at 72/75 characters. Commit Message: https://review.coreboot.org/c/coreboot/+/77556/comment/b9770820_0a5865b0 : PS6, Line 21: cache post in FSP-S (at ramstage). Hence, this patch provides an API to Nit: this sentence needs to be rephrased. -- To view, visit https://review.coreboot.org/c/coreboot/+/77556?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id1e91d25916594f59d1e467a142f5042c6138b51 Gerrit-Change-Number: 77556 Gerrit-PatchSet: 6 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.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-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Sun, 03 Sep 2023 03:47:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Subrata Banik <subratabanik(a)google.com> Gerrit-MessageType: comment

1 0

[L] Change in coreboot[master]: SNB+MRC boards: Do not redo PEI data struct in hook
by Keith Hui (Code Review) 03 Sep '23

03 Sep '23

Attention is currently required from: Alexander Couzens, Angel Pons, Elyes Haouas, Martin L Roth. Keith Hui has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/76962?usp=email ) Change subject: SNB+MRC boards: Do not redo PEI data struct in hook ...................................................................... Patch Set 4: (6 comments) File src/mainboard/asus/p8x7x-series/variants/p8z77-m_pro/early_init.c: https://review.coreboot.org/c/coreboot/+/76962/comment/70e776bd_bb04f7a4 : PS4, Line 16: #include <northbridge/intel/sandybridge/pei_data.h> > why remove this? I think we'd want this in every file - we prefer not to use indirect includes. Part of my goal with this is to have all the boards include only raminit.h no matter what raminit it is going to use. Is the reason why I placed the Haswell API definitions in raminit.h. Question: If I want to achieve this while sticking to the no-indirect-include rule, would I have to get rid of mainboard_fill_pei_data() completely as well? raminit.h itself already includes pei_data.h for its prototypes. https://review.coreboot.org/c/coreboot/+/76962/comment/cce6cd4e_d302ec4f : PS4, Line 59: pei_data) > This name was changed in this file, but not in others. […] I used the shorter "pei" when I added p8z77-m board. Although I do like to shorten this as I go as well, I try to keep this name same to reduce the number of lines changed. Mismatches are most likely oversight. https://review.coreboot.org/c/coreboot/+/76962/comment/f0ccf2f0_4ce7125f : PS4, Line 60: [] > Nit: I'd probably use a #define here to make sure it matches the size of spd_addresses in the pei_da […] I'm following existing convention (as in p8z77-m, the only board already doing what this patch looks to achieve). And as you see in subsequent patches in the train, this paricular line won't be around for long. https://review.coreboot.org/c/coreboot/+/76962/comment/20f24c97_b88878e9 : PS4, Line 61: 16][3 > Nit: These magic numbers could use some #defines to make sure they're in sync with the definition of […] This will need to be addressed separately because it is part of a bigger problem: SNB/IVB boards also have to specify their USB configurations twice, because value orders are different and one of them has different representation. Eventually we also want to harmonize this value and then stuff it into devicetree, this being another set of static data. We have a setting reserved for this but it also remains unused to this date. https://review.coreboot.org/c/coreboot/+/76962/comment/9cf37237_53e40037 : PS4, Line 62: /* {enabled, oc_pin, cable len 0x0080=<8inches/20cm} */ > This comment or something similar could be applied in other files if you wanted. Up to you. To be addressed separately. https://review.coreboot.org/c/coreboot/+/76962/comment/c32e5e80_520342a3 : PS4, Line 79: sizeof(usbcfg) > Maybe use the size of pei.usb_port_config & pei. […] Makes sense. I'll update the USB stuff for next patch set. SPD stuff won't be around for long. -- To view, visit https://review.coreboot.org/c/coreboot/+/76962?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie349a8f400eecca3cdbc196ea0790aebe0549e39 Gerrit-Change-Number: 76962 Gerrit-PatchSet: 4 Gerrit-Owner: Keith Hui <buurin(a)gmail.com> Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Comment-Date: Sun, 03 Sep 2023 03:29:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Martin L Roth <gaumless(a)gmail.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: {drivers/intel/fsp2_0, soc/intel}: Rename `SAVE_MRC_AFTER_FSPS` config
by Martin L Roth (Code Review) 03 Sep '23

03 Sep '23

Attention is currently required from: Andrey Petrov, Arthur Heymans, Christian Walter, Johnny Lin, Kapil Porwal, Lean Sheng Tan, Nico Huber, Subrata Banik, Tim Chu. Martin L Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77616?usp=email ) Change subject: {drivers/intel/fsp2_0, soc/intel}: Rename `SAVE_MRC_AFTER_FSPS` config ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/77616?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I815a64263fa1415bfe30bb3c1c35e4adee307e86 Gerrit-Change-Number: 77616 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Sun, 03 Sep 2023 03:28:09 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: soc/intel/cannonlake/Kconfig: Deduplicate selections
by 9elements QA (Code Review) 03 Sep '23

03 Sep '23

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77447?usp=email ) Change subject: soc/intel/cannonlake/Kconfig: Deduplicate selections ...................................................................... Patch Set 2: Automatic boot test returned (PASS/FAIL/TOTAL): 14 / 1 / 15 PASS: x86_32 "Hermes CFL" , build config PRODRIVE_HERMES_ and payload TianoCore_UefiPayloadPkg : https://lava.9esec.io/r/182904 PASS: x86_32 "Hermes CFL" , build config PRODRIVE_HERMES and payload TianoCore_UefiPayloadPkg : https://lava.9esec.io/r/182902 PASS: x86_32 "ThinkPad T500" , build config LENOVO_T500 and payload SeaBIOS : https://lava.9esec.io/r/182901 PASS: x86_32 "HP Z220 SFF Workstation" , build config HP_Z220_SFF_WORKSTATION and payload LinuxBoot_BB_kexec : https://lava.9esec.io/r/182900 PASS: x86_64 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC.X86_64 and payload TianoCore : https://lava.9esec.io/r/182899 PASS: x86_64 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC.X86_64 and payload SeaBIOS : https://lava.9esec.io/r/182898 PASS: x86_32 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC and payload TianoCore : https://lava.9esec.io/r/182897 PASS: x86_32 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC and payload SeaBIOS : https://lava.9esec.io/r/182896 PASS: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35_SMM_TSEG and payload SeaBIOS : https://lava.9esec.io/r/182894 FAIL: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35 and payload TianoCore : https://lava.9esec.io/r/182893 PASS: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35 and payload SeaBIOS : https://lava.9esec.io/r/182892 PASS: x86_64 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_X86_64 and payload SeaBIOS : https://lava.9esec.io/r/182891 PASS: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_ASAN and payload SeaBIOS : https://lava.9esec.io/r/182890 PASS: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_ and payload SeaBIOS : https://lava.9esec.io/r/182889 PASS: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX and payload SeaBIOS : https://lava.9esec.io/r/182888 Please note: This test is under development and might not be accurate at all! -- To view, visit https://review.coreboot.org/c/coreboot/+/77447?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I6ce5edb2ba2c138b44601b32c3ecba2e761136f7 Gerrit-Change-Number: 77447 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com> Gerrit-Comment-Date: Sun, 03 Sep 2023 03:20:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[master]: soc/intel/cannonlake/Makefile.inc: Remove dead code of CNL SoC
by 9elements QA (Code Review) 03 Sep '23

03 Sep '23

9elements QA has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77446?usp=email ) Change subject: soc/intel/cannonlake/Makefile.inc: Remove dead code of CNL SoC ...................................................................... Patch Set 2: Automatic boot test returned (PASS/FAIL/TOTAL): 14 / 1 / 15 PASS: x86_32 "Hermes CFL" , build config PRODRIVE_HERMES_ and payload TianoCore_UefiPayloadPkg : https://lava.9esec.io/r/182921 PASS: x86_32 "Hermes CFL" , build config PRODRIVE_HERMES and payload TianoCore_UefiPayloadPkg : https://lava.9esec.io/r/182919 PASS: x86_32 "ThinkPad T500" , build config LENOVO_T500 and payload SeaBIOS : https://lava.9esec.io/r/182918 PASS: x86_32 "HP Z220 SFF Workstation" , build config HP_Z220_SFF_WORKSTATION and payload LinuxBoot_BB_kexec : https://lava.9esec.io/r/182917 PASS: x86_64 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC.X86_64 and payload TianoCore : https://lava.9esec.io/r/182916 PASS: x86_64 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC.X86_64 and payload SeaBIOS : https://lava.9esec.io/r/182915 PASS: x86_32 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC and payload TianoCore : https://lava.9esec.io/r/182914 PASS: x86_32 "HP Compaq 8200 Elite SFF PC" , build config HP_COMPAQ_8200_ELITE_SFF_PC and payload SeaBIOS : https://lava.9esec.io/r/182913 PASS: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35_SMM_TSEG and payload SeaBIOS : https://lava.9esec.io/r/182911 FAIL: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35 and payload TianoCore : https://lava.9esec.io/r/182910 PASS: x86_32 "QEMU x86 q35/ich9" , build config EMULATION_QEMU_X86_Q35 and payload SeaBIOS : https://lava.9esec.io/r/182909 PASS: x86_64 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_X86_64 and payload SeaBIOS : https://lava.9esec.io/r/182908 PASS: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_ASAN and payload SeaBIOS : https://lava.9esec.io/r/182907 PASS: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX_ and payload SeaBIOS : https://lava.9esec.io/r/182906 PASS: x86_32 "QEMU x86 i440fx/piix4" , build config EMULATION_QEMU_X86_I440FX and payload SeaBIOS : https://lava.9esec.io/r/182905 Please note: This test is under development and might not be accurate at all! -- To view, visit https://review.coreboot.org/c/coreboot/+/77446?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I53cd9d53b01e26f530684aa9c404f50b305c1f54 Gerrit-Change-Number: 77446 Gerrit-PatchSet: 2 Gerrit-Owner: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Felix Singer <service+coreboot-gerrit(a)felixsinger.de> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: 9elements QA <hardwaretestrobot(a)gmail.com> Gerrit-Comment-Date: Sun, 03 Sep 2023 03:06:50 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: soc/intel/xeon_sp: Scan and allocate resources on all stacks
by David Hendricks (Code Review) 03 Sep '23

03 Sep '23

Attention is currently required from: Arthur Heymans, Christian Walter, Nill Ge, Shuo Liu, TangYiwei, Tim Chu. David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77600?usp=email ) Change subject: soc/intel/xeon_sp: Scan and allocate resources on all stacks ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/77600?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie3e19bbd4683237ecfc427d32aa29f55964d80a5 Gerrit-Change-Number: 77600 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Nill Ge <geshijian(a)bytedance.com> Gerrit-Reviewer: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: TangYiwei Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Nill Ge <geshijian(a)bytedance.com> Gerrit-Attention: TangYiwei Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Sat, 02 Sep 2023 23:42:05 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: drivers/intel/fsp2_0: Introduce MRC cache store after FSP-M/S APIs
by Subrata Banik (Code Review) 03 Sep '23

03 Sep '23

Attention is currently required from: Andrey Petrov, Felix Held, Fred Reitberger, Jason Glenesk, Johnny Lin, Kapil Porwal, Lean Sheng Tan, Martin L Roth, Matt DeVillier, Nico Huber, Paul Menzel, Raul Rangel. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77556?usp=email ) Change subject: drivers/intel/fsp2_0: Introduce MRC cache store after FSP-M/S APIs ...................................................................... Patch Set 6: (1 comment) Patchset: PS6: @Jonny, can I request you to pick CB:77616 and CB:77556 Cls and share your feedback. I have verified the exclusive boot scenario related to xeon_sp where FSP-S populates the NVS data. Please take a look into the MRC cache write in successful post FSP-S. ``` First boot | Post FSP-S [DEBUG] FMAP: area RW_MRC_CACHE found @ 1710000 (65536 bytes) [DEBUG] MRC: Checking cached data update for 'RW_MRC_CACHE'. [INFO ] SF: Detected 00 0000 with sector size 0x1000, total 0x4000000 [ERROR] SF size 0x4000000 does not correspond to CONFIG_ROM_SIZE 0x2000000!! [NOTE ] MRC: no data in 'RW_MRC_CACHE' [DEBUG] MRC: cache data 'RW_MRC_CACHE' needs update. [DEBUG] MRC: updated 'RW_MRC_CACHE'. [DEBUG] FMAP: area RW_ELOG found @ 1720000 (16384 bytes) [INFO ] ELOG: NV offset 0x1720000 size 0x4000 [ERROR] ELOG: NV Buffer Cleared. consecutive boot | Post FSP-S [DEBUG] FMAP: area RW_MRC_CACHE found @ 1710000 (65536 bytes) [DEBUG] MRC: Checking cached data update for 'RW_MRC_CACHE'. [INFO ] SF: Detected 00 0000 with sector size 0x1000, total 0x4000000 [ERROR] SF size 0x4000000 does not correspond to CONFIG_ROM_SIZE 0x2000000!! [DEBUG] MRC: 'RW_MRC_CACHE' does not need update. ``` -- To view, visit https://review.coreboot.org/c/coreboot/+/77556?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id1e91d25916594f59d1e467a142f5042c6138b51 Gerrit-Change-Number: 77556 Gerrit-PatchSet: 6 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.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-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Sat, 02 Sep 2023 22:21:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: {drivers/intel/fsp2_0, soc/intel}: Rename `SAVE_MRC_AFTER_FSPS` config
by Subrata Banik (Code Review) 03 Sep '23

03 Sep '23

Attention is currently required from: Andrey Petrov, Arthur Heymans, Christian Walter, Johnny Lin, Tim Chu. Subrata Banik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/77616?usp=email ) Change subject: {drivers/intel/fsp2_0, soc/intel}: Rename `SAVE_MRC_AFTER_FSPS` config ...................................................................... {drivers/intel/fsp2_0, soc/intel}: Rename `SAVE_MRC_AFTER_FSPS` config This patch renames `SAVE_MRC_AFTER_FSPS` config to `FSP_NVS_DATA_POST_SILICON_INIT` to highlight the violation in the Xeon SP FSP implementation, where the FSP Silicon Init API produces Non-Volatile Storage (NVS) instead of the FSP-Memory Init API. According to the FSP 2.x specification (section 11.3), the FSP populates the NVS data using the FSP_NON_VOLATILE_STORAGE_HOB and expects the boot firmware to parse the FSP_NON_VOLATILE_STORAGE_HOB after the FspMemoryInit() API in API mode. However, not all Intel SoC platforms that support the FSP 2.x specification adhere to this requirement. For example, the FSP binary for XEON SP platform produces NVS data (aka FSP_NON_VOLATILE_STORAGE_HOB) after the FspSiliconInit() API. Therefore, attempting to locate NVS data after the FspMemoryInit() API on these platforms would result in an error. The `save_mrc_data.c` implementation provides the required hooks to locate the NVS post FSP-Silicon Init and store into Non-Volatile Storage. BUG=b:296704537 TEST=Able to build and boot Intel Xeon SP w/o any functional impact. Change-Id: I815a64263fa1415bfe30bb3c1c35e4adee307e86 Signed-off-by: Subrata Banik <subratabanik(a)google.com> --- M src/drivers/intel/fsp2_0/Kconfig M src/drivers/intel/fsp2_0/save_mrc_data.c M src/soc/intel/xeon_sp/spr/Kconfig 3 files changed, 19 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/16/77616/1 diff --git a/src/drivers/intel/fsp2_0/Kconfig b/src/drivers/intel/fsp2_0/Kconfig index f5399ea..ca17bc7 100644 --- a/src/drivers/intel/fsp2_0/Kconfig +++ b/src/drivers/intel/fsp2_0/Kconfig @@ -410,13 +410,26 @@ coreboot native debug driver when coreboot has integrated the debug FSP binaries. coreboot disables serial messages when this config is not enabled. -config SAVE_MRC_AFTER_FSPS +config FSP_NVS_DATA_POST_SILICON_INIT bool default n - depends on XEON_SP_COMMON_BASE help - Save MRC training data after FSP-S. Select this on platforms that generate MRC - cache HOB data as part of FSP-S rather than FSP-M. + Select this config to enable the workaround for Intel SoC platforms that + do not adhere to the FSP 2.x specification requirement, where the FSP + Silicon Init API produces Non-Volatile Storage (NVS) data instead of the + FSP-Memory Init API. + + According to the FSP 2.x specification (section 11.3), the FSP populates the + NVS data using the FSP_NON_VOLATILE_STORAGE_HOB and expects the boot firmware + to parse the FSP_NON_VOLATILE_STORAGE_HOB after the FspMemoryInit() API in API + mode. + + However, not all Intel SoC platforms that support the FSP 2.x specification + adhere to this requirement. For example, the FSP binary for XEON SP platform + produces NVS data (aka FSP_NON_VOLATILE_STORAGE_HOB) after the FspSiliconInit() + API. Therefore, attempting to locate NVS data after the FspMemoryInit() API on + these platforms would result in an error. Use this config to find the NVS data + and store it in Non-Volatile Storage after the FspSiliconInit() API. config FSP_MULTIPHASE_SI_INIT_RETURN_BROKEN bool diff --git a/src/drivers/intel/fsp2_0/save_mrc_data.c b/src/drivers/intel/fsp2_0/save_mrc_data.c index 19e8a52..1d682b9 100644 --- a/src/drivers/intel/fsp2_0/save_mrc_data.c +++ b/src/drivers/intel/fsp2_0/save_mrc_data.c @@ -46,7 +46,7 @@ * Should be done before ramstage_cse_fw_sync() to avoid traning memory twice on * a cold boot after a full firmware update. */ -#if CONFIG(SAVE_MRC_AFTER_FSPS) +#if CONFIG(FSP_NVS_DATA_POST_SILICON_INIT) BOOT_STATE_INIT_ENTRY(BS_DEV_INIT_CHIPS, BS_ON_EXIT, save_mrc_data, NULL); #else BOOT_STATE_INIT_ENTRY(BS_PRE_DEVICE, BS_ON_ENTRY, save_mrc_data, NULL); diff --git a/src/soc/intel/xeon_sp/spr/Kconfig b/src/soc/intel/xeon_sp/spr/Kconfig index 9c19f3b..dac093f 100644 --- a/src/soc/intel/xeon_sp/spr/Kconfig +++ b/src/soc/intel/xeon_sp/spr/Kconfig @@ -2,8 +2,8 @@ config SOC_INTEL_SAPPHIRERAPIDS_SP bool + select FSP_NVS_DATA_POST_SILICON_INIT select MICROCODE_BLOB_NOT_HOOKED_UP - select SAVE_MRC_AFTER_FSPS select SOC_INTEL_MEM_MAPPED_PM_CONFIGURATION select DISABLE_ACPI_HIBERNATE select DEFAULT_X2APIC_RUNTIME -- To view, visit https://review.coreboot.org/c/coreboot/+/77616?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I815a64263fa1415bfe30bb3c1c35e4adee307e86 Gerrit-Change-Number: 77616 Gerrit-PatchSet: 1 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Christian Walter <christian.walter(a)9elements.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-MessageType: newchange

1 0

[S] Change in coreboot[master]: drivers/intel/fsp2_0: Introduce MRC cache store after FSP-M/S APIs
by Subrata Banik (Code Review) 03 Sep '23

03 Sep '23

Attention is currently required from: Andrey Petrov, Felix Held, Fred Reitberger, Jason Glenesk, Kapil Porwal, Lean Sheng Tan, Martin L Roth, Matt DeVillier, Nico Huber, Paul Menzel, Raul Rangel. Hello Andrey Petrov, Felix Held, Fred Reitberger, Jason Glenesk, Kapil Porwal, Lean Sheng Tan, Martin L Roth, Matt DeVillier, Nico Huber, Raul Rangel, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/77556?usp=email to look at the new patch set (#6). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: drivers/intel/fsp2_0: Introduce MRC cache store after FSP-M/S APIs ...................................................................... drivers/intel/fsp2_0: Introduce MRC cache store after FSP-M/S APIs This patch refactors the existing MRC cache storing logic, which was spread between the ROM and RAM stages, into a single early MRC cache store stage. The only exception is when SoC user selects FSP_NVS_DATA_POST_SILICON_INIT to store MRC cache from ramstage (after FSP-S). It reverts all the boot-state logic previously used to locate and store MRC cache from NVS HOB into NVS because majority of the platform can potentially use the early MRC cache store with improved memory caching at the pre-RAM phase (with the ramtop implementation). The only exception is xeon_sp platform (so far) that locates MRC cache post in FSP-S (at ramstage). Hence, this patch provides an API to in FSP 2.x silicon init code to perform late storing of MRC cache. In majority cases the updated logic, the romstage (post FSP-M) will attempt to save the MRC cache. Platform that selects FSP_NVS_DATA_POST_SILICON_INIT config performs the same operation post FSP-S. Depending on whether the MRC_STASH_TO_CBMEM config is enabled, the MRC cache will either be written directly to NVRAM at the romstage or stashed into CBMEM for a late NVRAM write at ramstage. Below table captures the change in the boot state w/ and w/o this patch for storing the MRC cache. Overall the goal is to ensure the platform behavior is remain unchanged before and after this patch. w/o this patch: | | Save MRC | Finalize | Lock the | | | Cache | MRC Cache | Boot Medium | +-----------+----------------+----------------+----------------+ | MRC_WRITE | BS_OS_RESUME | BS_OS_RESUME | BS_ON_RESUME | | NV_LATE | CHECK_ENTRY | CHECK_ENTRY | CHECK_EXIT | +-----------+----------------+----------------+----------------+ | MRC_STASH | BS_DEV | BS_DEV | BS_DEV | | TO_CBMEM | ENUMERATE_EXIT | ENUMERATE_EXIT | RESOURCES_ENTRY| +-----------+----------------+----------------+----------------+ | FSP_NVS | BS_DEV_INIT | BS_DEV | BS_DEV | | DATA_POST | CHIPS_EXIT | ENUMERATE_EXIT | RESOURCES_ENTRY| | SILICON | | | | | INIT | | | | +-----------+----------------+----------------+----------------+ | Platform | BS_PRE | BS_DEV | BS_DEV | | w/o above | DEVICE_ENTRY | ENUMERATE_EXIT | ENUMERATE_ENTRY| | config | | | | | (FSP 2.0 | | | | | platforms | | | | w/ this patch: | | Save MRC | Finalize | Lock the | | | Cache | MRC Cache | Boot Medium | +-----------+----------------+----------------+----------------+ | MRC_WRITE | BS_OS_RESUME | BS_OS_RESUME | BS_ON_RESUME | | NV_LATE | CHECK_ENTRY | CHECK_ENTRY | CHECK_EXIT | +-----------+----------------+----------------+----------------+ | MRC_STASH | BS_DEV | BS_DEV | BS_DEV | | TO_CBMEM | ENUMERATE_EXIT | ENUMERATE_EXIT | RESOURCES_ENTRY| +-----------+----------------+----------------+----------------+ | FSP_NVS | Post FSP-S | BS_DEV | BS_DEV | | DATA_POST | (ramstage) | ENUMERATE_EXIT | RESOURCES_ENTRY| | SILICON | | | | | INIT | | | | +-----------+----------------+----------------+----------------+ | Platform | Post FSP-M | BS_DEV | BS_DEV | | w/o above | (romstage) | ENUMERATE_EXIT | ENUMERATE_ENTRY| | config | | | | | (FSP 2.0 | | | | | platforms | | | | BUG=b:296704537 TEST=Able to build and boot google/rex without any boot time impact. Change-Id: Id1e91d25916594f59d1e467a142f5042c6138b51 Signed-off-by: Subrata Banik <subratabanik(a)google.com> --- M src/drivers/intel/fsp2_0/Makefile.inc M src/drivers/intel/fsp2_0/memory_init.c M src/drivers/intel/fsp2_0/save_mrc_data.c M src/drivers/intel/fsp2_0/silicon_init.c M src/include/mrc_cache.h 5 files changed, 17 insertions(+), 14 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/56/77556/6 -- To view, visit https://review.coreboot.org/c/coreboot/+/77556?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id1e91d25916594f59d1e467a142f5042c6138b51 Gerrit-Change-Number: 77556 Gerrit-PatchSet: 6 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Martin L Roth <gaumless(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Raul Rangel <rrangel(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Raul Rangel <rrangel(a)chromium.org> Gerrit-Attention: Martin L Roth <gaumless(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

[L] Change in coreboot[master]: mb/x11-lga1151-series: Add x11ssw-f
by Michael Niewöhner (Code Review) 02 Sep '23

02 Sep '23

Attention is currently required from: Kieran Kunhya. Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/77610?usp=email ) Change subject: mb/x11-lga1151-series: Add x11ssw-f ...................................................................... Patch Set 2: (3 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/77610/comment/570c89c4_b066fd5e : PS2, Line 8: Maybe elaborate on the board. Something like `add a new variant X11SSW-F which is similiar to .... differs in ... provides a riser slot and ...`? File Documentation/mainboard/supermicro/x11-lga1151-series/x11ssw-f/x11ssw-f.md: https://review.coreboot.org/c/coreboot/+/77610/comment/9880f705_688c0413 : PS2, Line 22: - Right PCIe slot : - NVMe : any chance you could test these? File src/mainboard/supermicro/x11-lga1151-series/Kconfig: https://review.coreboot.org/c/coreboot/+/77610/comment/1dc88fc9_638a14f0 : PS2, Line 32: X11SSW shouldn't this be X11SSW-F? -- To view, visit https://review.coreboot.org/c/coreboot/+/77610?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I53a0b6012ae64cf1ba4b625f11aaf771637307f3 Gerrit-Change-Number: 77610 Gerrit-PatchSet: 2 Gerrit-Owner: Kieran Kunhya <kieran(a)kunhya.com> Gerrit-Reviewer: Michael Niewöhner <foss(a)mniewoehner.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Kieran Kunhya <kieran(a)kunhya.com> Gerrit-Comment-Date: Sat, 02 Sep 2023 21:15:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit September 2023