From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) (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 01F4D21D0A24F for ; Sun, 13 Aug 2017 22:09:38 -0700 (PDT) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga105.fm.intel.com with ESMTP; 13 Aug 2017 22:12:02 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.41,372,1498546800"; d="scan'208";a="299740555" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by fmsmga004.fm.intel.com with ESMTP; 13 Aug 2017 22:12:02 -0700 Received: from fmsmsx123.amr.corp.intel.com (10.18.125.38) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.319.2; Sun, 13 Aug 2017 22:12:01 -0700 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by fmsmsx123.amr.corp.intel.com (10.18.125.38) with Microsoft SMTP Server (TLS) id 14.3.319.2; Sun, 13 Aug 2017 22:12:01 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.183]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.25]) with mapi id 14.03.0319.002; Mon, 14 Aug 2017 13:11:59 +0800 From: "Gao, Liming" To: "Zeng, Star" , "edk2-devel@lists.01.org" CC: "Ni, Ruiyu" , "Dong, Eric" , "Fan, Jeff" Thread-Topic: [PATCH] UefiCpuPkg MpInitLib: Save/restore original WakeupBuffer for DxeMpLib Thread-Index: AQHTFKmBqIDf7vvFE0u8ojJqFj/96KKDTloA Date: Mon, 14 Aug 2017 05:11:58 +0000 Message-ID: <4A89E2EF3DFEDB4C8BFDE51014F606A14D770117@shsmsx102.ccr.corp.intel.com> References: <1502679630-119332-1-git-send-email-star.zeng@intel.com> In-Reply-To: <1502679630-119332-1-git-send-email-star.zeng@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH] UefiCpuPkg MpInitLib: Save/restore original WakeupBuffer for DxeMpLib X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 14 Aug 2017 05:09:39 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Reviewed-by: Liming Gao >-----Original Message----- >From: Zeng, Star >Sent: Monday, August 14, 2017 11:01 AM >To: edk2-devel@lists.01.org >Cc: Zeng, Star ; Gao, Liming ; = Ni, >Ruiyu ; Dong, Eric ; Fan, Jeff > >Subject: [PATCH] UefiCpuPkg MpInitLib: Save/restore original WakeupBuffer >for DxeMpLib > >Current code always allocates/frees < 1MB WakeupBuffer for DxeMpLib >until ExitBootService, but the allocation may be failed at late >phase of the boot. > >This patch is to always save/restore original WakeupBuffer for >DxeMpLib, it is aligned with the solution for PeiMpLib at >9293d6e42e677e4a38e055258c0993ad8a9df14e, then AllocateResetVector() >and FreeResetVector() will be common and moved to MpLib.c. >Only difference is GetWakeupBuffer() that will be in PeiMpLib or >DxeMpLib respectively. > >Cc: Liming Gao >Cc: Ruiyu Ni >Cc: Eric Dong >Cc: Jeff Fan >Contributed-under: TianoCore Contribution Agreement 1.0 >Signed-off-by: Star Zeng >--- > UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 84 ++++++++--------------- > UefiCpuPkg/Library/MpInitLib/MpLib.c | 114 +++++++++++++++++++++---- >------- > UefiCpuPkg/Library/MpInitLib/MpLib.h | 43 +++--------- > UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 36 ---------- > 4 files changed, 109 insertions(+), 168 deletions(-) > >diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c >b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c >index b393244e0582..479f8189f655 100644 >--- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c >+++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c >@@ -75,72 +75,41 @@ SaveCpuMpData ( > } > > /** >- Allocate reset vector buffer. >+ Get available system memory below 1MB by specified size. > >- @param[in, out] CpuMpData The pointer to CPU MP Data structure. >-**/ >-VOID >-AllocateResetVector ( >- IN OUT CPU_MP_DATA *CpuMpData >- ) >-{ >- EFI_STATUS Status; >- UINTN ApResetVectorSize; >- EFI_PHYSICAL_ADDRESS StartAddress; >+ @param[in] WakeupBufferSize Wakeup buffer size required > >- if (CpuMpData->SaveRestoreFlag) { >- BackupAndPrepareWakeupBuffer (CpuMpData); >- } else { >- ApResetVectorSize =3D CpuMpData->AddressMap.RendezvousFunnelSize + >- sizeof (MP_CPU_EXCHANGE_INFO); >- >- StartAddress =3D BASE_1MB; >- Status =3D gBS->AllocatePages ( >- AllocateMaxAddress, >- EfiACPIMemoryNVS, >- EFI_SIZE_TO_PAGES (ApResetVectorSize), >- &StartAddress >- ); >- ASSERT_EFI_ERROR (Status); >- >- CpuMpData->WakeupBuffer =3D (UINTN) StartAddress; >- CpuMpData->MpCpuExchangeInfo =3D (MP_CPU_EXCHANGE_INFO *) >(UINTN) >- (CpuMpData->WakeupBuffer + CpuMpData- >>AddressMap.RendezvousFunnelSize); >- // >- // copy AP reset code in it >- // >- CopyMem ( >- (VOID *) CpuMpData->WakeupBuffer, >- (VOID *) CpuMpData->AddressMap.RendezvousFunnelAddress, >- CpuMpData->AddressMap.RendezvousFunnelSize >- ); >- } >-} >- >-/** >- Free AP reset vector buffer. >- >- @param[in] CpuMpData The pointer to CPU MP Data structure. >+ @retval other Return wakeup buffer address below 1MB. >+ @retval -1 Cannot find free memory below 1MB. > **/ >-VOID >-FreeResetVector ( >- IN CPU_MP_DATA *CpuMpData >+UINTN >+GetWakeupBuffer ( >+ IN UINTN WakeupBufferSize > ) > { >- EFI_STATUS Status; >- UINTN ApResetVectorSize; >- >- if (CpuMpData->SaveRestoreFlag) { >- RestoreWakeupBuffer (CpuMpData); >- } else { >- ApResetVectorSize =3D CpuMpData->AddressMap.RendezvousFunnelSize + >- sizeof (MP_CPU_EXCHANGE_INFO); >+ EFI_STATUS Status; >+ EFI_PHYSICAL_ADDRESS StartAddress; >+ >+ StartAddress =3D BASE_1MB; >+ Status =3D gBS->AllocatePages ( >+ AllocateMaxAddress, >+ EfiBootServicesData, >+ EFI_SIZE_TO_PAGES (WakeupBufferSize), >+ &StartAddress >+ ); >+ ASSERT_EFI_ERROR (Status); >+ if (!EFI_ERROR (Status)) { > Status =3D gBS->FreePages( >- (EFI_PHYSICAL_ADDRESS)CpuMpData->WakeupBuffer, >- EFI_SIZE_TO_PAGES (ApResetVectorSize) >+ StartAddress, >+ EFI_SIZE_TO_PAGES (WakeupBufferSize) > ); > ASSERT_EFI_ERROR (Status); >+ DEBUG ((DEBUG_INFO, "WakeupBufferStart =3D %x, WakeupBufferSize >=3D %x\n", >+ (UINTN) StartAddress, WakeupBufferSize)); >+ } else { >+ StartAddress =3D (EFI_PHYSICAL_ADDRESS) -1; > } >+ return (UINTN) StartAddress; > } > > /** >@@ -299,7 +268,6 @@ MpInitChangeApLoopCallback ( > CPU_MP_DATA *CpuMpData; > > CpuMpData =3D GetCpuMpData (); >- CpuMpData->SaveRestoreFlag =3D TRUE; > CpuMpData->PmCodeSegment =3D GetProtectedModeCS (); > CpuMpData->ApLoopMode =3D PcdGet8 (PcdCpuApLoopMode); > mNumberToFinish =3D CpuMpData->CpuCount - 1; >diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c >b/UefiCpuPkg/Library/MpInitLib/MpLib.c >index a3eea29d6148..ed1f55e955c2 100644 >--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c >+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c >@@ -796,6 +796,81 @@ TimedWaitForApFinish ( > ); > > /** >+ Get available system memory below 1MB by specified size. >+ >+ @param[in] CpuMpData The pointer to CPU MP Data structure. >+**/ >+VOID >+BackupAndPrepareWakeupBuffer( >+ IN CPU_MP_DATA *CpuMpData >+ ) >+{ >+ CopyMem ( >+ (VOID *) CpuMpData->BackupBuffer, >+ (VOID *) CpuMpData->WakeupBuffer, >+ CpuMpData->BackupBufferSize >+ ); >+ CopyMem ( >+ (VOID *) CpuMpData->WakeupBuffer, >+ (VOID *) CpuMpData->AddressMap.RendezvousFunnelAddress, >+ CpuMpData->AddressMap.RendezvousFunnelSize >+ ); >+} >+ >+/** >+ Restore wakeup buffer data. >+ >+ @param[in] CpuMpData The pointer to CPU MP Data structure. >+**/ >+VOID >+RestoreWakeupBuffer( >+ IN CPU_MP_DATA *CpuMpData >+ ) >+{ >+ CopyMem ( >+ (VOID *) CpuMpData->WakeupBuffer, >+ (VOID *) CpuMpData->BackupBuffer, >+ CpuMpData->BackupBufferSize >+ ); >+} >+ >+/** >+ Allocate reset vector buffer. >+ >+ @param[in, out] CpuMpData The pointer to CPU MP Data structure. >+**/ >+VOID >+AllocateResetVector ( >+ IN OUT CPU_MP_DATA *CpuMpData >+ ) >+{ >+ UINTN ApResetVectorSize; >+ >+ if (CpuMpData->WakeupBuffer =3D=3D (UINTN) -1) { >+ ApResetVectorSize =3D CpuMpData->AddressMap.RendezvousFunnelSize + >+ sizeof (MP_CPU_EXCHANGE_INFO); >+ >+ CpuMpData->WakeupBuffer =3D GetWakeupBuffer (ApResetVectorSize); >+ CpuMpData->MpCpuExchangeInfo =3D (MP_CPU_EXCHANGE_INFO *) >(UINTN) >+ (CpuMpData->WakeupBuffer + CpuMpData- >>AddressMap.RendezvousFunnelSize); >+ } >+ BackupAndPrepareWakeupBuffer (CpuMpData); >+} >+ >+/** >+ Free AP reset vector buffer. >+ >+ @param[in] CpuMpData The pointer to CPU MP Data structure. >+**/ >+VOID >+FreeResetVector ( >+ IN CPU_MP_DATA *CpuMpData >+ ) >+{ >+ RestoreWakeupBuffer (CpuMpData); >+} >+ >+/** > This function will be called by BSP to wakeup AP. > > @param[in] CpuMpData Pointer to CPU MP Data >@@ -1353,7 +1428,6 @@ MpInitLibInitialize ( > CpuMpData->CpuApStackSize =3D ApStackSize; > CpuMpData->BackupBuffer =3D BackupBufferAddr; > CpuMpData->BackupBufferSize =3D ApResetVectorSize; >- CpuMpData->SaveRestoreFlag =3D FALSE; > CpuMpData->WakeupBuffer =3D (UINTN) -1; > CpuMpData->CpuCount =3D 1; > CpuMpData->BspNumber =3D 0; >@@ -2120,41 +2194,3 @@ GetCpuMpDataFromGuidedHob ( > return CpuMpData; > } > >-/** >- Get available system memory below 1MB by specified size. >- >- @param[in] CpuMpData The pointer to CPU MP Data structure. >-**/ >-VOID >-BackupAndPrepareWakeupBuffer( >- IN CPU_MP_DATA *CpuMpData >- ) >-{ >- CopyMem ( >- (VOID *) CpuMpData->BackupBuffer, >- (VOID *) CpuMpData->WakeupBuffer, >- CpuMpData->BackupBufferSize >- ); >- CopyMem ( >- (VOID *) CpuMpData->WakeupBuffer, >- (VOID *) CpuMpData->AddressMap.RendezvousFunnelAddress, >- CpuMpData->AddressMap.RendezvousFunnelSize >- ); >-} >- >-/** >- Restore wakeup buffer data. >- >- @param[in] CpuMpData The pointer to CPU MP Data structure. >-**/ >-VOID >-RestoreWakeupBuffer( >- IN CPU_MP_DATA *CpuMpData >- ) >-{ >- CopyMem ( >- (VOID *) CpuMpData->WakeupBuffer, >- (VOID *) CpuMpData->BackupBuffer, >- CpuMpData->BackupBufferSize >- ); >-} >diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h >b/UefiCpuPkg/Library/MpInitLib/MpLib.h >index ea56412cbce0..19defdaf4ba4 100644 >--- a/UefiCpuPkg/Library/MpInitLib/MpLib.h >+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h >@@ -201,7 +201,6 @@ struct _CPU_MP_DATA { > UINTN WakeupBuffer; > UINTN BackupBuffer; > UINTN BackupBufferSize; >- BOOLEAN SaveRestoreFlag; > > volatile UINT32 StartCount; > volatile UINT32 FinishedCount; >@@ -310,24 +309,18 @@ SaveCpuMpData ( > IN CPU_MP_DATA *CpuMpData > ); > >-/** >- Allocate reset vector buffer. >- >- @param[in, out] CpuMpData The pointer to CPU MP Data structure. >-**/ >-VOID >-AllocateResetVector ( >- IN OUT CPU_MP_DATA *CpuMpData >- ); > > /** >- Free AP reset vector buffer. >+ Get available system memory below 1MB by specified size. > >- @param[in] CpuMpData The pointer to CPU MP Data structure. >+ @param[in] WakeupBufferSize Wakeup buffer size required >+ >+ @retval other Return wakeup buffer address below 1MB. >+ @retval -1 Cannot find free memory below 1MB. > **/ >-VOID >-FreeResetVector ( >- IN CPU_MP_DATA *CpuMpData >+UINTN >+GetWakeupBuffer ( >+ IN UINTN WakeupBufferSize > ); > > /** >@@ -543,26 +536,6 @@ IsMwaitSupport ( > ); > > /** >- Get available system memory below 1MB by specified size. >- >- @param[in] CpuMpData The pointer to CPU MP Data structure. >-**/ >-VOID >-BackupAndPrepareWakeupBuffer( >- IN CPU_MP_DATA *CpuMpData >- ); >- >-/** >- Restore wakeup buffer data. >- >- @param[in] CpuMpData The pointer to CPU MP Data structure. >-**/ >-VOID >-RestoreWakeupBuffer( >- IN CPU_MP_DATA *CpuMpData >- ); >- >-/** > Enable Debug Agent to support source debugging on AP function. > > **/ >diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c >b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c >index 9ee5aca57b2b..70c2bc7323f6 100644 >--- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c >+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c >@@ -188,42 +188,6 @@ GetWakeupBuffer ( > } > > /** >- Allocate reset vector buffer. >- >- @param[in, out] CpuMpData The pointer to CPU MP Data structure. >-**/ >-VOID >-AllocateResetVector ( >- IN OUT CPU_MP_DATA *CpuMpData >- ) >-{ >- UINTN ApResetVectorSize; >- >- if (CpuMpData->WakeupBuffer =3D=3D (UINTN) -1) { >- ApResetVectorSize =3D CpuMpData->AddressMap.RendezvousFunnelSize + >- sizeof (MP_CPU_EXCHANGE_INFO); >- >- CpuMpData->WakeupBuffer =3D GetWakeupBuffer (ApResetVectorSize); >- CpuMpData->MpCpuExchangeInfo =3D (MP_CPU_EXCHANGE_INFO *) >(UINTN) >- (CpuMpData->WakeupBuffer + CpuMpData- >>AddressMap.RendezvousFunnelSize); >- } >- BackupAndPrepareWakeupBuffer (CpuMpData); >-} >- >-/** >- Free AP reset vector buffer. >- >- @param[in] CpuMpData The pointer to CPU MP Data structure. >-**/ >-VOID >-FreeResetVector ( >- IN CPU_MP_DATA *CpuMpData >- ) >-{ >- RestoreWakeupBuffer (CpuMpData); >-} >- >-/** > Checks APs status and updates APs status if needed. > > **/ >-- >2.7.0.windows.1