Re: [PATCH v3 1/5] Documentation/firmware: add imx/se to other_interfaces

[Randy Dunlap <rdunlap@infradead.org>]

Hi--

IMO there is an overuse of hyphens (dashes) here.
Please consider the changes below.


On 6/17/24 12:29 AM, Pankaj Gupta wrote:
> Documents i.MX SoC's Service layer and C_DEV driver for selected SoC(s)
> that contains the NXP hardware IP(s) for secure-enclaves(se) like:

Is the product referred to as "secure-enclaves"?  If not, "secure enclaves"
should be sufficient.

Hm, https://www.nxp.com/products/nxp-product-information/nxp-product-programs/edgelock-secure-enclave:EDGELOCK-SECURE-ENCLAVE
just says "Secure Enclave".



> - NXP EdgeLock Enclave on i.MX93 & i.MX8ULP
> 
> Signed-off-by: Pankaj Gupta <pankaj.gupta@nxp.com>
> ---
>  .../driver-api/firmware/other_interfaces.rst       | 119 +++++++++++++++++++++
>  1 file changed, 119 insertions(+)
> 
> diff --git a/Documentation/driver-api/firmware/other_interfaces.rst b/Documentation/driver-api/firmware/other_interfaces.rst
> index 06ac89adaafb..65e69396e22a 100644
> --- a/Documentation/driver-api/firmware/other_interfaces.rst
> +++ b/Documentation/driver-api/firmware/other_interfaces.rst
> @@ -49,3 +49,122 @@ of the requests on to a secure monitor (EL3).
>  
>  .. kernel-doc:: drivers/firmware/stratix10-svc.c
>     :export:
> +
> +NXP Secure Enclave Firmware Interface
> +=====================================
> +
> +Introduction
> +------------
> +The NXP's i.MX HW IP like EdgeLock-Enclave, V2X etc., creates an embedded secure

                             Edgelock Enclave

> +enclave within the SoC boundary to enable features like
> + - Hardware Security Module (HSM)
> + - Security Hardware Extension (SHE)
> + - Vehicular to Anything (V2X)
> +
> +Each of the above feature, is enabled through dedicated NXP H/W IP on the SoC.

                     features is enabled

> +On a single SoC, multiple hardware IP (or can say more than one secure enclave)

                                         (or more than one secure enclave)

> +can exists.

   can exist.

> +
> +NXP SoCs enabled with the such secure enclaves(SEs) IPs are:

                    with such

> +i.MX93, i.MX8ULP
> +
> +To communicate with one or more co-existing SE(s) on SoC, there is/are dedicated

hm, "co-existing" is a (UK) alternative for "coexisting" and since we accept
British spellings, it is OK.

> +messaging units(MU) per SE. Each co-existing 'se' can have one or multiple exclusive

                                  why not       'SE'
?

> +MU(s), dedicated to itself. None of the MU is shared between two SEs.

                                           MUs
or
                                           MU(s)

> +Communication of the MU is realized using the Linux mailbox driver.
> +
> +NXP Secure Enclave(SE) Interface
> +--------------------------------
> +All those SE interfaces 'se-if' that is/are dedicated to a particular SE, will be

                                                               no comma    ^

> +enumerated and provisioned under the very single 'SE' node.
> +
> +Each 'se-if', comprise of twp layers:

      no comma ^             two

> +- (C_DEV Layer) User-Space software-access interface.
> +- (Service Layer) OS-level software-access interface.
> +
> +   +--------------------------------------------+
> +   |            Character Device(C_DEV)         |
> +   |                                            |
> +   |   +---------+ +---------+     +---------+  |
> +   |   | misc #1 | | misc #2 | ... | misc #n |  |
> +   |   |  dev    | |  dev    |     | dev     |  |
> +   |   +---------+ +---------+     +---------+  |
> +   |        +-------------------------+         |
> +   |        | Misc. Dev Synchr. Logic |         |
> +   |        +-------------------------+         |
> +   |                                            |
> +   +--------------------------------------------+
> +
> +   +--------------------------------------------+
> +   |               Service Layer                |
> +   |                                            |
> +   |      +-----------------------------+       |
> +   |      | Message Serialization Logic |       |
> +   |      +-----------------------------+       |
> +   |          +---------------+                 |
> +   |          |  imx-mailbox  |                 |
> +   |          |   mailbox.c   |                 |
> +   |          +---------------+                 |
> +   |                                            |
> +   +--------------------------------------------+
> +
> +- service layer:
> +  This layer is responsible for ensuring the communication protocol, that is defined

                                                           no comma   ^

> +  for communication with firmware.
> +
> +  FW Communication protocol ensures two things:
> +  - Serializing the messages to be sent over an MU.
> +
> +  - FW can handle one command-message at a time.

                         command message

> +
> +- c_dev:
> +  This layer offers character device contexts, created as '/dev/<se>_mux_chx'.
> +  Using these multiple device contexts, that are getting multiplexed over a single MU,

                            no comma     ^ that are multiplexed over


> +  user-space application(s) can call fops like write/read to send the command-message,

                                                                         command message,

I prefer 'userspace' or 'user space' over 'user-space'.  'user-space' is the 3rd most used
of the 3 spellings in the kernel source tree.

> +  and read back the command-response-message to/from Firmware.

                       command response message

> +  fops like read & write uses the above defined service layer API(s) to communicate with

                            use

> +  Firmware.
> +
> +  Misc-device(/dev/<se>_mux_chn) synchronization protocol:
> +
> +                                Non-Secure               +   Secure
> +                                                         |
> +                                                         |
> +                  +---------+      +-------------+       |
> +                  | se_fw.c +<---->+imx-mailbox.c|       |
> +                  |         |      |  mailbox.c  +<-->+------+    +------+
> +                  +---+-----+      +-------------+    | MU X +<-->+ ELE |
> +                      |                               +------+    +------+
> +                      +----------------+                 |
> +                      |                |                 |
> +                      v                v                 |
> +                  logical           logical              |
> +                  receiver          waiter               |
> +                     +                 +                 |
> +                     |                 |                 |
> +                     |                 |                 |
> +                     |            +----+------+          |
> +                     |            |           |          |
> +                     |            |           |          |
> +              device_ctx     device_ctx     device_ctx   |
> +                                                         |
> +                User 0        User 1       User Y        |
> +                +------+      +------+     +------+      |
> +                |misc.c|      |misc.c|     |misc.c|      |
> + kernel space   +------+      +------+     +------+      |
> +                                                         |
> + +------------------------------------------------------ |
> +                    |             |           |          |
> + userspace     /dev/ele_muXch0    |           |          |
> +                          /dev/ele_muXch1     |          |
> +                                        /dev/ele_muXchY  |
> +                                                         |
> +
> +When a user sends a command to the firmware, it registers its device_ctx
> +as waiter of a response from firmware.
> +
> +Enclave's Firmware owns the storage management, over linux filesystem.

                                                        Linux

> +For this c_dev provisions a dedicated slave device called "receiver".
> +
> +.. kernel-doc:: drivers/firmware/imx/se_fw.c
> +   :export:
> 

-- 
~Randy