[PATCH 3/4] MpInitLib: Put SEV logic in separate file


Ni, Ray
 

Signed-off-by: Ray Ni <ray.ni@...>
Cc: Eric Dong <eric.dong@...>
Cc: Ray Ni <ray.ni@...>
Cc: Rahul Kumar <rahul1.kumar@...>
Cc: Michael Roth <michael.roth@...>
Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Erdem Aktas <erdemaktas@...>
Cc: Gerd Hoffmann <kraxel@...>
---
.../Library/MpInitLib/Ia32/MpFuncs.nasm | 3 +-
UefiCpuPkg/Library/MpInitLib/MpLib.c | 13 +-
UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm | 148 +++++++++++++++++
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 157 +-----------------
4 files changed, 159 insertions(+), 162 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Li=
brary/MpInitLib/Ia32/MpFuncs.nasm
index 8981c32722..67f9ed05cf 100644
--- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
@@ -199,7 +199,6 @@ CProcedureInvoke:
call eax ; Invoke C function=0D
=0D
jmp $ ; Never reach here=0D
-RendezvousFunnelProcEnd:=0D
=0D
;-------------------------------------------------------------------------=
------------=0D
;SwitchToRealProc procedure follows.=0D
@@ -209,6 +208,8 @@ SwitchToRealProcStart:
jmp $ ; Never reach here=0D
SwitchToRealProcEnd:=0D
=0D
+RendezvousFunnelProcEnd:=0D
+=0D
;-------------------------------------------------------------------------=
------------=0D
; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment, TopOfAp=
Stack, CountTofinish, Pm16CodeSegment, SevEsAPJumpTable, WakeupBuffer);=0D
;=0D
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpIn=
itLib/MpLib.c
index 3dc1b9f872..722ff3fd42 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -938,8 +938,7 @@ FillExchangeInfoData (
// EfiBootServicesCode to avoid page fault if NX memory protection is en=
abled.=0D
//=0D
if (CpuMpData->WakeupBufferHigh !=3D 0) {=0D
- Size =3D CpuMpData->AddressMap.RendezvousFunnelSize +=0D
- CpuMpData->AddressMap.SwitchToRealSize -=0D
+ Size =3D CpuMpData->AddressMap.RendezvousFunnelSize -=0D
CpuMpData->AddressMap.ModeTransitionOffset;=0D
CopyMem (=0D
(VOID *)CpuMpData->WakeupBufferHigh,=0D
@@ -993,8 +992,7 @@ BackupAndPrepareWakeupBuffer (
CopyMem (=0D
(VOID *)CpuMpData->WakeupBuffer,=0D
(VOID *)CpuMpData->AddressMap.RendezvousFunnelAddress,=0D
- CpuMpData->AddressMap.RendezvousFunnelSize +=0D
- CpuMpData->AddressMap.SwitchToRealSize=0D
+ CpuMpData->AddressMap.RendezvousFunnelSize=0D
);=0D
}=0D
=0D
@@ -1031,7 +1029,6 @@ GetApResetVectorSize (
UINTN Size;=0D
=0D
Size =3D AddressMap->RendezvousFunnelSize +=0D
- AddressMap->SwitchToRealSize +=0D
sizeof (MP_CPU_EXCHANGE_INFO);=0D
=0D
return Size;=0D
@@ -1056,11 +1053,9 @@ AllocateResetVector (
CpuMpData->WakeupBuffer =3D GetWakeupBuffer (ApResetVectorSize);=
=0D
CpuMpData->MpCpuExchangeInfo =3D (MP_CPU_EXCHANGE_INFO *)(UINTN)=0D
(CpuMpData->WakeupBuffer +=0D
- CpuMpData->AddressMap.RendezvousFunnel=
Size +=0D
- CpuMpData->AddressMap.SwitchToRealSize=
);=0D
+ CpuMpData->AddressMap.RendezvousFunnel=
Size);=0D
CpuMpData->WakeupBufferHigh =3D AllocateCodeBuffer (=0D
- CpuMpData->AddressMap.RendezvousFunnel=
Size +=0D
- CpuMpData->AddressMap.SwitchToRealSize=
-=0D
+ CpuMpData->AddressMap.RendezvousFunnel=
Size -=0D
CpuMpData->AddressMap.ModeTransitionOf=
fset=0D
);=0D
//=0D
diff --git a/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm b/UefiCpuPkg/Libr=
ary/MpInitLib/X64/AmdSev.nasm
index 8bb1161fa0..7c2469f9c5 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm
@@ -198,3 +198,151 @@ RestoreGhcb:
=0D
SevEsGetApicIdExit:=0D
OneTimeCallRet SevEsGetApicId=0D
+=0D
+=0D
+;-------------------------------------------------------------------------=
------------=0D
+;SwitchToRealProc procedure follows.=0D
+;ALSO THIS PROCEDURE IS EXECUTED BY APs TRANSITIONING TO 16 BIT MODE. HENC=
E THIS PROC=0D
+;IS IN MACHINE CODE.=0D
+; SwitchToRealProc (UINTN BufferStart, UINT16 Code16, UINT16 Code32, UINT=
N StackStart)=0D
+; rcx - Buffer Start=0D
+; rdx - Code16 Selector Offset=0D
+; r8 - Code32 Selector Offset=0D
+; r9 - Stack Start=0D
+;-------------------------------------------------------------------------=
------------=0D
+SwitchToRealProcStart:=0D
+BITS 64=0D
+ cli=0D
+=0D
+ ;=0D
+ ; Get RDX reset value before changing stacks since the=0D
+ ; new stack won't be able to accomodate a #VC exception.=0D
+ ;=0D
+ push rax=0D
+ push rbx=0D
+ push rcx=0D
+ push rdx=0D
+=0D
+ mov rax, 1=0D
+ cpuid=0D
+ mov rsi, rax ; Save off the reset value for =
RDX=0D
+=0D
+ pop rdx=0D
+ pop rcx=0D
+ pop rbx=0D
+ pop rax=0D
+=0D
+ ;=0D
+ ; Establish stack below 1MB=0D
+ ;=0D
+ mov rsp, r9=0D
+=0D
+ ;=0D
+ ; Push ultimate Reset Vector onto the stack=0D
+ ;=0D
+ mov rax, rcx=0D
+ shr rax, 4=0D
+ push word 0x0002 ; RFLAGS=0D
+ push ax ; CS=0D
+ push word 0x0000 ; RIP=0D
+ push word 0x0000 ; For alignment, will be discar=
ded=0D
+=0D
+ ;=0D
+ ; Get address of "16-bit operand size" label=0D
+ ;=0D
+ lea rbx, [PM16Mode]=0D
+=0D
+ ;=0D
+ ; Push addresses used to change to compatibility mode=0D
+ ;=0D
+ lea rax, [CompatMode]=0D
+ push r8=0D
+ push rax=0D
+=0D
+ ;=0D
+ ; Clear R8 - R15, for reset, before going into 32-bit mode=0D
+ ;=0D
+ xor r8, r8=0D
+ xor r9, r9=0D
+ xor r10, r10=0D
+ xor r11, r11=0D
+ xor r12, r12=0D
+ xor r13, r13=0D
+ xor r14, r14=0D
+ xor r15, r15=0D
+=0D
+ ;=0D
+ ; Far return into 32-bit mode=0D
+ ;=0D
+ retfq=0D
+=0D
+BITS 32=0D
+CompatMode:=0D
+ ;=0D
+ ; Set up stack to prepare for exiting protected mode=0D
+ ;=0D
+ push edx ; Code16 CS=0D
+ push ebx ; PM16Mode label address=0D
+=0D
+ ;=0D
+ ; Disable paging=0D
+ ;=0D
+ mov eax, cr0 ; Read CR0=0D
+ btr eax, 31 ; Set PG=3D0=0D
+ mov cr0, eax ; Write CR0=0D
+=0D
+ ;=0D
+ ; Disable long mode=0D
+ ;=0D
+ mov ecx, 0c0000080h ; EFER MSR number=0D
+ rdmsr ; Read EFER=0D
+ btr eax, 8 ; Set LME=3D0=0D
+ wrmsr ; Write EFER=0D
+=0D
+ ;=0D
+ ; Disable PAE=0D
+ ;=0D
+ mov eax, cr4 ; Read CR4=0D
+ btr eax, 5 ; Set PAE=3D0=0D
+ mov cr4, eax ; Write CR4=0D
+=0D
+ mov edx, esi ; Restore RDX reset value=0D
+=0D
+ ;=0D
+ ; Switch to 16-bit operand size=0D
+ ;=0D
+ retf=0D
+=0D
+BITS 16=0D
+ ;=0D
+ ; At entry to this label=0D
+ ; - RDX will have its reset value=0D
+ ; - On the top of the stack=0D
+ ; - Alignment data (two bytes) to be discarded=0D
+ ; - IP for Real Mode (two bytes)=0D
+ ; - CS for Real Mode (two bytes)=0D
+ ;=0D
+ ; This label is also used with AsmRelocateApLoop. During MP finalizati=
on,=0D
+ ; the code from PM16Mode to SwitchToRealProcEnd is copied to the start=
of=0D
+ ; the WakeupBuffer, allowing a parked AP to be booted by an OS.=0D
+ ;=0D
+PM16Mode:=0D
+ mov eax, cr0 ; Read CR0=0D
+ btr eax, 0 ; Set PE=3D0=0D
+ mov cr0, eax ; Write CR0=0D
+=0D
+ pop ax ; Discard alignment data=0D
+=0D
+ ;=0D
+ ; Clear registers (except RDX and RSP) before going into 16-bit mode=0D
+ ;=0D
+ xor eax, eax=0D
+ xor ebx, ebx=0D
+ xor ecx, ecx=0D
+ xor esi, esi=0D
+ xor edi, edi=0D
+ xor ebp, ebp=0D
+=0D
+ iret=0D
+=0D
+SwitchToRealProcEnd:=0D
diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Lib=
rary/MpInitLib/X64/MpFuncs.nasm
index d7e0e1fabd..53df478661 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
@@ -152,11 +152,6 @@ SkipEnable5LevelPaging:
=0D
BITS 64=0D
=0D
-;=0D
-; Required for the AMD SEV helper functions=0D
-;=0D
-%include "AmdSev.nasm"=0D
-=0D
LongModeStart:=0D
mov esi, ebx=0D
lea edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (InitFlag)]=0D
@@ -265,154 +260,12 @@ CProcedureInvoke:
add rsp, 20h=0D
jmp $ ; Should never reach here=0D
=0D
-RendezvousFunnelProcEnd:=0D
-=0D
-;-------------------------------------------------------------------------=
------------=0D
-;SwitchToRealProc procedure follows.=0D
-;ALSO THIS PROCEDURE IS EXECUTED BY APs TRANSITIONING TO 16 BIT MODE. HENC=
E THIS PROC=0D
-;IS IN MACHINE CODE.=0D
-; SwitchToRealProc (UINTN BufferStart, UINT16 Code16, UINT16 Code32, UINT=
N StackStart)=0D
-; rcx - Buffer Start=0D
-; rdx - Code16 Selector Offset=0D
-; r8 - Code32 Selector Offset=0D
-; r9 - Stack Start=0D
-;-------------------------------------------------------------------------=
------------=0D
-SwitchToRealProcStart:=0D
-BITS 64=0D
- cli=0D
-=0D
- ;=0D
- ; Get RDX reset value before changing stacks since the=0D
- ; new stack won't be able to accomodate a #VC exception.=0D
- ;=0D
- push rax=0D
- push rbx=0D
- push rcx=0D
- push rdx=0D
-=0D
- mov rax, 1=0D
- cpuid=0D
- mov rsi, rax ; Save off the reset value for =
RDX=0D
-=0D
- pop rdx=0D
- pop rcx=0D
- pop rbx=0D
- pop rax=0D
-=0D
- ;=0D
- ; Establish stack below 1MB=0D
- ;=0D
- mov rsp, r9=0D
-=0D
- ;=0D
- ; Push ultimate Reset Vector onto the stack=0D
- ;=0D
- mov rax, rcx=0D
- shr rax, 4=0D
- push word 0x0002 ; RFLAGS=0D
- push ax ; CS=0D
- push word 0x0000 ; RIP=0D
- push word 0x0000 ; For alignment, will be discar=
ded=0D
-=0D
- ;=0D
- ; Get address of "16-bit operand size" label=0D
- ;=0D
- lea rbx, [PM16Mode]=0D
-=0D
- ;=0D
- ; Push addresses used to change to compatibility mode=0D
- ;=0D
- lea rax, [CompatMode]=0D
- push r8=0D
- push rax=0D
-=0D
- ;=0D
- ; Clear R8 - R15, for reset, before going into 32-bit mode=0D
- ;=0D
- xor r8, r8=0D
- xor r9, r9=0D
- xor r10, r10=0D
- xor r11, r11=0D
- xor r12, r12=0D
- xor r13, r13=0D
- xor r14, r14=0D
- xor r15, r15=0D
-=0D
- ;=0D
- ; Far return into 32-bit mode=0D
- ;=0D
- retfq=0D
-=0D
-BITS 32=0D
-CompatMode:=0D
- ;=0D
- ; Set up stack to prepare for exiting protected mode=0D
- ;=0D
- push edx ; Code16 CS=0D
- push ebx ; PM16Mode label address=0D
-=0D
- ;=0D
- ; Disable paging=0D
- ;=0D
- mov eax, cr0 ; Read CR0=0D
- btr eax, 31 ; Set PG=3D0=0D
- mov cr0, eax ; Write CR0=0D
-=0D
- ;=0D
- ; Disable long mode=0D
- ;=0D
- mov ecx, 0c0000080h ; EFER MSR number=0D
- rdmsr ; Read EFER=0D
- btr eax, 8 ; Set LME=3D0=0D
- wrmsr ; Write EFER=0D
-=0D
- ;=0D
- ; Disable PAE=0D
- ;=0D
- mov eax, cr4 ; Read CR4=0D
- btr eax, 5 ; Set PAE=3D0=0D
- mov cr4, eax ; Write CR4=0D
-=0D
- mov edx, esi ; Restore RDX reset value=0D
-=0D
- ;=0D
- ; Switch to 16-bit operand size=0D
- ;=0D
- retf=0D
-=0D
-BITS 16=0D
- ;=0D
- ; At entry to this label=0D
- ; - RDX will have its reset value=0D
- ; - On the top of the stack=0D
- ; - Alignment data (two bytes) to be discarded=0D
- ; - IP for Real Mode (two bytes)=0D
- ; - CS for Real Mode (two bytes)=0D
- ;=0D
- ; This label is also used with AsmRelocateApLoop. During MP finalizati=
on,=0D
- ; the code from PM16Mode to SwitchToRealProcEnd is copied to the start=
of=0D
- ; the WakeupBuffer, allowing a parked AP to be booted by an OS.=0D
- ;=0D
-PM16Mode:=0D
- mov eax, cr0 ; Read CR0=0D
- btr eax, 0 ; Set PE=3D0=0D
- mov cr0, eax ; Write CR0=0D
-=0D
- pop ax ; Discard alignment data=0D
-=0D
- ;=0D
- ; Clear registers (except RDX and RSP) before going into 16-bit mode=0D
- ;=0D
- xor eax, eax=0D
- xor ebx, ebx=0D
- xor ecx, ecx=0D
- xor esi, esi=0D
- xor edi, edi=0D
- xor ebp, ebp=0D
-=0D
- iret=0D
+;=0D
+; Required for the AMD SEV helper functions=0D
+;=0D
+%include "AmdSev.nasm"=0D
=0D
-SwitchToRealProcEnd:=0D
+RendezvousFunnelProcEnd:=0D
=0D
;-------------------------------------------------------------------------=
------------=0D
; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment, TopOfAp=
Stack, CountTofinish, Pm16CodeSegment, SevEsAPJumpTable, WakeupBuffer);=0D
--=20
2.32.0.windows.1


Lendacky, Thomas
 

On 5/7/22 10:13, Ray Ni wrote:

Probably should have a commit message here explaining the reason for the changes.

Overall, this works, but it does seem strange to have the SwitchToRealProc procedure in the middle of the RendezvousFunnelProc procedure. Not sure if it would be worth just creating a second SEV nasm file (and maybe renaming the first one):

AmdSevRendevous.nasm
AmdSevSwitchToReal.nasm

and then including the two in different locations.

Then you wouldn't have to change any of the size calculations either.

If you want to keep the function within the function and eliminate the use of SwitchToRealSize, you should probably update the struct in MpLib.h to remove SwitchToRealSize and then update the Ia32 and X64 AsmGetAddressMap function to no longer set SwitchToRealSize (or just reserve the field so that none of the other offsets change and just remove the set in AsmGetAddressMap).

Thanks,
Tom

Signed-off-by: Ray Ni <ray.ni@...>
Cc: Eric Dong <eric.dong@...>
Cc: Ray Ni <ray.ni@...>
Cc: Rahul Kumar <rahul1.kumar@...>
Cc: Michael Roth <michael.roth@...>
Cc: James Bottomley <jejb@...>
Cc: Min Xu <min.m.xu@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Tom Lendacky <thomas.lendacky@...>
Cc: Jordan Justen <jordan.l.justen@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Erdem Aktas <erdemaktas@...>
Cc: Gerd Hoffmann <kraxel@...>
---
.../Library/MpInitLib/Ia32/MpFuncs.nasm | 3 +-
UefiCpuPkg/Library/MpInitLib/MpLib.c | 13 +-
UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm | 148 +++++++++++++++++
UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 157 +-----------------
4 files changed, 159 insertions(+), 162 deletions(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
index 8981c32722..67f9ed05cf 100644
--- a/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/Ia32/MpFuncs.nasm
@@ -199,7 +199,6 @@ CProcedureInvoke:
call eax ; Invoke C function
jmp $ ; Never reach here
-RendezvousFunnelProcEnd:
;-------------------------------------------------------------------------------------
;SwitchToRealProc procedure follows.
@@ -209,6 +208,8 @@ SwitchToRealProcStart:
jmp $ ; Never reach here
SwitchToRealProcEnd:
+RendezvousFunnelProcEnd:
+
;-------------------------------------------------------------------------------------
; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment, TopOfApStack, CountTofinish, Pm16CodeSegment, SevEsAPJumpTable, WakeupBuffer);
;
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 3dc1b9f872..722ff3fd42 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -938,8 +938,7 @@ FillExchangeInfoData (
// EfiBootServicesCode to avoid page fault if NX memory protection is enabled.
//
if (CpuMpData->WakeupBufferHigh != 0) {
- Size = CpuMpData->AddressMap.RendezvousFunnelSize +
- CpuMpData->AddressMap.SwitchToRealSize -
+ Size = CpuMpData->AddressMap.RendezvousFunnelSize -
CpuMpData->AddressMap.ModeTransitionOffset;
CopyMem (
(VOID *)CpuMpData->WakeupBufferHigh,
@@ -993,8 +992,7 @@ BackupAndPrepareWakeupBuffer (
CopyMem (
(VOID *)CpuMpData->WakeupBuffer,
(VOID *)CpuMpData->AddressMap.RendezvousFunnelAddress,
- CpuMpData->AddressMap.RendezvousFunnelSize +
- CpuMpData->AddressMap.SwitchToRealSize
+ CpuMpData->AddressMap.RendezvousFunnelSize
);
}
@@ -1031,7 +1029,6 @@ GetApResetVectorSize (
UINTN Size;
Size = AddressMap->RendezvousFunnelSize +
- AddressMap->SwitchToRealSize +
sizeof (MP_CPU_EXCHANGE_INFO);
return Size;
@@ -1056,11 +1053,9 @@ AllocateResetVector (
CpuMpData->WakeupBuffer = GetWakeupBuffer (ApResetVectorSize);
CpuMpData->MpCpuExchangeInfo = (MP_CPU_EXCHANGE_INFO *)(UINTN)
(CpuMpData->WakeupBuffer +
- CpuMpData->AddressMap.RendezvousFunnelSize +
- CpuMpData->AddressMap.SwitchToRealSize);
+ CpuMpData->AddressMap.RendezvousFunnelSize);
CpuMpData->WakeupBufferHigh = AllocateCodeBuffer (
- CpuMpData->AddressMap.RendezvousFunnelSize +
- CpuMpData->AddressMap.SwitchToRealSize -
+ CpuMpData->AddressMap.RendezvousFunnelSize -
CpuMpData->AddressMap.ModeTransitionOffset
);
//
diff --git a/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm
index 8bb1161fa0..7c2469f9c5 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/AmdSev.nasm
@@ -198,3 +198,151 @@ RestoreGhcb:
SevEsGetApicIdExit:
OneTimeCallRet SevEsGetApicId
+
+
+;-------------------------------------------------------------------------------------
+;SwitchToRealProc procedure follows.
+;ALSO THIS PROCEDURE IS EXECUTED BY APs TRANSITIONING TO 16 BIT MODE. HENCE THIS PROC
+;IS IN MACHINE CODE.
+; SwitchToRealProc (UINTN BufferStart, UINT16 Code16, UINT16 Code32, UINTN StackStart)
+; rcx - Buffer Start
+; rdx - Code16 Selector Offset
+; r8 - Code32 Selector Offset
+; r9 - Stack Start
+;-------------------------------------------------------------------------------------
+SwitchToRealProcStart:
+BITS 64
+ cli
+
+ ;
+ ; Get RDX reset value before changing stacks since the
+ ; new stack won't be able to accomodate a #VC exception.
+ ;
+ push rax
+ push rbx
+ push rcx
+ push rdx
+
+ mov rax, 1
+ cpuid
+ mov rsi, rax ; Save off the reset value for RDX
+
+ pop rdx
+ pop rcx
+ pop rbx
+ pop rax
+
+ ;
+ ; Establish stack below 1MB
+ ;
+ mov rsp, r9
+
+ ;
+ ; Push ultimate Reset Vector onto the stack
+ ;
+ mov rax, rcx
+ shr rax, 4
+ push word 0x0002 ; RFLAGS
+ push ax ; CS
+ push word 0x0000 ; RIP
+ push word 0x0000 ; For alignment, will be discarded
+
+ ;
+ ; Get address of "16-bit operand size" label
+ ;
+ lea rbx, [PM16Mode]
+
+ ;
+ ; Push addresses used to change to compatibility mode
+ ;
+ lea rax, [CompatMode]
+ push r8
+ push rax
+
+ ;
+ ; Clear R8 - R15, for reset, before going into 32-bit mode
+ ;
+ xor r8, r8
+ xor r9, r9
+ xor r10, r10
+ xor r11, r11
+ xor r12, r12
+ xor r13, r13
+ xor r14, r14
+ xor r15, r15
+
+ ;
+ ; Far return into 32-bit mode
+ ;
+ retfq
+
+BITS 32
+CompatMode:
+ ;
+ ; Set up stack to prepare for exiting protected mode
+ ;
+ push edx ; Code16 CS
+ push ebx ; PM16Mode label address
+
+ ;
+ ; Disable paging
+ ;
+ mov eax, cr0 ; Read CR0
+ btr eax, 31 ; Set PG=0
+ mov cr0, eax ; Write CR0
+
+ ;
+ ; Disable long mode
+ ;
+ mov ecx, 0c0000080h ; EFER MSR number
+ rdmsr ; Read EFER
+ btr eax, 8 ; Set LME=0
+ wrmsr ; Write EFER
+
+ ;
+ ; Disable PAE
+ ;
+ mov eax, cr4 ; Read CR4
+ btr eax, 5 ; Set PAE=0
+ mov cr4, eax ; Write CR4
+
+ mov edx, esi ; Restore RDX reset value
+
+ ;
+ ; Switch to 16-bit operand size
+ ;
+ retf
+
+BITS 16
+ ;
+ ; At entry to this label
+ ; - RDX will have its reset value
+ ; - On the top of the stack
+ ; - Alignment data (two bytes) to be discarded
+ ; - IP for Real Mode (two bytes)
+ ; - CS for Real Mode (two bytes)
+ ;
+ ; This label is also used with AsmRelocateApLoop. During MP finalization,
+ ; the code from PM16Mode to SwitchToRealProcEnd is copied to the start of
+ ; the WakeupBuffer, allowing a parked AP to be booted by an OS.
+ ;
+PM16Mode:
+ mov eax, cr0 ; Read CR0
+ btr eax, 0 ; Set PE=0
+ mov cr0, eax ; Write CR0
+
+ pop ax ; Discard alignment data
+
+ ;
+ ; Clear registers (except RDX and RSP) before going into 16-bit mode
+ ;
+ xor eax, eax
+ xor ebx, ebx
+ xor ecx, ecx
+ xor esi, esi
+ xor edi, edi
+ xor ebp, ebp
+
+ iret
+
+SwitchToRealProcEnd:
diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
index d7e0e1fabd..53df478661 100644
--- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
+++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
@@ -152,11 +152,6 @@ SkipEnable5LevelPaging:
BITS 64
-;
-; Required for the AMD SEV helper functions
-;
-%include "AmdSev.nasm"
-
LongModeStart:
mov esi, ebx
lea edi, [esi + MP_CPU_EXCHANGE_INFO_FIELD (InitFlag)]
@@ -265,154 +260,12 @@ CProcedureInvoke:
add rsp, 20h
jmp $ ; Should never reach here
-RendezvousFunnelProcEnd:
-
-;-------------------------------------------------------------------------------------
-;SwitchToRealProc procedure follows.
-;ALSO THIS PROCEDURE IS EXECUTED BY APs TRANSITIONING TO 16 BIT MODE. HENCE THIS PROC
-;IS IN MACHINE CODE.
-; SwitchToRealProc (UINTN BufferStart, UINT16 Code16, UINT16 Code32, UINTN StackStart)
-; rcx - Buffer Start
-; rdx - Code16 Selector Offset
-; r8 - Code32 Selector Offset
-; r9 - Stack Start
-;-------------------------------------------------------------------------------------
-SwitchToRealProcStart:
-BITS 64
- cli
-
- ;
- ; Get RDX reset value before changing stacks since the
- ; new stack won't be able to accomodate a #VC exception.
- ;
- push rax
- push rbx
- push rcx
- push rdx
-
- mov rax, 1
- cpuid
- mov rsi, rax ; Save off the reset value for RDX
-
- pop rdx
- pop rcx
- pop rbx
- pop rax
-
- ;
- ; Establish stack below 1MB
- ;
- mov rsp, r9
-
- ;
- ; Push ultimate Reset Vector onto the stack
- ;
- mov rax, rcx
- shr rax, 4
- push word 0x0002 ; RFLAGS
- push ax ; CS
- push word 0x0000 ; RIP
- push word 0x0000 ; For alignment, will be discarded
-
- ;
- ; Get address of "16-bit operand size" label
- ;
- lea rbx, [PM16Mode]
-
- ;
- ; Push addresses used to change to compatibility mode
- ;
- lea rax, [CompatMode]
- push r8
- push rax
-
- ;
- ; Clear R8 - R15, for reset, before going into 32-bit mode
- ;
- xor r8, r8
- xor r9, r9
- xor r10, r10
- xor r11, r11
- xor r12, r12
- xor r13, r13
- xor r14, r14
- xor r15, r15
-
- ;
- ; Far return into 32-bit mode
- ;
- retfq
-
-BITS 32
-CompatMode:
- ;
- ; Set up stack to prepare for exiting protected mode
- ;
- push edx ; Code16 CS
- push ebx ; PM16Mode label address
-
- ;
- ; Disable paging
- ;
- mov eax, cr0 ; Read CR0
- btr eax, 31 ; Set PG=0
- mov cr0, eax ; Write CR0
-
- ;
- ; Disable long mode
- ;
- mov ecx, 0c0000080h ; EFER MSR number
- rdmsr ; Read EFER
- btr eax, 8 ; Set LME=0
- wrmsr ; Write EFER
-
- ;
- ; Disable PAE
- ;
- mov eax, cr4 ; Read CR4
- btr eax, 5 ; Set PAE=0
- mov cr4, eax ; Write CR4
-
- mov edx, esi ; Restore RDX reset value
-
- ;
- ; Switch to 16-bit operand size
- ;
- retf
-
-BITS 16
- ;
- ; At entry to this label
- ; - RDX will have its reset value
- ; - On the top of the stack
- ; - Alignment data (two bytes) to be discarded
- ; - IP for Real Mode (two bytes)
- ; - CS for Real Mode (two bytes)
- ;
- ; This label is also used with AsmRelocateApLoop. During MP finalization,
- ; the code from PM16Mode to SwitchToRealProcEnd is copied to the start of
- ; the WakeupBuffer, allowing a parked AP to be booted by an OS.
- ;
-PM16Mode:
- mov eax, cr0 ; Read CR0
- btr eax, 0 ; Set PE=0
- mov cr0, eax ; Write CR0
-
- pop ax ; Discard alignment data
-
- ;
- ; Clear registers (except RDX and RSP) before going into 16-bit mode
- ;
- xor eax, eax
- xor ebx, ebx
- xor ecx, ecx
- xor esi, esi
- xor edi, edi
- xor ebp, ebp
-
- iret
+;
+; Required for the AMD SEV helper functions
+;
+%include "AmdSev.nasm"
-SwitchToRealProcEnd:
+RendezvousFunnelProcEnd:
;-------------------------------------------------------------------------------------
; AsmRelocateApLoop (MwaitSupport, ApTargetCState, PmCodeSegment, TopOfApStack, CountTofinish, Pm16CodeSegment, SevEsAPJumpTable, WakeupBuffer);


Ni, Ray
 

-----Original Message-----
From: Tom Lendacky <thomas.lendacky@...>
Sent: Thursday, May 12, 2022 10:13 PM
To: Ni, Ray <ray.ni@...>; devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@...>; Kumar, Rahul1 <rahul1.kumar@...>; Michael Roth <michael.roth@...>;
James Bottomley <jejb@...>; Xu, Min M <min.m.xu@...>; Yao, Jiewen <jiewen.yao@...>; Justen,
Jordan L <jordan.l.justen@...>; Ard Biesheuvel <ardb+tianocore@...>; Aktas, Erdem <erdemaktas@...>;
Gerd Hoffmann <kraxel@...>
Subject: Re: [PATCH 3/4] MpInitLib: Put SEV logic in separate file

On 5/7/22 10:13, Ray Ni wrote:

Probably should have a commit message here explaining the reason for the
changes.

Overall, this works, but it does seem strange to have the SwitchToRealProc
procedure in the middle of the RendezvousFunnelProc procedure. Not sure if
it would be worth just creating a second SEV nasm file (and maybe renaming
the first one):

AmdSevRendevous.nasm
AmdSevSwitchToReal.nasm

and then including the two in different locations.
I would prefer to keep them in one file AmdSev.nasm.



Then you wouldn't have to change any of the size calculations either.

If you want to keep the function within the function and eliminate the use
of SwitchToRealSize, you should probably update the struct in MpLib.h to
remove SwitchToRealSize and then update the Ia32 and X64 AsmGetAddressMap
function to no longer set SwitchToRealSize (or just reserve the field so
that none of the other offsets change and just remove the set in
AsmGetAddressMap).
You are right. I will post v2 patches to remove the SwitchToRealSize/Offset fields.