* [PATCH edk2-platforms v2 1/1] Silicon/SynQuacer: add optional OP-TEE DT node
@ 2018-07-23 13:19 Sumit Garg
2018-07-25 10:04 ` Ard Biesheuvel
0 siblings, 1 reply; 11+ messages in thread
From: Sumit Garg @ 2018-07-23 13:19 UTC (permalink / raw)
To: edk2-devel; +Cc: patches, Sumit Garg, Ard Biesheuvel, Leif Lindholm
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>
---
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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH edk2-platforms v2 1/1] Silicon/SynQuacer: add optional OP-TEE DT node
2018-07-23 13:19 [PATCH edk2-platforms v2 1/1] Silicon/SynQuacer: add optional OP-TEE DT node Sumit Garg
@ 2018-07-25 10:04 ` Ard Biesheuvel
2018-07-26 7:36 ` Daniel Thompson
0 siblings, 1 reply; 11+ messages in thread
From: Ard Biesheuvel @ 2018-07-25 10:04 UTC (permalink / raw)
To: Sumit Garg, Daniel Thompson
Cc: edk2-devel@lists.01.org, Patch Tracking, Leif Lindholm
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
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH edk2-platforms v2 1/1] Silicon/SynQuacer: add optional OP-TEE DT node
2018-07-25 10:04 ` Ard Biesheuvel
@ 2018-07-26 7:36 ` Daniel Thompson
2018-07-26 7:39 ` Ard Biesheuvel
0 siblings, 1 reply; 11+ messages in thread
From: Daniel Thompson @ 2018-07-26 7:36 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Sumit Garg, edk2-devel@lists.01.org, Patch Tracking,
Leif Lindholm
On Wed, Jul 25, 2018 at 12:04:58PM +0200, Ard Biesheuvel wrote:
> 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.
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?
Daniel.
>
> > 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
> >
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH edk2-platforms v2 1/1] Silicon/SynQuacer: add optional OP-TEE DT node
2018-07-26 7:36 ` Daniel Thompson
@ 2018-07-26 7:39 ` Ard Biesheuvel
2018-07-26 7:50 ` Daniel Thompson
0 siblings, 1 reply; 11+ messages in thread
From: Ard Biesheuvel @ 2018-07-26 7:39 UTC (permalink / raw)
To: Daniel Thompson
Cc: Sumit Garg, edk2-devel@lists.01.org, Patch Tracking,
Leif Lindholm
On 26 July 2018 at 09:36, Daniel Thompson <daniel.thompson@linaro.org> wrote:
> On Wed, Jul 25, 2018 at 12:04:58PM +0200, Ard Biesheuvel wrote:
>> 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.
>
> 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 <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
>> >
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH edk2-platforms v2 1/1] Silicon/SynQuacer: add optional OP-TEE DT node
2018-07-26 7:39 ` Ard Biesheuvel
@ 2018-07-26 7:50 ` Daniel Thompson
2018-07-26 8:42 ` Sumit Garg
0 siblings, 1 reply; 11+ messages in thread
From: Daniel Thompson @ 2018-07-26 7:50 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: Sumit Garg, edk2-devel@lists.01.org, Patch Tracking,
Leif Lindholm
On Thu, Jul 26, 2018 at 09:39:37AM +0200, Ard Biesheuvel wrote:
> On 26 July 2018 at 09:36, Daniel Thompson <daniel.thompson@linaro.org> wrote:
> > On Wed, Jul 25, 2018 at 12:04:58PM +0200, Ard Biesheuvel wrote:
> >> 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.
> >
> > 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?
I guess it could implement a secure monitor call to provide it. In
fact I find it a rather pleasing approach. However I think it still loops
us round to pretty much the same question as before. Does TF-A "protec
" a normal world that makes an SMC to an OP-TEE that isn't there by
failing the call in a nice way?
Daniel.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH edk2-platforms v2 1/1] Silicon/SynQuacer: add optional OP-TEE DT node
2018-07-26 7:50 ` Daniel Thompson
@ 2018-07-26 8:42 ` Sumit Garg
2018-07-27 11:10 ` Daniel Thompson
2018-07-27 12:49 ` Mark Rutland
0 siblings, 2 replies; 11+ messages in thread
From: Sumit Garg @ 2018-07-26 8:42 UTC (permalink / raw)
To: Daniel Thompson; +Cc: Ard Biesheuvel, edk2-devel, Patch Tracking, Leif Lindholm
On Thu, 26 Jul 2018 at 13:20, Daniel Thompson
<daniel.thompson@linaro.org> wrote:
>
> On Thu, Jul 26, 2018 at 09:39:37AM +0200, Ard Biesheuvel wrote:
> > On 26 July 2018 at 09:36, Daniel Thompson <daniel.thompson@linaro.org> wrote:
> > > On Wed, Jul 25, 2018 at 12:04:58PM +0200, Ard Biesheuvel wrote:
> > >> 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.
> > >
> > > 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?
> > >
If we pass a DT with OP-TEE and OP-TEE not present, Linux TEE generic
driver exits gracefully giving following message:
[ 1.976021] optee: probing for conduit method from DT.
[ 1.976033] optee: api uid mismatch
> >
> > Is there any way we can let OP-TEE supply a DT overlay?
>
> I guess it could implement a secure monitor call to provide it. In
> fact I find it a rather pleasing approach. However I think it still loops
> us round to pretty much the same question as before. Does TF-A "protec
> " a normal world that makes an SMC to an OP-TEE that isn't there by
> failing the call in a nice way?
>
TF-A returns SMC call for OP-TEE as unknown (error code: -1 in "x0"
register) if OP-TEE is not present.
-Sumit
>
> Daniel.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH edk2-platforms v2 1/1] Silicon/SynQuacer: add optional OP-TEE DT node
2018-07-26 8:42 ` Sumit Garg
@ 2018-07-27 11:10 ` Daniel Thompson
2018-07-27 11:37 ` Sumit Garg
2018-07-27 12:49 ` Mark Rutland
1 sibling, 1 reply; 11+ messages in thread
From: Daniel Thompson @ 2018-07-27 11:10 UTC (permalink / raw)
To: Sumit Garg; +Cc: Ard Biesheuvel, edk2-devel, Patch Tracking, Leif Lindholm
On 26/07/18 09:42, Sumit Garg wrote:
> On Thu, 26 Jul 2018 at 13:20, Daniel Thompson
> <daniel.thompson@linaro.org> wrote:
>>
>> On Thu, Jul 26, 2018 at 09:39:37AM +0200, Ard Biesheuvel wrote:
>>> On 26 July 2018 at 09:36, Daniel Thompson <daniel.thompson@linaro.org> wrote:
>>>> On Wed, Jul 25, 2018 at 12:04:58PM +0200, Ard Biesheuvel wrote:
>>>>> 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.
>>>>
>>>> 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?
>>>>
>
> If we pass a DT with OP-TEE and OP-TEE not present, Linux TEE generic
> driver exits gracefully giving following message:
>
> [ 1.976021] optee: probing for conduit method from DT.
> [ 1.976033] optee: api uid mismatch
That certainly means we can be pretty relaxed about version skew of
normal world components (since nothing bad happens if thinks get skewed).
>>> Is there any way we can let OP-TEE supply a DT overlay?
>>
>> I guess it could implement a secure monitor call to provide it. In
>> fact I find it a rather pleasing approach. However I think it still loops
>> us round to pretty much the same question as before. Does TF-A "protec
>> " a normal world that makes an SMC to an OP-TEE that isn't there by
>> failing the call in a nice way?
>>
>
> TF-A returns SMC call for OP-TEE as unknown (error code: -1 in "x0"
> register) if OP-TEE is not present.
It is possible to experiment with getting EDK2 to detect OP-TEE using
SMC? This would be fully generic and presumably be the first step in
having an EFI OP-TEE driver.
Daniel.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH edk2-platforms v2 1/1] Silicon/SynQuacer: add optional OP-TEE DT node
2018-07-27 11:10 ` Daniel Thompson
@ 2018-07-27 11:37 ` Sumit Garg
2018-07-27 12:12 ` Ard Biesheuvel
0 siblings, 1 reply; 11+ messages in thread
From: Sumit Garg @ 2018-07-27 11:37 UTC (permalink / raw)
To: Daniel Thompson; +Cc: Ard Biesheuvel, edk2-devel, Patch Tracking, Leif Lindholm
On Fri, 27 Jul 2018 at 16:40, Daniel Thompson
<daniel.thompson@linaro.org> wrote:
>
> On 26/07/18 09:42, Sumit Garg wrote:
> > On Thu, 26 Jul 2018 at 13:20, Daniel Thompson
> > <daniel.thompson@linaro.org> wrote:
> >>
> >> On Thu, Jul 26, 2018 at 09:39:37AM +0200, Ard Biesheuvel wrote:
> >>> On 26 July 2018 at 09:36, Daniel Thompson <daniel.thompson@linaro.org> wrote:
> >>>> On Wed, Jul 25, 2018 at 12:04:58PM +0200, Ard Biesheuvel wrote:
> >>>>> 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.
> >>>>
> >>>> 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?
> >>>>
> >
> > If we pass a DT with OP-TEE and OP-TEE not present, Linux TEE generic
> > driver exits gracefully giving following message:
> >
> > [ 1.976021] optee: probing for conduit method from DT.
> > [ 1.976033] optee: api uid mismatch
>
> That certainly means we can be pretty relaxed about version skew of
> normal world components (since nothing bad happens if thinks get skewed).
>
>
> >>> Is there any way we can let OP-TEE supply a DT overlay?
> >>
> >> I guess it could implement a secure monitor call to provide it. In
> >> fact I find it a rather pleasing approach. However I think it still loops
> >> us round to pretty much the same question as before. Does TF-A "protec
> >> " a normal world that makes an SMC to an OP-TEE that isn't there by
> >> failing the call in a nice way?
> >>
> >
> > TF-A returns SMC call for OP-TEE as unknown (error code: -1 in "x0"
> > register) if OP-TEE is not present.
>
> It is possible to experiment with getting EDK2 to detect OP-TEE using
> SMC? This would be fully generic and presumably be the first step in
> having an EFI OP-TEE driver.
>
Agree. I will try to detect OP-TEE version via SMC call. If SMC
unknown is returned, then we say OP-TEE is not present and remove
corresponding DT node.
So I think this EFI OP-TEE driver makes more sense in edk2 rather than
edk2-platforms.
-Sumit
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH edk2-platforms v2 1/1] Silicon/SynQuacer: add optional OP-TEE DT node
2018-07-27 11:37 ` Sumit Garg
@ 2018-07-27 12:12 ` Ard Biesheuvel
0 siblings, 0 replies; 11+ messages in thread
From: Ard Biesheuvel @ 2018-07-27 12:12 UTC (permalink / raw)
To: Sumit Garg
Cc: Daniel Thompson, edk2-devel@lists.01.org, Patch Tracking,
Leif Lindholm
On 27 July 2018 at 13:37, Sumit Garg <sumit.garg@linaro.org> wrote:
> On Fri, 27 Jul 2018 at 16:40, Daniel Thompson
> <daniel.thompson@linaro.org> wrote:
>>
>> On 26/07/18 09:42, Sumit Garg wrote:
>> > On Thu, 26 Jul 2018 at 13:20, Daniel Thompson
>> > <daniel.thompson@linaro.org> wrote:
>> >>
>> >> On Thu, Jul 26, 2018 at 09:39:37AM +0200, Ard Biesheuvel wrote:
>> >>> On 26 July 2018 at 09:36, Daniel Thompson <daniel.thompson@linaro.org> wrote:
>> >>>> On Wed, Jul 25, 2018 at 12:04:58PM +0200, Ard Biesheuvel wrote:
>> >>>>> 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.
>> >>>>
>> >>>> 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?
>> >>>>
>> >
>> > If we pass a DT with OP-TEE and OP-TEE not present, Linux TEE generic
>> > driver exits gracefully giving following message:
>> >
>> > [ 1.976021] optee: probing for conduit method from DT.
>> > [ 1.976033] optee: api uid mismatch
>>
>> That certainly means we can be pretty relaxed about version skew of
>> normal world components (since nothing bad happens if thinks get skewed).
>>
>>
>> >>> Is there any way we can let OP-TEE supply a DT overlay?
>> >>
>> >> I guess it could implement a secure monitor call to provide it. In
>> >> fact I find it a rather pleasing approach. However I think it still loops
>> >> us round to pretty much the same question as before. Does TF-A "protec
>> >> " a normal world that makes an SMC to an OP-TEE that isn't there by
>> >> failing the call in a nice way?
>> >>
>> >
>> > TF-A returns SMC call for OP-TEE as unknown (error code: -1 in "x0"
>> > register) if OP-TEE is not present.
>>
>> It is possible to experiment with getting EDK2 to detect OP-TEE using
>> SMC? This would be fully generic and presumably be the first step in
>> having an EFI OP-TEE driver.
>>
>
> Agree. I will try to detect OP-TEE version via SMC call. If SMC
> unknown is returned, then we say OP-TEE is not present and remove
> corresponding DT node.
>
> So I think this EFI OP-TEE driver makes more sense in edk2 rather than
> edk2-platforms.
>
Indeed.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH edk2-platforms v2 1/1] Silicon/SynQuacer: add optional OP-TEE DT node
2018-07-26 8:42 ` Sumit Garg
2018-07-27 11:10 ` Daniel Thompson
@ 2018-07-27 12:49 ` Mark Rutland
2018-07-27 14:29 ` Sumit Garg
1 sibling, 1 reply; 11+ messages in thread
From: Mark Rutland @ 2018-07-27 12:49 UTC (permalink / raw)
To: Sumit Garg; +Cc: Daniel Thompson, edk2-devel, Patch Tracking
On Thu, Jul 26, 2018 at 02:12:04PM +0530, Sumit Garg wrote:
> On Thu, 26 Jul 2018 at 13:20, Daniel Thompson <daniel.thompson@linaro.org> wrote:
> > I guess it could implement a secure monitor call to provide it. In
> > fact I find it a rather pleasing approach. However I think it still loops
> > us round to pretty much the same question as before. Does TF-A "protec
> > " a normal world that makes an SMC to an OP-TEE that isn't there by
> > failing the call in a nice way?
>
> TF-A returns SMC call for OP-TEE as unknown (error code: -1 in "x0"
> register) if OP-TEE is not present.
Be careful here; you can't use an arbitrary SMC since that could be
implemented by another trusted OS (with a completely different meaning).
Assuming you know the system provides SMCCC, you can use the "Call UID
Query" in the trusted OS range, and check that returned value matches
OP-TEE's UID.
i.e
uid = smccc_uid_query(OPTEE_RANGE);
if (uid == OPTEEE_SMCCC_UID) {
[ OP-TEE present ]
} else {
[ unknown/no trusted OS present ]
}
Thanks,
Mark.
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH edk2-platforms v2 1/1] Silicon/SynQuacer: add optional OP-TEE DT node
2018-07-27 12:49 ` Mark Rutland
@ 2018-07-27 14:29 ` Sumit Garg
0 siblings, 0 replies; 11+ messages in thread
From: Sumit Garg @ 2018-07-27 14:29 UTC (permalink / raw)
To: mark.rutland; +Cc: Daniel Thompson, edk2-devel, Patch Tracking
On Fri, 27 Jul 2018 at 18:19, Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Thu, Jul 26, 2018 at 02:12:04PM +0530, Sumit Garg wrote:
> > On Thu, 26 Jul 2018 at 13:20, Daniel Thompson <daniel.thompson@linaro.org> wrote:
> > > I guess it could implement a secure monitor call to provide it. In
> > > fact I find it a rather pleasing approach. However I think it still loops
> > > us round to pretty much the same question as before. Does TF-A "protec
> > > " a normal world that makes an SMC to an OP-TEE that isn't there by
> > > failing the call in a nice way?
> >
> > TF-A returns SMC call for OP-TEE as unknown (error code: -1 in "x0"
> > register) if OP-TEE is not present.
>
> Be careful here; you can't use an arbitrary SMC since that could be
> implemented by another trusted OS (with a completely different meaning).
>
> Assuming you know the system provides SMCCC, you can use the "Call UID
> Query" in the trusted OS range, and check that returned value matches
> OP-TEE's UID.
>
> i.e
>
> uid = smccc_uid_query(OPTEE_RANGE);
> if (uid == OPTEEE_SMCCC_UID) {
> [ OP-TEE present ]
> } else {
> [ unknown/no trusted OS present ]
> }
>
Thanks Mark for this useful suggestion. Will try to use it.
-Sumit
> Thanks,
> Mark.
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-07-27 14:29 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-07-23 13:19 [PATCH edk2-platforms v2 1/1] Silicon/SynQuacer: add optional OP-TEE DT node Sumit Garg
2018-07-25 10:04 ` Ard Biesheuvel
2018-07-26 7:36 ` Daniel Thompson
2018-07-26 7:39 ` Ard Biesheuvel
2018-07-26 7:50 ` Daniel Thompson
2018-07-26 8:42 ` Sumit Garg
2018-07-27 11:10 ` Daniel Thompson
2018-07-27 11:37 ` Sumit Garg
2018-07-27 12:12 ` Ard Biesheuvel
2018-07-27 12:49 ` Mark Rutland
2018-07-27 14:29 ` Sumit Garg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox