[PATCH v3 0/5] ARM: tegra: support for Trusted Foundations

[PATCH v3 0/5] ARM: tegra: support for Trusted Foundations

[Alexandre Courbot]

This is the long-overdue third version of this series, hopefully addressing the 
suggestions received in v2.

Since I could confirm that the SMC interface used by Trusted Foundations is not 
Tegra-specific, I have moved core TF support into arch/arm/firmware as a 
proposal to consolidate all implementations of Tomasz Figa's firmware interface 
into that directory.

Changes from v2:
- Moved core TF support into arch/arm/firmware
- Added TF version number in DT bindings
- Cleaned TF registration code
- Tegra kernel now fails if TF is declared in the device tree but support
  is not compiled in
- Tegra kernel now fails if attempt to set CPU reset vector through secure
  monitor returned an error
- Moved update to tegra_defconfig into its own patch

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        | 16 ++++++
 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                          | 22 ++++++++
 arch/arm/firmware/Makefile                         |  1 +
 arch/arm/firmware/trusted_foundations.c            | 58 ++++++++++++++++++++++
 arch/arm/include/asm/trusted_foundations.h         | 36 ++++++++++++++
 arch/arm/mach-tegra/Kconfig                        |  1 +
 arch/arm/mach-tegra/Makefile                       |  1 +
 arch/arm/mach-tegra/common.c                       |  2 +
 arch/arm/mach-tegra/firmware.c                     | 41 +++++++++++++++
 arch/arm/mach-tegra/firmware.h                     | 19 +++++++
 arch/arm/mach-tegra/reset.c                        | 40 +++++++++++----
 16 files changed, 236 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
 create mode 100644 arch/arm/mach-tegra/firmware.c
 create mode 100644 arch/arm/mach-tegra/firmware.h

-- 
1.8.3.4

[PATCH v3 0/5] ARM: tegra: support for Trusted Foundations

[Alexandre Courbot]

This is the long-overdue third version of this series, hopefully addressing the 
suggestions received in v2.

Since I could confirm that the SMC interface used by Trusted Foundations is not 
Tegra-specific, I have moved core TF support into arch/arm/firmware as a 
proposal to consolidate all implementations of Tomasz Figa's firmware interface 
into that directory.

Changes from v2:
- Moved core TF support into arch/arm/firmware
- Added TF version number in DT bindings
- Cleaned TF registration code
- Tegra kernel now fails if TF is declared in the device tree but support
  is not compiled in
- Tegra kernel now fails if attempt to set CPU reset vector through secure
  monitor returned an error
- Moved update to tegra_defconfig into its own patch

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        | 16 ++++++
 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                          | 22 ++++++++
 arch/arm/firmware/Makefile                         |  1 +
 arch/arm/firmware/trusted_foundations.c            | 58 ++++++++++++++++++++++
 arch/arm/include/asm/trusted_foundations.h         | 36 ++++++++++++++
 arch/arm/mach-tegra/Kconfig                        |  1 +
 arch/arm/mach-tegra/Makefile                       |  1 +
 arch/arm/mach-tegra/common.c                       |  2 +
 arch/arm/mach-tegra/firmware.c                     | 41 +++++++++++++++
 arch/arm/mach-tegra/firmware.h                     | 19 +++++++
 arch/arm/mach-tegra/reset.c                        | 40 +++++++++++----
 16 files changed, 236 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
 create mode 100644 arch/arm/mach-tegra/firmware.c
 create mode 100644 arch/arm/mach-tegra/firmware.h

-- 
1.8.3.4

[PATCH v3 0/5] ARM: tegra: support for Trusted Foundations

[Alexandre Courbot]

This is the long-overdue third version of this series, hopefully addressing the 
suggestions received in v2.

Since I could confirm that the SMC interface used by Trusted Foundations is not 
Tegra-specific, I have moved core TF support into arch/arm/firmware as a 
proposal to consolidate all implementations of Tomasz Figa's firmware interface 
into that directory.

Changes from v2:
- Moved core TF support into arch/arm/firmware
- Added TF version number in DT bindings
- Cleaned TF registration code
- Tegra kernel now fails if TF is declared in the device tree but support
  is not compiled in
- Tegra kernel now fails if attempt to set CPU reset vector through secure
  monitor returned an error
- Moved update to tegra_defconfig into its own patch

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        | 16 ++++++
 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                          | 22 ++++++++
 arch/arm/firmware/Makefile                         |  1 +
 arch/arm/firmware/trusted_foundations.c            | 58 ++++++++++++++++++++++
 arch/arm/include/asm/trusted_foundations.h         | 36 ++++++++++++++
 arch/arm/mach-tegra/Kconfig                        |  1 +
 arch/arm/mach-tegra/Makefile                       |  1 +
 arch/arm/mach-tegra/common.c                       |  2 +
 arch/arm/mach-tegra/firmware.c                     | 41 +++++++++++++++
 arch/arm/mach-tegra/firmware.h                     | 19 +++++++
 arch/arm/mach-tegra/reset.c                        | 40 +++++++++++----
 16 files changed, 236 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
 create mode 100644 arch/arm/mach-tegra/firmware.c
 create mode 100644 arch/arm/mach-tegra/firmware.h

-- 
1.8.3.4

[PATCH v3 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>
---
 .../arm/firmware/tl,trusted-foundations.txt        | 16 ++++++
 .../devicetree/bindings/vendor-prefixes.txt        |  1 +
 arch/arm/Kconfig                                   |  2 +
 arch/arm/Makefile                                  |  1 +
 arch/arm/firmware/Kconfig                          | 22 ++++++++
 arch/arm/firmware/Makefile                         |  1 +
 arch/arm/firmware/trusted_foundations.c            | 58 ++++++++++++++++++++++
 arch/arm/include/asm/trusted_foundations.h         | 36 ++++++++++++++
 8 files changed, 137 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..2609f78
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
@@ -0,0 +1,16 @@
+Trusted Foundations
+
+Boards that use the Trusted Foundations secure monitor can signal its
+presence by declaring a node compatible with "tl,trusted-foundations"
+(the name and location of the node does not matter).
+
+Required properties:
+- compatible : "tl,trusted-foundations"
+- version : Must contain the version number string of the Trusted Foundation
+	firmware.
+
+Example:
+	firmware {
+		compatible = "tl,trusted-foundations";
+		version = "02.08";
+	};
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..ad0eb6c
--- /dev/null
+++ b/arch/arm/firmware/Kconfig
@@ -0,0 +1,22 @@
+config ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
+	bool
+
+menu "Firmware options"
+	depends on ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
+
+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.
+
+	  Devices using Trusted Foundations should pass a device tree containing
+	  a node compatible with "tl,trusted-foundations" to signal the presence
+	  of the secure monitor.
+
+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..051482d
--- /dev/null
+++ b/arch/arm/firmware/trusted_foundations.c
@@ -0,0 +1,58 @@
+/*
+ * 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>
+
+#define TF_SET_CPU_BOOT_ADDR_SMC 0xfffff200
+
+static void __attribute__((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(const char *version)
+{
+	/* we are not using version information for now since currently
+	 * supported SMCs are compatible with all TF releases */
+	register_firmware_ops(&trusted_foundations_ops);
+}
diff --git a/arch/arm/include/asm/trusted_foundations.h b/arch/arm/include/asm/trusted_foundations.h
new file mode 100644
index 0000000..d187905
--- /dev/null
+++ b/arch/arm/include/asm/trusted_foundations.h
@@ -0,0 +1,36 @@
+/*
+ * 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>
+#include <linux/printk.h>
+#include <asm/bug.h>
+
+#if IS_ENABLED(CONFIG_TRUSTED_FOUNDATIONS)
+
+void register_trusted_foundations(const char *version);
+
+#else
+
+static inline void register_trusted_foundations(const char *version)
+{
+	pr_crit("No support for Trusted Foundations, stopping...\n");
+	BUG();
+}
+
+#endif /* IS_ENABLED(CONFIG_TRUSTED_FOUNDATIONS) */
+
+#endif
-- 
1.8.3.4

[PATCH v3 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>
---
 .../arm/firmware/tl,trusted-foundations.txt        | 16 ++++++
 .../devicetree/bindings/vendor-prefixes.txt        |  1 +
 arch/arm/Kconfig                                   |  2 +
 arch/arm/Makefile                                  |  1 +
 arch/arm/firmware/Kconfig                          | 22 ++++++++
 arch/arm/firmware/Makefile                         |  1 +
 arch/arm/firmware/trusted_foundations.c            | 58 ++++++++++++++++++++++
 arch/arm/include/asm/trusted_foundations.h         | 36 ++++++++++++++
 8 files changed, 137 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..2609f78
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
@@ -0,0 +1,16 @@
+Trusted Foundations
+
+Boards that use the Trusted Foundations secure monitor can signal its
+presence by declaring a node compatible with "tl,trusted-foundations"
+(the name and location of the node does not matter).
+
+Required properties:
+- compatible : "tl,trusted-foundations"
+- version : Must contain the version number string of the Trusted Foundation
+	firmware.
+
+Example:
+	firmware {
+		compatible = "tl,trusted-foundations";
+		version = "02.08";
+	};
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..ad0eb6c
--- /dev/null
+++ b/arch/arm/firmware/Kconfig
@@ -0,0 +1,22 @@
+config ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
+	bool
+
+menu "Firmware options"
+	depends on ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
+
+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.
+
+	  Devices using Trusted Foundations should pass a device tree containing
+	  a node compatible with "tl,trusted-foundations" to signal the presence
+	  of the secure monitor.
+
+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..051482d
--- /dev/null
+++ b/arch/arm/firmware/trusted_foundations.c
@@ -0,0 +1,58 @@
+/*
+ * 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>
+
+#define TF_SET_CPU_BOOT_ADDR_SMC 0xfffff200
+
+static void __attribute__((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(const char *version)
+{
+	/* we are not using version information for now since currently
+	 * supported SMCs are compatible with all TF releases */
+	register_firmware_ops(&trusted_foundations_ops);
+}
diff --git a/arch/arm/include/asm/trusted_foundations.h b/arch/arm/include/asm/trusted_foundations.h
new file mode 100644
index 0000000..d187905
--- /dev/null
+++ b/arch/arm/include/asm/trusted_foundations.h
@@ -0,0 +1,36 @@
+/*
+ * 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>
+#include <linux/printk.h>
+#include <asm/bug.h>
+
+#if IS_ENABLED(CONFIG_TRUSTED_FOUNDATIONS)
+
+void register_trusted_foundations(const char *version);
+
+#else
+
+static inline void register_trusted_foundations(const char *version)
+{
+	pr_crit("No support for Trusted Foundations, stopping...\n");
+	BUG();
+}
+
+#endif /* IS_ENABLED(CONFIG_TRUSTED_FOUNDATIONS) */
+
+#endif
-- 
1.8.3.4

[PATCH v3 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>
---
 .../arm/firmware/tl,trusted-foundations.txt        | 16 ++++++
 .../devicetree/bindings/vendor-prefixes.txt        |  1 +
 arch/arm/Kconfig                                   |  2 +
 arch/arm/Makefile                                  |  1 +
 arch/arm/firmware/Kconfig                          | 22 ++++++++
 arch/arm/firmware/Makefile                         |  1 +
 arch/arm/firmware/trusted_foundations.c            | 58 ++++++++++++++++++++++
 arch/arm/include/asm/trusted_foundations.h         | 36 ++++++++++++++
 8 files changed, 137 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..2609f78
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
@@ -0,0 +1,16 @@
+Trusted Foundations
+
+Boards that use the Trusted Foundations secure monitor can signal its
+presence by declaring a node compatible with "tl,trusted-foundations"
+(the name and location of the node does not matter).
+
+Required properties:
+- compatible : "tl,trusted-foundations"
+- version : Must contain the version number string of the Trusted Foundation
+	firmware.
+
+Example:
+	firmware {
+		compatible = "tl,trusted-foundations";
+		version = "02.08";
+	};
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..ad0eb6c
--- /dev/null
+++ b/arch/arm/firmware/Kconfig
@@ -0,0 +1,22 @@
+config ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
+	bool
+
+menu "Firmware options"
+	depends on ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
+
+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.
+
+	  Devices using Trusted Foundations should pass a device tree containing
+	  a node compatible with "tl,trusted-foundations" to signal the presence
+	  of the secure monitor.
+
+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..051482d
--- /dev/null
+++ b/arch/arm/firmware/trusted_foundations.c
@@ -0,0 +1,58 @@
+/*
+ * 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>
+
+#define TF_SET_CPU_BOOT_ADDR_SMC 0xfffff200
+
+static void __attribute__((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(const char *version)
+{
+	/* we are not using version information for now since currently
+	 * supported SMCs are compatible with all TF releases */
+	register_firmware_ops(&trusted_foundations_ops);
+}
diff --git a/arch/arm/include/asm/trusted_foundations.h b/arch/arm/include/asm/trusted_foundations.h
new file mode 100644
index 0000000..d187905
--- /dev/null
+++ b/arch/arm/include/asm/trusted_foundations.h
@@ -0,0 +1,36 @@
+/*
+ * 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>
+#include <linux/printk.h>
+#include <asm/bug.h>
+
+#if IS_ENABLED(CONFIG_TRUSTED_FOUNDATIONS)
+
+void register_trusted_foundations(const char *version);
+
+#else
+
+static inline void register_trusted_foundations(const char *version)
+{
+	pr_crit("No support for Trusted Foundations, stopping...\n");
+	BUG();
+}
+
+#endif /* IS_ENABLED(CONFIG_TRUSTED_FOUNDATIONS) */
+
+#endif
-- 
1.8.3.4

[PATCH v3 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-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 Documentation/devicetree/bindings/arm/tegra.txt |  5 +++
 arch/arm/mach-tegra/Kconfig                     |  1 +
 arch/arm/mach-tegra/Makefile                    |  1 +
 arch/arm/mach-tegra/common.c                    |  2 ++
 arch/arm/mach-tegra/firmware.c                  | 41 +++++++++++++++++++++++++
 arch/arm/mach-tegra/firmware.h                  | 19 ++++++++++++
 6 files changed, 69 insertions(+)
 create mode 100644 arch/arm/mach-tegra/firmware.c
 create mode 100644 arch/arm/mach-tegra/firmware.h

diff --git a/Documentation/devicetree/bindings/arm/tegra.txt b/Documentation/devicetree/bindings/arm/tegra.txt
index ed9c853..5423f51 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 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/Makefile b/arch/arm/mach-tegra/Makefile
index 98b184e..5ae9afb 100644
--- a/arch/arm/mach-tegra/Makefile
+++ b/arch/arm/mach-tegra/Makefile
@@ -12,6 +12,7 @@ obj-y					+= pm.o
 obj-y					+= reset.o
 obj-y					+= reset-handler.o
 obj-y					+= sleep.o
+obj-y					+= firmware.o
 obj-y					+= tegra.o
 obj-$(CONFIG_CPU_IDLE)			+= cpuidle.o
 obj-$(CONFIG_ARCH_TEGRA_2x_SOC)		+= tegra20_speedo.o
diff --git a/arch/arm/mach-tegra/common.c b/arch/arm/mach-tegra/common.c
index 94a119a..6794536 100644
--- a/arch/arm/mach-tegra/common.c
+++ b/arch/arm/mach-tegra/common.c
@@ -39,6 +39,7 @@
 #include "sleep.h"
 #include "pm.h"
 #include "reset.h"
+#include "firmware.h"
 
 /*
  * Storage for debug-macro.S's state.
@@ -99,6 +100,7 @@ static void __init tegra_init_cache(void)
 
 void __init tegra_init_early(void)
 {
+	tegra_init_firmware();
 	tegra_cpu_reset_handler_init();
 	tegra_apb_io_init();
 	tegra_init_fuse();
diff --git a/arch/arm/mach-tegra/firmware.c b/arch/arm/mach-tegra/firmware.c
new file mode 100644
index 0000000..4ad9af6
--- /dev/null
+++ b/arch/arm/mach-tegra/firmware.c
@@ -0,0 +1,41 @@
+/*
+ * Firmware/Secure monitor support for Tegra 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/of.h>
+#include <asm/trusted_foundations.h>
+
+void __init tegra_init_firmware(void)
+{
+	struct device_node *node;
+
+	if (!of_have_populated_dt())
+		return;
+
+	node = of_find_compatible_node(NULL, NULL, "tl,trusted-foundations");
+	if (node) {
+		const char *tf_version;
+		int err;
+
+		err = of_property_read_string(node, "version", &tf_version);
+		if (err != 0) {
+			pr_crit("Cannot read Trusted Foundation version: %d\n",
+				err);
+			BUG();
+		} else {
+			register_trusted_foundations(tf_version);
+		}
+	}
+}
diff --git a/arch/arm/mach-tegra/firmware.h b/arch/arm/mach-tegra/firmware.h
new file mode 100644
index 0000000..77c62fb
--- /dev/null
+++ b/arch/arm/mach-tegra/firmware.h
@@ -0,0 +1,19 @@
+/*
+ * Copyright (c) 2013, NVIDIA Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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 __TEGRA_FIRMWARE_H
+#define __TEGRA_FIRMWARE_H
+
+void tegra_init_firmware(void);
+
+#endif
-- 
1.8.3.4

[PATCH v3 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>
---
 Documentation/devicetree/bindings/arm/tegra.txt |  5 +++
 arch/arm/mach-tegra/Kconfig                     |  1 +
 arch/arm/mach-tegra/Makefile                    |  1 +
 arch/arm/mach-tegra/common.c                    |  2 ++
 arch/arm/mach-tegra/firmware.c                  | 41 +++++++++++++++++++++++++
 arch/arm/mach-tegra/firmware.h                  | 19 ++++++++++++
 6 files changed, 69 insertions(+)
 create mode 100644 arch/arm/mach-tegra/firmware.c
 create mode 100644 arch/arm/mach-tegra/firmware.h

diff --git a/Documentation/devicetree/bindings/arm/tegra.txt b/Documentation/devicetree/bindings/arm/tegra.txt
index ed9c853..5423f51 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 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/Makefile b/arch/arm/mach-tegra/Makefile
index 98b184e..5ae9afb 100644
--- a/arch/arm/mach-tegra/Makefile
+++ b/arch/arm/mach-tegra/Makefile
@@ -12,6 +12,7 @@ obj-y					+= pm.o
 obj-y					+= reset.o
 obj-y					+= reset-handler.o
 obj-y					+= sleep.o
+obj-y					+= firmware.o
 obj-y					+= tegra.o
 obj-$(CONFIG_CPU_IDLE)			+= cpuidle.o
 obj-$(CONFIG_ARCH_TEGRA_2x_SOC)		+= tegra20_speedo.o
diff --git a/arch/arm/mach-tegra/common.c b/arch/arm/mach-tegra/common.c
index 94a119a..6794536 100644
--- a/arch/arm/mach-tegra/common.c
+++ b/arch/arm/mach-tegra/common.c
@@ -39,6 +39,7 @@
 #include "sleep.h"
 #include "pm.h"
 #include "reset.h"
+#include "firmware.h"
 
 /*
  * Storage for debug-macro.S's state.
@@ -99,6 +100,7 @@ static void __init tegra_init_cache(void)
 
 void __init tegra_init_early(void)
 {
+	tegra_init_firmware();
 	tegra_cpu_reset_handler_init();
 	tegra_apb_io_init();
 	tegra_init_fuse();
diff --git a/arch/arm/mach-tegra/firmware.c b/arch/arm/mach-tegra/firmware.c
new file mode 100644
index 0000000..4ad9af6
--- /dev/null
+++ b/arch/arm/mach-tegra/firmware.c
@@ -0,0 +1,41 @@
+/*
+ * Firmware/Secure monitor support for Tegra 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/of.h>
+#include <asm/trusted_foundations.h>
+
+void __init tegra_init_firmware(void)
+{
+	struct device_node *node;
+
+	if (!of_have_populated_dt())
+		return;
+
+	node = of_find_compatible_node(NULL, NULL, "tl,trusted-foundations");
+	if (node) {
+		const char *tf_version;
+		int err;
+
+		err = of_property_read_string(node, "version", &tf_version);
+		if (err != 0) {
+			pr_crit("Cannot read Trusted Foundation version: %d\n",
+				err);
+			BUG();
+		} else {
+			register_trusted_foundations(tf_version);
+		}
+	}
+}
diff --git a/arch/arm/mach-tegra/firmware.h b/arch/arm/mach-tegra/firmware.h
new file mode 100644
index 0000000..77c62fb
--- /dev/null
+++ b/arch/arm/mach-tegra/firmware.h
@@ -0,0 +1,19 @@
+/*
+ * Copyright (c) 2013, NVIDIA Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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 __TEGRA_FIRMWARE_H
+#define __TEGRA_FIRMWARE_H
+
+void tegra_init_firmware(void);
+
+#endif
-- 
1.8.3.4

[PATCH v3 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>
---
 Documentation/devicetree/bindings/arm/tegra.txt |  5 +++
 arch/arm/mach-tegra/Kconfig                     |  1 +
 arch/arm/mach-tegra/Makefile                    |  1 +
 arch/arm/mach-tegra/common.c                    |  2 ++
 arch/arm/mach-tegra/firmware.c                  | 41 +++++++++++++++++++++++++
 arch/arm/mach-tegra/firmware.h                  | 19 ++++++++++++
 6 files changed, 69 insertions(+)
 create mode 100644 arch/arm/mach-tegra/firmware.c
 create mode 100644 arch/arm/mach-tegra/firmware.h

diff --git a/Documentation/devicetree/bindings/arm/tegra.txt b/Documentation/devicetree/bindings/arm/tegra.txt
index ed9c853..5423f51 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 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/Makefile b/arch/arm/mach-tegra/Makefile
index 98b184e..5ae9afb 100644
--- a/arch/arm/mach-tegra/Makefile
+++ b/arch/arm/mach-tegra/Makefile
@@ -12,6 +12,7 @@ obj-y					+= pm.o
 obj-y					+= reset.o
 obj-y					+= reset-handler.o
 obj-y					+= sleep.o
+obj-y					+= firmware.o
 obj-y					+= tegra.o
 obj-$(CONFIG_CPU_IDLE)			+= cpuidle.o
 obj-$(CONFIG_ARCH_TEGRA_2x_SOC)		+= tegra20_speedo.o
diff --git a/arch/arm/mach-tegra/common.c b/arch/arm/mach-tegra/common.c
index 94a119a..6794536 100644
--- a/arch/arm/mach-tegra/common.c
+++ b/arch/arm/mach-tegra/common.c
@@ -39,6 +39,7 @@
 #include "sleep.h"
 #include "pm.h"
 #include "reset.h"
+#include "firmware.h"
 
 /*
  * Storage for debug-macro.S's state.
@@ -99,6 +100,7 @@ static void __init tegra_init_cache(void)
 
 void __init tegra_init_early(void)
 {
+	tegra_init_firmware();
 	tegra_cpu_reset_handler_init();
 	tegra_apb_io_init();
 	tegra_init_fuse();
diff --git a/arch/arm/mach-tegra/firmware.c b/arch/arm/mach-tegra/firmware.c
new file mode 100644
index 0000000..4ad9af6
--- /dev/null
+++ b/arch/arm/mach-tegra/firmware.c
@@ -0,0 +1,41 @@
+/*
+ * Firmware/Secure monitor support for Tegra 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/of.h>
+#include <asm/trusted_foundations.h>
+
+void __init tegra_init_firmware(void)
+{
+	struct device_node *node;
+
+	if (!of_have_populated_dt())
+		return;
+
+	node = of_find_compatible_node(NULL, NULL, "tl,trusted-foundations");
+	if (node) {
+		const char *tf_version;
+		int err;
+
+		err = of_property_read_string(node, "version", &tf_version);
+		if (err != 0) {
+			pr_crit("Cannot read Trusted Foundation version: %d\n",
+				err);
+			BUG();
+		} else {
+			register_trusted_foundations(tf_version);
+		}
+	}
+}
diff --git a/arch/arm/mach-tegra/firmware.h b/arch/arm/mach-tegra/firmware.h
new file mode 100644
index 0000000..77c62fb
--- /dev/null
+++ b/arch/arm/mach-tegra/firmware.h
@@ -0,0 +1,19 @@
+/*
+ * Copyright (c) 2013, NVIDIA Corporation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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 __TEGRA_FIRMWARE_H
+#define __TEGRA_FIRMWARE_H
+
+void tegra_init_firmware(void);
+
+#endif
-- 
1.8.3.4

[PATCH v3 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>
---
 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.3.4

[PATCH v3 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>
---
 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.3.4

[PATCH v3 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>
---
 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.3.4

[PATCH v3 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>
---
 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.3.4

[PATCH v3 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>
---
 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.3.4

[PATCH v3 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>
---
 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.3.4

[PATCH v3 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@nvidia.com>
---
 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.3.4

[PATCH v3 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@nvidia.com>
---
 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.3.4

[PATCH v3 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@nvidia.com>
---
 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.3.4

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

[Stephen Warren]

On 08/12/2013 08:29 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/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt

> +Trusted Foundations
> +
> +Boards that use the Trusted Foundations secure monitor can signal its
> +presence by declaring a node compatible with "tl,trusted-foundations"
> +(the name and location of the node does not matter).
> +
> +Required properties:
> +- compatible : "tl,trusted-foundations"

> +- version : Must contain the version number string of the Trusted Foundation
> +	firmware.

Can the version be queried at run-time from the firmware itself?

If not, I wonder if we shouldn't instead encode the version number into
the compatible value.

Some comments on the exact format of the version property would be
useful; from the example I assume it's "%02d.%02d" % (major_ver, minor_ver)?

> diff --git a/arch/arm/firmware/Kconfig b/arch/arm/firmware/Kconfig

> +config ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
> +	bool

Shouldn't that be "config ARCH_SUPPORTS_FIRMWARE", since presumably in
the future there will be more entries in the menu, and hence we want the
menu to appear if any of those entries are useful?

> +
> +menu "Firmware options"
> +	depends on ARCH_SUPPORTS_TRUSTED_FOUNDATIONS

Or perhaps that comment is more appropriate for that "depends".

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

[Stephen Warren]

On 08/12/2013 08:29 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/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt

> +Trusted Foundations
> +
> +Boards that use the Trusted Foundations secure monitor can signal its
> +presence by declaring a node compatible with "tl,trusted-foundations"
> +(the name and location of the node does not matter).
> +
> +Required properties:
> +- compatible : "tl,trusted-foundations"

> +- version : Must contain the version number string of the Trusted Foundation
> +	firmware.

Can the version be queried at run-time from the firmware itself?

If not, I wonder if we shouldn't instead encode the version number into
the compatible value.

Some comments on the exact format of the version property would be
useful; from the example I assume it's "%02d.%02d" % (major_ver, minor_ver)?

> diff --git a/arch/arm/firmware/Kconfig b/arch/arm/firmware/Kconfig

> +config ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
> +	bool

Shouldn't that be "config ARCH_SUPPORTS_FIRMWARE", since presumably in
the future there will be more entries in the menu, and hence we want the
menu to appear if any of those entries are useful?

> +
> +menu "Firmware options"
> +	depends on ARCH_SUPPORTS_TRUSTED_FOUNDATIONS

Or perhaps that comment is more appropriate for that "depends".

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

[Stephen Warren]

On 08/12/2013 08:29 PM, Alexandre Courbot wrote:
> Register the firmware operations for Trusted Foundations if the device
> tree indicates it is active on the device.

> diff --git a/arch/arm/mach-tegra/firmware.c b/arch/arm/mach-tegra/firmware.c

> +void __init tegra_init_firmware(void)
> +{
> +	struct device_node *node;
> +
> +	if (!of_have_populated_dt())
> +		return;

That's never false on Tegra, at least upstream.

> +	node = of_find_compatible_node(NULL, NULL, "tl,trusted-foundations");
> +	if (node) {
> +		const char *tf_version;
> +		int err;
> +
> +		err = of_property_read_string(node, "version", &tf_version);
> +		if (err != 0) {
> +			pr_crit("Cannot read Trusted Foundation version: %d\n",
> +				err);
> +			BUG();
> +		} else {
> +			register_trusted_foundations(tf_version);
> +		}
> +	}
> +}

I think most/all of that function will be common across all SoCs that
support the TF monitor. I think you want to move the code into
register_trusted_foundations() (or a function that calls that) in patch 1?

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

[Stephen Warren]

On 08/12/2013 08:29 PM, Alexandre Courbot wrote:
> Register the firmware operations for Trusted Foundations if the device
> tree indicates it is active on the device.

> diff --git a/arch/arm/mach-tegra/firmware.c b/arch/arm/mach-tegra/firmware.c

> +void __init tegra_init_firmware(void)
> +{
> +	struct device_node *node;
> +
> +	if (!of_have_populated_dt())
> +		return;

That's never false on Tegra, at least upstream.

> +	node = of_find_compatible_node(NULL, NULL, "tl,trusted-foundations");
> +	if (node) {
> +		const char *tf_version;
> +		int err;
> +
> +		err = of_property_read_string(node, "version", &tf_version);
> +		if (err != 0) {
> +			pr_crit("Cannot read Trusted Foundation version: %d\n",
> +				err);
> +			BUG();
> +		} else {
> +			register_trusted_foundations(tf_version);
> +		}
> +	}
> +}

I think most/all of that function will be common across all SoCs that
support the TF monitor. I think you want to move the code into
register_trusted_foundations() (or a function that calls that) in patch 1?

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

[Stephen Warren]

On 08/12/2013 08:29 PM, Alexandre Courbot wrote:
> 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.

> diff --git a/arch/arm/mach-tegra/reset.c b/arch/arm/mach-tegra/reset.c

> +	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();
> +	}

Instead of trying and failing, does it make sense to register
tegra_cpu_reset_handler_set() as the set_cpu_boot_addr firmware op when
there is no firmware present? That would simplify all call-sites of any
firmware op.

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

[Stephen Warren]

On 08/12/2013 08:29 PM, Alexandre Courbot wrote:
> 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.

> diff --git a/arch/arm/mach-tegra/reset.c b/arch/arm/mach-tegra/reset.c

> +	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();
> +	}

Instead of trying and failing, does it make sense to register
tegra_cpu_reset_handler_set() as the set_cpu_boot_addr firmware op when
there is no firmware present? That would simplify all call-sites of any
firmware op.

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

[Stephen Warren]

On 08/12/2013 08:29 PM, Alexandre Courbot wrote:
> 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.

> diff --git a/arch/arm/mach-tegra/reset.c b/arch/arm/mach-tegra/reset.c

> +	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();
> +	}

Instead of trying and failing, does it make sense to register
tegra_cpu_reset_handler_set() as the set_cpu_boot_addr firmware op when
there is no firmware present? That would simplify all call-sites of any
firmware op.

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

[Dave Martin]

On Tue, Aug 13, 2013 at 11:29:48AM +0900, 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@nvidia.com>
> ---
>  .../arm/firmware/tl,trusted-foundations.txt        | 16 ++++++
>  .../devicetree/bindings/vendor-prefixes.txt        |  1 +
>  arch/arm/Kconfig                                   |  2 +
>  arch/arm/Makefile                                  |  1 +
>  arch/arm/firmware/Kconfig                          | 22 ++++++++
>  arch/arm/firmware/Makefile                         |  1 +
>  arch/arm/firmware/trusted_foundations.c            | 58 ++++++++++++++++++++++
>  arch/arm/include/asm/trusted_foundations.h         | 36 ++++++++++++++
>  8 files changed, 137 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..2609f78
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
> @@ -0,0 +1,16 @@
> +Trusted Foundations
> +
> +Boards that use the Trusted Foundations secure monitor can signal its
> +presence by declaring a node compatible with "tl,trusted-foundations"
> +(the name and location of the node does not matter).

We shouldn't encougrage people to arrange DT nodes at random.

Unless there's a good reason not to, this should just be a child
of /, with the meaning that the firmware is callable (at least to
the point of discoverability) on all CPUs.

The meaning of the node should be left unspecified if it is in any
other location.

This is the convention already adopted for things like arm,psci (though
to be fair, the arm,psci binding documentation fails to mention that
also -- nonetheless, the node is placed in /).

> +
> +Required properties:
> +- compatible : "tl,trusted-foundations"
> +- version : Must contain the version number string of the Trusted Foundation
> +	firmware.

Are you sure there is no low-level way to probe vendor and version info?
If there is, then the DT should describe nothing except the fact that
the probe interface exists.

I also worry that two integrations on different SoCs might have the
same version number, yet still be different due to vendor-specific
features and options.

> +
> +Example:
> +	firmware {
> +		compatible = "tl,trusted-foundations";
> +		version = "02.08";
> +	};
> 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..ad0eb6c
> --- /dev/null
> +++ b/arch/arm/firmware/Kconfig
> @@ -0,0 +1,22 @@
> +config ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
> +	bool
> +
> +menu "Firmware options"
> +	depends on ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
> +
> +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.
> +
> +	  Devices using Trusted Foundations should pass a device tree containing
> +	  a node compatible with "tl,trusted-foundations" to signal the presence
> +	  of the secure monitor.
> +
> +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..051482d
> --- /dev/null
> +++ b/arch/arm/firmware/trusted_foundations.c
> @@ -0,0 +1,58 @@
> +/*
> + * 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>
> +
> +#define TF_SET_CPU_BOOT_ADDR_SMC 0xfffff200
> +

Better to use __naked instead of an explicit attribute.

> +static void __attribute__((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(const char *version)
> +{
> +	/* we are not using version information for now since currently
> +	 * supported SMCs are compatible with all TF releases */

I think it would be wise at least to sanity-check the version,
even if there's only one major version supported for now.

> +	register_firmware_ops(&trusted_foundations_ops);
> +}
> diff --git a/arch/arm/include/asm/trusted_foundations.h b/arch/arm/include/asm/trusted_foundations.h
> new file mode 100644
> index 0000000..d187905
> --- /dev/null
> +++ b/arch/arm/include/asm/trusted_foundations.h
> @@ -0,0 +1,36 @@
> +/*
> + * 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>
> +#include <linux/printk.h>
> +#include <asm/bug.h>
> +
> +#if IS_ENABLED(CONFIG_TRUSTED_FOUNDATIONS)
> +
> +void register_trusted_foundations(const char *version);
> +
> +#else
> +
> +static inline void register_trusted_foundations(const char *version)
> +{
> +	pr_crit("No support for Trusted Foundations, stopping...\n");
> +	BUG();
> +}
> +
> +#endif /* IS_ENABLED(CONFIG_TRUSTED_FOUNDATIONS) */
> +
> +#endif
> -- 
> 1.8.3.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Dave Martin]

On Tue, Aug 13, 2013 at 11:29:48AM +0900, 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@nvidia.com>
> ---
>  .../arm/firmware/tl,trusted-foundations.txt        | 16 ++++++
>  .../devicetree/bindings/vendor-prefixes.txt        |  1 +
>  arch/arm/Kconfig                                   |  2 +
>  arch/arm/Makefile                                  |  1 +
>  arch/arm/firmware/Kconfig                          | 22 ++++++++
>  arch/arm/firmware/Makefile                         |  1 +
>  arch/arm/firmware/trusted_foundations.c            | 58 ++++++++++++++++++++++
>  arch/arm/include/asm/trusted_foundations.h         | 36 ++++++++++++++
>  8 files changed, 137 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..2609f78
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
> @@ -0,0 +1,16 @@
> +Trusted Foundations
> +
> +Boards that use the Trusted Foundations secure monitor can signal its
> +presence by declaring a node compatible with "tl,trusted-foundations"
> +(the name and location of the node does not matter).

We shouldn't encougrage people to arrange DT nodes at random.

Unless there's a good reason not to, this should just be a child
of /, with the meaning that the firmware is callable (at least to
the point of discoverability) on all CPUs.

The meaning of the node should be left unspecified if it is in any
other location.

This is the convention already adopted for things like arm,psci (though
to be fair, the arm,psci binding documentation fails to mention that
also -- nonetheless, the node is placed in /).

> +
> +Required properties:
> +- compatible : "tl,trusted-foundations"
> +- version : Must contain the version number string of the Trusted Foundation
> +	firmware.

Are you sure there is no low-level way to probe vendor and version info?
If there is, then the DT should describe nothing except the fact that
the probe interface exists.

I also worry that two integrations on different SoCs might have the
same version number, yet still be different due to vendor-specific
features and options.

> +
> +Example:
> +	firmware {
> +		compatible = "tl,trusted-foundations";
> +		version = "02.08";
> +	};
> 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..ad0eb6c
> --- /dev/null
> +++ b/arch/arm/firmware/Kconfig
> @@ -0,0 +1,22 @@
> +config ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
> +	bool
> +
> +menu "Firmware options"
> +	depends on ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
> +
> +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.
> +
> +	  Devices using Trusted Foundations should pass a device tree containing
> +	  a node compatible with "tl,trusted-foundations" to signal the presence
> +	  of the secure monitor.
> +
> +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..051482d
> --- /dev/null
> +++ b/arch/arm/firmware/trusted_foundations.c
> @@ -0,0 +1,58 @@
> +/*
> + * 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>
> +
> +#define TF_SET_CPU_BOOT_ADDR_SMC 0xfffff200
> +

Better to use __naked instead of an explicit attribute.

> +static void __attribute__((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(const char *version)
> +{
> +	/* we are not using version information for now since currently
> +	 * supported SMCs are compatible with all TF releases */

I think it would be wise at least to sanity-check the version,
even if there's only one major version supported for now.

> +	register_firmware_ops(&trusted_foundations_ops);
> +}
> diff --git a/arch/arm/include/asm/trusted_foundations.h b/arch/arm/include/asm/trusted_foundations.h
> new file mode 100644
> index 0000000..d187905
> --- /dev/null
> +++ b/arch/arm/include/asm/trusted_foundations.h
> @@ -0,0 +1,36 @@
> +/*
> + * 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>
> +#include <linux/printk.h>
> +#include <asm/bug.h>
> +
> +#if IS_ENABLED(CONFIG_TRUSTED_FOUNDATIONS)
> +
> +void register_trusted_foundations(const char *version);
> +
> +#else
> +
> +static inline void register_trusted_foundations(const char *version)
> +{
> +	pr_crit("No support for Trusted Foundations, stopping...\n");
> +	BUG();
> +}
> +
> +#endif /* IS_ENABLED(CONFIG_TRUSTED_FOUNDATIONS) */
> +
> +#endif
> -- 
> 1.8.3.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Dave Martin]

On Tue, Aug 13, 2013 at 11:29:48AM +0900, 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@nvidia.com>
> ---
>  .../arm/firmware/tl,trusted-foundations.txt        | 16 ++++++
>  .../devicetree/bindings/vendor-prefixes.txt        |  1 +
>  arch/arm/Kconfig                                   |  2 +
>  arch/arm/Makefile                                  |  1 +
>  arch/arm/firmware/Kconfig                          | 22 ++++++++
>  arch/arm/firmware/Makefile                         |  1 +
>  arch/arm/firmware/trusted_foundations.c            | 58 ++++++++++++++++++++++
>  arch/arm/include/asm/trusted_foundations.h         | 36 ++++++++++++++
>  8 files changed, 137 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..2609f78
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
> @@ -0,0 +1,16 @@
> +Trusted Foundations
> +
> +Boards that use the Trusted Foundations secure monitor can signal its
> +presence by declaring a node compatible with "tl,trusted-foundations"
> +(the name and location of the node does not matter).

We shouldn't encougrage people to arrange DT nodes at random.

Unless there's a good reason not to, this should just be a child
of /, with the meaning that the firmware is callable (at least to
the point of discoverability) on all CPUs.

The meaning of the node should be left unspecified if it is in any
other location.

This is the convention already adopted for things like arm,psci (though
to be fair, the arm,psci binding documentation fails to mention that
also -- nonetheless, the node is placed in /).

> +
> +Required properties:
> +- compatible : "tl,trusted-foundations"
> +- version : Must contain the version number string of the Trusted Foundation
> +	firmware.

Are you sure there is no low-level way to probe vendor and version info?
If there is, then the DT should describe nothing except the fact that
the probe interface exists.

I also worry that two integrations on different SoCs might have the
same version number, yet still be different due to vendor-specific
features and options.

> +
> +Example:
> +	firmware {
> +		compatible = "tl,trusted-foundations";
> +		version = "02.08";
> +	};
> 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..ad0eb6c
> --- /dev/null
> +++ b/arch/arm/firmware/Kconfig
> @@ -0,0 +1,22 @@
> +config ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
> +	bool
> +
> +menu "Firmware options"
> +	depends on ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
> +
> +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.
> +
> +	  Devices using Trusted Foundations should pass a device tree containing
> +	  a node compatible with "tl,trusted-foundations" to signal the presence
> +	  of the secure monitor.
> +
> +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..051482d
> --- /dev/null
> +++ b/arch/arm/firmware/trusted_foundations.c
> @@ -0,0 +1,58 @@
> +/*
> + * 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>
> +
> +#define TF_SET_CPU_BOOT_ADDR_SMC 0xfffff200
> +

Better to use __naked instead of an explicit attribute.

> +static void __attribute__((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(const char *version)
> +{
> +	/* we are not using version information for now since currently
> +	 * supported SMCs are compatible with all TF releases */

I think it would be wise at least to sanity-check the version,
even if there's only one major version supported for now.

> +	register_firmware_ops(&trusted_foundations_ops);
> +}
> diff --git a/arch/arm/include/asm/trusted_foundations.h b/arch/arm/include/asm/trusted_foundations.h
> new file mode 100644
> index 0000000..d187905
> --- /dev/null
> +++ b/arch/arm/include/asm/trusted_foundations.h
> @@ -0,0 +1,36 @@
> +/*
> + * 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>
> +#include <linux/printk.h>
> +#include <asm/bug.h>
> +
> +#if IS_ENABLED(CONFIG_TRUSTED_FOUNDATIONS)
> +
> +void register_trusted_foundations(const char *version);
> +
> +#else
> +
> +static inline void register_trusted_foundations(const char *version)
> +{
> +	pr_crit("No support for Trusted Foundations, stopping...\n");
> +	BUG();
> +}
> +
> +#endif /* IS_ENABLED(CONFIG_TRUSTED_FOUNDATIONS) */
> +
> +#endif
> -- 
> 1.8.3.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Stephen Warren]

On 08/15/2013 05:52 AM, Dave Martin wrote:
> On Tue, Aug 13, 2013 at 11:29:48AM +0900, 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/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt

>> +Required properties:
>> +- compatible : "tl,trusted-foundations"
>> +- version : Must contain the version number string of the Trusted Foundation
>> +	firmware.
> 
> Are you sure there is no low-level way to probe vendor and version info?
> If there is, then the DT should describe nothing except the fact that
> the probe interface exists.
> 
> I also worry that two integrations on different SoCs might have the
> same version number, yet still be different due to vendor-specific
> features and options.

I would expect HW-specific compatible values also to be present in a DT.
For example, perhaps:

compatible = "tl,trusted-foundations-nvidia-shield",
"tl,trusted-foundations";

(nvidia vendor, shield board/implementation)

This would allow matching on the specific value
"tl,trusted-foundations-nvidia-shield" in the future if some quirking
was needed, but if this wasn't needed, drivers could just bind to the
generic "tl,trusted-foundations".

>> +- version : Must contain the version number string of the Trusted Foundation
>> +	firmware.
> 
> Are you sure there is no low-level way to probe vendor and version info?
> If there is, then the DT should describe nothing except the fact that
> the probe interface exists.
> 
> I also worry that two integrations on different SoCs might have the
> same version number, yet still be different due to vendor-specific
> features and options.

Talking of the version - if we do need to represent this in the DT, how
about 2 separate cells for major/minor version rather than encoding it
into a string? Then, no parsing would be required.

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

[Stephen Warren]

On 08/15/2013 05:52 AM, Dave Martin wrote:
> On Tue, Aug 13, 2013 at 11:29:48AM +0900, 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/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt

>> +Required properties:
>> +- compatible : "tl,trusted-foundations"
>> +- version : Must contain the version number string of the Trusted Foundation
>> +	firmware.
> 
> Are you sure there is no low-level way to probe vendor and version info?
> If there is, then the DT should describe nothing except the fact that
> the probe interface exists.
> 
> I also worry that two integrations on different SoCs might have the
> same version number, yet still be different due to vendor-specific
> features and options.

I would expect HW-specific compatible values also to be present in a DT.
For example, perhaps:

compatible = "tl,trusted-foundations-nvidia-shield",
"tl,trusted-foundations";

(nvidia vendor, shield board/implementation)

This would allow matching on the specific value
"tl,trusted-foundations-nvidia-shield" in the future if some quirking
was needed, but if this wasn't needed, drivers could just bind to the
generic "tl,trusted-foundations".

>> +- version : Must contain the version number string of the Trusted Foundation
>> +	firmware.
> 
> Are you sure there is no low-level way to probe vendor and version info?
> If there is, then the DT should describe nothing except the fact that
> the probe interface exists.
> 
> I also worry that two integrations on different SoCs might have the
> same version number, yet still be different due to vendor-specific
> features and options.

Talking of the version - if we do need to represent this in the DT, how
about 2 separate cells for major/minor version rather than encoding it
into a string? Then, no parsing would be required.

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

[Stephen Warren]

On 08/15/2013 05:52 AM, Dave Martin wrote:
> On Tue, Aug 13, 2013 at 11:29:48AM +0900, 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/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt

>> +Required properties:
>> +- compatible : "tl,trusted-foundations"
>> +- version : Must contain the version number string of the Trusted Foundation
>> +	firmware.
> 
> Are you sure there is no low-level way to probe vendor and version info?
> If there is, then the DT should describe nothing except the fact that
> the probe interface exists.
> 
> I also worry that two integrations on different SoCs might have the
> same version number, yet still be different due to vendor-specific
> features and options.

I would expect HW-specific compatible values also to be present in a DT.
For example, perhaps:

compatible = "tl,trusted-foundations-nvidia-shield",
"tl,trusted-foundations";

(nvidia vendor, shield board/implementation)

This would allow matching on the specific value
"tl,trusted-foundations-nvidia-shield" in the future if some quirking
was needed, but if this wasn't needed, drivers could just bind to the
generic "tl,trusted-foundations".

>> +- version : Must contain the version number string of the Trusted Foundation
>> +	firmware.
> 
> Are you sure there is no low-level way to probe vendor and version info?
> If there is, then the DT should describe nothing except the fact that
> the probe interface exists.
> 
> I also worry that two integrations on different SoCs might have the
> same version number, yet still be different due to vendor-specific
> features and options.

Talking of the version - if we do need to represent this in the DT, how
about 2 separate cells for major/minor version rather than encoding it
into a string? Then, no parsing would be required.

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

[Dave Martin]

On Thu, Aug 15, 2013 at 04:02:52PM -0600, Stephen Warren wrote:
> On 08/15/2013 05:52 AM, Dave Martin wrote:
> > On Tue, Aug 13, 2013 at 11:29:48AM +0900, 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/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
> 
> >> +Required properties:
> >> +- compatible : "tl,trusted-foundations"
> >> +- version : Must contain the version number string of the Trusted Foundation
> >> +	firmware.
> > 
> > Are you sure there is no low-level way to probe vendor and version info?
> > If there is, then the DT should describe nothing except the fact that
> > the probe interface exists.
> > 
> > I also worry that two integrations on different SoCs might have the
> > same version number, yet still be different due to vendor-specific
> > features and options.
> 
> I would expect HW-specific compatible values also to be present in a DT.
> For example, perhaps:
> 
> compatible = "tl,trusted-foundations-nvidia-shield",
> "tl,trusted-foundations";
> 
> (nvidia vendor, shield board/implementation)
> 
> This would allow matching on the specific value
> "tl,trusted-foundations-nvidia-shield" in the future if some quirking
> was needed, but if this wasn't needed, drivers could just bind to the
> generic "tl,trusted-foundations".

That seems reasonable *unless* there is a reliable way to obtain
a vendor ID from the SMC ABI directly, in which case we should just
use that.

One could debate whether the extra compatible string should have
"nvidia," or "tl," but the fact that "nvidia" is in the name at all
pretty much narrows it down.

> 
> >> +- version : Must contain the version number string of the Trusted Foundation
> >> +	firmware.
> > 
> > Are you sure there is no low-level way to probe vendor and version info?
> > If there is, then the DT should describe nothing except the fact that
> > the probe interface exists.
> > 
> > I also worry that two integrations on different SoCs might have the
> > same version number, yet still be different due to vendor-specific
> > features and options.
> 
> Talking of the version - if we do need to represent this in the DT, how
> about 2 separate cells for major/minor version rather than encoding it
> into a string? Then, no parsing would be required.

I think the key thing here is to match whatever TF's native notion of
version is.

If it's truly a string with specific comparison rules, we should leave it
a string and write code to examine it.  If it's a simple <major minor>
pair, then putting it in the DT in this form makes sense.

And, as you previosly suggested, if there's a reliable ABI for getting
the firmware to tell you its version number, we should just use that and
not describe the version number in the DT at all.

I'm not in a position to answer that one.

If we really have no idea, we'll have to go with a best guess, and
change the compatible string later if necessary.  But it would be better
to get it right if we can.


If TF has version/vendor probe interfaces but those might be missing/
broken/wrong, we could still specify "vendor" and "version" properties
which allow the probed values to be overridden.

Cheers
---Dave

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

[Dave Martin]

On Thu, Aug 15, 2013 at 04:02:52PM -0600, Stephen Warren wrote:
> On 08/15/2013 05:52 AM, Dave Martin wrote:
> > On Tue, Aug 13, 2013 at 11:29:48AM +0900, 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/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
> 
> >> +Required properties:
> >> +- compatible : "tl,trusted-foundations"
> >> +- version : Must contain the version number string of the Trusted Foundation
> >> +	firmware.
> > 
> > Are you sure there is no low-level way to probe vendor and version info?
> > If there is, then the DT should describe nothing except the fact that
> > the probe interface exists.
> > 
> > I also worry that two integrations on different SoCs might have the
> > same version number, yet still be different due to vendor-specific
> > features and options.
> 
> I would expect HW-specific compatible values also to be present in a DT.
> For example, perhaps:
> 
> compatible = "tl,trusted-foundations-nvidia-shield",
> "tl,trusted-foundations";
> 
> (nvidia vendor, shield board/implementation)
> 
> This would allow matching on the specific value
> "tl,trusted-foundations-nvidia-shield" in the future if some quirking
> was needed, but if this wasn't needed, drivers could just bind to the
> generic "tl,trusted-foundations".

That seems reasonable *unless* there is a reliable way to obtain
a vendor ID from the SMC ABI directly, in which case we should just
use that.

One could debate whether the extra compatible string should have
"nvidia," or "tl," but the fact that "nvidia" is in the name at all
pretty much narrows it down.

> 
> >> +- version : Must contain the version number string of the Trusted Foundation
> >> +	firmware.
> > 
> > Are you sure there is no low-level way to probe vendor and version info?
> > If there is, then the DT should describe nothing except the fact that
> > the probe interface exists.
> > 
> > I also worry that two integrations on different SoCs might have the
> > same version number, yet still be different due to vendor-specific
> > features and options.
> 
> Talking of the version - if we do need to represent this in the DT, how
> about 2 separate cells for major/minor version rather than encoding it
> into a string? Then, no parsing would be required.

I think the key thing here is to match whatever TF's native notion of
version is.

If it's truly a string with specific comparison rules, we should leave it
a string and write code to examine it.  If it's a simple <major minor>
pair, then putting it in the DT in this form makes sense.

And, as you previosly suggested, if there's a reliable ABI for getting
the firmware to tell you its version number, we should just use that and
not describe the version number in the DT at all.

I'm not in a position to answer that one.

If we really have no idea, we'll have to go with a best guess, and
change the compatible string later if necessary.  But it would be better
to get it right if we can.


If TF has version/vendor probe interfaces but those might be missing/
broken/wrong, we could still specify "vendor" and "version" properties
which allow the probed values to be overridden.

Cheers
---Dave

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

[Alexandre Courbot]

On Thu, Aug 15, 2013 at 6:38 AM, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
> On 08/12/2013 08:29 PM, Alexandre Courbot wrote:
>> Register the firmware operations for Trusted Foundations if the device
>> tree indicates it is active on the device.
>
>> diff --git a/arch/arm/mach-tegra/firmware.c b/arch/arm/mach-tegra/firmware.c
>
>> +void __init tegra_init_firmware(void)
>> +{
>> +     struct device_node *node;
>> +
>> +     if (!of_have_populated_dt())
>> +             return;
>
> That's never false on Tegra, at least upstream.

Will remove, thanks.

>> +     node = of_find_compatible_node(NULL, NULL, "tl,trusted-foundations");
>> +     if (node) {
>> +             const char *tf_version;
>> +             int err;
>> +
>> +             err = of_property_read_string(node, "version", &tf_version);
>> +             if (err != 0) {
>> +                     pr_crit("Cannot read Trusted Foundation version: %d\n",
>> +                             err);
>> +                     BUG();
>> +             } else {
>> +                     register_trusted_foundations(tf_version);
>> +             }
>> +     }
>> +}
>
> I think most/all of that function will be common across all SoCs that
> support the TF monitor. I think you want to move the code into
> register_trusted_foundations() (or a function that calls that) in patch 1?

Agreed. If we push the logic a bit further, we should also have some
platform parameters for non-DT users, along with an
of_register_trusted_foundations() for those who prefer DT.

Thanks,
Alex.

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

[Alexandre Courbot]

On Thu, Aug 15, 2013 at 6:38 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 08/12/2013 08:29 PM, Alexandre Courbot wrote:
>> Register the firmware operations for Trusted Foundations if the device
>> tree indicates it is active on the device.
>
>> diff --git a/arch/arm/mach-tegra/firmware.c b/arch/arm/mach-tegra/firmware.c
>
>> +void __init tegra_init_firmware(void)
>> +{
>> +     struct device_node *node;
>> +
>> +     if (!of_have_populated_dt())
>> +             return;
>
> That's never false on Tegra, at least upstream.

Will remove, thanks.

>> +     node = of_find_compatible_node(NULL, NULL, "tl,trusted-foundations");
>> +     if (node) {
>> +             const char *tf_version;
>> +             int err;
>> +
>> +             err = of_property_read_string(node, "version", &tf_version);
>> +             if (err != 0) {
>> +                     pr_crit("Cannot read Trusted Foundation version: %d\n",
>> +                             err);
>> +                     BUG();
>> +             } else {
>> +                     register_trusted_foundations(tf_version);
>> +             }
>> +     }
>> +}
>
> I think most/all of that function will be common across all SoCs that
> support the TF monitor. I think you want to move the code into
> register_trusted_foundations() (or a function that calls that) in patch 1?

Agreed. If we push the logic a bit further, we should also have some
platform parameters for non-DT users, along with an
of_register_trusted_foundations() for those who prefer DT.

Thanks,
Alex.

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

[Alexandre Courbot]

On Thu, Aug 15, 2013 at 6:38 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 08/12/2013 08:29 PM, Alexandre Courbot wrote:
>> Register the firmware operations for Trusted Foundations if the device
>> tree indicates it is active on the device.
>
>> diff --git a/arch/arm/mach-tegra/firmware.c b/arch/arm/mach-tegra/firmware.c
>
>> +void __init tegra_init_firmware(void)
>> +{
>> +     struct device_node *node;
>> +
>> +     if (!of_have_populated_dt())
>> +             return;
>
> That's never false on Tegra, at least upstream.

Will remove, thanks.

>> +     node = of_find_compatible_node(NULL, NULL, "tl,trusted-foundations");
>> +     if (node) {
>> +             const char *tf_version;
>> +             int err;
>> +
>> +             err = of_property_read_string(node, "version", &tf_version);
>> +             if (err != 0) {
>> +                     pr_crit("Cannot read Trusted Foundation version: %d\n",
>> +                             err);
>> +                     BUG();
>> +             } else {
>> +                     register_trusted_foundations(tf_version);
>> +             }
>> +     }
>> +}
>
> I think most/all of that function will be common across all SoCs that
> support the TF monitor. I think you want to move the code into
> register_trusted_foundations() (or a function that calls that) in patch 1?

Agreed. If we push the logic a bit further, we should also have some
platform parameters for non-DT users, along with an
of_register_trusted_foundations() for those who prefer DT.

Thanks,
Alex.

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

[Alexandre Courbot]

On Thu, Aug 15, 2013 at 6:40 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 08/12/2013 08:29 PM, Alexandre Courbot wrote:
>> 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.
>
>> diff --git a/arch/arm/mach-tegra/reset.c b/arch/arm/mach-tegra/reset.c
>
>> +     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();
>> +     }
>
> Instead of trying and failing, does it make sense to register
> tegra_cpu_reset_handler_set() as the set_cpu_boot_addr firmware op when
> there is no firmware present? That would simplify all call-sites of any
> firmware op.

We discussed that point in v2 already IIRC (that is some time ago).
The reason I did it this way is because I wanted to follow the way
Tomasz was using his interface with Exynos - it seemed appropriate
that all users of an interface use it the same way. But if you prefer
to use a "non-firmware" firmware_ops for Tegra, I have absolutely
nothing against this.

Thanks,
Alex.

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

[Alexandre Courbot]

On Thu, Aug 15, 2013 at 6:40 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 08/12/2013 08:29 PM, Alexandre Courbot wrote:
>> 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.
>
>> diff --git a/arch/arm/mach-tegra/reset.c b/arch/arm/mach-tegra/reset.c
>
>> +     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();
>> +     }
>
> Instead of trying and failing, does it make sense to register
> tegra_cpu_reset_handler_set() as the set_cpu_boot_addr firmware op when
> there is no firmware present? That would simplify all call-sites of any
> firmware op.

We discussed that point in v2 already IIRC (that is some time ago).
The reason I did it this way is because I wanted to follow the way
Tomasz was using his interface with Exynos - it seemed appropriate
that all users of an interface use it the same way. But if you prefer
to use a "non-firmware" firmware_ops for Tegra, I have absolutely
nothing against this.

Thanks,
Alex.

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

[Alexandre Courbot]

On Thu, Aug 15, 2013 at 6:40 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 08/12/2013 08:29 PM, Alexandre Courbot wrote:
>> 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.
>
>> diff --git a/arch/arm/mach-tegra/reset.c b/arch/arm/mach-tegra/reset.c
>
>> +     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();
>> +     }
>
> Instead of trying and failing, does it make sense to register
> tegra_cpu_reset_handler_set() as the set_cpu_boot_addr firmware op when
> there is no firmware present? That would simplify all call-sites of any
> firmware op.

We discussed that point in v2 already IIRC (that is some time ago).
The reason I did it this way is because I wanted to follow the way
Tomasz was using his interface with Exynos - it seemed appropriate
that all users of an interface use it the same way. But if you prefer
to use a "non-firmware" firmware_ops for Tegra, I have absolutely
nothing against this.

Thanks,
Alex.

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

[Alexandre Courbot]

On Thu, Aug 15, 2013 at 6:35 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 08/12/2013 08:29 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/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
>
>> +Trusted Foundations
>> +
>> +Boards that use the Trusted Foundations secure monitor can signal its
>> +presence by declaring a node compatible with "tl,trusted-foundations"
>> +(the name and location of the node does not matter).
>> +
>> +Required properties:
>> +- compatible : "tl,trusted-foundations"
>
>> +- version : Must contain the version number string of the Trusted Foundation
>> +     firmware.
>
> Can the version be queried at run-time from the firmware itself?

I'm afraid there is not, unfortunately. :/

> If not, I wonder if we shouldn't instead encode the version number into
> the compatible value.

Why? This would make the node harder to find and would also complicate
the case where a given firmware handler can support a given range of
versions (which is what I expect will happen).

> Some comments on the exact format of the version property would be
> useful; from the example I assume it's "%02d.%02d" % (major_ver, minor_ver)?

Agreed.

>> diff --git a/arch/arm/firmware/Kconfig b/arch/arm/firmware/Kconfig
>
>> +config ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
>> +     bool
>
> Shouldn't that be "config ARCH_SUPPORTS_FIRMWARE", since presumably in
> the future there will be more entries in the menu, and hence we want the
> menu to appear if any of those entries are useful?

The things is that because you support one firmware does not mean you
will support then all. Only some set of architectures will support
firmware X, and another set (maybe not inclusive) will support
firmware Y. We do not want to allow selecting firmware Y if the kernel
does not support any of the architectures that may make use of it.

>> +
>> +menu "Firmware options"
>> +     depends on ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
>
> Or perhaps that comment is more appropriate for that "depends".

Here the idea is to not show the "Firmware options" menu if the kernel
does not include support for any architecture that uses them. As more
firmwares get added, the depends line should expand into something
like "depends on ARCH_SUPPORTS_FIRMWARE_X || ARCH_SUPPORTS_FIRMWARE_Y
|| ...

Maybe there's a better way to express this?

Thanks,
Alex.

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

[Alexandre Courbot]

On Thu, Aug 15, 2013 at 6:35 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 08/12/2013 08:29 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/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
>
>> +Trusted Foundations
>> +
>> +Boards that use the Trusted Foundations secure monitor can signal its
>> +presence by declaring a node compatible with "tl,trusted-foundations"
>> +(the name and location of the node does not matter).
>> +
>> +Required properties:
>> +- compatible : "tl,trusted-foundations"
>
>> +- version : Must contain the version number string of the Trusted Foundation
>> +     firmware.
>
> Can the version be queried at run-time from the firmware itself?

I'm afraid there is not, unfortunately. :/

> If not, I wonder if we shouldn't instead encode the version number into
> the compatible value.

Why? This would make the node harder to find and would also complicate
the case where a given firmware handler can support a given range of
versions (which is what I expect will happen).

> Some comments on the exact format of the version property would be
> useful; from the example I assume it's "%02d.%02d" % (major_ver, minor_ver)?

Agreed.

>> diff --git a/arch/arm/firmware/Kconfig b/arch/arm/firmware/Kconfig
>
>> +config ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
>> +     bool
>
> Shouldn't that be "config ARCH_SUPPORTS_FIRMWARE", since presumably in
> the future there will be more entries in the menu, and hence we want the
> menu to appear if any of those entries are useful?

The things is that because you support one firmware does not mean you
will support then all. Only some set of architectures will support
firmware X, and another set (maybe not inclusive) will support
firmware Y. We do not want to allow selecting firmware Y if the kernel
does not support any of the architectures that may make use of it.

>> +
>> +menu "Firmware options"
>> +     depends on ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
>
> Or perhaps that comment is more appropriate for that "depends".

Here the idea is to not show the "Firmware options" menu if the kernel
does not include support for any architecture that uses them. As more
firmwares get added, the depends line should expand into something
like "depends on ARCH_SUPPORTS_FIRMWARE_X || ARCH_SUPPORTS_FIRMWARE_Y
|| ...

Maybe there's a better way to express this?

Thanks,
Alex.

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

[Alexandre Courbot]

On Thu, Aug 15, 2013 at 6:35 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
> On 08/12/2013 08:29 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/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
>
>> +Trusted Foundations
>> +
>> +Boards that use the Trusted Foundations secure monitor can signal its
>> +presence by declaring a node compatible with "tl,trusted-foundations"
>> +(the name and location of the node does not matter).
>> +
>> +Required properties:
>> +- compatible : "tl,trusted-foundations"
>
>> +- version : Must contain the version number string of the Trusted Foundation
>> +     firmware.
>
> Can the version be queried at run-time from the firmware itself?

I'm afraid there is not, unfortunately. :/

> If not, I wonder if we shouldn't instead encode the version number into
> the compatible value.

Why? This would make the node harder to find and would also complicate
the case where a given firmware handler can support a given range of
versions (which is what I expect will happen).

> Some comments on the exact format of the version property would be
> useful; from the example I assume it's "%02d.%02d" % (major_ver, minor_ver)?

Agreed.

>> diff --git a/arch/arm/firmware/Kconfig b/arch/arm/firmware/Kconfig
>
>> +config ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
>> +     bool
>
> Shouldn't that be "config ARCH_SUPPORTS_FIRMWARE", since presumably in
> the future there will be more entries in the menu, and hence we want the
> menu to appear if any of those entries are useful?

The things is that because you support one firmware does not mean you
will support then all. Only some set of architectures will support
firmware X, and another set (maybe not inclusive) will support
firmware Y. We do not want to allow selecting firmware Y if the kernel
does not support any of the architectures that may make use of it.

>> +
>> +menu "Firmware options"
>> +     depends on ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
>
> Or perhaps that comment is more appropriate for that "depends".

Here the idea is to not show the "Firmware options" menu if the kernel
does not include support for any architecture that uses them. As more
firmwares get added, the depends line should expand into something
like "depends on ARCH_SUPPORTS_FIRMWARE_X || ARCH_SUPPORTS_FIRMWARE_Y
|| ...

Maybe there's a better way to express this?

Thanks,
Alex.

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

[Alexandre Courbot]

On Thu, Aug 15, 2013 at 8:52 PM, Dave Martin <Dave.Martin-5wv7dgnIgG8@public.gmane.org> wrote:
> On Tue, Aug 13, 2013 at 11:29:48AM +0900, 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>
>> ---
>>  .../arm/firmware/tl,trusted-foundations.txt        | 16 ++++++
>>  .../devicetree/bindings/vendor-prefixes.txt        |  1 +
>>  arch/arm/Kconfig                                   |  2 +
>>  arch/arm/Makefile                                  |  1 +
>>  arch/arm/firmware/Kconfig                          | 22 ++++++++
>>  arch/arm/firmware/Makefile                         |  1 +
>>  arch/arm/firmware/trusted_foundations.c            | 58 ++++++++++++++++++++++
>>  arch/arm/include/asm/trusted_foundations.h         | 36 ++++++++++++++
>>  8 files changed, 137 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..2609f78
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
>> @@ -0,0 +1,16 @@
>> +Trusted Foundations
>> +
>> +Boards that use the Trusted Foundations secure monitor can signal its
>> +presence by declaring a node compatible with "tl,trusted-foundations"
>> +(the name and location of the node does not matter).
>
> We shouldn't encougrage people to arrange DT nodes at random.
>
> Unless there's a good reason not to, this should just be a child
> of /, with the meaning that the firmware is callable (at least to
> the point of discoverability) on all CPUs.
>
> The meaning of the node should be left unspecified if it is in any
> other location.
>
> This is the convention already adopted for things like arm,psci (though
> to be fair, the arm,psci binding documentation fails to mention that
> also -- nonetheless, the node is placed in /).

I agree the node should be a child of /. At the moment this is not
enforced at runtime though (because of the use of
of_find_compatible_node()), is it ok if this is just specified as a
rule in the bindings?

>> +
>> +Required properties:
>> +- compatible : "tl,trusted-foundations"
>> +- version : Must contain the version number string of the Trusted Foundation
>> +     firmware.
>
> Are you sure there is no low-level way to probe vendor and version info?
> If there is, then the DT should describe nothing except the fact that
> the probe interface exists.

I have looked for such a probing method myself and asked some people
who know better than me, and the answer was that there was none,
unfortunately. :(

> I also worry that two integrations on different SoCs might have the
> same version number, yet still be different due to vendor-specific
> features and options.

I also investigated that point (since it was raised for v2 as well)
and the answer was that for TF as least, the interface should not be
different across vendors.

>
>> +
>> +Example:
>> +     firmware {
>> +             compatible = "tl,trusted-foundations";
>> +             version = "02.08";
>> +     };
>> 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..ad0eb6c
>> --- /dev/null
>> +++ b/arch/arm/firmware/Kconfig
>> @@ -0,0 +1,22 @@
>> +config ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
>> +     bool
>> +
>> +menu "Firmware options"
>> +     depends on ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
>> +
>> +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.
>> +
>> +       Devices using Trusted Foundations should pass a device tree containing
>> +       a node compatible with "tl,trusted-foundations" to signal the presence
>> +       of the secure monitor.
>> +
>> +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..051482d
>> --- /dev/null
>> +++ b/arch/arm/firmware/trusted_foundations.c
>> @@ -0,0 +1,58 @@
>> +/*
>> + * 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>
>> +
>> +#define TF_SET_CPU_BOOT_ADDR_SMC 0xfffff200
>> +
>
> Better to use __naked instead of an explicit attribute.

Will fix, thanks.

>
>> +static void __attribute__((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(const char *version)
>> +{
>> +     /* we are not using version information for now since currently
>> +      * supported SMCs are compatible with all TF releases */
>
> I think it would be wise at least to sanity-check the version,
> even if there's only one major version supported for now.

Ok.

Thanks,
Alex.

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

[Alexandre Courbot]

On Thu, Aug 15, 2013 at 8:52 PM, Dave Martin <Dave.Martin@arm.com> wrote:
> On Tue, Aug 13, 2013 at 11:29:48AM +0900, 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@nvidia.com>
>> ---
>>  .../arm/firmware/tl,trusted-foundations.txt        | 16 ++++++
>>  .../devicetree/bindings/vendor-prefixes.txt        |  1 +
>>  arch/arm/Kconfig                                   |  2 +
>>  arch/arm/Makefile                                  |  1 +
>>  arch/arm/firmware/Kconfig                          | 22 ++++++++
>>  arch/arm/firmware/Makefile                         |  1 +
>>  arch/arm/firmware/trusted_foundations.c            | 58 ++++++++++++++++++++++
>>  arch/arm/include/asm/trusted_foundations.h         | 36 ++++++++++++++
>>  8 files changed, 137 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..2609f78
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
>> @@ -0,0 +1,16 @@
>> +Trusted Foundations
>> +
>> +Boards that use the Trusted Foundations secure monitor can signal its
>> +presence by declaring a node compatible with "tl,trusted-foundations"
>> +(the name and location of the node does not matter).
>
> We shouldn't encougrage people to arrange DT nodes at random.
>
> Unless there's a good reason not to, this should just be a child
> of /, with the meaning that the firmware is callable (at least to
> the point of discoverability) on all CPUs.
>
> The meaning of the node should be left unspecified if it is in any
> other location.
>
> This is the convention already adopted for things like arm,psci (though
> to be fair, the arm,psci binding documentation fails to mention that
> also -- nonetheless, the node is placed in /).

I agree the node should be a child of /. At the moment this is not
enforced at runtime though (because of the use of
of_find_compatible_node()), is it ok if this is just specified as a
rule in the bindings?

>> +
>> +Required properties:
>> +- compatible : "tl,trusted-foundations"
>> +- version : Must contain the version number string of the Trusted Foundation
>> +     firmware.
>
> Are you sure there is no low-level way to probe vendor and version info?
> If there is, then the DT should describe nothing except the fact that
> the probe interface exists.

I have looked for such a probing method myself and asked some people
who know better than me, and the answer was that there was none,
unfortunately. :(

> I also worry that two integrations on different SoCs might have the
> same version number, yet still be different due to vendor-specific
> features and options.

I also investigated that point (since it was raised for v2 as well)
and the answer was that for TF as least, the interface should not be
different across vendors.

>
>> +
>> +Example:
>> +     firmware {
>> +             compatible = "tl,trusted-foundations";
>> +             version = "02.08";
>> +     };
>> 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..ad0eb6c
>> --- /dev/null
>> +++ b/arch/arm/firmware/Kconfig
>> @@ -0,0 +1,22 @@
>> +config ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
>> +     bool
>> +
>> +menu "Firmware options"
>> +     depends on ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
>> +
>> +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.
>> +
>> +       Devices using Trusted Foundations should pass a device tree containing
>> +       a node compatible with "tl,trusted-foundations" to signal the presence
>> +       of the secure monitor.
>> +
>> +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..051482d
>> --- /dev/null
>> +++ b/arch/arm/firmware/trusted_foundations.c
>> @@ -0,0 +1,58 @@
>> +/*
>> + * 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>
>> +
>> +#define TF_SET_CPU_BOOT_ADDR_SMC 0xfffff200
>> +
>
> Better to use __naked instead of an explicit attribute.

Will fix, thanks.

>
>> +static void __attribute__((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(const char *version)
>> +{
>> +     /* we are not using version information for now since currently
>> +      * supported SMCs are compatible with all TF releases */
>
> I think it would be wise at least to sanity-check the version,
> even if there's only one major version supported for now.

Ok.

Thanks,
Alex.

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

[Alexandre Courbot]

On Thu, Aug 15, 2013 at 8:52 PM, Dave Martin <Dave.Martin@arm.com> wrote:
> On Tue, Aug 13, 2013 at 11:29:48AM +0900, 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@nvidia.com>
>> ---
>>  .../arm/firmware/tl,trusted-foundations.txt        | 16 ++++++
>>  .../devicetree/bindings/vendor-prefixes.txt        |  1 +
>>  arch/arm/Kconfig                                   |  2 +
>>  arch/arm/Makefile                                  |  1 +
>>  arch/arm/firmware/Kconfig                          | 22 ++++++++
>>  arch/arm/firmware/Makefile                         |  1 +
>>  arch/arm/firmware/trusted_foundations.c            | 58 ++++++++++++++++++++++
>>  arch/arm/include/asm/trusted_foundations.h         | 36 ++++++++++++++
>>  8 files changed, 137 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..2609f78
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
>> @@ -0,0 +1,16 @@
>> +Trusted Foundations
>> +
>> +Boards that use the Trusted Foundations secure monitor can signal its
>> +presence by declaring a node compatible with "tl,trusted-foundations"
>> +(the name and location of the node does not matter).
>
> We shouldn't encougrage people to arrange DT nodes at random.
>
> Unless there's a good reason not to, this should just be a child
> of /, with the meaning that the firmware is callable (at least to
> the point of discoverability) on all CPUs.
>
> The meaning of the node should be left unspecified if it is in any
> other location.
>
> This is the convention already adopted for things like arm,psci (though
> to be fair, the arm,psci binding documentation fails to mention that
> also -- nonetheless, the node is placed in /).

I agree the node should be a child of /. At the moment this is not
enforced at runtime though (because of the use of
of_find_compatible_node()), is it ok if this is just specified as a
rule in the bindings?

>> +
>> +Required properties:
>> +- compatible : "tl,trusted-foundations"
>> +- version : Must contain the version number string of the Trusted Foundation
>> +     firmware.
>
> Are you sure there is no low-level way to probe vendor and version info?
> If there is, then the DT should describe nothing except the fact that
> the probe interface exists.

I have looked for such a probing method myself and asked some people
who know better than me, and the answer was that there was none,
unfortunately. :(

> I also worry that two integrations on different SoCs might have the
> same version number, yet still be different due to vendor-specific
> features and options.

I also investigated that point (since it was raised for v2 as well)
and the answer was that for TF as least, the interface should not be
different across vendors.

>
>> +
>> +Example:
>> +     firmware {
>> +             compatible = "tl,trusted-foundations";
>> +             version = "02.08";
>> +     };
>> 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..ad0eb6c
>> --- /dev/null
>> +++ b/arch/arm/firmware/Kconfig
>> @@ -0,0 +1,22 @@
>> +config ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
>> +     bool
>> +
>> +menu "Firmware options"
>> +     depends on ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
>> +
>> +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.
>> +
>> +       Devices using Trusted Foundations should pass a device tree containing
>> +       a node compatible with "tl,trusted-foundations" to signal the presence
>> +       of the secure monitor.
>> +
>> +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..051482d
>> --- /dev/null
>> +++ b/arch/arm/firmware/trusted_foundations.c
>> @@ -0,0 +1,58 @@
>> +/*
>> + * 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>
>> +
>> +#define TF_SET_CPU_BOOT_ADDR_SMC 0xfffff200
>> +
>
> Better to use __naked instead of an explicit attribute.

Will fix, thanks.

>
>> +static void __attribute__((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(const char *version)
>> +{
>> +     /* we are not using version information for now since currently
>> +      * supported SMCs are compatible with all TF releases */
>
> I think it would be wise at least to sanity-check the version,
> even if there's only one major version supported for now.

Ok.

Thanks,
Alex.

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

[Alexandre Courbot]

On Fri, Aug 16, 2013 at 10:23 PM, Dave Martin <Dave.Martin-5wv7dgnIgG8@public.gmane.org> wrote:
> On Thu, Aug 15, 2013 at 04:02:52PM -0600, Stephen Warren wrote:
>> On 08/15/2013 05:52 AM, Dave Martin wrote:
>> > On Tue, Aug 13, 2013 at 11:29:48AM +0900, 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/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
>>
>> >> +Required properties:
>> >> +- compatible : "tl,trusted-foundations"
>> >> +- version : Must contain the version number string of the Trusted Foundation
>> >> +  firmware.
>> >
>> > Are you sure there is no low-level way to probe vendor and version info?
>> > If there is, then the DT should describe nothing except the fact that
>> > the probe interface exists.
>> >
>> > I also worry that two integrations on different SoCs might have the
>> > same version number, yet still be different due to vendor-specific
>> > features and options.
>>
>> I would expect HW-specific compatible values also to be present in a DT.
>> For example, perhaps:
>>
>> compatible = "tl,trusted-foundations-nvidia-shield",
>> "tl,trusted-foundations";
>>
>> (nvidia vendor, shield board/implementation)
>>
>> This would allow matching on the specific value
>> "tl,trusted-foundations-nvidia-shield" in the future if some quirking
>> was needed, but if this wasn't needed, drivers could just bind to the
>> generic "tl,trusted-foundations".
>
> That seems reasonable *unless* there is a reliable way to obtain
> a vendor ID from the SMC ABI directly, in which case we should just
> use that.
>
> One could debate whether the extra compatible string should have
> "nvidia," or "tl," but the fact that "nvidia" is in the name at all
> pretty much narrows it down.
>
>>
>> >> +- version : Must contain the version number string of the Trusted Foundation
>> >> +  firmware.
>> >
>> > Are you sure there is no low-level way to probe vendor and version info?
>> > If there is, then the DT should describe nothing except the fact that
>> > the probe interface exists.
>> >
>> > I also worry that two integrations on different SoCs might have the
>> > same version number, yet still be different due to vendor-specific
>> > features and options.
>>
>> Talking of the version - if we do need to represent this in the DT, how
>> about 2 separate cells for major/minor version rather than encoding it
>> into a string? Then, no parsing would be required.
>
> I think the key thing here is to match whatever TF's native notion of
> version is.
>
> If it's truly a string with specific comparison rules, we should leave it
> a string and write code to examine it.  If it's a simple <major minor>
> pair, then putting it in the DT in this form makes sense.

TF's native version is this major/minor pair, and indeed it should be
sensible and harmless to turn it into a pair of cells.

Then there are other components that can be added to build a more
precise "version string". You can have a look at this file to see how
this is done:

http://nv-tegra.nvidia.com/gitweb/?p=linux-2.6.git;a=blob;f=security/tf_driver/s_version.h;h=d75c5f35d32d597b664c9533b1c5a52696e81b49;hb=rel-roth-ota-1

The version string I used to far is the S_VERSION_MAIN macro, since it
is the only one that seems to be platform-independant.
S_VERSION_PLATFORM and S_VERSION_OS can be used to indentify if
Android is used and which version. Then all these strings (plus a few
others) are concatenated to build S_VERSION_STRING which is the
ultimate version, but also considerably more complicated to handle.

At the current level of support, I don't think it makes sense to look
further than S_VERSION_MAIN for the moment. If we switch it to a pair
of integer cells as Stephen suggested, nothing prevents us to extend
the bindings with other properties (for instance version-platform and
version-os) if they become necessary. But at the moment I don't think
it makes sense to overthink versions beyond the main version number,
especially if it can be extended safely if needed in the future.

Thanks,
Alex.

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

[Alexandre Courbot]

On Fri, Aug 16, 2013 at 10:23 PM, Dave Martin <Dave.Martin@arm.com> wrote:
> On Thu, Aug 15, 2013 at 04:02:52PM -0600, Stephen Warren wrote:
>> On 08/15/2013 05:52 AM, Dave Martin wrote:
>> > On Tue, Aug 13, 2013 at 11:29:48AM +0900, 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/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
>>
>> >> +Required properties:
>> >> +- compatible : "tl,trusted-foundations"
>> >> +- version : Must contain the version number string of the Trusted Foundation
>> >> +  firmware.
>> >
>> > Are you sure there is no low-level way to probe vendor and version info?
>> > If there is, then the DT should describe nothing except the fact that
>> > the probe interface exists.
>> >
>> > I also worry that two integrations on different SoCs might have the
>> > same version number, yet still be different due to vendor-specific
>> > features and options.
>>
>> I would expect HW-specific compatible values also to be present in a DT.
>> For example, perhaps:
>>
>> compatible = "tl,trusted-foundations-nvidia-shield",
>> "tl,trusted-foundations";
>>
>> (nvidia vendor, shield board/implementation)
>>
>> This would allow matching on the specific value
>> "tl,trusted-foundations-nvidia-shield" in the future if some quirking
>> was needed, but if this wasn't needed, drivers could just bind to the
>> generic "tl,trusted-foundations".
>
> That seems reasonable *unless* there is a reliable way to obtain
> a vendor ID from the SMC ABI directly, in which case we should just
> use that.
>
> One could debate whether the extra compatible string should have
> "nvidia," or "tl," but the fact that "nvidia" is in the name at all
> pretty much narrows it down.
>
>>
>> >> +- version : Must contain the version number string of the Trusted Foundation
>> >> +  firmware.
>> >
>> > Are you sure there is no low-level way to probe vendor and version info?
>> > If there is, then the DT should describe nothing except the fact that
>> > the probe interface exists.
>> >
>> > I also worry that two integrations on different SoCs might have the
>> > same version number, yet still be different due to vendor-specific
>> > features and options.
>>
>> Talking of the version - if we do need to represent this in the DT, how
>> about 2 separate cells for major/minor version rather than encoding it
>> into a string? Then, no parsing would be required.
>
> I think the key thing here is to match whatever TF's native notion of
> version is.
>
> If it's truly a string with specific comparison rules, we should leave it
> a string and write code to examine it.  If it's a simple <major minor>
> pair, then putting it in the DT in this form makes sense.

TF's native version is this major/minor pair, and indeed it should be
sensible and harmless to turn it into a pair of cells.

Then there are other components that can be added to build a more
precise "version string". You can have a look at this file to see how
this is done:

http://nv-tegra.nvidia.com/gitweb/?p=linux-2.6.git;a=blob;f=security/tf_driver/s_version.h;h=d75c5f35d32d597b664c9533b1c5a52696e81b49;hb=rel-roth-ota-1

The version string I used to far is the S_VERSION_MAIN macro, since it
is the only one that seems to be platform-independant.
S_VERSION_PLATFORM and S_VERSION_OS can be used to indentify if
Android is used and which version. Then all these strings (plus a few
others) are concatenated to build S_VERSION_STRING which is the
ultimate version, but also considerably more complicated to handle.

At the current level of support, I don't think it makes sense to look
further than S_VERSION_MAIN for the moment. If we switch it to a pair
of integer cells as Stephen suggested, nothing prevents us to extend
the bindings with other properties (for instance version-platform and
version-os) if they become necessary. But at the moment I don't think
it makes sense to overthink versions beyond the main version number,
especially if it can be extended safely if needed in the future.

Thanks,
Alex.

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

[Alexandre Courbot]

On Fri, Aug 16, 2013 at 10:23 PM, Dave Martin <Dave.Martin@arm.com> wrote:
> On Thu, Aug 15, 2013 at 04:02:52PM -0600, Stephen Warren wrote:
>> On 08/15/2013 05:52 AM, Dave Martin wrote:
>> > On Tue, Aug 13, 2013 at 11:29:48AM +0900, 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/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
>>
>> >> +Required properties:
>> >> +- compatible : "tl,trusted-foundations"
>> >> +- version : Must contain the version number string of the Trusted Foundation
>> >> +  firmware.
>> >
>> > Are you sure there is no low-level way to probe vendor and version info?
>> > If there is, then the DT should describe nothing except the fact that
>> > the probe interface exists.
>> >
>> > I also worry that two integrations on different SoCs might have the
>> > same version number, yet still be different due to vendor-specific
>> > features and options.
>>
>> I would expect HW-specific compatible values also to be present in a DT.
>> For example, perhaps:
>>
>> compatible = "tl,trusted-foundations-nvidia-shield",
>> "tl,trusted-foundations";
>>
>> (nvidia vendor, shield board/implementation)
>>
>> This would allow matching on the specific value
>> "tl,trusted-foundations-nvidia-shield" in the future if some quirking
>> was needed, but if this wasn't needed, drivers could just bind to the
>> generic "tl,trusted-foundations".
>
> That seems reasonable *unless* there is a reliable way to obtain
> a vendor ID from the SMC ABI directly, in which case we should just
> use that.
>
> One could debate whether the extra compatible string should have
> "nvidia," or "tl," but the fact that "nvidia" is in the name at all
> pretty much narrows it down.
>
>>
>> >> +- version : Must contain the version number string of the Trusted Foundation
>> >> +  firmware.
>> >
>> > Are you sure there is no low-level way to probe vendor and version info?
>> > If there is, then the DT should describe nothing except the fact that
>> > the probe interface exists.
>> >
>> > I also worry that two integrations on different SoCs might have the
>> > same version number, yet still be different due to vendor-specific
>> > features and options.
>>
>> Talking of the version - if we do need to represent this in the DT, how
>> about 2 separate cells for major/minor version rather than encoding it
>> into a string? Then, no parsing would be required.
>
> I think the key thing here is to match whatever TF's native notion of
> version is.
>
> If it's truly a string with specific comparison rules, we should leave it
> a string and write code to examine it.  If it's a simple <major minor>
> pair, then putting it in the DT in this form makes sense.

TF's native version is this major/minor pair, and indeed it should be
sensible and harmless to turn it into a pair of cells.

Then there are other components that can be added to build a more
precise "version string". You can have a look at this file to see how
this is done:

http://nv-tegra.nvidia.com/gitweb/?p=linux-2.6.git;a=blob;f=security/tf_driver/s_version.h;h=d75c5f35d32d597b664c9533b1c5a52696e81b49;hb=rel-roth-ota-1

The version string I used to far is the S_VERSION_MAIN macro, since it
is the only one that seems to be platform-independant.
S_VERSION_PLATFORM and S_VERSION_OS can be used to indentify if
Android is used and which version. Then all these strings (plus a few
others) are concatenated to build S_VERSION_STRING which is the
ultimate version, but also considerably more complicated to handle.

At the current level of support, I don't think it makes sense to look
further than S_VERSION_MAIN for the moment. If we switch it to a pair
of integer cells as Stephen suggested, nothing prevents us to extend
the bindings with other properties (for instance version-platform and
version-os) if they become necessary. But at the moment I don't think
it makes sense to overthink versions beyond the main version number,
especially if it can be extended safely if needed in the future.

Thanks,
Alex.

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

[Stephen Warren]

On 08/18/2013 02:37 AM, Alexandre Courbot wrote:
> On Thu, Aug 15, 2013 at 6:35 AM, Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org> wrote:
>> On 08/12/2013 08:29 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/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
>>
>>> +Trusted Foundations
>>> +
>>> +Boards that use the Trusted Foundations secure monitor can signal its
>>> +presence by declaring a node compatible with "tl,trusted-foundations"
>>> +(the name and location of the node does not matter).
>>> +
>>> +Required properties:
>>> +- compatible : "tl,trusted-foundations"
>>
>>> +- version : Must contain the version number string of the Trusted Foundation
>>> +     firmware.
>>
>> Can the version be queried at run-time from the firmware itself?
> 
> I'm afraid there is not, unfortunately. :/
> 
>> If not, I wonder if we shouldn't instead encode the version number into
>> the compatible value.
> 
> Why? This would make the node harder to find and would also complicate
> the case where a given firmware handler can support a given range of
> versions (which is what I expect will happen).

I guess that would be painful. There are probably a lot more valid
version numbers for a SW firmware interface than there are for a HW IP
block interface, so it makes less sense to encoded the version into the
compatible value, at least in cases where the interface is "similar
enough"; we can add some kind of basic version number into the
compatible values for radically different interfaces if they appear in
the future.

>>> diff --git a/arch/arm/firmware/Kconfig b/arch/arm/firmware/Kconfig
>>
>>> +config ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
>>> +     bool
>>
>> Shouldn't that be "config ARCH_SUPPORTS_FIRMWARE", since presumably in
>> the future there will be more entries in the menu, and hence we want the
>> menu to appear if any of those entries are useful?
> 
> The things is that because you support one firmware does not mean you
> will support then all. Only some set of architectures will support
> firmware X, and another set (maybe not inclusive) will support
> firmware Y. We do not want to allow selecting firmware Y if the kernel
> does not support any of the architectures that may make use of it.
> 
>>> +
>>> +menu "Firmware options"
>>> +     depends on ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
>>
>> Or perhaps that comment is more appropriate for that "depends".
> 
> Here the idea is to not show the "Firmware options" menu if the kernel
> does not include support for any architecture that uses them. As more
> firmwares get added, the depends line should expand into something
> like "depends on ARCH_SUPPORTS_FIRMWARE_X || ARCH_SUPPORTS_FIRMWARE_Y
> || ...
> 
> Maybe there's a better way to express this?

Is there a need for a menu at all? I suppose it might group things
nicely. If so, how about:

config ARCH_SUPPORTS_FIRMWARE
	bool

config ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
	bool
	select ARCH_SUPPORTS_FIRMWARE

config ARCH_SUPPORTS_SOME_OTHER_FW
	bool
	select ARCH_SUPPORTS_FIRMWARE

menu "Firmware options"
	depends on ARCH_SUPPORTS_FIRMWARE

That's probably slightly easier to read than a huge "depends" statement
in the menu stanza, and ARCH_SUPPORTS_FIRMWARE might be a useful to
build in support for any core firmware utility code.

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

[Stephen Warren]

On 08/18/2013 02:37 AM, Alexandre Courbot wrote:
> On Thu, Aug 15, 2013 at 6:35 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 08/12/2013 08:29 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/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
>>
>>> +Trusted Foundations
>>> +
>>> +Boards that use the Trusted Foundations secure monitor can signal its
>>> +presence by declaring a node compatible with "tl,trusted-foundations"
>>> +(the name and location of the node does not matter).
>>> +
>>> +Required properties:
>>> +- compatible : "tl,trusted-foundations"
>>
>>> +- version : Must contain the version number string of the Trusted Foundation
>>> +     firmware.
>>
>> Can the version be queried at run-time from the firmware itself?
> 
> I'm afraid there is not, unfortunately. :/
> 
>> If not, I wonder if we shouldn't instead encode the version number into
>> the compatible value.
> 
> Why? This would make the node harder to find and would also complicate
> the case where a given firmware handler can support a given range of
> versions (which is what I expect will happen).

I guess that would be painful. There are probably a lot more valid
version numbers for a SW firmware interface than there are for a HW IP
block interface, so it makes less sense to encoded the version into the
compatible value, at least in cases where the interface is "similar
enough"; we can add some kind of basic version number into the
compatible values for radically different interfaces if they appear in
the future.

>>> diff --git a/arch/arm/firmware/Kconfig b/arch/arm/firmware/Kconfig
>>
>>> +config ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
>>> +     bool
>>
>> Shouldn't that be "config ARCH_SUPPORTS_FIRMWARE", since presumably in
>> the future there will be more entries in the menu, and hence we want the
>> menu to appear if any of those entries are useful?
> 
> The things is that because you support one firmware does not mean you
> will support then all. Only some set of architectures will support
> firmware X, and another set (maybe not inclusive) will support
> firmware Y. We do not want to allow selecting firmware Y if the kernel
> does not support any of the architectures that may make use of it.
> 
>>> +
>>> +menu "Firmware options"
>>> +     depends on ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
>>
>> Or perhaps that comment is more appropriate for that "depends".
> 
> Here the idea is to not show the "Firmware options" menu if the kernel
> does not include support for any architecture that uses them. As more
> firmwares get added, the depends line should expand into something
> like "depends on ARCH_SUPPORTS_FIRMWARE_X || ARCH_SUPPORTS_FIRMWARE_Y
> || ...
> 
> Maybe there's a better way to express this?

Is there a need for a menu at all? I suppose it might group things
nicely. If so, how about:

config ARCH_SUPPORTS_FIRMWARE
	bool

config ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
	bool
	select ARCH_SUPPORTS_FIRMWARE

config ARCH_SUPPORTS_SOME_OTHER_FW
	bool
	select ARCH_SUPPORTS_FIRMWARE

menu "Firmware options"
	depends on ARCH_SUPPORTS_FIRMWARE

That's probably slightly easier to read than a huge "depends" statement
in the menu stanza, and ARCH_SUPPORTS_FIRMWARE might be a useful to
build in support for any core firmware utility code.

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

[Stephen Warren]

On 08/18/2013 02:37 AM, Alexandre Courbot wrote:
> On Thu, Aug 15, 2013 at 6:35 AM, Stephen Warren <swarren@wwwdotorg.org> wrote:
>> On 08/12/2013 08:29 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/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
>>
>>> +Trusted Foundations
>>> +
>>> +Boards that use the Trusted Foundations secure monitor can signal its
>>> +presence by declaring a node compatible with "tl,trusted-foundations"
>>> +(the name and location of the node does not matter).
>>> +
>>> +Required properties:
>>> +- compatible : "tl,trusted-foundations"
>>
>>> +- version : Must contain the version number string of the Trusted Foundation
>>> +     firmware.
>>
>> Can the version be queried at run-time from the firmware itself?
> 
> I'm afraid there is not, unfortunately. :/
> 
>> If not, I wonder if we shouldn't instead encode the version number into
>> the compatible value.
> 
> Why? This would make the node harder to find and would also complicate
> the case where a given firmware handler can support a given range of
> versions (which is what I expect will happen).

I guess that would be painful. There are probably a lot more valid
version numbers for a SW firmware interface than there are for a HW IP
block interface, so it makes less sense to encoded the version into the
compatible value, at least in cases where the interface is "similar
enough"; we can add some kind of basic version number into the
compatible values for radically different interfaces if they appear in
the future.

>>> diff --git a/arch/arm/firmware/Kconfig b/arch/arm/firmware/Kconfig
>>
>>> +config ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
>>> +     bool
>>
>> Shouldn't that be "config ARCH_SUPPORTS_FIRMWARE", since presumably in
>> the future there will be more entries in the menu, and hence we want the
>> menu to appear if any of those entries are useful?
> 
> The things is that because you support one firmware does not mean you
> will support then all. Only some set of architectures will support
> firmware X, and another set (maybe not inclusive) will support
> firmware Y. We do not want to allow selecting firmware Y if the kernel
> does not support any of the architectures that may make use of it.
> 
>>> +
>>> +menu "Firmware options"
>>> +     depends on ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
>>
>> Or perhaps that comment is more appropriate for that "depends".
> 
> Here the idea is to not show the "Firmware options" menu if the kernel
> does not include support for any architecture that uses them. As more
> firmwares get added, the depends line should expand into something
> like "depends on ARCH_SUPPORTS_FIRMWARE_X || ARCH_SUPPORTS_FIRMWARE_Y
> || ...
> 
> Maybe there's a better way to express this?

Is there a need for a menu at all? I suppose it might group things
nicely. If so, how about:

config ARCH_SUPPORTS_FIRMWARE
	bool

config ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
	bool
	select ARCH_SUPPORTS_FIRMWARE

config ARCH_SUPPORTS_SOME_OTHER_FW
	bool
	select ARCH_SUPPORTS_FIRMWARE

menu "Firmware options"
	depends on ARCH_SUPPORTS_FIRMWARE

That's probably slightly easier to read than a huge "depends" statement
in the menu stanza, and ARCH_SUPPORTS_FIRMWARE might be a useful to
build in support for any core firmware utility code.