On Wed, 28 Nov 2018 at 19:41, Laszlo Ersek <ler...@redhat.com> wrote: > > 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 <ard.biesheu...@linaro.org> > > Reviewed-by: Leif Lindholm <leif.lindh...@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 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): > > 6f1209fb-bb89-a70f-ba0e-3ebf2e12e459@redhat.com">http://mid.mail-archive.com/6f1209fb-bb89-a70f-ba0e-3ebf2e12e459@redhat.com > > It would be nice to see answers. :) >
Yep > 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. > Indeed. > (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.) > That doesn't make any sense at all. What does 'native' mean in the context of BaseTools anyway? > 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 :) > Thanks > 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 edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
