On 02/04/21 03:59, 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...@intel.com> > Cc: Eric Dong <eric.d...@intel.com> > Cc: Laszlo Ersek <ler...@redhat.com> > Cc: Rahul Kumar <rahul1.ku...@intel.com> > --- > UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc | 52 ++++++++++-------- > .../Library/MpInitLib/Ia32/MpFuncs.nasm | 35 ++++++------ > UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc | 54 ++++++++++--------- > UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 46 ++++++++-------- > 4 files changed, 98 insertions(+), 89 deletions(-) > > diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc > b/UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc > index 4f5a7c859a..244c1e72b7 100644 > --- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc > +++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpEqu.inc > @@ -1,5 +1,5 @@ > > ;------------------------------------------------------------------------------ > ; > -; Copyright (c) 2015 - 2016, 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: > @@ -19,25 +19,31 @@ 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 > - > +MP_CPU_EXCHANGE_INFO_OFFSET equ (SwitchToRealProcEnd - > RendezvousFunnelProcStart) > +struc MP_CPU_EXCHANGE_INFO > + .Lock: resd 1 > + .StackStart: resd 1 > + .StackSize: resd 1 > + .CFunction: resd 1 > + .GdtrProfile: resb 6 > + .IdtrProfile: resb 6 > + .BufferStart: resd 1 > + .ModeOffset: resd 1 > + .ApIndex: resd 1 > + .CodeSegment: resd 1 > + .DataSegment: resd 1 > + .EnableExecuteDisable: resd 1 > + .Cr3: resd 1 > + .InitFlag: resd 1 > + .CpuInfo: resd 1 > + .NumApsExecuting: resd 1 > + .CpuMpData: resd 1 > + .InitializeFloatingPointUnits: resd 1
(1) please align the "res*" on the other lines with this (in the X64 file too) > + .ModeTransitionMemory: resd 1 > + .ModeTransitionSegment:resw 1 > + .ModeHighMemory: resd 1 > + .ModeHighSegment: resw 1 > + .Enable5LevelPaging: resb 1 > + .SevEsIsEnabled: resb 1 > + .GhcbBase: resd 1 > +endstruc > diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm > b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm > index 7e81d24aa6..908c2eb447 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_OFFSET + > MP_CPU_EXCHANGE_INFO.BufferStart (2) please introduce a macro for this; in my opinion, with the currently proposed change, the code is *harder* to read and modify than before. Now we have a lot of fluff to spell out, for every single field reference. Thanks Laszlo > mov ebx, [si] > > - mov si, DataSegmentLocation > + mov si, MP_CPU_EXCHANGE_INFO_OFFSET + > MP_CPU_EXCHANGE_INFO.DataSegment > mov edx, [si] > > ; > ; Get start address of 32-bit code in low memory (<1MB) > ; > - mov edi, ModeTransitionMemoryLocation > + mov edi, MP_CPU_EXCHANGE_INFO_OFFSET + > MP_CPU_EXCHANGE_INFO.ModeTransitionMemory > > - mov si, GdtrLocation > + mov si, MP_CPU_EXCHANGE_INFO_OFFSET + > MP_CPU_EXCHANGE_INFO.GdtrProfile > o32 lgdt [cs:si] > > - mov si, IdtrLocation > + mov si, MP_CPU_EXCHANGE_INFO_OFFSET + > MP_CPU_EXCHANGE_INFO.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_OFFSET + > MP_CPU_EXCHANGE_INFO.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_OFFSET + MP_CPU_EXCHANGE_INFO.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_OFFSET + > MP_CPU_EXCHANGE_INFO.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_OFFSET + > MP_CPU_EXCHANGE_INFO.NumApsExecuting > lock inc dword [edi] > > ; AP init > mov edi, esi > - add edi, LockLocation > + add edi, MP_CPU_EXCHANGE_INFO_OFFSET > mov eax, NotVacantFlag > > TestLock: > @@ -131,7 +131,7 @@ TestLock: > jz TestLock > > mov ecx, esi > - add ecx, ApIndexLocation > + add ecx, MP_CPU_EXCHANGE_INFO_OFFSET + > MP_CPU_EXCHANGE_INFO.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_OFFSET + > MP_CPU_EXCHANGE_INFO.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_OFFSET + > MP_CPU_EXCHANGE_INFO.StackStart > add eax, [edi] > mov esp, eax > jmp CProcedureInvoke > @@ -179,7 +179,7 @@ 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_OFFSET + > MP_CPU_EXCHANGE_INFO.CpuInfo] > mov edi, [eax] > > GetNextProcNumber: > @@ -203,13 +203,12 @@ 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_OFFSET + > MP_CPU_EXCHANGE_INFO.CFunction > mov eax, [edi] > - > call eax ; Invoke C function > > jmp $ ; Never reach here > diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc > b/UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc > index c92daaaffd..3974330991 100644 > --- a/UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc > +++ b/UefiCpuPkg/Library/MpInitLib/X64/MpEqu.inc > @@ -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: > @@ -19,27 +19,31 @@ 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 > +MP_CPU_EXCHANGE_INFO_OFFSET equ (SwitchToRealProcEnd - > RendezvousFunnelProcStart) > +struc MP_CPU_EXCHANGE_INFO > + .Lock: resq 1 > + .StackStart: resq 1 > + .StackSize: resq 1 > + .CFunction: resq 1 > + .GdtrProfile: resb 10 > + .IdtrProfile: resb 10 > + .BufferStart: resq 1 > + .ModeOffset: resq 1 > + .ApIndex: resq 1 > + .CodeSegment: resq 1 > + .DataSegment: resq 1 > + .EnableExecuteDisable: resq 1 > + .Cr3: resq 1 > + .InitFlag: resq 1 > + .CpuInfo: resq 1 > + .NumApsExecuting: resq 1 > + .CpuMpData: resq 1 > + .InitializeFloatingPointUnits: resq 1 > + .ModeTransitionMemory: resd 1 > + .ModeTransitionSegment:resw 1 > + .ModeHighMemory: resd 1 > + .ModeHighSegment: resw 1 > + .Enable5LevelPaging: resb 1 > + .SevEsIsEnabled: resb 1 > + .GhcbBase: resq 1 > +endstruc > diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm > b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm > index aecfd07bc0..423beb2cca 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_OFFSET + > MP_CPU_EXCHANGE_INFO.BufferStart > mov ebx, [si] > > - mov si, DataSegmentLocation > + mov si, MP_CPU_EXCHANGE_INFO_OFFSET + > MP_CPU_EXCHANGE_INFO.DataSegment > mov edx, [si] > > ; > ; Get start address of 32-bit code in low memory (<1MB) > ; > - mov edi, ModeTransitionMemoryLocation > + mov edi, MP_CPU_EXCHANGE_INFO_OFFSET + > MP_CPU_EXCHANGE_INFO.ModeTransitionMemory > > - mov si, GdtrLocation > + mov si, MP_CPU_EXCHANGE_INFO_OFFSET + > MP_CPU_EXCHANGE_INFO.GdtrProfile > o32 lgdt [cs:si] > > - mov si, IdtrLocation > + mov si, MP_CPU_EXCHANGE_INFO_OFFSET + > MP_CPU_EXCHANGE_INFO.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_OFFSET + > MP_CPU_EXCHANGE_INFO.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_OFFSET + > MP_CPU_EXCHANGE_INFO.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_OFFSET + MP_CPU_EXCHANGE_INFO.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_OFFSET + > MP_CPU_EXCHANGE_INFO.ModeHighMemory > add edi, ebx > jmp far [edi] > > BITS 64 > LongModeStart: > mov esi, ebx > - lea edi, [esi + InitFlagLocation] > + lea edi, [esi + MP_CPU_EXCHANGE_INFO_OFFSET + > MP_CPU_EXCHANGE_INFO.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_OFFSET + > MP_CPU_EXCHANGE_INFO.NumApsExecuting > lock inc dword [edi] > > ; AP init > mov edi, esi > - add edi, LockLocation > + add edi, MP_CPU_EXCHANGE_INFO_OFFSET + MP_CPU_EXCHANGE_INFO.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_OFFSET + > MP_CPU_EXCHANGE_INFO.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_OFFSET + > MP_CPU_EXCHANGE_INFO.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_OFFSET + > MP_CPU_EXCHANGE_INFO.StackStart > add rax, qword [edi] > mov rsp, rax > > - lea edi, [esi + SevEsIsEnabledLocation] > + lea edi, [esi + MP_CPU_EXCHANGE_INFO_OFFSET + > MP_CPU_EXCHANGE_INFO.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_OFFSET + > MP_CPU_EXCHANGE_INFO.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_OFFSET + > MP_CPU_EXCHANGE_INFO.SevEsIsEnabled] > cmp byte [edi], 1 ; SevEsIsEnabled > jne DoCpuid > > @@ -302,7 +302,7 @@ 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_OFFSET + > MP_CPU_EXCHANGE_INFO.CpuInfo] > mov rdi, [eax] > > GetNextProcNumber: > @@ -321,17 +321,17 @@ CProcedureInvoke: > push rbp > mov rbp, rsp > > - mov rax, qword [esi + InitializeFloatingPointUnitsAddress] > + mov rax, qword [esi + MP_CPU_EXCHANGE_INFO_OFFSET + > MP_CPU_EXCHANGE_INFO.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_OFFSET + > MP_CPU_EXCHANGE_INFO.CFunction > mov rax, qword [edi] > > sub rsp, 20h > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#71233): https://edk2.groups.io/g/devel/message/71233 Mute This Topic: https://groups.io/mt/80372085/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-