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 <[email protected]> > Cc: Liming Gao <[email protected]> > Cc: Laszlo Ersek <[email protected]> > Cc: Eric Auger <[email protected]> > Cc: Andrew Jones <[email protected]> > Cc: Philippe Mathieu-Daude <[email protected]> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel <[email protected]> > --- > 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 <[email protected]> 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.) Thanks, Laszlo _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
