Re: [PATCH v2 0/2] crypto: qat - Fix for invalid dma mapping and numa

2014-10-15 Thread Nikolay Aleksandrov

On 14/10/14 03:24, Tadeusz Struk wrote:

Hi,
These two patches fix invalid (zero length) dma mapping and
enforce numa configuration for maximum performance.

Change log:
v2 - Removed numa node calculation based bus number and use predefined
functions instead.

Signed-off-by: Tadeusz Struk tadeusz.st...@intel.com
---

Tadeusz Struk (2):
   crypto: qat - Prevent dma mapping zero length assoc data
   crypto: qat - Enforce valid numa configuration


  drivers/crypto/qat/qat_common/adf_accel_devices.h |3 +-
  drivers/crypto/qat/qat_common/adf_transport.c |   12 +---
  drivers/crypto/qat/qat_common/qat_algs.c  |7 +++--
  drivers/crypto/qat/qat_common/qat_crypto.c|8 +++--
  drivers/crypto/qat/qat_dh895xcc/adf_admin.c   |2 +
  drivers/crypto/qat/qat_dh895xcc/adf_drv.c |   32 -
  drivers/crypto/qat/qat_dh895xcc/adf_isr.c |2 +
  7 files changed, 32 insertions(+), 34 deletions(-)



I just gave a quick run of these patches and they seem to fix the NUMA issue and 
the 0 length warnings.


Tested-by: Nikolay Aleksandrov niko...@redhat.com
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 0/2] crypto: qat - Fix for invalid dma mapping and numa

2014-10-15 Thread Prarit Bhargava


On 10/15/2014 06:35 AM, Nikolay Aleksandrov wrote:
 On 14/10/14 03:24, Tadeusz Struk wrote:
 Hi,
 These two patches fix invalid (zero length) dma mapping and
 enforce numa configuration for maximum performance.

 Change log:
 v2 - Removed numa node calculation based bus number and use predefined
 functions instead.

 Signed-off-by: Tadeusz Struk tadeusz.st...@intel.com
 ---

 Tadeusz Struk (2):
crypto: qat - Prevent dma mapping zero length assoc data
crypto: qat - Enforce valid numa configuration


   drivers/crypto/qat/qat_common/adf_accel_devices.h |3 +-
   drivers/crypto/qat/qat_common/adf_transport.c |   12 +---
   drivers/crypto/qat/qat_common/qat_algs.c  |7 +++--
   drivers/crypto/qat/qat_common/qat_crypto.c|8 +++--
   drivers/crypto/qat/qat_dh895xcc/adf_admin.c   |2 +
   drivers/crypto/qat/qat_dh895xcc/adf_drv.c |   32 
 -
   drivers/crypto/qat/qat_dh895xcc/adf_isr.c |2 +
   7 files changed, 32 insertions(+), 34 deletions(-)

 
 I just gave a quick run of these patches and they seem to fix the NUMA issue 
 and
 the 0 length warnings.
 
 Tested-by: Nikolay Aleksandrov niko...@redhat.com

Thanks Nik :)

Reviewed-by: Prarit Bhargava pra...@redhat.com

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


Re: [PATCH v2 2/2] crypto: caam - add support for rfc4106(gcm(aes))

2014-10-15 Thread Tudor Ambarus



On 10/14/2014 12:17 PM, Tudor Ambarus wrote:


+static int rfc4106_setkey(struct crypto_aead *aead,
+ const u8 *key, unsigned int keylen)
+{
+   struct caam_ctx *ctx = crypto_aead_ctx(aead);
+   struct device *jrdev = ctx-jrdev;
+   int ret = 0;
+
+   /*
+* The last four bytes of the key material are used as the salt value
+* in the nonce. Update the AES key length.
+*/
+   if (keylen  4)
+   return -EINVAL;
+   keylen -= 4;


The salt will not be copied in the ctx-key, so the descriptors will use 
as a salt whatever resides in memory after (updated) keylen bytes.


I will submit a new patch set in which I will update the AES key length 
after copying the key material in ctx-key.


tcrypt tests passed because they use salt values of zero.
Back-to-back tests passed because I used caam with rfc4106 accelerated 
on both boards.



+
+#ifdef DEBUG
+   print_hex_dump(KERN_ERR, key in @__stringify(__LINE__): ,
+  DUMP_PREFIX_ADDRESS, 16, 4, key, keylen, 1);
+#endif
+
+   memcpy(ctx-key, key, keylen);


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


Re: [PATCH v2 0/2] crypto: qat - Fix for invalid dma mapping and numa

2014-10-15 Thread Tadeusz Struk
On 10/15/2014 04:25 AM, Prarit Bhargava wrote:
 I just gave a quick run of these patches and they seem to fix the NUMA issue 
 and
  the 0 length warnings.
  
  Tested-by: Nikolay Aleksandrov niko...@redhat.com
 Thanks Nik :)
 
 Reviewed-by: Prarit Bhargava pra...@redhat.com

Thank you Nik and Prarit
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] kernel crypto API interface specification

2014-10-15 Thread Jason Cooper
Stephan,

Wow.  This is very thorough.  Herbert and others will be making the
final call on this, but if I may make a suggestion:

On Tue, Oct 14, 2014 at 09:46:50PM +0200, Stephan Mueller wrote:
 The update adds a complete interface documentation of the kernel crypto
 API. All cipher types supported by the kernel crypto API are documented.
 
 In addition, kernel and user space example code is provided. The sample
 code covers synchronous and asynchronous cipher operation, random
 number generation and performing hashing as well as encryption and
 decryption in user space.

This really needs to be split into at least two pieces.  The kernel and
the userspace API.  I'd venture to say the userspace API portion of this
document is almost ready.  But I'm not certain that the kernel
interfaces are best described in a specification.

APIs within the kernel are intentionally not nailed down and are very
fluid.  Any attempt to spell them out in a document would mean either a)
the document would be out of date quickly, or b) the maintainer now has
to ask for changes to the docs every time a patch with a kernel API
change comes in.  Neither scenario is good. :-(

We certainly don't want to lose all of the effort you've put into
grokking the API, so we need to find a maintainable place to add it.  I
personally think adding comments above the respective function
blocks would work well.

The examples (kernel API) are another matter entirely.  Examples that
aren't up-to-date and usable as a template aren't helpful to anyone.
Some would even say detrimental.  And since example code isn't actually
*used* in the real world, it would be an extra burden keeping it up to
date.  I think these could best be used as a reference to compare all of
the current users to.  Anything not up to par would generate a patch.
The best examples should be the current users in the kernel.

 Signed-off-by: Stephan Mueller smuel...@chronox.de
 ---
  Documentation/crypto/crypto-API-spec.txt | 2110 
 ++
  1 file changed, 2110 insertions(+)
  create mode 100644 Documentation/crypto/crypto-API-spec.txt
 
 diff --git a/Documentation/crypto/crypto-API-spec.txt 
 b/Documentation/crypto/crypto-API-spec.txt
 new file mode 100644
 index 000..027fd4f
 --- /dev/null
 +++ b/Documentation/crypto/crypto-API-spec.txt
 @@ -0,0 +1,2110 @@

[snip detailed explanation of current kernel API]

 +User space API
 +==
 +
 +The kernel crypto API is accessible from user space. Currently, the following
 +ciphers are accessible:
 +
 + * Message digest including keyed message digest
 +
 + * Symmetric ciphers
 +
 +The interface is provided via Netlink using the type AF_ALG. In addition, the
 +setsockopt option type is SOL_ALG. In case the user space header files do not
 +export these flags yet, use the following macros:
 +
 +#ifndef AF_ALG
 +#define AF_ALG 38
 +#endif
 +#ifndef SOL_ALG
 +#define SOL_ALG 279
 +#endif
 +
 +A cipher is accessed with the same name as done for the in-kernel API calls.

Perhaps a reference here to the final location of the kernel API
explanations?

 +
 +To interact with the kernel crypto API, a Netlink socket must be created by
 +the user space application. User space invokes the cipher operation with the
 +send/write system call family. The result of the cipher operation is obtained
 +with the read/recv system call family.
 +
 +The following API calls assume that the Netlink socket descriptor is already
 +opened by the user space application and discusses only the kernel crypto API
 +specific invocations.
 +
 +Message digest API
 +--
 +
 +The message digest type to be used for the cipher operation is selected when
 +invoking the bind syscall. bind requires the caller to provide a filled
 +struct sockaddr data structure. This data structure must be filled as 
 follows:
 +
 +struct sockaddr_alg sa = {
 + .salg_family = AF_ALG,
 + .salg_type = hash, /* this selects the hash logic in the kernel */
 + .salg_nmae = sha1 /* this is the cipher name */
 +};
 +
 +Using the send() system call, the application provides the data that should 
 be
 +processed with the message digest. The send system call allows the following
 +flags to be specified:
 +
 + * MSG_MORE: If this flag is set, the send system call acts like a
 + message digest update function where the final hash is not
 + yet calculated. If the flag is not set, the send system call
 + calculates the final message digest immediately.
 +
 +With the read() system call, the application can read the message digest from
 +the kernel crypto API. If the buffer is too small for the message digest, the
 +flag MSG_TRUNC is set by the kernel.
 +
 +In order to set a message digest key, the calling application must use the
 +setsockopt() option of ALG_SET_KEY.

What happens if this is omitted?

 +
 +
 +Symmetric cipher API
 +
 +
 +The operation is very similar to 

[cryptodev PATCH] qat: cleanup coccicheck warning - NULL check before freeing functions

2014-10-15 Thread Bruce Allan
Analyzing with coccinelle MODE=report...

Please check for false positives in the output before submitting a patch.
When using patch mode, carefully review the patch before submitting it.

drivers/crypto/qat/qat_dh895xcc/adf_isr.c:191:3-8: WARNING: NULL check
before freeing functions like kfree, debugfs_remove,
debugfs_remove_recursive or usb_free_urb is not needed. Maybe consider
reorganizing relevant code to avoid passing NULL values.

drivers/crypto/qat/qat_dh895xcc/adf_isr.c:208:3-8: WARNING: NULL check
before freeing functions like kfree, debugfs_remove,
debugfs_remove_recursive or usb_free_urb is not needed. Maybe consider
reorganizing relevant code to avoid passing NULL values.

Signed-off-by: Bruce Allan bruce.w.al...@intel.com
---

 drivers/crypto/qat/qat_dh895xcc/adf_isr.c |   12 
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/drivers/crypto/qat/qat_dh895xcc/adf_isr.c 
b/drivers/crypto/qat/qat_dh895xcc/adf_isr.c
index 67ec61e..c9212b9 100644
--- a/drivers/crypto/qat/qat_dh895xcc/adf_isr.c
+++ b/drivers/crypto/qat/qat_dh895xcc/adf_isr.c
@@ -186,10 +186,8 @@ static int adf_isr_alloc_msix_entry_table(struct 
adf_accel_dev *accel_dev)
accel_dev-accel_pci_dev.msix_entries.names = names;
return 0;
 err:
-   for (i = 0; i  msix_num_entries; i++) {
-   if (*(names + i))
-   kfree(*(names + i));
-   }
+   for (i = 0; i  msix_num_entries; i++)
+   kfree(*(names + i));
kfree(entries);
kfree(names);
return -ENOMEM;
@@ -203,10 +201,8 @@ static void adf_isr_free_msix_entry_table(struct 
adf_accel_dev *accel_dev)
int i;
 
kfree(accel_dev-accel_pci_dev.msix_entries.entries);
-   for (i = 0; i  msix_num_entries; i++) {
-   if (*(names + i))
-   kfree(*(names + i));
-   }
+   for (i = 0; i  msix_num_entries; i++)
+   kfree(*(names + i));
kfree(names);
 }
 

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


[cryptodev PATCH] qat: cleanup unnecessary break checkpatch warning

2014-10-15 Thread Bruce Allan
WARNING:UNNECESSARY_BREAK: break is not useful after a goto or return
#472: FILE: drivers/crypto/qat/qat_common/qat_algs.c:472:
+   goto bad_key;
+   break;

Signed-off-by: Bruce Allan bruce.w.al...@intel.com
---

 drivers/crypto/qat/qat_common/qat_algs.c |1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/crypto/qat/qat_common/qat_algs.c 
b/drivers/crypto/qat/qat_common/qat_algs.c
index 3e26fa2..e03afe9 100644
--- a/drivers/crypto/qat/qat_common/qat_algs.c
+++ b/drivers/crypto/qat/qat_common/qat_algs.c
@@ -469,7 +469,6 @@ static int qat_alg_init_sessions(struct qat_alg_session_ctx 
*ctx,
break;
default:
goto bad_key;
-   break;
}
 
if (qat_alg_init_enc_session(ctx, alg, keys))

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


[cryptodev PATCH] qat: fix bad unlock balance

2014-10-15 Thread Bruce Allan
The mutex table_lock is unlocked in two functions without first being locked.
Fix the functions to properly protect the accel_table with the table_lock.

Also, fix a spelling error in one of the function's header comment.

Signed-off-by: Bruce Allan bruce.w.al...@intel.com
---

 drivers/crypto/qat/qat_common/adf_dev_mgr.c |6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/qat/qat_common/adf_dev_mgr.c 
b/drivers/crypto/qat/qat_common/adf_dev_mgr.c
index ae71555..4a0a829d 100644
--- a/drivers/crypto/qat/qat_common/adf_dev_mgr.c
+++ b/drivers/crypto/qat/qat_common/adf_dev_mgr.c
@@ -129,12 +129,13 @@ struct adf_accel_dev *adf_devmgr_get_first(void)
  * Function returns acceleration device associated with the given pci device.
  * To be used by QAT device specific drivers.
  *
- * Return: pinter to accel_dev or NULL if not found.
+ * Return: pointer to accel_dev or NULL if not found.
  */
 struct adf_accel_dev *adf_devmgr_pci_to_accel_dev(struct pci_dev *pci_dev)
 {
struct list_head *itr;
 
+   mutex_lock(table_lock);
list_for_each(itr, accel_table) {
struct adf_accel_dev *ptr =
list_entry(itr, struct adf_accel_dev, list);
@@ -144,6 +145,7 @@ struct adf_accel_dev *adf_devmgr_pci_to_accel_dev(struct 
pci_dev *pci_dev)
return ptr;
}
}
+   mutex_unlock(table_lock);
return NULL;
 }
 EXPORT_SYMBOL_GPL(adf_devmgr_pci_to_accel_dev);
@@ -152,6 +154,7 @@ struct adf_accel_dev *adf_devmgr_get_dev_by_id(uint32_t id)
 {
struct list_head *itr;
 
+   mutex_lock(table_lock);
list_for_each(itr, accel_table) {
struct adf_accel_dev *ptr =
list_entry(itr, struct adf_accel_dev, list);
@@ -161,6 +164,7 @@ struct adf_accel_dev *adf_devmgr_get_dev_by_id(uint32_t id)
return ptr;
}
}
+   mutex_unlock(table_lock);
return NULL;
 }
 

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