On 11/17/17 17:09, Ard Biesheuvel wrote:
> Clone the existing ArmPlatformGetVirtualMemoryMap () for this platform,
> clean it up slightly (by removing the support for uncached DRAM mappings),
> and turn it into a new ArmVirtMemInfoLib implementation.
I've looked at this patch with "git show --find-copies-harder". It looks
OK, but the commit message could be improved:
(1) the support for uncached DRAM mappings is removed in the copy-origin
lib instance, in patch 09/15 ("ArmVirtPkg/ArmVirtPlatformLib: remove
support for uncached mappings"). I think this sentence should be dropped
from the commit message.
(2) There are other cleanups however:
- factor out TableSize and TopOfMemory
- replace EFI_D_* with DEBUG_*
- fetch PcdFdBaseAddress with PcdGet64(), not FixedPcdGet64(). (This is
matched by the [Pcd] / [FixedPcd] sections in the new INF file.)
Can you elaborate on the last item? I wonder if that change qualifies as
cleanup. (I'm fine if the change is justified by some other flexibility,
but it should be documented please.)
With the commit message updated:
Reviewed-by: Laszlo Ersek <ler...@redhat.com>
Thanks
Laszlo
>
> 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 <ard.biesheu...@linaro.org>
> ---
> ArmVirtPkg/ArmVirtQemuKernel.dsc | 1 +
> ArmVirtPkg/Library/QemuVirtMemInfoLib/AArch64/PhysAddrTop.S | 39 ++++++++
> ArmVirtPkg/Library/QemuVirtMemInfoLib/Arm/PhysAddrTop.S | 24 +++++
> ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c | 101
> ++++++++++++++++++++
> ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf | 53 ++++++++++
> 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 <AsmMacroIoLibV8.h>
> +
> +//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 <AsmMacroIoLib.h>
> +
> +//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..b08305fe4a4f
> --- /dev/null
> +++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.c
> @@ -0,0 +1,101 @@
> +/** @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 <Base.h>
> +#include <Library/ArmLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +
> +// 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
> +
> +**/
> +VOID
> +ArmVirtGetMemoryMap (
> + OUT ARM_MEMORY_REGION_DESCRIPTOR **VirtualMemoryMap
> + )
> +{
> + ARM_MEMORY_REGION_DESCRIPTOR *VirtualMemoryTable;
> + UINT64 TopOfMemory;
> + UINTN TableSize;
> +
> + ASSERT (VirtualMemoryMap != NULL);
> +
> + TableSize = sizeof (ARM_MEMORY_REGION_DESCRIPTOR) *
> + MAX_VIRTUAL_MEMORY_MAP_DESCRIPTORS;
> + VirtualMemoryTable = AllocatePages (EFI_SIZE_TO_PAGES (TableSize));
> +
> + if (VirtualMemoryTable == NULL) {
> + DEBUG ((DEBUG_ERROR, "%a: Error: Failed AllocatePages()\n",
> __FUNCTION__));
> + return;
> + }
> +
> + // 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..8f6ba006fdfd
> --- /dev/null
> +++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoLib.inf
> @@ -0,0 +1,53 @@
> +#/* @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
> +
> +[Pcd]
> + gArmTokenSpaceGuid.PcdFdBaseAddress
> + gArmTokenSpaceGuid.PcdSystemMemoryBase
> + gArmTokenSpaceGuid.PcdSystemMemorySize
> +
> +[FixedPcd]
> + gArmTokenSpaceGuid.PcdFdSize
> + gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
