From: "Gao, Liming" <liming.gao@intel.com>
To: "Zeng, Star" <star.zeng@intel.com>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Ni, Ruiyu" <ruiyu.ni@intel.com>,
"Dong, Eric" <eric.dong@intel.com>,
"Fan, Jeff" <jeff.fan@intel.com>
Subject: Re: [PATCH] UefiCpuPkg MpInitLib: Save/restore original WakeupBuffer for DxeMpLib
Date: Mon, 14 Aug 2017 05:11:58 +0000 [thread overview]
Message-ID: <4A89E2EF3DFEDB4C8BFDE51014F606A14D770117@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <1502679630-119332-1-git-send-email-star.zeng@intel.com>
Reviewed-by: Liming Gao <liming.gao@intel.com>
>-----Original Message-----
>From: Zeng, Star
>Sent: Monday, August 14, 2017 11:01 AM
>To: edk2-devel@lists.01.org
>Cc: Zeng, Star <star.zeng@intel.com>; Gao, Liming <liming.gao@intel.com>; Ni,
>Ruiyu <ruiyu.ni@intel.com>; Dong, Eric <eric.dong@intel.com>; Fan, Jeff
><jeff.fan@intel.com>
>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 <liming.gao@intel.com>
>Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>Cc: Eric Dong <eric.dong@intel.com>
>Cc: Jeff Fan <jeff.fan@intel.com>
>Contributed-under: TianoCore Contribution Agreement 1.0
>Signed-off-by: Star Zeng <star.zeng@intel.com>
>---
> 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 = CpuMpData->AddressMap.RendezvousFunnelSize +
>- sizeof (MP_CPU_EXCHANGE_INFO);
>-
>- StartAddress = BASE_1MB;
>- Status = gBS->AllocatePages (
>- AllocateMaxAddress,
>- EfiACPIMemoryNVS,
>- EFI_SIZE_TO_PAGES (ApResetVectorSize),
>- &StartAddress
>- );
>- ASSERT_EFI_ERROR (Status);
>-
>- CpuMpData->WakeupBuffer = (UINTN) StartAddress;
>- CpuMpData->MpCpuExchangeInfo = (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 = CpuMpData->AddressMap.RendezvousFunnelSize +
>- sizeof (MP_CPU_EXCHANGE_INFO);
>+ EFI_STATUS Status;
>+ EFI_PHYSICAL_ADDRESS StartAddress;
>+
>+ StartAddress = BASE_1MB;
>+ Status = gBS->AllocatePages (
>+ AllocateMaxAddress,
>+ EfiBootServicesData,
>+ EFI_SIZE_TO_PAGES (WakeupBufferSize),
>+ &StartAddress
>+ );
>+ ASSERT_EFI_ERROR (Status);
>+ if (!EFI_ERROR (Status)) {
> Status = 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 = %x, WakeupBufferSize
>= %x\n",
>+ (UINTN) StartAddress, WakeupBufferSize));
>+ } else {
>+ StartAddress = (EFI_PHYSICAL_ADDRESS) -1;
> }
>+ return (UINTN) StartAddress;
> }
>
> /**
>@@ -299,7 +268,6 @@ MpInitChangeApLoopCallback (
> CPU_MP_DATA *CpuMpData;
>
> CpuMpData = GetCpuMpData ();
>- CpuMpData->SaveRestoreFlag = TRUE;
> CpuMpData->PmCodeSegment = GetProtectedModeCS ();
> CpuMpData->ApLoopMode = PcdGet8 (PcdCpuApLoopMode);
> mNumberToFinish = 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 == (UINTN) -1) {
>+ ApResetVectorSize = CpuMpData->AddressMap.RendezvousFunnelSize +
>+ sizeof (MP_CPU_EXCHANGE_INFO);
>+
>+ CpuMpData->WakeupBuffer = GetWakeupBuffer (ApResetVectorSize);
>+ CpuMpData->MpCpuExchangeInfo = (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 = ApStackSize;
> CpuMpData->BackupBuffer = BackupBufferAddr;
> CpuMpData->BackupBufferSize = ApResetVectorSize;
>- CpuMpData->SaveRestoreFlag = FALSE;
> CpuMpData->WakeupBuffer = (UINTN) -1;
> CpuMpData->CpuCount = 1;
> CpuMpData->BspNumber = 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 == (UINTN) -1) {
>- ApResetVectorSize = CpuMpData->AddressMap.RendezvousFunnelSize +
>- sizeof (MP_CPU_EXCHANGE_INFO);
>-
>- CpuMpData->WakeupBuffer = GetWakeupBuffer (ApResetVectorSize);
>- CpuMpData->MpCpuExchangeInfo = (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
prev parent reply other threads:[~2017-08-14 5:09 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-14 3:00 [PATCH] UefiCpuPkg MpInitLib: Save/restore original WakeupBuffer for DxeMpLib Star Zeng
2017-08-14 5:11 ` Gao, Liming [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4A89E2EF3DFEDB4C8BFDE51014F606A14D770117@shsmsx102.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox