diff mbox series

[v5,01/11] dt-bindings: arm: fsl: add imx-se-fw binding doc

Message ID 20230823073330.1712721-2-pankaj.gupta@nxp.com (mailing list archive)
State New, archived
Headers show
Series firmware: imx: NXP Secure-Enclave FW Driver | expand

Commit Message

Pankaj Gupta Aug. 23, 2023, 7:33 a.m. UTC
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@nxp.com>
---
 .../bindings/firmware/fsl,imx-se-fw.yaml      | 121 ++++++++++++++++++
 1 file changed, 121 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/firmware/fsl,imx-se-fw.yaml

Comments

Rob Herring (Arm) Aug. 23, 2023, 8:28 a.m. UTC | #1
On Wed, 23 Aug 2023 13:03:20 +0530, 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@nxp.com>
> ---
>  .../bindings/firmware/fsl,imx-se-fw.yaml      | 121 ++++++++++++++++++
>  1 file changed, 121 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/firmware/fsl,imx-se-fw.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/firmware/fsl,imx-se-fw.example.dtb: se-fw: 'mu-id' is a required property
	from schema $id: http://devicetree.org/schemas/firmware/fsl,imx-se-fw.yaml#
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/firmware/fsl,imx-se-fw.example.dtb: se-fw: 'fsl,mu-id' does not match any of the regexes: 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/firmware/fsl,imx-se-fw.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230823073330.1712721-2-pankaj.gupta@nxp.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Pankaj Gupta Aug. 23, 2023, 10:42 a.m. UTC | #2
> -----Original Message-----
> From: Rob Herring <robh@kernel.org>
> Sent: Wednesday, August 23, 2023 1:59 PM
> To: Pankaj Gupta <pankaj.gupta@nxp.com>
> Cc: linux-arm-kernel@lists.infradead.org; Bough Chen
> <haibo.chen@nxp.com>; festevam@gmail.com; Peng Fan
> <peng.fan@nxp.com>; conor+dt@kernel.org; Varun Sethi
> <V.Sethi@nxp.com>; Sahil Malhotra <sahil.malhotra@nxp.com>;
> shawnguo@kernel.org; s.hauer@pengutronix.de; clin@suse.com; Gaurav Jain
> <gaurav.jain@nxp.com>; dl-linux-imx <linux-imx@nxp.com>;
> robh+dt@kernel.org; Jacky Bai <ping.bai@nxp.com>;
> krzysztof.kozlowski+dt@linaro.org; Aisheng Dong <aisheng.dong@nxp.com>;
> Clark Wang <xiaoning.wang@nxp.com>; devicetree@vger.kernel.org;
> davem@davemloft.net; Wei Fang <wei.fang@nxp.com>;
> alexander.stein@ew.tq-group.com; pierre.gondois@arm.com; linux-
> kernel@vger.kernel.org; kernel@pengutronix.de
> Subject: [EXT] Re: [PATCH v5 01/11] dt-bindings: arm: fsl: add imx-se-fw
> binding doc
> 
> 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 Wed, 23 Aug 2023 13:03:20 +0530, 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@nxp.com>
> > ---
> >  .../bindings/firmware/fsl,imx-se-fw.yaml      | 121 ++++++++++++++++++
> >  1 file changed, 121 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/firmware/fsl,imx-se-fw.yaml
> >
> 
> My bot found errors running 'make DT_CHECKER_FLAGS=-m
> dt_binding_check'
> on your patch (DT_CHECKER_FLAGS is new in v5.13):
> 
> yamllint warnings/errors:
> 
> dtschema/dtc warnings/errors:
> /builds/robherring/dt-review-
> ci/linux/Documentation/devicetree/bindings/firmware/fsl,imx-se-
> fw.example.dtb: se-fw: 'mu-id' is a required property
>         from schema $id:
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetr
> ee.org%2Fschemas%2Ffirmware%2Ffsl%2Cimx-se-
> fw.yaml%23&data=05%7C01%7Cpankaj.gupta%40nxp.com%7C7e28c89d24c6
> 4b9c000108dba3b2f2d0%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7
> C0%7C638283761210875688%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4
> wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C
> %7C%7C&sdata=SFldwBvByjOkEf%2FlzBmMJ3oZI9cBdWJe1ZmSh2AsflA%3D&r
> eserved=0
> /builds/robherring/dt-review-
> ci/linux/Documentation/devicetree/bindings/firmware/fsl,imx-se-
> fw.example.dtb: se-fw: 'fsl,mu-id' does not match any of the regexes: 'pinctrl-
> [0-9]+'
>         from schema $id:
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetr
> ee.org%2Fschemas%2Ffirmware%2Ffsl%2Cimx-se-
> fw.yaml%23&data=05%7C01%7Cpankaj.gupta%40nxp.com%7C7e28c89d24c6
> 4b9c000108dba3b2f2d0%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7
> C0%7C638283761210875688%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4
> wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C
> %7C%7C&sdata=SFldwBvByjOkEf%2FlzBmMJ3oZI9cBdWJe1ZmSh2AsflA%3D&r
> eserved=0
> 
> doc reference errors (make refcheckdocs):
> 
> See
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchw
> ork.ozlabs.org%2Fproject%2Fdevicetree-
> bindings%2Fpatch%2F20230823073330.1712721-2-
> pankaj.gupta%40nxp.com&data=05%7C01%7Cpankaj.gupta%40nxp.com%7C7
> e28c89d24c64b9c000108dba3b2f2d0%7C686ea1d3bc2b4c6fa92cd99c5c30
> 1635%7C0%7C0%7C638283761210875688%7CUnknown%7CTWFpbGZsb3d8
> eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D
> %7C3000%7C%7C%7C&sdata=afDClssaony%2B7aLwRX8VWJ1QyPrfZJJnEOcyU
> LYxR4g%3D&reserved=0
> 
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
> 
> If you already ran 'make dt_binding_check' and didn't see the above error(s),
> then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.

Accepted.
Re-submitting the patch V6 by end of this, after fixing it.
Rob Herring (Arm) Aug. 23, 2023, 12:43 p.m. UTC | #3
On Wed, Aug 23, 2023 at 01:03:20PM +0530, 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@nxp.com>
> ---

v5? Where's the changelog for *this* patch?

>  .../bindings/firmware/fsl,imx-se-fw.yaml      | 121 ++++++++++++++++++
>  1 file changed, 121 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/firmware/fsl,imx-se-fw.yaml
> 
> diff --git a/Documentation/devicetree/bindings/firmware/fsl,imx-se-fw.yaml b/Documentation/devicetree/bindings/firmware/fsl,imx-se-fw.yaml
> new file mode 100644
> index 000000000000..f7230f93e56d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/firmware/fsl,imx-se-fw.yaml
> @@ -0,0 +1,121 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/firmware/fsl,imx-se-fw.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP i.MX EdgeLock Enclave Firmware (ELEFW)
> +
> +maintainers:
> +  - Pankaj Gupta <pankaj.gupta@nxp.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 firmware, it registers its device_ctx
> +  as waiter of a response from firmware.
> +
> +  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 firmware on NXP SoC, where current
> +  possible value, i.e., supported SoC(s) are imx8ulp, imx93.

Looks like a bunch of Linux details which don't belong in the binding.

Why do you need your own custom interface to userspace? No one else has 
a similar feature in their platforms? Something like virtio or rpmsg 
doesn't work?

> +
> +properties:
> +  compatible:
> +    enum:
> +      - fsl,imx8ulp-se-fw
> +      - fsl,imx93-se-fw
> +
> +  mboxes:
> +    description:
> +      All MU channels must be within the same MU instance. Cross instances are
> +      not allowed. 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:
> +      By design, Domain is a clean separated processing island with separate power,
> +      clocking and peripheral; but with a tightly integrated bus fabric for efficient
> +      communication. The Domain to which this message-unit is associated, is identified
> +      via Domain ID or did.
> +
> +  sram-pool:

I believe 'sram' is the somewhat standard property to refer to an SRAM 
region.

> +    items:
> +      - description: SRAM memory instance.

Used for what?

> +
> +  memory-region:
> +    items:
> +      - description: Reserved memory region that can be accessed by firmware. Used for
> +          exchanging the buffers between driver and firmware.
> +
> +required:
> +  - compatible
> +  - mboxes
> +  - mbox-names
> +  - mu-id
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    ele_fw: se-fw {
> +      compatible = "fsl,imx8ulp-se-fw";
> +      mbox-names = "tx", "rx";
> +      mboxes = <&s4muap 0 0>, <&s4muap 1 0>;
> +      fsl,mu-id = <2>;
> +    };
> -- 
> 2.34.1
>
Krzysztof Kozlowski Aug. 24, 2023, 6:45 p.m. UTC | #4
On 23/08/2023 14:43, Rob Herring wrote:
>> +                                                          |
>> +  +------------------------------------------------------ |
>> +                     |             |           |          |
>> +  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.
>> +
>> +  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 firmware on NXP SoC, where current
>> +  possible value, i.e., supported SoC(s) are imx8ulp, imx93.
> 
> Looks like a bunch of Linux details which don't belong in the binding.
> 
> Why do you need your own custom interface to userspace? No one else has 
> a similar feature in their platforms? Something like virtio or rpmsg 
> doesn't work?

+Cc Greg,

I doubt they care. This is some stub-driver to pass messages from
user-space to the firmware. The interface is undocumented, without
examples and no user-space user.

Best regards,
Krzysztof
Greg Kroah-Hartman Aug. 24, 2023, 7:23 p.m. UTC | #5
On Thu, Aug 24, 2023 at 08:45:41PM +0200, Krzysztof Kozlowski wrote:
> On 23/08/2023 14:43, Rob Herring wrote:
> >> +                                                          |
> >> +  +------------------------------------------------------ |
> >> +                     |             |           |          |
> >> +  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.
> >> +
> >> +  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 firmware on NXP SoC, where current
> >> +  possible value, i.e., supported SoC(s) are imx8ulp, imx93.
> > 
> > Looks like a bunch of Linux details which don't belong in the binding.
> > 
> > Why do you need your own custom interface to userspace? No one else has 
> > a similar feature in their platforms? Something like virtio or rpmsg 
> > doesn't work?
> 
> +Cc Greg,
> 
> I doubt they care. This is some stub-driver to pass messages from
> user-space to the firmware. The interface is undocumented, without
> examples and no user-space user.

Great, no user?  Let's delete the code then :)

thanks,

greg k-h
Varun Sethi Aug. 25, 2023, 7:56 a.m. UTC | #6
Please find comments inline.


Regards
Varun

> -----Original Message-----
> From: Pankaj Gupta <pankaj.gupta@nxp.com>
> Sent: Friday, August 25, 2023 12:56 PM
> To: Varun Sethi <V.Sethi@nxp.com>
> Subject: FW: [EXT] Re: [PATCH v5 01/11] dt-bindings: arm: fsl: add imx-se-fw
> binding doc
>
>
>
> -----Original Message-----
> From: Krzysztof Kozlowski <krzk@kernel.org>
> Sent: Friday, August 25, 2023 12:16 AM
> To: Rob Herring <robh@kernel.org>; Pankaj Gupta <pankaj.gupta@nxp.com>
> Cc: shawnguo@kernel.org; s.hauer@pengutronix.de;
> kernel@pengutronix.de; clin@suse.com; conor+dt@kernel.org;
> pierre.gondois@arm.com; Jacky Bai <ping.bai@nxp.com>; Clark Wang
> <xiaoning.wang@nxp.com>; Wei Fang <wei.fang@nxp.com>; Peng Fan
> <peng.fan@nxp.com>; Bough Chen <haibo.chen@nxp.com>;
> festevam@gmail.com; dl-linux-imx <linux-imx@nxp.com>;
> davem@davemloft.net; krzysztof.kozlowski+dt@linaro.org; linux-arm-
> kernel@lists.infradead.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; Gaurav Jain <gaurav.jain@nxp.com>;
> alexander.stein@ew.tq-group.com; Sahil Malhotra
> <sahil.malhotra@nxp.com>; Aisheng Dong <aisheng.dong@nxp.com>; Varun
> Sethi <V.Sethi@nxp.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>
> Subject: [EXT] Re: [PATCH v5 01/11] dt-bindings: arm: fsl: add imx-se-fw
> binding doc
>
> 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 23/08/2023 14:43, Rob Herring wrote:
> >> +                                                          |
> >> +  +------------------------------------------------------ |
> >> +                     |             |           |          |
> >> +  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.
> >> +
> >> +  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 firmware on NXP SoC, where current  possible value, i.e., supported
> SoC(s) are imx8ulp, imx93.
> >
> > Looks like a bunch of Linux details which don't belong in the binding.
> >
> > Why do you need your own custom interface to userspace? No one else
> > has a similar feature in their platforms? Something like virtio or
> > rpmsg doesn't work?
>
> +Cc Greg,
>
> I doubt they care. This is some stub-driver to pass messages from user-space
> to the firmware. The interface is undocumented, without examples and no
> user-space user.
>
> Best regards,
> Krzysztof

[Varun] The goal of the driver is to pass messages between firmware and user 
space/kernel consumers.
At the same time driver also enables firmware to use the storage via Linux 
user space. We do have
middleware applications that utilize the driver. These applications leverage 
cryptographic operations and trust
provisioning capabilities offered by the firmware. We can provide references 
to the middleware applications in the next
version of the patch. We do plan to enable the kernel crypto API interface to 
leverage operations provided by the firmware.

The operations that are exposed by the firmware are performance-sensitive and 
session-based (thus serialized). So, the Virtio/rpmsg model
is not suited for our implementation. Also, the secure enclave IP, with which 
the driver facilitates the communication is an opaque block
that doesn't fit the rpmsg model.
Varun Sethi Aug. 28, 2023, 6 a.m. UTC | #7
Hi Greg,

> -----Original Message-----
> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Sent: Friday, August 25, 2023 12:54 AM
> To: Krzysztof Kozlowski <krzk@kernel.org>
> Cc: Rob Herring <robh@kernel.org>; Pankaj Gupta
> <pankaj.gupta@nxp.com>; shawnguo@kernel.org; s.hauer@pengutronix.de;
> kernel@pengutronix.de; clin@suse.com; conor+dt@kernel.org;
> pierre.gondois@arm.com; Jacky Bai <ping.bai@nxp.com>; Clark Wang
> <xiaoning.wang@nxp.com>; Wei Fang <wei.fang@nxp.com>; Peng Fan
> <peng.fan@nxp.com>; Bough Chen <haibo.chen@nxp.com>;
> festevam@gmail.com; dl-linux-imx <linux-imx@nxp.com>;
> davem@davemloft.net; krzysztof.kozlowski+dt@linaro.org; linux-arm-
> kernel@lists.infradead.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; Gaurav Jain <gaurav.jain@nxp.com>;
> alexander.stein@ew.tq-group.com; Sahil Malhotra
> <sahil.malhotra@nxp.com>; Aisheng Dong <aisheng.dong@nxp.com>; Varun
> Sethi <V.Sethi@nxp.com>
> Subject: [EXT] Re: [PATCH v5 01/11] dt-bindings: arm: fsl: add imx-se-fw
> binding doc
> 
> 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 Thu, Aug 24, 2023 at 08:45:41PM +0200, Krzysztof Kozlowski wrote:
> > On 23/08/2023 14:43, Rob Herring wrote:
> > >> +                                                          |
> > >> +  +------------------------------------------------------ |
> > >> +                     |             |           |          |
> > >> +  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.
> > >> +
> > >> +  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 firmware on NXP SoC, where current  possible
> value, i.e., supported SoC(s) are imx8ulp, imx93.
> > >
> > > Looks like a bunch of Linux details which don't belong in the binding.
> > >
> > > Why do you need your own custom interface to userspace? No one else
> > > has a similar feature in their platforms? Something like virtio or
> > > rpmsg doesn't work?
> >
> > +Cc Greg,
> >
> > I doubt they care. This is some stub-driver to pass messages from
> > user-space to the firmware. The interface is undocumented, without
> > examples and no user-space user.
> 
> Great, no user?  Let's delete the code then :)
> 
[Varun] We do have middleware applications that utilize the driver.
Following
are the links:
https://github.com/nxp-imx/imx-secure-enclave
https://github.com/nxp-imx/imx-smw

Also, we plan to enable the kernel crypto API interface to 
leverage operations provided by the firmware.

Regards
Varun
Krzysztof Kozlowski Aug. 28, 2023, 6:55 a.m. UTC | #8
On 28/08/2023 08:00, Varun Sethi wrote:
> Hi Greg,
> 
>> -----Original Message-----
>> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Sent: Friday, August 25, 2023 12:54 AM
>> To: Krzysztof Kozlowski <krzk@kernel.org>
>> Cc: Rob Herring <robh@kernel.org>; Pankaj Gupta
>> <pankaj.gupta@nxp.com>; shawnguo@kernel.org; s.hauer@pengutronix.de;
>> kernel@pengutronix.de; clin@suse.com; conor+dt@kernel.org;
>> pierre.gondois@arm.com; Jacky Bai <ping.bai@nxp.com>; Clark Wang
>> <xiaoning.wang@nxp.com>; Wei Fang <wei.fang@nxp.com>; Peng Fan
>> <peng.fan@nxp.com>; Bough Chen <haibo.chen@nxp.com>;
>> festevam@gmail.com; dl-linux-imx <linux-imx@nxp.com>;
>> davem@davemloft.net; krzysztof.kozlowski+dt@linaro.org; linux-arm-
>> kernel@lists.infradead.org; devicetree@vger.kernel.org; linux-
>> kernel@vger.kernel.org; Gaurav Jain <gaurav.jain@nxp.com>;
>> alexander.stein@ew.tq-group.com; Sahil Malhotra
>> <sahil.malhotra@nxp.com>; Aisheng Dong <aisheng.dong@nxp.com>; Varun
>> Sethi <V.Sethi@nxp.com>
>> Subject: [EXT] Re: [PATCH v5 01/11] dt-bindings: arm: fsl: add imx-se-fw
>> binding doc
>>
>> 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 Thu, Aug 24, 2023 at 08:45:41PM +0200, Krzysztof Kozlowski wrote:
>>> On 23/08/2023 14:43, Rob Herring wrote:
>>>>> +                                                          |
>>>>> +  +------------------------------------------------------ |
>>>>> +                     |             |           |          |
>>>>> +  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.
>>>>> +
>>>>> +  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 firmware on NXP SoC, where current  possible
>> value, i.e., supported SoC(s) are imx8ulp, imx93.
>>>>
>>>> Looks like a bunch of Linux details which don't belong in the binding.
>>>>
>>>> Why do you need your own custom interface to userspace? No one else
>>>> has a similar feature in their platforms? Something like virtio or
>>>> rpmsg doesn't work?
>>>
>>> +Cc Greg,
>>>
>>> I doubt they care. This is some stub-driver to pass messages from
>>> user-space to the firmware. The interface is undocumented, without
>>> examples and no user-space user.
>>
>> Great, no user?  Let's delete the code then :)
>>
> [Varun] We do have middleware applications that utilize the driver.
> Following
> are the links:
> https://github.com/nxp-imx/imx-secure-enclave
> https://github.com/nxp-imx/imx-smw
> 

Why this is not explained in the cover letter and in the patch adding
the interfaces? You still need to document and explain the interface.

Best regards,
Krzysztof
Varun Sethi Aug. 28, 2023, 9:14 a.m. UTC | #9
Hi Krzysztof,

> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: Monday, August 28, 2023 12:25 PM
> To: Varun Sethi <V.Sethi@nxp.com>; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; Krzysztof Kozlowski <krzk@kernel.org>
> Cc: Rob Herring <robh@kernel.org>; Pankaj Gupta
> <pankaj.gupta@nxp.com>; shawnguo@kernel.org; s.hauer@pengutronix.de;
> kernel@pengutronix.de; clin@suse.com; conor+dt@kernel.org;
> pierre.gondois@arm.com; Jacky Bai <ping.bai@nxp.com>; Clark Wang
> <xiaoning.wang@nxp.com>; Wei Fang <wei.fang@nxp.com>; Peng Fan
> <peng.fan@nxp.com>; Bough Chen <haibo.chen@nxp.com>;
> festevam@gmail.com; dl-linux-imx <linux-imx@nxp.com>;
> davem@davemloft.net; krzysztof.kozlowski+dt@linaro.org; linux-arm-
> kernel@lists.infradead.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; Gaurav Jain <gaurav.jain@nxp.com>;
> alexander.stein@ew.tq-group.com; Sahil Malhotra
> <sahil.malhotra@nxp.com>; Aisheng Dong <aisheng.dong@nxp.com>
> Subject: Re: [EXT] Re: [PATCH v5 01/11] dt-bindings: arm: fsl: add
imx-se-fw
> binding doc
> 
> 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 28/08/2023 08:00, Varun Sethi wrote:
> > Hi Greg,
> >
> >> -----Original Message-----
> >> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >> Sent: Friday, August 25, 2023 12:54 AM
> >> To: Krzysztof Kozlowski <krzk@kernel.org>
> >> Cc: Rob Herring <robh@kernel.org>; Pankaj Gupta
> >> <pankaj.gupta@nxp.com>; shawnguo@kernel.org;
> s.hauer@pengutronix.de;
> >> kernel@pengutronix.de; clin@suse.com; conor+dt@kernel.org;
> >> pierre.gondois@arm.com; Jacky Bai <ping.bai@nxp.com>; Clark Wang
> >> <xiaoning.wang@nxp.com>; Wei Fang <wei.fang@nxp.com>; Peng Fan
> >> <peng.fan@nxp.com>; Bough Chen <haibo.chen@nxp.com>;
> >> festevam@gmail.com; dl-linux-imx <linux-imx@nxp.com>;
> >> davem@davemloft.net; krzysztof.kozlowski+dt@linaro.org; linux-arm-
> >> kernel@lists.infradead.org; devicetree@vger.kernel.org; linux-
> >> kernel@vger.kernel.org; Gaurav Jain <gaurav.jain@nxp.com>;
> >> alexander.stein@ew.tq-group.com; Sahil Malhotra
> >> <sahil.malhotra@nxp.com>; Aisheng Dong <aisheng.dong@nxp.com>;
> Varun
> >> Sethi <V.Sethi@nxp.com>
> >> Subject: [EXT] Re: [PATCH v5 01/11] dt-bindings: arm: fsl: add
> >> imx-se-fw binding doc
> >>
> >> 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 Thu, Aug 24, 2023 at 08:45:41PM +0200, Krzysztof Kozlowski wrote:
> >>> On 23/08/2023 14:43, Rob Herring wrote:
> >>>>> +                                                          |
> >>>>> +  +------------------------------------------------------ |
> >>>>> +                     |             |           |          |
> >>>>> +  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.
> >>>>> +
> >>>>> +  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 firmware on NXP SoC, where current  possible
> >> value, i.e., supported SoC(s) are imx8ulp, imx93.
> >>>>
> >>>> Looks like a bunch of Linux details which don't belong in the
binding.
> >>>>
> >>>> Why do you need your own custom interface to userspace? No one
> else
> >>>> has a similar feature in their platforms? Something like virtio or
> >>>> rpmsg doesn't work?
> >>>
> >>> +Cc Greg,
> >>>
> >>> I doubt they care. This is some stub-driver to pass messages from
> >>> user-space to the firmware. The interface is undocumented, without
> >>> examples and no user-space user.
> >>
> >> Great, no user?  Let's delete the code then :)
> >>
> > [Varun] We do have middleware applications that utilize the driver.
> > Following
> > are the links:
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> > ub.com%2Fnxp-imx%2Fimx-secure-
> enclave&data=05%7C01%7CV.Sethi%40nxp.com
> > %7Ceda172f10a684cc926a908dba793b776%7C686ea1d3bc2b4c6fa92cd99c5c
> 301635
> > %7C0%7C0%7C638288025110398317%7CUnknown%7CTWFpbGZsb3d8eyJW
> IjoiMC4wLjAw
> >
> MDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%
> 7C&sda
> >
> ta=jJ%2FZl1myldPaciswIps2dDuuzp%2BqXGZDnvh07yD%2BEFU%3D&reserve
> d=0
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgith
> > ub.com%2Fnxp-imx%2Fimx-
> smw&data=05%7C01%7CV.Sethi%40nxp.com%7Ceda172f1
> >
> 0a684cc926a908dba793b776%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7
> C0%7C
> >
> 638288025110398317%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwM
> DAiLCJQIjo
> >
> iV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=U
> VjBwmzZ
> > myHu%2Fmy6WZiQcz2qksA67W1BtwFIJ8%2Ft7nQ%3D&reserved=0
> >
> 
> Why this is not explained in the cover letter and in the patch adding the
> interfaces? You still need to document and explain the interface.
> 

[Varun] Following information would be added in the next version of the
patchset:
1. Details of the users for this driver.
2. The ABI document would be enhanced to provide information about the
interfaces leveraged
by the user space middleware applications.

Please let me know if this would be sufficient.

Regards
Varun Sethi
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/firmware/fsl,imx-se-fw.yaml b/Documentation/devicetree/bindings/firmware/fsl,imx-se-fw.yaml
new file mode 100644
index 000000000000..f7230f93e56d
--- /dev/null
+++ b/Documentation/devicetree/bindings/firmware/fsl,imx-se-fw.yaml
@@ -0,0 +1,121 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/firmware/fsl,imx-se-fw.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP i.MX EdgeLock Enclave Firmware (ELEFW)
+
+maintainers:
+  - Pankaj Gupta <pankaj.gupta@nxp.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 firmware, it registers its device_ctx
+  as waiter of a response from firmware.
+
+  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 firmware on NXP SoC, where current
+  possible value, i.e., supported SoC(s) are imx8ulp, imx93.
+
+properties:
+  compatible:
+    enum:
+      - fsl,imx8ulp-se-fw
+      - fsl,imx93-se-fw
+
+  mboxes:
+    description:
+      All MU channels must be within the same MU instance. Cross instances are
+      not allowed. 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:
+      By design, Domain is a clean separated processing island with separate power,
+      clocking and peripheral; but with a tightly integrated bus fabric for efficient
+      communication. The Domain to which this message-unit is associated, is identified
+      via Domain ID or did.
+
+  sram-pool:
+    items:
+      - description: SRAM memory instance.
+
+  memory-region:
+    items:
+      - description: Reserved memory region that can be accessed by firmware. Used for
+          exchanging the buffers between driver and firmware.
+
+required:
+  - compatible
+  - mboxes
+  - mbox-names
+  - mu-id
+
+additionalProperties: false
+
+examples:
+  - |
+    ele_fw: se-fw {
+      compatible = "fsl,imx8ulp-se-fw";
+      mbox-names = "tx", "rx";
+      mboxes = <&s4muap 0 0>, <&s4muap 1 0>;
+      fsl,mu-id = <2>;
+    };