UefiCpuPkg/Library/MpInitLib/X64/MpFuncs.nasm | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
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@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]
GetNextProcNumber:
- cmp dword [edi], edx ; APIC ID match?
+ cmp dword [rdi], edx ; APIC ID match?
jz ProgramStack
- add edi, 20
+ add rdi, 20
inc ebx
jmp GetNextProcNumber
ProgramStack:
- mov rsp, qword [edi + 12]
+ mov rsp, qword [rdi + 12]
CProcedureInvoke:
push rbp ; Push BIST data at top of AP stack
--
2.16.2.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69880): https://edk2.groups.io/g/devel/message/69880
Mute This Topic: https://groups.io/mt/79488123/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
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@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@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/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
Reviewed-by: Ray Ni <ray.ni@intel.com> > -----Original Message----- > From: Laszlo Ersek <lersek@redhat.com> > Sent: Thursday, January 7, 2021 7:52 PM > To: Dong, Guo <guo.dong@intel.com>; devel@edk2.groups.io > Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Kumar, Rahul1 <rahul1.kumar@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@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@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/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.