* [PATCH v2 0/3] Use XADD to avoid lock acquire/release
@ 2021-02-05 7:58 Ni, Ray
2021-02-05 7:58 ` [PATCH v2 1/3] MdePkg/Nasm.inc: add macros for C types used in structure definition Ni, Ray
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Ni, Ray @ 2021-02-05 7:58 UTC (permalink / raw)
To: devel
Patch #1 follows Laszlo's suggestion to add global NASM macros
for NASM struc usage.
Patch #2 changes all hardcode offset to use struc.
Patch #3 doesn't have any change comparing to V1 except
1). dword/qword prefix is added.
2). the comments "program AP stack" is removed.
Ray Ni (3):
MdePkg/Nasm.inc: add macros for C types used in structure definition
UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset
UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release
MdePkg/Include/Ia32/Nasm.inc | 38 ++++++
MdePkg/Include/X64/Nasm.inc | 38 ++++++
UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 5 +-
UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc | 43 -------
.../Library/MpInitLib/Ia32/MpFuncs.nasm | 98 +++++++---------
UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 99 ++++++++++++++++
UefiCpuPkg/Library/MpInitLib/MpLib.c | 1 -
UefiCpuPkg/Library/MpInitLib/MpLib.h | 3 +-
UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 5 +-
UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc | 45 --------
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 108 ++++++++----------
11 files changed, 272 insertions(+), 211 deletions(-)
delete mode 100644 UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc
create mode 100644 UefiCpuPkg/Library/MpInitLib/MpEqu.inc
delete mode 100644 UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
--
2.27.0.windows.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2 1/3] MdePkg/Nasm.inc: add macros for C types used in structure definition
2021-02-05 7:58 [PATCH v2 0/3] Use XADD to avoid lock acquire/release Ni, Ray
@ 2021-02-05 7:58 ` Ni, Ray
2021-02-05 15:02 ` [edk2-devel] " Laszlo Ersek
2021-02-05 7:58 ` [PATCH v2 2/3] UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset Ni, Ray
` (2 subsequent siblings)
3 siblings, 1 reply; 11+ messages in thread
From: Ni, Ray @ 2021-02-05 7:58 UTC (permalink / raw)
To: devel; +Cc: Michael D Kinney, Liming Gao, Zhiguang Liu
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Zhiguang Liu <zhiguang.liu@intel.com>
---
MdePkg/Include/Ia32/Nasm.inc | 38 ++++++++++++++++++++++++++++++++++++
MdePkg/Include/X64/Nasm.inc | 38 ++++++++++++++++++++++++++++++++++++
2 files changed, 76 insertions(+)
diff --git a/MdePkg/Include/Ia32/Nasm.inc b/MdePkg/Include/Ia32/Nasm.inc
index 31ce861f1e..017fe5ffd8 100644
--- a/MdePkg/Include/Ia32/Nasm.inc
+++ b/MdePkg/Include/Ia32/Nasm.inc
@@ -20,3 +20,41 @@
%macro INCSSP_EAX 0
DB 0xF3, 0x0F, 0xAE, 0xE8
%endmacro
+
+; NASM provides built-in macros STRUC and ENDSTRUC for structure definition.
+; For example, to define a structure called mytype containing a longword,
+; a word, a byte and a string of bytes, you might code
+;
+; struc mytype
+;
+; mt_long: resd 1
+; mt_word: resw 1
+; mt_byte: resb 1
+; mt_str: resb 32
+;
+; endstruc
+;
+; Below macros are help to map the C types and the RESB family of pseudo-instructions.
+; So that the above structure definition can be coded as
+;
+; struc mytype
+;
+; mt_long: CTYPE_UINT32 1
+; mt_word: CTYPE_UINT16 1
+; mt_byte: CTYPE_UINT8 1
+; mt_str: CTYPE_CHAR8 32
+;
+; endstruc
+%define CTYPE_UINT64 resq
+%define CTYPE_INT64 resq
+%define CTYPE_UINT32 resd
+%define CTYPE_INT32 resd
+%define CTYPE_UINT16 resw
+%define CTYPE_INT16 resw
+%define CTYPE_BOOLEAN resb
+%define CTYPE_UINT8 resb
+%define CTYPE_CHAR8 resb
+%define CTYPE_INT8 resb
+
+%define CTYPE_UINTN resd
+%define CTYPE_INTN resd
diff --git a/MdePkg/Include/X64/Nasm.inc b/MdePkg/Include/X64/Nasm.inc
index 42412735ea..b48d8680bb 100644
--- a/MdePkg/Include/X64/Nasm.inc
+++ b/MdePkg/Include/X64/Nasm.inc
@@ -20,3 +20,41 @@
%macro INCSSP_RAX 0
DB 0xF3, 0x48, 0x0F, 0xAE, 0xE8
%endmacro
+
+; NASM provides built-in macros STRUC and ENDSTRUC for structure definition.
+; For example, to define a structure called mytype containing a longword,
+; a word, a byte and a string of bytes, you might code
+;
+; struc mytype
+;
+; mt_long: resd 1
+; mt_word: resw 1
+; mt_byte: resb 1
+; mt_str: resb 32
+;
+; endstruc
+;
+; Below macros are help to map the C types and the RESB family of pseudo-instructions.
+; So that the above structure definition can be coded as
+;
+; struc mytype
+;
+; mt_long: CTYPE_UINT32 1
+; mt_word: CTYPE_UINT16 1
+; mt_byte: CTYPE_UINT8 1
+; mt_str: CTYPE_CHAR8 32
+;
+; endstruc
+%define CTYPE_UINT64 resq
+%define CTYPE_INT64 resq
+%define CTYPE_UINT32 resd
+%define CTYPE_INT32 resd
+%define CTYPE_UINT16 resw
+%define CTYPE_INT16 resw
+%define CTYPE_BOOLEAN resb
+%define CTYPE_UINT8 resb
+%define CTYPE_CHAR8 resb
+%define CTYPE_INT8 resb
+
+%define CTYPE_UINTN resq
+%define CTYPE_INTN resq
--
2.27.0.windows.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 2/3] UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset
2021-02-05 7:58 [PATCH v2 0/3] Use XADD to avoid lock acquire/release Ni, Ray
2021-02-05 7:58 ` [PATCH v2 1/3] MdePkg/Nasm.inc: add macros for C types used in structure definition Ni, Ray
@ 2021-02-05 7:58 ` Ni, Ray
2021-02-05 15:09 ` [edk2-devel] " Laszlo Ersek
2021-02-05 7:58 ` [PATCH v2 3/3] UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release Ni, Ray
2021-02-05 17:11 ` [edk2-devel] [PATCH v2 0/3] " Michael D Kinney
3 siblings, 1 reply; 11+ messages in thread
From: Ni, Ray @ 2021-02-05 7:58 UTC (permalink / raw)
To: devel; +Cc: Eric Dong, Laszlo Ersek, Rahul Kumar
In Windows environment, "dumpbin /disasm" is used to verify the
disassembly before and after using NASM struc doesn't change.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
---
UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 5 +-
UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc | 43 --------
.../Library/MpInitLib/Ia32/MpFuncs.nasm | 82 +++++++-------
UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 103 ++++++++++++++++++
UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 5 +-
UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc | 45 --------
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 94 ++++++++--------
7 files changed, 195 insertions(+), 182 deletions(-)
delete mode 100644 UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc
create mode 100644 UefiCpuPkg/Library/MpInitLib/MpEqu.inc
delete mode 100644 UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
index 1771575c69..860a9750e2 100644
--- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
@@ -1,7 +1,7 @@
## @file
# MP Initialize Library instance for DXE driver.
#
-# Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2016 - 2021, Intel Corporation. All rights reserved.<BR>
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
##
@@ -22,14 +22,13 @@ [Defines]
#
[Sources.IA32]
- Ia32/MpEqu.inc
Ia32/MpFuncs.nasm
[Sources.X64]
- X64/MpEqu.inc
X64/MpFuncs.nasm
[Sources.common]
+ MpEqu.inc
DxeMpLib.c
MpLib.c
MpLib.h
diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc b/UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc
deleted file mode 100644
index 4f5a7c859a..0000000000
--- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc
+++ /dev/null
@@ -1,43 +0,0 @@
-;------------------------------------------------------------------------------ ;
-; Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>
-; SPDX-License-Identifier: BSD-2-Clause-Patent
-;
-; Module Name:
-;
-; MpEqu.inc
-;
-; Abstract:
-;
-; This is the equates file for Multiple Processor support
-;
-;-------------------------------------------------------------------------------
-
-VacantFlag equ 00h
-NotVacantFlag equ 0ffh
-
-CPU_SWITCH_STATE_IDLE equ 0
-CPU_SWITCH_STATE_STORED equ 1
-CPU_SWITCH_STATE_LOADED equ 2
-
-LockLocation equ (SwitchToRealProcEnd - RendezvousFunnelProcStart)
-StackStartAddressLocation equ LockLocation + 04h
-StackSizeLocation equ LockLocation + 08h
-ApProcedureLocation equ LockLocation + 0Ch
-GdtrLocation equ LockLocation + 10h
-IdtrLocation equ LockLocation + 16h
-BufferStartLocation equ LockLocation + 1Ch
-ModeOffsetLocation equ LockLocation + 20h
-ApIndexLocation equ LockLocation + 24h
-CodeSegmentLocation equ LockLocation + 28h
-DataSegmentLocation equ LockLocation + 2Ch
-EnableExecuteDisableLocation equ LockLocation + 30h
-Cr3Location equ LockLocation + 34h
-InitFlagLocation equ LockLocation + 38h
-CpuInfoLocation equ LockLocation + 3Ch
-NumApsExecutingLocation equ LockLocation + 40h
-InitializeFloatingPointUnitsAddress equ LockLocation + 48h
-ModeTransitionMemoryLocation equ LockLocation + 4Ch
-ModeTransitionSegmentLocation equ LockLocation + 50h
-ModeHighMemoryLocation equ LockLocation + 52h
-ModeHighSegmentLocation equ LockLocation + 56h
-
diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
index 7e81d24aa6..2f1b102717 100644
--- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
@@ -1,5 +1,5 @@
;------------------------------------------------------------------------------ ;
-; Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
; SPDX-License-Identifier: BSD-2-Clause-Patent
;
; Module Name:
@@ -39,21 +39,21 @@ BITS 16
mov fs, ax
mov gs, ax
- mov si, BufferStartLocation
+ mov si, MP_CPU_EXCHANGE_INFO_FIELD (BufferStart)
mov ebx, [si]
- mov si, DataSegmentLocation
+ mov si, MP_CPU_EXCHANGE_INFO_FIELD (DataSegment)
mov edx, [si]
;
; Get start address of 32-bit code in low memory (<1MB)
;
- mov edi, ModeTransitionMemoryLocation
+ mov edi, MP_CPU_EXCHANGE_INFO_FIELD (ModeTransitionMemory)
- mov si, GdtrLocation
+ mov si, MP_CPU_EXCHANGE_INFO_FIELD (GdtrProfile)
o32 lgdt [cs:si]
- mov si, IdtrLocation
+ mov si, MP_CPU_EXCHANGE_INFO_FIELD (IdtrProfile)
o32 lidt [cs:si]
;
@@ -82,7 +82,7 @@ Flat32Start: ; protected mode entry point
mov esi, ebx
mov edi, esi
- add edi, EnableExecuteDisableLocation
+ add edi, MP_CPU_EXCHANGE_INFO_FIELD (EnableExecuteDisable)
cmp byte [edi], 0
jz SkipEnableExecuteDisable
@@ -96,7 +96,7 @@ Flat32Start: ; protected mode entry point
wrmsr
mov edi, esi
- add edi, Cr3Location
+ add edi, MP_CPU_EXCHANGE_INFO_FIELD (Cr3)
mov eax, dword [edi]
mov cr3, eax
@@ -110,19 +110,19 @@ Flat32Start: ; protected mode entry point
SkipEnableExecuteDisable:
mov edi, esi
- add edi, InitFlagLocation
+ add edi, MP_CPU_EXCHANGE_INFO_FIELD (InitFlag)
cmp dword [edi], 1 ; 1 == ApInitConfig
jnz GetApicId
; Increment the number of APs executing here as early as possible
; This is decremented in C code when AP is finished executing
mov edi, esi
- add edi, NumApsExecutingLocation
+ add edi, MP_CPU_EXCHANGE_INFO_FIELD (NumApsExecuting)
lock inc dword [edi]
; AP init
mov edi, esi
- add edi, LockLocation
+ add edi, MP_CPU_EXCHANGE_INFO_FIELD (Lock)
mov eax, NotVacantFlag
TestLock:
@@ -131,7 +131,7 @@ TestLock:
jz TestLock
mov ecx, esi
- add ecx, ApIndexLocation
+ add ecx, MP_CPU_EXCHANGE_INFO_FIELD (ApIndex)
inc dword [ecx]
mov ebx, [ecx]
@@ -140,13 +140,13 @@ Releaselock:
xchg [edi], eax
mov edi, esi
- add edi, StackSizeLocation
+ add edi, MP_CPU_EXCHANGE_INFO_FIELD (StackSize)
mov eax, [edi]
mov ecx, ebx
inc ecx
mul ecx ; EAX = StackSize * (CpuNumber + 1)
mov edi, esi
- add edi, StackStartAddressLocation
+ add edi, MP_CPU_EXCHANGE_INFO_FIELD (StackStart)
add eax, [edi]
mov esp, eax
jmp CProcedureInvoke
@@ -179,18 +179,18 @@ GetProcessorNumber:
; Note that BSP may become an AP due to SwitchBsp()
;
xor ebx, ebx
- lea eax, [esi + CpuInfoLocation]
+ lea eax, [esi + MP_CPU_EXCHANGE_INFO_FIELD (CpuInfo)]
mov edi, [eax]
GetNextProcNumber:
- cmp [edi], edx ; APIC ID match?
+ cmp dword [edi + CPU_INFO_IN_HOB.InitialApicId], edx ; APIC ID match?
jz ProgramStack
- add edi, 20
+ add edi, CPU_INFO_IN_HOB_size
inc ebx
jmp GetNextProcNumber
ProgramStack:
- mov esp, [edi + 12]
+ mov esp, dword [edi + CPU_INFO_IN_HOB.ApTopOfStack]
CProcedureInvoke:
push ebp ; push BIST data at top of AP stack
@@ -203,11 +203,11 @@ CProcedureInvoke:
push ebx ; Push ApIndex
mov eax, esi
- add eax, LockLocation
+ add eax, MP_CPU_EXCHANGE_INFO_OFFSET
push eax ; push address of exchange info data buffer
mov edi, esi
- add edi, ApProcedureLocation
+ add edi, MP_CPU_EXCHANGE_INFO_FIELD (CFunction)
mov eax, [edi]
call eax ; Invoke C function
@@ -270,17 +270,17 @@ ASM_PFX(AsmGetAddressMap):
mov ebp,esp
mov ebx, [ebp + 24h]
- mov dword [ebx], RendezvousFunnelProcStart
- mov dword [ebx + 4h], Flat32Start - RendezvousFunnelProcStart
- mov dword [ebx + 8h], RendezvousFunnelProcEnd - RendezvousFunnelProcStart
- mov dword [ebx + 0Ch], AsmRelocateApLoopStart
- mov dword [ebx + 10h], AsmRelocateApLoopEnd - AsmRelocateApLoopStart
- mov dword [ebx + 14h], Flat32Start - RendezvousFunnelProcStart
- mov dword [ebx + 18h], SwitchToRealProcEnd - SwitchToRealProcStart ; SwitchToRealSize
- mov dword [ebx + 1Ch], SwitchToRealProcStart - RendezvousFunnelProcStart ; SwitchToRealOffset
- mov dword [ebx + 20h], SwitchToRealProcStart - Flat32Start ; SwitchToRealNoNxOffset
- mov dword [ebx + 24h], 0 ; SwitchToRealPM16ModeOffset
- mov dword [ebx + 28h], 0 ; SwitchToRealPM16ModeSize
+ mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.RendezvousFunnelAddress], RendezvousFunnelProcStart
+ mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.ModeEntryOffset], Flat32Start - RendezvousFunnelProcStart
+ mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.RendezvousFunnelSize], RendezvousFunnelProcEnd - RendezvousFunnelProcStart
+ mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.RelocateApLoopFuncAddress], AsmRelocateApLoopStart
+ mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.RelocateApLoopFuncSize], AsmRelocateApLoopEnd - AsmRelocateApLoopStart
+ mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.ModeTransitionOffset], Flat32Start - RendezvousFunnelProcStart
+ mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.SwitchToRealSize], SwitchToRealProcEnd - SwitchToRealProcStart
+ mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.SwitchToRealOffset], SwitchToRealProcStart - RendezvousFunnelProcStart
+ mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.SwitchToRealNoNxOffset], SwitchToRealProcStart - Flat32Start
+ mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.SwitchToRealPM16ModeOffset], 0
+ mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.SwitchToRealPM16ModeSize], 0
popad
ret
@@ -310,18 +310,18 @@ ASM_PFX(AsmExchangeRole):
mov eax, cr0
push eax
- sgdt [esi + 8]
- sidt [esi + 14]
+ sgdt [esi + CPU_EXCHANGE_ROLE_INFO.Gdtr]
+ sidt [esi + CPU_EXCHANGE_ROLE_INFO.Idtr]
; Store the its StackPointer
- mov [esi + 4],esp
+ mov [esi + CPU_EXCHANGE_ROLE_INFO.StackPointer],esp
; update its switch state to STORED
- mov byte [esi], CPU_SWITCH_STATE_STORED
+ mov byte [esi + CPU_EXCHANGE_ROLE_INFO.State], CPU_SWITCH_STATE_STORED
WaitForOtherStored:
; wait until the other CPU finish storing its state
- cmp byte [edi], CPU_SWITCH_STATE_STORED
+ cmp byte [edi + CPU_EXCHANGE_ROLE_INFO.State], CPU_SWITCH_STATE_STORED
jz OtherStored
pause
jmp WaitForOtherStored
@@ -329,21 +329,21 @@ WaitForOtherStored:
OtherStored:
; Since another CPU already stored its state, load them
; load GDTR value
- lgdt [edi + 8]
+ lgdt [edi + CPU_EXCHANGE_ROLE_INFO.Gdtr]
; load IDTR value
- lidt [edi + 14]
+ lidt [edi + CPU_EXCHANGE_ROLE_INFO.Idtr]
; load its future StackPointer
- mov esp, [edi + 4]
+ mov esp, [edi + CPU_EXCHANGE_ROLE_INFO.StackPointer]
; update the other CPU's switch state to LOADED
- mov byte [edi], CPU_SWITCH_STATE_LOADED
+ mov byte [edi + CPU_EXCHANGE_ROLE_INFO.State], CPU_SWITCH_STATE_LOADED
WaitForOtherLoaded:
; wait until the other CPU finish loading new state,
; otherwise the data in stack may corrupt
- cmp byte [esi], CPU_SWITCH_STATE_LOADED
+ cmp byte [esi + CPU_EXCHANGE_ROLE_INFO.State], CPU_SWITCH_STATE_LOADED
jz OtherLoaded
pause
jmp WaitForOtherLoaded
diff --git a/UefiCpuPkg/Library/MpInitLib/MpEqu.inc b/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
new file mode 100644
index 0000000000..46c2b5c116
--- /dev/null
+++ b/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
@@ -0,0 +1,103 @@
+;------------------------------------------------------------------------------ ;
+; Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
+; SPDX-License-Identifier: BSD-2-Clause-Patent
+;
+; Module Name:
+;
+; MpEqu.inc
+;
+; Abstract:
+;
+; This is the equates file for Multiple Processor support
+;
+;-------------------------------------------------------------------------------
+%include "Nasm.inc"
+
+VacantFlag equ 00h
+NotVacantFlag equ 0ffh
+
+CPU_SWITCH_STATE_IDLE equ 0
+CPU_SWITCH_STATE_STORED equ 1
+CPU_SWITCH_STATE_LOADED equ 2
+
+;
+; Equivalent NASM structure of MP_ASSEMBLY_ADDRESS_MAP
+;
+struc MP_ASSEMBLY_ADDRESS_MAP
+ .RendezvousFunnelAddress CTYPE_UINTN 1
+ .ModeEntryOffset CTYPE_UINTN 1
+ .RendezvousFunnelSize CTYPE_UINTN 1
+ .RelocateApLoopFuncAddress CTYPE_UINTN 1
+ .RelocateApLoopFuncSize CTYPE_UINTN 1
+ .ModeTransitionOffset CTYPE_UINTN 1
+ .SwitchToRealSize CTYPE_UINTN 1
+ .SwitchToRealOffset CTYPE_UINTN 1
+ .SwitchToRealNoNxOffset CTYPE_UINTN 1
+ .SwitchToRealPM16ModeOffset CTYPE_UINTN 1
+ .SwitchToRealPM16ModeSize CTYPE_UINTN 1
+endstruc
+
+;
+; Equivalent NASM structure of IA32_DESCRIPTOR
+;
+struc IA32_DESCRIPTOR
+ .Limit CTYPE_UINT16 1
+ .Base CTYPE_UINTN 1
+endstruc
+
+;
+; Equivalent NASM structure of CPU_EXCHANGE_ROLE_INFO
+;
+struc CPU_EXCHANGE_ROLE_INFO
+ ; State is defined as UINT8 in C header file
+ ; Define it as UINTN here to guarantee the fields that follow State
+ ; is naturally aligned. The structure layout doesn't change.
+ .State CTYPE_UINTN 1
+ .StackPointer CTYPE_UINTN 1
+ .Gdtr CTYPE_UINT8 IA32_DESCRIPTOR_size
+ .Idtr CTYPE_UINT8 IA32_DESCRIPTOR_size
+endstruc
+
+;
+; Equivalent NASM structure of CPU_INFO_IN_HOB
+;
+struc CPU_INFO_IN_HOB
+ .InitialApicId CTYPE_UINT32 1
+ .ApicId CTYPE_UINT32 1
+ .Health CTYPE_UINT32 1
+ .ApTopOfStack CTYPE_UINT64 1
+endstruc
+
+;
+; Equivalent NASM structure of MP_CPU_EXCHANGE_INFO
+;
+struc MP_CPU_EXCHANGE_INFO
+ .Lock: CTYPE_UINTN 1
+ .StackStart: CTYPE_UINTN 1
+ .StackSize: CTYPE_UINTN 1
+ .CFunction: 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
+ .ModeTransitionSegment: CTYPE_UINT16 1
+ .ModeHighMemory: CTYPE_UINT32 1
+ .ModeHighSegment: CTYPE_UINT16 1
+ .Enable5LevelPaging: CTYPE_BOOLEAN 1
+ .SevEsIsEnabled: CTYPE_BOOLEAN 1
+ .GhcbBase: CTYPE_UINTN 1
+endstruc
+
+MP_CPU_EXCHANGE_INFO_OFFSET equ (SwitchToRealProcEnd - RendezvousFunnelProcStart)
+%define MP_CPU_EXCHANGE_INFO_FIELD(Field) (MP_CPU_EXCHANGE_INFO_OFFSET + MP_CPU_EXCHANGE_INFO. %+ Field)
diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
index 34abf25d43..49b0ffe8be 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
@@ -1,7 +1,7 @@
## @file
# MP Initialize Library instance for PEI driver.
#
-# Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2016 - 2021, Intel Corporation. All rights reserved.<BR>
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
##
@@ -22,14 +22,13 @@ [Defines]
#
[Sources.IA32]
- Ia32/MpEqu.inc
Ia32/MpFuncs.nasm
[Sources.X64]
- X64/MpEqu.inc
X64/MpFuncs.nasm
[Sources.common]
+ MpEqu.inc
PeiMpLib.c
MpLib.c
MpLib.h
diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc b/UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
deleted file mode 100644
index c92daaaffd..0000000000
--- a/UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
+++ /dev/null
@@ -1,45 +0,0 @@
-;------------------------------------------------------------------------------ ;
-; Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
-; SPDX-License-Identifier: BSD-2-Clause-Patent
-;
-; Module Name:
-;
-; MpEqu.inc
-;
-; Abstract:
-;
-; This is the equates file for Multiple Processor support
-;
-;-------------------------------------------------------------------------------
-
-VacantFlag equ 00h
-NotVacantFlag equ 0ffh
-
-CPU_SWITCH_STATE_IDLE equ 0
-CPU_SWITCH_STATE_STORED equ 1
-CPU_SWITCH_STATE_LOADED equ 2
-
-LockLocation equ (SwitchToRealProcEnd - RendezvousFunnelProcStart)
-StackStartAddressLocation equ LockLocation + 08h
-StackSizeLocation equ LockLocation + 10h
-ApProcedureLocation equ LockLocation + 18h
-GdtrLocation equ LockLocation + 20h
-IdtrLocation equ LockLocation + 2Ah
-BufferStartLocation equ LockLocation + 34h
-ModeOffsetLocation equ LockLocation + 3Ch
-ApIndexLocation equ LockLocation + 44h
-CodeSegmentLocation equ LockLocation + 4Ch
-DataSegmentLocation equ LockLocation + 54h
-EnableExecuteDisableLocation equ LockLocation + 5Ch
-Cr3Location equ LockLocation + 64h
-InitFlagLocation equ LockLocation + 6Ch
-CpuInfoLocation equ LockLocation + 74h
-NumApsExecutingLocation equ LockLocation + 7Ch
-InitializeFloatingPointUnitsAddress equ LockLocation + 8Ch
-ModeTransitionMemoryLocation equ LockLocation + 94h
-ModeTransitionSegmentLocation equ LockLocation + 98h
-ModeHighMemoryLocation equ LockLocation + 9Ah
-ModeHighSegmentLocation equ LockLocation + 9Eh
-Enable5LevelPagingLocation equ LockLocation + 0A0h
-SevEsIsEnabledLocation equ LockLocation + 0A1h
-GhcbBaseLocation equ LockLocation + 0A2h
diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
index aecfd07bc0..bf7faaf60b 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
@@ -1,5 +1,5 @@
;------------------------------------------------------------------------------ ;
-; Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
+; Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
; SPDX-License-Identifier: BSD-2-Clause-Patent
;
; Module Name:
@@ -43,21 +43,21 @@ BITS 16
mov fs, ax
mov gs, ax
- mov si, BufferStartLocation
+ mov si, MP_CPU_EXCHANGE_INFO_FIELD (BufferStart)
mov ebx, [si]
- mov si, DataSegmentLocation
+ mov si, MP_CPU_EXCHANGE_INFO_FIELD (DataSegment)
mov edx, [si]
;
; Get start address of 32-bit code in low memory (<1MB)
;
- mov edi, ModeTransitionMemoryLocation
+ mov edi, MP_CPU_EXCHANGE_INFO_FIELD (ModeTransitionMemory)
- mov si, GdtrLocation
+ mov si, MP_CPU_EXCHANGE_INFO_FIELD (GdtrProfile)
o32 lgdt [cs:si]
- mov si, IdtrLocation
+ mov si, MP_CPU_EXCHANGE_INFO_FIELD (IdtrProfile)
o32 lidt [cs:si]
;
@@ -85,7 +85,7 @@ Flat32Start: ; protected mode entry point
;
; Enable execute disable bit
;
- mov esi, EnableExecuteDisableLocation
+ mov esi, MP_CPU_EXCHANGE_INFO_FIELD (EnableExecuteDisable)
cmp byte [ebx + esi], 0
jz SkipEnableExecuteDisableBit
@@ -101,7 +101,7 @@ SkipEnableExecuteDisableBit:
mov eax, cr4
bts eax, 5
- mov esi, Enable5LevelPagingLocation
+ mov esi, MP_CPU_EXCHANGE_INFO_FIELD (Enable5LevelPaging)
cmp byte [ebx + esi], 0
jz SkipEnable5LevelPaging
@@ -117,7 +117,7 @@ SkipEnable5LevelPaging:
;
; Load page table
;
- mov esi, Cr3Location ; Save CR3 in ecx
+ mov esi, MP_CPU_EXCHANGE_INFO_FIELD (Cr3) ; Save CR3 in ecx
mov ecx, [ebx + esi]
mov cr3, ecx ; Load CR3
@@ -139,26 +139,26 @@ SkipEnable5LevelPaging:
;
; Far jump to 64-bit code
;
- mov edi, ModeHighMemoryLocation
+ mov edi, MP_CPU_EXCHANGE_INFO_FIELD (ModeHighMemory)
add edi, ebx
jmp far [edi]
BITS 64
LongModeStart:
mov esi, ebx
- lea edi, [esi + InitFlagLocation]
+ lea edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (InitFlag)]
cmp qword [edi], 1 ; ApInitConfig
jnz GetApicId
; Increment the number of APs executing here as early as possible
; This is decremented in C code when AP is finished executing
mov edi, esi
- add edi, NumApsExecutingLocation
+ add edi, MP_CPU_EXCHANGE_INFO_FIELD (NumApsExecuting)
lock inc dword [edi]
; AP init
mov edi, esi
- add edi, LockLocation
+ add edi, MP_CPU_EXCHANGE_INFO_FIELD (Lock)
mov rax, NotVacantFlag
TestLock:
@@ -166,7 +166,7 @@ TestLock:
cmp rax, NotVacantFlag
jz TestLock
- lea ecx, [esi + ApIndexLocation]
+ lea ecx, [esi + MP_CPU_EXCHANGE_INFO_FIELD (ApIndex)]
inc dword [ecx]
mov ebx, [ecx]
@@ -175,17 +175,17 @@ Releaselock:
xchg qword [edi], rax
; program stack
mov edi, esi
- add edi, StackSizeLocation
+ add edi, MP_CPU_EXCHANGE_INFO_FIELD (StackSize)
mov eax, dword [edi]
mov ecx, ebx
inc ecx
mul ecx ; EAX = StackSize * (CpuNumber + 1)
mov edi, esi
- add edi, StackStartAddressLocation
+ add edi, MP_CPU_EXCHANGE_INFO_FIELD (StackStart)
add rax, qword [edi]
mov rsp, rax
- lea edi, [esi + SevEsIsEnabledLocation]
+ lea edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (SevEsIsEnabled)]
cmp byte [edi], 1 ; SevEsIsEnabled
jne CProcedureInvoke
@@ -199,7 +199,7 @@ Releaselock:
mov ecx, ebx
mul ecx ; EAX = SIZE_4K * 2 * CpuNumber
mov edi, esi
- add edi, GhcbBaseLocation
+ add edi, MP_CPU_EXCHANGE_INFO_FIELD (GhcbBase)
add rax, qword [edi]
mov rdx, rax
shr rdx, 32
@@ -208,7 +208,7 @@ Releaselock:
jmp CProcedureInvoke
GetApicId:
- lea edi, [esi + SevEsIsEnabledLocation]
+ lea edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (SevEsIsEnabled)]
cmp byte [edi], 1 ; SevEsIsEnabled
jne DoCpuid
@@ -302,18 +302,18 @@ GetProcessorNumber:
; Note that BSP may become an AP due to SwitchBsp()
;
xor ebx, ebx
- lea eax, [esi + CpuInfoLocation]
+ lea eax, [esi + MP_CPU_EXCHANGE_INFO_FIELD (CpuInfo)]
mov rdi, [eax]
GetNextProcNumber:
- cmp dword [rdi], edx ; APIC ID match?
+ cmp dword [rdi + CPU_INFO_IN_HOB.InitialApicId], edx ; APIC ID match?
jz ProgramStack
- add rdi, 20
+ add rdi, CPU_INFO_IN_HOB_size
inc ebx
jmp GetNextProcNumber
ProgramStack:
- mov rsp, qword [rdi + 12]
+ mov rsp, qword [rdi + CPU_INFO_IN_HOB.ApTopOfStack]
CProcedureInvoke:
push rbp ; Push BIST data at top of AP stack
@@ -321,17 +321,17 @@ CProcedureInvoke:
push rbp
mov rbp, rsp
- mov rax, qword [esi + InitializeFloatingPointUnitsAddress]
+ mov rax, qword [esi + MP_CPU_EXCHANGE_INFO_FIELD (InitializeFloatingPointUnits)]
sub rsp, 20h
call rax ; Call assembly function to initialize FPU per UEFI spec
add rsp, 20h
mov edx, ebx ; edx is ApIndex
mov ecx, esi
- add ecx, LockLocation ; rcx is address of exchange info data buffer
+ add ecx, MP_CPU_EXCHANGE_INFO_OFFSET ; rcx is address of exchange info data buffer
mov edi, esi
- add edi, ApProcedureLocation
+ add edi, MP_CPU_EXCHANGE_INFO_FIELD (CFunction)
mov rax, qword [edi]
sub rsp, 20h
@@ -667,18 +667,18 @@ AsmRelocateApLoopEnd:
global ASM_PFX(AsmGetAddressMap)
ASM_PFX(AsmGetAddressMap):
lea rax, [ASM_PFX(RendezvousFunnelProc)]
- mov qword [rcx], rax
- mov qword [rcx + 8h], LongModeStart - RendezvousFunnelProcStart
- mov qword [rcx + 10h], RendezvousFunnelProcEnd - RendezvousFunnelProcStart
+ mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.RendezvousFunnelAddress], rax
+ mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.ModeEntryOffset], LongModeStart - RendezvousFunnelProcStart
+ mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.RendezvousFunnelSize], RendezvousFunnelProcEnd - RendezvousFunnelProcStart
lea rax, [ASM_PFX(AsmRelocateApLoop)]
- mov qword [rcx + 18h], rax
- mov qword [rcx + 20h], AsmRelocateApLoopEnd - AsmRelocateApLoopStart
- mov qword [rcx + 28h], Flat32Start - RendezvousFunnelProcStart
- mov qword [rcx + 30h], SwitchToRealProcEnd - SwitchToRealProcStart ; SwitchToRealSize
- mov qword [rcx + 38h], SwitchToRealProcStart - RendezvousFunnelProcStart ; SwitchToRealOffset
- mov qword [rcx + 40h], SwitchToRealProcStart - Flat32Start ; SwitchToRealNoNxOffset
- mov qword [rcx + 48h], PM16Mode - RendezvousFunnelProcStart ; SwitchToRealPM16ModeOffset
- mov qword [rcx + 50h], SwitchToRealProcEnd - PM16Mode ; SwitchToRealPM16ModeSize
+ mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.RelocateApLoopFuncAddress], rax
+ mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.RelocateApLoopFuncSize], AsmRelocateApLoopEnd - AsmRelocateApLoopStart
+ mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.ModeTransitionOffset], Flat32Start - RendezvousFunnelProcStart
+ mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.SwitchToRealSize], SwitchToRealProcEnd - SwitchToRealProcStart
+ mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.SwitchToRealOffset], SwitchToRealProcStart - RendezvousFunnelProcStart
+ mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.SwitchToRealNoNxOffset], SwitchToRealProcStart - Flat32Start
+ mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.SwitchToRealPM16ModeOffset], PM16Mode - RendezvousFunnelProcStart
+ mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.SwitchToRealPM16ModeSize], SwitchToRealProcEnd - PM16Mode
ret
;-------------------------------------------------------------------------------------
@@ -721,18 +721,18 @@ ASM_PFX(AsmExchangeRole):
;Store EFLAGS, GDTR and IDTR regiter to stack
pushfq
- sgdt [rsi + 16]
- sidt [rsi + 26]
+ sgdt [rsi + CPU_EXCHANGE_ROLE_INFO.Gdtr]
+ sidt [rsi + CPU_EXCHANGE_ROLE_INFO.Idtr]
; Store the its StackPointer
- mov [rsi + 8], rsp
+ mov [rsi + CPU_EXCHANGE_ROLE_INFO.StackPointer], rsp
; update its switch state to STORED
- mov byte [rsi], CPU_SWITCH_STATE_STORED
+ mov byte [rsi + CPU_EXCHANGE_ROLE_INFO.State], CPU_SWITCH_STATE_STORED
WaitForOtherStored:
; wait until the other CPU finish storing its state
- cmp byte [rdi], CPU_SWITCH_STATE_STORED
+ cmp byte [rdi + CPU_EXCHANGE_ROLE_INFO.State], CPU_SWITCH_STATE_STORED
jz OtherStored
pause
jmp WaitForOtherStored
@@ -740,21 +740,21 @@ WaitForOtherStored:
OtherStored:
; Since another CPU already stored its state, load them
; load GDTR value
- lgdt [rdi + 16]
+ lgdt [rdi + CPU_EXCHANGE_ROLE_INFO.Gdtr]
; load IDTR value
- lidt [rdi + 26]
+ lidt [rdi + CPU_EXCHANGE_ROLE_INFO.Idtr]
; load its future StackPointer
- mov rsp, [rdi + 8]
+ mov rsp, [rdi + CPU_EXCHANGE_ROLE_INFO.StackPointer]
; update the other CPU's switch state to LOADED
- mov byte [rdi], CPU_SWITCH_STATE_LOADED
+ mov byte [rdi + CPU_EXCHANGE_ROLE_INFO.State], CPU_SWITCH_STATE_LOADED
WaitForOtherLoaded:
; wait until the other CPU finish loading new state,
; otherwise the data in stack may corrupt
- cmp byte [rsi], CPU_SWITCH_STATE_LOADED
+ cmp byte [rsi + CPU_EXCHANGE_ROLE_INFO.State], CPU_SWITCH_STATE_LOADED
jz OtherLoaded
pause
jmp WaitForOtherLoaded
--
2.27.0.windows.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH v2 3/3] UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release
2021-02-05 7:58 [PATCH v2 0/3] Use XADD to avoid lock acquire/release Ni, Ray
2021-02-05 7:58 ` [PATCH v2 1/3] MdePkg/Nasm.inc: add macros for C types used in structure definition Ni, Ray
2021-02-05 7:58 ` [PATCH v2 2/3] UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset Ni, Ray
@ 2021-02-05 7:58 ` Ni, Ray
2021-02-05 15:11 ` [edk2-devel] " Laszlo Ersek
2021-02-05 17:11 ` [edk2-devel] [PATCH v2 0/3] " Michael D Kinney
3 siblings, 1 reply; 11+ messages in thread
From: Ni, Ray @ 2021-02-05 7:58 UTC (permalink / raw)
To: devel; +Cc: Laszlo Ersek, Eric Dong, Rahul1 Kumar
When AP firstly wakes up, MpFuncs.nasm contains below logic to assign
an unique ApIndex to each AP according to who comes first:
---NASM---
mov edi, esi
add edi, MP_CPU_EXCHANGE_INFO_FIELD (Lock)
mov eax, NotVacantFlag
TestLock:
xchg [edi], eax
cmp eax, NotVacantFlag
jz TestLock
mov ecx, esi
add ecx, MP_CPU_EXCHANGE_INFO_FIELD (ApIndex)
inc dword [ecx]
mov ebx, [ecx]
Releaselock:
mov eax, VacantFlag
xchg [edi], eax
---NASM END---
"LOCK INC" cannot be used to increase MP_CPU_EXCHANGE_INFO.ApIndex
because not only the MP_CPU_EXCHANGE_INFO.ApIndex should be
increased, but also the result should be stored to a thread local
general purpose register EBX.
This patch learns from the NASM implementation of
InternalSyncIncrement() to use "XADD" instruction which can increase
the global ApIndex and store the original ApIndex to EBX in one
instruction.
With this patch, OVMF when running in a 255 threads QEMU spends about
one second to wakeup all APs. Original implementation needs more than
10 seconds.
Signed-off-by: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Rahul1 Kumar <rahul1.kumar@intel.com>
---
.../Library/MpInitLib/Ia32/MpFuncs.nasm | 20 ++++---------------
UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 4 ----
UefiCpuPkg/Library/MpInitLib/MpLib.c | 1 -
UefiCpuPkg/Library/MpInitLib/MpLib.h | 3 +--
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 18 ++++-------------
5 files changed, 9 insertions(+), 37 deletions(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
index 2f1b102717..7bd2415670 100644
--- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
@@ -122,22 +122,10 @@ SkipEnableExecuteDisable:
; AP init
mov edi, esi
- add edi, MP_CPU_EXCHANGE_INFO_FIELD (Lock)
- mov eax, NotVacantFlag
-
-TestLock:
- xchg [edi], eax
- cmp eax, NotVacantFlag
- jz TestLock
-
- mov ecx, esi
- add ecx, MP_CPU_EXCHANGE_INFO_FIELD (ApIndex)
- inc dword [ecx]
- mov ebx, [ecx]
-
-Releaselock:
- mov eax, VacantFlag
- xchg [edi], eax
+ add edi, MP_CPU_EXCHANGE_INFO_FIELD (ApIndex)
+ mov ebx, 1
+ lock xadd dword [edi], ebx ; EBX = ApIndex++
+ inc ebx ; EBX is CpuNumber
mov edi, esi
add edi, MP_CPU_EXCHANGE_INFO_FIELD (StackSize)
diff --git a/UefiCpuPkg/Library/MpInitLib/MpEqu.inc b/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
index 46c2b5c116..2e9368a374 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
+++ b/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
@@ -13,9 +13,6 @@
;-------------------------------------------------------------------------------
%include "Nasm.inc"
-VacantFlag equ 00h
-NotVacantFlag equ 0ffh
-
CPU_SWITCH_STATE_IDLE equ 0
CPU_SWITCH_STATE_STORED equ 1
CPU_SWITCH_STATE_LOADED equ 2
@@ -72,7 +69,6 @@ endstruc
; Equivalent NASM structure of MP_CPU_EXCHANGE_INFO
;
struc MP_CPU_EXCHANGE_INFO
- .Lock: CTYPE_UINTN 1
.StackStart: CTYPE_UINTN 1
.StackSize: CTYPE_UINTN 1
.CFunction: CTYPE_UINTN 1
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 2568986d8c..5040053dad 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -1006,7 +1006,6 @@ FillExchangeInfoData (
IA32_CR4 Cr4;
ExchangeInfo = CpuMpData->MpCpuExchangeInfo;
- ExchangeInfo->Lock = 0;
ExchangeInfo->StackStart = CpuMpData->Buffer;
ExchangeInfo->StackSize = CpuMpData->CpuApStackSize;
ExchangeInfo->BufferStart = CpuMpData->WakeupBuffer;
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
index 02652eaae1..0bd60388b1 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
@@ -1,7 +1,7 @@
/** @file
Common header file for MP Initialize Library.
- Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2016 - 2021, Intel Corporation. All rights reserved.<BR>
Copyright (c) 2020, AMD Inc. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -190,7 +190,6 @@ typedef struct _CPU_MP_DATA CPU_MP_DATA;
// into this structure are used in assembly code in this module
//
typedef struct {
- UINTN Lock;
UINTN StackStart;
UINTN StackSize;
UINTN CFunction;
diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
index bf7faaf60b..50df802d1f 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
@@ -158,21 +158,11 @@ LongModeStart:
; AP init
mov edi, esi
- add edi, MP_CPU_EXCHANGE_INFO_FIELD (Lock)
- mov rax, NotVacantFlag
+ add edi, MP_CPU_EXCHANGE_INFO_FIELD (ApIndex)
+ mov ebx, 1
+ lock xadd dword [edi], ebx ; EBX = ApIndex++
+ inc ebx ; EBX is CpuNumber
-TestLock:
- xchg qword [edi], rax
- cmp rax, NotVacantFlag
- jz TestLock
-
- lea ecx, [esi + MP_CPU_EXCHANGE_INFO_FIELD (ApIndex)]
- inc dword [ecx]
- mov ebx, [ecx]
-
-Releaselock:
- mov rax, VacantFlag
- xchg qword [edi], rax
; program stack
mov edi, esi
add edi, MP_CPU_EXCHANGE_INFO_FIELD (StackSize)
--
2.27.0.windows.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH v2 1/3] MdePkg/Nasm.inc: add macros for C types used in structure definition
2021-02-05 7:58 ` [PATCH v2 1/3] MdePkg/Nasm.inc: add macros for C types used in structure definition Ni, Ray
@ 2021-02-05 15:02 ` Laszlo Ersek
0 siblings, 0 replies; 11+ messages in thread
From: Laszlo Ersek @ 2021-02-05 15:02 UTC (permalink / raw)
To: devel, ray.ni; +Cc: Michael D Kinney, Liming Gao, Zhiguang Liu
On 02/05/21 08:58, Ni, Ray wrote:
> Signed-off-by: Ray Ni <ray.ni@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
> ---
> MdePkg/Include/Ia32/Nasm.inc | 38 ++++++++++++++++++++++++++++++++++++
> MdePkg/Include/X64/Nasm.inc | 38 ++++++++++++++++++++++++++++++++++++
> 2 files changed, 76 insertions(+)
>
> diff --git a/MdePkg/Include/Ia32/Nasm.inc b/MdePkg/Include/Ia32/Nasm.inc
> index 31ce861f1e..017fe5ffd8 100644
> --- a/MdePkg/Include/Ia32/Nasm.inc
> +++ b/MdePkg/Include/Ia32/Nasm.inc
> @@ -20,3 +20,41 @@
> %macro INCSSP_EAX 0
> DB 0xF3, 0x0F, 0xAE, 0xE8
> %endmacro
> +
> +; NASM provides built-in macros STRUC and ENDSTRUC for structure definition.
> +; For example, to define a structure called mytype containing a longword,
> +; a word, a byte and a string of bytes, you might code
> +;
> +; struc mytype
> +;
> +; mt_long: resd 1
> +; mt_word: resw 1
> +; mt_byte: resb 1
> +; mt_str: resb 32
> +;
> +; endstruc
> +;
> +; Below macros are help to map the C types and the RESB family of pseudo-instructions.
> +; So that the above structure definition can be coded as
> +;
> +; struc mytype
> +;
> +; mt_long: CTYPE_UINT32 1
> +; mt_word: CTYPE_UINT16 1
> +; mt_byte: CTYPE_UINT8 1
> +; mt_str: CTYPE_CHAR8 32
> +;
> +; endstruc
> +%define CTYPE_UINT64 resq
> +%define CTYPE_INT64 resq
> +%define CTYPE_UINT32 resd
> +%define CTYPE_INT32 resd
> +%define CTYPE_UINT16 resw
> +%define CTYPE_INT16 resw
> +%define CTYPE_BOOLEAN resb
> +%define CTYPE_UINT8 resb
> +%define CTYPE_CHAR8 resb
> +%define CTYPE_INT8 resb
> +
> +%define CTYPE_UINTN resd
> +%define CTYPE_INTN resd
> diff --git a/MdePkg/Include/X64/Nasm.inc b/MdePkg/Include/X64/Nasm.inc
> index 42412735ea..b48d8680bb 100644
> --- a/MdePkg/Include/X64/Nasm.inc
> +++ b/MdePkg/Include/X64/Nasm.inc
> @@ -20,3 +20,41 @@
> %macro INCSSP_RAX 0
> DB 0xF3, 0x48, 0x0F, 0xAE, 0xE8
> %endmacro
> +
> +; NASM provides built-in macros STRUC and ENDSTRUC for structure definition.
> +; For example, to define a structure called mytype containing a longword,
> +; a word, a byte and a string of bytes, you might code
> +;
> +; struc mytype
> +;
> +; mt_long: resd 1
> +; mt_word: resw 1
> +; mt_byte: resb 1
> +; mt_str: resb 32
> +;
> +; endstruc
> +;
> +; Below macros are help to map the C types and the RESB family of pseudo-instructions.
> +; So that the above structure definition can be coded as
> +;
> +; struc mytype
> +;
> +; mt_long: CTYPE_UINT32 1
> +; mt_word: CTYPE_UINT16 1
> +; mt_byte: CTYPE_UINT8 1
> +; mt_str: CTYPE_CHAR8 32
> +;
> +; endstruc
> +%define CTYPE_UINT64 resq
> +%define CTYPE_INT64 resq
> +%define CTYPE_UINT32 resd
> +%define CTYPE_INT32 resd
> +%define CTYPE_UINT16 resw
> +%define CTYPE_INT16 resw
> +%define CTYPE_BOOLEAN resb
> +%define CTYPE_UINT8 resb
> +%define CTYPE_CHAR8 resb
> +%define CTYPE_INT8 resb
> +
> +%define CTYPE_UINTN resq
> +%define CTYPE_INTN resq
>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH v2 2/3] UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset
2021-02-05 7:58 ` [PATCH v2 2/3] UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset Ni, Ray
@ 2021-02-05 15:09 ` Laszlo Ersek
0 siblings, 0 replies; 11+ messages in thread
From: Laszlo Ersek @ 2021-02-05 15:09 UTC (permalink / raw)
To: devel, ray.ni; +Cc: Eric Dong, Rahul Kumar
On 02/05/21 08:58, Ni, Ray wrote:
> In Windows environment, "dumpbin /disasm" is used to verify the
> disassembly before and after using NASM struc doesn't change.
>
> Signed-off-by: Ray Ni <ray.ni@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> ---
> UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 5 +-
> UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc | 43 --------
> .../Library/MpInitLib/Ia32/MpFuncs.nasm | 82 +++++++-------
> UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 103 ++++++++++++++++++
> UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 5 +-
> UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc | 45 --------
> UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 94 ++++++++--------
> 7 files changed, 195 insertions(+), 182 deletions(-)
> delete mode 100644 UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc
> create mode 100644 UefiCpuPkg/Library/MpInitLib/MpEqu.inc
> delete mode 100644 UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
>
> diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> index 1771575c69..860a9750e2 100644
> --- a/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> @@ -1,7 +1,7 @@
> ## @file
> # MP Initialize Library instance for DXE driver.
> #
> -# Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2016 - 2021, Intel Corporation. All rights reserved.<BR>
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> ##
> @@ -22,14 +22,13 @@ [Defines]
> #
>
> [Sources.IA32]
> - Ia32/MpEqu.inc
> Ia32/MpFuncs.nasm
>
> [Sources.X64]
> - X64/MpEqu.inc
> X64/MpFuncs.nasm
>
> [Sources.common]
> + MpEqu.inc
> DxeMpLib.c
> MpLib.c
> MpLib.h
> diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc b/UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc
> deleted file mode 100644
> index 4f5a7c859a..0000000000
> --- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc
> +++ /dev/null
> @@ -1,43 +0,0 @@
> -;------------------------------------------------------------------------------ ;
> -; Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.<BR>
> -; SPDX-License-Identifier: BSD-2-Clause-Patent
> -;
> -; Module Name:
> -;
> -; MpEqu.inc
> -;
> -; Abstract:
> -;
> -; This is the equates file for Multiple Processor support
> -;
> -;-------------------------------------------------------------------------------
> -
> -VacantFlag equ 00h
> -NotVacantFlag equ 0ffh
> -
> -CPU_SWITCH_STATE_IDLE equ 0
> -CPU_SWITCH_STATE_STORED equ 1
> -CPU_SWITCH_STATE_LOADED equ 2
> -
> -LockLocation equ (SwitchToRealProcEnd - RendezvousFunnelProcStart)
> -StackStartAddressLocation equ LockLocation + 04h
> -StackSizeLocation equ LockLocation + 08h
> -ApProcedureLocation equ LockLocation + 0Ch
> -GdtrLocation equ LockLocation + 10h
> -IdtrLocation equ LockLocation + 16h
> -BufferStartLocation equ LockLocation + 1Ch
> -ModeOffsetLocation equ LockLocation + 20h
> -ApIndexLocation equ LockLocation + 24h
> -CodeSegmentLocation equ LockLocation + 28h
> -DataSegmentLocation equ LockLocation + 2Ch
> -EnableExecuteDisableLocation equ LockLocation + 30h
> -Cr3Location equ LockLocation + 34h
> -InitFlagLocation equ LockLocation + 38h
> -CpuInfoLocation equ LockLocation + 3Ch
> -NumApsExecutingLocation equ LockLocation + 40h
> -InitializeFloatingPointUnitsAddress equ LockLocation + 48h
> -ModeTransitionMemoryLocation equ LockLocation + 4Ch
> -ModeTransitionSegmentLocation equ LockLocation + 50h
> -ModeHighMemoryLocation equ LockLocation + 52h
> -ModeHighSegmentLocation equ LockLocation + 56h
> -
> diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> index 7e81d24aa6..2f1b102717 100644
> --- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> +++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> @@ -1,5 +1,5 @@
> ;------------------------------------------------------------------------------ ;
> -; Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
> +; Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
> ; SPDX-License-Identifier: BSD-2-Clause-Patent
> ;
> ; Module Name:
> @@ -39,21 +39,21 @@ BITS 16
> mov fs, ax
> mov gs, ax
>
> - mov si, BufferStartLocation
> + mov si, MP_CPU_EXCHANGE_INFO_FIELD (BufferStart)
> mov ebx, [si]
>
> - mov si, DataSegmentLocation
> + mov si, MP_CPU_EXCHANGE_INFO_FIELD (DataSegment)
> mov edx, [si]
>
> ;
> ; Get start address of 32-bit code in low memory (<1MB)
> ;
> - mov edi, ModeTransitionMemoryLocation
> + mov edi, MP_CPU_EXCHANGE_INFO_FIELD (ModeTransitionMemory)
>
> - mov si, GdtrLocation
> + mov si, MP_CPU_EXCHANGE_INFO_FIELD (GdtrProfile)
> o32 lgdt [cs:si]
>
> - mov si, IdtrLocation
> + mov si, MP_CPU_EXCHANGE_INFO_FIELD (IdtrProfile)
> o32 lidt [cs:si]
>
> ;
> @@ -82,7 +82,7 @@ Flat32Start: ; protected mode entry point
> mov esi, ebx
>
> mov edi, esi
> - add edi, EnableExecuteDisableLocation
> + add edi, MP_CPU_EXCHANGE_INFO_FIELD (EnableExecuteDisable)
> cmp byte [edi], 0
> jz SkipEnableExecuteDisable
>
> @@ -96,7 +96,7 @@ Flat32Start: ; protected mode entry point
> wrmsr
>
> mov edi, esi
> - add edi, Cr3Location
> + add edi, MP_CPU_EXCHANGE_INFO_FIELD (Cr3)
> mov eax, dword [edi]
> mov cr3, eax
>
> @@ -110,19 +110,19 @@ Flat32Start: ; protected mode entry point
>
> SkipEnableExecuteDisable:
> mov edi, esi
> - add edi, InitFlagLocation
> + add edi, MP_CPU_EXCHANGE_INFO_FIELD (InitFlag)
> cmp dword [edi], 1 ; 1 == ApInitConfig
> jnz GetApicId
>
> ; Increment the number of APs executing here as early as possible
> ; This is decremented in C code when AP is finished executing
> mov edi, esi
> - add edi, NumApsExecutingLocation
> + add edi, MP_CPU_EXCHANGE_INFO_FIELD (NumApsExecuting)
> lock inc dword [edi]
>
> ; AP init
> mov edi, esi
> - add edi, LockLocation
> + add edi, MP_CPU_EXCHANGE_INFO_FIELD (Lock)
> mov eax, NotVacantFlag
>
> TestLock:
> @@ -131,7 +131,7 @@ TestLock:
> jz TestLock
>
> mov ecx, esi
> - add ecx, ApIndexLocation
> + add ecx, MP_CPU_EXCHANGE_INFO_FIELD (ApIndex)
> inc dword [ecx]
> mov ebx, [ecx]
>
> @@ -140,13 +140,13 @@ Releaselock:
> xchg [edi], eax
>
> mov edi, esi
> - add edi, StackSizeLocation
> + add edi, MP_CPU_EXCHANGE_INFO_FIELD (StackSize)
> mov eax, [edi]
> mov ecx, ebx
> inc ecx
> mul ecx ; EAX = StackSize * (CpuNumber + 1)
> mov edi, esi
> - add edi, StackStartAddressLocation
> + add edi, MP_CPU_EXCHANGE_INFO_FIELD (StackStart)
> add eax, [edi]
> mov esp, eax
> jmp CProcedureInvoke
> @@ -179,18 +179,18 @@ GetProcessorNumber:
> ; Note that BSP may become an AP due to SwitchBsp()
> ;
> xor ebx, ebx
> - lea eax, [esi + CpuInfoLocation]
> + lea eax, [esi + MP_CPU_EXCHANGE_INFO_FIELD (CpuInfo)]
> mov edi, [eax]
>
> GetNextProcNumber:
> - cmp [edi], edx ; APIC ID match?
> + cmp dword [edi + CPU_INFO_IN_HOB.InitialApicId], edx ; APIC ID match?
> jz ProgramStack
> - add edi, 20
> + add edi, CPU_INFO_IN_HOB_size
> inc ebx
> jmp GetNextProcNumber
>
> ProgramStack:
> - mov esp, [edi + 12]
> + mov esp, dword [edi + CPU_INFO_IN_HOB.ApTopOfStack]
>
> CProcedureInvoke:
> push ebp ; push BIST data at top of AP stack
> @@ -203,11 +203,11 @@ CProcedureInvoke:
>
> push ebx ; Push ApIndex
> mov eax, esi
> - add eax, LockLocation
> + add eax, MP_CPU_EXCHANGE_INFO_OFFSET
> push eax ; push address of exchange info data buffer
>
> mov edi, esi
> - add edi, ApProcedureLocation
> + add edi, MP_CPU_EXCHANGE_INFO_FIELD (CFunction)
> mov eax, [edi]
>
> call eax ; Invoke C function
> @@ -270,17 +270,17 @@ ASM_PFX(AsmGetAddressMap):
> mov ebp,esp
>
> mov ebx, [ebp + 24h]
> - mov dword [ebx], RendezvousFunnelProcStart
> - mov dword [ebx + 4h], Flat32Start - RendezvousFunnelProcStart
> - mov dword [ebx + 8h], RendezvousFunnelProcEnd - RendezvousFunnelProcStart
> - mov dword [ebx + 0Ch], AsmRelocateApLoopStart
> - mov dword [ebx + 10h], AsmRelocateApLoopEnd - AsmRelocateApLoopStart
> - mov dword [ebx + 14h], Flat32Start - RendezvousFunnelProcStart
> - mov dword [ebx + 18h], SwitchToRealProcEnd - SwitchToRealProcStart ; SwitchToRealSize
> - mov dword [ebx + 1Ch], SwitchToRealProcStart - RendezvousFunnelProcStart ; SwitchToRealOffset
> - mov dword [ebx + 20h], SwitchToRealProcStart - Flat32Start ; SwitchToRealNoNxOffset
> - mov dword [ebx + 24h], 0 ; SwitchToRealPM16ModeOffset
> - mov dword [ebx + 28h], 0 ; SwitchToRealPM16ModeSize
> + mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.RendezvousFunnelAddress], RendezvousFunnelProcStart
> + mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.ModeEntryOffset], Flat32Start - RendezvousFunnelProcStart
> + mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.RendezvousFunnelSize], RendezvousFunnelProcEnd - RendezvousFunnelProcStart
> + mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.RelocateApLoopFuncAddress], AsmRelocateApLoopStart
> + mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.RelocateApLoopFuncSize], AsmRelocateApLoopEnd - AsmRelocateApLoopStart
> + mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.ModeTransitionOffset], Flat32Start - RendezvousFunnelProcStart
> + mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.SwitchToRealSize], SwitchToRealProcEnd - SwitchToRealProcStart
> + mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.SwitchToRealOffset], SwitchToRealProcStart - RendezvousFunnelProcStart
> + mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.SwitchToRealNoNxOffset], SwitchToRealProcStart - Flat32Start
> + mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.SwitchToRealPM16ModeOffset], 0
> + mov dword [ebx + MP_ASSEMBLY_ADDRESS_MAP.SwitchToRealPM16ModeSize], 0
>
> popad
> ret
> @@ -310,18 +310,18 @@ ASM_PFX(AsmExchangeRole):
> mov eax, cr0
> push eax
>
> - sgdt [esi + 8]
> - sidt [esi + 14]
> + sgdt [esi + CPU_EXCHANGE_ROLE_INFO.Gdtr]
> + sidt [esi + CPU_EXCHANGE_ROLE_INFO.Idtr]
>
> ; Store the its StackPointer
> - mov [esi + 4],esp
> + mov [esi + CPU_EXCHANGE_ROLE_INFO.StackPointer],esp
>
> ; update its switch state to STORED
> - mov byte [esi], CPU_SWITCH_STATE_STORED
> + mov byte [esi + CPU_EXCHANGE_ROLE_INFO.State], CPU_SWITCH_STATE_STORED
>
> WaitForOtherStored:
> ; wait until the other CPU finish storing its state
> - cmp byte [edi], CPU_SWITCH_STATE_STORED
> + cmp byte [edi + CPU_EXCHANGE_ROLE_INFO.State], CPU_SWITCH_STATE_STORED
> jz OtherStored
> pause
> jmp WaitForOtherStored
> @@ -329,21 +329,21 @@ WaitForOtherStored:
> OtherStored:
> ; Since another CPU already stored its state, load them
> ; load GDTR value
> - lgdt [edi + 8]
> + lgdt [edi + CPU_EXCHANGE_ROLE_INFO.Gdtr]
>
> ; load IDTR value
> - lidt [edi + 14]
> + lidt [edi + CPU_EXCHANGE_ROLE_INFO.Idtr]
>
> ; load its future StackPointer
> - mov esp, [edi + 4]
> + mov esp, [edi + CPU_EXCHANGE_ROLE_INFO.StackPointer]
>
> ; update the other CPU's switch state to LOADED
> - mov byte [edi], CPU_SWITCH_STATE_LOADED
> + mov byte [edi + CPU_EXCHANGE_ROLE_INFO.State], CPU_SWITCH_STATE_LOADED
>
> WaitForOtherLoaded:
> ; wait until the other CPU finish loading new state,
> ; otherwise the data in stack may corrupt
> - cmp byte [esi], CPU_SWITCH_STATE_LOADED
> + cmp byte [esi + CPU_EXCHANGE_ROLE_INFO.State], CPU_SWITCH_STATE_LOADED
> jz OtherLoaded
> pause
> jmp WaitForOtherLoaded
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpEqu.inc b/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
> new file mode 100644
> index 0000000000..46c2b5c116
> --- /dev/null
> +++ b/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
> @@ -0,0 +1,103 @@
> +;------------------------------------------------------------------------------ ;
> +; Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
> +; SPDX-License-Identifier: BSD-2-Clause-Patent
> +;
> +; Module Name:
> +;
> +; MpEqu.inc
> +;
> +; Abstract:
> +;
> +; This is the equates file for Multiple Processor support
> +;
> +;-------------------------------------------------------------------------------
> +%include "Nasm.inc"
> +
> +VacantFlag equ 00h
> +NotVacantFlag equ 0ffh
> +
> +CPU_SWITCH_STATE_IDLE equ 0
> +CPU_SWITCH_STATE_STORED equ 1
> +CPU_SWITCH_STATE_LOADED equ 2
> +
> +;
> +; Equivalent NASM structure of MP_ASSEMBLY_ADDRESS_MAP
> +;
> +struc MP_ASSEMBLY_ADDRESS_MAP
> + .RendezvousFunnelAddress CTYPE_UINTN 1
> + .ModeEntryOffset CTYPE_UINTN 1
> + .RendezvousFunnelSize CTYPE_UINTN 1
> + .RelocateApLoopFuncAddress CTYPE_UINTN 1
> + .RelocateApLoopFuncSize CTYPE_UINTN 1
> + .ModeTransitionOffset CTYPE_UINTN 1
> + .SwitchToRealSize CTYPE_UINTN 1
> + .SwitchToRealOffset CTYPE_UINTN 1
> + .SwitchToRealNoNxOffset CTYPE_UINTN 1
> + .SwitchToRealPM16ModeOffset CTYPE_UINTN 1
> + .SwitchToRealPM16ModeSize CTYPE_UINTN 1
> +endstruc
> +
> +;
> +; Equivalent NASM structure of IA32_DESCRIPTOR
> +;
> +struc IA32_DESCRIPTOR
> + .Limit CTYPE_UINT16 1
> + .Base CTYPE_UINTN 1
> +endstruc
> +
> +;
> +; Equivalent NASM structure of CPU_EXCHANGE_ROLE_INFO
> +;
> +struc CPU_EXCHANGE_ROLE_INFO
> + ; State is defined as UINT8 in C header file
> + ; Define it as UINTN here to guarantee the fields that follow State
> + ; is naturally aligned. The structure layout doesn't change.
> + .State CTYPE_UINTN 1
> + .StackPointer CTYPE_UINTN 1
> + .Gdtr CTYPE_UINT8 IA32_DESCRIPTOR_size
> + .Idtr CTYPE_UINT8 IA32_DESCRIPTOR_size
> +endstruc
> +
> +;
> +; Equivalent NASM structure of CPU_INFO_IN_HOB
> +;
> +struc CPU_INFO_IN_HOB
> + .InitialApicId CTYPE_UINT32 1
> + .ApicId CTYPE_UINT32 1
> + .Health CTYPE_UINT32 1
> + .ApTopOfStack CTYPE_UINT64 1
> +endstruc
> +
> +;
> +; Equivalent NASM structure of MP_CPU_EXCHANGE_INFO
> +;
> +struc MP_CPU_EXCHANGE_INFO
> + .Lock: CTYPE_UINTN 1
> + .StackStart: CTYPE_UINTN 1
> + .StackSize: CTYPE_UINTN 1
> + .CFunction: 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
> + .ModeTransitionSegment: CTYPE_UINT16 1
> + .ModeHighMemory: CTYPE_UINT32 1
> + .ModeHighSegment: CTYPE_UINT16 1
> + .Enable5LevelPaging: CTYPE_BOOLEAN 1
> + .SevEsIsEnabled: CTYPE_BOOLEAN 1
> + .GhcbBase: CTYPE_UINTN 1
> +endstruc
> +
> +MP_CPU_EXCHANGE_INFO_OFFSET equ (SwitchToRealProcEnd - RendezvousFunnelProcStart)
> +%define MP_CPU_EXCHANGE_INFO_FIELD(Field) (MP_CPU_EXCHANGE_INFO_OFFSET + MP_CPU_EXCHANGE_INFO. %+ Field)
> diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
> index 34abf25d43..49b0ffe8be 100644
> --- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
> +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
> @@ -1,7 +1,7 @@
> ## @file
> # MP Initialize Library instance for PEI driver.
> #
> -# Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2016 - 2021, Intel Corporation. All rights reserved.<BR>
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> ##
> @@ -22,14 +22,13 @@ [Defines]
> #
>
> [Sources.IA32]
> - Ia32/MpEqu.inc
> Ia32/MpFuncs.nasm
>
> [Sources.X64]
> - X64/MpEqu.inc
> X64/MpFuncs.nasm
>
> [Sources.common]
> + MpEqu.inc
> PeiMpLib.c
> MpLib.c
> MpLib.h
> diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc b/UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
> deleted file mode 100644
> index c92daaaffd..0000000000
> --- a/UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
> +++ /dev/null
> @@ -1,45 +0,0 @@
> -;------------------------------------------------------------------------------ ;
> -; Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
> -; SPDX-License-Identifier: BSD-2-Clause-Patent
> -;
> -; Module Name:
> -;
> -; MpEqu.inc
> -;
> -; Abstract:
> -;
> -; This is the equates file for Multiple Processor support
> -;
> -;-------------------------------------------------------------------------------
> -
> -VacantFlag equ 00h
> -NotVacantFlag equ 0ffh
> -
> -CPU_SWITCH_STATE_IDLE equ 0
> -CPU_SWITCH_STATE_STORED equ 1
> -CPU_SWITCH_STATE_LOADED equ 2
> -
> -LockLocation equ (SwitchToRealProcEnd - RendezvousFunnelProcStart)
> -StackStartAddressLocation equ LockLocation + 08h
> -StackSizeLocation equ LockLocation + 10h
> -ApProcedureLocation equ LockLocation + 18h
> -GdtrLocation equ LockLocation + 20h
> -IdtrLocation equ LockLocation + 2Ah
> -BufferStartLocation equ LockLocation + 34h
> -ModeOffsetLocation equ LockLocation + 3Ch
> -ApIndexLocation equ LockLocation + 44h
> -CodeSegmentLocation equ LockLocation + 4Ch
> -DataSegmentLocation equ LockLocation + 54h
> -EnableExecuteDisableLocation equ LockLocation + 5Ch
> -Cr3Location equ LockLocation + 64h
> -InitFlagLocation equ LockLocation + 6Ch
> -CpuInfoLocation equ LockLocation + 74h
> -NumApsExecutingLocation equ LockLocation + 7Ch
> -InitializeFloatingPointUnitsAddress equ LockLocation + 8Ch
> -ModeTransitionMemoryLocation equ LockLocation + 94h
> -ModeTransitionSegmentLocation equ LockLocation + 98h
> -ModeHighMemoryLocation equ LockLocation + 9Ah
> -ModeHighSegmentLocation equ LockLocation + 9Eh
> -Enable5LevelPagingLocation equ LockLocation + 0A0h
> -SevEsIsEnabledLocation equ LockLocation + 0A1h
> -GhcbBaseLocation equ LockLocation + 0A2h
> diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> index aecfd07bc0..bf7faaf60b 100644
> --- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> +++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> @@ -1,5 +1,5 @@
> ;------------------------------------------------------------------------------ ;
> -; Copyright (c) 2015 - 2019, Intel Corporation. All rights reserved.<BR>
> +; Copyright (c) 2015 - 2021, Intel Corporation. All rights reserved.<BR>
> ; SPDX-License-Identifier: BSD-2-Clause-Patent
> ;
> ; Module Name:
> @@ -43,21 +43,21 @@ BITS 16
> mov fs, ax
> mov gs, ax
>
> - mov si, BufferStartLocation
> + mov si, MP_CPU_EXCHANGE_INFO_FIELD (BufferStart)
> mov ebx, [si]
>
> - mov si, DataSegmentLocation
> + mov si, MP_CPU_EXCHANGE_INFO_FIELD (DataSegment)
> mov edx, [si]
>
> ;
> ; Get start address of 32-bit code in low memory (<1MB)
> ;
> - mov edi, ModeTransitionMemoryLocation
> + mov edi, MP_CPU_EXCHANGE_INFO_FIELD (ModeTransitionMemory)
>
> - mov si, GdtrLocation
> + mov si, MP_CPU_EXCHANGE_INFO_FIELD (GdtrProfile)
> o32 lgdt [cs:si]
>
> - mov si, IdtrLocation
> + mov si, MP_CPU_EXCHANGE_INFO_FIELD (IdtrProfile)
> o32 lidt [cs:si]
>
> ;
> @@ -85,7 +85,7 @@ Flat32Start: ; protected mode entry point
> ;
> ; Enable execute disable bit
> ;
> - mov esi, EnableExecuteDisableLocation
> + mov esi, MP_CPU_EXCHANGE_INFO_FIELD (EnableExecuteDisable)
> cmp byte [ebx + esi], 0
> jz SkipEnableExecuteDisableBit
>
> @@ -101,7 +101,7 @@ SkipEnableExecuteDisableBit:
> mov eax, cr4
> bts eax, 5
>
> - mov esi, Enable5LevelPagingLocation
> + mov esi, MP_CPU_EXCHANGE_INFO_FIELD (Enable5LevelPaging)
> cmp byte [ebx + esi], 0
> jz SkipEnable5LevelPaging
>
> @@ -117,7 +117,7 @@ SkipEnable5LevelPaging:
> ;
> ; Load page table
> ;
> - mov esi, Cr3Location ; Save CR3 in ecx
> + mov esi, MP_CPU_EXCHANGE_INFO_FIELD (Cr3) ; Save CR3 in ecx
> mov ecx, [ebx + esi]
> mov cr3, ecx ; Load CR3
>
> @@ -139,26 +139,26 @@ SkipEnable5LevelPaging:
> ;
> ; Far jump to 64-bit code
> ;
> - mov edi, ModeHighMemoryLocation
> + mov edi, MP_CPU_EXCHANGE_INFO_FIELD (ModeHighMemory)
> add edi, ebx
> jmp far [edi]
>
> BITS 64
> LongModeStart:
> mov esi, ebx
> - lea edi, [esi + InitFlagLocation]
> + lea edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (InitFlag)]
> cmp qword [edi], 1 ; ApInitConfig
> jnz GetApicId
>
> ; Increment the number of APs executing here as early as possible
> ; This is decremented in C code when AP is finished executing
> mov edi, esi
> - add edi, NumApsExecutingLocation
> + add edi, MP_CPU_EXCHANGE_INFO_FIELD (NumApsExecuting)
> lock inc dword [edi]
>
> ; AP init
> mov edi, esi
> - add edi, LockLocation
> + add edi, MP_CPU_EXCHANGE_INFO_FIELD (Lock)
> mov rax, NotVacantFlag
>
> TestLock:
> @@ -166,7 +166,7 @@ TestLock:
> cmp rax, NotVacantFlag
> jz TestLock
>
> - lea ecx, [esi + ApIndexLocation]
> + lea ecx, [esi + MP_CPU_EXCHANGE_INFO_FIELD (ApIndex)]
> inc dword [ecx]
> mov ebx, [ecx]
>
> @@ -175,17 +175,17 @@ Releaselock:
> xchg qword [edi], rax
> ; program stack
> mov edi, esi
> - add edi, StackSizeLocation
> + add edi, MP_CPU_EXCHANGE_INFO_FIELD (StackSize)
> mov eax, dword [edi]
> mov ecx, ebx
> inc ecx
> mul ecx ; EAX = StackSize * (CpuNumber + 1)
> mov edi, esi
> - add edi, StackStartAddressLocation
> + add edi, MP_CPU_EXCHANGE_INFO_FIELD (StackStart)
> add rax, qword [edi]
> mov rsp, rax
>
> - lea edi, [esi + SevEsIsEnabledLocation]
> + lea edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (SevEsIsEnabled)]
> cmp byte [edi], 1 ; SevEsIsEnabled
> jne CProcedureInvoke
>
> @@ -199,7 +199,7 @@ Releaselock:
> mov ecx, ebx
> mul ecx ; EAX = SIZE_4K * 2 * CpuNumber
> mov edi, esi
> - add edi, GhcbBaseLocation
> + add edi, MP_CPU_EXCHANGE_INFO_FIELD (GhcbBase)
> add rax, qword [edi]
> mov rdx, rax
> shr rdx, 32
> @@ -208,7 +208,7 @@ Releaselock:
> jmp CProcedureInvoke
>
> GetApicId:
> - lea edi, [esi + SevEsIsEnabledLocation]
> + lea edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (SevEsIsEnabled)]
> cmp byte [edi], 1 ; SevEsIsEnabled
> jne DoCpuid
>
> @@ -302,18 +302,18 @@ GetProcessorNumber:
> ; Note that BSP may become an AP due to SwitchBsp()
> ;
> xor ebx, ebx
> - lea eax, [esi + CpuInfoLocation]
> + lea eax, [esi + MP_CPU_EXCHANGE_INFO_FIELD (CpuInfo)]
> mov rdi, [eax]
>
> GetNextProcNumber:
> - cmp dword [rdi], edx ; APIC ID match?
> + cmp dword [rdi + CPU_INFO_IN_HOB.InitialApicId], edx ; APIC ID match?
> jz ProgramStack
> - add rdi, 20
> + add rdi, CPU_INFO_IN_HOB_size
> inc ebx
> jmp GetNextProcNumber
>
> ProgramStack:
> - mov rsp, qword [rdi + 12]
> + mov rsp, qword [rdi + CPU_INFO_IN_HOB.ApTopOfStack]
>
> CProcedureInvoke:
> push rbp ; Push BIST data at top of AP stack
> @@ -321,17 +321,17 @@ CProcedureInvoke:
> push rbp
> mov rbp, rsp
>
> - mov rax, qword [esi + InitializeFloatingPointUnitsAddress]
> + mov rax, qword [esi + MP_CPU_EXCHANGE_INFO_FIELD (InitializeFloatingPointUnits)]
> sub rsp, 20h
> call rax ; Call assembly function to initialize FPU per UEFI spec
> add rsp, 20h
>
> mov edx, ebx ; edx is ApIndex
> mov ecx, esi
> - add ecx, LockLocation ; rcx is address of exchange info data buffer
> + add ecx, MP_CPU_EXCHANGE_INFO_OFFSET ; rcx is address of exchange info data buffer
>
> mov edi, esi
> - add edi, ApProcedureLocation
> + add edi, MP_CPU_EXCHANGE_INFO_FIELD (CFunction)
> mov rax, qword [edi]
>
> sub rsp, 20h
> @@ -667,18 +667,18 @@ AsmRelocateApLoopEnd:
> global ASM_PFX(AsmGetAddressMap)
> ASM_PFX(AsmGetAddressMap):
> lea rax, [ASM_PFX(RendezvousFunnelProc)]
> - mov qword [rcx], rax
> - mov qword [rcx + 8h], LongModeStart - RendezvousFunnelProcStart
> - mov qword [rcx + 10h], RendezvousFunnelProcEnd - RendezvousFunnelProcStart
> + mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.RendezvousFunnelAddress], rax
> + mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.ModeEntryOffset], LongModeStart - RendezvousFunnelProcStart
> + mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.RendezvousFunnelSize], RendezvousFunnelProcEnd - RendezvousFunnelProcStart
> lea rax, [ASM_PFX(AsmRelocateApLoop)]
> - mov qword [rcx + 18h], rax
> - mov qword [rcx + 20h], AsmRelocateApLoopEnd - AsmRelocateApLoopStart
> - mov qword [rcx + 28h], Flat32Start - RendezvousFunnelProcStart
> - mov qword [rcx + 30h], SwitchToRealProcEnd - SwitchToRealProcStart ; SwitchToRealSize
> - mov qword [rcx + 38h], SwitchToRealProcStart - RendezvousFunnelProcStart ; SwitchToRealOffset
> - mov qword [rcx + 40h], SwitchToRealProcStart - Flat32Start ; SwitchToRealNoNxOffset
> - mov qword [rcx + 48h], PM16Mode - RendezvousFunnelProcStart ; SwitchToRealPM16ModeOffset
> - mov qword [rcx + 50h], SwitchToRealProcEnd - PM16Mode ; SwitchToRealPM16ModeSize
> + mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.RelocateApLoopFuncAddress], rax
> + mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.RelocateApLoopFuncSize], AsmRelocateApLoopEnd - AsmRelocateApLoopStart
> + mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.ModeTransitionOffset], Flat32Start - RendezvousFunnelProcStart
> + mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.SwitchToRealSize], SwitchToRealProcEnd - SwitchToRealProcStart
> + mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.SwitchToRealOffset], SwitchToRealProcStart - RendezvousFunnelProcStart
> + mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.SwitchToRealNoNxOffset], SwitchToRealProcStart - Flat32Start
> + mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.SwitchToRealPM16ModeOffset], PM16Mode - RendezvousFunnelProcStart
> + mov qword [rcx + MP_ASSEMBLY_ADDRESS_MAP.SwitchToRealPM16ModeSize], SwitchToRealProcEnd - PM16Mode
> ret
>
> ;-------------------------------------------------------------------------------------
> @@ -721,18 +721,18 @@ ASM_PFX(AsmExchangeRole):
>
> ;Store EFLAGS, GDTR and IDTR regiter to stack
> pushfq
> - sgdt [rsi + 16]
> - sidt [rsi + 26]
> + sgdt [rsi + CPU_EXCHANGE_ROLE_INFO.Gdtr]
> + sidt [rsi + CPU_EXCHANGE_ROLE_INFO.Idtr]
>
> ; Store the its StackPointer
> - mov [rsi + 8], rsp
> + mov [rsi + CPU_EXCHANGE_ROLE_INFO.StackPointer], rsp
>
> ; update its switch state to STORED
> - mov byte [rsi], CPU_SWITCH_STATE_STORED
> + mov byte [rsi + CPU_EXCHANGE_ROLE_INFO.State], CPU_SWITCH_STATE_STORED
>
> WaitForOtherStored:
> ; wait until the other CPU finish storing its state
> - cmp byte [rdi], CPU_SWITCH_STATE_STORED
> + cmp byte [rdi + CPU_EXCHANGE_ROLE_INFO.State], CPU_SWITCH_STATE_STORED
> jz OtherStored
> pause
> jmp WaitForOtherStored
> @@ -740,21 +740,21 @@ WaitForOtherStored:
> OtherStored:
> ; Since another CPU already stored its state, load them
> ; load GDTR value
> - lgdt [rdi + 16]
> + lgdt [rdi + CPU_EXCHANGE_ROLE_INFO.Gdtr]
>
> ; load IDTR value
> - lidt [rdi + 26]
> + lidt [rdi + CPU_EXCHANGE_ROLE_INFO.Idtr]
>
> ; load its future StackPointer
> - mov rsp, [rdi + 8]
> + mov rsp, [rdi + CPU_EXCHANGE_ROLE_INFO.StackPointer]
>
> ; update the other CPU's switch state to LOADED
> - mov byte [rdi], CPU_SWITCH_STATE_LOADED
> + mov byte [rdi + CPU_EXCHANGE_ROLE_INFO.State], CPU_SWITCH_STATE_LOADED
>
> WaitForOtherLoaded:
> ; wait until the other CPU finish loading new state,
> ; otherwise the data in stack may corrupt
> - cmp byte [rsi], CPU_SWITCH_STATE_LOADED
> + cmp byte [rsi + CPU_EXCHANGE_ROLE_INFO.State], CPU_SWITCH_STATE_LOADED
> jz OtherLoaded
> pause
> jmp WaitForOtherLoaded
>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH v2 3/3] UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release
2021-02-05 7:58 ` [PATCH v2 3/3] UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release Ni, Ray
@ 2021-02-05 15:11 ` Laszlo Ersek
0 siblings, 0 replies; 11+ messages in thread
From: Laszlo Ersek @ 2021-02-05 15:11 UTC (permalink / raw)
To: devel, ray.ni; +Cc: Eric Dong, Rahul1 Kumar
On 02/05/21 08:58, Ni, Ray wrote:
> When AP firstly wakes up, MpFuncs.nasm contains below logic to assign
> an unique ApIndex to each AP according to who comes first:
> ---NASM---
> mov edi, esi
> add edi, MP_CPU_EXCHANGE_INFO_FIELD (Lock)
> mov eax, NotVacantFlag
>
> TestLock:
> xchg [edi], eax
> cmp eax, NotVacantFlag
> jz TestLock
>
> mov ecx, esi
> add ecx, MP_CPU_EXCHANGE_INFO_FIELD (ApIndex)
> inc dword [ecx]
> mov ebx, [ecx]
>
> Releaselock:
> mov eax, VacantFlag
> xchg [edi], eax
> ---NASM END---
>
> "LOCK INC" cannot be used to increase MP_CPU_EXCHANGE_INFO.ApIndex
> because not only the MP_CPU_EXCHANGE_INFO.ApIndex should be
> increased, but also the result should be stored to a thread local
> general purpose register EBX.
>
> This patch learns from the NASM implementation of
> InternalSyncIncrement() to use "XADD" instruction which can increase
> the global ApIndex and store the original ApIndex to EBX in one
> instruction.
>
> With this patch, OVMF when running in a 255 threads QEMU spends about
> one second to wakeup all APs. Original implementation needs more than
> 10 seconds.
>
> Signed-off-by: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Rahul1 Kumar <rahul1.kumar@intel.com>
> ---
> .../Library/MpInitLib/Ia32/MpFuncs.nasm | 20 ++++---------------
> UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 4 ----
> UefiCpuPkg/Library/MpInitLib/MpLib.c | 1 -
> UefiCpuPkg/Library/MpInitLib/MpLib.h | 3 +--
> UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 18 ++++-------------
> 5 files changed, 9 insertions(+), 37 deletions(-)
>
> diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> index 2f1b102717..7bd2415670 100644
> --- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> +++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
> @@ -122,22 +122,10 @@ SkipEnableExecuteDisable:
>
> ; AP init
> mov edi, esi
> - add edi, MP_CPU_EXCHANGE_INFO_FIELD (Lock)
> - mov eax, NotVacantFlag
> -
> -TestLock:
> - xchg [edi], eax
> - cmp eax, NotVacantFlag
> - jz TestLock
> -
> - mov ecx, esi
> - add ecx, MP_CPU_EXCHANGE_INFO_FIELD (ApIndex)
> - inc dword [ecx]
> - mov ebx, [ecx]
> -
> -Releaselock:
> - mov eax, VacantFlag
> - xchg [edi], eax
> + add edi, MP_CPU_EXCHANGE_INFO_FIELD (ApIndex)
> + mov ebx, 1
> + lock xadd dword [edi], ebx ; EBX = ApIndex++
> + inc ebx ; EBX is CpuNumber
>
> mov edi, esi
> add edi, MP_CPU_EXCHANGE_INFO_FIELD (StackSize)
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpEqu.inc b/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
> index 46c2b5c116..2e9368a374 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
> +++ b/UefiCpuPkg/Library/MpInitLib/MpEqu.inc
> @@ -13,9 +13,6 @@
> ;-------------------------------------------------------------------------------
> %include "Nasm.inc"
>
> -VacantFlag equ 00h
> -NotVacantFlag equ 0ffh
> -
> CPU_SWITCH_STATE_IDLE equ 0
> CPU_SWITCH_STATE_STORED equ 1
> CPU_SWITCH_STATE_LOADED equ 2
> @@ -72,7 +69,6 @@ endstruc
> ; Equivalent NASM structure of MP_CPU_EXCHANGE_INFO
> ;
> struc MP_CPU_EXCHANGE_INFO
> - .Lock: CTYPE_UINTN 1
> .StackStart: CTYPE_UINTN 1
> .StackSize: CTYPE_UINTN 1
> .CFunction: CTYPE_UINTN 1
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index 2568986d8c..5040053dad 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -1006,7 +1006,6 @@ FillExchangeInfoData (
> IA32_CR4 Cr4;
>
> ExchangeInfo = CpuMpData->MpCpuExchangeInfo;
> - ExchangeInfo->Lock = 0;
> ExchangeInfo->StackStart = CpuMpData->Buffer;
> ExchangeInfo->StackSize = CpuMpData->CpuApStackSize;
> ExchangeInfo->BufferStart = CpuMpData->WakeupBuffer;
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> index 02652eaae1..0bd60388b1 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> @@ -1,7 +1,7 @@
> /** @file
> Common header file for MP Initialize Library.
>
> - Copyright (c) 2016 - 2020, Intel Corporation. All rights reserved.<BR>
> + Copyright (c) 2016 - 2021, Intel Corporation. All rights reserved.<BR>
> Copyright (c) 2020, AMD Inc. All rights reserved.<BR>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -190,7 +190,6 @@ typedef struct _CPU_MP_DATA CPU_MP_DATA;
> // into this structure are used in assembly code in this module
> //
> typedef struct {
> - UINTN Lock;
> UINTN StackStart;
> UINTN StackSize;
> UINTN CFunction;
> diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> index bf7faaf60b..50df802d1f 100644
> --- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> +++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> @@ -158,21 +158,11 @@ LongModeStart:
>
> ; AP init
> mov edi, esi
> - add edi, MP_CPU_EXCHANGE_INFO_FIELD (Lock)
> - mov rax, NotVacantFlag
> + add edi, MP_CPU_EXCHANGE_INFO_FIELD (ApIndex)
> + mov ebx, 1
> + lock xadd dword [edi], ebx ; EBX = ApIndex++
> + inc ebx ; EBX is CpuNumber
>
> -TestLock:
> - xchg qword [edi], rax
> - cmp rax, NotVacantFlag
> - jz TestLock
> -
> - lea ecx, [esi + MP_CPU_EXCHANGE_INFO_FIELD (ApIndex)]
> - inc dword [ecx]
> - mov ebx, [ecx]
> -
> -Releaselock:
> - mov rax, VacantFlag
> - xchg qword [edi], rax
> ; program stack
> mov edi, esi
> add edi, MP_CPU_EXCHANGE_INFO_FIELD (StackSize)
>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release
2021-02-05 7:58 [PATCH v2 0/3] Use XADD to avoid lock acquire/release Ni, Ray
` (2 preceding siblings ...)
2021-02-05 7:58 ` [PATCH v2 3/3] UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release Ni, Ray
@ 2021-02-05 17:11 ` Michael D Kinney
2021-02-05 18:38 ` Ni, Ray
3 siblings, 1 reply; 11+ messages in thread
From: Michael D Kinney @ 2021-02-05 17:11 UTC (permalink / raw)
To: devel@edk2.groups.io, Ni, Ray, Kinney, Michael D
Hi Ray,
I really like the cleanup to remove hard coded offsets, but I think that change should be its own patch series.
Can we make the functional change to use XADD as its own patch series before the change to remove hard coded offsets and use struct?
Then have a 2nd patch series that is a non-functional change to remove hard coded offsets and use struct and remove the unused Lock field?
Thanks,
Mike
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
> Sent: Thursday, February 4, 2021 11:58 PM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release
>
> Patch #1 follows Laszlo's suggestion to add global NASM macros
>
> for NASM struc usage.
>
> Patch #2 changes all hardcode offset to use struc.
>
> Patch #3 doesn't have any change comparing to V1 except
>
> 1). dword/qword prefix is added.
>
> 2). the comments "program AP stack" is removed.
>
> Ray Ni (3):
> MdePkg/Nasm.inc: add macros for C types used in structure definition
> UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset
> UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release
>
> MdePkg/Include/Ia32/Nasm.inc | 38 ++++++
> MdePkg/Include/X64/Nasm.inc | 38 ++++++
> UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 5 +-
> UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc | 43 -------
> .../Library/MpInitLib/Ia32/MpFuncs.nasm | 98 +++++++---------
> UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 99 ++++++++++++++++
> UefiCpuPkg/Library/MpInitLib/MpLib.c | 1 -
> UefiCpuPkg/Library/MpInitLib/MpLib.h | 3 +-
> UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 5 +-
> UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc | 45 --------
> UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 108 ++++++++----------
> 11 files changed, 272 insertions(+), 211 deletions(-)
> delete mode 100644 UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc
> create mode 100644 UefiCpuPkg/Library/MpInitLib/MpEqu.inc
> delete mode 100644 UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
>
> --
> 2.27.0.windows.1
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#71344): https://edk2.groups.io/g/devel/message/71344
> Mute This Topic: https://groups.io/mt/80401290/1643496
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [michael.d.kinney@intel.com]
> -=-=-=-=-=-=
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release
2021-02-05 17:11 ` [edk2-devel] [PATCH v2 0/3] " Michael D Kinney
@ 2021-02-05 18:38 ` Ni, Ray
2021-02-05 19:54 ` Michael D Kinney
0 siblings, 1 reply; 11+ messages in thread
From: Ni, Ray @ 2021-02-05 18:38 UTC (permalink / raw)
To: Kinney, Michael D, devel@edk2.groups.io
[-- Attachment #1: Type: text/plain, Size: 3246 bytes --]
Mike,
The clean up doesn’t cause any final instruction change and I have verified that.
The reason I put the fix in last because the Lock field is not needed with the fix but removing the Lock requires to adjust all the hard code offsets.
What potential issue do you see?
Thanks,
Ray
thanks,
ray
________________________________
发件人: Kinney, Michael D <michael.d.kinney@intel.com>
发送时间: Saturday, February 6, 2021 1:11:19 AM
收件人: devel@edk2.groups.io <devel@edk2.groups.io>; Ni, Ray <ray.ni@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
主题: RE: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release
Hi Ray,
I really like the cleanup to remove hard coded offsets, but I think that change should be its own patch series.
Can we make the functional change to use XADD as its own patch series before the change to remove hard coded offsets and use struct?
Then have a 2nd patch series that is a non-functional change to remove hard coded offsets and use struct and remove the unused Lock field?
Thanks,
Mike
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
> Sent: Thursday, February 4, 2021 11:58 PM
> To: devel@edk2.groups.io
> Subject: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release
>
> Patch #1 follows Laszlo's suggestion to add global NASM macros
>
> for NASM struc usage.
>
> Patch #2 changes all hardcode offset to use struc.
>
> Patch #3 doesn't have any change comparing to V1 except
>
> 1). dword/qword prefix is added.
>
> 2). the comments "program AP stack" is removed.
>
> Ray Ni (3):
> MdePkg/Nasm.inc: add macros for C types used in structure definition
> UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset
> UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release
>
> MdePkg/Include/Ia32/Nasm.inc | 38 ++++++
> MdePkg/Include/X64/Nasm.inc | 38 ++++++
> UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 5 +-
> UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc | 43 -------
> .../Library/MpInitLib/Ia32/MpFuncs.nasm | 98 +++++++---------
> UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 99 ++++++++++++++++
> UefiCpuPkg/Library/MpInitLib/MpLib.c | 1 -
> UefiCpuPkg/Library/MpInitLib/MpLib.h | 3 +-
> UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 5 +-
> UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc | 45 --------
> UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 108 ++++++++----------
> 11 files changed, 272 insertions(+), 211 deletions(-)
> delete mode 100644 UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc
> create mode 100644 UefiCpuPkg/Library/MpInitLib/MpEqu.inc
> delete mode 100644 UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
>
> --
> 2.27.0.windows.1
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#71344): https://edk2.groups.io/g/devel/message/71344
> Mute This Topic: https://groups.io/mt/80401290/1643496
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [michael.d.kinney@intel.com]
> -=-=-=-=-=-=
>
[-- Attachment #2: Type: text/html, Size: 5882 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release
2021-02-05 18:38 ` Ni, Ray
@ 2021-02-05 19:54 ` Michael D Kinney
2021-02-06 4:32 ` Ni, Ray
0 siblings, 1 reply; 11+ messages in thread
From: Michael D Kinney @ 2021-02-05 19:54 UTC (permalink / raw)
To: Ni, Ray, devel@edk2.groups.io, Kinney, Michael D
[-- Attachment #1: Type: text/plain, Size: 3903 bytes --]
My comment is only to make the history of changes easier to understand by separating the functional and non-functional changes.
Mike
From: Ni, Ray <ray.ni@intel.com>
Sent: Friday, February 5, 2021 10:38 AM
To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release
Mike,
The clean up doesn’t cause any final instruction change and I have verified that.
The reason I put the fix in last because the Lock field is not needed with the fix but removing the Lock requires to adjust all the hard code offsets.
What potential issue do you see?
Thanks,
Ray
thanks,
ray
________________________________
发件人: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
发送时间: Saturday, February 6, 2021 1:11:19 AM
收件人: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
主题: RE: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release
Hi Ray,
I really like the cleanup to remove hard coded offsets, but I think that change should be its own patch series.
Can we make the functional change to use XADD as its own patch series before the change to remove hard coded offsets and use struct?
Then have a 2nd patch series that is a non-functional change to remove hard coded offsets and use struct and remove the unused Lock field?
Thanks,
Mike
> -----Original Message-----
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Ni, Ray
> Sent: Thursday, February 4, 2021 11:58 PM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Subject: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release
>
> Patch #1 follows Laszlo's suggestion to add global NASM macros
>
> for NASM struc usage.
>
> Patch #2 changes all hardcode offset to use struc.
>
> Patch #3 doesn't have any change comparing to V1 except
>
> 1). dword/qword prefix is added.
>
> 2). the comments "program AP stack" is removed.
>
> Ray Ni (3):
> MdePkg/Nasm.inc: add macros for C types used in structure definition
> UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset
> UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release
>
> MdePkg/Include/Ia32/Nasm.inc | 38 ++++++
> MdePkg/Include/X64/Nasm.inc | 38 ++++++
> UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 5 +-
> UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc | 43 -------
> .../Library/MpInitLib/Ia32/MpFuncs.nasm | 98 +++++++---------
> UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 99 ++++++++++++++++
> UefiCpuPkg/Library/MpInitLib/MpLib.c | 1 -
> UefiCpuPkg/Library/MpInitLib/MpLib.h | 3 +-
> UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 5 +-
> UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc | 45 --------
> UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 108 ++++++++----------
> 11 files changed, 272 insertions(+), 211 deletions(-)
> delete mode 100644 UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc
> create mode 100644 UefiCpuPkg/Library/MpInitLib/MpEqu.inc
> delete mode 100644 UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
>
> --
> 2.27.0.windows.1
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#71344): https://edk2.groups.io/g/devel/message/71344
> Mute This Topic: https://groups.io/mt/80401290/1643496
> Group Owner: devel+owner@edk2.groups.io<mailto:devel+owner@edk2.groups.io>
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [michael.d.kinney@intel.com]
> -=-=-=-=-=-=
>
[-- Attachment #2: Type: text/html, Size: 47382 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release
2021-02-05 19:54 ` Michael D Kinney
@ 2021-02-06 4:32 ` Ni, Ray
0 siblings, 0 replies; 11+ messages in thread
From: Ni, Ray @ 2021-02-06 4:32 UTC (permalink / raw)
To: Kinney, Michael D, devel@edk2.groups.io
[-- Attachment #1: Type: text/plain, Size: 4299 bytes --]
I see. Will send a V3.
From: Kinney, Michael D <michael.d.kinney@intel.com>
Sent: Saturday, February 6, 2021 3:54 AM
To: Ni, Ray <ray.ni@intel.com>; devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
Subject: RE: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release
My comment is only to make the history of changes easier to understand by separating the functional and non-functional changes.
Mike
From: Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>
Sent: Friday, February 5, 2021 10:38 AM
To: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
Subject: Re: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release
Mike,
The clean up doesn’t cause any final instruction change and I have verified that.
The reason I put the fix in last because the Lock field is not needed with the fix but removing the Lock requires to adjust all the hard code offsets.
What potential issue do you see?
Thanks,
Ray
thanks,
ray
________________________________
发件人: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
发送时间: Saturday, February 6, 2021 1:11:19 AM
收件人: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Ni, Ray <ray.ni@intel.com<mailto:ray.ni@intel.com>>; Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
主题: RE: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release
Hi Ray,
I really like the cleanup to remove hard coded offsets, but I think that change should be its own patch series.
Can we make the functional change to use XADD as its own patch series before the change to remove hard coded offsets and use struct?
Then have a 2nd patch series that is a non-functional change to remove hard coded offsets and use struct and remove the unused Lock field?
Thanks,
Mike
> -----Original Message-----
> From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Ni, Ray
> Sent: Thursday, February 4, 2021 11:58 PM
> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Subject: [edk2-devel] [PATCH v2 0/3] Use XADD to avoid lock acquire/release
>
> Patch #1 follows Laszlo's suggestion to add global NASM macros
>
> for NASM struc usage.
>
> Patch #2 changes all hardcode offset to use struc.
>
> Patch #3 doesn't have any change comparing to V1 except
>
> 1). dword/qword prefix is added.
>
> 2). the comments "program AP stack" is removed.
>
> Ray Ni (3):
> MdePkg/Nasm.inc: add macros for C types used in structure definition
> UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset
> UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release
>
> MdePkg/Include/Ia32/Nasm.inc | 38 ++++++
> MdePkg/Include/X64/Nasm.inc | 38 ++++++
> UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf | 5 +-
> UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc | 43 -------
> .../Library/MpInitLib/Ia32/MpFuncs.nasm | 98 +++++++---------
> UefiCpuPkg/Library/MpInitLib/MpEqu.inc | 99 ++++++++++++++++
> UefiCpuPkg/Library/MpInitLib/MpLib.c | 1 -
> UefiCpuPkg/Library/MpInitLib/MpLib.h | 3 +-
> UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf | 5 +-
> UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc | 45 --------
> UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 108 ++++++++----------
> 11 files changed, 272 insertions(+), 211 deletions(-)
> delete mode 100644 UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc
> create mode 100644 UefiCpuPkg/Library/MpInitLib/MpEqu.inc
> delete mode 100644 UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc
>
> --
> 2.27.0.windows.1
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#71344): https://edk2.groups.io/g/devel/message/71344
> Mute This Topic: https://groups.io/mt/80401290/1643496
> Group Owner: devel+owner@edk2.groups.io<mailto:devel+owner@edk2.groups.io>
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [michael.d.kinney@intel.com]
> -=-=-=-=-=-=
>
[-- Attachment #2: Type: text/html, Size: 11926 bytes --]
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2021-02-06 4:32 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-02-05 7:58 [PATCH v2 0/3] Use XADD to avoid lock acquire/release Ni, Ray
2021-02-05 7:58 ` [PATCH v2 1/3] MdePkg/Nasm.inc: add macros for C types used in structure definition Ni, Ray
2021-02-05 15:02 ` [edk2-devel] " Laszlo Ersek
2021-02-05 7:58 ` [PATCH v2 2/3] UefiCpuPkg/MpInitLib: Use NASM struc to avoid hardcode offset Ni, Ray
2021-02-05 15:09 ` [edk2-devel] " Laszlo Ersek
2021-02-05 7:58 ` [PATCH v2 3/3] UefiCpuPkg/MpInitLib: Use XADD to avoid lock acquire/release Ni, Ray
2021-02-05 15:11 ` [edk2-devel] " Laszlo Ersek
2021-02-05 17:11 ` [edk2-devel] [PATCH v2 0/3] " Michael D Kinney
2021-02-05 18:38 ` Ni, Ray
2021-02-05 19:54 ` Michael D Kinney
2021-02-06 4:32 ` Ni, Ray
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox