[pve-devel] [PATCH edk2-firmware v2] fix #6430: backport patch to fix split lock detection warnings
Friedrich Weber
f.weber at proxmox.com
Thu Jul 10 14:50:29 CEST 2025
On host CPUs with the split_lock_detect flag (newer Intel CPUs),
booting an OVMF VM with more than one core may trigger the host
kernel's split lock detection, as reported in [1].
With default settings, a kernel >= 5.19 slows down the corresponding
thread for 10ms when it detects a split lock operation, as documented
in [2]. A warning is logged, e.g.:
> kernel: x86/split lock detection: #AC: CPU 0/KVM/5677 took a split_lock trap at address: 0x56339a9888c3
The kernel only prints this warning once per thread.
Booting an OVMF VM apparently only performs 0-2 split lock operations
per vCPU thread, so the slowdown is negligible. However, another
downside is that, since the warning is only printed once per vCPU
thread, these spurious warnings mask any subsequent split lock
operations performed by the same vCPU thread. As a result, users are
not warned when their actual VM workload triggers split lock
detection, where the slowdowns would potentially be more noticeable.
Hence, backport an upstream patch that fixes the split lock detection
warnings.
[1] https://bugzilla.proxmox.com/show_bug.cgi?id=6430
[2] https://pve.proxmox.com/wiki/Split_lock_detection
Signed-off-by: Friedrich Weber <f.weber at proxmox.com>
---
Notes:
Seems like a new upstream release is planned for August anyway [1],
but it might be nice to get this fixed for PVE 9 already.
Apparently needs to be applied with `git am --keep-cr`.
Only got it to build on bookworm (on trixie, the dependency
python3-distutils doesn't seem to exist?)
changes since v2:
- reworded commit message
[1] https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning/cb019c651d162e37f6cc7fed9a32bf7ca4153336
...tLib-Fix-split-lock-violation-from-M.patch | 163 ++++++++++++++++++
debian/patches/series | 1 +
2 files changed, 164 insertions(+)
create mode 100644 debian/patches/UefiCpuPkg-MpInitLib-Fix-split-lock-violation-from-M.patch
diff --git a/debian/patches/UefiCpuPkg-MpInitLib-Fix-split-lock-violation-from-M.patch b/debian/patches/UefiCpuPkg-MpInitLib-Fix-split-lock-violation-from-M.patch
new file mode 100644
index 0000000..2583417
--- /dev/null
+++ b/debian/patches/UefiCpuPkg-MpInitLib-Fix-split-lock-violation-from-M.patch
@@ -0,0 +1,163 @@
+From 98b8e5055073989cfd838b4f2cea39b2718dc5bb Mon Sep 17 00:00:00 2001
+From: Aaron Young <Aaron.Young at oracle.com>
+Date: Thu, 15 May 2025 09:54:00 -0700
+Subject: [PATCH] UefiCpuPkg/MpInitLib: Fix split-lock violation from
+ MP_CPU_EXCHANGE_INFO
+
+A split-lock violation in OVMF was discovered due to the
+NumApsExecuting field of the MP_CPU_EXCHANGE_INFO
+struct (which is used atomically by the AP Reset Vector
+assembly code) crossing a cacheline boundary.
+
+Since the MP_CPU_EXCHANGE_INFO struct is unaligned and
+the NumApsExecuting field resides after other non-UINTN aligned
+fields in the struct (i.e. GdtrProfile/IdtrProfile), the
+NumApsExecuting field was allocated at a non-UINTN aligned
+address (crossing a cache-line) resulting in the split-lock
+violation.
+
+Therefore, align the MP_CPU_EXCHANGE_INFO struct (on a UINTN
+boundary) and move the NumApsExecuting field to before the
+GdtrProfile/IdtrProfile fields to ensure it is UINTN aligned and
+thus resides within a single cacheline avoiding the split-lock.
+Do the same for the ApIndex field as it is also used atomically
+and thus subject to a split-lock violation.
+
+Cc: Ray Ni <ray.ni at intel.com>
+Cc: Jiaxin Wu <jiaxin.wu at intel.com>
+Cc: Zhiguang Liu <zhiguang.liu at intel.com>
+Cc: Dun Tan <dun.tan at intel.com>
+Cc: Rahul Kumar <rahul1.kumar at intel.com>
+Cc: Gerd Hoffmann <kraxel at redhat.com>
+Cc: Star Zeng <star.zeng at intel.com>
+Signed-off-by: Aaron Young <aaron.young at oracle.com>
+(cherry picked from commit b0bc23d1f246dac977b639470a51bcef1bcd6e1d)
+Signed-off-by: Friedrich Weber <f.weber at proxmox.com>
+---
+ UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 15 ++++++++++++---
+ UefiCpuPkg/Library/MpInitLib/MpLib.c | 15 ++++++++++-----
+ UefiCpuPkg/Library/MpInitLib/MpLib.h | 9 +++++++--
+ 3 files changed, 29 insertions(+), 10 deletions(-)
+
+diff --git a/UefiCpuPkg/Library/MpInitLib/MpEqu.inc b/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
+index 317e627b58..ded603f8f8 100644
+--- a/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
++++ b/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
+@@ -74,18 +74,18 @@ struc MP_CPU_EXCHANGE_INFO
+ .StackStart: CTYPE_UINTN 1
+ .StackSize: CTYPE_UINTN 1
+ .CFunction: CTYPE_UINTN 1
++ .NumApsExecuting: CTYPE_UINTN 1
++ .ApIndex: CTYPE_UINTN 1
+ .GdtrProfile: CTYPE_UINT8 IA32_DESCRIPTOR_size
+ .IdtrProfile: CTYPE_UINT8 IA32_DESCRIPTOR_size
+ .BufferStart: CTYPE_UINTN 1
+ .ModeOffset: CTYPE_UINTN 1
+- .ApIndex: CTYPE_UINTN 1
+ .CodeSegment: CTYPE_UINTN 1
+ .DataSegment: CTYPE_UINTN 1
+ .EnableExecuteDisable: CTYPE_UINTN 1
+ .Cr3: CTYPE_UINTN 1
+ .InitFlag: CTYPE_UINTN 1
+ .CpuInfo: CTYPE_UINTN 1
+- .NumApsExecuting: CTYPE_UINTN 1
+ .CpuMpData: CTYPE_UINTN 1
+ .InitializeFloatingPointUnits: CTYPE_UINTN 1
+ .ModeTransitionMemory: CTYPE_UINT32 1
+@@ -99,5 +99,14 @@ struc MP_CPU_EXCHANGE_INFO
+ .ExtTopoAvail: CTYPE_BOOLEAN 1
+ endstruc
+
+-MP_CPU_EXCHANGE_INFO_OFFSET equ (Flat32Start - RendezvousFunnelProcStart)
++;
++; Declare a UINTN struct for the purpose of
++; of obtaining the size of a UINTN (UINTN_size).
++;
++struc UINTN
++ .Data CTYPE_UINTN 1
++endstruc
++
++; MP_CPU_EXCHANGE_INFO Offset (UINTN aligned)
++MP_CPU_EXCHANGE_INFO_OFFSET equ (((Flat32Start - RendezvousFunnelProcStart) + (UINTN_size - 1)) & ~(UINTN_size - 1))
+ %define MP_CPU_EXCHANGE_INFO_FIELD(Field) (MP_CPU_EXCHANGE_INFO_OFFSET + MP_CPU_EXCHANGE_INFO. %+ Field)
+diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
+index fdcc21d794..ffaff1855f 100644
+--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
++++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
+@@ -960,7 +960,7 @@ GetApResetVectorSize (
+ )
+ {
+ if (SizeBelow1Mb != NULL) {
+- *SizeBelow1Mb = AddressMap->ModeTransitionOffset + sizeof (MP_CPU_EXCHANGE_INFO);
++ *SizeBelow1Mb = ALIGN_VALUE (AddressMap->ModeTransitionOffset, sizeof (UINTN)) + sizeof (MP_CPU_EXCHANGE_INFO);
+ }
+
+ if (SizeAbove1Mb != NULL) {
+@@ -1092,7 +1092,7 @@ BackupAndPrepareWakeupBuffer (
+ CopyMem (
+ (VOID *)CpuMpData->WakeupBuffer,
+ (VOID *)CpuMpData->AddressMap.RendezvousFunnelAddress,
+- CpuMpData->BackupBufferSize - sizeof (MP_CPU_EXCHANGE_INFO)
++ CpuMpData->AddressMap.ModeTransitionOffset
+ );
+ }
+
+@@ -1126,17 +1126,22 @@ AllocateResetVectorBelow1Mb (
+ UINTN ApResetStackSize;
+
+ if (CpuMpData->WakeupBuffer == (UINTN)-1) {
+- CpuMpData->WakeupBuffer = GetWakeupBuffer (CpuMpData->BackupBufferSize);
++ CpuMpData->WakeupBuffer = GetWakeupBuffer (CpuMpData->BackupBufferSize);
++ //
++ // Align MpCpuExchangeInfo to avoid split-lock violations.
++ //
+ CpuMpData->MpCpuExchangeInfo = (MP_CPU_EXCHANGE_INFO *)(UINTN)
+- (CpuMpData->WakeupBuffer + CpuMpData->BackupBufferSize - sizeof (MP_CPU_EXCHANGE_INFO));
++ (CpuMpData->WakeupBuffer +
++ ALIGN_VALUE (CpuMpData->AddressMap.ModeTransitionOffset, sizeof (UINTN)));
+ DEBUG ((
+ DEBUG_INFO,
+ "AP Vector: 16-bit = %p/%x, ExchangeInfo = %p/%x\n",
+ CpuMpData->WakeupBuffer,
+- CpuMpData->BackupBufferSize - sizeof (MP_CPU_EXCHANGE_INFO),
++ CpuMpData->AddressMap.ModeTransitionOffset,
+ CpuMpData->MpCpuExchangeInfo,
+ sizeof (MP_CPU_EXCHANGE_INFO)
+ ));
++
+ //
+ // The AP reset stack is only used by SEV-ES guests. Do not allocate it
+ // if SEV-ES is not enabled. An SEV-SNP guest is also considered
+diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
+index 145538b6ee..fc08ae2ce6 100644
+--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
++++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
+@@ -213,18 +213,23 @@ typedef struct {
+ UINTN StackStart;
+ UINTN StackSize;
+ UINTN CFunction;
++ //
++ // NumApsExecuting and ApIndex are used atomically (in the
++ // assembly code). To avoid split-lock violations, keep them
++ // naturally aligned within a single cacheline.
++ //
++ UINTN NumApsExecuting;
++ UINTN ApIndex;
+ IA32_DESCRIPTOR GdtrProfile;
+ IA32_DESCRIPTOR IdtrProfile;
+ UINTN BufferStart;
+ UINTN ModeOffset;
+- UINTN ApIndex;
+ UINTN CodeSegment;
+ UINTN DataSegment;
+ UINTN EnableExecuteDisable;
+ UINTN Cr3;
+ UINTN InitFlag;
+ CPU_INFO_IN_HOB *CpuInfo;
+- UINTN NumApsExecuting;
+ CPU_MP_DATA *CpuMpData;
+ UINTN InitializeFloatingPointUnitsAddress;
+ UINT32 ModeTransitionMemory;
+--
+2.47.2
+
diff --git a/debian/patches/series b/debian/patches/series
index 45b3050..f9e3582 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -4,3 +4,4 @@ x64-baseline-abi.patch
Revert-ArmVirtPkg-make-EFI_LOADER_DATA-non-executabl.patch
ArmVirtPkg-disable-the-EFI_MEMORY_ATTRIBUTE-protocol.patch
Revert-UefiCpuPkg-Produce-EFI-memory-attributes-prot.patch
+UefiCpuPkg-MpInitLib-Fix-split-lock-violation-from-M.patch
--
2.39.5
More information about the pve-devel
mailing list