lkml.org 
[lkml]   [2019]   [Jun]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RESEND PATCH 02/10] crypto: sa2ul: Add crypto driver
From
Date


On 28/06/19 10:37 AM, Eric Biggers wrote:
> On Fri, Jun 28, 2019 at 09:57:37AM +0530, Keerthy wrote:
>> The Security Accelerator (SA2_UL) subsystem provides hardware
>> cryptographic acceleration for the following use cases:
>> • Encryption and authentication for secure boot
>> • Encryption and authentication of content in applications
>> requiring DRM (digital rights management) and
>> content/asset protection
>> The device includes one instantiation of SA2_UL named SA2_UL0
>>
>> SA2_UL supports the following cryptographic industry standards to enable data authentication, data
>> integrity and data confidentiality.
>>
>> Crypto function library for software acceleration
>> o AES operation
>> o 3DES operation
>> o SHA1 operation
>> o MD5 operation
>> o SHA2 – 224, 256, 384, 512 operation
>>
>> Authentication supported via following hardware cores
>> o SHA1
>> o MD5
>> o SHA2 -224
>> o SHA2-256
>> o SHA2-384
>> o SHA2-512
>
> What about HMAC?
>
> Your actual driver only exposes HMAC-SHA*, not SHA* anything.
>
> What does the hardware actually support?

Hardware supports both SHA and HMAC-SHA

>
>> diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
>> index 603413f28fa3..b9a3fa026c74 100644
>> --- a/drivers/crypto/Kconfig
>> +++ b/drivers/crypto/Kconfig
>> @@ -785,4 +785,21 @@ config CRYPTO_DEV_CCREE
>>
>> source "drivers/crypto/hisilicon/Kconfig"
>>
>> +config CRYPTO_DEV_SA2UL
>> + tristate "Support for TI security accelerator"
>> + depends on ARCH_K3 || COMPILE_TEST
>> + select ARM64_CRYPTO
>> + select CRYPTO_AES
>> + select CRYPTO_AES_ARM64
>> + select CRYPTO_SHA1
>> + select CRYPTO_MD5
>> + select CRYPTO_ALGAPI
>> + select CRYPTO_AUTHENC
>> + select HW_RANDOM
>> + default m if ARCH_K3
>> + help
>> + Keystone devices include a security accelerator engine that may be
>> + used for crypto offload. Select this if you want to use hardware
>> + acceleration for cryptographic algorithms on these devices.
>
> This shouldn't be enabled by default. Note that arm64 defconfig sets ARCH_K3 as
> well as lots of other ARCH_* options, so clearly just because ARCH_K3 is set
> doesn't mean the kernel is being built specifically for your platform.

okay. I will remove that.

>
>> +/*
>> + * Mode Control Instructions for various Key lengths 128, 192, 256
>> + * For CBC (Cipher Block Chaining) mode for encryption
>> + */
>> +static u8 mci_cbc_enc_array[3][MODE_CONTROL_BYTES] = {
>> + { 0x21, 0x00, 0x00, 0x18, 0x88, 0x0a, 0xaa, 0x4b, 0x7e, 0x00,
>> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 },
>> + { 0x21, 0x00, 0x00, 0x18, 0x88, 0x4a, 0xaa, 0x4b, 0x7e, 0x00,
>> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 },
>> + { 0x21, 0x00, 0x00, 0x18, 0x88, 0x8a, 0xaa, 0x4b, 0x7e, 0x00,
>> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
>> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 },
>> +};
>
> Use 'const' for static constants.

Okay

>
>> +static int sa_aes_cbc_setkey(struct crypto_ablkcipher *tfm, const u8 *key,
>> + unsigned int keylen)
>> +{
>> + struct algo_data *ad = kzalloc(sizeof(*ad), GFP_KERNEL);
>
> Need to check from error for all memory allocations.
>
>> +static struct sa_alg_tmpl sa_algs[] = {
>> + {.type = CRYPTO_ALG_TYPE_ABLKCIPHER,
>
> ablkcipher API is deprecated. Use skcipher instead.

Okay

>
> (To be clear, these are just a few things I happened to notice from very quickly
> skimming through this patch. I don't have time to do a proper review of random
> drivers.)

I will incorporate the comments in v2.

Thanks for your quick review.

>
> - Eric
>

\
 
 \ /
  Last update: 2019-06-28 07:24    [W:0.096 / U:0.748 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site