From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c0b::244; helo=mail-it0-x244.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it0-x244.google.com (mail-it0-x244.google.com [IPv6:2607:f8b0:4001:c0b::244]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id E4E6E203BBBA6 for ; Thu, 24 May 2018 05:54:23 -0700 (PDT) Received: by mail-it0-x244.google.com with SMTP id p3-v6so2199795itc.0 for ; Thu, 24 May 2018 05:54:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=B3MEZh1Hfowxn/In+MKqhpwPNAcAC0YLdJGrIEpvASs=; b=RJefZREPuqkuVThf7Bc4oTdU1pRn9dLcGF4CjstSfO1qQYb801ir9OBY46jZIgF6vg C1IfuQiyW5sV7UpF6elC0W6VyQObQWNYCLCvhw3c6dfKljSzUGkwllnYaL5Ko5Lt+8NK NMaf5Tnu+TSXfmWeNeIajKzfwDXpoN7HobOWc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=B3MEZh1Hfowxn/In+MKqhpwPNAcAC0YLdJGrIEpvASs=; b=N8Ciw2n1bwsIfe30+B3mQEzRRoQC60js/uLh8ChXtZ8uR6aDrxb2ybgXIDHhuwWTCT fNs/TugG1YZT10n/7U/hI9fbY3TkLbvoziR1PRH/OZVPN7RimFYIKVQ0tE3WWWW5KhdK Zh8pLejiESkhEAVVYK4lyS/61CYztzJuzwRbQsdxsl4FLB8K347lzjtWaTzN0bjNzJCW Q3PVmbgamBIpMkyC4Tkh44VPH89KR1KoevOlLy8N1mZOhKebmAjAMWXbovnvQjX7Ca2H DKpjOY0ohZMkItaCvscUF2TgXF92pFpko+hzwE+FISGUWePwfnKTC/KcNo8Zfbs2kJZC 42AA== X-Gm-Message-State: ALKqPwcWVDPUDMWinF8yS0HhJadCau2c+9AiiVV2jeSdhH0TYbCUSf1v EQAlzACwSZT1ZJleHMGFvrMcnTxcquNaupR8DcQurg== X-Google-Smtp-Source: AB8JxZqbUGPuKZygt272pQW39BCvXwCSpblKLYwlv3y7Hc3PfeJZYn7oxbSZ6GKjMVnRijCF16SlXvfr/6lkrxATKTw= X-Received: by 2002:a24:5390:: with SMTP id n138-v6mr8872934itb.42.1527166463016; Thu, 24 May 2018 05:54:23 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a6b:bb86:0:0:0:0:0 with HTTP; Thu, 24 May 2018 05:54:22 -0700 (PDT) In-Reply-To: References: <20180524090945.10289-1-ard.biesheuvel@linaro.org> <20180524090945.10289-5-ard.biesheuvel@linaro.org> From: Ard Biesheuvel Date: Thu, 24 May 2018 14:54:22 +0200 Message-ID: To: Laszlo Ersek Cc: "edk2-devel@lists.01.org" , Leif Lindholm , Michael D Kinney , Liming Gao , Star Zeng , Eric Dong , Dandan Bi 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: Thu, 24 May 2018 12:54:24 -0000 Content-Type: text/plain; charset="UTF-8" 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 >> AllocatePeiAccessiblePages, which boils down to the same on X64, >> but 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, given >> that the subsequent CopyMem() call supersedes it immediately. > > I'm not sure the ZeroMem() is redundant. The allocation size -- before rounding up to full pages -- is computed like this: > > BootPerformanceDataSize = sizeof (BOOT_PERFORMANCE_TABLE) + mPerformanceLength + PcdGet32 (PcdExtFpdtBootRecordPadSize); > if (SmmCommData != NULL && SmmBootRecordData != NULL) { > BootPerformanceDataSize += 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 invocation of AllocatePeiAccessiblePages() that I am adding in this file. > The patch looks good to me otherwise. > > Thanks > Laszlo > > >> >> MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c | 45 ++------------------ >> 1 file changed, 4 insertions(+), 41 deletions(-) >> >> diff --git a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c >> index 71d624fc9ce9..b1f09710b65c 100644 >> --- a/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.c >> +++ b/MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerformanceLib.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 = NULL; >> - Pages = EFI_SIZE_TO_PAGES (Size); >> - Address = 0xffffffff; >> - >> - Status = gBS->AllocatePages ( >> - AllocateMaxAddress, >> - EfiReservedMemoryType, >> - Pages, >> - &Address >> - ); >> - ASSERT_EFI_ERROR (Status); >> - >> - if (!EFI_ERROR (Status)) { >> - Buffer = (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 any address. >> // >> - mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) FpdtAllocateReservedMemoryBelow4G (BootPerformanceDataSize); >> + mAcpiBootPerformanceTable = (BOOT_PERFORMANCE_TABLE *) AllocatePeiAccessiblePages ( >> + EfiReservedMemoryType, >> + EFI_SIZE_TO_PAGES (BootPerformanceDataSize) >> + ); >> } >> DEBUG ((DEBUG_INFO, "DxeCorePerformanceLib: ACPI Boot Performance Table address = 0x%x\n", mAcpiBootPerformanceTable)); >> >> >