Re: [PATCH edk2-platforms v2 1/1] Silicon/SynQuacer: add optional OP-TEE DT node

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 23 July 2018 at 15:19, Sumit Garg <sumit.garg@linaro.org> wrote:
> OP-TEE is optional on Developerbox controlled via SCP firmware. To check
> if we need to delete OP-TEE DT node, we use DRAM1 region info as SCP
> firmware conditionally carves out Secure memory from DRAM1 region.
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Sumit Garg <sumit.garg@linaro.org>
> ---
>

As discussed on IRC, i am not a fan of inferring the presence of
OP-TEE from the base/size values of the first DRAM region.

Please refer to the existing PCIe code how to read a GPIO in PEI and
set a dynamic PCD accordingly, so you can use its value in
PlatformDxe.

> Changes since v1:
>   - Add support for optional OP-TEE DT node addition
>
>  Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf |  3 ++
>  Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c   | 33 ++++++++++++++++++++
>  Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi                               |  7 +++++
>  3 files changed, 43 insertions(+)
>
> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
> index 548d62fd5c0a..46cd3f85e509 100644
> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.inf
> @@ -35,6 +35,9 @@ [LibraryClasses]
>    FdtLib
>    MemoryAllocationLib
>
> +[FixedPcd]
> +  gSynQuacerTokenSpaceGuid.PcdDramInfoBase
> +
>  [Pcd]
>    gSynQuacerTokenSpaceGuid.PcdPcieEnableMask
>    gSynQuacerTokenSpaceGuid.PcdPlatformSettings
> diff --git a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
> index 897d06743708..da1209b4a613 100644
> --- a/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
> +++ b/Silicon/Socionext/SynQuacer/Library/SynQuacerDtbLoaderLib/SynQuacerDtbLoaderLib.c
> @@ -19,10 +19,13 @@
>  #include <Library/DebugLib.h>
>  #include <Library/DxeServicesLib.h>
>  #include <Library/MemoryAllocationLib.h>
> +#include <Platform/DramInfo.h>
>  #include <Platform/VarStore.h>
>
>  // add enough space for three instances of 'status = "disabled"'
>  #define DTB_PADDING               64
> +// base address for OP-TEE used to determine its presence
> +#define OPTEE_BASE_ADDR           0xFC000000
>
>  STATIC
>  VOID
> @@ -47,6 +50,29 @@ DisableDtNode (
>    }
>  }
>
> +STATIC
> +VOID
> +DeleteDtNode (
> +  IN  VOID                        *Dtb,
> +  IN  CONST CHAR8                 *NodePath
> +  )
> +{
> +  INT32                           Node;
> +  INT32                           Rc;
> +
> +  Node = fdt_path_offset (Dtb, NodePath);
> +  if (Node < 0) {
> +    DEBUG ((DEBUG_ERROR, "%a: failed to locate DT path '%a': %a\n",
> +      __FUNCTION__, NodePath, fdt_strerror (Node)));
> +    return;
> +  }
> +  Rc = fdt_del_node (Dtb, Node);
> +  if (Rc < 0) {
> +    DEBUG ((DEBUG_ERROR, "%a: failed to delete node on '%a': %a\n",
> +      __FUNCTION__, NodePath, fdt_strerror (Rc)));
> +  }
> +}
> +
>  /**
>    Return a pool allocated copy of the DTB image that is appropriate for
>    booting the current platform via DT.
> @@ -73,6 +99,7 @@ DtPlatformLoadDtb (
>    UINTN                             CopyDtbSize;
>    INT32                             Rc;
>    UINT64                            SettingsVal;
> +  DRAM_INFO                         *DramInfo;
>    SYNQUACER_PLATFORM_VARSTORE_DATA  *Settings;
>
>    Status = GetSectionFromAnyFv (&gDtPlatformDefaultDtbFileGuid,
> @@ -107,6 +134,12 @@ DtPlatformLoadDtb (
>      DisableDtNode (CopyDtb, "/sdhci@52300000");
>    }
>
> +  DramInfo = (VOID *)(UINTN)FixedPcdGet64 (PcdDramInfoBase);
> +
> +  if ((DramInfo->Entry[0].Base + DramInfo->Entry[0].Size) > OPTEE_BASE_ADDR) {
> +    DeleteDtNode (CopyDtb, "/firmware/optee");
> +  }
> +
>    *Dtb = CopyDtb;
>    *DtbSize = CopyDtbSize;
>
> diff --git a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
> index 37d642e4b237..d109a5742793 100644
> --- a/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
> +++ b/Silicon/Socionext/SynQuacer/DeviceTree/SynQuacer.dtsi
> @@ -574,6 +574,13 @@
>          #address-cells = <1>;
>          #size-cells = <0>;
>      };
> +
> +    firmware {
> +        optee {
> +            compatible = "linaro,optee-tz";
> +            method = "smc";
> +        };
> +    };
>  };
>
>  #include "SynQuacerCaches.dtsi"
> --
> 2.7.4
>