2023-12-19 12:57:13

by Akhil R

[permalink] [raw]
Subject: [PATCH v2 1/5] dt-bindings: crypto: Add Tegra Security Engine

Add DT binding document for Tegra Security Engine.
The AES and HASH algorithms are handled independently by separate
engines within the Security Engine. These engines are registered
as two separate crypto engine drivers.

Signed-off-by: Akhil R <[email protected]>
---
.../crypto/nvidia,tegra234-se-aes.yaml | 53 +++++++++++++++++++
.../crypto/nvidia,tegra234-se-hash.yaml | 53 +++++++++++++++++++
2 files changed, 106 insertions(+)
create mode 100644 Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-aes.yaml
create mode 100644 Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-hash.yaml

diff --git a/Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-aes.yaml b/Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-aes.yaml
new file mode 100644
index 000000000000..35c2e701bd42
--- /dev/null
+++ b/Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-aes.yaml
@@ -0,0 +1,53 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/crypto/nvidia,tegra234-se-aes.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NVIDIA Tegra Security Engine for AES algorithms
+
+description: |
+ The Tegra Security Engine accelerates the following AES encryption/decryption
+ algorithms.
+ AES-ECB, AES-CBC, AES-OFB, AES-XTS, AES-CTR, AES-GCM, AES-CCM, AES-CMAC
+
+maintainers:
+ - Akhil R <[email protected]>
+
+properties:
+ compatible:
+ const: nvidia,tegra234-se2-aes
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ iommus:
+ maxItems: 1
+
+ dma-coherent: true
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - iommus
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/memory/tegra234-mc.h>
+ #include <dt-bindings/clock/tegra234-clock.h>
+
+ crypto@15820000 {
+ compatible = "nvidia,tegra234-se2-aes";
+ reg = <0x15820000 0x10000>;
+ clocks = <&bpmp TEGRA234_CLK_SE>;
+ iommus = <&smmu TEGRA234_SID_SES_SE1>;
+ dma-coherent;
+ };
+...
diff --git a/Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-hash.yaml b/Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-hash.yaml
new file mode 100644
index 000000000000..e3848e9a53b5
--- /dev/null
+++ b/Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-hash.yaml
@@ -0,0 +1,53 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/crypto/nvidia,tegra234-se-hash.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NVIDIA Tegra Security Engine for HASH algorithms
+
+description: |
+ The Tegra Security HASH Engine accelerates the following HASH functions.
+ SHA1, SHA224, SHA256, SHA384, SHA512, SHA3-224, SHA3-256, SHA3-384, SHA3-512
+ HMAC(SHA224), HMAC(SHA256), HMAC(SHA384), HMAC(SHA512)
+
+maintainers:
+ - Akhil R <[email protected]>
+
+properties:
+ compatible:
+ const: nvidia,tegra234-se4-hash
+
+ reg:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ iommus:
+ maxItems: 1
+
+ dma-coherent: true
+
+required:
+ - compatible
+ - reg
+ - clocks
+ - iommus
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/memory/tegra234-mc.h>
+ #include <dt-bindings/clock/tegra234-clock.h>
+
+ crypto@15840000 {
+ compatible = "nvidia,tegra234-se4-hash";
+ reg = <0x15840000 0x10000>;
+ clocks = <&bpmp TEGRA234_CLK_SE>;
+ iommus = <&smmu TEGRA234_SID_SES_SE2>;
+ dma-coherent;
+ };
+...
--
2.17.1



2023-12-28 09:34:08

by Akhil R

[permalink] [raw]
Subject: RE: [PATCH v2 1/5] dt-bindings: crypto: Add Tegra Security Engine

> On 19/12/2023 13:56, Akhil R wrote:
> > Add DT binding document for Tegra Security Engine.
> > The AES and HASH algorithms are handled independently by separate
> > engines within the Security Engine. These engines are registered
> > as two separate crypto engine drivers.
> >
> > Signed-off-by: Akhil R <[email protected]>
> > ---
> > .../crypto/nvidia,tegra234-se-aes.yaml | 53 +++++++++++++++++++
> > .../crypto/nvidia,tegra234-se-hash.yaml | 53 +++++++++++++++++++
> > 2 files changed, 106 insertions(+)
> > create mode 100644
> Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-aes.yaml
> > create mode 100644
> Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-hash.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-
> aes.yaml b/Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-
> aes.yaml
> > new file mode 100644
> > index 000000000000..35c2e701bd42
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-aes.yaml
> > @@ -0,0 +1,53 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/crypto/nvidia,tegra234-se-aes.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: NVIDIA Tegra Security Engine for AES algorithms
> > +
> > +description: |
>
> Do not need '|' unless you need to preserve formatting.
Okay.

>
> > + The Tegra Security Engine accelerates the following AES
> encryption/decryption
> > + algorithms.
>
> s/./:/ and join lines? Wasn't that your intention here?
Yes. Correct.

>
> > + AES-ECB, AES-CBC, AES-OFB, AES-XTS, AES-CTR, AES-GCM, AES-CCM, AES-
> CMAC
> > +
> > +maintainers:
> > + - Akhil R <[email protected]>
> > +
> > +properties:
> > + compatible:
> > + const: nvidia,tegra234-se2-aes
>
> Why "se2"?
>
> Anyway, filename like compatible.
The instance is named SE2 in the hardware. Thought to align the compatible as well.

There is one more instance with a slightly different capability, which we aren't using currently.
In case it needs to be supported, we would need a different compatible, but the driver will need
only minor changes. I suppose the current compatible can help to maintain the ABI with DT then.

>
>
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + clocks:
> > + maxItems: 1
> > +
> > + iommus:
> > + maxItems: 1
> > +
> > + dma-coherent: true
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - clocks
> > + - iommus
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > + - |
> > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > + #include <dt-bindings/memory/tegra234-mc.h>
> > + #include <dt-bindings/clock/tegra234-clock.h>
> > +
> > + crypto@15820000 {
> > + compatible = "nvidia,tegra234-se2-aes";
> > + reg = <0x15820000 0x10000>;
> > + clocks = <&bpmp TEGRA234_CLK_SE>;
> > + iommus = <&smmu TEGRA234_SID_SES_SE1>;
> > + dma-coherent;
> > + };
> > +...
> > diff --git a/Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-
> hash.yaml b/Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-
> hash.yaml
> > new file mode 100644
> > index 000000000000..e3848e9a53b5
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/crypto/nvidia,tegra234-se-
> hash.yaml
> > @@ -0,0 +1,53 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/crypto/nvidia,tegra234-se-hash.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: NVIDIA Tegra Security Engine for HASH algorithms
> > +
> > +description: |
> > + The Tegra Security HASH Engine accelerates the following HASH functions.
>
> Similar comment
>
> > + SHA1, SHA224, SHA256, SHA384, SHA512, SHA3-224, SHA3-256, SHA3-384,
> SHA3-512
> > + HMAC(SHA224), HMAC(SHA256), HMAC(SHA384), HMAC(SHA512)
> > +
>
> > +maintainers:
> > + - Akhil R <[email protected]>
> > +
> > +properties:
> > + compatible:
> > + const: nvidia,tegra234-se4-hash
>
> What is se4?
>
> Anyway, filename like compatible.
Similar to the above, the hardware name is SE4.

nvidia,tegra234-se-aes and nvidia,tegra234-se-hash does look good to me. But I am a bit concerned
about the ABI breakage in case, we need a different compatible for the remaining instance.

Thanks and Regards,
Akhil

2023-12-29 07:11:45

by Akhil R

[permalink] [raw]
Subject: RE: [PATCH v2 1/5] dt-bindings: crypto: Add Tegra Security Engine

> On 28/12/2023 10:33, Akhil R wrote:
> >>> +properties:
> >>> + compatible:
> >>> + const: nvidia,tegra234-se4-hash
> >>
> >> What is se4?
> >>
> >> Anyway, filename like compatible.
> > Similar to the above, the hardware name is SE4.
> >
> > nvidia,tegra234-se-aes and nvidia,tegra234-se-hash does look good to
> > me. But I am a bit concerned about the ABI breakage in case, we need a
> different compatible for the remaining instance.
>
> Isn't this a new device? What ABI breakage? What would be affected?

I meant a scenario where we need to support SE1 instance as well.

There is one more SE instance in Tegra, which is very similar to SE2 AES Engine.
But right now, it does not have a good use case in Linux. Now if we add
nvidia,tegra234-se-aes and nvidia,tegra234-se-hash, when SE1 needs to be
supported, I guess it would be confusing to find the right compatible for it.

Regards,
Akhil

2023-12-30 14:22:13

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH v2 1/5] dt-bindings: crypto: Add Tegra Security Engine

On 29/12/2023 08:11, Akhil R wrote:
>> On 28/12/2023 10:33, Akhil R wrote:
>>>>> +properties:
>>>>> + compatible:
>>>>> + const: nvidia,tegra234-se4-hash
>>>>
>>>> What is se4?
>>>>
>>>> Anyway, filename like compatible.
>>> Similar to the above, the hardware name is SE4.
>>>
>>> nvidia,tegra234-se-aes and nvidia,tegra234-se-hash does look good to
>>> me. But I am a bit concerned about the ABI breakage in case, we need a
>> different compatible for the remaining instance.
>>
>> Isn't this a new device? What ABI breakage? What would be affected?
>
> I meant a scenario where we need to support SE1 instance as well.
>
> There is one more SE instance in Tegra, which is very similar to SE2 AES Engine.
> But right now, it does not have a good use case in Linux. Now if we add
> nvidia,tegra234-se-aes and nvidia,tegra234-se-hash, when SE1 needs to be
> supported, I guess it would be confusing to find the right compatible for it.

Hm, I still do not see possibility of breaking of ABI, but sure, se4
makes sense if instances are really different. Otherwise could be one
compatible with some property. It kind of depends on the differences.

Anyway, name the file based on the compatible.

Best regards,
Krzysztof


2024-01-02 04:28:02

by Akhil R

[permalink] [raw]
Subject: RE: [PATCH v2 1/5] dt-bindings: crypto: Add Tegra Security Engine

> On 29/12/2023 08:11, Akhil R wrote:
> >> On 28/12/2023 10:33, Akhil R wrote:
> >>>>> +properties:
> >>>>> + compatible:
> >>>>> + const: nvidia,tegra234-se4-hash
> >>>>
> >>>> What is se4?
> >>>>
> >>>> Anyway, filename like compatible.
> >>> Similar to the above, the hardware name is SE4.
> >>>
> >>> nvidia,tegra234-se-aes and nvidia,tegra234-se-hash does look good to
> >>> me. But I am a bit concerned about the ABI breakage in case, we need a
> >> different compatible for the remaining instance.
> >>
> >> Isn't this a new device? What ABI breakage? What would be affected?
> >
> > I meant a scenario where we need to support SE1 instance as well.
> >
> > There is one more SE instance in Tegra, which is very similar to SE2 AES Engine.
> > But right now, it does not have a good use case in Linux. Now if we add
> > nvidia,tegra234-se-aes and nvidia,tegra234-se-hash, when SE1 needs to be
> > supported, I guess it would be confusing to find the right compatible for it.
>
> Hm, I still do not see possibility of breaking of ABI, but sure, se4
> makes sense if instances are really different. Otherwise could be one
> compatible with some property. It kind of depends on the differences.
>
> Anyway, name the file based on the compatible.

One compatible with some property looks to be a good approach to me.
Instances aren't totally different.
So, I will update the compatible to nvidia,tegra234-se-aes and
nvidia,tegra234-se-hash in the next revision.

Thanks for the comments and inputs.

Regards,
Akhil