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.88; helo=mga01.intel.com; envelope-from=dandan.bi@intel.com; receiver=edk2-devel@lists.01.org Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) (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 B8F35207E4DE3 for ; Thu, 24 May 2018 19:00:58 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 24 May 2018 19:00:58 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.49,438,1520924400"; d="scan'208";a="61526120" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by orsmga002.jf.intel.com with ESMTP; 24 May 2018 19:00:58 -0700 Received: from fmsmsx152.amr.corp.intel.com (10.18.125.5) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 24 May 2018 19:00:57 -0700 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by FMSMSX152.amr.corp.intel.com (10.18.125.5) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 24 May 2018 19:00:57 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.79]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.210]) with mapi id 14.03.0319.002; Fri, 25 May 2018 10:00:55 +0800 From: "Bi, Dandan" To: Ard Biesheuvel , Laszlo Ersek CC: "Dong, Eric" , "edk2-devel@lists.01.org" , "Gao, Liming" , "Leif Lindholm" , "Kinney, Michael D" , "Zeng, Star" Thread-Topic: [edk2] [PATCH v2 4/5] MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages Thread-Index: AQHT8z8Fg0v+sNgcPE+1GtwO8kAHLqQ+TtQAgAABPQCAAVyFgA== Date: Fri, 25 May 2018 02:00:54 +0000 Message-ID: <3C0D5C461C9E904E8F62152F6274C0BB3BAD963A@shsmsx102.ccr.corp.intel.com> References: <20180524090945.10289-1-ard.biesheuvel@linaro.org> <20180524090945.10289-5-ard.biesheuvel@linaro.org> In-Reply-To: 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 v2 4/5] MdeModulePkg/DxeCorePerformanceLib: use AllocatePeiAccessiblePages X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.26 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 25 May 2018 02:00:58 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Ard, Thank you very much for helping fix this issue. And for ZeroMem(), it should be added back. Because after the memory is allocated, the optional padding (from the PCD) = memory will be used directly.=20 If ZeoMem() is dropped, then some contents in the padding memory will not b= e zero. Thus will impact the tool to parse the contents. Thanks, Dandan -----Original Message----- From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ard = Biesheuvel Sent: Thursday, May 24, 2018 8:54 PM To: Laszlo Ersek Cc: Dong, Eric ; edk2-devel@lists.01.org; Gao, Liming = ; Bi, Dandan ; Leif Lindholm ; Kinney, Michael D ; Ze= ng, Star Subject: Re: [edk2] [PATCH v2 4/5] MdeModulePkg/DxeCorePerformanceLib: use = AllocatePeiAccessiblePages On 24 May 2018 at 14:49, Laszlo Ersek wrote: > On 05/24/18 11:09, Ard Biesheuvel wrote: >> Replace the call to and implementation of the function >> FpdtAllocateReservedMemoryBelow4G() with a call to=20 >> AllocatePeiAccessiblePages, which boils down to the same on X64, but=20 >> does not crash non-X64 systems that lack memory below 4 GB. >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Ard Biesheuvel >> --- >> Note that the ZeroMem() call is dropped, but it is redundant anyway,=20 >> given that the subsequent CopyMem() call supersedes it immediately. > > I'm not sure the ZeroMem() is redundant. The allocation size -- before ro= unding up to full pages -- is computed like this: > > BootPerformanceDataSize =3D sizeof (BOOT_PERFORMANCE_TABLE) + mPerforma= nceLength + PcdGet32 (PcdExtFpdtBootRecordPadSize); > if (SmmCommData !=3D NULL && SmmBootRecordData !=3D NULL) { > BootPerformanceDataSize +=3D SmmBootRecordDataSize; > } > > ZeroMem() covers all of the above, but the CopyMem() calls don't seem to = cover the optional padding (from the PCD). > > I'm unsure if that matters, of course. > You're quite right. I'm not sure how I missed that. In any case, I can add back the ZeroMem () quite easily after the single in= vocation of AllocatePeiAccessiblePages() that I am adding in this file. > The patch looks good to me otherwise. > > Thanks > Laszlo > > >> >> MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c |=20 >> 45 ++------------------ >> 1 file changed, 4 insertions(+), 41 deletions(-) >> >> diff --git=20 >> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c=20 >> b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c >> index 71d624fc9ce9..b1f09710b65c 100644 >> ---=20 >> a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c >> +++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLi >> +++ b.c >> @@ -165,46 +165,6 @@ IsKnownID ( >> } >> } >> >> -/** >> - Allocate EfiReservedMemoryType below 4G memory address. >> - >> - This function allocates EfiReservedMemoryType below 4G memory address= . >> - >> - @param[in] Size Size of memory to allocate. >> - >> - @return Allocated address for output. >> - >> -**/ >> -VOID * >> -FpdtAllocateReservedMemoryBelow4G ( >> - IN UINTN Size >> - ) >> -{ >> - UINTN Pages; >> - EFI_PHYSICAL_ADDRESS Address; >> - EFI_STATUS Status; >> - VOID *Buffer; >> - >> - Buffer =3D NULL; >> - Pages =3D EFI_SIZE_TO_PAGES (Size); >> - Address =3D 0xffffffff; >> - >> - Status =3D gBS->AllocatePages ( >> - AllocateMaxAddress, >> - EfiReservedMemoryType, >> - Pages, >> - &Address >> - ); >> - ASSERT_EFI_ERROR (Status); >> - >> - if (!EFI_ERROR (Status)) { >> - Buffer =3D (VOID *) (UINTN) Address; >> - ZeroMem (Buffer, Size); >> - } >> - >> - return Buffer; >> -} >> - >> /** >> Allocate buffer for Boot Performance table. >> >> @@ -348,7 +308,10 @@ AllocateBootPerformanceTable ( >> // >> // Fail to allocate at specified address, continue to allocate at a= ny address. >> // >> - mAcpiBootPerformanceTable =3D (BOOT_PERFORMANCE_TABLE *) FpdtAlloca= teReservedMemoryBelow4G (BootPerformanceDataSize); >> + mAcpiBootPerformanceTable =3D (BOOT_PERFORMANCE_TABLE *) AllocatePe= iAccessiblePages ( >> + EfiReserve= dMemoryType, >> + EFI_SIZE_T= O_PAGES (BootPerformanceDataSize) >> + ); >> } >> DEBUG ((DEBUG_INFO, "DxeCorePerformanceLib: ACPI Boot Performance=20 >> Table address =3D 0x%x\n", mAcpiBootPerformanceTable)); >> >> > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel