[edk2-devel] [PATCH] UefiCpuPkg/MpInitLib: Fix a hang in above 4GB case
Ni, Ray
ray.ni at intel.com
Sat Jan 9 02:21:59 UTC 2021
Reviewed-by: Ray Ni <ray.ni at intel.com>
> -----Original Message-----
> From: Laszlo Ersek <lersek at redhat.com>
> Sent: Thursday, January 7, 2021 7:52 PM
> To: Dong, Guo <guo.dong at intel.com>; devel at edk2.groups.io
> Cc: Dong, Eric <eric.dong at intel.com>; Ni, Ray <ray.ni at intel.com>; Kumar, Rahul1 <rahul1.kumar at intel.com>
> Subject: Re: [edk2-devel] [PATCH] UefiCpuPkg/MpInitLib: Fix a hang in above 4GB case
>
> 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 (#70074): https://edk2.groups.io/g/devel/message/70074
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]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list