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 3660821A02937 for ; Wed, 28 Nov 2018 11:26:18 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 82EACA4021; Wed, 28 Nov 2018 19:26:17 +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 9EED91054FD3; Wed, 28 Nov 2018 19:26:06 +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-9-ard.biesheuvel@linaro.org> From: Laszlo Ersek Message-ID: Date: Wed, 28 Nov 2018 20:26:05 +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-9-ard.biesheuvel@linaro.org> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.38]); Wed, 28 Nov 2018 19:26:17 +0000 (UTC) Subject: Re: [PATCH v3 08/16] ArmPkg/ArmMmuLib: take the CPU supported maximum PA space into account 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 19:26:18 -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: > In preparation of dropping PcdPrePiCpuMemorySize entirely, base the > maximum size of the identity map on the capabilities of the CPU. > Since that may exceed what is architecturally permitted when using > 4 KB pages, take MAX_ADDRESS into account as well. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel > --- > ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf | 3 --- > ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf | 3 --- > ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 11 +++++++++-- > 3 files changed, 9 insertions(+), 8 deletions(-) > > diff --git a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf > index b9f264de8d26..246963361e45 100644 > --- a/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf > +++ b/ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf > @@ -40,8 +40,5 @@ [LibraryClasses] > CacheMaintenanceLib > MemoryAllocationLib > > -[Pcd.AARCH64] > - gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize > - > [Pcd.ARM] > gArmTokenSpaceGuid.PcdNormalMemoryNonshareableOverride > diff --git a/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf b/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf > index ecf13f790734..f689c193b862 100644 > --- a/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf > +++ b/ArmPkg/Library/ArmMmuLib/ArmMmuPeiLib.inf > @@ -35,6 +35,3 @@ [LibraryClasses] > ArmLib > CacheMaintenanceLib > MemoryAllocationLib > - > -[Pcd.AARCH64] > - gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize OK > diff --git a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c > index 4b62ecb6a476..5403b8d4070e 100644 > --- a/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c > +++ b/ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c > @@ -604,8 +604,15 @@ ArmConfigureMmu ( > return EFI_INVALID_PARAMETER; > } > > - // Cover the entire GCD memory space > - MaxAddress = (1UL << PcdGet8 (PcdPrePiCpuMemorySize)) - 1; > + // > + // 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); > > // Lookup the Table Level to get the information > LookupAddresstoRootTable (MaxAddress, &T0SZ, &RootTableEntryCount); > This part of edk2 (or ArmVirtQemu) is where I *always* get lost. So let me check the call tree. ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf [ArmVirtPkg/ArmVirtQemu.dsc] InitializeMemory() [ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c] PeiServicesInstallPeiMemory() [...] // now we've got permanent PEI RAM MemoryPeim() [ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c] InitMmu() [ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c] ArmVirtGetMemoryMap() [ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c] ArmConfigureMmu() [ArmPkg/Library/ArmMmuLib/AArch64/ArmMmuLibCore.c] In patch v3 04/16, we modify ArmVirtGetMemoryMap() so that the mapping not cover the "Peripheral space after DRAM" (which is practically "limitless"). Good. In this patch, we have to modify ArmConfigureMmu() as well, because the descriptor array that we pass it from ArmVirtGetMemoryMap() does not contain all the information that is necessary for setting up "paging" in general. Also considering mappings that could be added later during DXE, such as MMIO ranges of platform drivers / devices, discontiguous RAM ranges, and so on. So we need a "maximum" here. The maximum that we choose doesn't "seriously" translate to an increased memory footprint of paging artifacts, *unlike* the size of the address space that we actually map for access. Hence in patch v3 04/16, we are conservative, but in this patch, when selecting the maximum, we go as high as we can -- either the architecturally permitted limit (with 4KB page size), or, if the latter is lower, the limit supported by the current CPU. The GCD *could* go higher (even though we'd never attempt to map that in UEFI), assuming the CPU is capable enough (using 64KB pages at OS runtime). This bends my brain. :) I'm now slowly comprehending your blurb. Indeed, the rest of the series pertains to the GCD memory space map. Nit: the second argument of the MIN() macro is not indented idiomatically. With that fixed (no need to repost the series just because of it): Reviewed-by: Laszlo Ersek Thanks Laszlo