On 11/28/18 15:33, Ard Biesheuvel wrote: > AArch64 supports the use of more than 48 bits for physical and/or > virtual addressing, but only if the page size is set to 64 KB, > which is not supported by UEFI. So redefine MAX_ADDRESS to cover > only 48 address bits. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel <[email protected]> > Reviewed-by: Leif Lindholm <[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 968c18f915ae..dad75df1c579 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 > +/// Maximum legal AARCH64 address (48 bits for 4 KB page size) > /// > -#define MAX_ADDRESS 0xFFFFFFFFFFFFFFFFULL > +#define MAX_ADDRESS 0xFFFFFFFFFFFFULL > > /// > /// Maximum legal AArch64 INTN and UINTN values. >
Hmmm. I bit the bullet and grepped the tree for MAX_ADDRESS. The amount of hits is staggering. I can't audit all of them. Generally, MAX_ADDRESS seems to be used in checks that prevent address wrap-around. In that regard, this change looks valid. I can't guarantee this change won't regress anything though. In the previous posting of this patch, I asked Liming some questions (IIRC): [email protected]">http://mid.mail-archive.com/[email protected] It would be nice to see answers. :) In addition: (a) in "BaseTools/Source/C/Include/AArch64/ProcessorBind.h", we have another instance of the macro definition. I suspect it should be kept in sync. (b) in "BaseTools/Source/C/Common/CommonLib.h", we have: #define MAX_UINTN MAX_ADDRESS which I think relies on (a), and hence it will be amusingly wrong after we synchronize (a) with MdePkg. (BTW, (b) is exactly the kind of assumption that scares me about this patch.) We're not much past the last stable tag (edk2-stable201811), so let's hope there's going to be enough time to catch any regressions. With (a) and (b) investigated / fixed up, I'd be willing to A-b this. Cautiously :) Anyway, this is for MdePkg, so my review is not required. (I certainly do not intend to *oppose* this patch.) Thanks Laszlo _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
