[Skiboot] [PATCH v4 04/11] doc: add opal secure variable documentation

[Oliver O'Halloran]

On Sat, 2019-10-26 at 04:45 -0500, Eric Richter wrote:
> This patch contains the work-in-progress documentation for the
> secure variable design in OPAL. Future revisions of this patch
> set will (hopefully) add new pieces of documentation here.
> 
> V3:
>  - removed metadata
>  - removed get_size
>  - updated _get semantics for size queries
>  - added/expanded device tree properties
> 
> V4:
>  - updated for new device tree changes
> 
> Signed-off-by: Eric Richter <erichte at linux.ibm.com>
> ---
>  doc/device-tree/ibm,secureboot.rst |  10 ++
>  doc/device-tree/secvar.rst         |  84 +++++++++++++
>  doc/opal-api/opal-secvar.rst       | 192 +++++++++++++++++++++++++++++
>  3 files changed, 286 insertions(+)
>  create mode 100644 doc/device-tree/secvar.rst
>  create mode 100644 doc/opal-api/opal-secvar.rst
> 
> diff --git a/doc/device-tree/ibm,secureboot.rst b/doc/device-tree/ibm,secureboot.rst
> index 42c4ed7d..b4729a9d 100644
> --- a/doc/device-tree/ibm,secureboot.rst
> +++ b/doc/device-tree/ibm,secureboot.rst
> @@ -21,6 +21,13 @@ Required properties
>                                                It described by the ibm,cvc child
>                                                node.
>  
> +                        ibm,secureboot-v3  :  The container-verification-code
> +                                              is stored in a reserved memory.
> +                                              It described by the ibm,cvc child
> +                                              node. Secure variables are
> +                                              supported. `secvar` node should
> +                                              be created.
> +
>      secure-enabled:     this property exists when the firmware stack is booting
>                          in secure mode (hardware secure boot jumper asserted).
>  
> @@ -33,6 +40,9 @@ Required properties
>  
>      hw-key-hash-size:   hw-key-hash size
>  
> +    secvar:             this node is created if the platform supports secure
> +                        variables. Contains information about the current
> +                        secvar status, see 'secvar.rst'.
>  
>  Obsolete properties
>  -------------------
> diff --git a/doc/device-tree/secvar.rst b/doc/device-tree/secvar.rst
> new file mode 100644
> index 00000000..ddf15b38
> --- /dev/null
> +++ b/doc/device-tree/secvar.rst
> @@ -0,0 +1,84 @@
> +.. _device-tree/ibm,opal/secvar:
> +
> +secvar
> +======
> +
> +The ``secvar`` node provides secure variable information for the secure
> +boot of the target OS.
> +
> +Required properties
> +-------------------
> +
> +.. code-block:: none
> +
> +    status:             set to "fail" if the secure variables could not
> +                        be initialized, validated, or some other
> +                        hardware problem.
> +
> +    update-status:      contains the return code of the update queue
> +                        process run during initialization. Signifies if
> +                        updates were processed or not, and if there was
> +                        an error. See table below.
> +                        TODO: This probably belongs in the backend node.
> +
> +    os-secure-enforcing: If this property exists, the system is in
> +                        considered to be in "OS secure mode". Kexec
> +                        images should be signature checked, etc.
> +
> +    backend:            This node contains any backend-specific
> +                        information, and is maintained by the backend driver.
> +
> +    storage:            This node contains any storage-specific
> +                        information, and is mainted by the storage driver.
> +
> +    max-var-size:       This property must be exposed as a child of the
> +                        storage driver, and determines how large a
> +                        variable can be.
> +
> +Example
> +-------
> +
> +.. code-block:: dts
> +
> +    secvar {
> +        compatible = "ibm,secvar-v1";
> +        status = "okay";
> +        os-secure-enforcing = <0x0>;
> +        update-status = <0x0>;
> +        storage {
> +            compatible = "ibm,secboot-tpm-v1";
> +            status = "okay";
> +            max-var-size = <0x1000>;
> +        }
> +        backend {
> +            compatible = "ibm,edk2-compat-v1";
> +            status = "okay";
> +        }

I don't know how much sense it makes to have storage and backend as
seperate nodes rather than having a single secvar node and dumping
everything in there.

The API has a single flat key-space so any limitations of the backend
and storage drivers are really limitations that apply to the API as a
whole. If you look at Nayna's patches you can see that having max-var-
size in the storage node is kind of awkward since you need to look up
the node somehow. You can find it using compatible (creates a
dependency to the specific backend) or by assuming the storage node is
actually named "storage." The latter is preferable, but is there even a
compelling reason to even tell the kernel what the storage driver is?
The whole point of having the Secvar API is to abstract away details of
how the secvars are stored.

There's a better argument for having the backend node since we need to
communicate how updates are handled. However, couldn't that be done
using a property?

Oliver

ps. You probably want to include the total space available for secvars
in the DT.