[edk2] [PATCH] Revert "MdePkg/ProcessorBind.h AARCH64: limit MAX_ADDRESS to 48 bits"
lersek at redhat.com
Fri Dec 7 02:41:09 PST 2018
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
> Cc: Leif Lindholm <leif.lindholm at linaro.org>
> Cc: Liming Gao <liming.gao at intel.com>
> Cc: Laszlo Ersek <lersek at redhat.com>
> Cc: Eric Auger <eric.auger at redhat.com>
> Cc: Andrew Jones <drjones at redhat.com>
> Cc: Philippe Mathieu-Daude <philmd at redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> 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.
For this change:
Reviewed-by: Laszlo Ersek <lersek at redhat.com>
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,
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.)
More information about the edk2-devel