From: "Ni, Ray" <ray.ni@intel.com>
To: Tom Lendacky <thomas.lendacky@amd.com>
Cc: "Dong, Eric" <eric.dong@intel.com>,
"Kumar, Rahul1" <rahul1.kumar@intel.com>,
Michael Roth <michael.roth@amd.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>,
"Xu, Min M" <min.m.xu@intel.com>,
"Yao, Jiewen" <jiewen.yao@intel.com>,
Tom Lendacky <thomas.lendacky@amd.com>,
"Justen, Jordan L" <jordan.l.justen@intel.com>,
Ard Biesheuvel <ardb+tianocore@kernel.org>,
"Aktas, Erdem" <erdemaktas@google.com>,
Gerd Hoffmann <kraxel@redhat.com>,
James Bottomley <jejb@linux.ibm.com>
Subject: Re: [edk2-devel] [PATCH 3/4] MpInitLib: Put SEV logic in separate file
Date: Mon, 9 May 2022 11:54:41 +0000 [thread overview]
Message-ID: <MWHPR11MB16314ADF451DB099676B7EF18CC69@MWHPR11MB1631.namprd11.prod.outlook.com> (raw)
In-Reply-To: <16ECDB685492F55B.14104@groups.io>
Tom,
Can you please review this change? Does it cause any regression to SEV feature?
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
> Sent: Saturday, May 7, 2022 11:13 PM
> To: devel@edk2.groups.io
> Cc: Dong, Eric <eric.dong@intel.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>; Michael Roth <michael.roth@amd.com>;
> James Bottomley <jejb@linux.ibm.com>; Xu, Min M <min.m.xu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Tom
> Lendacky <thomas.lendacky@amd.com>; Justen, Jordan L <jordan.l.justen@intel.com>; Ard Biesheuvel
> <ardb+tianocore@kernel.org>; Aktas, Erdem <erdemaktas@google.com>; Gerd Hoffmann <kraxel@redhat.com>
> Subject: [edk2-devel] [PATCH 3/4] MpInitLib: Put SEV logic in separate file
>
> Signed-off-by: Ray Ni <ray.ni@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Cc: Michael Roth <michael.roth@amd.com>
> Cc: James Bottomley <jejb@linux.ibm.com>
> Cc: Min Xu <min.m.xu@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Erdem Aktas <erdemaktas@google.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> ---
> .../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);
>
> --
> 2.32.0.windows.1
>
>
>
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#89578): https://edk2.groups.io/g/devel/message/89578
> Mute This Topic: https://groups.io/mt/90954628/1712937
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [ray.ni@intel.com]
> -=-=-=-=-=-=
>
next prev parent reply other threads:[~2022-05-09 11:54 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-07 15:13 [PATCH 0/4] Refactor MpInitLib Ni, Ray
2022-05-07 15:13 ` [PATCH 1/4] MpInitLib: Allocate code buffer for PEI phase Ni, Ray
2022-05-07 15:13 ` [PATCH 2/4] MpInitLib: remove unneeded global ASM_PFX Ni, Ray
2022-05-07 15:13 ` [PATCH 3/4] MpInitLib: Put SEV logic in separate file Ni, Ray
2022-05-12 14:13 ` Lendacky, Thomas
2022-05-16 3:51 ` Ni, Ray
2022-05-07 15:13 ` [PATCH 4/4] MpInitLib: Only allocate below 1MB memory for 16bit code Ni, Ray
[not found] ` <16ECDB685492F55B.14104@groups.io>
2022-05-09 11:54 ` Ni, Ray [this message]
2022-05-09 16:35 ` [edk2-devel] [PATCH 3/4] MpInitLib: Put SEV logic in separate file Lendacky, Thomas
2022-05-09 21:39 ` [edk2-devel] [PATCH 0/4] Refactor MpInitLib Lendacky, Thomas
2022-05-09 23:16 ` Ni, Ray
2022-05-10 14:44 ` Lendacky, Thomas
2022-05-10 15:13 ` Lendacky, Thomas
2022-05-12 1:21 ` Ni, Ray
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=MWHPR11MB16314ADF451DB099676B7EF18CC69@MWHPR11MB1631.namprd11.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox