From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.115; helo=mga14.intel.com; envelope-from=eric.dong@intel.com; receiver=edk2-devel@lists.01.org Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) (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 957E1211CF377 for ; Mon, 4 Mar 2019 18:07:02 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Mar 2019 18:07:02 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.58,442,1544515200"; d="scan'208";a="139140735" Received: from ydong10-win10.ccr.corp.intel.com ([10.239.9.125]) by orsmga002.jf.intel.com with ESMTP; 04 Mar 2019 18:07:01 -0800 From: Eric Dong To: edk2-devel@lists.01.org Date: Tue, 5 Mar 2019 10:06:58 +0800 Message-Id: <20190305020658.23408-2-eric.dong@intel.com> X-Mailer: git-send-email 2.15.0.windows.1 In-Reply-To: <20190305020658.23408-1-eric.dong@intel.com> References: <20190305020658.23408-1-eric.dong@intel.com> Subject: [Patch] UefiCpuPkg/MpInitLib: Direct allocate buffer for Wake up Buffer. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 05 Mar 2019 02:07:02 -0000 https://bugzilla.tianocore.org/show_bug.cgi?id=1538 Current CpuDxe driver "borrows" Wakeup Buffer (through Allocate & free to get the buffer pointer, backup the buffer data before using it and restore it after using). Now this logic met a problem described in the above BZ because the test tool and the CpuDxe both use the same memory at the same time. In order to fix the above issue, CpuDxe changed to allocate the buffer below 1M instead of borrow it. After investigation, we found below 0x88000 is the possible space which can be used. For now, range 0x60000 ~ 0x88000 used for Legacy OPROMs by LegacyBios driver. And it tries to allocate these range page(4K size) by page. It just reports warning message if specific page been used by others already. Also CpuDxe driver will produce CPU arch protocol and LegacyBios driver has dependency for this protocol. So CpuDxe driver will start before LegacyBios driver and CpuDxe driver can allocate that space successful. With this change, CpuDxe driver will use 0x87000 ~ 0x88000 as wakeup buffer. Cc: Ray Ni Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Eric Dong --- UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c index b2307cbb61..5bc9a47efb 100644 --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c @@ -76,7 +76,7 @@ SaveCpuMpData ( } /** - Get available system memory below 1MB by specified size. + Get available system memory below 0x88000 by specified size. @param[in] WakeupBufferSize Wakeup buffer size required @@ -91,7 +91,19 @@ GetWakeupBuffer ( EFI_STATUS Status; EFI_PHYSICAL_ADDRESS StartAddress; - StartAddress = BASE_1MB; + // + // Current "Borrow" space mechanism caused potential race condition if both + // AP and the original owner use the share space. + // + // LegacyBios driver tries to allocate 4K pages between 0x60000 ~ 0x88000 + // space. It will just report an waring message if the page has been allocate + // by other drivers. + // LagacyBios driver depends on CPU Arch protocol, so it will start after + // CpuDxe driver which produce Cpu Arch Protocol and use this library. + // So below allocate logic will be trigged before LegacyBios driver and it + // will always return success. + // + StartAddress = BASE_512KB + BASE_32KB; Status = gBS->AllocatePages ( AllocateMaxAddress, EfiBootServicesData, @@ -99,17 +111,13 @@ GetWakeupBuffer ( &StartAddress ); ASSERT_EFI_ERROR (Status); - if (!EFI_ERROR (Status)) { - Status = gBS->FreePages( - StartAddress, - EFI_SIZE_TO_PAGES (WakeupBufferSize) - ); - ASSERT_EFI_ERROR (Status); - DEBUG ((DEBUG_INFO, "WakeupBufferStart = %x, WakeupBufferSize = %x\n", - (UINTN) StartAddress, WakeupBufferSize)); - } else { + if (EFI_ERROR (Status)) { StartAddress = (EFI_PHYSICAL_ADDRESS) -1; } + + DEBUG ((DEBUG_INFO, "WakeupBufferStart = %x, WakeupBufferSize = %x\n", + (UINTN) StartAddress, WakeupBufferSize)); + return (UINTN) StartAddress; } -- 2.15.0.windows.1