[PATCH] Documentation: dt: Add bindings for Secure-only devices

[PATCH] Documentation: dt: Add bindings for Secure-only devices

[Peter Maydell]

The existing device tree bindings assume that we are only trying to
describe a single address space with a device tree (for ARM, either
the Normal or the Secure world). Some uses for device tree need to
describe both Normal and Secure worlds in a single device tree. Add
documentation of how to do this, by adding extra properties which
describe when a device appears differently in the two worlds or when
it only appears in one of them.

The binding describes the general principles for adding new
properties describing the secure world, but for now we only need a
single new property, "secure-status", which can be used to annotate
devices to indicate that they are only visible in one of the two
worlds.

The primary expected use of this binding is for a virtual machine
like QEMU to describe the VM layout to a TrustZone aware firmware
(which would then use the secure-only devices itself, and pass the DT
on to a kernel running in the non-secure world, which ignores the
secure-only devices and uses the rest).

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
This binding doesn't affect the kernel itself, but the kernel
Documentation/ tree is the de-facto current place where all DT
bindings are documented, so Grant suggested this was the right
place to send a doc patch.

 Documentation/devicetree/bindings/arm/secure.txt | 41 ++++++++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/secure.txt

diff --git a/Documentation/devicetree/bindings/arm/secure.txt b/Documentation/devicetree/bindings/arm/secure.txt
new file mode 100644
index 0000000..4129fba
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/secure.txt
@@ -0,0 +1,41 @@
+* ARM Secure world bindings
+
+ARM CPUs with TrustZone support have two distinct address spaces,
+"Normal" and "Secure". Most devicetree consumers (including the Linux
+kernel) are not TrustZone aware and run entirely in either the Normal
+world or the Secure world. However some devicetree consumers are
+TrustZone aware and need to be able to determine whether devices are
+visible only in the Secure address space, only in the Normal address
+space, or visible in both. (One example of that situation would be a
+virtual machine which boots Secure firmware and wants to tell the
+firmware about the layout of the machine via devicetree.)
+
+The general principle of the naming scheme for Secure world bindings
+is that any property that needs a different value in the Secure world
+can be supported by prefixing the property name with "secure-". So for
+instance "secure-reg" would override "reg". If there is no "secure-"
+property then the Secure world value is the same as specified for the
+Normal world by the non-prefixed property. However, only the
+properties listed below may validly have "secure-" versions; this list
+will be enlarged on a case-by-case basis.
+
+Defining the bindings in this way means that a device tree which has
+been annotated to indicate the presence of Secure-only devices can
+still be processed unmodified by existing Non-secure software (and in
+particular by the kernel).
+
+Note that it is still valid for bindings intended for purely Secure
+world consumers (like kernels that run entirely in Secure) to simply
+describe the view of Secure world using the standard bindings. These
+secure- bindings only need to be used where both the Secure and Normal
+world views need to be described in a single device tree.
+
+Valid Secure world properties:
+
+- secure-status : specifies whether the device is present and usable
+  in the secure world. The combination of this with "status" allows
+  the various possible combinations of device visibility to be
+  specified:
+   status = "okay";                             // visible in S and NS
+   status = "disabled"; secure-status = "okay"; // S-only
+   status = "okay"; secure-status = "disabled"; // NS-only
-- 
1.9.1

Re: [PATCH] Documentation: dt: Add bindings for Secure-only devices

[Rob Herring]

On Thu, Oct 29, 2015 at 9:01 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> The existing device tree bindings assume that we are only trying to
> describe a single address space with a device tree (for ARM, either
> the Normal or the Secure world). Some uses for device tree need to
> describe both Normal and Secure worlds in a single device tree. Add
> documentation of how to do this, by adding extra properties which
> describe when a device appears differently in the two worlds or when
> it only appears in one of them.
>
> The binding describes the general principles for adding new
> properties describing the secure world, but for now we only need a
> single new property, "secure-status", which can be used to annotate
> devices to indicate that they are only visible in one of the two
> worlds.
>
> The primary expected use of this binding is for a virtual machine
> like QEMU to describe the VM layout to a TrustZone aware firmware
> (which would then use the secure-only devices itself, and pass the DT
> on to a kernel running in the non-secure world, which ignores the
> secure-only devices and uses the rest).
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> This binding doesn't affect the kernel itself, but the kernel
> Documentation/ tree is the de-facto current place where all DT
> bindings are documented, so Grant suggested this was the right
> place to send a doc patch.
>
>  Documentation/devicetree/bindings/arm/secure.txt | 41 ++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/secure.txt
>
> diff --git a/Documentation/devicetree/bindings/arm/secure.txt b/Documentation/devicetree/bindings/arm/secure.txt
> new file mode 100644
> index 0000000..4129fba
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/secure.txt
> @@ -0,0 +1,41 @@
> +* ARM Secure world bindings
> +
> +ARM CPUs with TrustZone support have two distinct address spaces,
> +"Normal" and "Secure". Most devicetree consumers (including the Linux
> +kernel) are not TrustZone aware and run entirely in either the Normal
> +world or the Secure world. However some devicetree consumers are
> +TrustZone aware and need to be able to determine whether devices are
> +visible only in the Secure address space, only in the Normal address
> +space, or visible in both. (One example of that situation would be a
> +virtual machine which boots Secure firmware and wants to tell the
> +firmware about the layout of the machine via devicetree.)
> +
> +The general principle of the naming scheme for Secure world bindings
> +is that any property that needs a different value in the Secure world
> +can be supported by prefixing the property name with "secure-". So for
> +instance "secure-reg" would override "reg". If there is no "secure-"
> +property then the Secure world value is the same as specified for the
> +Normal world by the non-prefixed property. However, only the
> +properties listed below may validly have "secure-" versions; this list
> +will be enlarged on a case-by-case basis.

Certainly having lots of properties prefixed this way is not what I
would like to see.

> +Defining the bindings in this way means that a device tree which has
> +been annotated to indicate the presence of Secure-only devices can
> +still be processed unmodified by existing Non-secure software (and in
> +particular by the kernel).
> +
> +Note that it is still valid for bindings intended for purely Secure
> +world consumers (like kernels that run entirely in Secure) to simply
> +describe the view of Secure world using the standard bindings. These
> +secure- bindings only need to be used where both the Secure and Normal
> +world views need to be described in a single device tree.
> +
> +Valid Secure world properties:
> +
> +- secure-status : specifies whether the device is present and usable
> +  in the secure world. The combination of this with "status" allows
> +  the various possible combinations of device visibility to be
> +  specified:
> +   status = "okay";                             // visible in S and NS

I assume neither property present or both okay also mean the same.

status = "okay"; secure-status = "okay";

We should be explicit.

> +   status = "disabled"; secure-status = "okay"; // S-only
> +   status = "okay"; secure-status = "disabled"; // NS-only

In HKG when we discussed this, 'status = "secure"' was the proposal.
That would be simpler:

S world can use "okay" or "secure"
NS world can use "okay" or no property.

That leaves out the case of disabled in S and enabled for NS. We could
want that for s/w reasons, but can we have h/w like that?

Rob

Re: [PATCH] Documentation: dt: Add bindings for Secure-only devices

[Peter Maydell]

On 30 October 2015 at 18:28, Rob Herring <robh+dt@kernel.org> wrote:
> On Thu, Oct 29, 2015 at 9:01 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> +Valid Secure world properties:
>> +
>> +- secure-status : specifies whether the device is present and usable
>> +  in the secure world. The combination of this with "status" allows
>> +  the various possible combinations of device visibility to be
>> +  specified:
>> +   status = "okay";                             // visible in S and NS
>
> I assume neither property present or both okay also mean the same.
>
> status = "okay"; secure-status = "okay";
>
> We should be explicit.

Yes; status defaults to "okay" (presumably this is listed in
the overal DT binding spec somewhere), and secure-status
defaults to "same as status, which might in turn be defaulted".
We can list the complete set of options (neither present,
both 'okay', status not present but secure-status present, etc),
though it gets a bit long-winded, especially if we later add
more secure- properties (they'd all have to have verbiage about
"if not present, same as non-prefixed property; if both not
present, both take the default the non-prefixed property takes
if it's not present; if prefixed property not present, it
defaults to same as non-prefixed property", which we already
say in the introductory section). Still, for just status it
would be easy enough to add a couple of lines:

+    status = "okay"; secure-status = "okay"; // ditto
+    secure-status = "okay";                  // ditto
+    // neither explicitly defined: ditto

(Do you want the full set of 9 options you get from multiplying
out "okay" vs "disabled" vs not-set for each property?)

>> +   status = "disabled"; secure-status = "okay"; // S-only
>> +   status = "okay"; secure-status = "disabled"; // NS-only
>
> In HKG when we discussed this, 'status = "secure"' was the proposal.
> That would be simpler:
>
> S world can use "okay" or "secure"
> NS world can use "okay" or no property.
>
> That leaves out the case of disabled in S and enabled for NS. We could
> want that for s/w reasons, but can we have h/w like that?

It's perfectly possible to design hardware like that (though
I can't think of a reason to do so offhand). I think it's the desire
to be able to describe all the possible valid h/w combinations
that brought us to this secure- prefix design. Plus it
extends nicely to cover other possibilities as we need it;
for instance "device A is at S-0x10000 but NS-0x20000" can be
done by specifying a device like:
   status = "okay";
   secure-status = "okay";
   reg = < 0x20000 0x1000 >;
   secure-regs = < 0x10000 0x1000 >;
(apologies if I've messed the syntax up there).

Just going for 'status=secure' would deal with the immediate
requirement, but my preference is for a description that
lets us describe all the possible configurations, not just
the ones we think are common, and secure-* is a neat way
of doing that (IIRC it was Grant's suggestion; speaking of
whom, I just noticed I forgot to cc him on the original patch).

thanks
-- PMM

Re: [PATCH] Documentation: dt: Add bindings for Secure-only devices

[Frank Rowand]

On 10/30/2015 1:07 PM, Peter Maydell wrote:
> On 30 October 2015 at 18:28, Rob Herring <robh+dt@kernel.org> wrote:
>> On Thu, Oct 29, 2015 at 9:01 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> +Valid Secure world properties:
>>> +
>>> +- secure-status : specifies whether the device is present and usable
>>> +  in the secure world. The combination of this with "status" allows
>>> +  the various possible combinations of device visibility to be
>>> +  specified:
>>> +   status = "okay";                             // visible in S and NS
>>
>> I assume neither property present or both okay also mean the same.
>>
>> status = "okay"; secure-status = "okay";
>>
>> We should be explicit.
> 
> Yes; status defaults to "okay" (presumably this is listed in
> the overal DT binding spec somewhere), 

Valid values for the "status" property do not seem to be well documented
for Linux.

The code, __of_device_is_available(), implements:

  value of "okay"  returns true
  value of "ok"    returns true
  status property does not exist returns true (thus defaults to true)

  anything else returns false.

Eight individual bindings define the status property (I'll put them
on my todo list to remove since they should be inherited).  The status
property should be in a top level binding, which should be created as
part of the bindings documentation cleanup.  I'll watch for it to get
created or do it myself.

(Lots of bindings show examples of using the status property in their
examples -- that is fine.)

ePAPR v1.1, section "2.3.4 status" does not list "ok", but does list:
  "okay"
  "disabled"
  "fail"
  "fail-sss"


< snip >

-Frank

Re: [PATCH] Documentation: dt: Add bindings for Secure-only devices

[Rob Herring]

On Fri, Oct 30, 2015 at 08:07:34PM +0000, Peter Maydell wrote:
> On 30 October 2015 at 18:28, Rob Herring <robh+dt@kernel.org> wrote:
> > On Thu, Oct 29, 2015 at 9:01 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> >> +Valid Secure world properties:
> >> +
> >> +- secure-status : specifies whether the device is present and usable
> >> +  in the secure world. The combination of this with "status" allows
> >> +  the various possible combinations of device visibility to be
> >> +  specified:
> >> +   status = "okay";                             // visible in S and NS
> >
> > I assume neither property present or both okay also mean the same.
> >
> > status = "okay"; secure-status = "okay";
> >
> > We should be explicit.
> 
> Yes; status defaults to "okay" (presumably this is listed in
> the overal DT binding spec somewhere), and secure-status
> defaults to "same as status, which might in turn be defaulted".
> We can list the complete set of options (neither present,
> both 'okay', status not present but secure-status present, etc),
> though it gets a bit long-winded, especially if we later add
> more secure- properties (they'd all have to have verbiage about
> "if not present, same as non-prefixed property; if both not
> present, both take the default the non-prefixed property takes
> if it's not present; if prefixed property not present, it
> defaults to same as non-prefixed property", which we already
> say in the introductory section). Still, for just status it
> would be easy enough to add a couple of lines:
> 
> +    status = "okay"; secure-status = "okay"; // ditto
> +    secure-status = "okay";                  // ditto
> +    // neither explicitly defined: ditto
> 
> (Do you want the full set of 9 options you get from multiplying
> out "okay" vs "disabled" vs not-set for each property?)

Better to err on completeness. The inheritance is easily missed.

Also, one nit. Use C style comments so when people copy-n-paste this it 
is the correct style.

> 
> >> +   status = "disabled"; secure-status = "okay"; // S-only
> >> +   status = "okay"; secure-status = "disabled"; // NS-only
> >
> > In HKG when we discussed this, 'status = "secure"' was the proposal.
> > That would be simpler:
> >
> > S world can use "okay" or "secure"
> > NS world can use "okay" or no property.
> >
> > That leaves out the case of disabled in S and enabled for NS. We could
> > want that for s/w reasons, but can we have h/w like that?
> 
> It's perfectly possible to design hardware like that (though
> I can't think of a reason to do so offhand). I think it's the desire
> to be able to describe all the possible valid h/w combinations
> that brought us to this secure- prefix design. Plus it
> extends nicely to cover other possibilities as we need it;
> for instance "device A is at S-0x10000 but NS-0x20000" can be
> done by specifying a device like:
>    status = "okay";
>    secure-status = "okay";
>    reg = < 0x20000 0x1000 >;
>    secure-regs = < 0x10000 0x1000 >;
> (apologies if I've messed the syntax up there).
> 
> Just going for 'status=secure' would deal with the immediate
> requirement, but my preference is for a description that
> lets us describe all the possible configurations, not just
> the ones we think are common, and secure-* is a neat way
> of doing that (IIRC it was Grant's suggestion; speaking of
> whom, I just noticed I forgot to cc him on the original patch).

I don't really want to give blanket approval to use secure-* prefix, but 
I'm okay with secure-status I guess.

For secure-reg in particular, I would do something different and 
similar to PCI where we add another address cell to define the address 
space as secure or non-secure. So your example becomes:

    reg = <0 0x20000 0x1000 >,
          <1 0x10000 0x1000 >;

The bus compatible would also have to be something like 
"arm,secure-bus". No point in debating this now. We can worry about it 
when we have an example.

Rob

Re: [PATCH] Documentation: dt: Add bindings for Secure-only devices

[Peter Maydell]

On 10 November 2015 at 14:51, Rob Herring <robh@kernel.org> wrote:
> On Fri, Oct 30, 2015 at 08:07:34PM +0000, Peter Maydell wrote:
>> +    status = "okay"; secure-status = "okay"; // ditto
>> +    secure-status = "okay";                  // ditto
>> +    // neither explicitly defined: ditto
>>
>> (Do you want the full set of 9 options you get from multiplying
>> out "okay" vs "disabled" vs not-set for each property?)
>
> Better to err on completeness. The inheritance is easily missed.
>
> Also, one nit. Use C style comments so when people copy-n-paste this it
> is the correct style.

OK; I'll send out a v2 with these fixes.

thanks
-- PMM