coreboot-gerrit April 2023

coreboot-gerrit@coreboot.org

1 participants
5761 discussions

[S] Change in coreboot[master]: soc/intel/cmn/cse: Make `cse_get_fpt_partition_info()` function static
by Arthur Heymans (Code Review) 15 Apr '23

15 Apr '23

Attention is currently required from: Tarun Tuli, SRIDHAR SIRICILLA, Subrata Banik, Kapil Porwal, Dinesh Gehlot. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74388 ) Change subject: soc/intel/cmn/cse: Make `cse_get_fpt_partition_info()` function static ...................................................................... Patch Set 5: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/74388 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ie50453946c8abe55c29e9001263f0264a73c8fac Gerrit-Change-Number: 74388 Gerrit-PatchSet: 5 Gerrit-Owner: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Kangheui Won <khwon(a)chromium.org> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: SRIDHAR SIRICILLA Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: SRIDHAR SIRICILLA Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Comment-Date: Sat, 15 Apr 2023 09:33:44 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: soc/intel/cmn/cse: Store ISH firmware version into CBMEM
by Arthur Heymans (Code Review) 15 Apr '23

15 Apr '23

1 0

[M] Change in coreboot[master]: {commonlib, soc/intel/cmn/cse}: Store CSE firmware version into CBMEM
by Arthur Heymans (Code Review) 15 Apr '23

15 Apr '23

Attention is currently required from: Tarun Tuli, Subrata Banik, Dinesh Gehlot, Sridhar Siricilla. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74256 ) Change subject: {commonlib, soc/intel/cmn/cse}: Store CSE firmware version into CBMEM ...................................................................... Patch Set 16: Code-Review-1 (3 comments) File src/soc/intel/common/block/cse/cse_lite.c: https://review.coreboot.org/c/coreboot/+/74256/comment/20ff3711_fbc6e3fd PS16, Line 1154: : static void cse_copy_fw_version(struct fw_version *dst, const struct fw_version *src) : { : if (src == NULL || dst == NULL) { : printk(BIOS_WARNING, "CSE: NULL value pointer provided for fw version copy.\n"); : return; : } : : dst->major = src->major; : dst->minor = src->minor; : dst->hotfix = src->hotfix; : dst->build = src->build; : } Just use memcpy... https://review.coreboot.org/c/coreboot/+/74256/comment/f471715c_931852cf PS16, Line 1250: if (CONFIG(SOC_INTEL_CSE_LITE_SYNC_IN_RAMSTAGE) && !s3wake) { : timestamp_add_now(TS_CSE_FW_SYNC_START); : cse_fw_sync(); : timestamp_add_now(TS_CSE_FW_SYNC_END); : : if (CONFIG(SOC_INTEL_STORE_CSE_FPT_PARTITION_VERSION)) : store_cse_rw_fw_version(); : } : } The codeflow is quite confusing... Can't you add store_cse_rw_fw_version in the same function call? Those functions for SOC_INTEL_CSE_LITE_SYNC_IN_RAMSTAGE and SOC_INTEL_CSE_LITE_SYNC_IN_ROMSTAGE look close to identical https://review.coreboot.org/c/coreboot/+/74256/comment/3e41437f_ecb3ce25 PS16, Line 1262: bool s3wake; : s3wake = acpi_get_sleep_type() == ACPI_S3; : : if (s3wake) : return; if (acpi_get_sleep_type() == ACPI_S3) return; -- To view, visit https://review.coreboot.org/c/coreboot/+/74256 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I923049d2f1f589f87e1a29e1ac94af7f5fccc2c8 Gerrit-Change-Number: 74256 Gerrit-PatchSet: 16 Gerrit-Owner: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Kangheui Won <khwon(a)chromium.org> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Comment-Date: Sat, 15 Apr 2023 09:29:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: drivers/intel/ish: Hook get ISH version into `.final`
by Arthur Heymans (Code Review) 15 Apr '23

15 Apr '23

Attention is currently required from: Tarun Tuli, Subrata Banik, Dinesh Gehlot, Paul Menzel, Sridhar Siricilla. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74209 ) Change subject: drivers/intel/ish: Hook get ISH version into `.final` ...................................................................... Patch Set 17: (1 comment) File src/drivers/intel/ish/ish.c: https://review.coreboot.org/c/coreboot/+/74209/comment/79aee8a9_9b984cfc PS17, Line 53: struct cse_fw_partition_info *version = cbmem_find(CBMEM_ID_CSE_PARTITION_VERSION); : if (version == NULL) : return; : : printk(BIOS_DEBUG, "ISH version: %d.%d.%d.%d\n", : version->ish_partition_info.cur_ish_fw_version.major, : version->ish_partition_info.cur_ish_fw_version.minor, : version->ish_partition_info.cur_ish_fw_version.hotfix, : version->ish_partition_info.cur_ish_fw_version.build); > I'm confused. Is finding the ish_partition_info ever not done in ramstage? Why do you need to save in cbmem and not in a local data structure (static global variable) with functions to report it? NVM I think I missed the assumption that cbmem can be recovered on warm reset. -- To view, visit https://review.coreboot.org/c/coreboot/+/74209 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib3f983d5de5b169474bcdb1e9e2934174a9dadf8 Gerrit-Change-Number: 74209 Gerrit-PatchSet: 17 Gerrit-Owner: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Kangheui Won <khwon(a)chromium.org> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-CC: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Comment-Date: Sat, 15 Apr 2023 09:20:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: drivers/intel/ish: Hook get ISH version into `.final`
by Arthur Heymans (Code Review) 15 Apr '23

15 Apr '23

Attention is currently required from: Tarun Tuli, Subrata Banik, Dinesh Gehlot, Paul Menzel, Sridhar Siricilla. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74209 ) Change subject: drivers/intel/ish: Hook get ISH version into `.final` ...................................................................... Patch Set 17: (1 comment) File src/drivers/intel/ish/ish.c: https://review.coreboot.org/c/coreboot/+/74209/comment/bf017294_0f337148 PS17, Line 53: struct cse_fw_partition_info *version = cbmem_find(CBMEM_ID_CSE_PARTITION_VERSION); : if (version == NULL) : return; : : printk(BIOS_DEBUG, "ISH version: %d.%d.%d.%d\n", : version->ish_partition_info.cur_ish_fw_version.major, : version->ish_partition_info.cur_ish_fw_version.minor, : version->ish_partition_info.cur_ish_fw_version.hotfix, : version->ish_partition_info.cur_ish_fw_version.build); I'm confused. Is finding the ish_partition_info ever not done in ramstage? Why do you need to save in cbmem and not in a local data structure (static global variable) with functions to report it? -- To view, visit https://review.coreboot.org/c/coreboot/+/74209 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib3f983d5de5b169474bcdb1e9e2934174a9dadf8 Gerrit-Change-Number: 74209 Gerrit-PatchSet: 17 Gerrit-Owner: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Kangheui Won <khwon(a)chromium.org> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-CC: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Comment-Date: Sat, 15 Apr 2023 09:19:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: soc/intel/cmd/block/cse: Add config option for storing fw version info
by Subrata Banik (Code Review) 15 Apr '23

15 Apr '23

Attention is currently required from: Paul Menzel, Dinesh Gehlot, Arthur Heymans. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74255 ) Change subject: soc/intel/cmd/block/cse: Add config option for storing fw version info ...................................................................... Patch Set 15: (1 comment) Patchset: PS15: > Can you squash the commit adding a Kconfig with the one doing something with it? i believe you are suggesting to add this kconfig with CB:74256 -- To view, visit https://review.coreboot.org/c/coreboot/+/74255 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78fef45fd2940536b3e91cfd4d184b7635238499 Gerrit-Change-Number: 74255 Gerrit-PatchSet: 15 Gerrit-Owner: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Kangheui Won <khwon(a)chromium.org> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Comment-Date: Sat, 15 Apr 2023 09:11:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: soc/intel/cmd/block/cse: Add config option for storing fw version info
by Arthur Heymans (Code Review) 15 Apr '23

15 Apr '23

Attention is currently required from: Subrata Banik, Paul Menzel, Dinesh Gehlot. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74255 ) Change subject: soc/intel/cmd/block/cse: Add config option for storing fw version info ...................................................................... Patch Set 15: (1 comment) Patchset: PS15: Can you squash the commit adding a Kconfig with the one doing something with it? -- To view, visit https://review.coreboot.org/c/coreboot/+/74255 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I78fef45fd2940536b3e91cfd4d184b7635238499 Gerrit-Change-Number: 74255 Gerrit-PatchSet: 15 Gerrit-Owner: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Kangheui Won <khwon(a)chromium.org> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Comment-Date: Sat, 15 Apr 2023 09:08:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: soc/intel/baytrail: Make acpi_madt_irq_overrides() static
by Nico Huber (Code Review) 15 Apr '23

15 Apr '23

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74325 ) Change subject: soc/intel/baytrail: Make acpi_madt_irq_overrides() static ...................................................................... Patch Set 4: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/74325/comment/bc9da16a_6dd60f9a PS2, Line 7: Move a function inside a file > Done Thanks -- To view, visit https://review.coreboot.org/c/coreboot/+/74325 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id362e023358054df2c4511fd108c313da868306d Gerrit-Change-Number: 74325 Gerrit-PatchSet: 4 Gerrit-Owner: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(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: Sat, 15 Apr 2023 09:02:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: soc/intel/xeon_sp: Cache DRAM with TSEG for FSP-S execution time
by Patrick Rudolph (Code Review) 15 Apr '23

15 Apr '23

Attention is currently required from: Johnny Lin, Paul Menzel, Christian Walter, Arthur Heymans, Shuming Chu (Shuming). Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/71085 ) Change subject: soc/intel/xeon_sp: Cache DRAM with TSEG for FSP-S execution time ...................................................................... Patch Set 6: Code-Review-1 (1 comment) Patchset: PS6: > > I'm not sure if this is still needed. Archer City 2S boots quite fast. […] Boot time on our Archer City 2S is about 2 minutes. **Without this commit:** Timestamp - calling FspSiliconInit: 50750376 usec POST: 0x93 FSPS returned 0 Timestamp - returning from FspSiliconInit: 58548788 usec **With this commit:** Timestamp - calling FspSiliconInit: 51050834 usec POST: 0x93 FSPS returned 0 Timestamp - returning from FspSiliconInit: 58782118 usec -- To view, visit https://review.coreboot.org/c/coreboot/+/71085 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib886eda81566b491325e8cd65c9dfb44c89977c7 Gerrit-Change-Number: 71085 Gerrit-PatchSet: 6 Gerrit-Owner: Shuming Chu (Shuming) <s1218944(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: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-CC: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Shuming Chu (Shuming) <s1218944(a)gmail.com> Gerrit-Comment-Date: Sat, 15 Apr 2023 07:48:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com> Comment-In-Reply-To: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: soc/intel/xeon_sp: Cache DRAM with TSEG for FSP-S execution time
by Arthur Heymans (Code Review) 15 Apr '23

15 Apr '23

Attention is currently required from: Patrick Rudolph, Johnny Lin, Paul Menzel, Christian Walter, Shuming Chu (Shuming). Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/71085 ) Change subject: soc/intel/xeon_sp: Cache DRAM with TSEG for FSP-S execution time ...................................................................... Patch Set 6: Code-Review-1 (1 comment) Patchset: PS6: > I'm not sure if this is still needed. Archer City 2S boots quite fast. > Will do some benchmarks here. Hmm I think this was necessary when FSP-M TempMemExit was unused because FSP-M added over a 100M of data in memory. Now TempMemExit is used and therefore this code is unnecessary and unused in postcar. -- To view, visit https://review.coreboot.org/c/coreboot/+/71085 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib886eda81566b491325e8cd65c9dfb44c89977c7 Gerrit-Change-Number: 71085 Gerrit-PatchSet: 6 Gerrit-Owner: Shuming Chu (Shuming) <s1218944(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: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Reviewer: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-CC: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-CC: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Shuming Chu (Shuming) <s1218944(a)gmail.com> Gerrit-Comment-Date: Sat, 15 Apr 2023 06:25:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: comment

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2023