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

[Alexandre Courbot]

On Thu, Aug 15, 2013 at 6:35 AM, Stephen Warren <swarren at 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.