From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.100; helo=mga07.intel.com; envelope-from=ruiyu.ni@intel.com; receiver=edk2-devel@lists.01.org Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 817A5203B99AD for ; Sun, 1 Jul 2018 23:01:16 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 01 Jul 2018 23:01:15 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,298,1526367600"; d="scan'208";a="71541079" Received: from ray-dev.ccr.corp.intel.com ([10.239.9.4]) by orsmga002.jf.intel.com with ESMTP; 01 Jul 2018 23:01:14 -0700 From: Ruiyu Ni To: edk2-devel@lists.01.org Cc: Jeff Fan , Eric Dong , Jiewen Yao , Fish Andrew , Laszlo Ersek Date: Mon, 2 Jul 2018 14:01:35 +0800 Message-Id: <20180702060135.264676-1-ruiyu.ni@intel.com> X-Mailer: git-send-email 2.16.1.windows.1 Subject: [PATCH] UefiCpuPkg/MpInitLib: Avoid calling PEI services from AP X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 02 Jul 2018 06:01:16 -0000 Today's MpInitLib PEI implementation directly calls PeiServices->GetHobList() from AP which may cause racing issue. This patch fixes this issue by duplicating IDT for APs. Because CpuMpData structure is stored just after IDT, the CpuMPData address equals to IDTR.BASE + IDTR.LIMIT + 1. v2: 1. Add ALIGN_VALUE() on BufferSize. 2. Add ASSERT() to make sure no memory usage outside of the allocated buffer. 3. Add more comments in InitConfig path when restoring CpuData[0].VolatileRegisters. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ruiyu Ni Cc: Jeff Fan Cc: Eric Dong Cc: Jiewen Yao Cc: Fish Andrew Cc: Laszlo Ersek --- UefiCpuPkg/Library/MpInitLib/MpLib.c | 59 +++++++++++++++++++++++++++------ UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 18 +++++++--- 2 files changed, 63 insertions(+), 14 deletions(-) diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c index eb2765910c..108eea0a6f 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c @@ -615,7 +615,9 @@ ApWakeupFunction ( // ApInitializeSync (CpuMpData); // - // Sync BSP's Control registers to APs + // CpuMpData->CpuData[0].VolatileRegisters is initialized based on BSP environment, + // to initialize AP in InitConfig path. + // NOTE: IDTR.BASE stored in CpuMpData->CpuData[0].VolatileRegisters points to a different IDT shared by all APs. // RestoreVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters, FALSE); InitializeApData (CpuMpData, ProcessorNumber, BistData, ApTopOfStack); @@ -1506,6 +1508,7 @@ MpInitLibInitialize ( UINT32 MaxLogicalProcessorNumber; UINT32 ApStackSize; MP_ASSEMBLY_ADDRESS_MAP AddressMap; + CPU_VOLATILE_REGISTERS VolatileRegisters; UINTN BufferSize; UINT32 MonitorFilterSize; VOID *MpBuffer; @@ -1516,6 +1519,7 @@ MpInitLibInitialize ( UINTN Index; UINTN ApResetVectorSize; UINTN BackupBufferAddr; + UINTN ApIdtBase; OldCpuMpData = GetCpuMpDataFromGuidedHob (); if (OldCpuMpData == NULL) { @@ -1530,19 +1534,48 @@ MpInitLibInitialize ( ApStackSize = PcdGet32(PcdCpuApStackSize); ApLoopMode = GetApLoopMode (&MonitorFilterSize); + // + // Save BSP's Control registers for APs + // + SaveVolatileRegisters (&VolatileRegisters); + BufferSize = ApStackSize * MaxLogicalProcessorNumber; BufferSize += MonitorFilterSize * MaxLogicalProcessorNumber; - BufferSize += sizeof (CPU_MP_DATA); BufferSize += ApResetVectorSize; + BufferSize = ALIGN_VALUE (BufferSize, 8); + BufferSize += VolatileRegisters.Idtr.Limit + 1; + BufferSize += sizeof (CPU_MP_DATA); BufferSize += (sizeof (CPU_AP_DATA) + sizeof (CPU_INFO_IN_HOB))* MaxLogicalProcessorNumber; MpBuffer = AllocatePages (EFI_SIZE_TO_PAGES (BufferSize)); ASSERT (MpBuffer != NULL); ZeroMem (MpBuffer, BufferSize); Buffer = (UINTN) MpBuffer; + // + // The layout of the Buffer is as below: + // + // +--------------------+ <-- Buffer + // AP Stacks (N) + // +--------------------+ <-- MonitorBuffer + // AP Monitor Filters (N) + // +--------------------+ <-- BackupBufferAddr (CpuMpData->BackupBuffer) + // Backup Buffer + // +--------------------+ + // Padding + // +--------------------+ <-- 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 + // CPU_MP_DATA + // +--------------------+ <-- CpuMpData->CpuData + // CPU_AP_DATA (N) + // +--------------------+ <-- CpuMpData->CpuInfoInHob + // CPU_INFO_IN_HOB (N) + // +--------------------+ + // MonitorBuffer = (UINT8 *) (Buffer + ApStackSize * MaxLogicalProcessorNumber); BackupBufferAddr = (UINTN) MonitorBuffer + MonitorFilterSize * MaxLogicalProcessorNumber; - CpuMpData = (CPU_MP_DATA *) (BackupBufferAddr + ApResetVectorSize); + ApIdtBase = ALIGN_VALUE (BackupBufferAddr + ApResetVectorSize, 8); + CpuMpData = (CPU_MP_DATA *) (ApIdtBase + VolatileRegisters.Idtr.Limit + 1); CpuMpData->Buffer = Buffer; CpuMpData->CpuApStackSize = ApStackSize; CpuMpData->BackupBuffer = BackupBufferAddr; @@ -1557,10 +1590,20 @@ MpInitLibInitialize ( CpuMpData->MicrocodePatchAddress = PcdGet64 (PcdCpuMicrocodePatchAddress); CpuMpData->MicrocodePatchRegionSize = PcdGet64 (PcdCpuMicrocodePatchRegionSize); InitializeSpinLock(&CpuMpData->MpLock); + + // + // Make sure no memory usage outside of the allocated buffer. // - // Save BSP's Control registers to APs + ASSERT ((CpuMpData->CpuInfoInHob + sizeof (CPU_INFO_IN_HOB) * MaxLogicalProcessorNumber) == + Buffer + BufferSize); + + // + // 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 // - SaveVolatileRegisters (&CpuMpData->CpuData[0].VolatileRegisters); + CopyMem ((VOID *)ApIdtBase, (VOID *)VolatileRegisters.Idtr.Base, VolatileRegisters.Idtr.Limit + 1); + VolatileRegisters.Idtr.Base = ApIdtBase; + CopyMem (&CpuMpData->CpuData[0].VolatileRegisters, &VolatileRegisters, sizeof (VolatileRegisters)); // // Set BSP basic information // @@ -1618,11 +1661,7 @@ MpInitLibInitialize ( } CpuMpData->CpuData[Index].CpuHealthy = (CpuInfoInHob[Index].Health == 0)? TRUE:FALSE; CpuMpData->CpuData[Index].ApFunction = 0; - CopyMem ( - &CpuMpData->CpuData[Index].VolatileRegisters, - &CpuMpData->CpuData[0].VolatileRegisters, - sizeof (CPU_VOLATILE_REGISTERS) - ); + CopyMem (&CpuMpData->CpuData[Index].VolatileRegisters, &VolatileRegisters, sizeof (CPU_VOLATILE_REGISTERS)); } if (MaxLogicalProcessorNumber > 1) { // diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c index 791ae9db6e..92f28681e4 100644 --- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c @@ -27,6 +27,8 @@ EnableDebugAgent ( /** Get pointer to CPU MP Data structure. + For BSP, the pointer is retrieved from HOB. + For AP, the structure is just after IDT. @return The pointer to CPU MP Data structure. **/ @@ -35,10 +37,18 @@ GetCpuMpData ( VOID ) { - CPU_MP_DATA *CpuMpData; - - CpuMpData = GetCpuMpDataFromGuidedHob (); - ASSERT (CpuMpData != NULL); + CPU_MP_DATA *CpuMpData; + MSR_IA32_APIC_BASE_REGISTER ApicBaseMsr; + IA32_DESCRIPTOR Idtr; + + ApicBaseMsr.Uint64 = AsmReadMsr64 (MSR_IA32_APIC_BASE); + if (ApicBaseMsr.Bits.BSP == 1) { + CpuMpData = GetCpuMpDataFromGuidedHob (); + ASSERT (CpuMpData != NULL); + } else { + AsmReadIdtr (&Idtr); + CpuMpData = (CPU_MP_DATA *) (Idtr.Base + Idtr.Limit + 1); + } return CpuMpData; } -- 2.16.1.windows.1