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

Lendacky, Thomas via groups.io thomas.lendacky=amd.com at groups.io
Thu May 12 14:13:00 UTC 2022


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 at intel.com>
> Cc: Eric Dong <eric.dong at intel.com>
> Cc: Ray Ni <ray.ni at intel.com>
> Cc: Rahul Kumar <rahul1.kumar at intel.com>
> Cc: Michael Roth <michael.roth at amd.com>
> Cc: James Bottomley <jejb at linux.ibm.com>
> Cc: Min Xu <min.m.xu at intel.com>
> Cc: Jiewen Yao <jiewen.yao at intel.com>
> Cc: Tom Lendacky <thomas.lendacky at amd.com>
> Cc: Jordan Justen <jordan.l.justen at intel.com>
> Cc: Ard Biesheuvel <ardb+tianocore at kernel.org>
> Cc: Erdem Aktas <erdemaktas at google.com>
> Cc: Gerd Hoffmann <kraxel at 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);


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#89706): https://edk2.groups.io/g/devel/message/89706
Mute This Topic: https://groups.io/mt/90954628/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-




More information about the edk2-devel-archive mailing list