From: Laszlo Ersek <lersek@redhat.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>, edk2-devel@lists.01.org
Subject: Re: [PATCH v2 12/14] ArmVirtPkg: create QemuVirtMemInfoLib version for ArmVirtQemu
Date: Thu, 23 Nov 2017 17:26:58 +0100 [thread overview]
Message-ID: <1e355c8e-0bca-7a07-eea1-0cff0b2eb721@redhat.com> (raw)
In-Reply-To: <20171122100731.24525-13-ard.biesheuvel@linaro.org>
On 11/22/17 11:07, Ard Biesheuvel wrote:
> The QemuVirtMemInfoLib ArmVirtMemInfoLib implementation created for
> ArmVirtQemuKernel does exactly what we need for ArmVirtQemu, the only
> difference being that the latter is PrePeiCore based, and so it uses
> a different method to ensure that PcdSystemMemorySize is set when
> ArmVirtGetMemoryMap() is called.
>
> On ArmVirtQemu, we currently abuse the implied ordering guarantees
> provided by ArmPlatformLib, by implementing this as follows:
>
> ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf [ArmVirtPkg/ArmVirtQemu.dsc]
> InitializeMemory() [ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.c]
> ArmPlatformInitializeSystemMemory() [ArmVirtPkg/Library/ArmVirtPlatformLib/Virt.c]
> //
> // set PcdSystemMemorySize from the DT
> //
> MemoryPeim() [ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c]
> InitMmu() [ArmVirtPkg/Library/ArmVirtMemoryInitPeiLib/ArmVirtMemoryInitPeiLib.c]
> ArmPlatformGetVirtualMemoryMap() [ArmVirtPkg/Library/ArmVirtPlatformLib/VirtMem.c]
> //
> // consume PcdSystemMemorySize
> //
>
> Given that we are trying to get rid of ArmPlatformLib, or at least remove
> some of these API functions that are never used for their original purpose
> by any platforms, we need to move the PCD assignment elsewhere.
>
> So create a PEIM-only version of QemuVirtMemInfoLib especially for
> ArmVirtQemu, and add the PCD assignment code to its constructor.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> ArmVirtPkg/ArmVirtQemu.dsc | 3 +
> ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf | 58 +++++++++++
> ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLibConstructor.c | 106 ++++++++++++++++++++
> 3 files changed, 167 insertions(+)
>
> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> index d14a0dd0d1d9..519c2ae2e939 100644
> --- a/ArmVirtPkg/ArmVirtQemu.dsc
> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> @@ -67,6 +67,9 @@ [LibraryClasses.common]
> HttpLib|MdeModulePkg/Library/DxeHttpLib/DxeHttpLib.inf
> !endif
>
> +[LibraryClasses.common.PEIM]
> + ArmVirtMemInfoLib|ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf
> +
> [LibraryClasses.common.UEFI_DRIVER]
> UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
>
> diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf
> new file mode 100644
> index 000000000000..e574a47443d0
> --- /dev/null
> +++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf
> @@ -0,0 +1,58 @@
> +#/* @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
(1) Please update BASE_NAME before pushing.
> + FILE_GUID = 0c4d10cf-d949-49b4-bd13-47a4ae22efce
> + MODULE_TYPE = BASE
> + VERSION_STRING = 1.0
> + LIBRARY_CLASS = ArmVirtMemInfoLib|PEIM
> + CONSTRUCTOR = QemuVirtMemInfoPeiLibConstructor
> +
> +[Sources]
> + QemuVirtMemInfoLib.c
> + QemuVirtMemInfoPeiLibConstructor.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
> + FdtLib
> + PcdLib
> + MemoryAllocationLib
> +
> +[Pcd]
> + gArmTokenSpaceGuid.PcdFdBaseAddress
> + gArmTokenSpaceGuid.PcdSystemMemoryBase
> + gArmTokenSpaceGuid.PcdSystemMemorySize
> +
> +[FixedPcd]
> + gArmTokenSpaceGuid.PcdFdSize
> + gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress
> + gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize
> diff --git a/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLibConstructor.c b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLibConstructor.c
> new file mode 100644
> index 000000000000..ef8ac6e018d1
> --- /dev/null
> +++ b/ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLibConstructor.c
> @@ -0,0 +1,106 @@
> +/** @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/DebugLib.h>
> +#include <Library/PcdLib.h>
> +#include <libfdt.h>
> +
> +RETURN_STATUS
> +EFIAPI
> +QemuVirtMemInfoPeiLibConstructor (
> + VOID
> + )
> +{
> + VOID *DeviceTreeBase;
> + INT32 Node, Prev;
> + UINT64 NewBase, CurBase;
> + UINT64 NewSize, CurSize;
> + CONST CHAR8 *Type;
> + INT32 Len;
> + CONST UINT64 *RegProp;
> + RETURN_STATUS PcdStatus;
> +
> + NewBase = 0;
> + NewSize = 0;
> +
> + DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeInitialBaseAddress);
> + ASSERT (DeviceTreeBase != NULL);
> +
> + //
> + // Make sure we have a valid device tree blob
> + //
> + ASSERT (fdt_check_header (DeviceTreeBase) == 0);
> +
> + //
> + // Look for the lowest memory node
> + //
> + for (Prev = 0;; Prev = Node) {
> + Node = fdt_next_node (DeviceTreeBase, Prev, NULL);
> + if (Node < 0) {
> + break;
> + }
> +
> + //
> + // Check for memory node
> + //
> + Type = fdt_getprop (DeviceTreeBase, Node, "device_type", &Len);
> + if (Type && AsciiStrnCmp (Type, "memory", Len) == 0) {
> + //
> + // Get the 'reg' property of this node. For now, we will assume
> + // two 8 byte quantities for base and size, respectively.
> + //
> + RegProp = fdt_getprop (DeviceTreeBase, Node, "reg", &Len);
> + if (RegProp != 0 && Len == (2 * sizeof (UINT64))) {
> +
> + CurBase = fdt64_to_cpu (ReadUnaligned64 (RegProp));
> + CurSize = fdt64_to_cpu (ReadUnaligned64 (RegProp + 1));
> +
> + DEBUG ((DEBUG_INFO, "%a: System RAM @ 0x%lx - 0x%lx\n",
> + __FUNCTION__, CurBase, CurBase + CurSize - 1));
> +
> + if (NewBase > CurBase || NewBase == 0) {
> + NewBase = CurBase;
> + NewSize = CurSize;
> + }
> + } else {
> + DEBUG ((DEBUG_ERROR, "%a: Failed to parse FDT memory node\n",
> + __FUNCTION__));
(2) Thanks for improving the indentation of the DEBUGs.
("--find-copies-harder" remains awesome :) )
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks!
Laszlo
> + }
> + }
> + }
> +
> + //
> + // Make sure the start of DRAM matches our expectation
> + //
> + ASSERT (FixedPcdGet64 (PcdSystemMemoryBase) == NewBase);
> + PcdStatus = PcdSet64S (PcdSystemMemorySize, NewSize);
> + ASSERT_RETURN_ERROR (PcdStatus);
> +
> + //
> + // We need to make sure that the machine we are running on has at least
> + // 128 MB of memory configured, and is currently executing this binary from
> + // NOR flash. This prevents a device tree image in DRAM from getting
> + // clobbered when our caller installs permanent PEI RAM, before we have a
> + // chance of marking its location as reserved or copy it to a freshly
> + // allocated block in the permanent PEI RAM in the platform PEIM.
> + //
> + ASSERT (NewSize >= SIZE_128MB);
> + ASSERT (
> + (((UINT64)PcdGet64 (PcdFdBaseAddress) +
> + (UINT64)PcdGet32 (PcdFdSize)) <= NewBase) ||
> + ((UINT64)PcdGet64 (PcdFdBaseAddress) >= (NewBase + NewSize)));
> +
> + return RETURN_SUCCESS;
> +}
>
next prev parent reply other threads:[~2017-11-23 16:22 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-11-22 10:07 [PATCH v2 00/14] ArmVirtPkg: get rid of ArmPlatformLib Ard Biesheuvel
2017-11-22 10:07 ` [PATCH v2 01/14] ArmVirtPkg/PrePi: run all library constructors by hand Ard Biesheuvel
2017-11-22 10:07 ` [PATCH v2 02/14] ArmVirtPkg/PrePi: remove unused GetPlatformPpi() function Ard Biesheuvel
2017-11-22 10:07 ` [PATCH v2 03/14] ArmVirtPkg/PrePi: remove bogus primary core check Ard Biesheuvel
2017-11-22 10:07 ` [PATCH v2 04/14] ArmVirtPkg/PrePi: move DRAM discovery code into PrePi Ard Biesheuvel
2017-11-22 10:07 ` [PATCH v2 05/14] ArmVirtPkg/PrePi: remove dependency on ArmPlatformLib Ard Biesheuvel
2017-11-22 10:07 ` [PATCH v2 06/14] ArmVirtPkg/PrePi: remove ArmPlatformStackLib dependency Ard Biesheuvel
2017-11-22 10:07 ` [PATCH v2 07/14] ArmVirtPkg/PrePi: remove bogus IntelFrameworkModulePkg.dec dependency Ard Biesheuvel
2017-11-22 10:07 ` [PATCH v2 08/14] ArmVirtPkg/ArmVirtPlatformLib: remove support for uncached mappings Ard Biesheuvel
2017-11-22 10:07 ` [PATCH v2 09/14] ArmVirtPkg: introduce ArmVirtMemInfoLib library class Ard Biesheuvel
2017-11-23 16:00 ` Laszlo Ersek
2017-11-22 10:07 ` [PATCH v2 10/14] ArmVirtPkg/ArmVirtXen: add ArmVirtMemInfoLib implementation Ard Biesheuvel
2017-11-22 17:03 ` Julien Grall
2017-11-22 10:07 ` [PATCH v2 11/14] ArmVirtPkg/ArmVirtQemu: " Ard Biesheuvel
2017-11-23 16:18 ` Laszlo Ersek
2017-11-22 10:07 ` [PATCH v2 12/14] ArmVirtPkg: create QemuVirtMemInfoLib version for ArmVirtQemu Ard Biesheuvel
2017-11-23 16:26 ` Laszlo Ersek [this message]
2017-11-22 10:07 ` [PATCH v2 13/14] ArmVirtPkg/ArmVirtMemoryInitPeiLib: move to ArmVirtMemInfoLib Ard Biesheuvel
2017-11-23 16:32 ` Laszlo Ersek
2017-11-22 10:07 ` [PATCH v2 14/14] ArmVirtPkg: remove ArmPlatformLib implementations Ard Biesheuvel
2017-11-23 16:56 ` [PATCH v2 00/14] ArmVirtPkg: get rid of ArmPlatformLib Ard Biesheuvel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1e355c8e-0bca-7a07-eea1-0cff0b2eb721@redhat.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox