coreboot-gerrit April 2023

coreboot-gerrit@coreboot.org

1 participants
5761 discussions

[M] Change in coreboot[master]: cpu/x86/mp_init.c: Keep track of initial lapic ID inside device_path
by Patrick Rudolph (Code Review) 05 Apr '23

05 Apr '23

Attention is currently required from: Arthur Heymans, Tarun Tuli, Subrata Banik, Patrick Rudolph, Kapil Porwal, Kyösti Mälkki. Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/64342 ) Change subject: cpu/x86/mp_init.c: Keep track of initial lapic ID inside device_path ...................................................................... Patch Set 24: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/64342 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I4d9f8d23c0b0e5c142f6907593428d8509e4e7bb Gerrit-Change-Number: 64342 Gerrit-PatchSet: 24 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Maximilian Brune <maximilian.brune(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> 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-CC: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-CC: Benjamin Doron <benjamin.doron00(a)gmail.com> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Arthur Heymans <arthur.heymans(a)9elements.com> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Comment-Date: Wed, 05 Apr 2023 07:41:18 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: soc/intel/cmn: Implement API to retrieve firmware partition information
by Subrata Banik (Code Review) 05 Apr '23

05 Apr '23

Attention is currently required from: Dinesh Gehlot, Kapil Porwal. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74005 ) Change subject: soc/intel/cmn: Implement API to retrieve firmware partition information ...................................................................... Patch Set 14: (3 comments) File src/soc/intel/common/block/cse/cse.c: https://review.coreboot.org/c/coreboot/+/74005/comment/003f3ef6_ff608cf9 PS14, Line 1216: cse_get_partition_info please remain to `send_get_boot_partition_info_cmd` https://review.coreboot.org/c/coreboot/+/74005/comment/512c90ca_2e166836 PS14, Line 1216: bool static bool https://review.coreboot.org/c/coreboot/+/74005/comment/9ab8082f_e9759ba8 PS14, Line 1239: We should add a check to avoid sending ISHC partition info if platform doesn't have ISH partition itself ``` bool cse_get_partition_info(enum fpt_partition_id id, struct fw_version_resp *resp) { if (id == FPT_PARTITION_NAME_ISHC && !CONFIG(DRIVERS_INTEL_ISH)) return false; return send_get_boot_partition_info_cmd(id, resp); } ``` -- To view, visit https://review.coreboot.org/c/coreboot/+/74005 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0582010bbb836bd4734f843a8c74dee49d203fd8 Gerrit-Change-Number: 74005 Gerrit-PatchSet: 14 Gerrit-Owner: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Dinesh Gehlot <digehlot(a)google.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Comment-Date: Wed, 05 Apr 2023 07:33:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: mainboard/google/rex/Kconfig: Enable EC boot timestamps for mtl-rex
by Subrata Banik (Code Review) 05 Apr '23

05 Apr '23

Attention is currently required from: Bora Guvendik, Hannah Williams, Tarun Tuli, Jamie Ryu, Wonkyu Kim, Kapil Porwal, Jay Patel. Subrata Banik has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/73192 ) Change subject: mainboard/google/rex/Kconfig: Enable EC boot timestamps for mtl-rex ...................................................................... Patch Set 6: (1 comment) Commit Message: https://review.coreboot.org/c/coreboot/+/73192/comment/aeaf271e_fb0d6cc3 PS6, Line 13: cbmem -t please share the boot_perf revised numbers w/ and w/o ur cl. -- To view, visit https://review.coreboot.org/c/coreboot/+/73192 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Id0b773fb4c7abfcb3c3ab061d970327987536fb2 Gerrit-Change-Number: 73192 Gerrit-PatchSet: 6 Gerrit-Owner: Jay Patel <jay2.patel(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Tarun Tuli <taruntuli(a)google.com> Gerrit-Reviewer: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Hannah Williams <hannah.williams(a)intel.com> Gerrit-Attention: Tarun Tuli <taruntuli(a)google.com> Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Wonkyu Kim <wonkyu.kim(a)intel.com> Gerrit-Attention: Kapil Porwal <kapilporwal(a)google.com> Gerrit-Attention: Jay Patel <jay2.patel(a)intel.com> Gerrit-Comment-Date: Wed, 05 Apr 2023 07:28:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: soc/intel/xeon_sp/spr: Default to X2APIC support
by Johnny Lin (Code Review) 05 Apr '23

05 Apr '23

Attention is currently required from: Patrick Rudolph, Jonathan Zhang, Ziang Wang, Christian Walter, Angel Pons, Shuo Liu, Nill Ge, TangYiwei, Tim Chu. Johnny Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74184 ) Change subject: soc/intel/xeon_sp/spr: Default to X2APIC support ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > I don't see why the mainboard code should select XAPIC_ONLY. […] Oh I see, XAPIC_ONLY is a Kconfig choice entry which cannot be enabled by 'select'. I can enable XAPIC_ONLY from defconfig. Thanks. -- To view, visit https://review.coreboot.org/c/coreboot/+/74184 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3e84cfbd2a7f05b142dc4d782764edce81646c8a Gerrit-Change-Number: 74184 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(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: 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: Ziang Wang <ziang.wang(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Ziang Wang <ziang.wang(a)intel.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Nill Ge <geshijian(a)bytedance.com> Gerrit-Attention: TangYiwei Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Wed, 05 Apr 2023 07:28:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com> Comment-In-Reply-To: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-MessageType: comment

1 0

[S] Change in coreboot[master]: soc/intel/xeon_sp/spr: Default to X2APIC support
by Patrick Rudolph (Code Review) 05 Apr '23

05 Apr '23

Attention is currently required from: Jonathan Zhang, Johnny Lin, Ziang Wang, Christian Walter, Angel Pons, Shuo Liu, Nill Ge, TangYiwei, Tim Chu. Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/74184 ) Change subject: soc/intel/xeon_sp/spr: Default to X2APIC support ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > True, if my linuxboot kernel supports X2APIC it can boot up fine. […] I don't see why the mainboard code should select XAPIC_ONLY. You can add it to your defconfig and build coreboot in XAPIC_ONLY mode. -- To view, visit https://review.coreboot.org/c/coreboot/+/74184 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I3e84cfbd2a7f05b142dc4d782764edce81646c8a Gerrit-Change-Number: 74184 Gerrit-PatchSet: 1 Gerrit-Owner: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Angel Pons <th3fanbus(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: 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: Ziang Wang <ziang.wang(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Lean Sheng Tan <sheng.tan(a)9elements.com> Gerrit-Attention: Jonathan Zhang <jon.zhixiong.zhang(a)gmail.com> Gerrit-Attention: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-Attention: Ziang Wang <ziang.wang(a)intel.com> Gerrit-Attention: Christian Walter <christian.walter(a)9elements.com> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Shuo Liu <shuo.liu(a)intel.com> Gerrit-Attention: Nill Ge <geshijian(a)bytedance.com> Gerrit-Attention: TangYiwei Gerrit-Attention: Tim Chu <Tim.Chu(a)quantatw.com> Gerrit-Comment-Date: Wed, 05 Apr 2023 06:45:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com> Comment-In-Reply-To: Johnny Lin <Johnny_Lin(a)wiwynn.com> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: soc/intel/common: Order the different types of cores based on APIC IDs
by Sridhar Siricilla (Code Review) 05 Apr '23

05 Apr '23

Attention is currently required from: Subrata Banik, Paul Menzel, Kyösti Mälkki, Elyes Haouas. Sridhar Siricilla has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/72132 ) Change subject: soc/intel/common: Order the different types of cores based on APIC IDs ...................................................................... Patch Set 16: (4 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/72132/comment/b2cdda9e_d2349f03 PS15, Line 7: Add code to > Redundant as patches are about code. Ack https://review.coreboot.org/c/coreboot/+/72132/comment/3c5601cc_037b0f8d PS15, Line 11: for c in {0..7}; > Please add a blank line above. Ack https://review.coreboot.org/c/coreboot/+/72132/comment/32344539_bda7b3c4 PS15, Line 11: for c in {0..7}; : do : cat /sys/devices/system/cpu/cpu$c/topology/thread_siblings_list; : done > Shorter: […] Ack https://review.coreboot.org/c/coreboot/+/72132/comment/b3034c6e_86acaeea PS15, Line 11: for c in {0..7}; : do : cat /sys/devices/system/cpu/cpu$c/topology/thread_siblings_list; : done : 0-1 : 4 : 5 : 6 : 7 : 0-1 : 2-3 : 2-3 > You could use Markdown formatting, and indent code blocks by four spaces. Ack -- To view, visit https://review.coreboot.org/c/coreboot/+/72132 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I21487a5eb0439ea0cb5976787d1769ee94777469 Gerrit-Change-Number: 72132 Gerrit-PatchSet: 16 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Sukumar Ghorai <sukumar.ghorai(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jan Samek <jan.samek(a)siemens.com> 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: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Comment-Date: Wed, 05 Apr 2023 06:06:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: soc/intel/common: Order the different types of cores based on APIC IDs
by build bot (Jenkins) (Code Review) 05 Apr '23

05 Apr '23

Attention is currently required from: Subrata Banik, Sridhar Siricilla, Kyösti Mälkki, Elyes Haouas. build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/72132 ) Change subject: soc/intel/common: Order the different types of cores based on APIC IDs ...................................................................... Patch Set 16: (1 comment) Commit Message: Robot Comment from checkpatch (run ID jenkins-coreboot-checkpatch-173380): https://review.coreboot.org/c/coreboot/+/72132/comment/c476c28b_9187224e PS16, Line 29: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/3963893 Possible unwrapped commit description (prefer a maximum 72 chars per line) -- To view, visit https://review.coreboot.org/c/coreboot/+/72132 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I21487a5eb0439ea0cb5976787d1769ee94777469 Gerrit-Change-Number: 72132 Gerrit-PatchSet: 16 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Sukumar Ghorai <sukumar.ghorai(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jan Samek <jan.samek(a)siemens.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Comment-Date: Wed, 05 Apr 2023 05:06:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in coreboot[master]: soc/intel/common: Order the different types of cores based on APIC IDs
by Sridhar Siricilla (Code Review) 05 Apr '23

05 Apr '23

Attention is currently required from: Subrata Banik, Sridhar Siricilla, Kyösti Mälkki, Elyes Haouas. Hello build bot (Jenkins), Subrata Banik, Ronak Kanabar, Sukumar Ghorai, Kyösti Mälkki, Elyes Haouas, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/72132 to look at the new patch set (#16). Change subject: soc/intel/common: Order the different types of cores based on APIC IDs ...................................................................... soc/intel/common: Order the different types of cores based on APIC IDs Currently coreboot presents the BSP core first, then efficient cores and Performance cores as indicated below: ``` grep . cat /sys/devices/system/cpu/cpu$c/topology/thread_siblings_list; 0-1 4 5 6 7 0-1 2-3 2-3 ``` Existing code presents mix of different cores to OS and causes CPU load balancing and power/performance impact. So, the patch fixes this disorder by ordering the Performance cores first, compute die efficient cores next, and finally SOC efficient cores if they are present. This is done to run the media applications in a power efficient manner, please refer thae ChromeOS patch for details: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/3963893 BUG=b:262886449 TEST=Verified the code on Rex system After the fix: ``` grep . cat /sys/devices/system/cpu/cpu$c/topology/thread_siblings_list; 0-1 0-1 2-3 2-3 4 5 6 7 ``` Change-Id: I21487a5eb0439ea0cb5976787d1769ee94777469 Signed-off-by: Sridhar Siricilla <sridhar.siricilla(a)intel.com> --- M src/soc/intel/common/block/acpi/acpi.c M src/soc/intel/common/block/acpi/cpu_hybrid.c M src/soc/intel/common/block/include/intelblocks/acpi.h 3 files changed, 89 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/32/72132/16 -- To view, visit https://review.coreboot.org/c/coreboot/+/72132 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I21487a5eb0439ea0cb5976787d1769ee94777469 Gerrit-Change-Number: 72132 Gerrit-PatchSet: 16 Gerrit-Owner: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Sukumar Ghorai <sukumar.ghorai(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Jan Samek <jan.samek(a)siemens.com> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Sridhar Siricilla <sridhar.siricilla(a)intel.com> Gerrit-Attention: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-MessageType: newpatchset

1 0

[M] Change in coreboot[master]: Revert "soc/intel/cmn/cse: Handle EOP completion asynchronously"
by Subrata Banik (Code Review) 05 Apr '23

05 Apr '23

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/74207 ) Change subject: Revert "soc/intel/cmn/cse: Handle EOP completion asynchronously" ...................................................................... Revert "soc/intel/cmn/cse: Handle EOP completion asynchronously" This reverts commit e7a1204f26fe3628de99b4ab4e3f32916565b95c. This initial change was causing a boot failure when transitioning into recovery mode. BUG=b:276927816 TEST='emerge-brya coreboot chromeos-bootimage', flash and boot a skolas SKU1 to kernel, then press Esc-Refresh-PowerButton to try to reboot into recovery mode. Change-Id: Ibebb20a000a239c344af1c96b8d376352b9c774e Signed-off-by: Nick Vaccaro <nvaccaro(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/74207 Reviewed-by: Tarun Tuli <taruntuli(a)google.com> Reviewed-by: Subrata Banik <subratabanik(a)google.com> Reviewed-by: Eran Mitrani <mitrani(a)google.com> Reviewed-by: Kapil Porwal <kapilporwal(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/soc/intel/common/block/cse/Kconfig M src/soc/intel/common/block/cse/cse.c M src/soc/intel/common/block/cse/cse_eop.c 3 files changed, 61 insertions(+), 108 deletions(-) Approvals: build bot (Jenkins): Verified Subrata Banik: Looks good to me, approved Eran Mitrani: Looks good to me, approved Tarun Tuli: Looks good to me, approved Kapil Porwal: Looks good to me, approved diff --git a/src/soc/intel/common/block/cse/Kconfig b/src/soc/intel/common/block/cse/Kconfig index 33d703f..f1f53d6 100644 --- a/src/soc/intel/common/block/cse/Kconfig +++ b/src/soc/intel/common/block/cse/Kconfig @@ -67,25 +67,6 @@ Starting with Jasper Lake, coreboot sends EOP before loading payload hence, this config is applicable for those platforms. -config SOC_INTEL_CSE_SEND_EOP_ASYNC - bool - depends on SOC_INTEL_COMMON_BLOCK_CSE - depends on !SOC_INTEL_CSE_SEND_EOP_LATE - depends on !SOC_INTEL_CSE_SEND_EOP_EARLY - help - Use this config to handle End Of Post (EOP) completion - asynchronously. The EOP command is sent first and the result - is checked later leaving time to CSE to complete the - operation while coreboot perform other activities. - Performing EOP asynchronously reduces the time spent - actively waiting for command completion which can have a - significant impact on boot time. - - Using this asynchronous approach comes with the limitation - that no HECI command should be sent between the time the EOP - request is posted (at CSE .final device operation) and the - time coreboot check for its completion (BS_PAYLOAD_LOAD). - config SOC_INTEL_CSE_LITE_SKU bool default n diff --git a/src/soc/intel/common/block/cse/cse.c b/src/soc/intel/common/block/cse/cse.c index 6c4bb73..9902094 100644 --- a/src/soc/intel/common/block/cse/cse.c +++ b/src/soc/intel/common/block/cse/cse.c @@ -1407,8 +1407,7 @@ */ void cse_late_finalize(void) { - if (!CONFIG(SOC_INTEL_CSE_SEND_EOP_LATE) && - !CONFIG(SOC_INTEL_CSE_SEND_EOP_ASYNC)) + if (!CONFIG(SOC_INTEL_CSE_SEND_EOP_LATE)) return; if (!CONFIG(USE_FSP_NOTIFY_PHASE_READY_TO_BOOT)) @@ -1432,14 +1431,6 @@ if (CONFIG(SOC_INTEL_CSE_SET_EOP)) cse_send_end_of_post(); - /* - * In asynchronous mode, the EOP command has most likely not been - * completed yet. Finalization steps will be run once the EOP command - * has successfully been completed. - */ - if (CONFIG(SOC_INTEL_CSE_SEND_EOP_ASYNC)) - return; - if (!CONFIG(USE_FSP_NOTIFY_PHASE_READY_TO_BOOT)) cse_final_ready_to_boot(); diff --git a/src/soc/intel/common/block/cse/cse_eop.c b/src/soc/intel/common/block/cse/cse_eop.c index 16cf523..53a4fa5 100644 --- a/src/soc/intel/common/block/cse/cse_eop.c +++ b/src/soc/intel/common/block/cse/cse_eop.c @@ -67,54 +67,13 @@ return CSE_CMD_RESULT_SUCCESS; } -static enum cse_cmd_result cse_receive_eop(void) +static enum cse_cmd_result cse_send_eop(void) { + enum cse_tx_rx_status ret; enum { EOP_REQUESTED_ACTION_CONTINUE = 0, EOP_REQUESTED_ACTION_GLOBAL_RESET = 1, }; - enum cse_tx_rx_status ret; - struct end_of_post_resp { - struct mkhi_hdr hdr; - uint32_t requested_actions; - } __packed resp = {}; - size_t resp_size = sizeof(resp); - - ret = heci_receive(&resp, &resp_size); - if (ret) - return decode_heci_send_receive_error(ret); - - if (resp.hdr.group_id != MKHI_GROUP_ID_GEN || - resp.hdr.command != MKHI_END_OF_POST) { - printk(BIOS_ERR, "HECI: EOP Unexpected response group or command.\n"); - if (CONFIG(SOC_INTEL_CSE_SEND_EOP_ASYNC)) - printk(BIOS_ERR, "HECI: It could be a HECI command conflict.\n"); - return CSE_CMD_RESULT_ERROR; - } - - if (resp.hdr.result) { - printk(BIOS_ERR, "HECI: EOP Resp Failed: %u\n", resp.hdr.result); - return CSE_CMD_RESULT_ERROR; - } - - printk(BIOS_INFO, "CSE: EOP requested action: "); - - switch (resp.requested_actions) { - case EOP_REQUESTED_ACTION_GLOBAL_RESET: - printk(BIOS_INFO, "global reset\n"); - return CSE_CMD_RESULT_GLOBAL_RESET_REQUESTED; - case EOP_REQUESTED_ACTION_CONTINUE: - printk(BIOS_INFO, "continue boot\n"); - return CSE_CMD_RESULT_SUCCESS; - default: - printk(BIOS_INFO, "unknown %u\n", resp.requested_actions); - return CSE_CMD_RESULT_ERROR; - } -} - -static enum cse_cmd_result cse_send_eop(void) -{ - enum cse_tx_rx_status ret; struct end_of_post_msg { struct mkhi_hdr hdr; } __packed msg = { @@ -123,6 +82,11 @@ .command = MKHI_END_OF_POST, }, }; + struct end_of_post_resp { + struct mkhi_hdr hdr; + uint32_t requested_actions; + } __packed resp = {}; + size_t resp_size = sizeof(resp); /* For a CSE-Lite SKU, if the CSE is running RO FW and the board is running vboot in recovery mode, the CSE is expected to be in SOFT @@ -155,22 +119,28 @@ printk(BIOS_INFO, "HECI: Sending End-of-Post\n"); - ret = heci_send(&msg, sizeof(msg), BIOS_HOST_ADDR, HECI_MKHI_ADDR); + ret = heci_send_receive(&msg, sizeof(msg), &resp, &resp_size, HECI_MKHI_ADDR); if (ret) return decode_heci_send_receive_error(ret); - return CSE_CMD_RESULT_SUCCESS; -} + if (resp.hdr.result) { + printk(BIOS_ERR, "HECI: EOP Resp Failed: %u\n", resp.hdr.result); + return CSE_CMD_RESULT_ERROR; + } -static enum cse_cmd_result cse_send_and_receive_eop(void) -{ - enum cse_cmd_result ret; + printk(BIOS_INFO, "CSE: EOP requested action: "); - ret = cse_send_eop(); - if (ret != CSE_CMD_RESULT_SUCCESS) - return ret; - - return cse_receive_eop(); + switch (resp.requested_actions) { + case EOP_REQUESTED_ACTION_GLOBAL_RESET: + printk(BIOS_INFO, "global reset\n"); + return CSE_CMD_RESULT_GLOBAL_RESET_REQUESTED; + case EOP_REQUESTED_ACTION_CONTINUE: + printk(BIOS_INFO, "continue boot\n"); + return CSE_CMD_RESULT_SUCCESS; + default: + printk(BIOS_INFO, "unknown %u\n", resp.requested_actions); + return CSE_CMD_RESULT_ERROR; + } } static enum cse_cmd_result cse_send_cmd_retries(enum cse_cmd_result (*cse_send_command)(void)) @@ -229,12 +199,11 @@ } } -static void do_send_end_of_post(bool wait_for_completion) +static void do_send_end_of_post(void) { - static bool eop_sent = false, eop_complete = false; - enum cse_cmd_result ret; + static bool eop_sent = false; - if (eop_complete) { + if (eop_sent) { printk(BIOS_WARNING, "EOP already sent\n"); return; } @@ -253,40 +222,26 @@ return; } - if (!eop_sent) { - set_cse_device_state(PCH_DEVFN_CSE, DEV_ACTIVE); - timestamp_add_now(TS_ME_END_OF_POST_START); - ret = cse_send_cmd_retries(cse_send_eop); - if (ret == CSE_CMD_RESULT_SUCCESS) - eop_sent = true; - } - - if (!wait_for_completion) - return; - set_cse_device_state(PCH_DEVFN_CSE, DEV_ACTIVE); - ret = cse_receive_eop(); - if (ret != CSE_CMD_RESULT_SUCCESS) { - ret = cse_send_cmd_retries(cse_send_and_receive_eop); - handle_cse_eop_result(ret); - } + + timestamp_add_now(TS_ME_END_OF_POST_START); + handle_cse_eop_result(cse_send_cmd_retries(cse_send_eop)); timestamp_add_now(TS_ME_END_OF_POST_END); set_cse_device_state(PCH_DEVFN_CSE, DEV_IDLE); - eop_complete = true; + eop_sent = true; } void cse_send_end_of_post(void) { - return do_send_end_of_post(!CONFIG(SOC_INTEL_CSE_SEND_EOP_ASYNC)); + return do_send_end_of_post(); } static void send_cse_eop_with_late_finalize(void *unused) { - do_send_end_of_post(true); - if (CONFIG(SOC_INTEL_CSE_SEND_EOP_LATE) || - CONFIG(SOC_INTEL_CSE_SEND_EOP_ASYNC)) + do_send_end_of_post(); + if (CONFIG(SOC_INTEL_CSE_SEND_EOP_LATE)) cse_late_finalize(); } -- To view, visit https://review.coreboot.org/c/coreboot/+/74207 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibebb20a000a239c344af1c96b8d376352b9c774e Gerrit-Change-Number: 74207 Gerrit-PatchSet: 2 Gerrit-Owner: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.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-MessageType: merged

1 0

[S] Change in coreboot[master]: Revert "mb/google/brya: Enable asynchronous End-Of-Post"
by Subrata Banik (Code Review) 05 Apr '23

05 Apr '23

Subrata Banik has submitted this change. ( https://review.coreboot.org/c/coreboot/+/74206 ) Change subject: Revert "mb/google/brya: Enable asynchronous End-Of-Post" ...................................................................... Revert "mb/google/brya: Enable asynchronous End-Of-Post" This reverts commit 11f2f88a277124713f7b0023f078fcc2e1a98c32. Revert initial change as it was causing a boot failure when transitioning into recovery mode. BUG=b:276927816 TEST='emerge-brya coreboot chromeos-bootimage', flash and boot a skolas SKU1 to kernel, then press Esc-Refresh-PowerButton to try to reboot into recovery mode. Change-Id: I91c8d0434a2354dedfa49dd6100caf0e5bfe3f4c Signed-off-by: Nick Vaccaro <nvaccaro(a)google.com> Reviewed-on: https://review.coreboot.org/c/coreboot/+/74206 Reviewed-by: Tarun Tuli <taruntuli(a)google.com> Reviewed-by: Subrata Banik <subratabanik(a)google.com> Reviewed-by: Eran Mitrani <mitrani(a)google.com> Reviewed-by: Kapil Porwal <kapilporwal(a)google.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/mainboard/google/brya/Kconfig M src/soc/intel/alderlake/Kconfig 2 files changed, 27 insertions(+), 2 deletions(-) Approvals: build bot (Jenkins): Verified Subrata Banik: Looks good to me, approved Eran Mitrani: Looks good to me, approved Tarun Tuli: Looks good to me, approved Kapil Porwal: Looks good to me, approved diff --git a/src/mainboard/google/brya/Kconfig b/src/mainboard/google/brya/Kconfig index e588524..9ae944d 100644 --- a/src/mainboard/google/brya/Kconfig +++ b/src/mainboard/google/brya/Kconfig @@ -32,7 +32,6 @@ select PMC_IPC_ACPI_INTERFACE select SOC_INTEL_COMMON_BLOCK_PCIE_RTD3 select SOC_INTEL_CSE_LITE_SKU - select SOC_INTEL_CSE_SEND_EOP_ASYNC select SOC_INTEL_ENABLE_USB4_PCIE_RESOURCES if SOC_INTEL_ALDERLAKE_PCH_P select SOC_INTEL_COMMON_BASECODE_DEBUG_FEATURE select SOC_INTEL_CRASHLOG diff --git a/src/soc/intel/alderlake/Kconfig b/src/soc/intel/alderlake/Kconfig index 2887439..1bc84ce 100644 --- a/src/soc/intel/alderlake/Kconfig +++ b/src/soc/intel/alderlake/Kconfig @@ -122,7 +122,7 @@ select SOC_INTEL_COMMON_FSP_RESET select SOC_INTEL_COMMON_PCH_CLIENT select SOC_INTEL_COMMON_RESET - select SOC_INTEL_CSE_SEND_EOP_LATE if !BOARD_GOOGLE_BRYA_COMMON + select SOC_INTEL_CSE_SEND_EOP_LATE select SOC_INTEL_CSE_SET_EOP select SOC_INTEL_MEM_MAPPED_PM_CONFIGURATION select HAVE_INTEL_COMPLIANCE_TEST_MODE -- To view, visit https://review.coreboot.org/c/coreboot/+/74206 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I91c8d0434a2354dedfa49dd6100caf0e5bfe3f4c Gerrit-Change-Number: 74206 Gerrit-PatchSet: 3 Gerrit-Owner: Nick Vaccaro <nvaccaro(a)google.com> Gerrit-Reviewer: Eran Mitrani <mitrani(a)google.com> Gerrit-Reviewer: Kapil Porwal <kapilporwal(a)google.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-MessageType: merged

1 0

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit April 2023