| Message ID | 1377770268-14014-2-git-send-email-acourbot@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <acourbot@nvidia.com> > --- > .../arm/firmware/tl,trusted-foundations.txt | 17 +++++ > .../devicetree/bindings/vendor-prefixes.txt | 1 + > arch/arm/Kconfig | 2 + > arch/arm/Makefile | 1 + > arch/arm/firmware/Kconfig | 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
Hi Tomasz! On Fri, Aug 30, 2013 at 5:20 PM, Tomasz Figa <tomasz.figa@gmail.com> 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.
On 09/02/2013 01:28 AM, Alexandre Courbot wrote: > Hi Tomasz! > > On Fri, Aug 30, 2013 at 5:20 PM, Tomasz Figa <tomasz.figa@gmail.com> 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"'?
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) */
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
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 | 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