Re: [Xen-devel] [PATCH v2 07/29] ArmVirtualizationPkg: use a HOB to store device tree blob

[Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>]

On 28 January 2015 at 15:04, Olivier Martin <olivier.martin@xxxxxxx> wrote:
> I do not have a strong opinion on this patch.
> It would be better to keep the dynamic PCD support in this patch. But I am
> aware it is not possible with PrePi (I had the issue a couple of weeks ago).
> Dynamic Pcds are actually supported when you use the PeiCore with PcdPeim.
> But the PeiCore only make sense at the moment on platforms that have DRAM
> initialized by the UEFI firmware.
> I would like to extend the PI spec to also be able to use PeiCore in the
> case where the DRAM is already initialized at the time of the UEFI firmware.
> That would mean we could use the PcdPeim and Dynamic Pcd. But it will take
> time before we have support for it.
> And I do not want to gate the patch set for this reason.
>

OK.

> I am ok to accept it if no one reject it.
>
>
> Anyway, this patch breaks the ARM Toolchain build:
>
> "armlink" --partial -o
> /tianocore/Build/ArmVirtualizationQemu-ARM/DEBUG_RVCTLINUX/ARM/ArmPlatformPk
> g/PrePeiCore/PrePeiCoreUniCore/OUTPUT/ArmPlatformPrePeiCore.lib --via
> /tianocore/Build/ArmVirtualizationQemu-ARM/DEBUG_RVCTLINUX/ARM/ArmPlatformPk
> g/PrePeiCore/PrePeiCoreUniCore/OUTPUT/object_files.lst
> "armlink"  --ro-base 0 --no_scanlib --reloc --no_exceptions --datacompressor
> off --strict --symbols --diag_style=ide --entry _ModuleEntryPoint --map
> --list
> /tianocore/Build/ArmVirtualizationQemu-ARM/DEBUG_RVCTLINUX/ARM/ArmPlatformPk
> g/PrePeiCore/PrePeiCoreUniCore/DEBUG/ArmPlatformPrePeiCore.map -o
> /tianocore/Build/ArmVirtualizationQemu-ARM/DEBUG_RVCTLINUX/ARM/ArmPlatformPk
> g/PrePeiCore/PrePeiCoreUniCore/DEBUG/ArmPlatformPrePeiCore.dll  --via
> /tianocore/Build/ArmVirtualizationQemu-ARM/DEBUG_RVCTLINUX/ARM/ArmPlatformPk
> g/PrePeiCore/PrePeiCoreUniCore/OUTPUT/static_library_files.lst
> armlink : error L6218:  Undefined symbol AllocatePages (referred from
> ArmVirtualizationPlatformLib.lib).
> armlink : Not enough information to list image symbols.
> armlink : Finished: 1 information, 0 warning and 1 error messages.
>

Probably just a missing MemoryAllocationLib dependency in
ArmVirtualizationPlatformLib.inf


>
>> -----Original Message-----
>> From: Ard Biesheuvel [mailto:ard.biesheuvel@xxxxxxxxxx]
>> Sent: 26 January 2015 19:03
>> To: edk2-devel@xxxxxxxxxxxxxxxxxxxxx; lersek@xxxxxxxxxx; Olivier
>> Martin; roy.franz@xxxxxxxxxx; leif.lindholm@xxxxxxxxxx;
>> stefano.stabellini@xxxxxxxxxxxxx; Ian.Campbell@xxxxxxxxxx;
>> anthony.perard@xxxxxxxxxx; christoffer.dall@xxxxxxxxxx; xen-
>> devel@xxxxxxxxxxxxx; ilias.biris@xxxxxxxxxx
>> Cc: Ard Biesheuvel
>> Subject: [PATCH v2 07/29] ArmVirtualizationPkg: use a HOB to store
>> device tree blob
>>
>> Instead of using a dynamic PCD, store the device tree address in a HOB
>> so that we can also run under a configuration that does not support
>> dynamic PCDs.
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
>> ---
>>  ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationPkg.dec
>> |  2 --
>>  ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.dsc
>> |  3 ---
>>
>> ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationPlatformLi
>> b/ArmVirtualizationPlatformLib.inf |  2 --
>>
>> ArmPlatformPkg/ArmVirtualizationPkg/Library/PlatformPeiLib/PlatformPeiL
>> ib.c                               | 11 ++++++++---
>>
>> ArmPlatformPkg/ArmVirtualizationPkg/Library/PlatformPeiLib/PlatformPeiL
>> ib.inf                             |  4 +---
>>  ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.c
>> | 10 ++++++++--
>>  ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.inf
>> |  3 ++-
>>  EmbeddedPkg/EmbeddedPkg.dec
>> |  2 ++
>>  EmbeddedPkg/Include/Guid/FdtHob.h
>> | 26 ++++++++++++++++++++++++++
>>  9 files changed, 47 insertions(+), 16 deletions(-)
>>
>> diff --git
>> a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationPkg.dec
>> b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationPkg.dec
>> index d83117fc6abe..868488906643 100644
>> --- a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationPkg.dec
>> +++ b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationPkg.dec
>> @@ -44,8 +44,6 @@
>>
>> gArmVirtualizationTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress|0x0|UI
>> NT64|0x00000001
>>
>>  [PcdsDynamic, PcdsFixedAtBuild]
>> -
>> gArmVirtualizationTokenSpaceGuid.PcdDeviceTreeBaseAddress|0x0|UINT64|0x
>> 00000002
>> -
>>    #
>>    # ARM PSCI function invocations can be done either through
>> hypervisor
>>    # calls (HVC) or secure monitor calls (SMC).
>> diff --git
>> a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.dsc
>> b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.dsc
>> index dff4e2507058..4f8eb632143c 100644
>> --- a/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.dsc
>> +++ b/ArmPlatformPkg/ArmVirtualizationPkg/ArmVirtualizationQemu.dsc
>> @@ -160,9 +160,6 @@
>>    # System Memory Size -- 1 MB initially, actual size will be fetched
>> from DT
>>    gArmTokenSpaceGuid.PcdSystemMemorySize|0x00100000
>>
>> -  # location of the device tree blob passed by QEMU
>> -  gArmVirtualizationTokenSpaceGuid.PcdDeviceTreeBaseAddress|0x0
>> -
>>    gArmTokenSpaceGuid.PcdArmArchTimerSecIntrNum|0x0
>>    gArmTokenSpaceGuid.PcdArmArchTimerIntrNum|0x0
>>    gArmTokenSpaceGuid.PcdArmArchTimerVirtIntrNum|0x0
>> diff --git
>> a/ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationPlatform
>> Lib/ArmVirtualizationPlatformLib.inf
>> b/ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationPlatform
>> Lib/ArmVirtualizationPlatformLib.inf
>> index 43b3c6ca1bef..c57002f3e9da 100644
>> ---
>> a/ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationPlatform
>> Lib/ArmVirtualizationPlatformLib.inf
>> +++
>> b/ArmPlatformPkg/ArmVirtualizationPkg/Library/ArmVirtualizationPlatform
>> Lib/ArmVirtualizationPlatformLib.inf
>> @@ -33,8 +33,6 @@
>>    ArmLib
>>    PrintLib
>>    FdtLib
>> -  SerialPortLib
>> -  HobLib
>>
>>  [Sources.common]
>>    Virt.c
>> diff --git
>> a/ArmPlatformPkg/ArmVirtualizationPkg/Library/PlatformPeiLib/PlatformPe
>> iLib.c
>> b/ArmPlatformPkg/ArmVirtualizationPkg/Library/PlatformPeiLib/PlatformPe
>> iLib.c
>> index 58bc2b828dcd..c500d5964b25 100644
>> ---
>> a/ArmPlatformPkg/ArmVirtualizationPkg/Library/PlatformPeiLib/PlatformPe
>> iLib.c
>> +++
>> b/ArmPlatformPkg/ArmVirtualizationPkg/Library/PlatformPeiLib/PlatformPe
>> iLib.c
>> @@ -22,6 +22,7 @@
>>  #include <libfdt.h>
>>
>>  #include <Guid/EarlyPL011BaseAddress.h>
>> +#include <Guid/FdtHob.h>
>>
>>  EFI_STATUS
>>  EFIAPI
>> @@ -32,6 +33,7 @@ PlatformPeim (
>>    VOID               *Base;
>>    VOID               *NewBase;
>>    UINTN              FdtSize;
>> +  UINT64             *FdtHobData;
>>    UINT64             *UartHobData;
>>    INT32              Node, Prev;
>>    CONST CHAR8        *Compatible;
>> @@ -41,15 +43,18 @@ PlatformPeim (
>>    UINT64             UartBase;
>>
>>
>> -  Base = (VOID*)(UINTN)FixedPcdGet64
>> (PcdDeviceTreeInitialBaseAddress);
>> +  Base = (VOID*)(UINTN)PcdGet64 (PcdDeviceTreeInitialBaseAddress);
>> +  ASSERT (Base != NULL);
>>    ASSERT (fdt_check_header (Base) == 0);
>>
>>    FdtSize = fdt_totalsize (Base);
>>    NewBase = AllocatePages (EFI_SIZE_TO_PAGES (FdtSize));
>>    ASSERT (NewBase != NULL);
>> -
>>    CopyMem (NewBase, Base, FdtSize);
>> -  PcdSet64 (PcdDeviceTreeBaseAddress, (UINT64)(UINTN)NewBase);
>> +
>> +  FdtHobData = BuildGuidHob (&gFdtHobGuid, sizeof *FdtHobData);
>> +  ASSERT (FdtHobData != NULL);
>> +  *FdtHobData = (UINTN)NewBase;
>>
>>    UartHobData = BuildGuidHob (&gEarlyPL011BaseAddressGuid, sizeof
>> *UartHobData);
>>    ASSERT (UartHobData != NULL);
>> diff --git
>> a/ArmPlatformPkg/ArmVirtualizationPkg/Library/PlatformPeiLib/PlatformPe
>> iLib.inf
>> b/ArmPlatformPkg/ArmVirtualizationPkg/Library/PlatformPeiLib/PlatformPe
>> iLib.inf
>> index a376fbd1f345..96019e4009ff 100644
>> ---
>> a/ArmPlatformPkg/ArmVirtualizationPkg/Library/PlatformPeiLib/PlatformPe
>> iLib.inf
>> +++
>> b/ArmPlatformPkg/ArmVirtualizationPkg/Library/PlatformPeiLib/PlatformPe
>> iLib.inf
>> @@ -41,11 +41,9 @@
>>    gArmTokenSpaceGuid.PcdFvSize
>>    gArmVirtualizationTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress
>>
>> -[Pcd]
>> -  gArmVirtualizationTokenSpaceGuid.PcdDeviceTreeBaseAddress
>> -
>>  [Guids]
>>    gEarlyPL011BaseAddressGuid
>> +  gFdtHobGuid
>>
>>  [Depex]
>>    gEfiPeiMemoryDiscoveredPpiGuid
>> diff --git
>> a/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.c
>> b/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.c
>> index 31164905d34e..34fac40fa803 100644
>> --- a/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.c
>> +++ b/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.c
>> @@ -24,10 +24,12 @@
>>  #include <Library/DevicePathLib.h>
>>  #include <Library/PcdLib.h>
>>  #include <Library/DxeServicesLib.h>
>> +#include <Library/HobLib.h>
>>  #include <libfdt.h>
>>
>>  #include <Guid/Fdt.h>
>>  #include <Guid/VirtioMmioTransport.h>
>> +#include <Guid/FdtHob.h>
>>
>>  #pragma pack (1)
>>  typedef struct {
>> @@ -105,6 +107,7 @@ InitializeVirtFdtDxe (
>>    IN EFI_SYSTEM_TABLE     *SystemTable
>>    )
>>  {
>> +  VOID                           *Hob;
>>    VOID                           *DeviceTreeBase;
>>    INT32                          Node, Prev;
>>    INT32                          RtcNode;
>> @@ -125,8 +128,11 @@ InitializeVirtFdtDxe (
>>    UINT64                         FwCfgDataAddress;
>>    UINT64                         FwCfgDataSize;
>>
>> -  DeviceTreeBase = (VOID *)(UINTN)PcdGet64 (PcdDeviceTreeBaseAddress);
>> -  ASSERT (DeviceTreeBase != NULL);
>> +  Hob = GetFirstGuidHob(&gFdtHobGuid);
>> +  if (Hob == NULL || GET_GUID_HOB_DATA_SIZE (Hob) != sizeof (UINT64))
>> {
>> +    return EFI_NOT_FOUND;
>> +  }
>> +  DeviceTreeBase = (VOID *)(UINTN)*(UINT64 *)GET_GUID_HOB_DATA (Hob);
>>
>>    if (fdt_check_header (DeviceTreeBase) != 0) {
>>      DEBUG ((EFI_D_ERROR, "%a: No DTB found @ 0x%p\n", __FUNCTION__,
>> DeviceTreeBase));
>> diff --git
>> a/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.inf
>> b/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.inf
>> index 514ce2fdf658..1392c7c3fa45 100644
>> --- a/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.inf
>> +++ b/ArmPlatformPkg/ArmVirtualizationPkg/VirtFdtDxe/VirtFdtDxe.inf
>> @@ -40,13 +40,14 @@
>>    DxeServicesLib
>>    FdtLib
>>    VirtioMmioDeviceLib
>> +  HobLib
>>
>>  [Guids]
>>    gFdtTableGuid
>>    gVirtioMmioTransportGuid
>> +  gFdtHobGuid
>>
>>  [Pcd]
>> -  gArmVirtualizationTokenSpaceGuid.PcdDeviceTreeBaseAddress
>>    gArmVirtualizationTokenSpaceGuid.PcdArmPsciMethod
>>    gArmVirtualizationTokenSpaceGuid.PcdFwCfgSelectorAddress
>>    gArmVirtualizationTokenSpaceGuid.PcdFwCfgDataAddress
>> diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
>> index 600d0e54c4b3..2f261ece9212 100644
>> --- a/EmbeddedPkg/EmbeddedPkg.dec
>> +++ b/EmbeddedPkg/EmbeddedPkg.dec
>> @@ -52,6 +52,8 @@
>>    ## FDT Configuration Table
>>    # Include/Guid/Fdt.h
>>    gFdtTableGuid = { 0xb1b621d5, 0xf19c, 0x41a5, { 0x83, 0x0b, 0xd9,
>> 0x15, 0x2c, 0x69, 0xaa, 0xe0 } }
>> +  # Include/Guid/FdtHob.h
>> +  gFdtHobGuid   = { 0x16958446, 0x19B7, 0x480B, { 0xB0, 0x47, 0x74,
>> 0x85, 0xAD, 0x3F, 0x71, 0x6D } }
>>
>>  [Protocols.common]
>>    gHardwareInterruptProtocolGuid =  { 0x2890B3EA, 0x053D, 0x1643, {
>> 0xAD, 0x0C, 0xD6, 0x48, 0x08, 0xDA, 0x3F, 0xF1 } }
>> diff --git a/EmbeddedPkg/Include/Guid/FdtHob.h
>> b/EmbeddedPkg/Include/Guid/FdtHob.h
>> new file mode 100644
>> index 000000000000..287729e0c350
>> --- /dev/null
>> +++ b/EmbeddedPkg/Include/Guid/FdtHob.h
>> @@ -0,0 +1,26 @@
>> +/** @file
>> +  GUID for the HOB that contains the copy of the flattened device tree
>> blob
>> +
>> +  Copyright (C) 2014, Linaro Ltd.
>> +
>> +  This program and the accompanying materials are licensed and made
>> available
>> +  under the terms and conditions of the BSD License that 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.
>> +
>> +**/
>> +
>> +#ifndef __FDT_HOB_H__
>> +#define __FDT_HOB_H__
>> +
>> +#define FDT_HOB_GUID { \
>> +          0x16958446, 0x19B7, 0x480B, \
>> +          { 0xB0, 0x47, 0x74, 0x85, 0xAD, 0x3F, 0x71, 0x6D } \
>> +        }
>> +
>> +extern EFI_GUID gFdtHobGuid;
>> +
>> +#endif
>> --
>> 1.8.3.2
>>
>
>
>
>

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel