lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f3965cf1-3f0a-15fe-7dd3-e83817a4ba3a@linaro.org>
Date:   Wed, 12 Jul 2023 20:38:43 +0200
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Pankaj Gupta <pankaj.gupta@....com>, shawnguo@...nel.org,
        s.hauer@...gutronix.de, kernel@...gutronix.de, clin@...e.com,
        conor+dt@...nel.org, pierre.gondois@....com, ping.bai@....com,
        xiaoning.wang@....com, wei.fang@....com, peng.fan@....com,
        haibo.chen@....com, festevam@...il.com, linux-imx@....com,
        davem@...emloft.net, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org,
        linux-arm-kernel@...ts.infradead.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, gaurav.jain@....com,
        alexander.stein@...tq-group.com, sahil.malhotra@....com,
        aisheng.dong@....com, V.Sethi@....com
Subject: Re: [PATCH v4 1/7] dt-bindings: arm: fsl: add se-fw binding doc

On 12/07/2023 14:12, Pankaj Gupta wrote:
> The NXP's i.MX EdgeLock Enclave, a HW IP creating an embedded
> secure enclave within the SoC boundary to enable features like
> - HSM
> - SHE
> - V2X
> 
> Communicates via message unit with linux kernel. This driver
> is enables communication ensuring well defined message sequence
> protocol between Application Core and enclave's firmware.
> 
> Driver configures multiple misc-device on the MU, for multiple
> user-space applications can communicate on single MU.
> 
> It exists on some i.MX processors. e.g. i.MX8ULP, i.MX93 etc.
> 
> Signed-off-by: Pankaj Gupta <pankaj.gupta@....com>
> ---
>  .../bindings/arm/freescale/fsl,se-fw.yaml     | 121 ++++++++++++++++++
>  1 file changed, 121 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/freescale/fsl,se-fw.yaml
> 
> diff --git a/Documentation/devicetree/bindings/arm/freescale/fsl,se-fw.yaml b/Documentation/devicetree/bindings/arm/freescale/fsl,se-fw.yaml
> new file mode 100644
> index 000000000000..7567da0b4c21
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/freescale/fsl,se-fw.yaml
> @@ -0,0 +1,121 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/arm/freescale/fsl,se-fw.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP i.MX EdgeLock Enclave Firmware (ELEFW)
> +
> +maintainers:
> +  - Pankaj Gupta <pankaj.gupta@....com>
> +
> +description: |
> +
> +  The NXP's i.MX EdgeLock Enclave, a HW IP creating an embedded
> +  secure enclave within the SoC boundary to enable features like
> +  - HSM
> +  - SHE
> +  - V2X
> +
> +  It uses message unit to communicate and coordinate to pass messages
> +  (e.g., data,  status and control) through its interfaces.
> +
> +  This driver configures multiple misc-devices on the MU, to exchange
> +  messages from User-space application and NXP's Edgelocke Enclave firmware.
> +  The driver ensures that the messages must follow the following protocol
> +  defined.
> +
> +                                     Non-Secure           +   Secure
> +                                                          |
> +                                                          |
> +                   +---------+      +-------------+       |
> +                   | ele_mu.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 ELE, it registers its device_ctx as
> +  waiter of a response from ELE.
> +
> +  A user can be registered as receiver of command from the ELE.
> +  Create char devices in /dev as channels of the form /dev/ele_muXchY with X
> +  the id of the driver and Y for each users. It allows to send and receive
> +  messages to the NXP EdgeLock Enclave IP on NXP SoC, where current possible
> +  value, i.e., supported SoC(s) are imx8ulp, imx93.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - fsl,imx-ele
> +      - fsl,imx93-ele
> +
> +  mboxes:
> +    description:
> +      A list of phandles of TX MU channels followed by a list of phandles of
> +      RX MU channels. The number of expected tx and rx channels is 1 TX, and
> +      1 RX channels. All MU channels must be within the same MU instance.
> +      Cross instances are not allowed. The MU instance to be used is S4MUAP
> +      for imx8ulp & imx93. Users need to ensure that used MU instance does not
> +      conflict with other execution environments.
> +    items:
> +      - description: TX0 MU channel
> +      - description: RX0 MU channel
> +
> +  mbox-names:
> +    items:
> +      - const: tx
> +      - const: rx
> +
> +  fsl,mu-did:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Owner of message-unit, is identified via Domain ID or did.

What is Domain ID?

> +
> +  fsl,mu-id:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Identifier to the message-unit among the multiple message-unit that exists on SoC.
> +      It is used to create the channels, default to 2

Do you expect then multiple ele nodes in the DTS? What are these two
properties and why they are fixed per SoC, but still embedded in DTS?

> +
> +

Drop stray blank line.

> +required:
> +  - compatible
> +  - mboxes
> +  - mbox-names
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    ele_mu: ele_mu {

No underscores in node names, generic node names, e.g. firmware. Look at
existing code.

> +      compatible = "fsl,imx93-ele";
> +      mbox-names = "tx", "rx";
> +      mboxes = <&s4muap 2 0
> +                &s4muap 3 0>;

Two items, not one.

> +      fsl,mu-did = <1>;
> +      fsl,mu-id = <1>;
> +    };

Plus you clearly did not test the binding and DTS. You said you did some
internal review, so I assume this also includes some testing. How did
you test your DTS?

Best regards,
Krzysztof

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ