Re: [PATCH 06/15] ArmVirtPkg/PrePi: move DRAM discovery code into PrePi

[Laszlo Ersek <lersek@redhat.com>]

On 11/17/17 17:09, Ard Biesheuvel wrote:
> ArmVirtQemuKernel and ArmVirtXen use essentially the same code to
> retrieve DRAM information from the DT /memory node at early boot,
> and invoke it via the ArmPlatformPeiBootAction () hook exposed by
> ArmPlatformLib. Let's move this code into the PrePi implementation
> these platforms share between them (and not with any other platforms)
> so we can eliminate another dependency on the messy ArmPlatformLib.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S         | 77 ++++++++++++++++-
>  ArmVirtPkg/PrePi/Arm/ModuleEntryPoint.S             | 71 +++++++++++++++
>  ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf |  2 +
>  ArmVirtPkg/PrePi/FdtParser.c                        | 90 ++++++++++++++++++++
>  4 files changed, 238 insertions(+), 2 deletions(-)
> 
> diff --git a/ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S b/ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S
> index 7a9c0c3787cc..3296aedfe9aa 100644
> --- a/ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S
> +++ b/ArmVirtPkg/PrePi/AArch64/ModuleEntryPoint.S
> @@ -49,8 +49,7 @@ ASM_FUNC(_ModuleEntryPoint)
>    b     .Lreloc_loop
>  .Lreloc_done:
>  
> -  // Do early platform specific actions
> -  bl    ASM_PFX(ArmPlatformPeiBootAction)
> +  bl    ASM_PFX(DiscoverDramFromDt)
>  
>    // Get ID of this CPU in Multicore system
>    bl    ASM_PFX(ArmReadMpidr)
> @@ -140,3 +139,77 @@ _GetStackBase:
>  
>  _NeverReturn:
>    b _NeverReturn
> +
> +// VOID
> +// DiscoverDramFromDt (
> +//   VOID   *DeviceTreeBaseAddress,   // passed by loader in x0
> +//   VOID   *ImageBase                // passed by FDF trampoline in x1
> +//   );
> +ASM_PFX(DiscoverDramFromDt):
> +  //
> +  // If we are booting from RAM using the Linux kernel boot protocol, x0 will
> +  // point to the DTB image in memory. Otherwise, use the default value defined
> +  // by the platform.
> +  //
> +  cbnz  x0, 0f
> +  ldr   x0, PcdGet64 (PcdDeviceTreeInitialBaseAddress)
> +
> +0:mov   x29, x30            // preserve LR
> +  mov   x28, x0             // preserve DTB pointer
> +  mov   x27, x1             // preserve base of image pointer
> +
> +  //
> +  // The base of the runtime image has been preserved in x1. Check whether
> +  // the expected magic number can be found in the header.
> +  //
> +  ldr   w8, .LArm64LinuxMagic
> +  ldr   w9, [x1, #0x38]
> +  cmp   w8, w9
> +  bne   .Lout
> +
> +  //
> +  //
> +  // OK, so far so good. We have confirmed that we likely have a DTB and are
> +  // booting via the arm64 Linux boot protocol. Update the base-of-image PCD
> +  // to the actual relocated value, and add the shift of PcdFdBaseAddress to
> +  // PcdFvBaseAddress as well
> +  //
> +  adr   x8, PcdGet64 (PcdFdBaseAddress)
> +  adr   x9, PcdGet64 (PcdFvBaseAddress)
> +  ldr   x6, [x8]
> +  ldr   x7, [x9]
> +  sub   x7, x7, x6
> +  add   x7, x7, x1
> +  str   x1, [x8]
> +  str   x7, [x9]
> +
> +  //
> +  // Discover the memory size and offset from the DTB, and record in the
> +  // respective PCDs. This will also return false if a corrupt DTB is
> +  // encountered. Since we are calling a C function, use the window at the
> +  // beginning of the FD image as a temp stack.
> +  //
> +  adr   x1, PcdGet64 (PcdSystemMemoryBase)
> +  adr   x2, PcdGet64 (PcdSystemMemorySize)
> +  mov   sp, x7
> +  bl    FindMemnode
> +  cbz   x0, .Lout
> +
> +  //
> +  // Copy the DTB to the slack space right after the 64 byte arm64/Linux style
> +  // image header at the base of this image (defined in the FDF), and record the
> +  // pointer in PcdDeviceTreeInitialBaseAddress.
> +  //
> +  adr   x8, PcdGet64 (PcdDeviceTreeInitialBaseAddress)
> +  add   x27, x27, #0x40
> +  str   x27, [x8]
> +
> +  mov   x0, x27
> +  mov   x1, x28
> +  bl    CopyFdt
> +
> +.Lout:
> +  ret    x29
> +
> +.LArm64LinuxMagic:
> +  .byte   0x41, 0x52, 0x4d, 0x64
> diff --git a/ArmVirtPkg/PrePi/Arm/ModuleEntryPoint.S b/ArmVirtPkg/PrePi/Arm/ModuleEntryPoint.S
> index eebf660acdb2..a918c191432e 100644
> --- a/ArmVirtPkg/PrePi/Arm/ModuleEntryPoint.S
> +++ b/ArmVirtPkg/PrePi/Arm/ModuleEntryPoint.S
> @@ -148,3 +148,74 @@ _GetStackBase:
>  
>  _NeverReturn:
>    b _NeverReturn
> +
> +ASM_PFX(ArmPlatformPeiBootAction):
> +  //
> +  // If we are booting from RAM using the Linux kernel boot protocol, r0 will
> +  // point to the DTB image in memory. Otherwise, use the default value defined
> +  // by the platform.
> +  //
> +  teq   r0, #0
> +  bne   0f
> +  LDRL  (r0, PcdGet64 (PcdDeviceTreeInitialBaseAddress))
> +
> +0:mov   r11, r14            // preserve LR
> +  mov   r10, r0             // preserve DTB pointer
> +  mov   r9, r1              // preserve base of image pointer
> +
> +  //
> +  // The base of the runtime image has been preserved in r1. Check whether
> +  // the expected magic number can be found in the header.
> +  //
> +  ldr   r8, .LArm32LinuxMagic
> +  ldr   r7, [r1, #0x24]
> +  cmp   r7, r8
> +  bne   .Lout
> +
> +  //
> +  //
> +  // OK, so far so good. We have confirmed that we likely have a DTB and are
> +  // booting via the ARM Linux boot protocol. Update the base-of-image PCD
> +  // to the actual relocated value, and add the shift of PcdFdBaseAddress to
> +  // PcdFvBaseAddress as well
> +  //
> +  ADRL  (r8, PcdGet64 (PcdFdBaseAddress))
> +  ADRL  (r7, PcdGet64 (PcdFvBaseAddress))
> +  ldr   r6, [r8]
> +  ldr   r5, [r7]
> +  sub   r5, r5, r6
> +  add   r5, r5, r1
> +  str   r1, [r8]
> +  str   r5, [r7]
> +
> +  //
> +  // Discover the memory size and offset from the DTB, and record in the
> +  // respective PCDs. This will also return false if a corrupt DTB is
> +  // encountered. Since we are calling a C function, use the window at the
> +  // beginning of the FD image as a temp stack.
> +  //
> +  ADRL  (r1, PcdGet64 (PcdSystemMemoryBase))
> +  ADRL  (r2, PcdGet64 (PcdSystemMemorySize))
> +  mov   sp, r5
> +  bl    FindMemnode
> +  teq   r0, #0
> +  beq   .Lout
> +
> +  //
> +  // Copy the DTB to the slack space right after the 64 byte arm64/Linux style
> +  // image header at the base of this image (defined in the FDF), and record the
> +  // pointer in PcdDeviceTreeInitialBaseAddress.
> +  //
> +  ADRL  (r8, PcdGet64 (PcdDeviceTreeInitialBaseAddress))
> +  add   r9, r9, #0x40
> +  str   r9, [r8]
> +
> +  mov   r0, r9
> +  mov   r1, r10
> +  bl    CopyFdt
> +
> +.Lout:
> +  bx    r11
> +
> +.LArm32LinuxMagic:
> +  .byte   0x18, 0x28, 0x6f, 0x01
> diff --git a/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf b/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf
> index 1d79b1360c22..e816e9583da8 100755
> --- a/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf
> +++ b/ArmVirtPkg/PrePi/ArmVirtPrePiUniCoreRelocatable.inf
> @@ -23,6 +23,7 @@ [Defines]
>  
>  [Sources]
>    PrePi.c
> +  FdtParser.c
>  
>  [Sources.AArch64]
>    AArch64/ArchPrePi.c
> @@ -44,6 +45,7 @@ [Packages]
>  [LibraryClasses]
>    BaseLib
>    DebugLib
> +  FdtLib
>    ArmLib
>    IoLib
>    TimerLib
> diff --git a/ArmVirtPkg/PrePi/FdtParser.c b/ArmVirtPkg/PrePi/FdtParser.c
> new file mode 100644
> index 000000000000..afdc81a8839d
> --- /dev/null
> +++ b/ArmVirtPkg/PrePi/FdtParser.c
> @@ -0,0 +1,90 @@
> +/*
> + * Copyright (c) 2015, Linaro Ltd. 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 <Uefi.h>
> +#include <Include/libfdt.h>
> +
> +BOOLEAN
> +FindMemnode (
> +  IN  VOID    *DeviceTreeBlob,
> +  OUT UINT64  *SystemMemoryBase,
> +  OUT UINT64  *SystemMemorySize
> +  )
> +{
> +  INT32         MemoryNode;
> +  INT32         AddressCells;
> +  INT32         SizeCells;
> +  INT32         Length;
> +  CONST INT32   *Prop;
> +
> +  if (fdt_check_header (DeviceTreeBlob) != 0) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Look for a node called "memory" at the lowest level of the tree
> +  //
> +  MemoryNode = fdt_path_offset (DeviceTreeBlob, "/memory");
> +  if (MemoryNode <= 0) {
> +    return FALSE;
> +  }
> +
> +  //
> +  // Retrieve the #address-cells and #size-cells properties
> +  // from the root node, or use the default if not provided.
> +  //
> +  AddressCells = 1;
> +  SizeCells = 1;
> +
> +  Prop = fdt_getprop (DeviceTreeBlob, 0, "#address-cells", &Length);
> +  if (Length == 4) {
> +    AddressCells = fdt32_to_cpu (*Prop);
> +  }
> +
> +  Prop = fdt_getprop (DeviceTreeBlob, 0, "#size-cells", &Length);
> +  if (Length == 4) {
> +    SizeCells = fdt32_to_cpu (*Prop);
> +  }
> +
> +  //
> +  // Now find the 'reg' property of the /memory node, and read the first
> +  // range listed.
> +  //
> +  Prop = fdt_getprop (DeviceTreeBlob, MemoryNode, "reg", &Length);
> +
> +  if (Length < (AddressCells + SizeCells) * sizeof (INT32)) {
> +    return FALSE;
> +  }
> +
> +  *SystemMemoryBase = fdt32_to_cpu (Prop[0]);
> +  if (AddressCells > 1) {
> +    *SystemMemoryBase = (*SystemMemoryBase << 32) | fdt32_to_cpu (Prop[1]);
> +  }
> +  Prop += AddressCells;
> +
> +  *SystemMemorySize = fdt32_to_cpu (Prop[0]);
> +  if (SizeCells > 1) {
> +    *SystemMemorySize = (*SystemMemorySize << 32) | fdt32_to_cpu (Prop[1]);
> +  }
> +
> +  return TRUE;
> +}
> +
> +VOID
> +CopyFdt (
> +  IN    VOID    *FdtDest,
> +  IN    VOID    *FdtSource
> +  )
> +{
> +  fdt_pack(FdtSource);
> +  CopyMem (FdtDest, FdtSource, fdt_totalsize (FdtSource));
> +}
> 

OK, this is where my curiosity about PrePi starts to wane :) /me checks
the diffstat.

Acked-by: Laszlo Ersek <lersek@redhat.com>