From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 1804D81E38 for ; Mon, 14 Nov 2016 17:42:50 -0800 (PST) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga105.jf.intel.com with ESMTP; 14 Nov 2016 17:42:54 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.31,640,1473145200"; d="scan'208";a="1059506838" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by orsmga001.jf.intel.com with ESMTP; 14 Nov 2016 17:42:54 -0800 Received: from fmsmsx158.amr.corp.intel.com (10.18.116.75) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 14 Nov 2016 17:42:54 -0800 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by fmsmsx158.amr.corp.intel.com (10.18.116.75) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 14 Nov 2016 17:42:54 -0800 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.239]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.239]) with mapi id 14.03.0248.002; Tue, 15 Nov 2016 09:42:51 +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/mdQYd2vPkqbUqN2Spf2KmPhKDYxFkAgACGiWA= Date: Tue, 15 Nov 2016 01:42:50 +0000 Message-ID: <542CF652F8836A4AB8DBFAAD40ED192A4A2DD1DD@shsmsx102.ccr.corp.intel.com> References: <20161111085644.11512-1-jeff.fan@intel.com> <54156d2c-cbf5-ea32-8a29-7b1e5ae9bdd7@redhat.com> In-Reply-To: <54156d2c-cbf5-ea32-8a29-7b1e5ae9bdd7@redhat.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMzRjOTJlODYtNjAyZC00ZTZmLTg4OGYtZDYzNDljZGRhNDVjIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE1LjkuNi42IiwiVHJ1c3RlZExhYmVsSGFzaCI6IktQRG9iSlpBSWZ1XC9QbmNhU1BERXZ6ZUhIOXUycWZaT2M2TnJoXC9WZENvST0ifQ== 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: Tue, 15 Nov 2016 01:42:50 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Laszlo, Good catch before I push this serial of patch. I will remove sizeof() when = I push the patch. Thanks! Jeff -----Original Message----- From: Laszlo Ersek [mailto:lersek@redhat.com]=20 Sent: Tuesday, November 15, 2016 9:40 AM 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 Jeff, independently from the other discussion, I found a small mistake in this pa= tch (after I reviewed, tested, and committed it for you...): On 11/11/16 09:56, Jeff Fan wrote: > Current implementation just allocates reserve memory for AsmRelocateApLoo= pFunc. > It not be safe because APs will be placed into 32bit protected mode on=20 > long mode DXE. This reserve memory must be located below 4GB memory. >=20 > This fix is to allocate < 4GB memory for AsmRelocateApLoopFunc. >=20 > 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(-) >=20 > 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 // allocate this buffer below 4GB due to APs=20 > + may be transferred to 32bit // protected mode on long mode DXE. > // Allocating it in advance since memory services are not available in > // Exit Boot Services callback function. > // > - mReservedApLoopFunc =3D AllocateReservedCopyPool ( > - CpuMpData->AddressMap.RelocateApLoopFuncSize, > - CpuMpData->AddressMap.RelocateApLoopFuncAddres= s > - ); > + Address =3D BASE_4GB - 1; > + Status =3D gBS->AllocatePages ( > + AllocateMaxAddress, > + EfiReservedMemoryType, > + EFI_SIZE_TO_PAGES (sizeof=20 > + (CpuMpData->AddressMap.RelocateApLoopFuncSize)), The "sizeof" operator should be unnecessary here. (It is no problem in practice because the function size is really small, so= it gets rounded up to 1 page anyway.) Sorry for not noticing this earlier. Thanks Laszlo > + &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