[EXT] Re: [PATCH 1/4] Documentation/firmware: add imx/se to other_interfaces

Tue May 14 03:03:06 PDT 2024


> -----Original Message-----
> From: Sascha Hauer <s.hauer at pengutronix.de>
> Sent: Monday, May 13, 2024 1:00 PM
> To: Pankaj Gupta <pankaj.gupta at nxp.com>
> Cc: Jonathan Corbet <corbet at lwn.net>; Rob Herring <robh+dt at kernel.org>;
> Krzysztof Kozlowski <krzysztof.kozlowski+dt at linaro.org>; Conor Dooley
> <conor+dt at kernel.org>; Shawn Guo <shawnguo at kernel.org>; Pengutronix
> Kernel Team <kernel at pengutronix.de>; Fabio Estevam
> <festevam at gmail.com>; linux-doc at vger.kernel.org; linux-
> kernel at vger.kernel.org; devicetree at vger.kernel.org; imx at lists.linux.dev;
> linux-arm-kernel at lists.infradead.org
> Subject: [EXT] Re: [PATCH 1/4] Documentation/firmware: add imx/se to
> other_interfaces
>
> Caution: This is an external email. Please take care when clicking links or
> opening attachments. When in doubt, report the message using the 'Report
> this email' button
>
>
> On Fri, May 10, 2024 at 06:57:27PM +0530, 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:
> > - NXP EdgeLock Enclave on i.MX93 & i.MX8ULP
> >
> > Signed-off-by: Pankaj Gupta <pankaj.gupta at nxp.com>
> > ---
> >  .../driver-api/firmware/other_interfaces.rst       | 126
> +++++++++++++++++++++
> >  1 file changed, 126 insertions(+)
> >
> > diff --git a/Documentation/driver-api/firmware/other_interfaces.rst
> > b/Documentation/driver-api/firmware/other_interfaces.rst
> > index 06ac89adaafb..c18c2d3e6e08 100644
> > --- a/Documentation/driver-api/firmware/other_interfaces.rst
> > +++ b/Documentation/driver-api/firmware/other_interfaces.rst
> > @@ -49,3 +49,129 @@ 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., creats an
> > +embedded secure
>
> s/creats/creates/
Accepted.

>
> > +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.
> > +On a single SoC, multiple hardware IP (or can say more than one
> > +secure enclave) can exists.
> > +
> > +NXP SoC(s) enabled with the such secure enclave(se) IP(s) are:
>
> There are already multiple NXP SoCs with a secure enclave, so you can drop
> the braces around the plural 's'.
Accepted.
>
> With (se) you refer to the acronym SE for secure enclave, right? If so, please
> write it in uppercase letters.

Accepted.
>
> > +i.MX93, i.MX8ULP
> > +
> > +To communicate with one or more co-existing 'se'(s) on SoC, there
> > +is/are dedicated messaging units(MU) per 'se'. Each co-existing 'se'
> > +can have one or multiple exclusive MU(s), dedicated to itself. None of the
> MU is shared between two se(s).
>
> between to SEs (the plural 's' is not optional here)
Accepted.

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

>
> Please use uppercase letters consistently for 'SE'
Accepted.

>
> > +enumerated and provisioned under the very single 'se' node.
> > +
> > +Each 'se-if', comprise of twp layers:
> > +- (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
> > +  for communication with firmware.
> > +
> > +  FW Communication protocol ensures two things:
> > +  - Serializing the multiple message(s) to be sent over an MU.
>
> Just "Serializing the messages to be sent over an MU"
Accepted.

>
> > +    A mutex locks instance "mu_lock" is instantiated per MU. It is taken to
> ensure
> > +    one message is sent over MU at a time. The lock "mu_lock" is unlocked,
> post sending
> > +    the message using the mbox api(s) exposed by mailbox kernel driver.
> > +
> > +  - FW can handle one command-message at a time.
> > +    Second command-message must wait till first command message is
> completely processed.
> > +    Hence, another mutex lock instance "mu_cmd_lock" is instantiated per
> MU. It is taken
> > +    to ensure one command-message is sent at a time, towards FW. This lock
> is not unlocked,
> > +    for the next command-message, till previous command message is
> processed completely.
>
> I don't think such implementation details belong here. They are easily
> changed in the code with the documentation update being forgotten. I'd just
> leave the bullet points here and add the detailed description as comments to
> the code.

Will re-visit and will try removing it.
>
> Sascha
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       |
> http://www.pe/
> ngutronix.de%2F&data=05%7C02%7Cpankaj.gupta%40nxp.com%7Ced22fbda
> 8a3143c2e6e708dc731e868e%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C
> 0%7C0%7C638511822140912745%7CUnknown%7CTWFpbGZsb3d8eyJWIjoi
> MC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C0%
> 7C%7C%7C&sdata=F4NbLNX37%2FB75Dv2q7e1DTRNaq6XZuAAMDdx9JFZ4U
> s%3D&reserved=0  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |