coreboot-gerrit January 2025

coreboot-gerrit@coreboot.org

1 participants
3269 discussions

[S] Change in coreboot[main]: cpu/x86/topology: Fix FSP-S crash caused by shared core ID
by Patrick Rudolph (Code Review) 16 Jan '25

16 Jan '25

Attention is currently required from: Andrey Petrov, Bora Guvendik, Cliff Huang, Intel coreboot Reviewers, Jamie Ryu, Jérémy Compostella, Name of user not set #1005776, Ronak Kanabar, Sean Rhodes, Zhixing Ma. Patrick Rudolph has posted comments on this change by Jérémy Compostella. ( https://review.coreboot.org/c/coreboot/+/86004?usp=email ) Change subject: cpu/x86/topology: Fix FSP-S crash caused by shared core ID ...................................................................... Patch Set 6: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/86004?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I06db580cddaeaf5c452fa72f131d37d10dbc5974 Gerrit-Change-Number: 86004 Gerrit-PatchSet: 6 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Name of user not set #1005776 Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Zhixing Ma <zhixing.ma(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Zhixing Ma <zhixing.ma(a)intel.com> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Name of user not set #1005776 Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Comment-Date: Thu, 16 Jan 2025 17:09:13 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in coreboot[main]: cpu/x86/topology: Fix FSP-S crash caused by shared core ID
by Jérémy Compostella (Code Review) 16 Jan '25

16 Jan '25

Attention is currently required from: Andrey Petrov, Bora Guvendik, Cliff Huang, Intel coreboot Reviewers, Jamie Ryu, Name of user not set #1005776, Patrick Rudolph, Ronak Kanabar, Sean Rhodes, Zhixing Ma. Jérémy Compostella has posted comments on this change by Jérémy Compostella. ( https://review.coreboot.org/c/coreboot/+/86004?usp=email ) Change subject: cpu/x86/topology: Fix FSP-S crash caused by shared core ID ...................................................................... Patch Set 6: (1 comment) File src/cpu/x86/topology.c: https://review.coreboot.org/c/coreboot/+/86004/comment/7c4dfdb9_8456f4f9?us… : PS2, Line 156: EFI_CPU_PHYSICAL_LOCATION > `mp_get_processor_info` does not know how many bit are reserved per domain and therefore cannot […] I believe that the latest version introducing `core_id_within_package` should alleviate this concern. -- To view, visit https://review.coreboot.org/c/coreboot/+/86004?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I06db580cddaeaf5c452fa72f131d37d10dbc5974 Gerrit-Change-Number: 86004 Gerrit-PatchSet: 6 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Name of user not set #1005776 Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Zhixing Ma <zhixing.ma(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Zhixing Ma <zhixing.ma(a)intel.com> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Name of user not set #1005776 Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Attention: Ronak Kanabar <ronak.kanabar(a)intel.com> Gerrit-Attention: Andrey Petrov <andrey.petrov(a)gmail.com> Gerrit-Comment-Date: Thu, 16 Jan 2025 17:07:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com> Comment-In-Reply-To: Jérémy Compostella <jeremy.compostella(a)intel.com>

1 0

[S] Change in coreboot[main]: cpu/x86/topology: Fix FSP-S crash caused by shared core ID
by Jérémy Compostella (Code Review) 16 Jan '25

16 Jan '25

Attention is currently required from: Bora Guvendik, Cliff Huang, Jamie Ryu, Name of user not set #1005776, Patrick Rudolph, Sean Rhodes, Zhixing Ma. Hello Bora Guvendik, Cliff Huang, Jamie Ryu, Name of user not set #1005776, Patrick Rudolph, Sean Rhodes, Zhixing Ma, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/86004?usp=email to look at the new patch set (#6). The following approvals got outdated and were removed: Code-Review+1 by Name of user not set #1005776, Code-Review+2 by Bora Guvendik, Verified+1 by build bot (Jenkins) Change subject: cpu/x86/topology: Fix FSP-S crash caused by shared core ID ...................................................................... cpu/x86/topology: Fix FSP-S crash caused by shared core ID This resolves a crash issue observed on Meteor Lake and introduced by commit 70bdd2e1fad9fe89835aab240ed4b41a02f15078 ("cpu/x86/topology: Simplify CPU topology initialization"). This commit simplifies the code and provides more detailed CPU topology information by generalizing the use of the Extended Topology Enumeration Leaves 0x1f. As a result, the coreboot APIC core_id field does not provide the fully detailed path information. It turns out that the topology core identifier is used by the coreboot MP service mp_get_processor_info() implementation. But the MP Service EFI_CPU_PHYSICAL_LOCATION data structure only captures information about the package, core, and thread. The core identifier returned to the MP service caller must incorporate the full hierarchical path (die group, die, module, tile, module and core). This commit adds a new field to the cpu topology structure to represent the core ID within the package. For reference, here is that signature of the crash: LAPIC 0x40 in X2APIC mode. CPU Index 2 - APIC 64 Unexpected Exception:13 @ 10:69f3d1e4 - Halting Code: 0 eflags: 00010046 cr2: 00000000 eax: 00000001 ebx: 69f313e8 ecx: 0000004e edx: 00000000 edi: 69f38018 esi: 00000029 ebp: 69aeee0c esp: 69aeedc0 [...] The crash occurred when FSP attempted to lock the Protected Processor Inventory Number Enable Control MSR (IA32_PPIN_CTL 0x4e). 69f3d1d3: 8b 43 f4 mov -0xc(%ebx),%eax 69f3d1d6: 89 4d c4 mov %ecx,-0x3c(%ebp) 69f3d1d9: 89 45 dc mov %eax,-0x24(%ebp) 69f3d1dc: 8b 55 c4 mov -0x3c(%ebp),%edx 69f3d1df: 8b 45 c0 mov -0x40(%ebp),%eax 69f3d1e2: 8b 4d dc mov -0x24(%ebp),%ecx 69f3d1e5: 0f 30 wrmsr 69f3d1e7: e9 ee fd ff ff jmp 0xfffffe39 FSP experiences issues due to attempting to lock the same register multiple times for a single core. This is caused by an inconsistency in the processor information data structure, where multiple cores share the same identifier. This is not permitted and triggers a General Protection Fault Exception. TEST=Executing CpuFeaturesPei.efi in FSP-S does not crash on a rex board. Change-Id: I06db580cddaeaf5c452fa72f131d37d10dbc5974 Signed-off-by: Jeremy Compostella <jeremy.compostella(a)intel.com> --- M src/cpu/x86/topology.c M src/drivers/intel/fsp2_0/ppi/mp_service_ppi.c M src/include/device/path.h 3 files changed, 11 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/86004/6 -- To view, visit https://review.coreboot.org/c/coreboot/+/86004?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I06db580cddaeaf5c452fa72f131d37d10dbc5974 Gerrit-Change-Number: 86004 Gerrit-PatchSet: 6 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Name of user not set #1005776 Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Zhixing Ma <zhixing.ma(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Attention: Zhixing Ma <zhixing.ma(a)intel.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Name of user not set #1005776 Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com>

1 0

[S] Change in coreboot[main]: tree: Use boolean for docking_supported
by Jérémy Compostella (Code Review) 16 Jan '25

16 Jan '25

Attention is currently required from: Alexander Couzens, Elyes Haouas, Intel coreboot Reviewers, Nicholas Chin, Stefan Ott. Jérémy Compostella has posted comments on this change by Elyes Haouas. ( https://review.coreboot.org/c/coreboot/+/86022?usp=email ) Change subject: tree: Use boolean for docking_supported ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/86022?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I25f09457edf4cfb9bec6939de3e56c2ea7965801 Gerrit-Change-Number: 86022 Gerrit-PatchSet: 1 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: Stefan Ott <coreboot(a)desire.ch> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Stefan Ott <coreboot(a)desire.ch> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Comment-Date: Thu, 16 Jan 2025 16:57:47 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[M] Change in coreboot[main]: tree: Use boolean for deep_s{3,5}_enable_{ac,dc}
by Jérémy Compostella (Code Review) 16 Jan '25

16 Jan '25

Attention is currently required from: Alexander Couzens, Elyes Haouas, Erik van den Bogaert, Frans Hendriks, Intel coreboot Reviewers, Jonathon Hall, Michał Żygowski, Nick Vaccaro, Piotr Król, Subrata Banik, Werner Zeh. Jérémy Compostella has posted comments on this change by Elyes Haouas. ( https://review.coreboot.org/c/coreboot/+/86020?usp=email ) Change subject: tree: Use boolean for deep_s{3,5}_enable_{ac,dc} ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/86020?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I1621e98e7925b140c608f893a6680c9384bac2f0 Gerrit-Change-Number: 86020 Gerrit-PatchSet: 1 Gerrit-Owner: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Reviewer: Erik van den Bogaert <ebogaert(a)eltan.com> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Reviewer: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Erik van den Bogaert <ebogaert(a)eltan.com> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Attention: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Nick Vaccaro <nvaccaro(a)chromium.org> Gerrit-Attention: Jonathon Hall <jonathon.hall(a)puri.sm> Gerrit-Attention: Alexander Couzens <lynxis(a)fe80.eu> Gerrit-Attention: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Attention: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Attention: Piotr Król <piotr.krol(a)3mdeb.com> Gerrit-Comment-Date: Thu, 16 Jan 2025 16:56:24 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XS] Change in coreboot[main]: sb/intel/bd82x6x: Drop xhci_overcurrent_mapping
by Jérémy Compostella (Code Review) 16 Jan '25

16 Jan '25

Attention is currently required from: Intel coreboot Reviewers, Keith Hui. Jérémy Compostella has posted comments on this change by Keith Hui. ( https://review.coreboot.org/c/coreboot/+/85925?usp=email ) Change subject: sb/intel/bd82x6x: Drop xhci_overcurrent_mapping ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/85925?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I8c5e9b2016cf56538de06575181a0a6b738c6a28 Gerrit-Change-Number: 85925 Gerrit-PatchSet: 4 Gerrit-Owner: Keith Hui <buurin(a)gmail.com> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Keith Hui <buurin(a)gmail.com> Gerrit-Comment-Date: Thu, 16 Jan 2025 16:54:46 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[XS] Change in coreboot[main]: sb/intel/bd82x6x: Apply EHCI mapping to xhci_overcurrent_mapping
by Jérémy Compostella (Code Review) 16 Jan '25

16 Jan '25

Attention is currently required from: Intel coreboot Reviewers, Keith Hui. Jérémy Compostella has posted comments on this change by Keith Hui. ( https://review.coreboot.org/c/coreboot/+/85922?usp=email ) Change subject: sb/intel/bd82x6x: Apply EHCI mapping to xhci_overcurrent_mapping ...................................................................... Patch Set 4: Code-Review+1 -- To view, visit https://review.coreboot.org/c/coreboot/+/85922?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Iab30a07c8df223e4053c5f28df5e5ed926f278f7 Gerrit-Change-Number: 85922 Gerrit-PatchSet: 4 Gerrit-Owner: Keith Hui <buurin(a)gmail.com> Gerrit-Reviewer: Elyes Haouas <ehaouas(a)noos.fr> Gerrit-Reviewer: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Reviewer: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Intel coreboot Reviewers <intel_coreboot_reviewers(a)intel.com> Gerrit-Attention: Keith Hui <buurin(a)gmail.com> Gerrit-Comment-Date: Thu, 16 Jan 2025 16:54:29 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[M] Change in coreboot[main]: soc/amd/phoenix/pci_irq_routing.c: Populate PCI IRQ routing table
by Felix Held (Code Review) 16 Jan '25

16 Jan '25

Attention is currently required from: Fred Reitberger, Jason Glenesk, Marshall Dawson, Matt DeVillier, Nick Kochlowski, Paul Menzel. Felix Held has posted comments on this change by Nick Kochlowski. ( https://review.coreboot.org/c/coreboot/+/85195?usp=email ) Change subject: soc/amd/phoenix/pci_irq_routing.c: Populate PCI IRQ routing table ...................................................................... Patch Set 12: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/85195?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: Id014ff3e675831eec42bc46c0a76271341e0e3e4 Gerrit-Change-Number: 85195 Gerrit-PatchSet: 12 Gerrit-Owner: Nick Kochlowski <nickkochlowski(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: Marshall Dawson <marshalldawson3rd(a)gmail.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)amd.corp-partner.google.com> Gerrit-Reviewer: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Jason Glenesk <jason.glenesk(a)gmail.com> Gerrit-Attention: Marshall Dawson <marshalldawson3rd(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: Nick Kochlowski <nickkochlowski(a)gmail.com> Gerrit-Attention: Fred Reitberger <reitbergerfred(a)gmail.com> Gerrit-Comment-Date: Thu, 16 Jan 2025 16:50:46 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in coreboot[main]: cpu/x86/topology: Fix FSP-S crash caused by shared core ID
by Jérémy Compostella (Code Review) 16 Jan '25

16 Jan '25

Attention is currently required from: Cliff Huang, Jamie Ryu, Patrick Rudolph, Sean Rhodes, Zhixing Ma. Jérémy Compostella has posted comments on this change by Jérémy Compostella. ( https://review.coreboot.org/c/coreboot/+/86004?usp=email ) Change subject: cpu/x86/topology: Fix FSP-S crash caused by shared core ID ...................................................................... Patch Set 5: (3 comments) Commit Message: https://review.coreboot.org/c/coreboot/+/86004/comment/31ebbf58_7048cd96?us… : PS2, Line 18: GetProcessorInfo > name the coreboot function: ` […] Done https://review.coreboot.org/c/coreboot/+/86004/comment/57d7494d_5261ab86?us… : PS2, Line 21: complete path > what is a complete path? […] Done File src/cpu/x86/topology.c: https://review.coreboot.org/c/coreboot/+/86004/comment/2c72ce5d_5b424ae0?us… : PS2, Line 156: EFI_CPU_PHYSICAL_LOCATION > FSP quirks should be handled in FSP wrapper code, for example in ` […] `mp_get_processor_info` does not know how many bit are reserved per domain and therefore cannot reconstitute the full path even if it was supplied with the die group, die, tile, module and core. As an alternative to the current approach, I can introduce a new field `complete_path_core_id`, fill that one and make MP service use it instead of `core_id`. It should help lift your concern. What do you think? -- To view, visit https://review.coreboot.org/c/coreboot/+/86004?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I06db580cddaeaf5c452fa72f131d37d10dbc5974 Gerrit-Change-Number: 86004 Gerrit-PatchSet: 5 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Name of user not set #1005776 Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Zhixing Ma <zhixing.ma(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Zhixing Ma <zhixing.ma(a)intel.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Comment-Date: Thu, 16 Jan 2025 16:23:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Patrick Rudolph <patrick.rudolph(a)9elements.com>

1 0

[S] Change in coreboot[main]: cpu/x86/topology: Fix FSP-S crash caused by shared core ID
by Jérémy Compostella (Code Review) 16 Jan '25

16 Jan '25

Attention is currently required from: Cliff Huang, Jamie Ryu, Jérémy Compostella, Sean Rhodes, Zhixing Ma. Hello Bora Guvendik, Cliff Huang, Jamie Ryu, Name of user not set #1005776, Patrick Rudolph, Sean Rhodes, Zhixing Ma, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/coreboot/+/86004?usp=email to look at the new patch set (#5). Change subject: cpu/x86/topology: Fix FSP-S crash caused by shared core ID ...................................................................... cpu/x86/topology: Fix FSP-S crash caused by shared core ID This resolves a crash issue observed on Meteor Lake and introduced by commit 70bdd2e1fad9fe89835aab240ed4b41a02f15078 ("cpu/x86/topology: Simplify CPU topology initialization"). This commit simplifies the code and provides more detailed CPU topology information by generalizing the use of the Extended Topology Enumeration Leaves 0x1f. As a result, the coreboot APIC core_id field does not provide the fully detailed path information. It turns out that the topology core identifier is used by the coreboot MP service mp_get_processor_info() implementation. But the MP Service EFI_CPU_PHYSICAL_LOCATION data structure only captures information about the package, core, and thread. The core identifier returned to the MP service caller must incorporate the full hierarchical path (die group, die, module, tile, module and core). As the core_id is solely used in coreboot by the MP Service, the fix is to calculate the core ID using the complete path information excluding the package and thread and store it in core_id. For reference, here is that signature of the crash: LAPIC 0x40 in X2APIC mode. CPU Index 2 - APIC 64 Unexpected Exception:13 @ 10:69f3d1e4 - Halting Code: 0 eflags: 00010046 cr2: 00000000 eax: 00000001 ebx: 69f313e8 ecx: 0000004e edx: 00000000 edi: 69f38018 esi: 00000029 ebp: 69aeee0c esp: 69aeedc0 [...] The crash occurred when FSP attempted to lock the Protected Processor Inventory Number Enable Control MSR (IA32_PPIN_CTL 0x4e). 69f3d1d3: 8b 43 f4 mov -0xc(%ebx),%eax 69f3d1d6: 89 4d c4 mov %ecx,-0x3c(%ebp) 69f3d1d9: 89 45 dc mov %eax,-0x24(%ebp) 69f3d1dc: 8b 55 c4 mov -0x3c(%ebp),%edx 69f3d1df: 8b 45 c0 mov -0x40(%ebp),%eax 69f3d1e2: 8b 4d dc mov -0x24(%ebp),%ecx 69f3d1e5: 0f 30 wrmsr 69f3d1e7: e9 ee fd ff ff jmp 0xfffffe39 FSP experiences issues due to attempting to lock the same register multiple times for a single core. This is caused by an inconsistency in the processor information data structure, where multiple cores share the same identifier. This is not permitted and triggers a General Protection Fault Exception. TEST=Executing CpuFeaturesPei.efi in FSP-S does not crash on a rex board. Change-Id: I06db580cddaeaf5c452fa72f131d37d10dbc5974 Signed-off-by: Jeremy Compostella <jeremy.compostella(a)intel.com> --- M src/cpu/x86/topology.c 1 file changed, 10 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/04/86004/5 -- To view, visit https://review.coreboot.org/c/coreboot/+/86004?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: coreboot Gerrit-Branch: main Gerrit-Change-Id: I06db580cddaeaf5c452fa72f131d37d10dbc5974 Gerrit-Change-Number: 86004 Gerrit-PatchSet: 5 Gerrit-Owner: Jérémy Compostella <jeremy.compostella(a)intel.com> Gerrit-Reviewer: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-Reviewer: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Reviewer: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Reviewer: Name of user not set #1005776 Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Reviewer: Zhixing Ma <zhixing.ma(a)intel.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Zhixing Ma <zhixing.ma(a)intel.com> Gerrit-Attention: Cliff Huang <cliff.huang(a)intel.com> Gerrit-Attention: Sean Rhodes <sean(a)starlabs.systems> Gerrit-Attention: Jamie Ryu <jamie.m.ryu(a)intel.com> Gerrit-Attention: Jérémy Compostella <jeremy.compostella(a)intel.com>

1 0

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit January 2025