From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4864:20::141; helo=mail-it1-x141.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it1-x141.google.com (mail-it1-x141.google.com [IPv6:2607:f8b0:4864:20::141]) (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 81C692194D3B3 for ; Fri, 7 Dec 2018 02:43:27 -0800 (PST) Received: by mail-it1-x141.google.com with SMTP id x19so6211570itl.1 for ; Fri, 07 Dec 2018 02:43:26 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=EnwYPddXiz7HxipzXfA7sYGc04R/J+i7WtCqP0m+EUM=; b=OyGJxijVZDNKFNtO7KZV3HBounaVVJgt/tfCr2GPaYJB/PjxvXaVrfDqEgx+p7uJdA 46x4PZefNqPKxS8TIJBwHcj3f+oHqii8TQ0mcGeHeZ1YvwltOQ7y2J9TzhWJV0nuSJgD Xj4r12hMaGXGRx8st2s6FTkL/hOGCTKiLHZVc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=EnwYPddXiz7HxipzXfA7sYGc04R/J+i7WtCqP0m+EUM=; b=rTSjWfrty78EtUQ2H5aW3imN/c2VtMHJDc5SW5s7dS6BMJOjczxednEkdrL7b9MBWB FLdSrUZIb1EOia/Ed3xptCCm1arQx2kScGNeiQalid1aY/IizVEwHEPx/6uYKsT9MtFu fPGW/YY6k3764KduC51DN30YjSGG1oRysqaGc6wDusAnBbBfJIDSXTIhJm/dMy482jDQ PyGEUOU50dRgWox8449AU3DpvZKSl4kJeqZJafqwcN3l62H/VJOmtIa4++rxSE2P5m6b c30sU42yqn1Dq4yCdfnbWY3t4RgGJCcESU+NVZSjkM4v+wlgEwNF+XzQo+SgLCJiqV2e uKGA== X-Gm-Message-State: AA+aEWZsTQSJC4ML4a9uFrtQYcOlykwl74UvfPOMfE1gziGaC7Erjs1x 6ywwvMEbXqD6EP1zDNOhsDAWKRNSsvMyLWgSdRu8zQ== X-Google-Smtp-Source: AFSGD/XjAw7Vu3qL+lCxdyzkgcRHuEIn+jDAC/Jx3LixpKpqz9Qa0myZyI5odKeDZdO6p+n/PZp9EMnf7MUgHMPiKlY= X-Received: by 2002:a24:710:: with SMTP id f16mr1393417itf.121.1544179405623; Fri, 07 Dec 2018 02:43:25 -0800 (PST) MIME-Version: 1.0 References: <20181206213722.7597-1-ard.biesheuvel@linaro.org> <14bc84c4-9b86-51b0-3b09-152e8877950c@redhat.com> In-Reply-To: <14bc84c4-9b86-51b0-3b09-152e8877950c@redhat.com> From: Ard Biesheuvel Date: Fri, 7 Dec 2018 11:43:14 +0100 Message-ID: To: Laszlo Ersek Cc: "edk2-devel@lists.01.org" , Leif Lindholm , "Gao, Liming" , Auger Eric , Andrew Jones , =?UTF-8?Q?Philippe_Mathieu=2DDaud=C3=A9?= Subject: Re: [PATCH] Revert "MdePkg/ProcessorBind.h AARCH64: limit MAX_ADDRESS to 48 bits" X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 07 Dec 2018 10:43:27 -0000 Content-Type: text/plain; charset="UTF-8" On Fri, 7 Dec 2018 at 11:41, Laszlo Ersek wrote: > > On 12/06/18 22:37, Ard Biesheuvel wrote: > > This reverts commit 82379bf6603274e81604d5a6f6bb14bdde616286. > > > > On AArch64, we can only use 48 address bits while running in UEFI, > > while the GCD and UEFI memory maps may describe up to 52 bits of > > physical address space. For this reason, MAX_ADDRESS was reduced > > to 48 bits, to ensure that the firmware does not inadvertently > > attempt to allocate memory that we cannot access. > > > > However, MAX_ADDRESS is used in runtime drivers as well, and > > runtime drivers may deal with kernel virtual addresses, which have > > bits [63:48] set. In fact, the OS may be running with 64 KB pages > > and pass addresses into the runtime services that use up to 52 > > bits of address space, either with the top bits set or cleared, > > even if the physical address space does not extend beyond 48 bits. > > > > In summary, changing MAX_ADDRESS is a mistake, and needs to be > > reverted. > > > > Cc: Leif Lindholm > > Cc: Liming Gao > > Cc: Laszlo Ersek > > Cc: Eric Auger > > Cc: Andrew Jones > > Cc: Philippe Mathieu-Daude > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Ard Biesheuvel > > --- > > MdePkg/Include/AArch64/ProcessorBind.h | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/MdePkg/Include/AArch64/ProcessorBind.h b/MdePkg/Include/AArch64/ProcessorBind.h > > index dad75df1c579..968c18f915ae 100644 > > --- a/MdePkg/Include/AArch64/ProcessorBind.h > > +++ b/MdePkg/Include/AArch64/ProcessorBind.h > > @@ -138,9 +138,9 @@ typedef INT64 INTN; > > #define MAX_2_BITS 0xC000000000000000ULL > > > > /// > > -/// Maximum legal AARCH64 address (48 bits for 4 KB page size) > > +/// Maximum legal AARCH64 address > > /// > > -#define MAX_ADDRESS 0xFFFFFFFFFFFFULL > > +#define MAX_ADDRESS 0xFFFFFFFFFFFFFFFFULL > > > > /// > > /// Maximum legal AArch64 INTN and UINTN values. > > > > I was worried the patch could regress some things, but unfortunately, I > couldn't name any specific area of concern. Sorry about that. > Sometimes a hunch should be taken more seriously, I suppose :-) > For this change: > > Reviewed-by: Laszlo Ersek > Thanks > Topic change: the patch that's being reverted was originally posted as: > > [edk2] [PATCH v3 05/16] > MdePkg/ProcessorBind.h AARCH64: limit MAX_ADDRESS to 48 bits > > in the series > > [edk2] [PATCH v3 00/16] > [Arm|ArmVirt|MdePkg|Embedded]Pkg: lift 40-bit IPA space limit > > In further patches of that series, we depended on the lowered limit of > MAX_ADDRESS. Given that MAX_ADDRESS is being raised back to its original > value, I think those dependent locations should be re-checked. > > For example, in > > [edk2] [PATCH v3 08/16] > ArmPkg/ArmMmuLib: take the CPU supported maximum PA space into account > > (commit e36b243c7178), we added > > // > // Limit the virtual address space to what we can actually use: UEFI > // mandates a 1:1 mapping, so no point in making the virtual address > // space larger than the physical address space. We also have to take > // into account the architectural limitations that result from UEFI's > // use of 4 KB pages. > // > MaxAddress = MIN (LShiftU64 (1ULL, ArmGetPhysicalAddressBits ()) - 1, > MAX_ADDRESS); > > Presumably, we should now replace MAX_ADDRESS with 0xFFFFFFFFFFFFULL. > > (I'm unsure if other modules updated by the rest of the patches are > affected -- I tried to grep them for MAX_ADDRESS, and I couldn't find > any (obvious) matches.) > I am looking into an alternative approach, involving the introduction of MAX_ALLOC_ADDRESS, and updating the page allocator to honour it. The MMU library should be updated to refer to it as well, thanks for reminding me.