mbox series

[v2,0/5] Add Tegra Security Engine driver

Message ID 20231219125614.33062-1-akhilrajeev@nvidia.com
Headers show
Series Add Tegra Security Engine driver | expand

Message

Akhil R Dec. 19, 2023, 12:56 p.m. UTC 
Add support for Tegra Security Engine which can accelerates various
crypto algorithms. The Engine has two separate instances within for
AES and HASH algorithms respectively.

The driver registers two crypto engines - one for AES and another for
HASH algorithms and these operate independently and both uses the host1x
bus. Additionally, it provides  hardware-assisted key protection for up to
15 symmetric keys which it can use for the cipher operations.

v1->v2:
* Update probe errors with 'dev_err_probe'.
* Clean up function prototypes and redundant prints.
* Remove readl/writel wrappers.
* Fix test bot warnings.

Akhil R (5):
  dt-bindings: crypto: Add Tegra Security Engine
  gpu: host1x: Add Tegra SE to SID table
  crypto: tegra: Add Tegra Security Engine driver
  arm64: defconfig: Enable Tegra Security Engine
  arm64: tegra: Add Tegra Security Engine DT nodes

 .../crypto/nvidia,tegra234-se-aes.yaml        |   53 +
 .../crypto/nvidia,tegra234-se-hash.yaml       |   53 +
 MAINTAINERS                                   |    5 +
 arch/arm64/boot/dts/nvidia/tegra234.dtsi      |   16 +
 arch/arm64/configs/defconfig                  |    1 +
 drivers/crypto/Kconfig                        |    8 +
 drivers/crypto/Makefile                       |    1 +
 drivers/crypto/tegra/Makefile                 |    9 +
 drivers/crypto/tegra/tegra-se-aes.c           | 1932 +++++++++++++++++
 drivers/crypto/tegra/tegra-se-hash.c          | 1022 +++++++++
 drivers/crypto/tegra/tegra-se-key.c           |  155 ++
 drivers/crypto/tegra/tegra-se-main.c          |  439 ++++
 drivers/crypto/tegra/tegra-se.h               |  569 +++++
 drivers/gpu/host1x/dev.c                      |   24 +
 14 files changed, 4287 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
 create mode 100644 drivers/crypto/tegra/Makefile
 create mode 100644 drivers/crypto/tegra/tegra-se-aes.c
 create mode 100644 drivers/crypto/tegra/tegra-se-hash.c
 create mode 100644 drivers/crypto/tegra/tegra-se-key.c
 create mode 100644 drivers/crypto/tegra/tegra-se-main.c
 create mode 100644 drivers/crypto/tegra/tegra-se.h

Comments

Krzysztof Kozlowski Dec. 20, 2023, 3:44 p.m. UTC | #1 
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 <akhilrajeev@nvidia.com>
> ---
>  .../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.

> +  The Tegra Security Engine accelerates the following AES encryption/decryption
> +  algorithms.

s/./:/ and join lines? Wasn't that your intention here?

> +  AES-ECB, AES-CBC, AES-OFB, AES-XTS, AES-CTR, AES-GCM, AES-CCM, AES-CMAC
> +
> +maintainers:
> +  - Akhil R <akhilrajeev@nvidia.com>
> +
> +properties:
> +  compatible:
> +    const: nvidia,tegra234-se2-aes

Why "se2"?

Anyway, filename like compatible.


> +
> +  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 <akhilrajeev@nvidia.com>
> +
> +properties:
> +  compatible:
> +    const: nvidia,tegra234-se4-hash

What is se4?

Anyway, filename like compatible.



Best regards,
Krzysztof
Akhil R Dec. 28, 2023, 9:33 a.m. UTC | #2 
> 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 <akhilrajeev@nvidia.com>
> > ---
> >  .../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 <akhilrajeev@nvidia.com>
> > +
> > +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 <akhilrajeev@nvidia.com>
> > +
> > +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
Krzysztof Kozlowski Dec. 28, 2023, 10:29 a.m. UTC | #3 
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?

Best regards,
Krzysztof
Akhil R Dec. 29, 2023, 7:11 a.m. UTC | #4 
> 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
Krzysztof Kozlowski Dec. 30, 2023, 2:21 p.m. UTC | #5 
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
Akhil R Jan. 2, 2024, 4:27 a.m. UTC | #6 
> 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