Attention is currently required from: Bora Guvendik, Cliff Huang, Jamie Ryu, Jérémy Compostella, 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 (#2).
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 GetProcessorInfo() 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 complete path information excluding the package and thread.
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@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/2