diff mbox series

[edk2,v2,12/14] ArmVirtPkg: create QemuVirtMemInfoLib version for ArmVirtQemu

Message ID 20171122100731.24525-13-ard.biesheuvel@linaro.org
State Accepted
Commit 048651260b66105fde6ce63de05902e5b731ac56
Headers show
Series ArmVirtPkg: get rid of ArmPlatformLib | expand

Commit Message

Ard Biesheuvel Nov. 22, 2017, 10:07 a.m. UTC
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(+)

-- 
2.11.0

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Laszlo Ersek Nov. 23, 2017, 4:26 p.m. UTC | #1
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;

> +}

> 


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
diff mbox series

Patch

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
+  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__));
+      }
+    }
+  }
+
+  //
+  // 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;
+}