Re: [PATCH] Crypto: gf128mul : fixed a parentheses coding style issue

2014-10-13 Thread Michael Roocroft


On 10/13/14 00:01, Joe Perches wrote:

On Sun, 2014-10-12 at 21:49 +0100, Mike Roocroft wrote:

Fixed a coding style issue.

[]

diff --git a/crypto/gf128mul.c b/crypto/gf128mul.c

[]

@@ -97,7 +97,7 @@
  the table above
  */
  
-#define xx(p, q)	0x##p##q

+#define xx(p, q)   (0x##p##q)
  
  #define xda_bbe(i) ( \

(i  0x80 ? xx(43, 80) : 0) ^ (i  0x40 ? xx(21, c0) : 0) ^ \

I think that macro is pretty useless and nothing
but obfuscation now.

The comment above it doesn't seem useful either.

How about just removing and replacing the uses
like this?

---
  crypto/gf128mul.c | 27 ---
  1 file changed, 8 insertions(+), 19 deletions(-)

diff --git a/crypto/gf128mul.c b/crypto/gf128mul.c
index 5276607..90cf17d 100644
--- a/crypto/gf128mul.c
+++ b/crypto/gf128mul.c
@@ -88,29 +88,18 @@
q(0xf8), q(0xf9), q(0xfa), q(0xfb), q(0xfc), q(0xfd), q(0xfe), q(0xff) \
  }
  
-/*	Given the value i in 0..255 as the byte overflow when a field element

-in GHASH is multiplied by x^8, this function will return the values that
-are generated in the lo 16-bit word of the field value by applying the
-modular polynomial. The values lo_byte and hi_byte are returned via the
-macro xp_fun(lo_byte, hi_byte) so that the values can be assembled into
-memory as required by a suitable definition of this macro operating on
-the table above
-*/
-
-#define xx(p, q)   0x##p##q
-
  #define xda_bbe(i) ( \
-   (i  0x80 ? xx(43, 80) : 0) ^ (i  0x40 ? xx(21, c0) : 0) ^ \
-   (i  0x20 ? xx(10, e0) : 0) ^ (i  0x10 ? xx(08, 70) : 0) ^ \
-   (i  0x08 ? xx(04, 38) : 0) ^ (i  0x04 ? xx(02, 1c) : 0) ^ \
-   (i  0x02 ? xx(01, 0e) : 0) ^ (i  0x01 ? xx(00, 87) : 0) \
+   (i  0x80 ? 0x4380 : 0) ^ (i  0x40 ? 0x21c0 : 0) ^ \
+   (i  0x20 ? 0x10e0 : 0) ^ (i  0x10 ? 0x0870 : 0) ^ \
+   (i  0x08 ? 0x0438 : 0) ^ (i  0x04 ? 0x021c : 0) ^ \
+   (i  0x02 ? 0x010e : 0) ^ (i  0x01 ? 0x0087 : 0) \
  )
  
  #define xda_lle(i) ( \

-   (i  0x80 ? xx(e1, 00) : 0) ^ (i  0x40 ? xx(70, 80) : 0) ^ \
-   (i  0x20 ? xx(38, 40) : 0) ^ (i  0x10 ? xx(1c, 20) : 0) ^ \
-   (i  0x08 ? xx(0e, 10) : 0) ^ (i  0x04 ? xx(07, 08) : 0) ^ \
-   (i  0x02 ? xx(03, 84) : 0) ^ (i  0x01 ? xx(01, c2) : 0) \
+   (i  0x80 ? 0xe100 : 0) ^ (i  0x40 ? 0x7080 : 0) ^ \
+   (i  0x20 ? 0x3840 : 0) ^ (i  0x10 ? 0x1c20 : 0) ^ \
+   (i  0x08 ? 0x0e10 : 0) ^ (i  0x04 ? 0x0708 : 0) ^ \
+   (i  0x02 ? 0x0384 : 0) ^ (i  0x01 ? 0x01c2 : 0) \
  )
  
  static const u16 gf128mul_table_lle[256] = gf128mul_dat(xda_lle);





Hi there,

I'm not really contributing anything other than getting code checkpatch clean, 
whilst

I relearn C and get a feel for various parts of the kernel.

--
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] Crypto: gf128mul : fixed a parentheses coding style issue

2014-10-13 Thread Joe Perches
On Mon, 2014-10-13 at 21:12 +0100, Michael Roocroft wrote:
 On 10/13/14 00:01, Joe Perches wrote:
  On Sun, 2014-10-12 at 21:49 +0100, Mike Roocroft wrote:
  Fixed a coding style issue.
  []
  diff --git a/crypto/gf128mul.c b/crypto/gf128mul.c
  []
  @@ -97,7 +97,7 @@
the table above
*/

  -#define xx(p, q)  0x##p##q
  +#define xx(p, q)  (0x##p##q)

#define xda_bbe(i) ( \
 (i  0x80 ? xx(43, 80) : 0) ^ (i  0x40 ? xx(21, c0) : 0) ^ \
  I think that macro is pretty useless and nothing
  but obfuscation now.
 
  The comment above it doesn't seem useful either.
 
  How about just removing and replacing the uses
  like this?
 
  ---
crypto/gf128mul.c | 27 ---
1 file changed, 8 insertions(+), 19 deletions(-)
 
  diff --git a/crypto/gf128mul.c b/crypto/gf128mul.c
  index 5276607..90cf17d 100644
  --- a/crypto/gf128mul.c
  +++ b/crypto/gf128mul.c
  @@ -88,29 +88,18 @@
  q(0xf8), q(0xf9), q(0xfa), q(0xfb), q(0xfc), q(0xfd), q(0xfe), q(0xff) \
}

  -/* Given the value i in 0..255 as the byte overflow when a field element
  -in GHASH is multiplied by x^8, this function will return the values 
  that
  -are generated in the lo 16-bit word of the field value by applying the
  -modular polynomial. The values lo_byte and hi_byte are returned via the
  -macro xp_fun(lo_byte, hi_byte) so that the values can be assembled into
  -memory as required by a suitable definition of this macro operating on
  -the table above
  -*/
  -
  -#define xx(p, q)   0x##p##q
  -
#define xda_bbe(i) ( \
  -   (i  0x80 ? xx(43, 80) : 0) ^ (i  0x40 ? xx(21, c0) : 0) ^ \
  -   (i  0x20 ? xx(10, e0) : 0) ^ (i  0x10 ? xx(08, 70) : 0) ^ \
  -   (i  0x08 ? xx(04, 38) : 0) ^ (i  0x04 ? xx(02, 1c) : 0) ^ \
  -   (i  0x02 ? xx(01, 0e) : 0) ^ (i  0x01 ? xx(00, 87) : 0) \
  +   (i  0x80 ? 0x4380 : 0) ^ (i  0x40 ? 0x21c0 : 0) ^ \
  +   (i  0x20 ? 0x10e0 : 0) ^ (i  0x10 ? 0x0870 : 0) ^ \
  +   (i  0x08 ? 0x0438 : 0) ^ (i  0x04 ? 0x021c : 0) ^ \
  +   (i  0x02 ? 0x010e : 0) ^ (i  0x01 ? 0x0087 : 0) \
)

#define xda_lle(i) ( \
  -   (i  0x80 ? xx(e1, 00) : 0) ^ (i  0x40 ? xx(70, 80) : 0) ^ \
  -   (i  0x20 ? xx(38, 40) : 0) ^ (i  0x10 ? xx(1c, 20) : 0) ^ \
  -   (i  0x08 ? xx(0e, 10) : 0) ^ (i  0x04 ? xx(07, 08) : 0) ^ \
  -   (i  0x02 ? xx(03, 84) : 0) ^ (i  0x01 ? xx(01, c2) : 0) \
  +   (i  0x80 ? 0xe100 : 0) ^ (i  0x40 ? 0x7080 : 0) ^ \
  +   (i  0x20 ? 0x3840 : 0) ^ (i  0x10 ? 0x1c20 : 0) ^ \
  +   (i  0x08 ? 0x0e10 : 0) ^ (i  0x04 ? 0x0708 : 0) ^ \
  +   (i  0x02 ? 0x0384 : 0) ^ (i  0x01 ? 0x01c2 : 0) \
)

static const u16 gf128mul_table_lle[256] = gf128mul_dat(xda_lle);
 
 
 
 Hi there,

Hi Mike.

 I'm not really contributing anything other than getting code checkpatch 
 clean, whilst
 I relearn C and get a feel for various parts of the kernel.

checkpatch cleanliness, while OK for some parts of the
kernel, should not be an end-goal.

checkpatch is really a tool to check patches.

If you want to submit neatening only patches, please
do your changes in drivers/staging/

Otherwise, please look for code that isn't simply a
style neatening bit, but something that actively makes
reading the code easier, makes the object code smaller
or faster, reduces complexity, adds extensibility,
etc, etc...

cheers, Joe



--
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] Crypto: gf128mul : fixed a parentheses coding style issue

2014-10-13 Thread Michael Roocroft

On 10/13/14 21:15, Joe Perches wrote:

On Mon, 2014-10-13 at 21:12 +0100, Michael Roocroft wrote:

On 10/13/14 00:01, Joe Perches wrote:

On Sun, 2014-10-12 at 21:49 +0100, Mike Roocroft wrote:

Fixed a coding style issue.

[]

diff --git a/crypto/gf128mul.c b/crypto/gf128mul.c

[]

@@ -97,7 +97,7 @@
   the table above
   */
   
-#define xx(p, q)	0x##p##q

+#define xx(p, q)   (0x##p##q)
   
   #define xda_bbe(i) ( \

(i  0x80 ? xx(43, 80) : 0) ^ (i  0x40 ? xx(21, c0) : 0) ^ \

I think that macro is pretty useless and nothing
but obfuscation now.

The comment above it doesn't seem useful either.

How about just removing and replacing the uses
like this?

---
   crypto/gf128mul.c | 27 ---
   1 file changed, 8 insertions(+), 19 deletions(-)

diff --git a/crypto/gf128mul.c b/crypto/gf128mul.c
index 5276607..90cf17d 100644
--- a/crypto/gf128mul.c
+++ b/crypto/gf128mul.c
@@ -88,29 +88,18 @@
q(0xf8), q(0xf9), q(0xfa), q(0xfb), q(0xfc), q(0xfd), q(0xfe), q(0xff) \
   }
   
-/*	Given the value i in 0..255 as the byte overflow when a field element

-in GHASH is multiplied by x^8, this function will return the values that
-are generated in the lo 16-bit word of the field value by applying the
-modular polynomial. The values lo_byte and hi_byte are returned via the
-macro xp_fun(lo_byte, hi_byte) so that the values can be assembled into
-memory as required by a suitable definition of this macro operating on
-the table above
-*/
-
-#define xx(p, q)   0x##p##q
-
   #define xda_bbe(i) ( \
-   (i  0x80 ? xx(43, 80) : 0) ^ (i  0x40 ? xx(21, c0) : 0) ^ \
-   (i  0x20 ? xx(10, e0) : 0) ^ (i  0x10 ? xx(08, 70) : 0) ^ \
-   (i  0x08 ? xx(04, 38) : 0) ^ (i  0x04 ? xx(02, 1c) : 0) ^ \
-   (i  0x02 ? xx(01, 0e) : 0) ^ (i  0x01 ? xx(00, 87) : 0) \
+   (i  0x80 ? 0x4380 : 0) ^ (i  0x40 ? 0x21c0 : 0) ^ \
+   (i  0x20 ? 0x10e0 : 0) ^ (i  0x10 ? 0x0870 : 0) ^ \
+   (i  0x08 ? 0x0438 : 0) ^ (i  0x04 ? 0x021c : 0) ^ \
+   (i  0x02 ? 0x010e : 0) ^ (i  0x01 ? 0x0087 : 0) \
   )
   
   #define xda_lle(i) ( \

-   (i  0x80 ? xx(e1, 00) : 0) ^ (i  0x40 ? xx(70, 80) : 0) ^ \
-   (i  0x20 ? xx(38, 40) : 0) ^ (i  0x10 ? xx(1c, 20) : 0) ^ \
-   (i  0x08 ? xx(0e, 10) : 0) ^ (i  0x04 ? xx(07, 08) : 0) ^ \
-   (i  0x02 ? xx(03, 84) : 0) ^ (i  0x01 ? xx(01, c2) : 0) \
+   (i  0x80 ? 0xe100 : 0) ^ (i  0x40 ? 0x7080 : 0) ^ \
+   (i  0x20 ? 0x3840 : 0) ^ (i  0x10 ? 0x1c20 : 0) ^ \
+   (i  0x08 ? 0x0e10 : 0) ^ (i  0x04 ? 0x0708 : 0) ^ \
+   (i  0x02 ? 0x0384 : 0) ^ (i  0x01 ? 0x01c2 : 0) \
   )
   
   static const u16 gf128mul_table_lle[256] = gf128mul_dat(xda_lle);





Hi there,

Hi Mike.


I'm not really contributing anything other than getting code checkpatch clean, 
whilst
I relearn C and get a feel for various parts of the kernel.

checkpatch cleanliness, while OK for some parts of the
kernel, should not be an end-goal.

checkpatch is really a tool to check patches.

If you want to submit neatening only patches, please
do your changes in drivers/staging/

Otherwise, please look for code that isn't simply a
style neatening bit, but something that actively makes
reading the code easier, makes the object code smaller
or faster, reduces complexity, adds extensibility,
etc, etc...

cheers, Joe



Hi Joe

Thanks for the Advice, I fully intend to making more meaningful contributions
when my confidence in writing C is better than it is at the moment. I'll 
concentrate
on making any changes to staging whilst I learn and get to grips with git, and
continue to look through the rest of the kernel tree as a learning exercise.

I am extremely new to all this and a little overwhelmed, but by looking and not 
doing
anyhing i'll never learn anything.

--
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] Crypto: gf128mul : fixed a parentheses coding style issue

2014-10-13 Thread Joe Perches
On Mon, 2014-10-13 at 21:52 +0100, Michael Roocroft wrote:
 I fully intend to making more meaningful contributions
 when my confidence in writing C is better than it is at the moment. I'll 
 concentrate
 on making any changes to staging whilst I learn and get to grips with git, and
 continue to look through the rest of the kernel tree as a learning exercise.

Sounds like a good plan to me.  welcome btw.

 by looking and not doing anything i'll never learn anything.

True words... 

cheers, Joe

--
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


[PATCH v2 1/2] crypto: qat - Prevent dma mapping zero length assoc data

2014-10-13 Thread Tadeusz Struk
Do not attempt to dma map associated data if it is zero length.

Signed-off-by: Tadeusz Struk tadeusz.st...@intel.com
---
 drivers/crypto/qat/qat_common/qat_algs.c |2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/crypto/qat/qat_common/qat_algs.c 
b/drivers/crypto/qat/qat_common/qat_algs.c
index 3e26fa2..bd4bd27 100644
--- a/drivers/crypto/qat/qat_common/qat_algs.c
+++ b/drivers/crypto/qat/qat_common/qat_algs.c
@@ -608,6 +608,8 @@ static int qat_alg_sgl_to_bufl(struct qat_crypto_instance 
*inst,
goto err;
 
for_each_sg(assoc, sg, assoc_n, i) {
+   if (!sg-length)
+   continue;
bufl-bufers[bufs].addr = dma_map_single(dev,
 sg_virt(sg),
 sg-length,

--
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


[PATCH v2 2/2] crypto: qat - Enforce valid numa configuration

2014-10-13 Thread Tadeusz Struk
In a system with NUMA configuration we want to enforce that the accelerator is
connected to a node with memory to avoid cross QPI memory transaction.
Otherwise there is no point in using the accelerator as the encryption in
software will be faster.

Signed-off-by: Tadeusz Struk tadeusz.st...@intel.com
---
 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  |5 ++-
 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, 30 insertions(+), 34 deletions(-)

diff --git a/drivers/crypto/qat/qat_common/adf_accel_devices.h 
b/drivers/crypto/qat/qat_common/adf_accel_devices.h
index 9282381..fe7b3f0 100644
--- a/drivers/crypto/qat/qat_common/adf_accel_devices.h
+++ b/drivers/crypto/qat/qat_common/adf_accel_devices.h
@@ -198,8 +198,7 @@ struct adf_accel_dev {
struct dentry *debugfs_dir;
struct list_head list;
struct module *owner;
-   uint8_t accel_id;
-   uint8_t numa_node;
struct adf_accel_pci accel_pci_dev;
+   uint8_t accel_id;
 } __packed;
 #endif
diff --git a/drivers/crypto/qat/qat_common/adf_transport.c 
b/drivers/crypto/qat/qat_common/adf_transport.c
index 5f3fa45..9dd2cb7 100644
--- a/drivers/crypto/qat/qat_common/adf_transport.c
+++ b/drivers/crypto/qat/qat_common/adf_transport.c
@@ -419,9 +419,10 @@ static int adf_init_bank(struct adf_accel_dev *accel_dev,
WRITE_CSR_RING_BASE(csr_addr, bank_num, i, 0);
ring = bank-rings[i];
if (hw_data-tx_rings_mask  (1  i)) {
-   ring-inflights = kzalloc_node(sizeof(atomic_t),
-  GFP_KERNEL,
-  accel_dev-numa_node);
+   ring-inflights =
+   kzalloc_node(sizeof(atomic_t),
+GFP_KERNEL,
+dev_to_node(GET_DEV(accel_dev)));
if (!ring-inflights)
goto err;
} else {
@@ -469,13 +470,14 @@ int adf_init_etr_data(struct adf_accel_dev *accel_dev)
int i, ret;
 
etr_data = kzalloc_node(sizeof(*etr_data), GFP_KERNEL,
-   accel_dev-numa_node);
+   dev_to_node(GET_DEV(accel_dev)));
if (!etr_data)
return -ENOMEM;
 
num_banks = GET_MAX_BANKS(accel_dev);
size = num_banks * sizeof(struct adf_etr_bank_data);
-   etr_data-banks = kzalloc_node(size, GFP_KERNEL, accel_dev-numa_node);
+   etr_data-banks = kzalloc_node(size, GFP_KERNEL,
+  dev_to_node(GET_DEV(accel_dev)));
if (!etr_data-banks) {
ret = -ENOMEM;
goto err_bank;
diff --git a/drivers/crypto/qat/qat_common/qat_algs.c 
b/drivers/crypto/qat/qat_common/qat_algs.c
index bd4bd27..7893a8b 100644
--- a/drivers/crypto/qat/qat_common/qat_algs.c
+++ b/drivers/crypto/qat/qat_common/qat_algs.c
@@ -599,7 +599,8 @@ static int qat_alg_sgl_to_bufl(struct qat_crypto_instance 
*inst,
if (unlikely(!n))
return -EINVAL;
 
-   bufl = kmalloc_node(sz, GFP_ATOMIC, inst-accel_dev-numa_node);
+   bufl = kmalloc_node(sz, GFP_ATOMIC,
+   dev_to_node(GET_DEV(inst-accel_dev)));
if (unlikely(!bufl))
return -ENOMEM;
 
@@ -645,7 +646,7 @@ static int qat_alg_sgl_to_bufl(struct qat_crypto_instance 
*inst,
struct qat_alg_buf *bufers;
 
buflout = kmalloc_node(sz, GFP_ATOMIC,
-  inst-accel_dev-numa_node);
+  dev_to_node(GET_DEV(inst-accel_dev)));
if (unlikely(!buflout))
goto err;
bloutp = dma_map_single(dev, buflout, sz, DMA_TO_DEVICE);
diff --git a/drivers/crypto/qat/qat_common/qat_crypto.c 
b/drivers/crypto/qat/qat_common/qat_crypto.c
index 0d59bcb..828f2a6 100644
--- a/drivers/crypto/qat/qat_common/qat_crypto.c
+++ b/drivers/crypto/qat/qat_common/qat_crypto.c
@@ -109,12 +109,14 @@ struct qat_crypto_instance 
*qat_crypto_get_instance_node(int node)
 
list_for_each(itr, adf_devmgr_get_head()) {
accel_dev = list_entry(itr, struct adf_accel_dev, list);
-   if (accel_dev-numa_node == node  adf_dev_started(accel_dev))
+   if ((node == dev_to_node(GET_DEV(accel_dev)) ||
+   dev_to_node(GET_DEV(accel_dev))  0)
+adf_dev_started(accel_dev))
break;

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

2014-10-13 Thread Tadeusz Struk
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(-)

-- 
--
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