From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 7F19D21A02937 for ; Wed, 28 Nov 2018 10:41:15 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 0C5713078AAF; Wed, 28 Nov 2018 18:41:15 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-170.rdu2.redhat.com [10.10.120.170]) by smtp.corp.redhat.com (Postfix) with ESMTP id 70AAD6B8C5; Wed, 28 Nov 2018 18:41:09 +0000 (UTC) To: Ard Biesheuvel , edk2-devel@lists.01.org Cc: Leif Lindholm , Eric Auger , Andrew Jones , Philippe Mathieu-Daude , Julien Grall References: <20181128143357.991-1-ard.biesheuvel@linaro.org> <20181128143357.991-6-ard.biesheuvel@linaro.org> From: Laszlo Ersek Message-ID: <8ad971d4-c8d8-2f6e-0d60-c61e4ed362d7@redhat.com> Date: Wed, 28 Nov 2018 19:41:08 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20181128143357.991-6-ard.biesheuvel@linaro.org> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.48]); Wed, 28 Nov 2018 18:41:15 +0000 (UTC) Subject: Re: [PATCH v3 05/16] 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: Wed, 28 Nov 2018 18:41:15 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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 > Reviewed-by: Leif Lindholm > --- > 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): http://mid.mail-archive.com/6f1209fb-bb89-a70f-ba0e-3ebf2e12e459@redhat.com 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