[edk2-devel] [PATCH] UefiCpuPkg/MpInitLib: Fix a hang in above 4GB case

[Laszlo Ersek]

On 01/07/21 00:53, Guo Dong wrote:
> This patch fixed the hang in UEFICpuPkg when it is dispatched above 4GB.
> In UEFI BIOS case CpuInfoInHob is provided to DXE under 4GB from PEI.
> When using UEFI payload and bootloaders, CpuInfoInHob will be allocated
> above 4GB since it is not provided from bootloader. so we need update
> the code to make sure this hob could be accessed correctly in this case.
> 
> Signed-off-by: Guo Dong <guo.dong at intel.com>
> ---
>  UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> index 5532a1d391..aecfd07bc0 100644
> --- a/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> +++ b/UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm
> @@ -303,17 +303,17 @@ GetProcessorNumber:
>      ;
>      xor         ebx, ebx
>      lea         eax, [esi + CpuInfoLocation]
> -    mov         edi, [eax]
> +    mov         rdi, [eax]
>  

This looks valid; MP_CPU_EXCHANGE_INFO.CpuInfo is a pointer, and on X64,
that means 8 bytes.

>  GetNextProcNumber:
> -    cmp         dword [edi], edx                      ; APIC ID match?
> +    cmp         dword [rdi], edx                      ; APIC ID match?

So this is "CpuInfo->InitialApicId", as far as I can tell. Using rdi
makes sense again.

>      jz          ProgramStack
> -    add         edi, 20
> +    add         rdi, 20

And this seems to advance CpuInfo to the next CPU_INFO_IN_HOB element in
the array (3 UINT32 fields, 1 UINT64 field).

>      inc         ebx
>      jmp         GetNextProcNumber
>  
>  ProgramStack:
> -    mov         rsp, qword [edi + 12]
> +    mov         rsp, qword [rdi + 12]

Seems to be CpuInfo->ApTopOfStack.

>  
>  CProcedureInvoke:
>      push       rbp               ; Push BIST data at top of AP stack
> 

Seems OK to me. Again, given the size of the
"MP_CPU_EXCHANGE_INFO.CpuInfo" pointer field, on X64, this assembly code
should have used rdi from the start, arguably.

Reviewed-by: Laszlo Ersek <lersek at redhat.com>

Thanks
Laszlo



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