[PATCH v5 0/5] ARM: support for Trusted Foundations secure monitor

[PATCH v5 0/5] ARM: support for Trusted Foundations secure monitor

[Alexandre Courbot]

After several rounds of reviews, this patchset should be close to maturity. This
fifth version gathers the Reviewed-by tags, addresses the (hopefully) last
issues, and seeks the Acked-bys of ARM maintainers to be merged, probably
through the Tegra tree.

Changes since v4:
- Added ifdefs to isolate open firmware-dependant code
- Changed BUGS() into more appropriate calls to panic()
- Fix hard-fail when TF support is not compiled in
- Make Tegra documentation reference the TF binding
- Tested extensively

As there is no consensus on the matter, I have left the TF version in device
tree as two separate nodes. Comments for or against this and references to prior
practices regarding version handling in the device tree are welcome.

Alexandre Courbot (5):
  ARM: add basic Trusted Foundations support
  ARM: tegra: add support for Trusted Foundations
  ARM: tegra: split setting of CPU reset handler
  ARM: tegra: set CPU reset handler with firmware op
  ARM: tegra: support Trusted Foundations by default

 .../arm/firmware/tl,trusted-foundations.txt        | 17 +++++
 Documentation/devicetree/bindings/arm/tegra.txt    |  5 ++
 .../devicetree/bindings/vendor-prefixes.txt        |  1 +
 arch/arm/Kconfig                                   |  2 +
 arch/arm/Makefile                                  |  1 +
 arch/arm/configs/tegra_defconfig                   |  1 +
 arch/arm/firmware/Kconfig                          | 25 +++++++
 arch/arm/firmware/Makefile                         |  1 +
 arch/arm/firmware/trusted_foundations.c            | 82 ++++++++++++++++++++++
 arch/arm/include/asm/trusted_foundations.h         | 53 ++++++++++++++
 arch/arm/mach-tegra/Kconfig                        |  1 +
 arch/arm/mach-tegra/common.c                       |  2 +
 arch/arm/mach-tegra/reset.c                        | 40 ++++++++---
 13 files changed, 220 insertions(+), 11 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
 create mode 100644 arch/arm/firmware/Kconfig
 create mode 100644 arch/arm/firmware/Makefile
 create mode 100644 arch/arm/firmware/trusted_foundations.c
 create mode 100644 arch/arm/include/asm/trusted_foundations.h

-- 
1.8.4

[PATCH v5 1/5] ARM: add basic Trusted Foundations support

[Alexandre Courbot]

Trusted Foundations is a TrustZone-based secure monitor for ARM that
can be invoked  using a consistent SMC-based API on all supported
platforms. This patch adds initial basic support for Trusted
Foundations using the ARM firmware API. Current features are limited
to the ability to boot secondary processors.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
---
 .../arm/firmware/tl,trusted-foundations.txt        | 17 +++++
 .../devicetree/bindings/vendor-prefixes.txt        |  1 +
 arch/arm/Kconfig                                   |  2 +
 arch/arm/Makefile                                  |  1 +
 arch/arm/firmware/Kconfig                          | 25 +++++++
 arch/arm/firmware/Makefile                         |  1 +
 arch/arm/firmware/trusted_foundations.c            | 82 ++++++++++++++++++++++
 arch/arm/include/asm/trusted_foundations.h         | 53 ++++++++++++++
 8 files changed, 182 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
 create mode 100644 arch/arm/firmware/Kconfig
 create mode 100644 arch/arm/firmware/Makefile
 create mode 100644 arch/arm/firmware/trusted_foundations.c
 create mode 100644 arch/arm/include/asm/trusted_foundations.h

diff --git a/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
new file mode 100644
index 0000000..3954bbd
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
@@ -0,0 +1,17 @@
+Trusted Foundations
+
+Boards that use the Trusted Foundations secure monitor can signal its
+presence by declaring a node compatible with "tl,trusted-foundations"
+under the root node.
+
+Required properties:
+- compatible : "tl,trusted-foundations"
+- version-major : major version number of Trusted Foundations firmware
+- version-minor: minor version number of Trusted Foundations firmware
+
+Example:
+	firmware {
+		compatible = "tl,trusted-foundations";
+		version-major = <2>;
+		version-minor = <8>;
+	};
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 366ce9b..20d61f3 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -63,6 +63,7 @@ ste	ST-Ericsson
 stericsson	ST-Ericsson
 toumaz	Toumaz
 ti	Texas Instruments
+tl	Trusted Logic
 toshiba	Toshiba Corporation
 v3	V3 Semiconductor
 via	VIA Technologies, Inc.
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 43594d5..7fbe800 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1074,6 +1074,8 @@ config ARM_TIMER_SP804
 	select CLKSRC_MMIO
 	select CLKSRC_OF if OF
 
+source "arch/arm/firmware/Kconfig"
+
 source arch/arm/mm/Kconfig
 
 config ARM_NR_BANKS
diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 6fd2cea..a48de17 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -267,6 +267,7 @@ core-$(CONFIG_KVM_ARM_HOST) 	+= arch/arm/kvm/
 core-y				+= arch/arm/kernel/ arch/arm/mm/ arch/arm/common/
 core-y				+= arch/arm/net/
 core-y				+= arch/arm/crypto/
+core-y				+= arch/arm/firmware/
 core-y				+= $(machdirs) $(platdirs)
 
 drivers-$(CONFIG_OPROFILE)      += arch/arm/oprofile/
diff --git a/arch/arm/firmware/Kconfig b/arch/arm/firmware/Kconfig
new file mode 100644
index 0000000..ccf44c8
--- /dev/null
+++ b/arch/arm/firmware/Kconfig
@@ -0,0 +1,25 @@
+config ARCH_SUPPORTS_FIRMWARE
+	bool
+
+config ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
+	bool
+	select ARCH_SUPPORTS_FIRMWARE
+
+menu "Firmware options"
+	depends on ARCH_SUPPORTS_FIRMWARE
+
+config TRUSTED_FOUNDATIONS
+	bool "Trusted Foundations secure monitor support"
+	depends on ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
+	help
+	  Some devices are booted with the Trusted Foundations secure monitor
+	  active, requiring some core operations to be performed by the secure
+	  monitor instead of the kernel.
+
+	  This option allows the kernel to invoke the secure monitor whenever
+	  required on devices using Trusted Foundations. See
+	  arch/arm/include/asm/trusted_foundations.h or the
+	  tl,trusted-foundations device tree binding documentation for details
+	  on how to use it.
+
+endmenu
diff --git a/arch/arm/firmware/Makefile b/arch/arm/firmware/Makefile
new file mode 100644
index 0000000..a71f165
--- /dev/null
+++ b/arch/arm/firmware/Makefile
@@ -0,0 +1 @@
+obj-$(CONFIG_TRUSTED_FOUNDATIONS)	+= trusted_foundations.o
diff --git a/arch/arm/firmware/trusted_foundations.c b/arch/arm/firmware/trusted_foundations.c
new file mode 100644
index 0000000..210ef83
--- /dev/null
+++ b/arch/arm/firmware/trusted_foundations.c
@@ -0,0 +1,82 @@
+/*
+ * Trusted Foundations support for ARM CPUs
+ *
+ * Copyright (c) 2013, NVIDIA Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/of.h>
+#include <asm/firmware.h>
+#include <asm/trusted_foundations.h>
+
+#define TF_SET_CPU_BOOT_ADDR_SMC 0xfffff200
+
+static void __naked tf_generic_smc(u32 type, u32 subtype, u32 arg)
+{
+	asm volatile(
+		".arch_extension	sec\n\t"
+		"stmfd	sp!, {r4 - r11, lr}\n\t"
+		__asmeq("%0", "r0")
+		__asmeq("%1", "r1")
+		__asmeq("%2", "r2")
+		"mov	r3, #0\n\t"
+		"mov	r4, #0\n\t"
+		"smc	#0\n\t"
+		"ldmfd	sp!, {r4 - r11, pc}"
+		:
+		: "r" (type), "r" (subtype), "r" (arg)
+		: "memory");
+}
+
+static int tf_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
+{
+	tf_generic_smc(TF_SET_CPU_BOOT_ADDR_SMC, boot_addr, 0);
+
+	return 0;
+}
+
+static const struct firmware_ops trusted_foundations_ops = {
+	.set_cpu_boot_addr = tf_set_cpu_boot_addr,
+};
+
+void register_trusted_foundations(struct trusted_foundations_platform_data *pd)
+{
+	/* we are not using version information for now since currently
+	 * supported SMCs are compatible with all TF releases */
+	register_firmware_ops(&trusted_foundations_ops);
+}
+
+#if IS_ENABLED(CONFIG_OF)
+
+void of_register_trusted_foundations(void)
+{
+	struct device_node *node;
+	struct trusted_foundations_platform_data pdata;
+	int err;
+
+	node = of_find_compatible_node(NULL, NULL, "tl,trusted-foundations");
+
+	if (!node)
+		return;
+
+	err = of_property_read_u32(node, "version-major", &pdata.version_major);
+	if (err != 0)
+		panic("Trusted Foundation: missing version-major property\n");
+	err = of_property_read_u32(node, "version-minor", &pdata.version_minor);
+	if (err != 0)
+		panic("Trusted Foundation: missing version-minor property\n");
+	register_trusted_foundations(&pdata);
+}
+
+#endif /* CONFIG_OF */
diff --git a/arch/arm/include/asm/trusted_foundations.h b/arch/arm/include/asm/trusted_foundations.h
new file mode 100644
index 0000000..558f616
--- /dev/null
+++ b/arch/arm/include/asm/trusted_foundations.h
@@ -0,0 +1,53 @@
+/*
+ * Copyright (c) 2013, NVIDIA Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+
+#ifndef __ASM_ARM_TRUSTED_FOUNDATIONS_H
+#define __ASM_ARM_TRUSTED_FOUNDATIONS_H
+
+#include <linux/kconfig.h>
+
+struct trusted_foundations_platform_data {
+	unsigned int version_major;
+	unsigned int version_minor;
+};
+
+#if IS_ENABLED(CONFIG_TRUSTED_FOUNDATIONS)
+void register_trusted_foundations(struct trusted_foundations_platform_data *pd);
+#if IS_ENABLED(CONFIG_OF)
+void of_register_trusted_foundations(void);
+#endif
+
+#else /* CONFIG_TRUSTED_FOUNDATIONS */
+
+#include <linux/printk.h>
+#include <linux/of.h>
+#include <asm/bug.h>
+
+static inline void register_trusted_foundations(
+				   struct trusted_foundations_platform_data *pd)
+{
+	panic("No support for Trusted Foundations, stopping...\n");
+}
+
+#if IS_ENABLED(CONFIG_OF)
+static inline void of_register_trusted_foundations(void)
+{
+	if (of_find_compatible_node(NULL, NULL, "tl,trusted-foundations"))
+		register_trusted_foundations(NULL);
+}
+#endif
+
+#endif /* CONFIG_TRUSTED_FOUNDATIONS */
+
+#endif
-- 
1.8.4

Re: [PATCH v5 1/5] ARM: add basic Trusted Foundations support

[Linus Walleij]

On Thu, Sep 5, 2013 at 5:27 AM, Alexandre Courbot <acourbot@nvidia.com> wrote:

> Trusted Foundations is a TrustZone-based secure monitor for ARM that
> can be invoked  using a consistent SMC-based API on all supported
> platforms. This patch adds initial basic support for Trusted
> Foundations using the ARM firmware API. Current features are limited
> to the ability to boot secondary processors.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> Reviewed-by: Tomasz Figa <t.figa@samsung.com>
> Reviewed-by: Stephen Warren <swarren@nvidia.com>

I take it that this means that it is a set of standard routines
living in the secure world that can be called by way of this
API, correct? The commit message is written a bit like
everybody knows what this is about but I don't :-)

Some more background here would be nice, like where did this
thing come from, and are there other platforms in existance that
use trusted foundations or is it a Tegra-only thing? Does a
specification of this thing listing available services exist for
example?

How does it relate to the (arch-neutral) trusted execution
environment and such things that other vendors are pushing for?
Can the trusted foundations be used "underneath" such
frameworks for trusted applications, or is it a parallell thing
altogether?

Simple things like that...

I tried googling it, is this a relevant URL?
http://www.arm.com/community/partners/display_product/rw/ProductId/5393/

Thanks,
Linus Walleij

Re: [PATCH v5 1/5] ARM: add basic Trusted Foundations support

[Alexandre Courbot]

On Sat, Sep 7, 2013 at 1:48 AM, Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> On Thu, Sep 5, 2013 at 5:27 AM, Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org> wrote:
>
>> Trusted Foundations is a TrustZone-based secure monitor for ARM that
>> can be invoked  using a consistent SMC-based API on all supported
>> platforms. This patch adds initial basic support for Trusted
>> Foundations using the ARM firmware API. Current features are limited
>> to the ability to boot secondary processors.
>>
>> Signed-off-by: Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> Reviewed-by: Tomasz Figa <t.figa-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> Reviewed-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>
> I take it that this means that it is a set of standard routines
> living in the secure world that can be called by way of this
> API, correct? The commit message is written a bit like
> everybody knows what this is about but I don't :-)

It's actually written a bit like I don't really know what this is
about, thanks for exposing my ignorance. :P

> Some more background here would be nice, like where did this
> thing come from, and are there other platforms in existance that
> use trusted foundations or is it a Tegra-only thing? Does a
> specification of this thing listing available services exist for
> example?

There is unfortunately no public specification of this AFAIK. Client
(kernel) side sources are available as Tegra downstream kernel
releases, but that's about it.

> How does it relate to the (arch-neutral) trusted execution
> environment and such things that other vendors are pushing for?
> Can the trusted foundations be used "underneath" such
> frameworks for trusted applications, or is it a parallell thing
> altogether?

My understanding is that TF serves the same purpose for ARM only AFAICT.

> I tried googling it, is this a relevant URL?
> http://www.arm.com/community/partners/display_product/rw/ProductId/5393/

Yes. TF is an essential feature to support in order to boot the kernel
on some Tegra-based consumer devices (e.g. SHIELD). Basic things such
as secondary CPU control must be performed through SMCs. It has many
more features but for the moment I don't see a need to go beyond the
very basic.

The issue is that as you pointed out, public information is scarce and
future direction uncertain. That's why this patchset limits itself to
what is needed to get secondary CPUs running - the procedure is very
simple and unlikely to change in the future. DT binding is also as
concise as possible to allow malleability since we cannot predict how
it will evolve in the future.

I will try to explain better what TF is in the commit log - at least
as far as I understand it myself.

Alex.

Re: [PATCH v5 1/5] ARM: add basic Trusted Foundations support

[Rob Herring]

On 09/04/2013 10:27 PM, Alexandre Courbot wrote:
> Trusted Foundations is a TrustZone-based secure monitor for ARM that
> can be invoked  using a consistent SMC-based API on all supported
> platforms. This patch adds initial basic support for Trusted
> Foundations using the ARM firmware API. Current features are limited
> to the ability to boot secondary processors.
> 
> Signed-off-by: Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> Reviewed-by: Tomasz Figa <t.figa-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Reviewed-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
>  .../arm/firmware/tl,trusted-foundations.txt        | 17 +++++
>  .../devicetree/bindings/vendor-prefixes.txt        |  1 +
>  arch/arm/Kconfig                                   |  2 +
>  arch/arm/Makefile                                  |  1 +
>  arch/arm/firmware/Kconfig                          | 25 +++++++
>  arch/arm/firmware/Makefile                         |  1 +
>  arch/arm/firmware/trusted_foundations.c            | 82 ++++++++++++++++++++++
>  arch/arm/include/asm/trusted_foundations.h         | 53 ++++++++++++++
>  8 files changed, 182 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
>  create mode 100644 arch/arm/firmware/Kconfig
>  create mode 100644 arch/arm/firmware/Makefile
>  create mode 100644 arch/arm/firmware/trusted_foundations.c
>  create mode 100644 arch/arm/include/asm/trusted_foundations.h
> 
> diff --git a/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
> new file mode 100644
> index 0000000..3954bbd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
> @@ -0,0 +1,17 @@
> +Trusted Foundations
> +
> +Boards that use the Trusted Foundations secure monitor can signal its
> +presence by declaring a node compatible with "tl,trusted-foundations"
> +under the root node.
> +
> +Required properties:
> +- compatible : "tl,trusted-foundations"
> +- version-major : major version number of Trusted Foundations firmware
> +- version-minor: minor version number of Trusted Foundations firmware
> +
> +Example:
> +	firmware {
> +		compatible = "tl,trusted-foundations";
> +		version-major = <2>;
> +		version-minor = <8>;
> +	};

I'm wondering how we fit this in with PSCI bindings? Both are pieces of
firmware functionality and may co-exist. There's nothing incompatible
here, but there should be some commonality. Will future versions of
Trusted Foundations follow the SMC calling conventions doc? What about
armv8 support.

Rob


> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index 366ce9b..20d61f3 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -63,6 +63,7 @@ ste	ST-Ericsson
>  stericsson	ST-Ericsson
>  toumaz	Toumaz
>  ti	Texas Instruments
> +tl	Trusted Logic
>  toshiba	Toshiba Corporation
>  v3	V3 Semiconductor
>  via	VIA Technologies, Inc.
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 43594d5..7fbe800 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1074,6 +1074,8 @@ config ARM_TIMER_SP804
>  	select CLKSRC_MMIO
>  	select CLKSRC_OF if OF
>  
> +source "arch/arm/firmware/Kconfig"
> +
>  source arch/arm/mm/Kconfig
>  
>  config ARM_NR_BANKS
> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> index 6fd2cea..a48de17 100644
> --- a/arch/arm/Makefile
> +++ b/arch/arm/Makefile
> @@ -267,6 +267,7 @@ core-$(CONFIG_KVM_ARM_HOST) 	+= arch/arm/kvm/
>  core-y				+= arch/arm/kernel/ arch/arm/mm/ arch/arm/common/
>  core-y				+= arch/arm/net/
>  core-y				+= arch/arm/crypto/
> +core-y				+= arch/arm/firmware/
>  core-y				+= $(machdirs) $(platdirs)
>  
>  drivers-$(CONFIG_OPROFILE)      += arch/arm/oprofile/
> diff --git a/arch/arm/firmware/Kconfig b/arch/arm/firmware/Kconfig
> new file mode 100644
> index 0000000..ccf44c8
> --- /dev/null
> +++ b/arch/arm/firmware/Kconfig
> @@ -0,0 +1,25 @@
> +config ARCH_SUPPORTS_FIRMWARE
> +	bool
> +
> +config ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
> +	bool
> +	select ARCH_SUPPORTS_FIRMWARE
> +
> +menu "Firmware options"
> +	depends on ARCH_SUPPORTS_FIRMWARE
> +
> +config TRUSTED_FOUNDATIONS
> +	bool "Trusted Foundations secure monitor support"
> +	depends on ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
> +	help
> +	  Some devices are booted with the Trusted Foundations secure monitor
> +	  active, requiring some core operations to be performed by the secure
> +	  monitor instead of the kernel.
> +
> +	  This option allows the kernel to invoke the secure monitor whenever
> +	  required on devices using Trusted Foundations. See
> +	  arch/arm/include/asm/trusted_foundations.h or the
> +	  tl,trusted-foundations device tree binding documentation for details
> +	  on how to use it.
> +
> +endmenu
> diff --git a/arch/arm/firmware/Makefile b/arch/arm/firmware/Makefile
> new file mode 100644
> index 0000000..a71f165
> --- /dev/null
> +++ b/arch/arm/firmware/Makefile
> @@ -0,0 +1 @@
> +obj-$(CONFIG_TRUSTED_FOUNDATIONS)	+= trusted_foundations.o
> diff --git a/arch/arm/firmware/trusted_foundations.c b/arch/arm/firmware/trusted_foundations.c
> new file mode 100644
> index 0000000..210ef83
> --- /dev/null
> +++ b/arch/arm/firmware/trusted_foundations.c
> @@ -0,0 +1,82 @@
> +/*
> + * Trusted Foundations support for ARM CPUs
> + *
> + * Copyright (c) 2013, NVIDIA Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/of.h>
> +#include <asm/firmware.h>
> +#include <asm/trusted_foundations.h>
> +
> +#define TF_SET_CPU_BOOT_ADDR_SMC 0xfffff200
> +
> +static void __naked tf_generic_smc(u32 type, u32 subtype, u32 arg)
> +{
> +	asm volatile(
> +		".arch_extension	sec\n\t"
> +		"stmfd	sp!, {r4 - r11, lr}\n\t"
> +		__asmeq("%0", "r0")
> +		__asmeq("%1", "r1")
> +		__asmeq("%2", "r2")
> +		"mov	r3, #0\n\t"
> +		"mov	r4, #0\n\t"
> +		"smc	#0\n\t"
> +		"ldmfd	sp!, {r4 - r11, pc}"
> +		:
> +		: "r" (type), "r" (subtype), "r" (arg)
> +		: "memory");
> +}
> +
> +static int tf_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
> +{
> +	tf_generic_smc(TF_SET_CPU_BOOT_ADDR_SMC, boot_addr, 0);
> +
> +	return 0;
> +}
> +
> +static const struct firmware_ops trusted_foundations_ops = {
> +	.set_cpu_boot_addr = tf_set_cpu_boot_addr,
> +};
> +
> +void register_trusted_foundations(struct trusted_foundations_platform_data *pd)
> +{
> +	/* we are not using version information for now since currently
> +	 * supported SMCs are compatible with all TF releases */
> +	register_firmware_ops(&trusted_foundations_ops);
> +}
> +
> +#if IS_ENABLED(CONFIG_OF)
> +
> +void of_register_trusted_foundations(void)
> +{
> +	struct device_node *node;
> +	struct trusted_foundations_platform_data pdata;
> +	int err;
> +
> +	node = of_find_compatible_node(NULL, NULL, "tl,trusted-foundations");
> +
> +	if (!node)
> +		return;
> +
> +	err = of_property_read_u32(node, "version-major", &pdata.version_major);
> +	if (err != 0)
> +		panic("Trusted Foundation: missing version-major property\n");
> +	err = of_property_read_u32(node, "version-minor", &pdata.version_minor);
> +	if (err != 0)
> +		panic("Trusted Foundation: missing version-minor property\n");
> +	register_trusted_foundations(&pdata);
> +}
> +
> +#endif /* CONFIG_OF */
> diff --git a/arch/arm/include/asm/trusted_foundations.h b/arch/arm/include/asm/trusted_foundations.h
> new file mode 100644
> index 0000000..558f616
> --- /dev/null
> +++ b/arch/arm/include/asm/trusted_foundations.h
> @@ -0,0 +1,53 @@
> +/*
> + * Copyright (c) 2013, NVIDIA Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + */
> +
> +#ifndef __ASM_ARM_TRUSTED_FOUNDATIONS_H
> +#define __ASM_ARM_TRUSTED_FOUNDATIONS_H
> +
> +#include <linux/kconfig.h>
> +
> +struct trusted_foundations_platform_data {
> +	unsigned int version_major;
> +	unsigned int version_minor;
> +};
> +
> +#if IS_ENABLED(CONFIG_TRUSTED_FOUNDATIONS)
> +void register_trusted_foundations(struct trusted_foundations_platform_data *pd);
> +#if IS_ENABLED(CONFIG_OF)
> +void of_register_trusted_foundations(void);
> +#endif
> +
> +#else /* CONFIG_TRUSTED_FOUNDATIONS */
> +
> +#include <linux/printk.h>
> +#include <linux/of.h>
> +#include <asm/bug.h>
> +
> +static inline void register_trusted_foundations(
> +				   struct trusted_foundations_platform_data *pd)
> +{
> +	panic("No support for Trusted Foundations, stopping...\n");
> +}
> +
> +#if IS_ENABLED(CONFIG_OF)
> +static inline void of_register_trusted_foundations(void)
> +{
> +	if (of_find_compatible_node(NULL, NULL, "tl,trusted-foundations"))
> +		register_trusted_foundations(NULL);
> +}
> +#endif
> +
> +#endif /* CONFIG_TRUSTED_FOUNDATIONS */
> +
> +#endif
> 

Re: [PATCH v5 1/5] ARM: add basic Trusted Foundations support

[Alexandre Courbot]

On Fri, Sep 6, 2013 at 3:35 AM, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On 09/04/2013 10:27 PM, Alexandre Courbot wrote:
>> Trusted Foundations is a TrustZone-based secure monitor for ARM that
>> can be invoked  using a consistent SMC-based API on all supported
>> platforms. This patch adds initial basic support for Trusted
>> Foundations using the ARM firmware API. Current features are limited
>> to the ability to boot secondary processors.
>>
>> Signed-off-by: Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> Reviewed-by: Tomasz Figa <t.figa-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> Reviewed-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>> ---
>>  .../arm/firmware/tl,trusted-foundations.txt        | 17 +++++
>>  .../devicetree/bindings/vendor-prefixes.txt        |  1 +
>>  arch/arm/Kconfig                                   |  2 +
>>  arch/arm/Makefile                                  |  1 +
>>  arch/arm/firmware/Kconfig                          | 25 +++++++
>>  arch/arm/firmware/Makefile                         |  1 +
>>  arch/arm/firmware/trusted_foundations.c            | 82 ++++++++++++++++++++++
>>  arch/arm/include/asm/trusted_foundations.h         | 53 ++++++++++++++
>>  8 files changed, 182 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
>>  create mode 100644 arch/arm/firmware/Kconfig
>>  create mode 100644 arch/arm/firmware/Makefile
>>  create mode 100644 arch/arm/firmware/trusted_foundations.c
>>  create mode 100644 arch/arm/include/asm/trusted_foundations.h
>>
>> diff --git a/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
>> new file mode 100644
>> index 0000000..3954bbd
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
>> @@ -0,0 +1,17 @@
>> +Trusted Foundations
>> +
>> +Boards that use the Trusted Foundations secure monitor can signal its
>> +presence by declaring a node compatible with "tl,trusted-foundations"
>> +under the root node.
>> +
>> +Required properties:
>> +- compatible : "tl,trusted-foundations"
>> +- version-major : major version number of Trusted Foundations firmware
>> +- version-minor: minor version number of Trusted Foundations firmware
>> +
>> +Example:
>> +     firmware {
>> +             compatible = "tl,trusted-foundations";
>> +             version-major = <2>;
>> +             version-minor = <8>;
>> +     };
>
> I'm wondering how we fit this in with PSCI bindings? Both are pieces of
> firmware functionality and may co-exist. There's nothing incompatible
> here, but there should be some commonality. Will future versions of
> Trusted Foundations follow the SMC calling conventions doc? What about
> armv8 support.

I don't have any information about the future of TF unfortunately,
excepted that it should remain backward-compatible. What is this SMC
calling convention doc your are talking about btw? Is there a standard
calling convention defined by ARM?

Alex.

Re: [PATCH v5 1/5] ARM: add basic Trusted Foundations support

[Will Deacon]

Hi guys,

On Mon, Sep 09, 2013 at 07:15:31AM +0100, Alexandre Courbot wrote:
> On Fri, Sep 6, 2013 at 3:35 AM, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > On 09/04/2013 10:27 PM, Alexandre Courbot wrote:
> >> Trusted Foundations is a TrustZone-based secure monitor for ARM that
> >> can be invoked  using a consistent SMC-based API on all supported
> >> platforms. This patch adds initial basic support for Trusted
> >> Foundations using the ARM firmware API. Current features are limited
> >> to the ability to boot secondary processors.

[...]

> >> +Example:
> >> +     firmware {
> >> +             compatible = "tl,trusted-foundations";
> >> +             version-major = <2>;
> >> +             version-minor = <8>;
> >> +     };
> >
> > I'm wondering how we fit this in with PSCI bindings? Both are pieces of
> > firmware functionality and may co-exist. There's nothing incompatible
> > here, but there should be some commonality. Will future versions of
> > Trusted Foundations follow the SMC calling conventions doc? What about
> > armv8 support.
> 
> I don't have any information about the future of TF unfortunately,
> excepted that it should remain backward-compatible. What is this SMC
> calling convention doc your are talking about btw? Is there a standard
> calling convention defined by ARM?

SMC calling convention:

https://silver.arm.com/download/download.tm?pv=1435186

PSCI:

https://silver.arm.com/download/download.tm?pv=1421585

Although access to these documents requires you to go through a
click-through license.

Will

Re: [PATCH v5 1/5] ARM: add basic Trusted Foundations support

[Linus Walleij]

On Tue, Sep 10, 2013 at 3:04 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Mon, Sep 09, 2013 at 07:15:31AM +0100, Alexandre Courbot wrote:

>> I don't have any information about the future of TF unfortunately,
>> excepted that it should remain backward-compatible. What is this SMC
>> calling convention doc your are talking about btw? Is there a standard
>> calling convention defined by ARM?
>
> SMC calling convention:
> https://silver.arm.com/download/download.tm?pv=1435186

According to the SMC calling convention r0 should be
the SMC function identifier.

+static void __naked tf_generic_smc(u32 type, u32 subtype, u32 arg)
+{
+       asm volatile(
+               ".arch_extension        sec\n\t"
+               "stmfd  sp!, {r4 - r11, lr}\n\t"
+               __asmeq("%0", "r0")
+               __asmeq("%1", "r1")
+               __asmeq("%2", "r2")
+               "mov    r3, #0\n\t"
+               "mov    r4, #0\n\t"
+               "smc    #0\n\t"
+               "ldmfd  sp!, {r4 - r11, pc}"
+               :
+               : "r" (type), "r" (subtype), "r" (arg)
+               : "memory");
+}

So type == function identifier? Or are these two totally
different things, such that trusted foundations are already
a different beast that what the SMC calling convention
specifies?

Anyway:
r0,r1,r2 = type/subtype/arg.

+#define TF_SET_CPU_BOOT_ADDR_SMC 0xfffff200
(...)
+static int tf_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
+{
+       tf_generic_smc(TF_SET_CPU_BOOT_ADDR_SMC, boot_addr, 0);
+
+       return 0;
+}

What kind of a "subtype" is the boot address? I could have
accepted it if the *last* thing, the argument contained the boot
address. With the type/subtype/arg convention it would be
more natural if the "subtype" was something like 0.

Should the SMC function rather have this signature:

tf_generic_smc(u32 type, u32 arg1, u32 arg2)

?

Then *sometimes* arg1 is a subtype, if the "type" is
such that it takes a subtype?

(That's a rough guess.)

So to begin with the arguments to tf_generic_smc() are either
confusingly named or tf_set_cpu_boot_addr() begins the
journey with violating the function signature.

I also wonder what kind if "type" starts its enumerator on
0xfffff200? Wouldn't it be more natural if it was e.g. 1?
It looks like the TF_SET_CPU_BOOT_ADDR_SMC
reflects some bit-wise encoding scheme, so some details
here wouldn't hurt?

The main thing is that the patch has to say that this
is an API toward trusted foundations, that it has nothing
to do with the SMC calling conventions, and only pertain
to devices that use trusted foundations, so as to avoid
any confusion.

Then I'm game :-)

Yours,
Linus Walleij

Re: [PATCH v5 1/5] ARM: add basic Trusted Foundations support

[Alexandre Courbot]

On Thu, Sep 12, 2013 at 6:18 PM, Linus Walleij <linus.walleij@linaro.org> wrote:
> On Tue, Sep 10, 2013 at 3:04 PM, Will Deacon <will.deacon@arm.com> wrote:
>> On Mon, Sep 09, 2013 at 07:15:31AM +0100, Alexandre Courbot wrote:
>
>>> I don't have any information about the future of TF unfortunately,
>>> excepted that it should remain backward-compatible. What is this SMC
>>> calling convention doc your are talking about btw? Is there a standard
>>> calling convention defined by ARM?
>>
>> SMC calling convention:
>> https://silver.arm.com/download/download.tm?pv=1435186
>
> According to the SMC calling convention r0 should be
> the SMC function identifier.
>
> +static void __naked tf_generic_smc(u32 type, u32 subtype, u32 arg)
> +{
> +       asm volatile(
> +               ".arch_extension        sec\n\t"
> +               "stmfd  sp!, {r4 - r11, lr}\n\t"
> +               __asmeq("%0", "r0")
> +               __asmeq("%1", "r1")
> +               __asmeq("%2", "r2")
> +               "mov    r3, #0\n\t"
> +               "mov    r4, #0\n\t"
> +               "smc    #0\n\t"
> +               "ldmfd  sp!, {r4 - r11, pc}"
> +               :
> +               : "r" (type), "r" (subtype), "r" (arg)
> +               : "memory");
> +}
>
> So type == function identifier? Or are these two totally
> different things, such that trusted foundations are already
> a different beast that what the SMC calling convention
> specifies?

After looking at the documents linked by Will and Catalin (thanks!), I
can state with confidence that TF does not follow the SMC calling
convention and has nothing to do with PSCI neither. The value passed
in r0 does not match the function identifier definition at all: with
its two MSBs set to 1, it would be interpreted as a SMC64 Fast Call,
which it clearly is not.

There is also no correspondance with any of the functions ids defined
in the PSCI specification.

> Anyway:
> r0,r1,r2 = type/subtype/arg.
>
> +#define TF_SET_CPU_BOOT_ADDR_SMC 0xfffff200
> (...)
> +static int tf_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
> +{
> +       tf_generic_smc(TF_SET_CPU_BOOT_ADDR_SMC, boot_addr, 0);
> +
> +       return 0;
> +}
>
> What kind of a "subtype" is the boot address? I could have
> accepted it if the *last* thing, the argument contained the boot
> address. With the type/subtype/arg convention it would be
> more natural if the "subtype" was something like 0.
>
> Should the SMC function rather have this signature:
>
> tf_generic_smc(u32 type, u32 arg1, u32 arg2)
>
> ?
>
> Then *sometimes* arg1 is a subtype, if the "type" is
> such that it takes a subtype?
>
> (That's a rough guess.)
>
> So to begin with the arguments to tf_generic_smc() are either
> confusingly named or tf_set_cpu_boot_addr() begins the
> journey with violating the function signature.

Indeed the subtype parameter is poorly named. That will teach me
reproducing the behavior of our downstream kernel without thinking. :/

> I also wonder what kind if "type" starts its enumerator on
> 0xfffff200? Wouldn't it be more natural if it was e.g. 1?
> It looks like the TF_SET_CPU_BOOT_ADDR_SMC
> reflects some bit-wise encoding scheme, so some details
> here wouldn't hurt?

I agree, unfortunately this raw value is all the details I myself have
access to. /o\

> The main thing is that the patch has to say that this
> is an API toward trusted foundations, that it has nothing
> to do with the SMC calling conventions, and only pertain
> to devices that use trusted foundations, so as to avoid
> any confusion.

Note that the patch never claimed to follow the SMC calling
conventions. You guys assumed it should for some reason. ;)

But clearly, it cannot hurt to state this unambiguously, so I will
make sure to do that. Then I hope the lack of detail about the
non-standard calling convention will not be an obstacle to its
merging, as this enables a bunch of retail Tegra devices (e.g. SHIELD)
to boot.

Thanks,
Alex.

Re: [PATCH v5 1/5] ARM: add basic Trusted Foundations support

[Catalin Marinas]

On 9 September 2013 07:15, Alexandre Courbot <gnurou-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Fri, Sep 6, 2013 at 3:35 AM, Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On 09/04/2013 10:27 PM, Alexandre Courbot wrote:
>>> Trusted Foundations is a TrustZone-based secure monitor for ARM that
>>> can be invoked  using a consistent SMC-based API on all supported
>>> platforms. This patch adds initial basic support for Trusted
>>> Foundations using the ARM firmware API. Current features are limited
>>> to the ability to boot secondary processors.
>>>
>>> Signed-off-by: Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>> Reviewed-by: Tomasz Figa <t.figa-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>>> Reviewed-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>> ---
>>>  .../arm/firmware/tl,trusted-foundations.txt        | 17 +++++
>>>  .../devicetree/bindings/vendor-prefixes.txt        |  1 +
>>>  arch/arm/Kconfig                                   |  2 +
>>>  arch/arm/Makefile                                  |  1 +
>>>  arch/arm/firmware/Kconfig                          | 25 +++++++
>>>  arch/arm/firmware/Makefile                         |  1 +
>>>  arch/arm/firmware/trusted_foundations.c            | 82 ++++++++++++++++++++++
>>>  arch/arm/include/asm/trusted_foundations.h         | 53 ++++++++++++++
>>>  8 files changed, 182 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
>>>  create mode 100644 arch/arm/firmware/Kconfig
>>>  create mode 100644 arch/arm/firmware/Makefile
>>>  create mode 100644 arch/arm/firmware/trusted_foundations.c
>>>  create mode 100644 arch/arm/include/asm/trusted_foundations.h
>>>
>>> diff --git a/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
>>> new file mode 100644
>>> index 0000000..3954bbd
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
>>> @@ -0,0 +1,17 @@
>>> +Trusted Foundations
>>> +
>>> +Boards that use the Trusted Foundations secure monitor can signal its
>>> +presence by declaring a node compatible with "tl,trusted-foundations"
>>> +under the root node.
>>> +
>>> +Required properties:
>>> +- compatible : "tl,trusted-foundations"
>>> +- version-major : major version number of Trusted Foundations firmware
>>> +- version-minor: minor version number of Trusted Foundations firmware
>>> +
>>> +Example:
>>> +     firmware {
>>> +             compatible = "tl,trusted-foundations";
>>> +             version-major = <2>;
>>> +             version-minor = <8>;
>>> +     };
>>
>> I'm wondering how we fit this in with PSCI bindings? Both are pieces of
>> firmware functionality and may co-exist. There's nothing incompatible
>> here, but there should be some commonality. Will future versions of
>> Trusted Foundations follow the SMC calling conventions doc? What about
>> armv8 support.
>
> I don't have any information about the future of TF unfortunately,
> excepted that it should remain backward-compatible. What is this SMC
> calling convention doc your are talking about btw? Is there a standard
> calling convention defined by ARM?

The SMC calling convention is here (it requires free registration):

http://infocenter.arm.com/help/topic/com.arm.doc.den0028a/index.html

>From the introduction:

"This document defines a common calling mechanism for use with the
Secure Monitor Call (SMC)
instruction in both the ARMv7 and ARMv8 architectures.

The SMC instruction is used to generate a synchronous exception that
is handled by Secure Monitor code running in EL3. The arguments are
passed in registers and then used to select which Secure function to
execute. These calls may then be passed on to a Trusted OS in S-EL1.

This specification aims to ease integration and reduce fragmentation
between software layers, such as Operating Systems, Hypervisors,
Trusted OS, Secure Monitor and System Firmware."


If you talk about booting secondary CPUs, there is also PSCI:

http://infocenter.arm.com/help/topic/com.arm.doc.den0022b/index.html

-- 
Catalin

Re: [PATCH v5 1/5] ARM: add basic Trusted Foundations support

[Stephen Warren]

On 09/04/2013 09:27 PM, Alexandre Courbot wrote:
> Trusted Foundations is a TrustZone-based secure monitor for ARM that
> can be invoked  using a consistent SMC-based API on all supported
> platforms. This patch adds initial basic support for Trusted
> Foundations using the ARM firmware API. Current features are limited
> to the ability to boot secondary processors.

> diff --git a/arch/arm/firmware/trusted_foundations.c b/arch/arm/firmware/trusted_foundations.c
...
> +void of_register_trusted_foundations(void)
> +{
> +	struct device_node *node;
> +	struct trusted_foundations_platform_data pdata;
> +	int err;
> +
> +	node = of_find_compatible_node(NULL, NULL, "tl,trusted-foundations");
> +
> +	if (!node)
> +		return;

If you're going to revise this patch for the comment below, then I would
suggest removing the blank line before that if statement.

> diff --git a/arch/arm/include/asm/trusted_foundations.h b/arch/arm/include/asm/trusted_foundations.h

Do we need to add the following here:

#if !IS_ENABLED(CONFIG_OF)
static inline void register_trusted_foundations(...) {}
#endif

So that there is a dummy no-op function for the non-DT-support case? I
guess Tegra always has CONFIG_OF enabled so that call from
mach-tegra/common.c in patch 2 will never be an issue, but perhaps it
might if anyone else uses this?

Re: [PATCH v5 1/5] ARM: add basic Trusted Foundations support

[Alexandre Courbot]

On Sat, Sep 7, 2013 at 4:29 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 09/04/2013 09:27 PM, Alexandre Courbot wrote:
>> Trusted Foundations is a TrustZone-based secure monitor for ARM that
>> can be invoked  using a consistent SMC-based API on all supported
>> platforms. This patch adds initial basic support for Trusted
>> Foundations using the ARM firmware API. Current features are limited
>> to the ability to boot secondary processors.
>
>> diff --git a/arch/arm/firmware/trusted_foundations.c b/arch/arm/firmware/trusted_foundations.c
> ...
>> +void of_register_trusted_foundations(void)
>> +{
>> +     struct device_node *node;
>> +     struct trusted_foundations_platform_data pdata;
>> +     int err;
>> +
>> +     node = of_find_compatible_node(NULL, NULL, "tl,trusted-foundations");
>> +
>> +     if (!node)
>> +             return;
>
> If you're going to revise this patch for the comment below, then I would
> suggest removing the blank line before that if statement.

Sure.

>
>> diff --git a/arch/arm/include/asm/trusted_foundations.h b/arch/arm/include/asm/trusted_foundations.h
>
> Do we need to add the following here:
>
> #if !IS_ENABLED(CONFIG_OF)
> static inline void register_trusted_foundations(...) {}
> #endif
>
> So that there is a dummy no-op function for the non-DT-support case? I
> guess Tegra always has CONFIG_OF enabled so that call from
> mach-tegra/common.c in patch 2 will never be an issue, but perhaps it
> might if anyone else uses this?

My expectation is that register_trusted_foundations() is called by the
platform code once it has established (through whatever mean it likes)
that Trusted Foundations is required. For platforms supporting DT,
of_register_trusted_foundations() takes care of that. Platforms that
don't support DT need another way to decide whether they *need* TF or
not. Once a platform decided that it needs TF, its absence is not an
option, and therefore I'd think that register_trusted_foundations()
should hard-fail if support is not compiled in. Or maybe I missed your
point?

Alex.

Re: [PATCH v5 1/5] ARM: add basic Trusted Foundations support

[Stephen Warren]

On 09/09/2013 12:38 AM, Alexandre Courbot wrote:
> On Sat, Sep 7, 2013 at 4:29 AM, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
>> On 09/04/2013 09:27 PM, Alexandre Courbot wrote:
>>> Trusted Foundations is a TrustZone-based secure monitor for ARM that
>>> can be invoked  using a consistent SMC-based API on all supported
>>> platforms. This patch adds initial basic support for Trusted
>>> Foundations using the ARM firmware API. Current features are limited
>>> to the ability to boot secondary processors.

>>> diff --git a/arch/arm/include/asm/trusted_foundations.h b/arch/arm/include/asm/trusted_foundations.h
>>
>> Do we need to add the following here:
>>
>> #if !IS_ENABLED(CONFIG_OF)
>> static inline void register_trusted_foundations(...) {}
>> #endif

Uggh. I meant to write **of_**register_trusted_foundations() there.

>> So that there is a dummy no-op function for the non-DT-support case? I
>> guess Tegra always has CONFIG_OF enabled so that call from
>> mach-tegra/common.c in patch 2 will never be an issue, but perhaps it
>> might if anyone else uses this?
> 
> My expectation is that register_trusted_foundations() is called by the
> platform code once it has established (through whatever mean it likes)
> that Trusted Foundations is required. For platforms supporting DT,
> of_register_trusted_foundations() takes care of that. Platforms that
> don't support DT need another way to decide whether they *need* TF or
> not. Once a platform decided that it needs TF, its absence is not an
> option, and therefore I'd think that register_trusted_foundations()
> should hard-fail if support is not compiled in. Or maybe I missed your
> point?

So yes, you missed my point, but it was my fault:-)

Most of_* can be called irrespective of CONFIG_OF, and since
of_register_trusted_foundations() is meant to handle internally the "do
I need to do anything" case, I think it makes sense to be allowed to
call it unconditionally without determining anything about either
CONFIG_OF or TF DT node presence?

Re: [PATCH v5 1/5] ARM: add basic Trusted Foundations support

[Alexandre Courbot]

On Tue, Sep 10, 2013 at 12:59 AM, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
> On 09/09/2013 12:38 AM, Alexandre Courbot wrote:
>> On Sat, Sep 7, 2013 at 4:29 AM, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
>>> On 09/04/2013 09:27 PM, Alexandre Courbot wrote:
>>>> Trusted Foundations is a TrustZone-based secure monitor for ARM that
>>>> can be invoked  using a consistent SMC-based API on all supported
>>>> platforms. This patch adds initial basic support for Trusted
>>>> Foundations using the ARM firmware API. Current features are limited
>>>> to the ability to boot secondary processors.
>
>>>> diff --git a/arch/arm/include/asm/trusted_foundations.h b/arch/arm/include/asm/trusted_foundations.h
>>>
>>> Do we need to add the following here:
>>>
>>> #if !IS_ENABLED(CONFIG_OF)
>>> static inline void register_trusted_foundations(...) {}
>>> #endif
>
> Uggh. I meant to write **of_**register_trusted_foundations() there.
>
>>> So that there is a dummy no-op function for the non-DT-support case? I
>>> guess Tegra always has CONFIG_OF enabled so that call from
>>> mach-tegra/common.c in patch 2 will never be an issue, but perhaps it
>>> might if anyone else uses this?
>>
>> My expectation is that register_trusted_foundations() is called by the
>> platform code once it has established (through whatever mean it likes)
>> that Trusted Foundations is required. For platforms supporting DT,
>> of_register_trusted_foundations() takes care of that. Platforms that
>> don't support DT need another way to decide whether they *need* TF or
>> not. Once a platform decided that it needs TF, its absence is not an
>> option, and therefore I'd think that register_trusted_foundations()
>> should hard-fail if support is not compiled in. Or maybe I missed your
>> point?
>
> So yes, you missed my point, but it was my fault:-)
>
> Most of_* can be called irrespective of CONFIG_OF, and since
> of_register_trusted_foundations() is meant to handle internally the "do
> I need to do anything" case, I think it makes sense to be allowed to
> call it unconditionally without determining anything about either
> CONFIG_OF or TF DT node presence?

Ah, ok, I follow you now. I wasn't sure what the convention was here,
but yeah, I see nothing against making a no-op
of_register_trusted_foundations() available when CONFIG_OF is not
enabled.

Thanks,
Alex.

[PATCH v5 2/5] ARM: tegra: add support for Trusted Foundations

[Alexandre Courbot]

Register the firmware operations for Trusted Foundations if the device
tree indicates it is active on the device.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
---
 Documentation/devicetree/bindings/arm/tegra.txt | 5 +++++
 arch/arm/mach-tegra/Kconfig                     | 1 +
 arch/arm/mach-tegra/common.c                    | 2 ++
 3 files changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/tegra.txt b/Documentation/devicetree/bindings/arm/tegra.txt
index ed9c853..8e51884 100644
--- a/Documentation/devicetree/bindings/arm/tegra.txt
+++ b/Documentation/devicetree/bindings/arm/tegra.txt
@@ -32,3 +32,8 @@ board-specific compatible values:
   nvidia,whistler
   toradex,colibri_t20-512
   toradex,iris
+
+Trusted Foundations
+-------------------------------------------
+Tegra supports the Trusted Foundation secure monitor. See the
+"tl,trusted-foundations" binding's documentation for more details.
diff --git a/arch/arm/mach-tegra/Kconfig b/arch/arm/mach-tegra/Kconfig
index ef3a8da..b6b7c44 100644
--- a/arch/arm/mach-tegra/Kconfig
+++ b/arch/arm/mach-tegra/Kconfig
@@ -2,6 +2,7 @@ config ARCH_TEGRA
 	bool "NVIDIA Tegra" if ARCH_MULTI_V7
 	select ARCH_HAS_CPUFREQ
 	select ARCH_REQUIRE_GPIOLIB
+	select ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
 	select CLKDEV_LOOKUP
 	select CLKSRC_MMIO
 	select CLKSRC_OF
diff --git a/arch/arm/mach-tegra/common.c b/arch/arm/mach-tegra/common.c
index 94a119a..b405e45 100644
--- a/arch/arm/mach-tegra/common.c
+++ b/arch/arm/mach-tegra/common.c
@@ -27,6 +27,7 @@
 #include <linux/clk-provider.h>
 
 #include <asm/hardware/cache-l2x0.h>
+#include <asm/trusted_foundations.h>
 
 #include "board.h"
 #include "common.h"
@@ -99,6 +100,7 @@ static void __init tegra_init_cache(void)
 
 void __init tegra_init_early(void)
 {
+	of_register_trusted_foundations();
 	tegra_cpu_reset_handler_init();
 	tegra_apb_io_init();
 	tegra_init_fuse();
-- 
1.8.4

[PATCH v5 3/5] ARM: tegra: split setting of CPU reset handler

[Alexandre Courbot]

Not all Tegra devices can set the CPU reset handler in the same way.
In particular, devices using a TrustZone secure monitor cannot set the
reset handler directly and need to do it through a firmware operation.

This patch separates the act of setting the reset handler from its
preparation, so the former can be implemented in a different way.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
---
 arch/arm/mach-tegra/reset.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-tegra/reset.c b/arch/arm/mach-tegra/reset.c
index 1ac434e..6964117 100644
--- a/arch/arm/mach-tegra/reset.c
+++ b/arch/arm/mach-tegra/reset.c
@@ -33,26 +33,18 @@
 
 static bool is_enabled;
 
-static void __init tegra_cpu_reset_handler_enable(void)
+static void __init tegra_cpu_reset_handler_set(const u32 reset_address)
 {
-	void __iomem *iram_base = IO_ADDRESS(TEGRA_IRAM_RESET_BASE);
 	void __iomem *evp_cpu_reset =
 		IO_ADDRESS(TEGRA_EXCEPTION_VECTORS_BASE + 0x100);
 	void __iomem *sb_ctrl = IO_ADDRESS(TEGRA_SB_BASE);
 	u32 reg;
 
-	BUG_ON(is_enabled);
-	BUG_ON(tegra_cpu_reset_handler_size > TEGRA_IRAM_RESET_HANDLER_SIZE);
-
-	memcpy(iram_base, (void *)__tegra_cpu_reset_handler_start,
-			tegra_cpu_reset_handler_size);
-
 	/*
 	 * NOTE: This must be the one and only write to the EVP CPU reset
 	 *       vector in the entire system.
 	 */
-	writel(TEGRA_IRAM_RESET_BASE + tegra_cpu_reset_handler_offset,
-			evp_cpu_reset);
+	writel(reset_address, evp_cpu_reset);
 	wmb();
 	reg = readl(evp_cpu_reset);
 
@@ -66,6 +58,21 @@ static void __init tegra_cpu_reset_handler_enable(void)
 		writel(reg, sb_ctrl);
 		wmb();
 	}
+}
+
+static void __init tegra_cpu_reset_handler_enable(void)
+{
+	void __iomem *iram_base = IO_ADDRESS(TEGRA_IRAM_RESET_BASE);
+	const u32 reset_address = TEGRA_IRAM_RESET_BASE +
+						tegra_cpu_reset_handler_offset;
+
+	BUG_ON(is_enabled);
+	BUG_ON(tegra_cpu_reset_handler_size > TEGRA_IRAM_RESET_HANDLER_SIZE);
+
+	memcpy(iram_base, (void *)__tegra_cpu_reset_handler_start,
+			tegra_cpu_reset_handler_size);
+
+	tegra_cpu_reset_handler_set(reset_address);
 
 	is_enabled = true;
 }
-- 
1.8.4

[PATCH v5 4/5] ARM: tegra: set CPU reset handler with firmware op

[Alexandre Courbot]

Use a firmware operation to set the CPU reset handler and only resort to
doing it ourselves if there is none defined.

This supports the booting of secondary CPUs on devices using a TrustZone
secure monitor.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Tomasz Figa <t.figa@samsung.com>
Reviewed-by: Stephen Warren <swarren@nvidia.com>
---
 arch/arm/mach-tegra/reset.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-tegra/reset.c b/arch/arm/mach-tegra/reset.c
index 6964117..fc9cf03 100644
--- a/arch/arm/mach-tegra/reset.c
+++ b/arch/arm/mach-tegra/reset.c
@@ -21,6 +21,7 @@
 
 #include <asm/cacheflush.h>
 #include <asm/hardware/cache-l2x0.h>
+#include <asm/firmware.h>
 
 #include "iomap.h"
 #include "irammap.h"
@@ -65,6 +66,7 @@ static void __init tegra_cpu_reset_handler_enable(void)
 	void __iomem *iram_base = IO_ADDRESS(TEGRA_IRAM_RESET_BASE);
 	const u32 reset_address = TEGRA_IRAM_RESET_BASE +
 						tegra_cpu_reset_handler_offset;
+	int err;
 
 	BUG_ON(is_enabled);
 	BUG_ON(tegra_cpu_reset_handler_size > TEGRA_IRAM_RESET_HANDLER_SIZE);
@@ -72,9 +74,18 @@ static void __init tegra_cpu_reset_handler_enable(void)
 	memcpy(iram_base, (void *)__tegra_cpu_reset_handler_start,
 			tegra_cpu_reset_handler_size);
 
-	tegra_cpu_reset_handler_set(reset_address);
-
-	is_enabled = true;
+	err = call_firmware_op(set_cpu_boot_addr, 0, reset_address);
+	switch (err) {
+	case -ENOSYS:
+		tegra_cpu_reset_handler_set(reset_address);
+		/* pass-through */
+	case 0:
+		is_enabled = true;
+		break;
+	default:
+		pr_crit("Cannot set CPU reset handler: %d\n", err);
+		BUG();
+	}
 }
 
 void __init tegra_cpu_reset_handler_init(void)
-- 
1.8.4

[PATCH v5 5/5] ARM: tegra: support Trusted Foundations by default

[Alexandre Courbot]

Support for Trusted Foundations is light and allows the kernel to run on
a wider range of devices, so enable it by default.

Signed-off-by: Alexandre Courbot <acourbot-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
Reviewed-by: Tomasz Figa <t.figa-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Reviewed-by: Stephen Warren <swarren-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 arch/arm/configs/tegra_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/configs/tegra_defconfig b/arch/arm/configs/tegra_defconfig
index 1effb43..90c90d9 100644
--- a/arch/arm/configs/tegra_defconfig
+++ b/arch/arm/configs/tegra_defconfig
@@ -28,6 +28,7 @@ CONFIG_ARCH_TEGRA_3x_SOC=y
 CONFIG_ARCH_TEGRA_114_SOC=y
 CONFIG_TEGRA_PCI=y
 CONFIG_TEGRA_EMC_SCALING_ENABLE=y
+CONFIG_TRUSTED_FOUNDATIONS=y
 CONFIG_SMP=y
 CONFIG_PREEMPT=y
 CONFIG_AEABI=y
-- 
1.8.4