coreboot-gerrit February 2024

coreboot-gerrit@coreboot.org

1 participants
3067 discussions

[M] Change in coreboot[main]: lib/romstage_handoff.c: Initialize using a cbmem hook
by Arthur Heymans (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: Andrey Petrov, Angel Pons, Damien Zammit, Felix Held, Huang Jin, Julius Werner, Lee Leahy, Matt DeVillier, Ronak Kanabar. Hello Andrey Petrov, Angel Pons, Damien Zammit, Felix Held, Huang Jin, Lee Leahy, Matt DeVillier, Patrick Rudolph, Ronak Kanabar, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/52933?usp=email to look at the new patch set (#6). Change subject: lib/romstage_handoff.c: Initialize using a cbmem hook ...................................................................... lib/romstage_handoff.c: Initialize using a cbmem hook This reduces platform boilerplate code. Change-Id: I93abf33a6eb055eeaba1fed5042387822de1aa20 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/drivers/amd/agesa/romstage.c M src/drivers/intel/fsp1_1/romstage.c M src/drivers/intel/fsp2_0/memory_init.c M src/include/romstage_handoff.h M src/lib/romstage_handoff.c M src/northbridge/intel/gm45/romstage.c M src/northbridge/intel/haswell/romstage.c M src/northbridge/intel/i945/early_init.c M src/northbridge/intel/ironlake/romstage.c M src/northbridge/intel/pineview/romstage.c M src/northbridge/intel/sandybridge/romstage.c M src/northbridge/intel/x4x/romstage.c M src/soc/amd/stoneyridge/romstage.c M src/soc/intel/baytrail/romstage/romstage.c M src/soc/intel/broadwell/romstage.c 15 files changed, 11 insertions(+), 62 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/33/52933/6 -- To view, visit https://review.coreboot.org/c/coreboot/+/52933?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I93abf33a6eb055eeaba1fed5042387822de1aa20 Gerrit-Change-Number: 52933 Gerrit-PatchSet: 6 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Damien Zammit Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Huang Jin Gerrit-Reviewer: Lee Leahy <leroy.p.leahy(a)intel.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Patrick Rudolph <rudolphpatrick05(a)gmail.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.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-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Damien Zammit Gerrit-Attention: Lee Leahy <leroy.p.leahy(a)intel.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Julius Werner <jwerner(a)chromium.org> Gerrit-Attention: Huang Jin Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in coreboot[main]: drivers/intel/fsp2_0: Add FSP_DOES_NOT_NEED_TEMP_RAM
by Arthur Heymans (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: Andrey Petrov, Chen, Gang C, David Hendricks, Johnny Lin, Lean Sheng Tan, Patrick Rudolph, Ronak Kanabar, Shuo Liu. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80328?usp=email ) Change subject: drivers/intel/fsp2_0: Add FSP_DOES_NOT_NEED_TEMP_RAM ...................................................................... Patch Set 3: (1 comment) File src/drivers/intel/fsp2_0/memory_init.c: https://review.coreboot.org/c/coreboot/+/80328/comment/5c552852_dea5576d : PS1, Line 156: goto temp_ram_assigned; > Yeah, for the RC sample fixes (thanks for providing the inputs :-)), we made careful evaluations. However, due to RC baseline logic changes, it cannot be directly applied to GNR, and also the performance scaling aspects needs to be considered along with the increasing of socket counts. The other related factor is thorough tests needed to be executed on all related configurations, include FSP dispatch mode and UEFI BIOS. Due to these opens, the agendas will be addressed on the next generation. Well you need to synchronize the CAR heaps across sockets. It's not hard to do that for both heaps separately which would have 0 performance impact and should be very straightforward to test: do both coreboot and UEFI still boot. Is avoiding 30 minutes of testing really worth it over the many meetings and email and friction to get workarounds in place at the wrong level (it is breaking the FSP spec!)? Note that this issue and fix was reported for SPR-SP early in the bringup, long before GNR was announced or even worked on. A proper fix was promised for GNR and now you're saying that's not happening either?! The process of working with Intel and fixing issues is really not how it should be (not blaming you ofc, but this issue is frustrating)! Anyway to move forward: let's call it a workaround and make the memory allocations explicit such that it's clear from the code and the comments what is going on. > As to the patch commit message, sure, let me try to add more info around for the ease of technically maintenance. BTW, we could also revise FSP integration guides for related generations to address this usage. Or admit that this is a workaround... Documenting it here in code is good enough IMO. -- To view, visit https://review.coreboot.org/c/coreboot/+/80328?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I17a73e4f627c91808ed17c712b72937662ae9293 Gerrit-Change-Number: 80328 Gerrit-PatchSet: 3 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Comment-Date: Fri, 02 Feb 2024 11:17:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: drivers/intel/fsp2_0: Add FSP_DOES_NOT_NEED_TEMP_RAM
by Shuo Liu (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: Andrey Petrov, Arthur Heymans, Chen, Gang C, David Hendricks, Johnny Lin, Lean Sheng Tan, Patrick Rudolph, Ronak Kanabar. Shuo Liu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80328?usp=email ) Change subject: drivers/intel/fsp2_0: Add FSP_DOES_NOT_NEED_TEMP_RAM ...................................................................... Patch Set 3: (1 comment) File src/drivers/intel/fsp2_0/memory_init.c: https://review.coreboot.org/c/coreboot/+/80328/comment/1b15ecd8_0ed9df17 : PS1, Line 156: goto temp_ram_assigned; > Yeah, for the RC sample fixes (thanks for providing the inputs :-)), we made careful evaluations. […] As to the patch commit message, sure, let me try to add more info around for the ease of technically maintenance. BTW, we could also revise FSP integration guides for related generations to address this usage. -- To view, visit https://review.coreboot.org/c/coreboot/+/80328?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I17a73e4f627c91808ed17c712b72937662ae9293 Gerrit-Change-Number: 80328 Gerrit-PatchSet: 3 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Comment-Date: Fri, 02 Feb 2024 11:04:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: drivers/intel/fsp2_0: Add FSP_DOES_NOT_NEED_TEMP_RAM
by Shuo Liu (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: Andrey Petrov, Arthur Heymans, Chen, Gang C, David Hendricks, Johnny Lin, Lean Sheng Tan, Patrick Rudolph, Ronak Kanabar. Shuo Liu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80328?usp=email ) Change subject: drivers/intel/fsp2_0: Add FSP_DOES_NOT_NEED_TEMP_RAM ...................................................................... Patch Set 3: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/80328/comment/495c7842_dbdfc47c : PS3, Line 11: some > > This change will cover SPR and GNR, and for future generations changes insides FSP will be address […] Good catch, if CPX needs, maybe community could help to test CPX as well? Anyway ... now we are having a plan to cover the issue on generations. -- To view, visit https://review.coreboot.org/c/coreboot/+/80328?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I17a73e4f627c91808ed17c712b72937662ae9293 Gerrit-Change-Number: 80328 Gerrit-PatchSet: 3 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Comment-Date: Fri, 02 Feb 2024 11:02:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com> Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[XS] Change in coreboot[main]: soc/amd/phoenix/Kconfig: add SOC_AMD_PHOENIX_OPENSIL option
by Arthur Heymans (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: Felix Held, Fred Reitberger, Jason Glenesk, Matt DeVillier. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80293?usp=email ) Change subject: soc/amd/phoenix/Kconfig: add SOC_AMD_PHOENIX_OPENSIL option ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/80293?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: If597ff3dc824ce832399d3efde32352b36354b21 Gerrit-Change-Number: 80293 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Reviewer: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 02 Feb 2024 10:59:33 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: drivers/intel/fsp2_0: Add FSP_DOES_NOT_NEED_TEMP_RAM
by Shuo Liu (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: Andrey Petrov, Arthur Heymans, Chen, Gang C, David Hendricks, Johnny Lin, Lean Sheng Tan, Patrick Rudolph, Ronak Kanabar. Shuo Liu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80328?usp=email ) Change subject: drivers/intel/fsp2_0: Add FSP_DOES_NOT_NEED_TEMP_RAM ...................................................................... Patch Set 3: (1 comment) File src/drivers/intel/fsp2_0/memory_init.c: https://review.coreboot.org/c/coreboot/+/80328/comment/5bf4a4cd_e423fce1 : PS1, Line 156: goto temp_ram_assigned; > > For this change, we are not inclined to name it as a workaround. […] Yeah, for the RC sample fixes (thanks for providing the inputs :-)), we made careful evaluations. However, due to RC baseline logic changes, it cannot be directly applied to GNR, and also the performance scaling aspects needs to be considered along with the increasing of socket counts. The other related factor is thorough tests needed to be executed on all related configurations, include FSP dispatch mode and UEFI BIOS. Due to these opens, the agendas will be addressed on the next generation. -- To view, visit https://review.coreboot.org/c/coreboot/+/80328?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I17a73e4f627c91808ed17c712b72937662ae9293 Gerrit-Change-Number: 80328 Gerrit-PatchSet: 3 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Comment-Date: Fri, 02 Feb 2024 10:59:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: vc/amd/opensil/genoa_poc: remove xSIM-api dependency from opensil.h
by Arthur Heymans (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: Felix Held. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80287?usp=email ) Change subject: vc/amd/opensil/genoa_poc: remove xSIM-api dependency from opensil.h ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/80287?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I969bc0862560b7254c48f04e9a03387417f328bc Gerrit-Change-Number: 80287 Gerrit-PatchSet: 1 Gerrit-Owner: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Varshit Pandya <pandyavarshit(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Comment-Date: Fri, 02 Feb 2024 10:57:46 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: drivers/intel/fsp2_0: Add FSP_DOES_NOT_NEED_TEMP_RAM
by Arthur Heymans (Code Review) 02 Feb '24

02 Feb '24

1 0

[S] Change in coreboot[main]: drivers/intel/fsp2_0: Add FSP_DOES_NOT_NEED_TEMP_RAM
by Shuo Liu (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: Andrey Petrov, Chen, Gang C, David Hendricks, Johnny Lin, Lean Sheng Tan, Patrick Rudolph, Ronak Kanabar, Shuo Liu. Shuo Liu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80328?usp=email ) Change subject: drivers/intel/fsp2_0: Add FSP_DOES_NOT_NEED_TEMP_RAM ...................................................................... Patch Set 3: (1 comment) Patchset: PS3: It doesn't surprise me to have many inputs with challenges :-) However, the intention is to have a feasible solution with thorough fixes and upstream maintenance (instead of vendor specific private fixes). Discussions are welcomed. -- To view, visit https://review.coreboot.org/c/coreboot/+/80328?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I17a73e4f627c91808ed17c712b72937662ae9293 Gerrit-Change-Number: 80328 Gerrit-PatchSet: 3 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Comment-Date: Fri, 02 Feb 2024 10:53:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[main]: drivers/intel/fsp2_0: Add FSP_DOES_NOT_NEED_TEMP_RAM
by Arthur Heymans (Code Review) 02 Feb '24

02 Feb '24

Attention is currently required from: Andrey Petrov, Chen, Gang C, David Hendricks, Johnny Lin, Lean Sheng Tan, Patrick Rudolph, Ronak Kanabar, Shuo Liu. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/80328?usp=email ) Change subject: drivers/intel/fsp2_0: Add FSP_DOES_NOT_NEED_TEMP_RAM ...................................................................... Patch Set 3: (1 comment) File src/drivers/intel/fsp2_0/memory_init.c: https://review.coreboot.org/c/coreboot/+/80328/comment/a892988a_bb03c3b2 : PS1, Line 156: goto temp_ram_assigned; > For this change, we are not inclined to name it as a workaround. Since the static allocation is reflected into UPD's default value, and it is an existing practice for coreboot to use UPD's default values, e.g. https://github.com/coreboot/coreboot/blob/main/src/drivers/intel/fsp2_0/mem…. Sorry but it really is a workaround as the flaw could just be fixed in FSP and we even send a ~5LOC fix and it's against the FSP spec. It's ok to use default FSP values, but I don't think memory allocations fall in that category 'leave it default': You want to be explicit about those, as overlap results in weird problems. Btw https://review.coreboot.org/c/coreboot/+/61164 achieves the same. -- To view, visit https://review.coreboot.org/c/coreboot/+/80328?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I17a73e4f627c91808ed17c712b72937662ae9293 Gerrit-Change-Number: 80328 Gerrit-PatchSet: 3 Gerrit-Owner: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Chen, Gang C <gang.c.chen(a)intel.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Comment-Date: Fri, 02 Feb 2024 10:50:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Shuo Liu <shuo.liu(a)intel.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit February 2024