Messages in this thread | | | Subject | Re: [RESEND PATCH 02/10] crypto: sa2ul: Add crypto driver | From | Keerthy <> | Date | Fri, 28 Jun 2019 10:54:36 +0530 |
| |
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 >
| |