From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c0b::242; helo=mail-it0-x242.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it0-x242.google.com (mail-it0-x242.google.com [IPv6:2607:f8b0:4001:c0b::242]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 9BD44210BFF66 for ; Thu, 26 Jul 2018 00:39:38 -0700 (PDT) Received: by mail-it0-x242.google.com with SMTP id w16-v6so1521721ita.0 for ; Thu, 26 Jul 2018 00:39:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=HnBIHzfWLS/LaqVinNOJxeKLYzGj8Wh5wxc+DGqzFF4=; b=acGmKC+BdPUAGhNglWS8ya7urqgxdmELe60JJEakdQIYWLROmNIdVCvJwwo8FZR/w9 6x7uwoCGu1fUcHIlaH6Uu46M7QhTTFtGJfMf+2INsA7JExqo49OA1AfqW2OAW9HUim9E qF/9P2QzxJFjMXNf6Cdtap+UPerqC6qC7UoPg= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=HnBIHzfWLS/LaqVinNOJxeKLYzGj8Wh5wxc+DGqzFF4=; b=QZtfrhk2kCvXihtTptB9Ryf3a7I39gL4ZRBegDXWj8yftKoreUzSFudD7YYq3zOewS aY2RuMTyibfCYaHBWl7ko6Px/gat5eUPhFiJ0REDpAvqDR19BVbBHzDxeMz/B1gxbEVs Ug6BSqTAPOV0Lb2kCwkZhuwmz0u7nwOjERhYAYQDbxe4kWgphPOznFac4d4yhWzRNCMn dG8K2KH+S6jV4F5xd9CCvbbh9sp8xlalU8Qzsf6Bt+4qAKCyKxfzUQ3ewgfxllexjRUE c3H1FKyPJ9+tGfARHS7dx1kDMv+N0G+SPUuM4fL/yl4742tiEvg/yfCVuRldSgsU57hb k40A== X-Gm-Message-State: AOUpUlHsYyr/jW43t16KgPfRRdEpmqczL1R5S6RgSq3x9EsUFfWrMkPm SV4Qs68acSEOg7VqwybwmIwOMKbHpTB0krLnPwxhYpBY X-Google-Smtp-Source: AAOMgpcA4T6bI2AOEvHEqI1r5sFqdguYTwPF5GJa67efsL5C1uIaGDM0n0wpc9UpmdlxIJha7YouToQR04JSMkCZqk4= X-Received: by 2002:a02:35a:: with SMTP id y87-v6mr820166jad.2.1532590777729; Thu, 26 Jul 2018 00:39:37 -0700 (PDT) MIME-Version: 1.0 Received: by 2002:a6b:ac05:0:0:0:0:0 with HTTP; Thu, 26 Jul 2018 00:39:37 -0700 (PDT) In-Reply-To: <20180726073616.ut62js3w6lxsvrvf@holly.lan> References: <1532351961-17377-1-git-send-email-sumit.garg@linaro.org> <20180726073616.ut62js3w6lxsvrvf@holly.lan> From: Ard Biesheuvel Date: Thu, 26 Jul 2018 09:39:37 +0200 Message-ID: To: Daniel Thompson Cc: Sumit Garg , "edk2-devel@lists.01.org" , Patch Tracking , Leif Lindholm Subject: Re: [PATCH edk2-platforms v2 1/1] Silicon/SynQuacer: add optional OP-TEE DT node X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 26 Jul 2018 07:39:38 -0000 Content-Type: text/plain; charset="UTF-8" On 26 July 2018 at 09:36, Daniel Thompson wrote: > On Wed, Jul 25, 2018 at 12:04:58PM +0200, Ard Biesheuvel wrote: >> On 23 July 2018 at 15:19, Sumit Garg 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 >> > Cc: Leif Lindholm >> > Contributed-under: TianoCore Contribution Agreement 1.1 >> > Signed-off-by: Sumit Garg >> > --- >> > >> >> 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. > > For Trusted Firmware I asked Sumit to look for the OP-TEE memory carve > out rather than looking at the switches. This was based on concerns > about version skew (new C-A53 firmware, old SCP firmware[1]), in particular > if TF-A jumps to an OP-TEE that isn't actually loaded the system will > fail in a not very transparent way (especially if the user hasn't found > the debug UART behind the back panel yet). > > What is the consequence of passing a DT with OP-TEE present if one is > not actually present? Do we at least get as far as bringing up the > framebuffer before things explode? > Is there any way we can let OP-TEE supply a DT overlay? > >> >> > 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 >> > #include >> > #include >> > +#include >> > #include >> > >> > // 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 >> >