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 D88BD220C1C36 for ; Thu, 23 Nov 2017 08:14:14 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2CDB085A07; Thu, 23 Nov 2017 16:18:32 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-221.rdu2.redhat.com [10.10.120.221]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2BB7F5D960; Thu, 23 Nov 2017 16:18:30 +0000 (UTC) To: Ard Biesheuvel , edk2-devel@lists.01.org References: <20171122100731.24525-1-ard.biesheuvel@linaro.org> <20171122100731.24525-12-ard.biesheuvel@linaro.org> From: Laszlo Ersek Message-ID: <5049dd7f-f33b-5cf2-2763-5ee2b9001b15@redhat.com> Date: Thu, 23 Nov 2017 17:18:30 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <20171122100731.24525-12-ard.biesheuvel@linaro.org> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Thu, 23 Nov 2017 16:18:32 +0000 (UTC) Subject: Re: [PATCH v2 11/14] ArmVirtPkg/ArmVirtQemu: add ArmVirtMemInfoLib implementation X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 23 Nov 2017 16:14:15 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 11/22/17 11:07, Ard Biesheuvel wrote: > Create a new ArmVirtMemInfoLib for ArmVirtQemuKernel by cloning the > existing ArmPlatformGetVirtualMemoryMap () for this platform, > (ArmQemuRelocatablePlatformLib *not* ArmVirtPlatformLib), and cleaning > it up: > - remove support for uncached DRAM mappings > - replace EFI_D_xxx with DEBUG_xxx throughout > - use a temp variable to hold the top of the physical address space > - use AllocatePool () instead of AllocatePages (), given that we use > 160 bytes only, and the memory is never freed. > > In a future patch, we will add this library to the ordinary ArmVirtQemu > platform as well. > > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel > --- > ArmVirtPkg/ArmVirtQemuKernel.dsc | 1 + > ArmVirtPkg/Library/QemuVirtMemInfoLib/AArch64/PhysAddrTop.S | 39 ++++++++ > ArmVirtPkg/Library/QemuVirtMemInfoLib/Arm/PhysAddrTop.S | 24 +++++ > ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c | 100 ++++++++++++++++++++ > ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf | 54 +++++++++++ > 5 files changed, 218 insertions(+) > > diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc > index 7e5d584344b4..f50d30388cf2 100644 > --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc > +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc > @@ -49,6 +49,7 @@ [LibraryClasses.common] > QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/BaseQemuFwCfgS3LibNull.inf > > ArmPlatformLib|ArmVirtPkg/Library/ArmQemuRelocatablePlatformLib/ArmQemuRelocatablePlatformLib.inf > + ArmVirtMemInfoLib|ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf > > TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf > NorFlashPlatformLib|ArmVirtPkg/Library/NorFlashQemuLib/NorFlashQemuLib.inf > diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/AArch64/PhysAddrTop.S b/ArmVirtPkg/Library/QemuVirtMemInfoLib/AArch64/PhysAddrTop.S > new file mode 100644 > index 000000000000..a1f6a194d59b > --- /dev/null > +++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/AArch64/PhysAddrTop.S > @@ -0,0 +1,39 @@ > +# > +# Copyright (c) 2011-2013, ARM Limited. All rights reserved. > +# Copyright (c) 2016-2017, Linaro Limited. All rights reserved. > +# > +# This program and the accompanying materials > +# are licensed and made available under the terms and conditions of the BSD License > +# which accompanies this distribution. The full text of the license may be found at > +# http://opensource.org/licenses/bsd-license.php > +# > +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > +# > +# > + > +#include > + > +//EFI_PHYSICAL_ADDRESS > +//GetPhysAddrTop ( > +// VOID > +// ); > +ASM_FUNC(ArmGetPhysAddrTop) > + mrs x0, id_aa64mmfr0_el1 > + adr x1, .LPARanges > + and x0, x0, #7 > + ldrb w1, [x1, x0] > + mov x0, #1 > + lsl x0, x0, x1 > + ret > + > +// > +// Bits 0..2 of the AA64MFR0_EL1 system register encode the size of the > +// physical address space support on this CPU: > +// 0 == 32 bits, 1 == 36 bits, etc etc > +// 6 and 7 are reserved > +// > +.LPARanges: > + .byte 32, 36, 40, 42, 44, 48, -1, -1 > + > +ASM_FUNCTION_REMOVE_IF_UNREFERENCED > diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/Arm/PhysAddrTop.S b/ArmVirtPkg/Library/QemuVirtMemInfoLib/Arm/PhysAddrTop.S > new file mode 100644 > index 000000000000..9cd81529fb3d > --- /dev/null > +++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/Arm/PhysAddrTop.S > @@ -0,0 +1,24 @@ > +# > +# Copyright (c) 2011-2013, ARM Limited. All rights reserved. > +# Copyright (c) 2014-2017, Linaro Limited. All rights reserved. > +# > +# This program and the accompanying materials > +# are licensed and made available under the terms and conditions of the BSD License > +# which accompanies this distribution. The full text of the license may be found at > +# http://opensource.org/licenses/bsd-license.php > +# > +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > +# > +# > + > +#include > + > +//EFI_PHYSICAL_ADDRESS > +//GetPhysAddrTop ( > +// VOID > +// ); > +ASM_FUNC(ArmGetPhysAddrTop) > + mov r0, #0x00000000 > + mov r1, #0x10000 > + bx lr > diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c > new file mode 100644 > index 000000000000..ea70f2c33b77 > --- /dev/null > +++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c > @@ -0,0 +1,100 @@ > +/** @file > + > + Copyright (c) 2014-2017, Linaro Limited. All rights reserved. > + > + This program and the accompanying materials are licensed and made available > + under the terms and conditions of the BSD License which accompanies this > + distribution. The full text of the license may be found at > + http://opensource.org/licenses/bsd-license.php > + > + THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > + WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > + > +**/ > + > +#include > +#include > +#include > +#include > +#include > + > +// Number of Virtual Memory Map Descriptors > +#define MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS 5 > + > +EFI_PHYSICAL_ADDRESS > +ArmGetPhysAddrTop ( > + VOID > + ); > + > +/** > + Return the Virtual Memory Map of your platform > + > + This Virtual Memory Map is used by MemoryInitPei Module to initialize the MMU > + on your platform. > + > + @param[out] VirtualMemoryMap Array of ARM_MEMORY_REGION_DESCRIPTOR > + describing a Physical-to-Virtual Memory > + mapping. This array must be ended by a > + zero-filled entry. The allocated memory > + will not be freed. > + > +**/ > +VOID > +ArmVirtGetMemoryMap ( > + OUT ARM_MEMORY_REGION_DESCRIPTOR **VirtualMemoryMap > + ) > +{ > + ARM_MEMORY_REGION_DESCRIPTOR *VirtualMemoryTable; > + UINT64 TopOfMemory; > + > + ASSERT (VirtualMemoryMap != NULL); > + > + VirtualMemoryTable = AllocatePool (sizeof (ARM_MEMORY_REGION_DESCRIPTOR) * > + MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS); Pool allocations like this are satisfied from HOBs, before permanent PEI RAM is installed. As part of the temp RAM migration, the HOBs are moved, and the previous addresses become invalid. (This is not true for memory allocated with AllocatePages().) However, I think in this specific case that should be fine. We actually don't want or need this allocation to stick around forever; the caller (InitMmu()) uses it immediately. And, as far as I can see in InitMmu(), the address of the map is not remembered. So this should be fine. > + > + if (VirtualMemoryTable == NULL) { > + DEBUG ((DEBUG_ERROR, "%a: Error: Failed AllocatePages()\n", __FUNCTION__)); > + return; > + } Not very important, but before pushing, please update the error message to say AllocatePool(). Reviewed-by: Laszlo Ersek Thanks Laszlo > + > + // System DRAM > + VirtualMemoryTable[0].PhysicalBase = PcdGet64 (PcdSystemMemoryBase); > + VirtualMemoryTable[0].VirtualBase = VirtualMemoryTable[0].PhysicalBase; > + VirtualMemoryTable[0].Length = PcdGet64 (PcdSystemMemorySize); > + VirtualMemoryTable[0].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK; > + > + DEBUG ((DEBUG_INFO, "%a: Dumping System DRAM Memory Map:\n" > + "\tPhysicalBase: 0x%lX\n" > + "\tVirtualBase: 0x%lX\n" > + "\tLength: 0x%lX\n", > + __FUNCTION__, > + VirtualMemoryTable[0].PhysicalBase, > + VirtualMemoryTable[0].VirtualBase, > + VirtualMemoryTable[0].Length)); > + > + // Peripheral space before DRAM > + VirtualMemoryTable[1].PhysicalBase = 0x0; > + VirtualMemoryTable[1].VirtualBase = 0x0; > + VirtualMemoryTable[1].Length = VirtualMemoryTable[0].PhysicalBase; > + VirtualMemoryTable[1].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE; > + > + // Peripheral space after DRAM > + TopOfMemory = MIN (1ULL << FixedPcdGet8 (PcdPrePiCpuMemorySize), > + ArmGetPhysAddrTop ()); > + VirtualMemoryTable[2].PhysicalBase = VirtualMemoryTable[0].Length + VirtualMemoryTable[1].Length; > + VirtualMemoryTable[2].VirtualBase = VirtualMemoryTable[2].PhysicalBase; > + VirtualMemoryTable[2].Length = TopOfMemory - > + VirtualMemoryTable[2].PhysicalBase; > + VirtualMemoryTable[2].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_DEVICE; > + > + // Remap the FD region as normal executable memory > + VirtualMemoryTable[3].PhysicalBase = PcdGet64 (PcdFdBaseAddress); > + VirtualMemoryTable[3].VirtualBase = VirtualMemoryTable[3].PhysicalBase; > + VirtualMemoryTable[3].Length = FixedPcdGet32 (PcdFdSize); > + VirtualMemoryTable[3].Attributes = ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK; > + > + // End of Table > + ZeroMem (&VirtualMemoryTable[4], sizeof (ARM_MEMORY_REGION_DESCRIPTOR)); > + > + *VirtualMemoryMap = VirtualMemoryTable; > +} > diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf > new file mode 100644 > index 000000000000..c72a97f9e78a > --- /dev/null > +++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf > @@ -0,0 +1,54 @@ > +#/* @file > +# > +# Copyright (c) 2011-2015, ARM Limited. All rights reserved. > +# Copyright (c) 2014-2017, Linaro Limited. All rights reserved. > +# > +# This program and the accompanying materials are licensed and made available > +# under the terms and conditions of the BSD License which accompanies this > +# distribution. The full text of the license may be found at > +# http://opensource.org/licenses/bsd-license.php > +# > +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, > +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. > +# > +#*/ > + > +[Defines] > + INF_VERSION = 0x0001001A > + BASE_NAME = QemuVirtMemInfoLib > + FILE_GUID = 9b30ca82-6746-4a82-a3e6-11ea79df3b46 > + MODULE_TYPE = BASE > + VERSION_STRING = 1.0 > + LIBRARY_CLASS = ArmVirtMemInfoLib > + > +[Sources] > + QemuVirtMemInfoLib.c > + > +[Sources.ARM] > + Arm/PhysAddrTop.S > + > +[Sources.AARCH64] > + AArch64/PhysAddrTop.S > + > +[Packages] > + ArmPkg/ArmPkg.dec > + ArmVirtPkg/ArmVirtPkg.dec > + EmbeddedPkg/EmbeddedPkg.dec > + MdeModulePkg/MdeModulePkg.dec > + MdePkg/MdePkg.dec > + > +[LibraryClasses] > + ArmLib > + BaseMemoryLib > + DebugLib > + MemoryAllocationLib > + PcdLib > + > +[Pcd] > + gArmTokenSpaceGuid.PcdFdBaseAddress > + gArmTokenSpaceGuid.PcdSystemMemoryBase > + gArmTokenSpaceGuid.PcdSystemMemorySize > + > +[FixedPcd] > + gArmTokenSpaceGuid.PcdFdSize > + gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize >