2013-08-29 09:58:01

by Alexandre Courbot

[permalink] [raw]
Subject: [PATCH v4 0/5] ARM: tegra: support for Trusted Foundations

New version revised according to comments received for v3. Hopefully
it will be good enough to be merged.

Changes since v3:
- Added of_register_trusted_foundations() function to avoid duplicate
device tree parsing code in arch files
- Added ability to initialize Trusted Foundations through platform data
- Changed TF version number to integers
- Refactored Kconfig menu for more clarity

A few requests could not reasonably be implemented:

TF version probing at runtime (requested by Dave) seems impossible
unfortunately. TF just does not provide an interface that allows such
queries. In the downstream Tegra kernel the TF version is
even hardcoded into the kernel.

Use of a firmware_op instance for non-firmware behavior (requested by
Stephen) would make it necessary to have a dedicated non-firmware
registration function that takes implementations from various different
files and would require these implementations to be exported. Checking
the return code of call_firmware_op() at call sites seems to be easier
to handle and is how current users of firmware_ops do.

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

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

--
1.8.4


2013-08-29 09:58:08

by Alexandre Courbot

[permalink] [raw]
Subject: [PATCH v4 2/5] ARM: tegra: add support for Trusted Foundations

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

Signed-off-by: Alexandre Courbot <[email protected]>
---
Documentation/devicetree/bindings/arm/tegra.txt | 5 +++++
arch/arm/mach-tegra/Kconfig | 1 +
arch/arm/mach-tegra/common.c | 2 ++
3 files changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/tegra.txt b/Documentation/devicetree/bindings/arm/tegra.txt
index ed9c853..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/common.c b/arch/arm/mach-tegra/common.c
index 94a119a..b405e45 100644
--- a/arch/arm/mach-tegra/common.c
+++ b/arch/arm/mach-tegra/common.c
@@ -27,6 +27,7 @@
#include <linux/clk-provider.h>

#include <asm/hardware/cache-l2x0.h>
+#include <asm/trusted_foundations.h>

#include "board.h"
#include "common.h"
@@ -99,6 +100,7 @@ static void __init tegra_init_cache(void)

void __init tegra_init_early(void)
{
+ of_register_trusted_foundations();
tegra_cpu_reset_handler_init();
tegra_apb_io_init();
tegra_init_fuse();
--
1.8.4

2013-08-29 09:58:14

by Alexandre Courbot

[permalink] [raw]
Subject: [PATCH v4 4/5] ARM: tegra: set CPU reset handler with firmware op

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 <[email protected]>
---
arch/arm/mach-tegra/reset.c | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-tegra/reset.c b/arch/arm/mach-tegra/reset.c
index 6964117..fc9cf03 100644
--- a/arch/arm/mach-tegra/reset.c
+++ b/arch/arm/mach-tegra/reset.c
@@ -21,6 +21,7 @@

#include <asm/cacheflush.h>
#include <asm/hardware/cache-l2x0.h>
+#include <asm/firmware.h>

#include "iomap.h"
#include "irammap.h"
@@ -65,6 +66,7 @@ static void __init tegra_cpu_reset_handler_enable(void)
void __iomem *iram_base = IO_ADDRESS(TEGRA_IRAM_RESET_BASE);
const u32 reset_address = TEGRA_IRAM_RESET_BASE +
tegra_cpu_reset_handler_offset;
+ int err;

BUG_ON(is_enabled);
BUG_ON(tegra_cpu_reset_handler_size > TEGRA_IRAM_RESET_HANDLER_SIZE);
@@ -72,9 +74,18 @@ static void __init tegra_cpu_reset_handler_enable(void)
memcpy(iram_base, (void *)__tegra_cpu_reset_handler_start,
tegra_cpu_reset_handler_size);

- tegra_cpu_reset_handler_set(reset_address);
-
- is_enabled = true;
+ err = call_firmware_op(set_cpu_boot_addr, 0, reset_address);
+ switch (err) {
+ case -ENOSYS:
+ tegra_cpu_reset_handler_set(reset_address);
+ /* pass-through */
+ case 0:
+ is_enabled = true;
+ break;
+ default:
+ pr_crit("Cannot set CPU reset handler: %d\n", err);
+ BUG();
+ }
}

void __init tegra_cpu_reset_handler_init(void)
--
1.8.4

2013-08-29 09:58:33

by Alexandre Courbot

[permalink] [raw]
Subject: [PATCH v4 5/5] ARM: tegra: support Trusted Foundations by default

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 <[email protected]>
---
arch/arm/configs/tegra_defconfig | 1 +
1 file changed, 1 insertion(+)

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

2013-08-29 09:58:57

by Alexandre Courbot

[permalink] [raw]
Subject: [PATCH v4 1/5] ARM: add basic Trusted Foundations support

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 <[email protected]>
---
.../arm/firmware/tl,trusted-foundations.txt | 17 +++++
.../devicetree/bindings/vendor-prefixes.txt | 1 +
arch/arm/Kconfig | 2 +
arch/arm/Makefile | 1 +
arch/arm/firmware/Kconfig | 26 +++++++
arch/arm/firmware/Makefile | 1 +
arch/arm/firmware/trusted_foundations.c | 83 ++++++++++++++++++++++
arch/arm/include/asm/trusted_foundations.h | 48 +++++++++++++
8 files changed, 179 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
create mode 100644 arch/arm/firmware/Kconfig
create mode 100644 arch/arm/firmware/Makefile
create mode 100644 arch/arm/firmware/trusted_foundations.c
create mode 100644 arch/arm/include/asm/trusted_foundations.h

diff --git a/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
new file mode 100644
index 0000000..3954bbd
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.txt
@@ -0,0 +1,17 @@
+Trusted Foundations
+
+Boards that use the Trusted Foundations secure monitor can signal its
+presence by declaring a node compatible with "tl,trusted-foundations"
+under the root node.
+
+Required properties:
+- compatible : "tl,trusted-foundations"
+- version-major : major version number of Trusted Foundations firmware
+- version-minor: minor version number of Trusted Foundations firmware
+
+Example:
+ firmware {
+ compatible = "tl,trusted-foundations";
+ version-major = <2>;
+ version-minor = <8>;
+ };
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 366ce9b..20d61f3 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -63,6 +63,7 @@ ste ST-Ericsson
stericsson ST-Ericsson
toumaz Toumaz
ti Texas Instruments
+tl Trusted Logic
toshiba Toshiba Corporation
v3 V3 Semiconductor
via VIA Technologies, Inc.
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 43594d5..7fbe800 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1074,6 +1074,8 @@ config ARM_TIMER_SP804
select CLKSRC_MMIO
select CLKSRC_OF if OF

+source "arch/arm/firmware/Kconfig"
+
source arch/arm/mm/Kconfig

config ARM_NR_BANKS
diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 6fd2cea..a48de17 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -267,6 +267,7 @@ core-$(CONFIG_KVM_ARM_HOST) += arch/arm/kvm/
core-y += arch/arm/kernel/ arch/arm/mm/ arch/arm/common/
core-y += arch/arm/net/
core-y += arch/arm/crypto/
+core-y += arch/arm/firmware/
core-y += $(machdirs) $(platdirs)

drivers-$(CONFIG_OPROFILE) += arch/arm/oprofile/
diff --git a/arch/arm/firmware/Kconfig b/arch/arm/firmware/Kconfig
new file mode 100644
index 0000000..fde21d0
--- /dev/null
+++ b/arch/arm/firmware/Kconfig
@@ -0,0 +1,26 @@
+config ARCH_SUPPORTS_FIRMWARE
+ bool
+
+config ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
+ bool
+ select ARCH_SUPPORTS_FIRMWARE
+
+menu "Firmware options"
+ depends on ARCH_SUPPORTS_FIRMWARE
+
+config TRUSTED_FOUNDATIONS
+ bool "Trusted Foundations secure monitor support"
+ depends on ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
+ help
+ Some devices are booted with the Trusted Foundations secure monitor
+ active, requiring some core operations to be performed by the secure
+ monitor instead of the kernel.
+
+ This option allows the kernel to invoke the secure monitor whenever
+ required on devices using Trusted Foundations.
+
+ 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..181f348
--- /dev/null
+++ b/arch/arm/firmware/trusted_foundations.c
@@ -0,0 +1,83 @@
+/*
+ * Trusted Foundations support for ARM CPUs
+ *
+ * Copyright (c) 2013, NVIDIA Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/of.h>
+#include <asm/firmware.h>
+#include <asm/trusted_foundations.h>
+
+#define TF_SET_CPU_BOOT_ADDR_SMC 0xfffff200
+
+static void __naked tf_generic_smc(u32 type, u32 subtype, u32 arg)
+{
+ asm volatile(
+ ".arch_extension sec\n\t"
+ "stmfd sp!, {r4 - r11, lr}\n\t"
+ __asmeq("%0", "r0")
+ __asmeq("%1", "r1")
+ __asmeq("%2", "r2")
+ "mov r3, #0\n\t"
+ "mov r4, #0\n\t"
+ "smc #0\n\t"
+ "ldmfd sp!, {r4 - r11, pc}"
+ :
+ : "r" (type), "r" (subtype), "r" (arg)
+ : "memory");
+}
+
+static int tf_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
+{
+ tf_generic_smc(TF_SET_CPU_BOOT_ADDR_SMC, boot_addr, 0);
+
+ return 0;
+}
+
+static const struct firmware_ops trusted_foundations_ops = {
+ .set_cpu_boot_addr = tf_set_cpu_boot_addr,
+};
+
+void register_trusted_foundations(struct trusted_foundations_platform_data *pd)
+{
+ /* we are not using version information for now since currently
+ * supported SMCs are compatible with all TF releases */
+ register_firmware_ops(&trusted_foundations_ops);
+}
+
+void of_register_trusted_foundations(void)
+{
+ struct device_node *node;
+
+ node = of_find_compatible_node(NULL, NULL, "tl,trusted-foundations");
+ if (node) {
+ struct trusted_foundations_platform_data pdata;
+ int err;
+
+ err = of_property_read_u32(node, "version-major",
+ &pdata.version_major);
+ if (err != 0) {
+ pr_crit("Trusted Foundation: missing version-major\n");
+ BUG();
+ }
+ err = of_property_read_u32(node, "version-minor",
+ &pdata.version_minor);
+ if (err != 0) {
+ pr_crit("Trusted Foundation: missing version-minor\n");
+ BUG();
+ }
+ register_trusted_foundations(&pdata);
+ }
+}
diff --git a/arch/arm/include/asm/trusted_foundations.h b/arch/arm/include/asm/trusted_foundations.h
new file mode 100644
index 0000000..77a4961
--- /dev/null
+++ b/arch/arm/include/asm/trusted_foundations.h
@@ -0,0 +1,48 @@
+/*
+ * 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>
+
+struct trusted_foundations_platform_data {
+ unsigned int version_major;
+ unsigned int version_minor;
+};
+
+#if IS_ENABLED(CONFIG_TRUSTED_FOUNDATIONS)
+
+void register_trusted_foundations(struct trusted_foundations_platform_data *pd);
+void of_register_trusted_foundations(void);
+
+#else
+
+static inline void register_trusted_foundations(
+ struct trusted_foundations_platform_data *pd)
+{
+ pr_crit("No support for Trusted Foundations, stopping...\n");
+ BUG();
+}
+
+static inline void of_register_trusted_foundations(void)
+{
+ register_trusted_foundations(NULL);
+}
+
+#endif /* IS_ENABLED(CONFIG_TRUSTED_FOUNDATIONS) */
+
+#endif
--
1.8.4

2013-08-29 09:58:55

by Alexandre Courbot

[permalink] [raw]
Subject: [PATCH v4 3/5] ARM: tegra: split setting of CPU reset handler

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 <[email protected]>
---
arch/arm/mach-tegra/reset.c | 27 +++++++++++++++++----------
1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mach-tegra/reset.c b/arch/arm/mach-tegra/reset.c
index 1ac434e..6964117 100644
--- a/arch/arm/mach-tegra/reset.c
+++ b/arch/arm/mach-tegra/reset.c
@@ -33,26 +33,18 @@

static bool is_enabled;

-static void __init tegra_cpu_reset_handler_enable(void)
+static void __init tegra_cpu_reset_handler_set(const u32 reset_address)
{
- void __iomem *iram_base = IO_ADDRESS(TEGRA_IRAM_RESET_BASE);
void __iomem *evp_cpu_reset =
IO_ADDRESS(TEGRA_EXCEPTION_VECTORS_BASE + 0x100);
void __iomem *sb_ctrl = IO_ADDRESS(TEGRA_SB_BASE);
u32 reg;

- BUG_ON(is_enabled);
- BUG_ON(tegra_cpu_reset_handler_size > TEGRA_IRAM_RESET_HANDLER_SIZE);
-
- memcpy(iram_base, (void *)__tegra_cpu_reset_handler_start,
- tegra_cpu_reset_handler_size);
-
/*
* NOTE: This must be the one and only write to the EVP CPU reset
* vector in the entire system.
*/
- writel(TEGRA_IRAM_RESET_BASE + tegra_cpu_reset_handler_offset,
- evp_cpu_reset);
+ writel(reset_address, evp_cpu_reset);
wmb();
reg = readl(evp_cpu_reset);

@@ -66,6 +58,21 @@ static void __init tegra_cpu_reset_handler_enable(void)
writel(reg, sb_ctrl);
wmb();
}
+}
+
+static void __init tegra_cpu_reset_handler_enable(void)
+{
+ void __iomem *iram_base = IO_ADDRESS(TEGRA_IRAM_RESET_BASE);
+ const u32 reset_address = TEGRA_IRAM_RESET_BASE +
+ tegra_cpu_reset_handler_offset;
+
+ BUG_ON(is_enabled);
+ BUG_ON(tegra_cpu_reset_handler_size > TEGRA_IRAM_RESET_HANDLER_SIZE);
+
+ memcpy(iram_base, (void *)__tegra_cpu_reset_handler_start,
+ tegra_cpu_reset_handler_size);
+
+ tegra_cpu_reset_handler_set(reset_address);

is_enabled = true;
}
--
1.8.4

2013-08-30 08:20:53

by Tomasz Figa

[permalink] [raw]
Subject: Re: [PATCH v4 1/5] ARM: add basic Trusted Foundations support

Hi Alexandre,

Overall this looks good to me, but I have some minor comments.

On Thursday 29 of August 2013 18:57:44 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 <[email protected]>
> ---
> .../arm/firmware/tl,trusted-foundations.txt | 17 +++++
> .../devicetree/bindings/vendor-prefixes.txt | 1 +
> arch/arm/Kconfig | 2 +
> arch/arm/Makefile | 1 +
> arch/arm/firmware/Kconfig | 26 +++++++
> arch/arm/firmware/Makefile | 1 +
> arch/arm/firmware/trusted_foundations.c | 83
> ++++++++++++++++++++++ arch/arm/include/asm/trusted_foundations.h
> | 48 +++++++++++++ 8 files changed, 179 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations.t
> xt create mode 100644 arch/arm/firmware/Kconfig
> create mode 100644 arch/arm/firmware/Makefile
> create mode 100644 arch/arm/firmware/trusted_foundations.c
> create mode 100644 arch/arm/include/asm/trusted_foundations.h
>
> diff --git
> a/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations
> .txt
> b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations
> .txt new file mode 100644
> index 0000000..3954bbd
> --- /dev/null
> +++
> b/Documentation/devicetree/bindings/arm/firmware/tl,trusted-foundations
> .txt @@ -0,0 +1,17 @@
> +Trusted Foundations
> +
> +Boards that use the Trusted Foundations secure monitor can signal its
> +presence by declaring a node compatible with "tl,trusted-foundations"
> +under the root node.
> +
> +Required properties:
> +- compatible : "tl,trusted-foundations"
> +- version-major : major version number of Trusted Foundations firmware
> +- version-minor: minor version number of Trusted Foundations firmware

Hmm, maybe you could simply define a single version property that could
have multiple cells? Like:

firmware {
compatible = "tl,trusted-foundations";
version = <2 8>;
};

> +Example:
> + firmware {
> + compatible = "tl,trusted-foundations";
> + version-major = <2>;
> + version-minor = <8>;
> + };
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt
> b/Documentation/devicetree/bindings/vendor-prefixes.txt index
> 366ce9b..20d61f3 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -63,6 +63,7 @@ ste ST-Ericsson
> stericsson ST-Ericsson
> toumaz Toumaz
> ti Texas Instruments
> +tl Trusted Logic
> toshiba Toshiba Corporation
> v3 V3 Semiconductor
> via VIA Technologies, Inc.
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 43594d5..7fbe800 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1074,6 +1074,8 @@ config ARM_TIMER_SP804
> select CLKSRC_MMIO
> select CLKSRC_OF if OF
>
> +source "arch/arm/firmware/Kconfig"
> +
> source arch/arm/mm/Kconfig
>
> config ARM_NR_BANKS
> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> index 6fd2cea..a48de17 100644
> --- a/arch/arm/Makefile
> +++ b/arch/arm/Makefile
> @@ -267,6 +267,7 @@ core-$(CONFIG_KVM_ARM_HOST) += arch/arm/kvm/
> core-y += arch/arm/kernel/ arch/arm/mm/
arch/arm/common/
> core-y += arch/arm/net/
> core-y += arch/arm/crypto/
> +core-y += arch/arm/firmware/
> core-y += $(machdirs) $(platdirs)
>
> drivers-$(CONFIG_OPROFILE) += arch/arm/oprofile/
> diff --git a/arch/arm/firmware/Kconfig b/arch/arm/firmware/Kconfig
> new file mode 100644
> index 0000000..fde21d0
> --- /dev/null
> +++ b/arch/arm/firmware/Kconfig
> @@ -0,0 +1,26 @@
> +config ARCH_SUPPORTS_FIRMWARE
> + bool
> +
> +config ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
> + bool
> + select ARCH_SUPPORTS_FIRMWARE
> +
> +menu "Firmware options"
> + depends on ARCH_SUPPORTS_FIRMWARE
> +
> +config TRUSTED_FOUNDATIONS
> + bool "Trusted Foundations secure monitor support"
> + depends on ARCH_SUPPORTS_TRUSTED_FOUNDATIONS
> + help
> + Some devices are booted with the Trusted Foundations secure
monitor
> + active, requiring some core operations to be performed by the
secure
> + monitor instead of the kernel.
> +
> + This option allows the kernel to invoke the secure monitor
whenever
> + required on devices using Trusted Foundations.
> +
> + 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.

What about pointing to the documentation file instead?

> +
> +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..181f348
> --- /dev/null
> +++ b/arch/arm/firmware/trusted_foundations.c
> @@ -0,0 +1,83 @@
> +/*
> + * Trusted Foundations support for ARM CPUs
> + *
> + * Copyright (c) 2013, NVIDIA Corporation.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published
> by + * the Free Software Foundation; either version 2 of the License,
> or + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful, but
> WITHOUT + * ANY WARRANTY; without even the implied warranty of
> MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> General Public License for + * more details.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/of.h>
> +#include <asm/firmware.h>
> +#include <asm/trusted_foundations.h>
> +
> +#define TF_SET_CPU_BOOT_ADDR_SMC 0xfffff200
> +
> +static void __naked tf_generic_smc(u32 type, u32 subtype, u32 arg)
> +{
> + asm volatile(
> + ".arch_extension sec\n\t"
> + "stmfd sp!, {r4 - r11, lr}\n\t"
> + __asmeq("%0", "r0")
> + __asmeq("%1", "r1")
> + __asmeq("%2", "r2")
> + "mov r3, #0\n\t"
> + "mov r4, #0\n\t"
> + "smc #0\n\t"
> + "ldmfd sp!, {r4 - r11, pc}"
> + :
> + : "r" (type), "r" (subtype), "r" (arg)
> + : "memory");
> +}
> +
> +static int tf_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
> +{
> + tf_generic_smc(TF_SET_CPU_BOOT_ADDR_SMC, boot_addr, 0);
> +
> + return 0;
> +}
> +
> +static const struct firmware_ops trusted_foundations_ops = {
> + .set_cpu_boot_addr = tf_set_cpu_boot_addr,
> +};
> +
> +void register_trusted_foundations(struct
> trusted_foundations_platform_data *pd) +{
> + /* we are not using version information for now since currently
> + * supported SMCs are compatible with all TF releases */
> + register_firmware_ops(&trusted_foundations_ops);
> +}
> +
> +void of_register_trusted_foundations(void)
> +{
> + struct device_node *node;
> +
> + node = of_find_compatible_node(NULL, NULL, "tl,trusted-
foundations");

nit:
if (!node)
return;

> + if (node) {
> + struct trusted_foundations_platform_data pdata;
> + int err;
> +
> + err = of_property_read_u32(node, "version-major",
> + &pdata.version_major);
> + if (err != 0) {
> + pr_crit("Trusted Foundation: missing version-
major\n");
> + BUG();
> + }
> + err = of_property_read_u32(node, "version-minor",
> + &pdata.version_minor);
> + if (err != 0) {
> + pr_crit("Trusted Foundation: missing version-
minor\n");
> + BUG();
> + }

If version was stored in multiple cells of a single property, then it
would be parsed by just one call to of_property_read_u32_array().

> + register_trusted_foundations(&pdata);
> + }
> +}
> diff --git a/arch/arm/include/asm/trusted_foundations.h
> b/arch/arm/include/asm/trusted_foundations.h new file mode 100644
> index 0000000..77a4961
> --- /dev/null
> +++ b/arch/arm/include/asm/trusted_foundations.h
> @@ -0,0 +1,48 @@
> +/*
> + * 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>
> +
> +struct trusted_foundations_platform_data {
> + unsigned int version_major;
> + unsigned int version_minor;
> +};
> +
> +#if IS_ENABLED(CONFIG_TRUSTED_FOUNDATIONS)
> +
> +void register_trusted_foundations(struct
> trusted_foundations_platform_data *pd); +void
> of_register_trusted_foundations(void);
> +
> +#else
> +
> +static inline void register_trusted_foundations(
> + struct
trusted_foundations_platform_data *pd)
> +{
> + pr_crit("No support for Trusted Foundations, stopping...\n");
> + BUG();

Hmm, why not simply panic()?

Best regards,
Tomasz

2013-08-30 08:23:11

by Tomasz Figa

[permalink] [raw]
Subject: Re: [PATCH v4 0/5] ARM: tegra: support for Trusted Foundations

Hi Alexandre,

On Thursday 29 of August 2013 18:57:43 Alexandre Courbot wrote:
> New version revised according to comments received for v3. Hopefully
> it will be good enough to be merged.
>
> Changes since v3:
> - Added of_register_trusted_foundations() function to avoid duplicate
> device tree parsing code in arch files
> - Added ability to initialize Trusted Foundations through platform data
> - Changed TF version number to integers
> - Refactored Kconfig menu for more clarity

Except some minor comments to patch 1/5, the series looks good to me.

Reviewed-by: Tomasz Figa <[email protected]>

Best regards,
Tomasz

2013-09-02 07:28:59

by Alexandre Courbot

[permalink] [raw]
Subject: Re: [PATCH v4 1/5] ARM: add basic Trusted Foundations support

Hi Tomasz!

On Fri, Aug 30, 2013 at 5:20 PM, Tomasz Figa <[email protected]> wrote:
>> +Required properties:
>> +- compatible : "tl,trusted-foundations"
>> +- version-major : major version number of Trusted Foundations firmware
>> +- version-minor: minor version number of Trusted Foundations firmware
>
> Hmm, maybe you could simply define a single version property that could
> have multiple cells? Like:
>
> firmware {
> compatible = "tl,trusted-foundations";
> version = <2 8>;
> };

I'm fine this way too, but do we have other bindings that use the same
scheme? What is the general convention for version number bindings?

>> + 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.
>
> What about pointing to the documentation file instead?

Yes, that would make more sense.

>> +void of_register_trusted_foundations(void)
>> +{
>> + struct device_node *node;
>> +
>> + node = of_find_compatible_node(NULL, NULL, "tl,trusted-
> foundations");
>
> nit:
> if (!node)
> return;

Fixed, thanks.

>> +static inline void register_trusted_foundations(
>> + struct
> trusted_foundations_platform_data *pd)
>> +{
>> + pr_crit("No support for Trusted Foundations, stopping...\n");
>> + BUG();
>
> Hmm, why not simply panic()?

Fixed that too.

Thanks for the review!
Alex.

2013-09-03 18:36:12

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH v4 1/5] ARM: add basic Trusted Foundations support

On 09/02/2013 01:28 AM, Alexandre Courbot wrote:
> Hi Tomasz!
>
> On Fri, Aug 30, 2013 at 5:20 PM, Tomasz Figa <[email protected]> wrote:
>>> +Required properties:
>>> +- compatible : "tl,trusted-foundations"
>>> +- version-major : major version number of Trusted Foundations firmware
>>> +- version-minor: minor version number of Trusted Foundations firmware
>>
>> Hmm, maybe you could simply define a single version property that could
>> have multiple cells? Like:
>>
>> firmware {
>> compatible = "tl,trusted-foundations";
>> version = <2 8>;
>> };
>
> I'm fine this way too, but do we have other bindings that use the same
> scheme? What is the general convention for version number bindings?

I don't know if there are enough cases of this for there to be a
convention. A 2-cell property seems fine to me.

>>> + 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.
>>
>> What about pointing to the documentation file instead?
>
> Yes, that would make more sense.

Possibly. What about when the binding document is no longer part of the
kernel though? Perhaps we could reference the documentation in some way
other than by the pathname within the kernel source tree though, e.g.
'see the device tree binding documentation for
compatible="tl,trusted-foundations"'?

2013-09-03 18:40:26

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH v4 1/5] ARM: add basic Trusted Foundations support

On 08/29/2013 03:57 AM, 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

Nit: s/smc/SMC/?

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

> +#if IS_ENABLED(CONFIG_TRUSTED_FOUNDATIONS)
> +
> +void register_trusted_foundations(struct trusted_foundations_platform_data *pd);
> +void of_register_trusted_foundations(void);
> +
> +#else
> +
> +static inline void register_trusted_foundations(
> + struct trusted_foundations_platform_data *pd)
> +{
> + pr_crit("No support for Trusted Foundations, stopping...\n");
> + BUG();
> +}
> +
> +static inline void of_register_trusted_foundations(void)
> +{
> + register_trusted_foundations(NULL);

That will cause a hard-fail. I assume that should only happen if the TL
DT node is present; if there's no DT node, it's fine for a kernel
without any TL support to run.

> +}
> +
> +#endif /* IS_ENABLED(CONFIG_TRUSTED_FOUNDATIONS) */

2013-09-03 18:42:28

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH v4 0/5] ARM: tegra: support for Trusted Foundations

On 08/29/2013 03:57 AM, Alexandre Courbot wrote:
> New version revised according to comments received for v3. Hopefully
> it will be good enough to be merged.

Aside from the small issue in patch 1/5, the series,
Reviewed-by: Stephen Warren <[email protected]>

2013-09-04 02:31:53

by Alexandre Courbot

[permalink] [raw]
Subject: Re: [PATCH v4 0/5] ARM: tegra: support for Trusted Foundations

On 09/04/2013 03:42 AM, Stephen Warren wrote:
> On 08/29/2013 03:57 AM, Alexandre Courbot wrote:
>> New version revised according to comments received for v3. Hopefully
>> it will be good enough to be merged.
>
> Aside from the small issue in patch 1/5, the series,
> Reviewed-by: Stephen Warren <[email protected]>

Thanks! I have adressed the issues you mentioned and will submit v5
soon. Despite the fact this adds some non-Tegra stuff, will you be able
to take it into your tree, or should I ask someone else?

2013-09-04 18:25:16

by Stephen Warren

[permalink] [raw]
Subject: Re: [PATCH v4 0/5] ARM: tegra: support for Trusted Foundations

On 09/03/2013 08:31 PM, Alex Courbot wrote:
> On 09/04/2013 03:42 AM, Stephen Warren wrote:
>> On 08/29/2013 03:57 AM, Alexandre Courbot wrote:
>>> New version revised according to comments received for v3. Hopefully
>>> it will be good enough to be merged.
>>
>> Aside from the small issue in patch 1/5, the series,
>> Reviewed-by: Stephen Warren <[email protected]>
>
> Thanks! I have adressed the issues you mentioned and will submit v5
> soon. Despite the fact this adds some non-Tegra stuff, will you be able
> to take it into your tree, or should I ask someone else?

I would like to see an ack from the core ARM maintainer (Russell) on
patch 1/5 (also CC'ing arm-soc maintainers for their opinion).

If I get that, I think it makes sense to take the series through the
Tegra tree. I can always create a stable branch for patch 1/5 if needed
to resolve conflicts elsewhere (e.g. Russell's trees or arm-soc) and/or
allow other people to build on top of that patch.