Felix, I disagree:) Sorry about that. :) The commit you mentioned might be made by me (didn't checked). Because I aimed to avoid calling PEI services from AP. That's a violation of PI spec and not safe by design.
The AP calling standard services concern was raised by Andrew initially. Thanks, Ray > -----Original Message----- > From: Felix Polyudov [mailto:fel...@ami.com] > Sent: Friday, November 30, 2018 8:36 AM > To: edk2-devel@lists.01.org > Cc: Dong, Eric <eric.d...@intel.com>; Ni, Ruiyu <ruiyu...@intel.com>; > ler...@redhat.com > Subject: [Patch] UefiCpuPkg/MpLib: Fix PEI Services Table pointer on AP > > According to PI specification PEI Services table pointer is stored right > before ITD > base. Starting from commit c563077a380437c1 BSP and AP have different IDT > instances. > PEI Services table pointer was not initialized in the AP IDT instance. > As a result, any attempt to use functions from PeiServicesTablePointerLib or > PeiServicesLib on AP caused CPU exception. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Felix Polyudov <fel...@ami.com> > --- > UefiCpuPkg/Library/MpInitLib/MpLib.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c > b/UefiCpuPkg/Library/MpInitLib/MpLib.c > index 7f4d6e6..0e3e362 100644 > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c > @@ -1567,6 +1567,7 @@ MpInitLibInitialize ( > BufferSize = ApStackSize * MaxLogicalProcessorNumber; > BufferSize += MonitorFilterSize * MaxLogicalProcessorNumber; > BufferSize += ApResetVectorSize; > + BufferSize += sizeof(UINTN); > BufferSize = ALIGN_VALUE (BufferSize, 8); > BufferSize += VolatileRegisters.Idtr.Limit + 1; > BufferSize += sizeof (CPU_MP_DATA); > @@ -1587,6 +1588,8 @@ MpInitLibInitialize ( > // Backup Buffer > // +--------------------+ > // Padding > + // +--------------------+ > + // PEI Services Table Pointer > // +--------------------+ <-- ApIdtBase (8-byte boundary) > // AP IDT All APs share one separate IDT. So AP can get > address of > CPU_MP_DATA from IDT Base. > // +--------------------+ <-- CpuMpData > @@ -1599,7 +1602,7 @@ MpInitLibInitialize ( > // > MonitorBuffer = (UINT8 *) (Buffer + ApStackSize * > MaxLogicalProcessorNumber); > BackupBufferAddr = (UINTN) MonitorBuffer + MonitorFilterSize * > MaxLogicalProcessorNumber; > - ApIdtBase = ALIGN_VALUE (BackupBufferAddr + ApResetVectorSize, 8); > + ApIdtBase = ALIGN_VALUE (BackupBufferAddr + ApResetVectorSize + > sizeof(UINTN), 8); > CpuMpData = (CPU_MP_DATA *) (ApIdtBase + > VolatileRegisters.Idtr.Limit + > 1); > CpuMpData->Buffer = Buffer; > CpuMpData->CpuApStackSize = ApStackSize; > @@ -1653,6 +1656,11 @@ MpInitLibInitialize ( > Buffer + BufferSize); > > // > + // Initialize PEI Services table pointer. Copy the address from BSP. > + // > + *(UINTN*)(ApIdtBase - sizeof(UINTN)) = > + *(UINTN*)(VolatileRegisters.Idtr.Base - sizeof (UINTN)); > + > + // > // Duplicate BSP's IDT to APs. > // All APs share one separate IDT. So AP can get the address of CpuMpData > by > using IDTR.BASE + IDTR.LIMIT + 1 > // > -- > 2.10.0.windows.1 > > > > Please consider the environment before printing this email. > > The information contained in this message may be confidential and proprietary > to American Megatrends, Inc. This communication is intended to be read only > by the individual or entity to whom it is addressed or by their designee. If > the > reader of this message is not the intended recipient, you are on notice that > any > distribution of this message, in any form, is strictly prohibited. Please > promptly > notify the sender by reply e-mail or by telephone at 770-246-8600, and then > delete or destroy all copies of the transmission. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
