Re: [Linaro-uefi] [RFC PATCH 1/8] ArmVirtualizationPkg: move early UART discovery to PlatformPeim

On 12/08/14 19:22, Ard Biesheuvel wrote:

This is partially motivated by the desire to use PrePi in a virt environment, and in that configuration, ArmPlatformInitializeMemory()

Typo in commit message, should say ArmPlatformInitializeSystemMemory().

is never called. But actually, this is a more suitable place anyway.

Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel ard.biesheuvel@linaro.org

---

.../Library/ArmVirtualizationPlatformLib/Virt.c | 46 +-------------------- .../Library/PlatformPeiLib/PlatformPeiLib.c | 48 ++++++++++++++++++++++ 2 files changed, 50 insertions(+), 44 deletions(-)

diff --git a/ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationPlatformLib/Virt.c b/ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationPlatformLib/Virt.c index aa4ced4582e8..463a6d6d3b9b 100644 --- a/ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationPlatformLib/Virt.c +++ b/ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationPlatformLib/Virt.c @@ -24,9 +24,6 @@ #include <Pi/PiBootMode.h> #include <Uefi/UefiBaseType.h> #include <Uefi/UefiMultiPhase.h> -#include <Pi/PiHob.h> -#include <Library/HobLib.h> -#include <Guid/EarlyPL011BaseAddress.h> /** Return the current Boot Mode @@ -77,25 +74,13 @@ ArmPlatformInitializeSystemMemory ( INT32 Node, Prev; UINT64 NewBase; UINT64 NewSize;

• BOOLEAN HaveMemory, HaveUART;
• UINT64 *HobData; CONST CHAR8 *Type;
• CONST CHAR8 *Compatible;
• CONST CHAR8 *CompItem; INT32 Len; CONST UINT64 *RegProp;
• UINT64 UartBase;

NewBase = 0; NewSize = 0;

• HaveMemory = FALSE;
• HaveUART = FALSE;
• HobData = BuildGuidHob (&gEarlyPL011BaseAddressGuid, sizeof *HobData);
• ASSERT (HobData != NULL);
• *HobData = 0;
• DeviceTreeBase = (VOID *)(UINTN)FixedPcdGet64 (PcdDeviceTreeInitialBaseAddress); ASSERT (DeviceTreeBase != NULL);

@@ -107,7 +92,7 @@ ArmPlatformInitializeSystemMemory ( // // Look for a memory node //

• for (Prev = 0; !(HaveMemory && HaveUART); Prev = Node) {

• for (Prev = 0;; Prev = Node) { Node = fdt_next_node (DeviceTreeBase, Prev, NULL); if (Node < 0) { break;

@@ -140,34 +125,7 @@ ArmPlatformInitializeSystemMemory ( DEBUG ((EFI_D_ERROR, "%a: Failed to parse FDT memory node\n", __FUNCTION__)); }

•  HaveMemory = TRUE;
  

•  continue;
  

• }
• //
• // Check for UART node
• //
• Compatible = fdt_getprop (DeviceTreeBase, Node, "compatible", &Len);
• //
• // Iterate over the NULL-separated items in the compatible string
• //
• for (CompItem = Compatible; CompItem != NULL && CompItem < Compatible + Len;

•  CompItem += 1 + AsciiStrLen (CompItem)) {
  

•  if (AsciiStrCmp (CompItem, "arm,pl011") == 0) {
  

•    RegProp = fdt_getprop (DeviceTreeBase, Node, "reg", &Len);
  

•    ASSERT (Len == 16);
  

•    UartBase = fdt64_to_cpu (ReadUnaligned64 (RegProp));
  

•    DEBUG ((EFI_D_INFO, "%a: PL011 UART @ 0x%lx\n", __FUNCTION__, UartBase));
  

•    *HobData = UartBase;
  

•    HaveUART = TRUE;
  

•    continue;
  

•  }
  

•  break;
  

  } }

diff --git a/ArmPlatformPkg/ArmVirtualizationPkg/Library/PlatformPeiLib/PlatformPeiLib.c b/ArmPlatformPkg/ArmVirtualizationPkg/Library/PlatformPeiLib/PlatformPeiLib.c index af0d6e87da9f..58bc2b828dcd 100644 --- a/ArmPlatformPkg/ArmVirtualizationPkg/Library/PlatformPeiLib/PlatformPeiLib.c +++ b/ArmPlatformPkg/ArmVirtualizationPkg/Library/PlatformPeiLib/PlatformPeiLib.c @@ -21,6 +21,8 @@ #include <Library/PcdLib.h> #include <libfdt.h> +#include <Guid/EarlyPL011BaseAddress.h>

EFI_STATUS EFIAPI PlatformPeim ( @@ -30,6 +32,14 @@ PlatformPeim ( VOID *Base; VOID *NewBase; UINTN FdtSize;

• UINT64 *UartHobData;
• INT32 Node, Prev;
• CONST CHAR8 *Compatible;
• CONST CHAR8 *CompItem;
• INT32 Len;
• CONST UINT64 *RegProp;
• UINT64 UartBase;

Base = (VOID*)(UINTN)FixedPcdGet64 (PcdDeviceTreeInitialBaseAddress); ASSERT (fdt_check_header (Base) == 0); @@ -41,6 +51,44 @@ PlatformPeim ( CopyMem (NewBase, Base, FdtSize); PcdSet64 (PcdDeviceTreeBaseAddress, (UINT64)(UINTN)NewBase);

• UartHobData = BuildGuidHob (&gEarlyPL011BaseAddressGuid, sizeof *UartHobData);
• ASSERT (UartHobData != NULL);
• *UartHobData = 0;
• //
• // Look for a UART node
• //
• for (Prev = 0;; Prev = Node) {
• Node = fdt_next_node (Base, Prev, NULL);
• if (Node < 0) {

•  break;
  

• }
• //
• // Check for UART node
• //
• Compatible = fdt_getprop (Base, Node, "compatible", &Len);
• //
• // Iterate over the NULL-separated items in the compatible string
• //
• for (CompItem = Compatible; CompItem != NULL && CompItem < Compatible + Len;

•  CompItem += 1 + AsciiStrLen (CompItem)) {
  

•  if (AsciiStrCmp (CompItem, "arm,pl011") == 0) {
  

•    RegProp = fdt_getprop (Base, Node, "reg", &Len);
  

•    ASSERT (Len == 16);
  

•    UartBase = fdt64_to_cpu (ReadUnaligned64 (RegProp));
  

•    DEBUG ((EFI_D_INFO, "%a: PL011 UART @ 0x%lx\n", __FUNCTION__, UartBase));
  

•    *UartHobData = UartBase;
  

•    break;
  

•  }
  

• }
• }
• BuildFvHob (PcdGet64 (PcdFvBaseAddress), PcdGet32 (PcdFvSize));

return EFI_SUCCESS;

The discovery code that is being moved is kept intact.

The source module (ie. the module that the original code is embedded into during build) is "ArmPlatformPkg/MemoryInitPei/MemoryInitPeim.inf", which has module type PEIM.

The target module is "ArmPlatformPkg/PlatformPei/PlatformPeim.inf", which is also of type PEIM.

These modules are both part of [FV.FVMAIN_COMPACT] in "ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.fdf", and their dispatch order is unspecified (neither is listed in APRIORI PEI). So neither can depend on anything the other does, which means we're allowed to move code between them.

Reviewed-by: Laszlo Ersek lersek@redhat.com