diff mbox

[08/10] crypto: omap-aes: gmc: Add algo info

Message ID 1435814320-30347-9-git-send-email-lokeshvutla@ti.com (mailing list archive)
State Changes Requested
Delegated to: Herbert Xu
Headers show

Commit Message

Lokesh Vutla July 2, 2015, 5:18 a.m. UTC
Now the driver supports gcm mode, add omap-aes-gcm
algo info to omap-aes driver.

Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
---
 drivers/crypto/omap-aes.c |   22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

Comments

Stephan Mueller July 2, 2015, 8 a.m. UTC | #1
Am Donnerstag, 2. Juli 2015, 10:48:38 schrieb Lokesh Vutla:

Hi Lokesh,

>Now the driver supports gcm mode, add omap-aes-gcm
>algo info to omap-aes driver.
>
>Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>---
> drivers/crypto/omap-aes.c |   22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>
>diff --git a/drivers/crypto/omap-aes.c b/drivers/crypto/omap-aes.c
>index e5e9a19..11f3850 100644
>--- a/drivers/crypto/omap-aes.c
>+++ b/drivers/crypto/omap-aes.c
>@@ -789,6 +789,28 @@ static struct crypto_alg algs_ctr[] = {
> 		.decrypt	= omap_aes_ctr_decrypt,
> 	}
> },
>+{
>+	.cra_name		= "gcm(aes)",
>+	.cra_driver_name	= "gcm-aes-omap",
>+	.cra_priority		= 100,

Why did you choose the priority 100? The software implementations commonly use 
100. crypto/gcm.c uses the prio of the underlying cipher. In case of ARM, 
there seem to be assembler implementations of AES which have the prio of 200 
or 300. So, such software implementation of gcm(aes) would have a higher 
precedence than your hw implementation.

So, if a user would use gcm(aes), isn't it more likely that he gets the 
software implementation?

>+	.cra_flags		= CRYPTO_ALG_TYPE_AEAD | CRYPTO_ALG_ASYNC |
>+				  CRYPTO_ALG_KERN_DRIVER_ONLY,
>+	.cra_blocksize		= AES_BLOCK_SIZE,
>+	.cra_ctxsize		= sizeof(struct omap_aes_ctx),
>+	.cra_alignmask		= 0xf,
>+	.cra_type		= &crypto_aead_type,
>+	.cra_module		= THIS_MODULE,
>+	.cra_init		= omap_aes_gcm_cra_init,
>+	.cra_exit		= omap_aes_cra_exit,
>+	.cra_u.aead = {
>+		.maxauthsize	= AES_BLOCK_SIZE,
>+		.geniv		= "eseqiv",
>+		.ivsize		= AES_BLOCK_SIZE,
>+		.setkey		= omap_aes_gcm_setkey,
>+		.encrypt	= omap_aes_gcm_encrypt,
>+		.decrypt	= omap_aes_gcm_decrypt,
>+	}
>+},
> };
>
> static struct omap_aes_algs_info omap_aes_algs_info_ecb_cbc[] = {


Ciao
Stephan
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lokesh Vutla July 2, 2015, 9:54 a.m. UTC | #2
On Thursday 02 July 2015 01:30 PM, Stephan Mueller wrote:
> Am Donnerstag, 2. Juli 2015, 10:48:38 schrieb Lokesh Vutla:
> 
> Hi Lokesh,
> 
>> Now the driver supports gcm mode, add omap-aes-gcm
>> algo info to omap-aes driver.
>>
>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
>> ---
>> drivers/crypto/omap-aes.c |   22 ++++++++++++++++++++++
>> 1 file changed, 22 insertions(+)
>>
>> diff --git a/drivers/crypto/omap-aes.c b/drivers/crypto/omap-aes.c
>> index e5e9a19..11f3850 100644
>> --- a/drivers/crypto/omap-aes.c
>> +++ b/drivers/crypto/omap-aes.c
>> @@ -789,6 +789,28 @@ static struct crypto_alg algs_ctr[] = {
>> 		.decrypt	= omap_aes_ctr_decrypt,
>> 	}
>> },
>> +{
>> +	.cra_name		= "gcm(aes)",
>> +	.cra_driver_name	= "gcm-aes-omap",
>> +	.cra_priority		= 100,
> 
> Why did you choose the priority 100? The software implementations commonly use 
> 100. crypto/gcm.c uses the prio of the underlying cipher. In case of ARM, 
> there seem to be assembler implementations of AES which have the prio of 200 
> or 300. So, such software implementation of gcm(aes) would have a higher 
> precedence than your hw implementation.
Yes, you are right.
Other hw algos in omap-aes also uses priority 100.
Only sw and hw implementations are enabled right now and both are at same priority.
And till now its lucky enough that hw algo gets picked.

Ill change the priority to 300 for all the modes.
Thanks for pointing it.

Regards,
Lokesh
> 
> So, if a user would use gcm(aes), isn't it more likely that he gets the 
> software implementation?
> 
>> +	.cra_flags		= CRYPTO_ALG_TYPE_AEAD | CRYPTO_ALG_ASYNC |
>> +				  CRYPTO_ALG_KERN_DRIVER_ONLY,
>> +	.cra_blocksize		= AES_BLOCK_SIZE,
>> +	.cra_ctxsize		= sizeof(struct omap_aes_ctx),
>> +	.cra_alignmask		= 0xf,
>> +	.cra_type		= &crypto_aead_type,
>> +	.cra_module		= THIS_MODULE,
>> +	.cra_init		= omap_aes_gcm_cra_init,
>> +	.cra_exit		= omap_aes_cra_exit,
>> +	.cra_u.aead = {
>> +		.maxauthsize	= AES_BLOCK_SIZE,
>> +		.geniv		= "eseqiv",
>> +		.ivsize		= AES_BLOCK_SIZE,
>> +		.setkey		= omap_aes_gcm_setkey,
>> +		.encrypt	= omap_aes_gcm_encrypt,
>> +		.decrypt	= omap_aes_gcm_decrypt,
>> +	}
>> +},
>> };
>>
>> static struct omap_aes_algs_info omap_aes_algs_info_ecb_cbc[] = {
> 
> 
> Ciao
> Stephan
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Stephan Mueller July 2, 2015, 9:56 a.m. UTC | #3
Am Donnerstag, 2. Juli 2015, 15:24:58 schrieb Lokesh Vutla:

Hi Lokesh,

>>> +{
>>> +	.cra_name		= "gcm(aes)",
>>> +	.cra_driver_name	= "gcm-aes-omap",
>>> +	.cra_priority		= 100,
>> 
>> Why did you choose the priority 100? The software implementations commonly
>> use 100. crypto/gcm.c uses the prio of the underlying cipher. In case of
>> ARM, there seem to be assembler implementations of AES which have the prio
>> of 200 or 300. So, such software implementation of gcm(aes) would have a
>> higher precedence than your hw implementation.
>
>Yes, you are right.
>Other hw algos in omap-aes also uses priority 100.
>Only sw and hw implementations are enabled right now and both are at same
>priority. And till now its lucky enough that hw algo gets picked.

Maybe those HW prios should be updated too?

Ciao
Stephan
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Herbert Xu July 6, 2015, 7:35 a.m. UTC | #4
On Thu, Jul 02, 2015 at 10:48:38AM +0530, Lokesh Vutla wrote:
> Now the driver supports gcm mode, add omap-aes-gcm
> algo info to omap-aes driver.
> 
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>

You're using the old AEAD interface.  We are now moving to the
new AEAD interface so I will not be accepting any new implementations
using the old interface.

Please convert your driver over to the new interface.

Also please merge your GCM patches into a single patch.  Splitting
out bug fixes makes no sense.

Thanks,
Lokesh Vutla July 6, 2015, 8:15 a.m. UTC | #5
Hi,
On Monday 06 July 2015 01:05 PM, Herbert Xu wrote:
> On Thu, Jul 02, 2015 at 10:48:38AM +0530, Lokesh Vutla wrote:
>> Now the driver supports gcm mode, add omap-aes-gcm
>> algo info to omap-aes driver.
>>
>> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> 
> You're using the old AEAD interface.  We are now moving to the
> new AEAD interface so I will not be accepting any new implementations
> using the old interface.
> 
> Please convert your driver over to the new interface.
Will convert omap-aes driver to new interface and repost.

Thanks and regards,
Lokesh
> 
> Also please merge your GCM patches into a single patch.  Splitting
> out bug fixes makes no sense.
> 
> Thanks,
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/crypto/omap-aes.c b/drivers/crypto/omap-aes.c
index e5e9a19..11f3850 100644
--- a/drivers/crypto/omap-aes.c
+++ b/drivers/crypto/omap-aes.c
@@ -789,6 +789,28 @@  static struct crypto_alg algs_ctr[] = {
 		.decrypt	= omap_aes_ctr_decrypt,
 	}
 },
+{
+	.cra_name		= "gcm(aes)",
+	.cra_driver_name	= "gcm-aes-omap",
+	.cra_priority		= 100,
+	.cra_flags		= CRYPTO_ALG_TYPE_AEAD | CRYPTO_ALG_ASYNC |
+				  CRYPTO_ALG_KERN_DRIVER_ONLY,
+	.cra_blocksize		= AES_BLOCK_SIZE,
+	.cra_ctxsize		= sizeof(struct omap_aes_ctx),
+	.cra_alignmask		= 0xf,
+	.cra_type		= &crypto_aead_type,
+	.cra_module		= THIS_MODULE,
+	.cra_init		= omap_aes_gcm_cra_init,
+	.cra_exit		= omap_aes_cra_exit,
+	.cra_u.aead = {
+		.maxauthsize	= AES_BLOCK_SIZE,
+		.geniv		= "eseqiv",
+		.ivsize		= AES_BLOCK_SIZE,
+		.setkey		= omap_aes_gcm_setkey,
+		.encrypt	= omap_aes_gcm_encrypt,
+		.decrypt	= omap_aes_gcm_decrypt,
+	}
+},
 };
 
 static struct omap_aes_algs_info omap_aes_algs_info_ecb_cbc[] = {