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::242; helo=mail-it0-x242.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it0-x242.google.com (mail-it0-x242.google.com [IPv6:2607:f8b0:4001:c0b::242]) (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 82960209603EB for ; Tue, 22 May 2018 10:47:11 -0700 (PDT) Received: by mail-it0-x242.google.com with SMTP id p3-v6so1029613itc.0 for ; Tue, 22 May 2018 10:47:11 -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=h66ZQzG5VUNMmmmYu3Sww6XM+/dS7FCmTk2ptaIcmbw=; b=CH0d8YC2mFF9iRPRgI+YaARbSnqLdNoO6jsLpbD+NV14J0pZqnNCvJSGGejz7zjthR MjFhDno7ZkOCAHibWt43FxUMHcSLTtb32ngIycUATpaF5B/sjwy+rH1Ar7zF/QFGmsDY WTXwq4jRLjW+RChwfPKqGe7ctT4cd4Qf09lxY= 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=h66ZQzG5VUNMmmmYu3Sww6XM+/dS7FCmTk2ptaIcmbw=; b=Lff6RFo8WNt7akxnYy3Z1lXnOrCnJl7O0pT/oTfvC2F93VGADX0CBZl4+x4tiuAHw/ TzjXFBjmxvBg9eNqZdsJ/np4huaiIfv1hrIZm3vQQTUEWvnwmsfPozG3Gcsw0je7IbHR HQHtKG3Ckf0SRhLbyfIMFQwWz5QLkN/dNhh3bwtn4FAApnSxc0hX448jUWJCWSC5sfGs VLEh18LDN4X4izKTZAfHZq8bkX81xI6her/GaSIlq5/Pq5Er8EloRxkkm688NRyyK/vT dwMIUlnSxxbbowWSCVEJCHl+HPaO6DASzyD58KjeLPkPFhkWygROEPGGT3flkh88s7DR 0I8Q== X-Gm-Message-State: ALKqPwcDjfMXrvSKxePYpU+l3rtIu3Kwzczob8FQRTI0QWmWiAAXTcVI Qpbb3aZM/91Ppabz1rbLQZGtFvJqwNX+2ETXYyfgaw== X-Google-Smtp-Source: AB8JxZpXyxrm860CX+vbeEkn47mFWkZi0r/x9E8aJsBDNheuEZLaAB/vsQcA17hhJK+OE/hqjpcPXOSN9ubPW1tJvdo= X-Received: by 2002:a24:af45:: with SMTP id l5-v6mr1632658iti.106.1527011230247; Tue, 22 May 2018 10:47:10 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a6b:bb86:0:0:0:0:0 with HTTP; Tue, 22 May 2018 10:47:09 -0700 (PDT) In-Reply-To: References: <20180522140850.30369-1-ard.biesheuvel@linaro.org> <20180522140850.30369-4-ard.biesheuvel@linaro.org> From: Ard Biesheuvel Date: Tue, 22 May 2018 19:47:09 +0200 Message-ID: To: "Kinney, Michael D" Cc: "edk2-devel@lists.01.org" , Laszlo Ersek , Leif Lindholm , "Gao, Liming" , "Zeng, Star" , "Dong, Eric" , "Bi, Dandan" Subject: Re: [PATCH 3/6] MdePkg/UefiLib: introduce EfiAllocatePeiAccessiblePages routine 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: Tue, 22 May 2018 17:47:11 -0000 Content-Type: text/plain; charset="UTF-8" On 22 May 2018 at 19:40, Kinney, Michael D wrote: > Ard, > > The corner case that does not work with this > approach is when X64 DXE is combined with an > X64 PEI. OVMF uses this and other platforms > could choose to use X64 PEI phase. > Actually, this approach simply encodes the current status quo. X64 DXE builds unconditionally limit the allocations to < 4 GB if PEI needs to access them, regardless of whether PEI is 32-bit or 64-bit. Also, OVMF's X64 PEI still only maps the lower 4 GB of DRAM, so it actually relies on the current behavior, and allocating above 4 GB under the assumption that a 64-bit PEI will be able to access it will actually break things. > The other mismatch here is adding some PI Spec > Concepts (e.g. PEI phase) to a UEFI library. > Maybe DxeServicesLib would be a better place > to put this type of API. > OK, fair enough. > One clue we have about the memory usage in the > PEI phase is from the EFI_HOB_HANDOFF_INFO_TABLE > HOB. > > /// > /// The highest address location of memory that is allocated for use by the HOB producer > /// phase. This address must be 4-KB aligned to meet page restrictions of UEFI. > /// > EFI_PHYSICAL_ADDRESS EfiMemoryTop; > > /// > /// The highest address location of free memory that is currently available > /// for use by the HOB producer phase. > /// > EFI_PHYSICAL_ADDRESS EfiFreeMemoryTop; > > So maybe we could have an X64 specific implementation > of this new API that checks one of these HOB fields. > If they are below 4GB, then allocate memory below > 4GB. If one is above 4GB, then no restrictions. > All other archs allocate with no restrictions. > That works for me. > Now this approach will still allocate below 4GB for > X64 PEI if the this HOB contains addressed below 4GB, > but that would match the memory usage for that > X64 PEI implementation. > OK, to summarize: - move the implementation of EfiAllocatePeiAccessiblePages() to DxeServicesLib (and perhaps rename it to something more appropriate for its new home) - only restrict the X64 version to below 4 GB if EfiMemoryTop and EfiFreeMemoryTop are both below 4 GB. I will give others some time to respond to this. Thanks, Ard. >> -----Original Message----- >> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] >> Sent: Tuesday, May 22, 2018 7:09 AM >> To: edk2-devel@lists.01.org >> Cc: Ard Biesheuvel ; Laszlo >> Ersek ; Leif Lindholm >> ; Kinney, Michael D >> ; Gao, Liming >> ; Zeng, Star >> ; Dong, Eric ; >> Bi, Dandan >> Subject: [PATCH 3/6] MdePkg/UefiLib: introduce >> EfiAllocatePeiAccessiblePages routine >> >> Add a routine to UefiLib that abstracts the allocation >> of memory that >> should be accessible by PEI after a warm reboot. We will >> use it to >> replace open coded implementations that limit the >> address to < 4 GB, >> which may not be possible on non-Intel systems that have >> no 32-bit >> addressable memory at all. >> >> Contributed-under: TianoCore Contribution Agreement 1.1 >> Signed-off-by: Ard Biesheuvel >> >> --- >> MdePkg/Include/Library/UefiLib.h | 23 ++++++++++ >> MdePkg/Library/UefiLib/UefiLib.c | 48 >> ++++++++++++++++++++ >> 2 files changed, 71 insertions(+) >> >> diff --git a/MdePkg/Include/Library/UefiLib.h >> b/MdePkg/Include/Library/UefiLib.h >> index 256498e3fd8d..8fa077af41e0 100644 >> --- a/MdePkg/Include/Library/UefiLib.h >> +++ b/MdePkg/Include/Library/UefiLib.h >> @@ -1520,4 +1520,27 @@ EfiLocateProtocolBuffer ( >> OUT UINTN *NoProtocols, >> OUT VOID ***Buffer >> ); >> + >> +/** >> + Allocates one or more 4KB pages of a given type from >> a memory region that is >> + accessible to PEI. >> + >> + Allocates the number of 4KB pages of type >> 'MemoryType' and returns a >> + pointer to the allocated buffer. The buffer returned >> is aligned on a 4KB >> + boundary. If Pages is 0, then NULL is returned. If >> there is not enough >> + memory remaining to satisfy the request, then NULL is >> returned. >> + >> + @param[in] MemoryType The memory type to >> allocate >> + @param[in] Pages The number of 4 KB >> pages to allocate. >> + >> + @return A pointer to the allocated buffer or NULL if >> allocation fails. >> + >> +**/ >> +VOID * >> +EFIAPI >> +EfiAllocatePeiAccessiblePages ( >> + IN EFI_MEMORY_TYPE MemoryType, >> + IN UINTN Pages >> + ); >> + >> #endif >> diff --git a/MdePkg/Library/UefiLib/UefiLib.c >> b/MdePkg/Library/UefiLib/UefiLib.c >> index ba449a1c34ce..3a9d75149dd7 100644 >> --- a/MdePkg/Library/UefiLib/UefiLib.c >> +++ b/MdePkg/Library/UefiLib/UefiLib.c >> @@ -1715,3 +1715,51 @@ EfiLocateProtocolBuffer ( >> >> return EFI_SUCCESS; >> } >> + >> +/** >> + Allocates one or more 4KB pages of a given type from >> a memory region that is >> + accessible to PEI. >> + >> + Allocates the number of 4KB pages of type >> 'MemoryType' and returns a >> + pointer to the allocated buffer. The buffer returned >> is aligned on a 4KB >> + boundary. If Pages is 0, then NULL is returned. If >> there is not enough >> + memory remaining to satisfy the request, then NULL is >> returned. >> + >> + @param[in] MemoryType The memory type to >> allocate >> + @param[in] Pages The number of 4 KB >> pages to allocate. >> + >> + @return A pointer to the allocated buffer or NULL if >> allocation fails. >> + >> +**/ >> +VOID * >> +EFIAPI >> +EfiAllocatePeiAccessiblePages ( >> + IN EFI_MEMORY_TYPE MemoryType, >> + IN UINTN Pages >> + ) >> +{ >> + EFI_STATUS Status; >> + EFI_PHYSICAL_ADDRESS Memory; >> + EFI_ALLOCATE_TYPE AllocType; >> + >> + if (Pages == 0) { >> + return NULL; >> + } >> + >> +#ifdef MDE_CPU_X64 >> + // >> + // On X64 systems, a X64 build of DXE may be combined >> with a 32-bit build of >> + // PEI, and so we need to allocate below 4 GB to >> ensure that the allocation >> + // is accessible by PEI. >> + // >> + AllocType = AllocateMaxAddress; >> + Memory = MAX_UINT32; >> +#else >> + AllocType = AllocateAnyPages; >> +#endif >> + Status = gBS->AllocatePages (AllocType, MemoryType, >> Pages, &Memory); >> + if (EFI_ERROR (Status)) { >> + return NULL; >> + } >> + return (VOID *)(UINTN)Memory; >> +} >> -- >> 2.17.0 >