From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) (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 169E881C95 for ; Fri, 11 Nov 2016 04:15:15 -0800 (PST) Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga101.jf.intel.com with ESMTP; 11 Nov 2016 04:15:18 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,621,1473145200"; d="scan'208";a="190307148" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by fmsmga004.fm.intel.com with ESMTP; 11 Nov 2016 04:15:17 -0800 Received: from fmsmsx120.amr.corp.intel.com (10.18.124.208) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.248.2; Fri, 11 Nov 2016 04:15:17 -0800 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by fmsmsx120.amr.corp.intel.com (10.18.124.208) with Microsoft SMTP Server (TLS) id 14.3.248.2; Fri, 11 Nov 2016 04:15:16 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.239]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.138]) with mapi id 14.03.0248.002; Fri, 11 Nov 2016 20:15:14 +0800 From: "Fan, Jeff" To: Laszlo Ersek , "edk2-devel@ml01.01.org" CC: "Kinney, Michael D" , Paolo Bonzini , "Yao, Jiewen" Thread-Topic: [edk2] [PATCH v2] UefiCpuPkg/DxeMpLib: Allocate below 4GB mem for AsmRelocateApLoopFunc Thread-Index: AQHSO/mdQYd2vPkqbUqN2Spf2KmPhKDTFREAgAAVaQCAAIfcoA== Date: Fri, 11 Nov 2016 12:15:14 +0000 Message-ID: <542CF652F8836A4AB8DBFAAD40ED192A4A2DAC11@shsmsx102.ccr.corp.intel.com> References: <20161111085644.11512-1-jeff.fan@intel.com> <94d628af-b57c-9d69-3629-3e57cf9192e6@redhat.com> <1a64a6db-39a6-6e67-218d-58e2feb9cd5d@redhat.com> In-Reply-To: <1a64a6db-39a6-6e67-218d-58e2feb9cd5d@redhat.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiODk0ZGM3MzktMWFiMi00MWE4LTk1ODgtYjg0NDdlZjMyYTBjIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6IjdZVDF5MTdHV3ZJWUV2TUJlMk9GemdhNmhFVmE5WUlsQ1NWdlwvM1Jta2lzPSJ9 x-ctpclassification: CTP_IC x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH v2] UefiCpuPkg/DxeMpLib: Allocate below 4GB mem for AsmRelocateApLoopFunc X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 11 Nov 2016 12:15:15 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Laszlo, Thanks your updating and pushing.=20 Jeff -----Original Message----- From: Laszlo Ersek [mailto:lersek@redhat.com]=20 Sent: Friday, November 11, 2016 8:08 PM To: Fan, Jeff; edk2-devel@ml01.01.org Cc: Kinney, Michael D; Paolo Bonzini; Yao, Jiewen Subject: Re: [edk2] [PATCH v2] UefiCpuPkg/DxeMpLib: Allocate below 4GB mem = for AsmRelocateApLoopFunc On 11/11/16 11:51, Laszlo Ersek wrote: > On 11/11/16 09:56, Jeff Fan wrote: >> Current implementation just allocates reserve memory for AsmRelocateApLo= opFunc. >> It not be safe because APs will be placed into 32bit protected mode=20 >> on long mode DXE. This reserve memory must be located below 4GB memory. >> >> This fix is to allocate < 4GB memory for AsmRelocateApLoopFunc. >> >> Cc: Laszlo Ersek >> Cc: Paolo Bonzini >> Cc: Jiewen Yao >> Cc: Michael D Kinney >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Jeff Fan >> --- >> UefiCpuPkg/Library/MpInitLib/DxeMpLib.c | 27=20 >> ++++++++++++++++++++------- >> 1 file changed, 20 insertions(+), 7 deletions(-) >> >> diff --git a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c=20 >> b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c >> index eb36d6f..4b929ff 100644 >> --- a/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c >> +++ b/UefiCpuPkg/Library/MpInitLib/DxeMpLib.c >> @@ -286,7 +286,8 @@ InitMpGlobalData ( >> IN CPU_MP_DATA *CpuMpData >> ) >> { >> - EFI_STATUS Status; >> + EFI_STATUS Status; >> + EFI_PHYSICAL_ADDRESS Address; >> =20 >> SaveCpuMpData (CpuMpData); >> =20 >> @@ -298,16 +299,28 @@ InitMpGlobalData ( >> } >> =20 >> // >> - // Avoid APs access invalid buff data which allocated by=20 >> BootServices, >> - // so we will allocate reserved data for AP loop code. >> + // Avoid APs access invalid buffer data which allocated by=20 >> + BootServices, // so we will allocate reserved data for AP loop=20 >> + code. We also need to There was a superfluous space character at the end of this line (reported b= y git-am), but I removed it. >> + // allocate this buffer below 4GB due to APs may be transferred to=20 >> + 32bit // protected mode on long mode DXE. >> // Allocating it in advance since memory services are not available i= n >> // Exit Boot Services callback function. >> // >> - mReservedApLoopFunc =3D AllocateReservedCopyPool ( >> - CpuMpData->AddressMap.RelocateApLoopFuncSize, >> - CpuMpData->AddressMap.RelocateApLoopFuncAddre= ss >> - ); >> + Address =3D BASE_4GB - 1; >> + Status =3D gBS->AllocatePages ( >> + AllocateMaxAddress, >> + EfiReservedMemoryType, >> + EFI_SIZE_TO_PAGES (sizeof (CpuMpData->AddressMap.Rel= ocateApLoopFuncSize)), >> + &Address >> + ); >> + ASSERT_EFI_ERROR (Status); >> + mReservedApLoopFunc =3D (VOID *) (UINTN) Address; >> ASSERT (mReservedApLoopFunc !=3D NULL); >> + CopyMem ( >> + mReservedApLoopFunc, >> + CpuMpData->AddressMap.RelocateApLoopFuncAddress, >> + CpuMpData->AddressMap.RelocateApLoopFuncSize >> + ); >> =20 >> Status =3D gBS->CreateEvent ( >> EVT_TIMER | EVT_NOTIFY_SIGNAL, >> >=20 > Reviewed-by: Laszlo Ersek >=20 Tested-by: Laszlo Ersek [lersek@redhat.com: strip whitespace at EOL] Signed-off-by: Laszlo Ersek Commit ffd6b0b1b65e. (I pushed the patch because it's simple -- Jeff is the sole maintainer, acc= ording to Maintainers.txt, of UefiCpuPkg, and I thought he'd push this simp= le patch with just my review anyway. I'm trying to flush my review / test q= ueue, simplifying the possible orderings between the pending patch sets.) Thanks Laszlo