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@intel.com>
Cc: Jiaxin Wu <jiaxin.wu@intel.com>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
Cc: Dun Tan <dun.tan@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Star Zeng <star.zeng@intel.com>
Signed-off-by: Aaron Young <aaron.young@oracle.com>
This commit is contained in:
Aaron Young
2025-05-15 09:54:00 -07:00
committed by mergify[bot]
parent 5a00ea00e9
commit b0bc23d1f2
3 changed files with 29 additions and 10 deletions

View File

@ -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
@ -100,5 +100,14 @@ struc MP_CPU_EXCHANGE_INFO
.SevSnpKnownInitApicId: 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)

View File

@ -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

View File

@ -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;