[PATCH v6 0/2] IV Generation algorithms for dm-crypt

[Binoy Jayan]

===
dm-crypt optimization for larger block sizes
===

Currently, the iv generation algorithms are implemented in dm-crypt.c. The goal
is to move these algorithms from the dm layer to the kernel crypto layer by
implementing them as template ciphers so they can be used in relation with
algorithms like aes, and with multiple modes like cbc, ecb etc. As part of this
patchset, the iv-generation code is moved from the dm layer to the crypto layer
and adapt the dm-layer to send a whole 'bio' (as defined in the block layer)
at a time. Each bio contains the in memory representation of physically
contiguous disk blocks. Since the bio itself may not be contiguous in main
memory, the dm layer sets up a chained scatterlist of these blocks split into
physically contiguous segments in memory so that DMA can be performed.

One challenge in doing so is that the IVs are generated based on a 512-byte
sector number. This infact limits the block sizes to 512 bytes. But this should
not be a problem if a hardware with iv generation support is used. The geniv
itself splits the segments into sectors so it could choose the IV based on
sector number. But it could be modelled in hardware effectively by not
splitting up the segments in the bio.

Another challenge faced is that dm-crypt has an option to use multiple keys.
The key selection is done based on the sector number. If the whole bio is
encrypted / decrypted with the same key, the encrypted volumes will not be
compatible with the original dm-crypt [without the changes]. So, the key
selection code is moved to crypto layer so the neighboring sectors are
encrypted with a different key.

The dm layer allocates space for iv. The hardware drivers can choose to make
use of this space to generate their IVs sequentially or allocate it on their
own. This can be moved to crypto layer too. Postponing this decision until
the requirement to integrate milan's changes are clear.

Interface to the crypto layer - include/crypto/geniv.h

More information on test procedure can be found in v1.
Results of performance tests with software crypto in v5.

The patch 'crypto: Multikey template for essiv' depends on
the following patches by Gilad:
 MAINTAINERS: add Gilad BY as maintainer for ccree
 staging: ccree: add devicetree bindings
 staging: ccree: add TODO list
 staging: add ccree crypto driver

Revisions:
--

v1: https://patchwork.kernel.org/patch/9439175
v2: https://patchwork.kernel.org/patch/9471923
v3: https://lkml.org/lkml/2017/1/18/170
v4: https://patchwork.kernel.org/patch/9559665
v5: https://patchwork.kernel.org/patch/9669237

v5 --> v6:
--

1. Moved allocation of initialization vectors to the iv-generator
2. Few consmetic changes as the consequence of the above
3. Few logical to boolean expressions for faster calculation
4. Included multikey template for splitting keys.
   This needs testing with real hardware (juno with ccree)
   and also modification. It is only for testing and not
   for inclusion upstream.

v4 --> v5
--

1. Fix for the multiple instance issue in /proc/crypto
2. Few cosmetic changes including struct alignment
3. Simplified 'struct geniv_req_info'

v3 --> v4
--
Fix for the bug reported by Gilad Ben-Yossef.
The element '__ctx' in 'struct skcipher_request req' overflowed into the
element 'struct scatterlist src' which immediately follows 'req' in
'struct geniv_subreq' and corrupted src.

v2 --> v3
--

1. Moved iv algorithms in dm-crypt.c for control
2. Key management code moved from dm layer to cryto layer
   so that cipher instance selection can be made depending on key_index
3. The revision v2 had scatterlist nodes created for every sector in the bio.
   It is modified to create only once scatterlist node to reduce memory
   foot print. Synchronous requests are processed sequentially. Asynchronous
   requests are processed in parallel and is freed in the async callback.
4. Changed allocation for sub-requests using mempool

v1 --> v2
--

1. dm-crypt changes to process larger block sizes (one segment in a bio)
2. Incorporated changes w.r.t. comments from Herbert.


Binoy Jayan (2):
  crypto: Add IV generation algorithms
  crypto: Multikey template for essiv

 drivers/md/dm-crypt.c| 1940 +++---
 drivers/staging/ccree/Makefile   |2 +-
 drivers/staging/ccree/essiv.c|  777 +++
 drivers/staging/ccree/essiv_sw.c | 1040 
 include/crypto/geniv.h   |   46 +
 5 files changed, 3251 insertions(+), 554 deletions(-)
 create mode 100644 drivers/staging/ccree/essiv.c
 create mode 100644 drivers/staging/ccree/essiv_sw.c
 create mode 100644 include/crypto/geniv.h

-- 
Binoy Jayan

[PATCH v6 0/2] IV Generation algorithms for dm-crypt

[Binoy Jayan]

===
dm-crypt optimization for larger block sizes
===

Currently, the iv generation algorithms are implemented in dm-crypt.c. The goal
is to move these algorithms from the dm layer to the kernel crypto layer by
implementing them as template ciphers so they can be used in relation with
algorithms like aes, and with multiple modes like cbc, ecb etc. As part of this
patchset, the iv-generation code is moved from the dm layer to the crypto layer
and adapt the dm-layer to send a whole 'bio' (as defined in the block layer)
at a time. Each bio contains the in memory representation of physically
contiguous disk blocks. Since the bio itself may not be contiguous in main
memory, the dm layer sets up a chained scatterlist of these blocks split into
physically contiguous segments in memory so that DMA can be performed.

One challenge in doing so is that the IVs are generated based on a 512-byte
sector number. This infact limits the block sizes to 512 bytes. But this should
not be a problem if a hardware with iv generation support is used. The geniv
itself splits the segments into sectors so it could choose the IV based on
sector number. But it could be modelled in hardware effectively by not
splitting up the segments in the bio.

Another challenge faced is that dm-crypt has an option to use multiple keys.
The key selection is done based on the sector number. If the whole bio is
encrypted / decrypted with the same key, the encrypted volumes will not be
compatible with the original dm-crypt [without the changes]. So, the key
selection code is moved to crypto layer so the neighboring sectors are
encrypted with a different key.

The dm layer allocates space for iv. The hardware drivers can choose to make
use of this space to generate their IVs sequentially or allocate it on their
own. This can be moved to crypto layer too. Postponing this decision until
the requirement to integrate milan's changes are clear.

Interface to the crypto layer - include/crypto/geniv.h

More information on test procedure can be found in v1.
Results of performance tests with software crypto in v5.

The patch 'crypto: Multikey template for essiv' depends on
the following patches by Gilad:
 MAINTAINERS: add Gilad BY as maintainer for ccree
 staging: ccree: add devicetree bindings
 staging: ccree: add TODO list
 staging: add ccree crypto driver

Revisions:
--

v1: https://patchwork.kernel.org/patch/9439175
v2: https://patchwork.kernel.org/patch/9471923
v3: https://lkml.org/lkml/2017/1/18/170
v4: https://patchwork.kernel.org/patch/9559665
v5: https://patchwork.kernel.org/patch/9669237

v5 --> v6:
--

1. Moved allocation of initialization vectors to the iv-generator
2. Few consmetic changes as the consequence of the above
3. Few logical to boolean expressions for faster calculation
4. Included multikey template for splitting keys.
   This needs testing with real hardware (juno with ccree)
   and also modification. It is only for testing and not
   for inclusion upstream.

v4 --> v5
--

1. Fix for the multiple instance issue in /proc/crypto
2. Few cosmetic changes including struct alignment
3. Simplified 'struct geniv_req_info'

v3 --> v4
--
Fix for the bug reported by Gilad Ben-Yossef.
The element '__ctx' in 'struct skcipher_request req' overflowed into the
element 'struct scatterlist src' which immediately follows 'req' in
'struct geniv_subreq' and corrupted src.

v2 --> v3
--

1. Moved iv algorithms in dm-crypt.c for control
2. Key management code moved from dm layer to cryto layer
   so that cipher instance selection can be made depending on key_index
3. The revision v2 had scatterlist nodes created for every sector in the bio.
   It is modified to create only once scatterlist node to reduce memory
   foot print. Synchronous requests are processed sequentially. Asynchronous
   requests are processed in parallel and is freed in the async callback.
4. Changed allocation for sub-requests using mempool

v1 --> v2
--

1. dm-crypt changes to process larger block sizes (one segment in a bio)
2. Incorporated changes w.r.t. comments from Herbert.


Binoy Jayan (2):
  crypto: Add IV generation algorithms
  crypto: Multikey template for essiv

 drivers/md/dm-crypt.c| 1940 +++---
 drivers/staging/ccree/Makefile   |2 +-
 drivers/staging/ccree/essiv.c|  777 +++
 drivers/staging/ccree/essiv_sw.c | 1040 
 include/crypto/geniv.h   |   46 +
 5 files changed, 3251 insertions(+), 554 deletions(-)
 create mode 100644 drivers/staging/ccree/essiv.c
 create mode 100644 drivers/staging/ccree/essiv_sw.c
 create mode 100644 include/crypto/geniv.h

-- 
Binoy Jayan

[PATCH v6 1/2] crypto: Add IV generation algorithms

[Binoy Jayan]

Just for reference. Not for merging.

Currently, the iv generation algorithms are implemented in dm-crypt.c.
The goal is to move these algorithms from the dm layer to the kernel
crypto layer by implementing them as template ciphers so they can be
implemented in hardware for performance. As part of this patchset, the
iv-generation code is moved from the dm layer to the crypto layer and
adapt the dm-layer to send a whole 'bio' (as defined in the block layer)
at a time. Each bio contains an in memory representation of physically
contiguous disk blocks. The dm layer sets up a chained scatterlist of
these blocks split into physically contiguous segments in memory so that
DMA can be performed. Also, the key management code is moved from dm layer
to the cryto layer since the key selection for encrypting neighboring
sectors depend on the keycount.

Synchronous crypto requests to encrypt/decrypt a sector are processed
sequentially. Asynchronous requests if processed in parallel, are freed
in the async callback. The storage space for the initialization vectors
are allocated in the iv generator implementations.

Interface to the crypto layer - include/crypto/geniv.h

Signed-off-by: Binoy Jayan <binoy.ja...@linaro.org>
---
 drivers/md/dm-crypt.c  | 1939 ++--
 include/crypto/geniv.h |   46 ++
 2 files changed, 1432 insertions(+), 553 deletions(-)
 create mode 100644 include/crypto/geniv.h

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 389a363..bef54f5 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -32,170 +32,120 @@
 #include 
 #include 
 #include 
-
 #include 
-
-#define DM_MSG_PREFIX "crypt"
-
-/*
- * context holding the current state of a multi-part conversion
- */
-struct convert_context {
-   struct completion restart;
-   struct bio *bio_in;
-   struct bio *bio_out;
-   struct bvec_iter iter_in;
-   struct bvec_iter iter_out;
-   sector_t cc_sector;
-   atomic_t cc_pending;
-   struct skcipher_request *req;
+#include 
+#include 
+#include 
+#include 
+
+#define DM_MSG_PREFIX  "crypt"
+#define MAX_SG_LIST(BIO_MAX_PAGES * 8)
+#define MIN_IOS64
+#define LMK_SEED_SIZE  64 /* hash + 0 */
+#define TCW_WHITENING_SIZE 16
+
+struct geniv_ctx;
+struct geniv_req_ctx;
+
+/* Sub request for each of the skcipher_request's for a segment */
+struct geniv_subreq {
+   struct scatterlist src;
+   struct scatterlist dst;
+   struct geniv_req_ctx *rctx;
+   struct skcipher_request req CRYPTO_MINALIGN_ATTR;
 };
 
-/*
- * per bio private data
- */
-struct dm_crypt_io {
-   struct crypt_config *cc;
-   struct bio *base_bio;
-   struct work_struct work;
-
-   struct convert_context ctx;
-
-   atomic_t io_pending;
-   int error;
-   sector_t sector;
-
-   struct rb_node rb_node;
-} CRYPTO_MINALIGN_ATTR;
-
-struct dm_crypt_request {
-   struct convert_context *ctx;
-   struct scatterlist sg_in;
-   struct scatterlist sg_out;
+struct geniv_req_ctx {
+   struct geniv_subreq *subreq;
+   int is_write;
sector_t iv_sector;
+   unsigned int nents;
+   struct completion restart;
+   atomic_t req_pending;
+   struct skcipher_request *req;
 };
 
-struct crypt_config;
-
 struct crypt_iv_operations {
-   int (*ctr)(struct crypt_config *cc, struct dm_target *ti,
-  const char *opts);
-   void (*dtr)(struct crypt_config *cc);
-   int (*init)(struct crypt_config *cc);
-   int (*wipe)(struct crypt_config *cc);
-   int (*generator)(struct crypt_config *cc, u8 *iv,
-struct dm_crypt_request *dmreq);
-   int (*post)(struct crypt_config *cc, u8 *iv,
-   struct dm_crypt_request *dmreq);
+   int (*ctr)(struct geniv_ctx *ctx);
+   void (*dtr)(struct geniv_ctx *ctx);
+   int (*init)(struct geniv_ctx *ctx);
+   int (*wipe)(struct geniv_ctx *ctx);
+   int (*generator)(struct geniv_ctx *ctx,
+struct geniv_req_ctx *rctx,
+struct geniv_subreq *subreq, u8 *iv);
+   int (*post)(struct geniv_ctx *ctx,
+   struct geniv_req_ctx *rctx,
+   struct geniv_subreq *subreq, u8 *iv);
 };
 
-struct iv_essiv_private {
+struct geniv_essiv_private {
struct crypto_ahash *hash_tfm;
u8 *salt;
 };
 
-struct iv_benbi_private {
+struct geniv_benbi_private {
int shift;
 };
 
-#define LMK_SEED_SIZE 64 /* hash + 0 */
-struct iv_lmk_private {
+struct geniv_lmk_private {
struct crypto_shash *hash_tfm;
u8 *seed;
 };
 
-#define TCW_WHITENING_SIZE 16
-struct iv_tcw_private {
+struct geniv_tcw_private {
struct crypto_shash *crc32_tfm;
u8 *iv_seed;
u8 *whitening;
 };
 
-/*
- * Crypt: maps a linear range of a block device
- * and encrypts / decrypts at the s

[PATCH v6 1/2] crypto: Add IV generation algorithms

[Binoy Jayan]

Just for reference. Not for merging.

Currently, the iv generation algorithms are implemented in dm-crypt.c.
The goal is to move these algorithms from the dm layer to the kernel
crypto layer by implementing them as template ciphers so they can be
implemented in hardware for performance. As part of this patchset, the
iv-generation code is moved from the dm layer to the crypto layer and
adapt the dm-layer to send a whole 'bio' (as defined in the block layer)
at a time. Each bio contains an in memory representation of physically
contiguous disk blocks. The dm layer sets up a chained scatterlist of
these blocks split into physically contiguous segments in memory so that
DMA can be performed. Also, the key management code is moved from dm layer
to the cryto layer since the key selection for encrypting neighboring
sectors depend on the keycount.

Synchronous crypto requests to encrypt/decrypt a sector are processed
sequentially. Asynchronous requests if processed in parallel, are freed
in the async callback. The storage space for the initialization vectors
are allocated in the iv generator implementations.

Interface to the crypto layer - include/crypto/geniv.h

Signed-off-by: Binoy Jayan 
---
 drivers/md/dm-crypt.c  | 1939 ++--
 include/crypto/geniv.h |   46 ++
 2 files changed, 1432 insertions(+), 553 deletions(-)
 create mode 100644 include/crypto/geniv.h

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 389a363..bef54f5 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -32,170 +32,120 @@
 #include 
 #include 
 #include 
-
 #include 
-
-#define DM_MSG_PREFIX "crypt"
-
-/*
- * context holding the current state of a multi-part conversion
- */
-struct convert_context {
-   struct completion restart;
-   struct bio *bio_in;
-   struct bio *bio_out;
-   struct bvec_iter iter_in;
-   struct bvec_iter iter_out;
-   sector_t cc_sector;
-   atomic_t cc_pending;
-   struct skcipher_request *req;
+#include 
+#include 
+#include 
+#include 
+
+#define DM_MSG_PREFIX  "crypt"
+#define MAX_SG_LIST(BIO_MAX_PAGES * 8)
+#define MIN_IOS64
+#define LMK_SEED_SIZE  64 /* hash + 0 */
+#define TCW_WHITENING_SIZE 16
+
+struct geniv_ctx;
+struct geniv_req_ctx;
+
+/* Sub request for each of the skcipher_request's for a segment */
+struct geniv_subreq {
+   struct scatterlist src;
+   struct scatterlist dst;
+   struct geniv_req_ctx *rctx;
+   struct skcipher_request req CRYPTO_MINALIGN_ATTR;
 };
 
-/*
- * per bio private data
- */
-struct dm_crypt_io {
-   struct crypt_config *cc;
-   struct bio *base_bio;
-   struct work_struct work;
-
-   struct convert_context ctx;
-
-   atomic_t io_pending;
-   int error;
-   sector_t sector;
-
-   struct rb_node rb_node;
-} CRYPTO_MINALIGN_ATTR;
-
-struct dm_crypt_request {
-   struct convert_context *ctx;
-   struct scatterlist sg_in;
-   struct scatterlist sg_out;
+struct geniv_req_ctx {
+   struct geniv_subreq *subreq;
+   int is_write;
sector_t iv_sector;
+   unsigned int nents;
+   struct completion restart;
+   atomic_t req_pending;
+   struct skcipher_request *req;
 };
 
-struct crypt_config;
-
 struct crypt_iv_operations {
-   int (*ctr)(struct crypt_config *cc, struct dm_target *ti,
-  const char *opts);
-   void (*dtr)(struct crypt_config *cc);
-   int (*init)(struct crypt_config *cc);
-   int (*wipe)(struct crypt_config *cc);
-   int (*generator)(struct crypt_config *cc, u8 *iv,
-struct dm_crypt_request *dmreq);
-   int (*post)(struct crypt_config *cc, u8 *iv,
-   struct dm_crypt_request *dmreq);
+   int (*ctr)(struct geniv_ctx *ctx);
+   void (*dtr)(struct geniv_ctx *ctx);
+   int (*init)(struct geniv_ctx *ctx);
+   int (*wipe)(struct geniv_ctx *ctx);
+   int (*generator)(struct geniv_ctx *ctx,
+struct geniv_req_ctx *rctx,
+struct geniv_subreq *subreq, u8 *iv);
+   int (*post)(struct geniv_ctx *ctx,
+   struct geniv_req_ctx *rctx,
+   struct geniv_subreq *subreq, u8 *iv);
 };
 
-struct iv_essiv_private {
+struct geniv_essiv_private {
struct crypto_ahash *hash_tfm;
u8 *salt;
 };
 
-struct iv_benbi_private {
+struct geniv_benbi_private {
int shift;
 };
 
-#define LMK_SEED_SIZE 64 /* hash + 0 */
-struct iv_lmk_private {
+struct geniv_lmk_private {
struct crypto_shash *hash_tfm;
u8 *seed;
 };
 
-#define TCW_WHITENING_SIZE 16
-struct iv_tcw_private {
+struct geniv_tcw_private {
struct crypto_shash *crc32_tfm;
u8 *iv_seed;
u8 *whitening;
 };
 
-/*
- * Crypt: maps a linear range of a block device
- * and encrypts / decrypts at the same time.
- */
-enum flags 

[PATCH v6 2/2] crypto: Multikey template for essiv

[Binoy Jayan]

Just for reference and to get the performance numbers.
Not for merging.

Depends on the following patches by Gilad:
 MAINTAINERS: add Gilad BY as maintainer for ccree
 staging: ccree: add devicetree bindings
 staging: ccree: add TODO list
 staging: add ccree crypto driver

A multi key template implementation which calls the underlying
iv generator 'essiv-aes-du512-dx' cum crypto algorithm. This
template sits on top of the underlying IV generator and accepts
a key length that is a multiple of the underlying key length.
This has not been tested on Juno with the CryptoCell accelerator
for which it was written for.

The underlying IV generator 'essiv-aes-du512-dx' generates IV for
every 512 byte blocks.

Signed-off-by: Binoy Jayan <binoy.ja...@linaro.org>
---
 drivers/md/dm-crypt.c|5 +-
 drivers/staging/ccree/Makefile   |2 +-
 drivers/staging/ccree/essiv.c|  777 
 drivers/staging/ccree/essiv_sw.c | 1040 ++
 4 files changed, 1821 insertions(+), 3 deletions(-)
 create mode 100644 drivers/staging/ccree/essiv.c
 create mode 100644 drivers/staging/ccree/essiv_sw.c

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index bef54f5..32f75dd 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -1555,7 +1555,8 @@ static int __init geniv_register_algs(void)
if (err)
goto out_undo_plain;
 
-   err = crypto_register_template(_essiv_tmpl);
+   err = 0;
+   // err = crypto_register_template(_essiv_tmpl);
if (err)
goto out_undo_plain64;
 
@@ -1594,7 +1595,7 @@ static void __exit geniv_deregister_algs(void)
 {
crypto_unregister_template(_plain_tmpl);
crypto_unregister_template(_plain64_tmpl);
-   crypto_unregister_template(_essiv_tmpl);
+   // crypto_unregister_template(_essiv_tmpl);
crypto_unregister_template(_benbi_tmpl);
crypto_unregister_template(_null_tmpl);
crypto_unregister_template(_lmk_tmpl);
diff --git a/drivers/staging/ccree/Makefile b/drivers/staging/ccree/Makefile
index 44f3e3e..524e930 100644
--- a/drivers/staging/ccree/Makefile
+++ b/drivers/staging/ccree/Makefile
@@ -1,3 +1,3 @@
 obj-$(CONFIG_CRYPTO_DEV_CCREE) := ccree.o
-ccree-y := ssi_driver.o ssi_sysfs.o ssi_buffer_mgr.o ssi_request_mgr.o 
ssi_cipher.o ssi_hash.o ssi_aead.o ssi_ivgen.o ssi_sram_mgr.o ssi_pm.o 
ssi_pm_ext.o
+ccree-y := ssi_driver.o ssi_sysfs.o ssi_buffer_mgr.o ssi_request_mgr.o 
ssi_cipher.o ssi_hash.o ssi_aead.o ssi_ivgen.o ssi_sram_mgr.o ssi_pm.o 
ssi_pm_ext.o essiv.o
 ccree-$(CCREE_FIPS_SUPPORT) += ssi_fips.o ssi_fips_ll.o ssi_fips_ext.o 
ssi_fips_local.o
diff --git a/drivers/staging/ccree/essiv.c b/drivers/staging/ccree/essiv.c
new file mode 100644
index 000..719b8bf
--- /dev/null
+++ b/drivers/staging/ccree/essiv.c
@@ -0,0 +1,777 @@
+/*
+ * Copyright (C) 2003 Jana Saout <j...@saout.de>
+ * Copyright (C) 2004 Clemens Fruhwirth <clem...@endorphin.org>
+ * Copyright (C) 2006-2015 Red Hat, Inc. All rights reserved.
+ * Copyright (C) 2013 Milan Broz <gmazyl...@gmail.com>
+ *
+ * This file is released under the GPL.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DM_MSG_PREFIX  "crypt"
+#define MAX_SG_LIST(BIO_MAX_PAGES * 8)
+#define MIN_IOS64
+#define LMK_SEED_SIZE  64 /* hash + 0 */
+#define TCW_WHITENING_SIZE 16
+
+struct geniv_ctx;
+struct geniv_req_ctx;
+
+/* Sub request for each of the skcipher_request's for a segment */
+struct geniv_subreq {
+   struct scatterlist src;
+   struct scatterlist dst;
+   struct geniv_req_ctx *rctx;
+   struct skcipher_request req CRYPTO_MINALIGN_ATTR;
+};
+
+struct geniv_req_ctx {
+   struct geniv_subreq *subreq;
+   int is_write;
+   sector_t iv_sector;
+   unsigned int nents;
+   struct completion restart;
+   atomic_t req_pending;
+   struct skcipher_request *req;
+};
+
+struct crypt_iv_operations {
+   int (*ctr)(struct geniv_ctx *ctx);
+   void (*dtr)(struct geniv_ctx *ctx);
+   int (*init)(struct geniv_ctx *ctx);
+   int (*wipe)(struct geniv_ctx *ctx);
+   int (*generator)(struct geniv_ctx *ctx,
+struct geniv_req_ctx *rctx,
+struct geniv_subreq *subreq, u8 *iv);
+   int (*post)(struct geniv_ctx *ctx,
+   struct geniv_req_ctx *rctx,
+   struct geniv_subreq *subreq, u8 *iv);
+};
+
+struct geniv_ctx {
+   unsigned int tfms_count;
+   struct crypto_skcipher *child;
+   struct crypto_skcipher **tfms;
+   char *ivmode;
+   unsigned int iv_s

[PATCH v6 2/2] crypto: Multikey template for essiv

[Binoy Jayan]

Just for reference and to get the performance numbers.
Not for merging.

Depends on the following patches by Gilad:
 MAINTAINERS: add Gilad BY as maintainer for ccree
 staging: ccree: add devicetree bindings
 staging: ccree: add TODO list
 staging: add ccree crypto driver

A multi key template implementation which calls the underlying
iv generator 'essiv-aes-du512-dx' cum crypto algorithm. This
template sits on top of the underlying IV generator and accepts
a key length that is a multiple of the underlying key length.
This has not been tested on Juno with the CryptoCell accelerator
for which it was written for.

The underlying IV generator 'essiv-aes-du512-dx' generates IV for
every 512 byte blocks.

Signed-off-by: Binoy Jayan 
---
 drivers/md/dm-crypt.c|5 +-
 drivers/staging/ccree/Makefile   |2 +-
 drivers/staging/ccree/essiv.c|  777 
 drivers/staging/ccree/essiv_sw.c | 1040 ++
 4 files changed, 1821 insertions(+), 3 deletions(-)
 create mode 100644 drivers/staging/ccree/essiv.c
 create mode 100644 drivers/staging/ccree/essiv_sw.c

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index bef54f5..32f75dd 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -1555,7 +1555,8 @@ static int __init geniv_register_algs(void)
if (err)
goto out_undo_plain;
 
-   err = crypto_register_template(_essiv_tmpl);
+   err = 0;
+   // err = crypto_register_template(_essiv_tmpl);
if (err)
goto out_undo_plain64;
 
@@ -1594,7 +1595,7 @@ static void __exit geniv_deregister_algs(void)
 {
crypto_unregister_template(_plain_tmpl);
crypto_unregister_template(_plain64_tmpl);
-   crypto_unregister_template(_essiv_tmpl);
+   // crypto_unregister_template(_essiv_tmpl);
crypto_unregister_template(_benbi_tmpl);
crypto_unregister_template(_null_tmpl);
crypto_unregister_template(_lmk_tmpl);
diff --git a/drivers/staging/ccree/Makefile b/drivers/staging/ccree/Makefile
index 44f3e3e..524e930 100644
--- a/drivers/staging/ccree/Makefile
+++ b/drivers/staging/ccree/Makefile
@@ -1,3 +1,3 @@
 obj-$(CONFIG_CRYPTO_DEV_CCREE) := ccree.o
-ccree-y := ssi_driver.o ssi_sysfs.o ssi_buffer_mgr.o ssi_request_mgr.o 
ssi_cipher.o ssi_hash.o ssi_aead.o ssi_ivgen.o ssi_sram_mgr.o ssi_pm.o 
ssi_pm_ext.o
+ccree-y := ssi_driver.o ssi_sysfs.o ssi_buffer_mgr.o ssi_request_mgr.o 
ssi_cipher.o ssi_hash.o ssi_aead.o ssi_ivgen.o ssi_sram_mgr.o ssi_pm.o 
ssi_pm_ext.o essiv.o
 ccree-$(CCREE_FIPS_SUPPORT) += ssi_fips.o ssi_fips_ll.o ssi_fips_ext.o 
ssi_fips_local.o
diff --git a/drivers/staging/ccree/essiv.c b/drivers/staging/ccree/essiv.c
new file mode 100644
index 000..719b8bf
--- /dev/null
+++ b/drivers/staging/ccree/essiv.c
@@ -0,0 +1,777 @@
+/*
+ * Copyright (C) 2003 Jana Saout 
+ * Copyright (C) 2004 Clemens Fruhwirth 
+ * Copyright (C) 2006-2015 Red Hat, Inc. All rights reserved.
+ * Copyright (C) 2013 Milan Broz 
+ *
+ * This file is released under the GPL.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define DM_MSG_PREFIX  "crypt"
+#define MAX_SG_LIST(BIO_MAX_PAGES * 8)
+#define MIN_IOS64
+#define LMK_SEED_SIZE  64 /* hash + 0 */
+#define TCW_WHITENING_SIZE 16
+
+struct geniv_ctx;
+struct geniv_req_ctx;
+
+/* Sub request for each of the skcipher_request's for a segment */
+struct geniv_subreq {
+   struct scatterlist src;
+   struct scatterlist dst;
+   struct geniv_req_ctx *rctx;
+   struct skcipher_request req CRYPTO_MINALIGN_ATTR;
+};
+
+struct geniv_req_ctx {
+   struct geniv_subreq *subreq;
+   int is_write;
+   sector_t iv_sector;
+   unsigned int nents;
+   struct completion restart;
+   atomic_t req_pending;
+   struct skcipher_request *req;
+};
+
+struct crypt_iv_operations {
+   int (*ctr)(struct geniv_ctx *ctx);
+   void (*dtr)(struct geniv_ctx *ctx);
+   int (*init)(struct geniv_ctx *ctx);
+   int (*wipe)(struct geniv_ctx *ctx);
+   int (*generator)(struct geniv_ctx *ctx,
+struct geniv_req_ctx *rctx,
+struct geniv_subreq *subreq, u8 *iv);
+   int (*post)(struct geniv_ctx *ctx,
+   struct geniv_req_ctx *rctx,
+   struct geniv_subreq *subreq, u8 *iv);
+};
+
+struct geniv_ctx {
+   unsigned int tfms_count;
+   struct crypto_skcipher *child;
+   struct crypto_skcipher **tfms;
+   char *ivmode;
+   unsigned int iv_size;
+   unsigned int iv_start;
+   char *algname;
+   char *ivopts;
+   char *cipher;
+ 

[PATCH v4] HID: Remove the semaphore driver_lock

[Binoy Jayan]

The semaphore 'driver_lock' is used as a simple mutex, and
also unnecessary as suggested by Arnd. Hence removing it, as
the concurrency between the probe and remove is already
handled in the driver core.

Suggested-by: Arnd Bergmann <a...@arndb.de>
Signed-off-by: Binoy Jayan <binoy.ja...@linaro.org>
Acked-by: Benjamin Tissoires <benjamin.tissoi...@redhat.com>
Reviewed-by: David Herrmann <dh.herrm...@gmail.com>
---

v3 --> v4:
Changed title

v2 --> v3:
Removed reference to driver_lock in comments

v1 --> v2:
Removed driver_lock

 drivers/hid/hid-core.c | 15 ---
 include/linux/hid.h|  3 +--
 2 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 04cee65..559533b 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2225,11 +2225,9 @@ static int hid_device_probe(struct device *dev)
const struct hid_device_id *id;
int ret = 0;
 
-   if (down_interruptible(>driver_lock))
-   return -EINTR;
if (down_interruptible(>driver_input_lock)) {
ret = -EINTR;
-   goto unlock_driver_lock;
+   goto end;
}
hdev->io_started = false;
 
@@ -2256,8 +2254,7 @@ static int hid_device_probe(struct device *dev)
 unlock:
if (!hdev->io_started)
up(>driver_input_lock);
-unlock_driver_lock:
-   up(>driver_lock);
+end:
return ret;
 }
 
@@ -2267,11 +2264,9 @@ static int hid_device_remove(struct device *dev)
struct hid_driver *hdrv;
int ret = 0;
 
-   if (down_interruptible(>driver_lock))
-   return -EINTR;
if (down_interruptible(>driver_input_lock)) {
ret = -EINTR;
-   goto unlock_driver_lock;
+   goto end;
}
hdev->io_started = false;
 
@@ -2287,8 +2282,7 @@ static int hid_device_remove(struct device *dev)
 
if (!hdev->io_started)
up(>driver_input_lock);
-unlock_driver_lock:
-   up(>driver_lock);
+end:
return ret;
 }
 
@@ -2745,7 +2739,6 @@ struct hid_device *hid_allocate_device(void)
init_waitqueue_head(>debug_wait);
INIT_LIST_HEAD(>debug_list);
spin_lock_init(>debug_list_lock);
-   sema_init(>driver_lock, 1);
sema_init(>driver_input_lock, 1);
 
return hdev;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 5be325d..a49203f 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -516,7 +516,6 @@ struct hid_device { 
/* device report descriptor */
struct hid_report_enum report_enum[HID_REPORT_TYPES];
struct work_struct led_work;/* 
delayed LED worker */
 
-   struct semaphore driver_lock;   /* 
protects the current driver, except during input */
struct semaphore driver_input_lock; /* 
protects the current driver */
struct device dev;  /* 
device */
struct hid_driver *driver;
@@ -538,7 +537,7 @@ struct hid_device { 
/* device report descriptor */
unsigned int status;/* see 
STAT flags above */
unsigned claimed;   /* 
Claimed by hidinput, hiddev? */
unsigned quirks;/* 
Various quirks the device can pull on us */
-   bool io_started;/* 
Protected by driver_lock. If IO has started */
+   bool io_started;/* If 
IO has started */
 
struct list_head inputs;/* The 
list of inputs */
void *hiddev;   /* The 
hiddev structure */
-- 
Binoy Jayan

[PATCH v4] HID: Remove the semaphore driver_lock

[Binoy Jayan]

The semaphore 'driver_lock' is used as a simple mutex, and
also unnecessary as suggested by Arnd. Hence removing it, as
the concurrency between the probe and remove is already
handled in the driver core.

Suggested-by: Arnd Bergmann 
Signed-off-by: Binoy Jayan 
Acked-by: Benjamin Tissoires 
Reviewed-by: David Herrmann 
---

v3 --> v4:
Changed title

v2 --> v3:
Removed reference to driver_lock in comments

v1 --> v2:
Removed driver_lock

 drivers/hid/hid-core.c | 15 ---
 include/linux/hid.h|  3 +--
 2 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 04cee65..559533b 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2225,11 +2225,9 @@ static int hid_device_probe(struct device *dev)
const struct hid_device_id *id;
int ret = 0;
 
-   if (down_interruptible(>driver_lock))
-   return -EINTR;
if (down_interruptible(>driver_input_lock)) {
ret = -EINTR;
-   goto unlock_driver_lock;
+   goto end;
}
hdev->io_started = false;
 
@@ -2256,8 +2254,7 @@ static int hid_device_probe(struct device *dev)
 unlock:
if (!hdev->io_started)
up(>driver_input_lock);
-unlock_driver_lock:
-   up(>driver_lock);
+end:
return ret;
 }
 
@@ -2267,11 +2264,9 @@ static int hid_device_remove(struct device *dev)
struct hid_driver *hdrv;
int ret = 0;
 
-   if (down_interruptible(>driver_lock))
-   return -EINTR;
if (down_interruptible(>driver_input_lock)) {
ret = -EINTR;
-   goto unlock_driver_lock;
+   goto end;
}
hdev->io_started = false;
 
@@ -2287,8 +2282,7 @@ static int hid_device_remove(struct device *dev)
 
if (!hdev->io_started)
up(>driver_input_lock);
-unlock_driver_lock:
-   up(>driver_lock);
+end:
return ret;
 }
 
@@ -2745,7 +2739,6 @@ struct hid_device *hid_allocate_device(void)
init_waitqueue_head(>debug_wait);
INIT_LIST_HEAD(>debug_list);
spin_lock_init(>debug_list_lock);
-   sema_init(>driver_lock, 1);
sema_init(>driver_input_lock, 1);
 
return hdev;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 5be325d..a49203f 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -516,7 +516,6 @@ struct hid_device { 
/* device report descriptor */
struct hid_report_enum report_enum[HID_REPORT_TYPES];
struct work_struct led_work;/* 
delayed LED worker */
 
-   struct semaphore driver_lock;   /* 
protects the current driver, except during input */
struct semaphore driver_input_lock; /* 
protects the current driver */
struct device dev;  /* 
device */
struct hid_driver *driver;
@@ -538,7 +537,7 @@ struct hid_device { 
/* device report descriptor */
unsigned int status;/* see 
STAT flags above */
unsigned claimed;   /* 
Claimed by hidinput, hiddev? */
unsigned quirks;/* 
Various quirks the device can pull on us */
-   bool io_started;/* 
Protected by driver_lock. If IO has started */
+   bool io_started;/* If 
IO has started */
 
struct list_head inputs;/* The 
list of inputs */
void *hiddev;       /* The 
hiddev structure */
-- 
Binoy Jayan

[PATCH v3] HID: Replace semaphore driver_lock with mutex

[Binoy Jayan]

The semaphore 'driver_lock' is used as a simple mutex, and
also unnecessary as suggested by Arnd. Hence removing it, as
the concurrency between the probe and remove is already
handled in the driver core.

Suggested-by: Arnd Bergmann <a...@arndb.de>
Signed-off-by: Binoy Jayan <binoy.ja...@linaro.org>
Acked-by: Benjamin Tissoires <benjamin.tissoi...@redhat.com>
Reviewed-by: David Herrmann <dh.herrm...@gmail.com>
---

v2 --> v3:
Removed reference to driver_lock in comments

v1 --> v2:
Removed driver_lock

 drivers/hid/hid-core.c | 15 ---
 include/linux/hid.h|  3 +--
 2 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 04cee65..559533b 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2225,11 +2225,9 @@ static int hid_device_probe(struct device *dev)
const struct hid_device_id *id;
int ret = 0;
 
-   if (down_interruptible(>driver_lock))
-   return -EINTR;
if (down_interruptible(>driver_input_lock)) {
ret = -EINTR;
-   goto unlock_driver_lock;
+   goto end;
}
hdev->io_started = false;
 
@@ -2256,8 +2254,7 @@ static int hid_device_probe(struct device *dev)
 unlock:
if (!hdev->io_started)
up(>driver_input_lock);
-unlock_driver_lock:
-   up(>driver_lock);
+end:
return ret;
 }
 
@@ -2267,11 +2264,9 @@ static int hid_device_remove(struct device *dev)
struct hid_driver *hdrv;
int ret = 0;
 
-   if (down_interruptible(>driver_lock))
-   return -EINTR;
if (down_interruptible(>driver_input_lock)) {
ret = -EINTR;
-   goto unlock_driver_lock;
+   goto end;
}
hdev->io_started = false;
 
@@ -2287,8 +2282,7 @@ static int hid_device_remove(struct device *dev)
 
if (!hdev->io_started)
up(>driver_input_lock);
-unlock_driver_lock:
-   up(>driver_lock);
+end:
return ret;
 }
 
@@ -2745,7 +2739,6 @@ struct hid_device *hid_allocate_device(void)
init_waitqueue_head(>debug_wait);
INIT_LIST_HEAD(>debug_list);
spin_lock_init(>debug_list_lock);
-   sema_init(>driver_lock, 1);
sema_init(>driver_input_lock, 1);
 
return hdev;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 5be325d..a49203f 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -516,7 +516,6 @@ struct hid_device { 
/* device report descriptor */
struct hid_report_enum report_enum[HID_REPORT_TYPES];
struct work_struct led_work;/* 
delayed LED worker */
 
-   struct semaphore driver_lock;   /* 
protects the current driver, except during input */
struct semaphore driver_input_lock; /* 
protects the current driver */
struct device dev;  /* 
device */
struct hid_driver *driver;
@@ -538,7 +537,7 @@ struct hid_device { 
/* device report descriptor */
unsigned int status;/* see 
STAT flags above */
unsigned claimed;   /* 
Claimed by hidinput, hiddev? */
unsigned quirks;/* 
Various quirks the device can pull on us */
-   bool io_started;/* 
Protected by driver_lock. If IO has started */
+   bool io_started;/* If 
IO has started */
 
struct list_head inputs;/* The 
list of inputs */
void *hiddev;       /* The 
hiddev structure */
-- 
Binoy Jayan

[PATCH v3] HID: Replace semaphore driver_lock with mutex

[Binoy Jayan]

The semaphore 'driver_lock' is used as a simple mutex, and
also unnecessary as suggested by Arnd. Hence removing it, as
the concurrency between the probe and remove is already
handled in the driver core.

Suggested-by: Arnd Bergmann 
Signed-off-by: Binoy Jayan 
Acked-by: Benjamin Tissoires 
Reviewed-by: David Herrmann 
---

v2 --> v3:
Removed reference to driver_lock in comments

v1 --> v2:
Removed driver_lock

 drivers/hid/hid-core.c | 15 ---
 include/linux/hid.h|  3 +--
 2 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 04cee65..559533b 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2225,11 +2225,9 @@ static int hid_device_probe(struct device *dev)
const struct hid_device_id *id;
int ret = 0;
 
-   if (down_interruptible(>driver_lock))
-   return -EINTR;
if (down_interruptible(>driver_input_lock)) {
ret = -EINTR;
-   goto unlock_driver_lock;
+   goto end;
}
hdev->io_started = false;
 
@@ -2256,8 +2254,7 @@ static int hid_device_probe(struct device *dev)
 unlock:
if (!hdev->io_started)
up(>driver_input_lock);
-unlock_driver_lock:
-   up(>driver_lock);
+end:
return ret;
 }
 
@@ -2267,11 +2264,9 @@ static int hid_device_remove(struct device *dev)
struct hid_driver *hdrv;
int ret = 0;
 
-   if (down_interruptible(>driver_lock))
-   return -EINTR;
if (down_interruptible(>driver_input_lock)) {
ret = -EINTR;
-   goto unlock_driver_lock;
+   goto end;
}
hdev->io_started = false;
 
@@ -2287,8 +2282,7 @@ static int hid_device_remove(struct device *dev)
 
if (!hdev->io_started)
up(>driver_input_lock);
-unlock_driver_lock:
-   up(>driver_lock);
+end:
return ret;
 }
 
@@ -2745,7 +2739,6 @@ struct hid_device *hid_allocate_device(void)
init_waitqueue_head(>debug_wait);
INIT_LIST_HEAD(>debug_list);
spin_lock_init(>debug_list_lock);
-   sema_init(>driver_lock, 1);
sema_init(>driver_input_lock, 1);
 
return hdev;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 5be325d..a49203f 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -516,7 +516,6 @@ struct hid_device { 
/* device report descriptor */
struct hid_report_enum report_enum[HID_REPORT_TYPES];
struct work_struct led_work;/* 
delayed LED worker */
 
-   struct semaphore driver_lock;   /* 
protects the current driver, except during input */
struct semaphore driver_input_lock; /* 
protects the current driver */
struct device dev;  /* 
device */
struct hid_driver *driver;
@@ -538,7 +537,7 @@ struct hid_device { 
/* device report descriptor */
unsigned int status;/* see 
STAT flags above */
unsigned claimed;   /* 
Claimed by hidinput, hiddev? */
unsigned quirks;/* 
Various quirks the device can pull on us */
-   bool io_started;/* 
Protected by driver_lock. If IO has started */
+   bool io_started;/* If 
IO has started */
 
struct list_head inputs;/* The 
list of inputs */
void *hiddev;       /* The 
hiddev structure */
-- 
Binoy Jayan

Re: [PATCH v2] HID: Replace semaphore driver_lock with mutex

[Binoy Jayan]

Hi,

On 14 June 2017 at 01:55, Arnd Bergmann  wrote:

>> The mutex code clearly states mutex_trylock() must not be used in
>> interrupt context (see kernel/locking/mutex.c), hence we used a
>> semaphore here. Unless the mutex code is changed to allow this, we
>> cannot switch away from semaphores.
>
> Right, that makes a lot of sense. I don't think changing the mutex
> code is an option here, but I wonder if we can replace the semaphore
> with something simpler anyway.
>
> From what I can tell, it currently does two things:
>
> 1. it acts as a simple flag to prevent  hid_input_report from derefencing
> the hid->driver pointer during initialization and exit. I think this could
> be done equally well using a simple atomic set_bit()/test_bit() or 
> similar.
>
> 2. it prevents the hid->driver pointer from becoming invalid while an
> asynchronous hid_input_report() is in progress. This actually seems to
> be a reference counting problem rather than a locking problem.
> I don't immediately see how to better address it, or how exactly this
> could go wrong in practice, but I would naively expect that either
> hdev->driver->remove() needs to wait for the last user of hdev->driver
> to complete, or we need kref_get/kref_put in hid_input_report()
> to trigger the actual release function.

Thank you everyone for the comments. I'll resend the patch with Benjamin's
comments incorporated and address the changes in the second semaphore later.

Regards,
Binoy

Re: [PATCH v2] HID: Replace semaphore driver_lock with mutex

[Binoy Jayan]

Hi,

On 14 June 2017 at 01:55, Arnd Bergmann  wrote:

>> The mutex code clearly states mutex_trylock() must not be used in
>> interrupt context (see kernel/locking/mutex.c), hence we used a
>> semaphore here. Unless the mutex code is changed to allow this, we
>> cannot switch away from semaphores.
>
> Right, that makes a lot of sense. I don't think changing the mutex
> code is an option here, but I wonder if we can replace the semaphore
> with something simpler anyway.
>
> From what I can tell, it currently does two things:
>
> 1. it acts as a simple flag to prevent  hid_input_report from derefencing
> the hid->driver pointer during initialization and exit. I think this could
> be done equally well using a simple atomic set_bit()/test_bit() or 
> similar.
>
> 2. it prevents the hid->driver pointer from becoming invalid while an
> asynchronous hid_input_report() is in progress. This actually seems to
> be a reference counting problem rather than a locking problem.
> I don't immediately see how to better address it, or how exactly this
> could go wrong in practice, but I would naively expect that either
> hdev->driver->remove() needs to wait for the last user of hdev->driver
> to complete, or we need kref_get/kref_put in hid_input_report()
> to trigger the actual release function.

Thank you everyone for the comments. I'll resend the patch with Benjamin's
comments incorporated and address the changes in the second semaphore later.

Regards,
Binoy

Re: [PATCH v2 0/3] ngene: Replace semaphores with mutexes

[Binoy Jayan]

Hi Arnd,

On 13 June 2017 at 15:19, Arnd Bergmann <a...@arndb.de> wrote:
> On Tue, Jun 13, 2017 at 10:58 AM, Binoy Jayan <binoy.ja...@linaro.org> wrote:
>> These are a set of patches [v2] which removes semaphores from ngene.
>> These are part of a bigger effort to eliminate unwanted semaphores
>> from the linux kernel.
>
> All three
>
> Acked-by: Arnd Bergmann <a...@arndb.de>
>
> I already gave an Ack for one or two of the patches in the first round, but
> you seem to have dropped that. When you resend a patch with an Ack,
> please include that above your Signed-off-by line. (No need to resend
> for an Ack otherwise, this normally gets picked up when the patch
> gets applied from the list.

Sorry I dropped it as there were changes in two of the patches.
But there were obvious ones anyway.

Thanks,
Binoy

Re: [PATCH v2 0/3] ngene: Replace semaphores with mutexes

[Binoy Jayan]

Hi Arnd,

On 13 June 2017 at 15:19, Arnd Bergmann  wrote:
> On Tue, Jun 13, 2017 at 10:58 AM, Binoy Jayan  wrote:
>> These are a set of patches [v2] which removes semaphores from ngene.
>> These are part of a bigger effort to eliminate unwanted semaphores
>> from the linux kernel.
>
> All three
>
> Acked-by: Arnd Bergmann 
>
> I already gave an Ack for one or two of the patches in the first round, but
> you seem to have dropped that. When you resend a patch with an Ack,
> please include that above your Signed-off-by line. (No need to resend
> for an Ack otherwise, this normally gets picked up when the patch
> gets applied from the list.

Sorry I dropped it as there were changes in two of the patches.
But there were obvious ones anyway.

Thanks,
Binoy

Re: [PATCH v2] HID: Replace semaphore driver_lock with mutex

[Binoy Jayan]

Hi Arnd,

On 13 June 2017 at 15:15, Arnd Bergmann  wrote:
> Looks good to me, but I see you didn't include David and Andrew on
> Cc, it would be good for at least one of them to provide an Ack as well.

Will include them, thank you yet again for reminding me.

> You forgot to actually drop the definition.

And for this too.

Regards,
Binoy

Re: [PATCH v2] HID: Replace semaphore driver_lock with mutex

[Binoy Jayan]

Hi Arnd,

On 13 June 2017 at 15:15, Arnd Bergmann  wrote:
> Looks good to me, but I see you didn't include David and Andrew on
> Cc, it would be good for at least one of them to provide an Ack as well.

Will include them, thank you yet again for reminding me.

> You forgot to actually drop the definition.

And for this too.

Regards,
Binoy

Re: [PATCH v2] HID: Replace semaphore driver_lock with mutex

[Binoy Jayan]

Hi,

On 13 June 2017 at 15:26, Benjamin Tissoires
 wrote:

>> Looks good to me, but I see you didn't include David and Andrew on
>> Cc, it would be good for at least one of them to provide an Ack as well.
>
> Please also CC linux-input@

Will do that.
> (one more nitpick below too)
> A little bit below, there is:
> bool io_started;/* 
> Protected by driver_lock. If IO has started */
>
> You should probably remove the mention to driver_lock here.

Will remove the reference too.

>> > -   struct semaphore driver_lock;   /* 
>> > protects the current driver, except during input */
>> > +   struct mutex driver_lock;   /* 
>> > protects the current driver, except during input */
>> > struct semaphore driver_input_lock; /* 
>> > protects the current driver */
>
> Unless I am mistaken, this one could also be converted to a mutex (in a
> separate patch, of course).

Thank you for noticing that, initially I missed it as I thought
'io_started' somehow
influences the increment of the semaphore, but its anyway used only in
hid-core.c

Thanks,
Binoy

Re: [PATCH v2] HID: Replace semaphore driver_lock with mutex

[Binoy Jayan]

Hi,

On 13 June 2017 at 15:26, Benjamin Tissoires
 wrote:

>> Looks good to me, but I see you didn't include David and Andrew on
>> Cc, it would be good for at least one of them to provide an Ack as well.
>
> Please also CC linux-input@

Will do that.
> (one more nitpick below too)
> A little bit below, there is:
> bool io_started;/* 
> Protected by driver_lock. If IO has started */
>
> You should probably remove the mention to driver_lock here.

Will remove the reference too.

>> > -   struct semaphore driver_lock;   /* 
>> > protects the current driver, except during input */
>> > +   struct mutex driver_lock;   /* 
>> > protects the current driver, except during input */
>> > struct semaphore driver_input_lock; /* 
>> > protects the current driver */
>
> Unless I am mistaken, this one could also be converted to a mutex (in a
> separate patch, of course).

Thank you for noticing that, initially I missed it as I thought
'io_started' somehow
influences the increment of the semaphore, but its anyway used only in
hid-core.c

Thanks,
Binoy

[PATCH v2] HID: Replace semaphore driver_lock with mutex

[Binoy Jayan]

The semaphore 'driver_lock' is used as a simple mutex, and
also unnecessary as suggested by Arnd. Hence removing it, as
the concurrency between the probe and remove is already
handled in the driver core.

Signed-off-by: Binoy Jayan <binoy.ja...@linaro.org>
Suggested-by: Arnd Bergmann <a...@arndb.de>
---

v1 --> v2

Removed driver_lock

 drivers/hid/hid-core.c | 15 ---
 include/linux/hid.h|  2 +-
 2 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 04cee65..559533b 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2225,11 +2225,9 @@ static int hid_device_probe(struct device *dev)
const struct hid_device_id *id;
int ret = 0;
 
-   if (down_interruptible(>driver_lock))
-   return -EINTR;
if (down_interruptible(>driver_input_lock)) {
ret = -EINTR;
-   goto unlock_driver_lock;
+   goto end;
}
hdev->io_started = false;
 
@@ -2256,8 +2254,7 @@ static int hid_device_probe(struct device *dev)
 unlock:
if (!hdev->io_started)
up(>driver_input_lock);
-unlock_driver_lock:
-   up(>driver_lock);
+end:
return ret;
 }
 
@@ -2267,11 +2264,9 @@ static int hid_device_remove(struct device *dev)
struct hid_driver *hdrv;
int ret = 0;
 
-   if (down_interruptible(>driver_lock))
-   return -EINTR;
if (down_interruptible(>driver_input_lock)) {
ret = -EINTR;
-   goto unlock_driver_lock;
+   goto end;
}
hdev->io_started = false;
 
@@ -2287,8 +2282,7 @@ static int hid_device_remove(struct device *dev)
 
if (!hdev->io_started)
up(>driver_input_lock);
-unlock_driver_lock:
-   up(>driver_lock);
+end:
return ret;
 }
 
@@ -2745,7 +2739,6 @@ struct hid_device *hid_allocate_device(void)
init_waitqueue_head(>debug_wait);
INIT_LIST_HEAD(>debug_list);
spin_lock_init(>debug_list_lock);
-   sema_init(>driver_lock, 1);
sema_init(>driver_input_lock, 1);
 
return hdev;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 5be325d..1add2b3 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -516,7 +516,7 @@ struct hid_device { 
/* device report descriptor */
struct hid_report_enum report_enum[HID_REPORT_TYPES];
struct work_struct led_work;/* 
delayed LED worker */
 
-   struct semaphore driver_lock;   /* 
protects the current driver, except during input */
+   struct mutex driver_lock;   /* 
protects the current driver, except during input */
struct semaphore driver_input_lock; /* 
protects the current driver */
struct device dev;      /* 
device */
struct hid_driver *driver;
-- 
Binoy Jayan

[PATCH v2] HID: Replace semaphore driver_lock with mutex

[Binoy Jayan]

The semaphore 'driver_lock' is used as a simple mutex, and
also unnecessary as suggested by Arnd. Hence removing it, as
the concurrency between the probe and remove is already
handled in the driver core.

Signed-off-by: Binoy Jayan 
Suggested-by: Arnd Bergmann 
---

v1 --> v2

Removed driver_lock

 drivers/hid/hid-core.c | 15 ---
 include/linux/hid.h|  2 +-
 2 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 04cee65..559533b 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -2225,11 +2225,9 @@ static int hid_device_probe(struct device *dev)
const struct hid_device_id *id;
int ret = 0;
 
-   if (down_interruptible(>driver_lock))
-   return -EINTR;
if (down_interruptible(>driver_input_lock)) {
ret = -EINTR;
-   goto unlock_driver_lock;
+   goto end;
}
hdev->io_started = false;
 
@@ -2256,8 +2254,7 @@ static int hid_device_probe(struct device *dev)
 unlock:
if (!hdev->io_started)
up(>driver_input_lock);
-unlock_driver_lock:
-   up(>driver_lock);
+end:
return ret;
 }
 
@@ -2267,11 +2264,9 @@ static int hid_device_remove(struct device *dev)
struct hid_driver *hdrv;
int ret = 0;
 
-   if (down_interruptible(>driver_lock))
-   return -EINTR;
if (down_interruptible(>driver_input_lock)) {
ret = -EINTR;
-   goto unlock_driver_lock;
+   goto end;
}
hdev->io_started = false;
 
@@ -2287,8 +2282,7 @@ static int hid_device_remove(struct device *dev)
 
if (!hdev->io_started)
up(>driver_input_lock);
-unlock_driver_lock:
-   up(>driver_lock);
+end:
return ret;
 }
 
@@ -2745,7 +2739,6 @@ struct hid_device *hid_allocate_device(void)
init_waitqueue_head(>debug_wait);
INIT_LIST_HEAD(>debug_list);
spin_lock_init(>debug_list_lock);
-   sema_init(>driver_lock, 1);
sema_init(>driver_input_lock, 1);
 
return hdev;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 5be325d..1add2b3 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -516,7 +516,7 @@ struct hid_device { 
/* device report descriptor */
struct hid_report_enum report_enum[HID_REPORT_TYPES];
struct work_struct led_work;/* 
delayed LED worker */
 
-   struct semaphore driver_lock;   /* 
protects the current driver, except during input */
+   struct mutex driver_lock;   /* 
protects the current driver, except during input */
struct semaphore driver_input_lock; /* 
protects the current driver */
struct device dev;  /* 
device */
struct hid_driver *driver;
-- 
Binoy Jayan

[PATCH v2 3/3] media: ngene: Replace semaphore i2c_switch_mutex with mutex

[Binoy Jayan]

The semaphore 'i2c_switch_mutex' is used as a simple mutex, so
it should be written as one. Semaphores are going away in the future.

Signed-off-by: Binoy Jayan <binoy.ja...@linaro.org>
---
 drivers/media/pci/ngene/ngene-core.c | 2 +-
 drivers/media/pci/ngene/ngene-i2c.c  | 6 +++---
 drivers/media/pci/ngene/ngene.h  | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/media/pci/ngene/ngene-core.c 
b/drivers/media/pci/ngene/ngene-core.c
index ea64901..8c92cb7 100644
--- a/drivers/media/pci/ngene/ngene-core.c
+++ b/drivers/media/pci/ngene/ngene-core.c
@@ -1345,7 +1345,7 @@ static int ngene_start(struct ngene *dev)
mutex_init(>cmd_mutex);
mutex_init(>stream_mutex);
sema_init(>pll_mutex, 1);
-   sema_init(>i2c_switch_mutex, 1);
+   mutex_init(>i2c_switch_mutex);
spin_lock_init(>cmd_lock);
for (i = 0; i < MAX_STREAM; i++)
spin_lock_init(>channel[i].state_lock);
diff --git a/drivers/media/pci/ngene/ngene-i2c.c 
b/drivers/media/pci/ngene/ngene-i2c.c
index cf39fcf..fbf3635 100644
--- a/drivers/media/pci/ngene/ngene-i2c.c
+++ b/drivers/media/pci/ngene/ngene-i2c.c
@@ -118,7 +118,7 @@ static int ngene_i2c_master_xfer(struct i2c_adapter 
*adapter,
(struct ngene_channel *)i2c_get_adapdata(adapter);
struct ngene *dev = chan->dev;
 
-   down(>i2c_switch_mutex);
+   mutex_lock(>i2c_switch_mutex);
ngene_i2c_set_bus(dev, chan->number);
 
if (num == 2 && msg[1].flags & I2C_M_RD && !(msg[0].flags & I2C_M_RD))
@@ -136,11 +136,11 @@ static int ngene_i2c_master_xfer(struct i2c_adapter 
*adapter,
msg[0].buf, msg[0].len, 0))
goto done;
 
-   up(>i2c_switch_mutex);
+   mutex_unlock(>i2c_switch_mutex);
return -EIO;
 
 done:
-   up(>i2c_switch_mutex);
+   mutex_unlock(>i2c_switch_mutex);
return num;
 }
 
diff --git a/drivers/media/pci/ngene/ngene.h b/drivers/media/pci/ngene/ngene.h
index 0dd15d6..7c7cd21 100644
--- a/drivers/media/pci/ngene/ngene.h
+++ b/drivers/media/pci/ngene/ngene.h
@@ -765,7 +765,7 @@ struct ngene {
struct mutex  cmd_mutex;
struct mutex  stream_mutex;
struct semaphore  pll_mutex;
-   struct semaphore  i2c_switch_mutex;
+   struct mutex  i2c_switch_mutex;
int   i2c_current_channel;
int       i2c_current_bus;
spinlock_tcmd_lock;
-- 
Binoy Jayan

[PATCH v2 3/3] media: ngene: Replace semaphore i2c_switch_mutex with mutex

[Binoy Jayan]

The semaphore 'i2c_switch_mutex' is used as a simple mutex, so
it should be written as one. Semaphores are going away in the future.

Signed-off-by: Binoy Jayan 
---
 drivers/media/pci/ngene/ngene-core.c | 2 +-
 drivers/media/pci/ngene/ngene-i2c.c  | 6 +++---
 drivers/media/pci/ngene/ngene.h  | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/media/pci/ngene/ngene-core.c 
b/drivers/media/pci/ngene/ngene-core.c
index ea64901..8c92cb7 100644
--- a/drivers/media/pci/ngene/ngene-core.c
+++ b/drivers/media/pci/ngene/ngene-core.c
@@ -1345,7 +1345,7 @@ static int ngene_start(struct ngene *dev)
mutex_init(>cmd_mutex);
mutex_init(>stream_mutex);
sema_init(>pll_mutex, 1);
-   sema_init(>i2c_switch_mutex, 1);
+   mutex_init(>i2c_switch_mutex);
spin_lock_init(>cmd_lock);
for (i = 0; i < MAX_STREAM; i++)
spin_lock_init(>channel[i].state_lock);
diff --git a/drivers/media/pci/ngene/ngene-i2c.c 
b/drivers/media/pci/ngene/ngene-i2c.c
index cf39fcf..fbf3635 100644
--- a/drivers/media/pci/ngene/ngene-i2c.c
+++ b/drivers/media/pci/ngene/ngene-i2c.c
@@ -118,7 +118,7 @@ static int ngene_i2c_master_xfer(struct i2c_adapter 
*adapter,
(struct ngene_channel *)i2c_get_adapdata(adapter);
struct ngene *dev = chan->dev;
 
-   down(>i2c_switch_mutex);
+   mutex_lock(>i2c_switch_mutex);
ngene_i2c_set_bus(dev, chan->number);
 
if (num == 2 && msg[1].flags & I2C_M_RD && !(msg[0].flags & I2C_M_RD))
@@ -136,11 +136,11 @@ static int ngene_i2c_master_xfer(struct i2c_adapter 
*adapter,
msg[0].buf, msg[0].len, 0))
goto done;
 
-   up(>i2c_switch_mutex);
+   mutex_unlock(>i2c_switch_mutex);
return -EIO;
 
 done:
-   up(>i2c_switch_mutex);
+   mutex_unlock(>i2c_switch_mutex);
return num;
 }
 
diff --git a/drivers/media/pci/ngene/ngene.h b/drivers/media/pci/ngene/ngene.h
index 0dd15d6..7c7cd21 100644
--- a/drivers/media/pci/ngene/ngene.h
+++ b/drivers/media/pci/ngene/ngene.h
@@ -765,7 +765,7 @@ struct ngene {
struct mutex  cmd_mutex;
struct mutex  stream_mutex;
struct semaphore  pll_mutex;
-   struct semaphore  i2c_switch_mutex;
+   struct mutex  i2c_switch_mutex;
int   i2c_current_channel;
int   i2c_current_bus;
spinlock_tcmd_lock;
-- 
Binoy Jayan

Re: [PATCH 1/3] media: ngene: Replace semaphore cmd_mutex with mutex

[Binoy Jayan]

Hi Arnd,

On 9 June 2017 at 16:06, Arnd Bergmann  wrote:

>> Thank you for pointing out that. I'll check the
>> concurrency part. By the way why do we need mutex_destoy?
>> To debug an aberrate condition?
>
> At first I suspected the down() here was added for the same
> purpose as a mutex_destroy: to ensure that we are in a sane
> state before we free the device structure, but the way they
> achieve that is completely different.
>
> However, if there is any way that a command may still be in
> progress by the time we get to ngene_stop(), we may also
> be lacking reference counting on the ngene structure here.
> So far I haven't found any of those, and think the mutex_destroy()
> is sufficient here as a debugging help.

I've made the necessary changes. Thank you for reviewing all the patches.

Regards,
Binoy

Re: [PATCH 1/3] media: ngene: Replace semaphore cmd_mutex with mutex

[Binoy Jayan]

Hi Arnd,

On 9 June 2017 at 16:06, Arnd Bergmann  wrote:

>> Thank you for pointing out that. I'll check the
>> concurrency part. By the way why do we need mutex_destoy?
>> To debug an aberrate condition?
>
> At first I suspected the down() here was added for the same
> purpose as a mutex_destroy: to ensure that we are in a sane
> state before we free the device structure, but the way they
> achieve that is completely different.
>
> However, if there is any way that a command may still be in
> progress by the time we get to ngene_stop(), we may also
> be lacking reference counting on the ngene structure here.
> So far I haven't found any of those, and think the mutex_destroy()
> is sufficient here as a debugging help.

I've made the necessary changes. Thank you for reviewing all the patches.

Regards,
Binoy

[PATCH v2 1/3] media: ngene: Replace semaphore cmd_mutex with mutex

[Binoy Jayan]

The semaphore 'cmd_mutex' is used as a simple mutex, so
it should be written as one. Also, replace down with
mutex_destroy to ensure sane state when ngene_stop is
called.

Signed-off-by: Binoy Jayan <binoy.ja...@linaro.org>
---
 drivers/media/pci/ngene/ngene-core.c | 12 ++--
 drivers/media/pci/ngene/ngene.h  |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/media/pci/ngene/ngene-core.c 
b/drivers/media/pci/ngene/ngene-core.c
index ce69e64..eeb61eb 100644
--- a/drivers/media/pci/ngene/ngene-core.c
+++ b/drivers/media/pci/ngene/ngene-core.c
@@ -336,9 +336,9 @@ int ngene_command(struct ngene *dev, struct ngene_command 
*com)
 {
int result;
 
-   down(>cmd_mutex);
+   mutex_lock(>cmd_mutex);
result = ngene_command_mutex(dev, com);
-   up(>cmd_mutex);
+   mutex_unlock(>cmd_mutex);
return result;
 }
 
@@ -1283,7 +1283,7 @@ static int ngene_load_firm(struct ngene *dev)
 
 static void ngene_stop(struct ngene *dev)
 {
-   down(>cmd_mutex);
+   mutex_destroy(>cmd_mutex);
i2c_del_adapter(&(dev->channel[0].i2c_adapter));
i2c_del_adapter(&(dev->channel[1].i2c_adapter));
ngwritel(0, NGENE_INT_ENABLE);
@@ -1346,7 +1346,7 @@ static int ngene_start(struct ngene *dev)
init_waitqueue_head(>cmd_wq);
init_waitqueue_head(>tx_wq);
init_waitqueue_head(>rx_wq);
-   sema_init(>cmd_mutex, 1);
+   mutex_init(>cmd_mutex);
sema_init(>stream_mutex, 1);
sema_init(>pll_mutex, 1);
sema_init(>i2c_switch_mutex, 1);
@@ -1606,10 +1606,10 @@ static void ngene_unlink(struct ngene *dev)
com.in_len = 3;
com.out_len = 1;
 
-   down(>cmd_mutex);
+   mutex_lock(>cmd_mutex);
ngwritel(0, NGENE_INT_ENABLE);
ngene_command_mutex(dev, );
-   up(>cmd_mutex);
+   mutex_unlock(>cmd_mutex);
 }
 
 void ngene_shutdown(struct pci_dev *pdev)
diff --git a/drivers/media/pci/ngene/ngene.h b/drivers/media/pci/ngene/ngene.h
index 10d8f74..e600b70 100644
--- a/drivers/media/pci/ngene/ngene.h
+++ b/drivers/media/pci/ngene/ngene.h
@@ -762,7 +762,7 @@ struct ngene {
 
wait_queue_head_t cmd_wq;
int   cmd_done;
-   struct semaphore  cmd_mutex;
+   struct mutex  cmd_mutex;
struct semaphore  stream_mutex;
struct semaphore  pll_mutex;
struct semaphore  i2c_switch_mutex;
-- 
Binoy Jayan

[PATCH v2 1/3] media: ngene: Replace semaphore cmd_mutex with mutex

[Binoy Jayan]

The semaphore 'cmd_mutex' is used as a simple mutex, so
it should be written as one. Also, replace down with
mutex_destroy to ensure sane state when ngene_stop is
called.

Signed-off-by: Binoy Jayan 
---
 drivers/media/pci/ngene/ngene-core.c | 12 ++--
 drivers/media/pci/ngene/ngene.h  |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/media/pci/ngene/ngene-core.c 
b/drivers/media/pci/ngene/ngene-core.c
index ce69e64..eeb61eb 100644
--- a/drivers/media/pci/ngene/ngene-core.c
+++ b/drivers/media/pci/ngene/ngene-core.c
@@ -336,9 +336,9 @@ int ngene_command(struct ngene *dev, struct ngene_command 
*com)
 {
int result;
 
-   down(>cmd_mutex);
+   mutex_lock(>cmd_mutex);
result = ngene_command_mutex(dev, com);
-   up(>cmd_mutex);
+   mutex_unlock(>cmd_mutex);
return result;
 }
 
@@ -1283,7 +1283,7 @@ static int ngene_load_firm(struct ngene *dev)
 
 static void ngene_stop(struct ngene *dev)
 {
-   down(>cmd_mutex);
+   mutex_destroy(>cmd_mutex);
i2c_del_adapter(&(dev->channel[0].i2c_adapter));
i2c_del_adapter(&(dev->channel[1].i2c_adapter));
ngwritel(0, NGENE_INT_ENABLE);
@@ -1346,7 +1346,7 @@ static int ngene_start(struct ngene *dev)
init_waitqueue_head(>cmd_wq);
init_waitqueue_head(>tx_wq);
init_waitqueue_head(>rx_wq);
-   sema_init(>cmd_mutex, 1);
+   mutex_init(>cmd_mutex);
sema_init(>stream_mutex, 1);
sema_init(>pll_mutex, 1);
sema_init(>i2c_switch_mutex, 1);
@@ -1606,10 +1606,10 @@ static void ngene_unlink(struct ngene *dev)
com.in_len = 3;
com.out_len = 1;
 
-   down(>cmd_mutex);
+   mutex_lock(>cmd_mutex);
ngwritel(0, NGENE_INT_ENABLE);
ngene_command_mutex(dev, );
-   up(>cmd_mutex);
+   mutex_unlock(>cmd_mutex);
 }
 
 void ngene_shutdown(struct pci_dev *pdev)
diff --git a/drivers/media/pci/ngene/ngene.h b/drivers/media/pci/ngene/ngene.h
index 10d8f74..e600b70 100644
--- a/drivers/media/pci/ngene/ngene.h
+++ b/drivers/media/pci/ngene/ngene.h
@@ -762,7 +762,7 @@ struct ngene {
 
wait_queue_head_t cmd_wq;
int   cmd_done;
-   struct semaphore  cmd_mutex;
+   struct mutex  cmd_mutex;
struct semaphore  stream_mutex;
struct semaphore  pll_mutex;
struct semaphore  i2c_switch_mutex;
-- 
Binoy Jayan

[PATCH v2 0/3] ngene: Replace semaphores with mutexes

[Binoy Jayan]

These are a set of patches [v2] which removes semaphores from ngene.
These are part of a bigger effort to eliminate unwanted semaphores
from the linux kernel.

v1 --> v2
-

Moved mutex_[lock/unlock] outside caller for stream_mutex
mutex_lock converted to mutex_destroy in cmd_mutex

Binoy Jayan (3):
  media: ngene: Replace semaphore cmd_mutex with mutex
  media: ngene: Replace semaphore stream_mutex with mutex
  media: ngene: Replace semaphore i2c_switch_mutex with mutex

 drivers/media/pci/ngene/ngene-core.c | 32 ++--
 drivers/media/pci/ngene/ngene-i2c.c  |  6 +++---
 drivers/media/pci/ngene/ngene.h  |  6 +++---
 3 files changed, 20 insertions(+), 24 deletions(-)

-- 
Binoy Jayan

[PATCH v2 2/3] media: ngene: Replace semaphore stream_mutex with mutex

[Binoy Jayan]

The semaphore 'stream_mutex' is used as a simple mutex, so
it should be written as one. Also moving the mutex_[lock/unlock]
to the caller as it is anyway locked at the beginning of the
callee thus avoiding repetition.

Signed-off-by: Binoy Jayan <binoy.ja...@linaro.org>
---
 drivers/media/pci/ngene/ngene-core.c | 18 +++---
 drivers/media/pci/ngene/ngene.h  |  2 +-
 2 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/media/pci/ngene/ngene-core.c 
b/drivers/media/pci/ngene/ngene-core.c
index eeb61eb..ea64901 100644
--- a/drivers/media/pci/ngene/ngene-core.c
+++ b/drivers/media/pci/ngene/ngene-core.c
@@ -560,7 +560,6 @@ static int ngene_command_stream_control(struct ngene *dev, 
u8 stream,
u16 BsSPI = ((stream & 1) ? 0x9800 : 0x9700);
u16 BsSDO = 0x9B00;
 
-   down(>stream_mutex);
memset(, 0, sizeof(com));
com.cmd.hdr.Opcode = CMD_CONTROL;
com.cmd.hdr.Length = sizeof(struct FW_STREAM_CONTROL) - 2;
@@ -586,17 +585,13 @@ static int ngene_command_stream_control(struct ngene 
*dev, u8 stream,
chan->State = KSSTATE_ACQUIRE;
chan->HWState = HWSTATE_STOP;
spin_unlock_irq(>state_lock);
-   if (ngene_command(dev, ) < 0) {
-   up(>stream_mutex);
+   if (ngene_command(dev, ) < 0)
return -1;
-   }
/* clear_buffers(chan); */
flush_buffers(chan);
-   up(>stream_mutex);
return 0;
}
spin_unlock_irq(>state_lock);
-   up(>stream_mutex);
return 0;
}
 
@@ -692,11 +687,9 @@ static int ngene_command_stream_control(struct ngene *dev, 
u8 stream,
chan->HWState = HWSTATE_STARTUP;
spin_unlock_irq(>state_lock);
 
-   if (ngene_command(dev, ) < 0) {
-   up(>stream_mutex);
+   if (ngene_command(dev, ) < 0)
return -1;
-   }
-   up(>stream_mutex);
+
return 0;
 }
 
@@ -750,8 +743,11 @@ void set_transfer(struct ngene_channel *chan, int state)
/* else printk(KERN_INFO DEVICE_NAME ": lock=%08x\n",
   ngreadl(0x9310)); */
 
+   mutex_lock(>stream_mutex);
ret = ngene_command_stream_control(dev, chan->number,
   control, mode, flags);
+   mutex_unlock(>stream_mutex);
+
if (!ret)
chan->running = state;
else
@@ -1347,7 +1343,7 @@ static int ngene_start(struct ngene *dev)
init_waitqueue_head(>tx_wq);
init_waitqueue_head(>rx_wq);
mutex_init(>cmd_mutex);
-   sema_init(>stream_mutex, 1);
+   mutex_init(>stream_mutex);
sema_init(>pll_mutex, 1);
sema_init(>i2c_switch_mutex, 1);
spin_lock_init(>cmd_lock);
diff --git a/drivers/media/pci/ngene/ngene.h b/drivers/media/pci/ngene/ngene.h
index e600b70..0dd15d6 100644
--- a/drivers/media/pci/ngene/ngene.h
+++ b/drivers/media/pci/ngene/ngene.h
@@ -763,7 +763,7 @@ struct ngene {
wait_queue_head_t cmd_wq;
int   cmd_done;
struct mutex  cmd_mutex;
-   struct semaphore  stream_mutex;
+   struct mutex  stream_mutex;
struct semaphore  pll_mutex;
    struct semaphore  i2c_switch_mutex;
int   i2c_current_channel;
-- 
Binoy Jayan

[PATCH v2 0/3] ngene: Replace semaphores with mutexes

[Binoy Jayan]

These are a set of patches [v2] which removes semaphores from ngene.
These are part of a bigger effort to eliminate unwanted semaphores
from the linux kernel.

v1 --> v2
-

Moved mutex_[lock/unlock] outside caller for stream_mutex
mutex_lock converted to mutex_destroy in cmd_mutex

Binoy Jayan (3):
  media: ngene: Replace semaphore cmd_mutex with mutex
  media: ngene: Replace semaphore stream_mutex with mutex
  media: ngene: Replace semaphore i2c_switch_mutex with mutex

 drivers/media/pci/ngene/ngene-core.c | 32 ++--
 drivers/media/pci/ngene/ngene-i2c.c  |  6 +++---
 drivers/media/pci/ngene/ngene.h  |  6 +++---
 3 files changed, 20 insertions(+), 24 deletions(-)

-- 
Binoy Jayan

[PATCH v2 2/3] media: ngene: Replace semaphore stream_mutex with mutex

[Binoy Jayan]

The semaphore 'stream_mutex' is used as a simple mutex, so
it should be written as one. Also moving the mutex_[lock/unlock]
to the caller as it is anyway locked at the beginning of the
callee thus avoiding repetition.

Signed-off-by: Binoy Jayan 
---
 drivers/media/pci/ngene/ngene-core.c | 18 +++---
 drivers/media/pci/ngene/ngene.h  |  2 +-
 2 files changed, 8 insertions(+), 12 deletions(-)

diff --git a/drivers/media/pci/ngene/ngene-core.c 
b/drivers/media/pci/ngene/ngene-core.c
index eeb61eb..ea64901 100644
--- a/drivers/media/pci/ngene/ngene-core.c
+++ b/drivers/media/pci/ngene/ngene-core.c
@@ -560,7 +560,6 @@ static int ngene_command_stream_control(struct ngene *dev, 
u8 stream,
u16 BsSPI = ((stream & 1) ? 0x9800 : 0x9700);
u16 BsSDO = 0x9B00;
 
-   down(>stream_mutex);
memset(, 0, sizeof(com));
com.cmd.hdr.Opcode = CMD_CONTROL;
com.cmd.hdr.Length = sizeof(struct FW_STREAM_CONTROL) - 2;
@@ -586,17 +585,13 @@ static int ngene_command_stream_control(struct ngene 
*dev, u8 stream,
chan->State = KSSTATE_ACQUIRE;
chan->HWState = HWSTATE_STOP;
spin_unlock_irq(>state_lock);
-   if (ngene_command(dev, ) < 0) {
-   up(>stream_mutex);
+   if (ngene_command(dev, ) < 0)
return -1;
-   }
/* clear_buffers(chan); */
flush_buffers(chan);
-   up(>stream_mutex);
return 0;
}
spin_unlock_irq(>state_lock);
-   up(>stream_mutex);
return 0;
}
 
@@ -692,11 +687,9 @@ static int ngene_command_stream_control(struct ngene *dev, 
u8 stream,
chan->HWState = HWSTATE_STARTUP;
spin_unlock_irq(>state_lock);
 
-   if (ngene_command(dev, ) < 0) {
-   up(>stream_mutex);
+   if (ngene_command(dev, ) < 0)
return -1;
-   }
-   up(>stream_mutex);
+
return 0;
 }
 
@@ -750,8 +743,11 @@ void set_transfer(struct ngene_channel *chan, int state)
/* else printk(KERN_INFO DEVICE_NAME ": lock=%08x\n",
   ngreadl(0x9310)); */
 
+   mutex_lock(>stream_mutex);
ret = ngene_command_stream_control(dev, chan->number,
   control, mode, flags);
+   mutex_unlock(>stream_mutex);
+
if (!ret)
chan->running = state;
else
@@ -1347,7 +1343,7 @@ static int ngene_start(struct ngene *dev)
init_waitqueue_head(>tx_wq);
init_waitqueue_head(>rx_wq);
mutex_init(>cmd_mutex);
-   sema_init(>stream_mutex, 1);
+   mutex_init(>stream_mutex);
sema_init(>pll_mutex, 1);
sema_init(>i2c_switch_mutex, 1);
spin_lock_init(>cmd_lock);
diff --git a/drivers/media/pci/ngene/ngene.h b/drivers/media/pci/ngene/ngene.h
index e600b70..0dd15d6 100644
--- a/drivers/media/pci/ngene/ngene.h
+++ b/drivers/media/pci/ngene/ngene.h
@@ -763,7 +763,7 @@ struct ngene {
wait_queue_head_t cmd_wq;
int   cmd_done;
struct mutex  cmd_mutex;
-   struct semaphore  stream_mutex;
+   struct mutex  stream_mutex;
struct semaphore  pll_mutex;
    struct semaphore  i2c_switch_mutex;
int   i2c_current_channel;
-- 
Binoy Jayan

Re: [PATCH 1/3] media: ngene: Replace semaphore cmd_mutex with mutex

[Binoy Jayan]

On 8 June 2017 at 20:40, Arnd Bergmann <a...@arndb.de> wrote:
> On Thu, Jun 8, 2017 at 12:04 PM, Binoy Jayan <binoy.ja...@linaro.org> wrote:
>> The semaphore 'cmd_mutex' is used as a simple mutex, so
>> it should be written as one. Semaphores are going away in the future.
>>
>> Signed-off-by: Binoy Jayan <binoy.ja...@linaro.org>
>> ---
>
>> @@ -1283,7 +1283,7 @@ static int ngene_load_firm(struct ngene *dev)
>>
>>  static void ngene_stop(struct ngene *dev)
>>  {
>> -   down(>cmd_mutex);
>> +   mutex_lock(>cmd_mutex);
>> i2c_del_adapter(&(dev->channel[0].i2c_adapter));
>> i2c_del_adapter(&(dev->channel[1].i2c_adapter));
>> ngwritel(0, NGENE_INT_ENABLE);
>
> Are you sure about this one? There is only one mutex_lock() and
> then the structure gets freed without a corresponding mutex_unlock().
>
> I suspect this violates some rules of mutexes, either when compile
> testing with "make C=1", or when running with lockdep enabled.
>
> Can we actually have a concurrently held mutex at the time we
> get here? If not, using mutex_destroy() in place of the down()
> may be the right answer.

I noticed the missing 'up' here, but may be semaphores do not have
to adhere to that rule? Thank you for pointing out that. I'll check the
concurrency part. By the way why do we need mutex_destoy?
To debug an aberrate condition?

Thanks,
Binoy

Re: [PATCH 1/3] media: ngene: Replace semaphore cmd_mutex with mutex

[Binoy Jayan]

On 8 June 2017 at 20:40, Arnd Bergmann  wrote:
> On Thu, Jun 8, 2017 at 12:04 PM, Binoy Jayan  wrote:
>> The semaphore 'cmd_mutex' is used as a simple mutex, so
>> it should be written as one. Semaphores are going away in the future.
>>
>> Signed-off-by: Binoy Jayan 
>> ---
>
>> @@ -1283,7 +1283,7 @@ static int ngene_load_firm(struct ngene *dev)
>>
>>  static void ngene_stop(struct ngene *dev)
>>  {
>> -   down(>cmd_mutex);
>> +   mutex_lock(>cmd_mutex);
>> i2c_del_adapter(&(dev->channel[0].i2c_adapter));
>> i2c_del_adapter(&(dev->channel[1].i2c_adapter));
>> ngwritel(0, NGENE_INT_ENABLE);
>
> Are you sure about this one? There is only one mutex_lock() and
> then the structure gets freed without a corresponding mutex_unlock().
>
> I suspect this violates some rules of mutexes, either when compile
> testing with "make C=1", or when running with lockdep enabled.
>
> Can we actually have a concurrently held mutex at the time we
> get here? If not, using mutex_destroy() in place of the down()
> may be the right answer.

I noticed the missing 'up' here, but may be semaphores do not have
to adhere to that rule? Thank you for pointing out that. I'll check the
concurrency part. By the way why do we need mutex_destoy?
To debug an aberrate condition?

Thanks,
Binoy

[PATCH 0/2] esas2r: Replace semaphores with mutexes

[Binoy Jayan]

These are a set of patches which removes semaphores from esas2r.
These are part of a bigger effort to eliminate unwanted semaphores
from the linux kernel.

Binoy Jayan (2):
  scsi: esas2r: Replace semaphore fm_api_semaphore with mutex
  scsi: esas2r: Replace semaphore fs_api_semaphore with mutex

 drivers/scsi/esas2r/esas2r.h   |  4 ++--
 drivers/scsi/esas2r/esas2r_init.c  |  4 ++--
 drivers/scsi/esas2r/esas2r_ioctl.c | 10 +-
 3 files changed, 9 insertions(+), 9 deletions(-)

-- 
Binoy Jayan

[PATCH 0/2] esas2r: Replace semaphores with mutexes

[Binoy Jayan]

These are a set of patches which removes semaphores from esas2r.
These are part of a bigger effort to eliminate unwanted semaphores
from the linux kernel.

Binoy Jayan (2):
  scsi: esas2r: Replace semaphore fm_api_semaphore with mutex
  scsi: esas2r: Replace semaphore fs_api_semaphore with mutex

 drivers/scsi/esas2r/esas2r.h   |  4 ++--
 drivers/scsi/esas2r/esas2r_init.c  |  4 ++--
 drivers/scsi/esas2r/esas2r_ioctl.c | 10 +-
 3 files changed, 9 insertions(+), 9 deletions(-)

-- 
Binoy Jayan

[PATCH 1/2] scsi: esas2r: Replace semaphore fm_api_semaphore with mutex

[Binoy Jayan]

The semaphore 'fm_api_semaphore' is used as a simple mutex, so
it should be written as one. Semaphores are going away in the future.

Signed-off-by: Binoy Jayan <binoy.ja...@linaro.org>
---
 drivers/scsi/esas2r/esas2r.h   | 2 +-
 drivers/scsi/esas2r/esas2r_init.c  | 2 +-
 drivers/scsi/esas2r/esas2r_ioctl.c | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/esas2r/esas2r.h b/drivers/scsi/esas2r/esas2r.h
index b6030e3..c5013de 100644
--- a/drivers/scsi/esas2r/esas2r.h
+++ b/drivers/scsi/esas2r/esas2r.h
@@ -945,7 +945,7 @@ struct esas2r_adapter {
struct list_head vrq_mds_head;
struct esas2r_mem_desc *vrq_mds;
int num_vrqs;
-   struct semaphore fm_api_semaphore;
+   struct mutex fm_api_mutex;
struct semaphore fs_api_semaphore;
struct semaphore nvram_semaphore;
struct atto_ioctl *local_atto_ioctl;
diff --git a/drivers/scsi/esas2r/esas2r_init.c 
b/drivers/scsi/esas2r/esas2r_init.c
index 6432a50..ad85b33 100644
--- a/drivers/scsi/esas2r/esas2r_init.c
+++ b/drivers/scsi/esas2r/esas2r_init.c
@@ -327,7 +327,7 @@ int esas2r_init_adapter(struct Scsi_Host *host, struct 
pci_dev *pcid,
esas2r_debug("new adapter %p, name %s", a, a->name);
spin_lock_init(>request_lock);
spin_lock_init(>fw_event_lock);
-   sema_init(>fm_api_semaphore, 1);
+   mutex_init(>fm_api_mutex);
sema_init(>fs_api_semaphore, 1);
sema_init(>nvram_semaphore, 1);
 
diff --git a/drivers/scsi/esas2r/esas2r_ioctl.c 
b/drivers/scsi/esas2r/esas2r_ioctl.c
index 2d4b7f0..c6b041a 100644
--- a/drivers/scsi/esas2r/esas2r_ioctl.c
+++ b/drivers/scsi/esas2r/esas2r_ioctl.c
@@ -110,7 +110,7 @@ static void do_fm_api(struct esas2r_adapter *a, struct 
esas2r_flash_img *fi)
 {
struct esas2r_request *rq;
 
-   if (down_interruptible(>fm_api_semaphore)) {
+   if (mutex_lock_interruptible(>fm_api_mutex)) {
fi->status = FI_STAT_BUSY;
return;
}
@@ -173,7 +173,7 @@ static void do_fm_api(struct esas2r_adapter *a, struct 
esas2r_flash_img *fi)
 free_req:
esas2r_free_request(a, (struct esas2r_request *)rq);
 free_sem:
-   up(>fm_api_semaphore);
+   mutex_unlock(>fm_api_mutex);
return;
 
 }
-- 
Binoy Jayan

[PATCH 1/2] scsi: esas2r: Replace semaphore fm_api_semaphore with mutex

[Binoy Jayan]

The semaphore 'fm_api_semaphore' is used as a simple mutex, so
it should be written as one. Semaphores are going away in the future.

Signed-off-by: Binoy Jayan 
---
 drivers/scsi/esas2r/esas2r.h   | 2 +-
 drivers/scsi/esas2r/esas2r_init.c  | 2 +-
 drivers/scsi/esas2r/esas2r_ioctl.c | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/esas2r/esas2r.h b/drivers/scsi/esas2r/esas2r.h
index b6030e3..c5013de 100644
--- a/drivers/scsi/esas2r/esas2r.h
+++ b/drivers/scsi/esas2r/esas2r.h
@@ -945,7 +945,7 @@ struct esas2r_adapter {
struct list_head vrq_mds_head;
struct esas2r_mem_desc *vrq_mds;
int num_vrqs;
-   struct semaphore fm_api_semaphore;
+   struct mutex fm_api_mutex;
struct semaphore fs_api_semaphore;
struct semaphore nvram_semaphore;
struct atto_ioctl *local_atto_ioctl;
diff --git a/drivers/scsi/esas2r/esas2r_init.c 
b/drivers/scsi/esas2r/esas2r_init.c
index 6432a50..ad85b33 100644
--- a/drivers/scsi/esas2r/esas2r_init.c
+++ b/drivers/scsi/esas2r/esas2r_init.c
@@ -327,7 +327,7 @@ int esas2r_init_adapter(struct Scsi_Host *host, struct 
pci_dev *pcid,
esas2r_debug("new adapter %p, name %s", a, a->name);
spin_lock_init(>request_lock);
spin_lock_init(>fw_event_lock);
-   sema_init(>fm_api_semaphore, 1);
+   mutex_init(>fm_api_mutex);
sema_init(>fs_api_semaphore, 1);
sema_init(>nvram_semaphore, 1);
 
diff --git a/drivers/scsi/esas2r/esas2r_ioctl.c 
b/drivers/scsi/esas2r/esas2r_ioctl.c
index 2d4b7f0..c6b041a 100644
--- a/drivers/scsi/esas2r/esas2r_ioctl.c
+++ b/drivers/scsi/esas2r/esas2r_ioctl.c
@@ -110,7 +110,7 @@ static void do_fm_api(struct esas2r_adapter *a, struct 
esas2r_flash_img *fi)
 {
struct esas2r_request *rq;
 
-   if (down_interruptible(>fm_api_semaphore)) {
+   if (mutex_lock_interruptible(>fm_api_mutex)) {
fi->status = FI_STAT_BUSY;
return;
}
@@ -173,7 +173,7 @@ static void do_fm_api(struct esas2r_adapter *a, struct 
esas2r_flash_img *fi)
 free_req:
esas2r_free_request(a, (struct esas2r_request *)rq);
 free_sem:
-   up(>fm_api_semaphore);
+   mutex_unlock(>fm_api_mutex);
return;
 
 }
-- 
Binoy Jayan

[PATCH 2/2] scsi: esas2r: Replace semaphore fs_api_semaphore with mutex

[Binoy Jayan]

The semaphore 'fs_api_semaphore' is used as a simple mutex, so
it should be written as one. Semaphores are going away in the future.

Signed-off-by: Binoy Jayan <binoy.ja...@linaro.org>
---
 drivers/scsi/esas2r/esas2r.h   | 2 +-
 drivers/scsi/esas2r/esas2r_init.c  | 2 +-
 drivers/scsi/esas2r/esas2r_ioctl.c | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/esas2r/esas2r.h b/drivers/scsi/esas2r/esas2r.h
index c5013de..1da6407 100644
--- a/drivers/scsi/esas2r/esas2r.h
+++ b/drivers/scsi/esas2r/esas2r.h
@@ -946,7 +946,7 @@ struct esas2r_adapter {
struct esas2r_mem_desc *vrq_mds;
int num_vrqs;
struct mutex fm_api_mutex;
-   struct semaphore fs_api_semaphore;
+   struct mutex fs_api_mutex;
struct semaphore nvram_semaphore;
struct atto_ioctl *local_atto_ioctl;
u8 fw_coredump_buff[ESAS2R_FWCOREDUMP_SZ];
diff --git a/drivers/scsi/esas2r/esas2r_init.c 
b/drivers/scsi/esas2r/esas2r_init.c
index ad85b33..5b14dd2 100644
--- a/drivers/scsi/esas2r/esas2r_init.c
+++ b/drivers/scsi/esas2r/esas2r_init.c
@@ -328,7 +328,7 @@ int esas2r_init_adapter(struct Scsi_Host *host, struct 
pci_dev *pcid,
spin_lock_init(>request_lock);
spin_lock_init(>fw_event_lock);
mutex_init(>fm_api_mutex);
-   sema_init(>fs_api_semaphore, 1);
+   mutex_init(>fs_api_mutex);
sema_init(>nvram_semaphore, 1);
 
esas2r_fw_event_off(a);
diff --git a/drivers/scsi/esas2r/esas2r_ioctl.c 
b/drivers/scsi/esas2r/esas2r_ioctl.c
index c6b041a..9762300 100644
--- a/drivers/scsi/esas2r/esas2r_ioctl.c
+++ b/drivers/scsi/esas2r/esas2r_ioctl.c
@@ -1962,7 +1962,7 @@ int esas2r_read_fs(struct esas2r_adapter *a, char *buf, 
long off, int count)
(struct esas2r_ioctl_fs *)a->fs_api_buffer;
 
/* If another flash request is already in progress, return. */
-   if (down_interruptible(>fs_api_semaphore)) {
+   if (mutex_lock_interruptible(>fs_api_mutex)) {
 busy:
fs->status = ATTO_STS_OUT_OF_RSRC;
return -EBUSY;
@@ -1978,7 +1978,7 @@ int esas2r_read_fs(struct esas2r_adapter *a, char *buf, 
long off, int count)
rq = esas2r_alloc_request(a);
if (rq == NULL) {
esas2r_debug("esas2r_read_fs: out of requests");
-   up(>fs_api_semaphore);
+   mutex_unlock(>fs_api_mutex);
goto busy;
}
 
@@ -2006,7 +2006,7 @@ int esas2r_read_fs(struct esas2r_adapter *a, char *buf, 
long off, int count)
;
 dont_wait:
/* Free the request and keep going */
-   up(>fs_api_semaphore);
+   mutex_unlock(>fs_api_mutex);
esas2r_free_request(a, (struct esas2r_request *)rq);
 
        /* Pick up possible error code from above */
-- 
Binoy Jayan

[PATCH 2/2] scsi: esas2r: Replace semaphore fs_api_semaphore with mutex

[Binoy Jayan]

The semaphore 'fs_api_semaphore' is used as a simple mutex, so
it should be written as one. Semaphores are going away in the future.

Signed-off-by: Binoy Jayan 
---
 drivers/scsi/esas2r/esas2r.h   | 2 +-
 drivers/scsi/esas2r/esas2r_init.c  | 2 +-
 drivers/scsi/esas2r/esas2r_ioctl.c | 6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/esas2r/esas2r.h b/drivers/scsi/esas2r/esas2r.h
index c5013de..1da6407 100644
--- a/drivers/scsi/esas2r/esas2r.h
+++ b/drivers/scsi/esas2r/esas2r.h
@@ -946,7 +946,7 @@ struct esas2r_adapter {
struct esas2r_mem_desc *vrq_mds;
int num_vrqs;
struct mutex fm_api_mutex;
-   struct semaphore fs_api_semaphore;
+   struct mutex fs_api_mutex;
struct semaphore nvram_semaphore;
struct atto_ioctl *local_atto_ioctl;
u8 fw_coredump_buff[ESAS2R_FWCOREDUMP_SZ];
diff --git a/drivers/scsi/esas2r/esas2r_init.c 
b/drivers/scsi/esas2r/esas2r_init.c
index ad85b33..5b14dd2 100644
--- a/drivers/scsi/esas2r/esas2r_init.c
+++ b/drivers/scsi/esas2r/esas2r_init.c
@@ -328,7 +328,7 @@ int esas2r_init_adapter(struct Scsi_Host *host, struct 
pci_dev *pcid,
spin_lock_init(>request_lock);
spin_lock_init(>fw_event_lock);
mutex_init(>fm_api_mutex);
-   sema_init(>fs_api_semaphore, 1);
+   mutex_init(>fs_api_mutex);
sema_init(>nvram_semaphore, 1);
 
esas2r_fw_event_off(a);
diff --git a/drivers/scsi/esas2r/esas2r_ioctl.c 
b/drivers/scsi/esas2r/esas2r_ioctl.c
index c6b041a..9762300 100644
--- a/drivers/scsi/esas2r/esas2r_ioctl.c
+++ b/drivers/scsi/esas2r/esas2r_ioctl.c
@@ -1962,7 +1962,7 @@ int esas2r_read_fs(struct esas2r_adapter *a, char *buf, 
long off, int count)
(struct esas2r_ioctl_fs *)a->fs_api_buffer;
 
/* If another flash request is already in progress, return. */
-   if (down_interruptible(>fs_api_semaphore)) {
+   if (mutex_lock_interruptible(>fs_api_mutex)) {
 busy:
fs->status = ATTO_STS_OUT_OF_RSRC;
return -EBUSY;
@@ -1978,7 +1978,7 @@ int esas2r_read_fs(struct esas2r_adapter *a, char *buf, 
long off, int count)
rq = esas2r_alloc_request(a);
if (rq == NULL) {
esas2r_debug("esas2r_read_fs: out of requests");
-   up(>fs_api_semaphore);
+   mutex_unlock(>fs_api_mutex);
goto busy;
}
 
@@ -2006,7 +2006,7 @@ int esas2r_read_fs(struct esas2r_adapter *a, char *buf, 
long off, int count)
;
 dont_wait:
/* Free the request and keep going */
-   up(>fs_api_semaphore);
+   mutex_unlock(>fs_api_mutex);
esas2r_free_request(a, (struct esas2r_request *)rq);
 
    /* Pick up possible error code from above */
-- 
Binoy Jayan

[PATCH 3/3] media: ngene: Replace semaphore i2c_switch_mutex with mutex

[Binoy Jayan]

The semaphore 'i2c_switch_mutex' is used as a simple mutex, so
it should be written as one. Semaphores are going away in the future.

Signed-off-by: Binoy Jayan <binoy.ja...@linaro.org>
---
 drivers/media/pci/ngene/ngene-core.c | 2 +-
 drivers/media/pci/ngene/ngene-i2c.c  | 6 +++---
 drivers/media/pci/ngene/ngene.h  | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/media/pci/ngene/ngene-core.c 
b/drivers/media/pci/ngene/ngene-core.c
index 59f2e5f..ca0c0f8 100644
--- a/drivers/media/pci/ngene/ngene-core.c
+++ b/drivers/media/pci/ngene/ngene-core.c
@@ -1349,7 +1349,7 @@ static int ngene_start(struct ngene *dev)
mutex_init(>cmd_mutex);
mutex_init(>stream_mutex);
sema_init(>pll_mutex, 1);
-   sema_init(>i2c_switch_mutex, 1);
+   mutex_init(>i2c_switch_mutex);
spin_lock_init(>cmd_lock);
for (i = 0; i < MAX_STREAM; i++)
spin_lock_init(>channel[i].state_lock);
diff --git a/drivers/media/pci/ngene/ngene-i2c.c 
b/drivers/media/pci/ngene/ngene-i2c.c
index cf39fcf..fbf3635 100644
--- a/drivers/media/pci/ngene/ngene-i2c.c
+++ b/drivers/media/pci/ngene/ngene-i2c.c
@@ -118,7 +118,7 @@ static int ngene_i2c_master_xfer(struct i2c_adapter 
*adapter,
(struct ngene_channel *)i2c_get_adapdata(adapter);
struct ngene *dev = chan->dev;
 
-   down(>i2c_switch_mutex);
+   mutex_lock(>i2c_switch_mutex);
ngene_i2c_set_bus(dev, chan->number);
 
if (num == 2 && msg[1].flags & I2C_M_RD && !(msg[0].flags & I2C_M_RD))
@@ -136,11 +136,11 @@ static int ngene_i2c_master_xfer(struct i2c_adapter 
*adapter,
msg[0].buf, msg[0].len, 0))
goto done;
 
-   up(>i2c_switch_mutex);
+   mutex_unlock(>i2c_switch_mutex);
return -EIO;
 
 done:
-   up(>i2c_switch_mutex);
+   mutex_unlock(>i2c_switch_mutex);
return num;
 }
 
diff --git a/drivers/media/pci/ngene/ngene.h b/drivers/media/pci/ngene/ngene.h
index 0dd15d6..7c7cd21 100644
--- a/drivers/media/pci/ngene/ngene.h
+++ b/drivers/media/pci/ngene/ngene.h
@@ -765,7 +765,7 @@ struct ngene {
struct mutex  cmd_mutex;
struct mutex  stream_mutex;
struct semaphore  pll_mutex;
-   struct semaphore  i2c_switch_mutex;
+   struct mutex  i2c_switch_mutex;
int   i2c_current_channel;
int       i2c_current_bus;
spinlock_tcmd_lock;
-- 
Binoy Jayan

[PATCH 1/3] media: ngene: Replace semaphore cmd_mutex with mutex

[Binoy Jayan]

The semaphore 'cmd_mutex' is used as a simple mutex, so
it should be written as one. Semaphores are going away in the future.

Signed-off-by: Binoy Jayan <binoy.ja...@linaro.org>
---
 drivers/media/pci/ngene/ngene-core.c | 12 ++--
 drivers/media/pci/ngene/ngene.h  |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/media/pci/ngene/ngene-core.c 
b/drivers/media/pci/ngene/ngene-core.c
index ce69e64..dfbd1e0 100644
--- a/drivers/media/pci/ngene/ngene-core.c
+++ b/drivers/media/pci/ngene/ngene-core.c
@@ -336,9 +336,9 @@ int ngene_command(struct ngene *dev, struct ngene_command 
*com)
 {
int result;
 
-   down(>cmd_mutex);
+   mutex_lock(>cmd_mutex);
result = ngene_command_mutex(dev, com);
-   up(>cmd_mutex);
+   mutex_unlock(>cmd_mutex);
return result;
 }
 
@@ -1283,7 +1283,7 @@ static int ngene_load_firm(struct ngene *dev)
 
 static void ngene_stop(struct ngene *dev)
 {
-   down(>cmd_mutex);
+   mutex_lock(>cmd_mutex);
i2c_del_adapter(&(dev->channel[0].i2c_adapter));
i2c_del_adapter(&(dev->channel[1].i2c_adapter));
ngwritel(0, NGENE_INT_ENABLE);
@@ -1346,7 +1346,7 @@ static int ngene_start(struct ngene *dev)
init_waitqueue_head(>cmd_wq);
init_waitqueue_head(>tx_wq);
init_waitqueue_head(>rx_wq);
-   sema_init(>cmd_mutex, 1);
+   mutex_init(>cmd_mutex);
sema_init(>stream_mutex, 1);
sema_init(>pll_mutex, 1);
sema_init(>i2c_switch_mutex, 1);
@@ -1606,10 +1606,10 @@ static void ngene_unlink(struct ngene *dev)
com.in_len = 3;
com.out_len = 1;
 
-   down(>cmd_mutex);
+   mutex_lock(>cmd_mutex);
ngwritel(0, NGENE_INT_ENABLE);
ngene_command_mutex(dev, );
-   up(>cmd_mutex);
+   mutex_unlock(>cmd_mutex);
 }
 
 void ngene_shutdown(struct pci_dev *pdev)
diff --git a/drivers/media/pci/ngene/ngene.h b/drivers/media/pci/ngene/ngene.h
index 10d8f74..e600b70 100644
--- a/drivers/media/pci/ngene/ngene.h
+++ b/drivers/media/pci/ngene/ngene.h
@@ -762,7 +762,7 @@ struct ngene {
 
wait_queue_head_t cmd_wq;
int   cmd_done;
-   struct semaphore  cmd_mutex;
+   struct mutex  cmd_mutex;
struct semaphore  stream_mutex;
struct semaphore  pll_mutex;
struct semaphore  i2c_switch_mutex;
-- 
Binoy Jayan

[PATCH 3/3] media: ngene: Replace semaphore i2c_switch_mutex with mutex

[Binoy Jayan]

The semaphore 'i2c_switch_mutex' is used as a simple mutex, so
it should be written as one. Semaphores are going away in the future.

Signed-off-by: Binoy Jayan 
---
 drivers/media/pci/ngene/ngene-core.c | 2 +-
 drivers/media/pci/ngene/ngene-i2c.c  | 6 +++---
 drivers/media/pci/ngene/ngene.h  | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/media/pci/ngene/ngene-core.c 
b/drivers/media/pci/ngene/ngene-core.c
index 59f2e5f..ca0c0f8 100644
--- a/drivers/media/pci/ngene/ngene-core.c
+++ b/drivers/media/pci/ngene/ngene-core.c
@@ -1349,7 +1349,7 @@ static int ngene_start(struct ngene *dev)
mutex_init(>cmd_mutex);
mutex_init(>stream_mutex);
sema_init(>pll_mutex, 1);
-   sema_init(>i2c_switch_mutex, 1);
+   mutex_init(>i2c_switch_mutex);
spin_lock_init(>cmd_lock);
for (i = 0; i < MAX_STREAM; i++)
spin_lock_init(>channel[i].state_lock);
diff --git a/drivers/media/pci/ngene/ngene-i2c.c 
b/drivers/media/pci/ngene/ngene-i2c.c
index cf39fcf..fbf3635 100644
--- a/drivers/media/pci/ngene/ngene-i2c.c
+++ b/drivers/media/pci/ngene/ngene-i2c.c
@@ -118,7 +118,7 @@ static int ngene_i2c_master_xfer(struct i2c_adapter 
*adapter,
(struct ngene_channel *)i2c_get_adapdata(adapter);
struct ngene *dev = chan->dev;
 
-   down(>i2c_switch_mutex);
+   mutex_lock(>i2c_switch_mutex);
ngene_i2c_set_bus(dev, chan->number);
 
if (num == 2 && msg[1].flags & I2C_M_RD && !(msg[0].flags & I2C_M_RD))
@@ -136,11 +136,11 @@ static int ngene_i2c_master_xfer(struct i2c_adapter 
*adapter,
msg[0].buf, msg[0].len, 0))
goto done;
 
-   up(>i2c_switch_mutex);
+   mutex_unlock(>i2c_switch_mutex);
return -EIO;
 
 done:
-   up(>i2c_switch_mutex);
+   mutex_unlock(>i2c_switch_mutex);
return num;
 }
 
diff --git a/drivers/media/pci/ngene/ngene.h b/drivers/media/pci/ngene/ngene.h
index 0dd15d6..7c7cd21 100644
--- a/drivers/media/pci/ngene/ngene.h
+++ b/drivers/media/pci/ngene/ngene.h
@@ -765,7 +765,7 @@ struct ngene {
struct mutex  cmd_mutex;
struct mutex  stream_mutex;
struct semaphore  pll_mutex;
-   struct semaphore  i2c_switch_mutex;
+   struct mutex  i2c_switch_mutex;
int   i2c_current_channel;
int   i2c_current_bus;
spinlock_tcmd_lock;
-- 
Binoy Jayan

[PATCH 1/3] media: ngene: Replace semaphore cmd_mutex with mutex

[Binoy Jayan]

The semaphore 'cmd_mutex' is used as a simple mutex, so
it should be written as one. Semaphores are going away in the future.

Signed-off-by: Binoy Jayan 
---
 drivers/media/pci/ngene/ngene-core.c | 12 ++--
 drivers/media/pci/ngene/ngene.h  |  2 +-
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/media/pci/ngene/ngene-core.c 
b/drivers/media/pci/ngene/ngene-core.c
index ce69e64..dfbd1e0 100644
--- a/drivers/media/pci/ngene/ngene-core.c
+++ b/drivers/media/pci/ngene/ngene-core.c
@@ -336,9 +336,9 @@ int ngene_command(struct ngene *dev, struct ngene_command 
*com)
 {
int result;
 
-   down(>cmd_mutex);
+   mutex_lock(>cmd_mutex);
result = ngene_command_mutex(dev, com);
-   up(>cmd_mutex);
+   mutex_unlock(>cmd_mutex);
return result;
 }
 
@@ -1283,7 +1283,7 @@ static int ngene_load_firm(struct ngene *dev)
 
 static void ngene_stop(struct ngene *dev)
 {
-   down(>cmd_mutex);
+   mutex_lock(>cmd_mutex);
i2c_del_adapter(&(dev->channel[0].i2c_adapter));
i2c_del_adapter(&(dev->channel[1].i2c_adapter));
ngwritel(0, NGENE_INT_ENABLE);
@@ -1346,7 +1346,7 @@ static int ngene_start(struct ngene *dev)
init_waitqueue_head(>cmd_wq);
init_waitqueue_head(>tx_wq);
init_waitqueue_head(>rx_wq);
-   sema_init(>cmd_mutex, 1);
+   mutex_init(>cmd_mutex);
sema_init(>stream_mutex, 1);
sema_init(>pll_mutex, 1);
sema_init(>i2c_switch_mutex, 1);
@@ -1606,10 +1606,10 @@ static void ngene_unlink(struct ngene *dev)
com.in_len = 3;
com.out_len = 1;
 
-   down(>cmd_mutex);
+   mutex_lock(>cmd_mutex);
ngwritel(0, NGENE_INT_ENABLE);
ngene_command_mutex(dev, );
-   up(>cmd_mutex);
+   mutex_unlock(>cmd_mutex);
 }
 
 void ngene_shutdown(struct pci_dev *pdev)
diff --git a/drivers/media/pci/ngene/ngene.h b/drivers/media/pci/ngene/ngene.h
index 10d8f74..e600b70 100644
--- a/drivers/media/pci/ngene/ngene.h
+++ b/drivers/media/pci/ngene/ngene.h
@@ -762,7 +762,7 @@ struct ngene {
 
wait_queue_head_t cmd_wq;
int   cmd_done;
-   struct semaphore  cmd_mutex;
+   struct mutex  cmd_mutex;
struct semaphore  stream_mutex;
struct semaphore  pll_mutex;
struct semaphore  i2c_switch_mutex;
-- 
Binoy Jayan

[PATCH 2/3] media: ngene: Replace semaphore stream_mutex with mutex

[Binoy Jayan]

The semaphore 'stream_mutex' is used as a simple mutex, so
it should be written as one. Semaphores are going away in the future.

Signed-off-by: Binoy Jayan <binoy.ja...@linaro.org>
---
 drivers/media/pci/ngene/ngene-core.c | 14 +++---
 drivers/media/pci/ngene/ngene.h  |  2 +-
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/media/pci/ngene/ngene-core.c 
b/drivers/media/pci/ngene/ngene-core.c
index dfbd1e0..59f2e5f 100644
--- a/drivers/media/pci/ngene/ngene-core.c
+++ b/drivers/media/pci/ngene/ngene-core.c
@@ -560,7 +560,7 @@ static int ngene_command_stream_control(struct ngene *dev, 
u8 stream,
u16 BsSPI = ((stream & 1) ? 0x9800 : 0x9700);
u16 BsSDO = 0x9B00;
 
-   down(>stream_mutex);
+   mutex_lock(>stream_mutex);
memset(, 0, sizeof(com));
com.cmd.hdr.Opcode = CMD_CONTROL;
com.cmd.hdr.Length = sizeof(struct FW_STREAM_CONTROL) - 2;
@@ -587,16 +587,16 @@ static int ngene_command_stream_control(struct ngene 
*dev, u8 stream,
chan->HWState = HWSTATE_STOP;
spin_unlock_irq(>state_lock);
if (ngene_command(dev, ) < 0) {
-   up(>stream_mutex);
+   mutex_unlock(>stream_mutex);
return -1;
}
/* clear_buffers(chan); */
flush_buffers(chan);
-   up(>stream_mutex);
+   mutex_unlock(>stream_mutex);
return 0;
}
spin_unlock_irq(>state_lock);
-   up(>stream_mutex);
+   mutex_unlock(>stream_mutex);
return 0;
}
 
@@ -693,10 +693,10 @@ static int ngene_command_stream_control(struct ngene 
*dev, u8 stream,
spin_unlock_irq(>state_lock);
 
if (ngene_command(dev, ) < 0) {
-   up(>stream_mutex);
+   mutex_unlock(>stream_mutex);
return -1;
}
-   up(>stream_mutex);
+   mutex_unlock(>stream_mutex);
return 0;
 }
 
@@ -1347,7 +1347,7 @@ static int ngene_start(struct ngene *dev)
init_waitqueue_head(>tx_wq);
init_waitqueue_head(>rx_wq);
mutex_init(>cmd_mutex);
-   sema_init(>stream_mutex, 1);
+   mutex_init(>stream_mutex);
sema_init(>pll_mutex, 1);
sema_init(>i2c_switch_mutex, 1);
spin_lock_init(>cmd_lock);
diff --git a/drivers/media/pci/ngene/ngene.h b/drivers/media/pci/ngene/ngene.h
index e600b70..0dd15d6 100644
--- a/drivers/media/pci/ngene/ngene.h
+++ b/drivers/media/pci/ngene/ngene.h
@@ -763,7 +763,7 @@ struct ngene {
wait_queue_head_t cmd_wq;
int   cmd_done;
struct mutex  cmd_mutex;
-   struct semaphore  stream_mutex;
+   struct mutex  stream_mutex;
struct semaphore  pll_mutex;
struct semaphore  i2c_switch_mutex;
int   i2c_current_channel;
-- 
Binoy Jayan

[PATCH 0/3] ngene: Replace semaphores with mutexes

[Binoy Jayan]

These are a set of patches which removes semaphores from ngene.
These are part of a bigger effort to eliminate unwanted semaphores
from the linux kernel.

Binoy Jayan (3):
  media: ngene: Replace semaphore cmd_mutex with mutex
  media: ngene: Replace semaphore stream_mutex with mutex
  media: ngene: Replace semaphore i2c_switch_mutex with mutex

 drivers/media/pci/ngene/ngene-core.c | 28 ++--
 drivers/media/pci/ngene/ngene-i2c.c  |  6 +++---
 drivers/media/pci/ngene/ngene.h  |  6 +++---
 3 files changed, 20 insertions(+), 20 deletions(-)

-- 
Binoy Jayan

[PATCH 2/3] media: ngene: Replace semaphore stream_mutex with mutex

[Binoy Jayan]

The semaphore 'stream_mutex' is used as a simple mutex, so
it should be written as one. Semaphores are going away in the future.

Signed-off-by: Binoy Jayan 
---
 drivers/media/pci/ngene/ngene-core.c | 14 +++---
 drivers/media/pci/ngene/ngene.h  |  2 +-
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/media/pci/ngene/ngene-core.c 
b/drivers/media/pci/ngene/ngene-core.c
index dfbd1e0..59f2e5f 100644
--- a/drivers/media/pci/ngene/ngene-core.c
+++ b/drivers/media/pci/ngene/ngene-core.c
@@ -560,7 +560,7 @@ static int ngene_command_stream_control(struct ngene *dev, 
u8 stream,
u16 BsSPI = ((stream & 1) ? 0x9800 : 0x9700);
u16 BsSDO = 0x9B00;
 
-   down(>stream_mutex);
+   mutex_lock(>stream_mutex);
memset(, 0, sizeof(com));
com.cmd.hdr.Opcode = CMD_CONTROL;
com.cmd.hdr.Length = sizeof(struct FW_STREAM_CONTROL) - 2;
@@ -587,16 +587,16 @@ static int ngene_command_stream_control(struct ngene 
*dev, u8 stream,
chan->HWState = HWSTATE_STOP;
spin_unlock_irq(>state_lock);
if (ngene_command(dev, ) < 0) {
-   up(>stream_mutex);
+   mutex_unlock(>stream_mutex);
return -1;
}
/* clear_buffers(chan); */
flush_buffers(chan);
-   up(>stream_mutex);
+   mutex_unlock(>stream_mutex);
return 0;
}
spin_unlock_irq(>state_lock);
-   up(>stream_mutex);
+   mutex_unlock(>stream_mutex);
return 0;
}
 
@@ -693,10 +693,10 @@ static int ngene_command_stream_control(struct ngene 
*dev, u8 stream,
spin_unlock_irq(>state_lock);
 
if (ngene_command(dev, ) < 0) {
-   up(>stream_mutex);
+   mutex_unlock(>stream_mutex);
return -1;
}
-   up(>stream_mutex);
+   mutex_unlock(>stream_mutex);
return 0;
 }
 
@@ -1347,7 +1347,7 @@ static int ngene_start(struct ngene *dev)
init_waitqueue_head(>tx_wq);
init_waitqueue_head(>rx_wq);
mutex_init(>cmd_mutex);
-   sema_init(>stream_mutex, 1);
+   mutex_init(>stream_mutex);
sema_init(>pll_mutex, 1);
sema_init(>i2c_switch_mutex, 1);
spin_lock_init(>cmd_lock);
diff --git a/drivers/media/pci/ngene/ngene.h b/drivers/media/pci/ngene/ngene.h
index e600b70..0dd15d6 100644
--- a/drivers/media/pci/ngene/ngene.h
+++ b/drivers/media/pci/ngene/ngene.h
@@ -763,7 +763,7 @@ struct ngene {
wait_queue_head_t cmd_wq;
int   cmd_done;
struct mutex  cmd_mutex;
-   struct semaphore  stream_mutex;
+   struct mutex  stream_mutex;
struct semaphore  pll_mutex;
struct semaphore      i2c_switch_mutex;
int   i2c_current_channel;
-- 
Binoy Jayan

[PATCH 0/3] ngene: Replace semaphores with mutexes

[Binoy Jayan]

These are a set of patches which removes semaphores from ngene.
These are part of a bigger effort to eliminate unwanted semaphores
from the linux kernel.

Binoy Jayan (3):
  media: ngene: Replace semaphore cmd_mutex with mutex
  media: ngene: Replace semaphore stream_mutex with mutex
  media: ngene: Replace semaphore i2c_switch_mutex with mutex

 drivers/media/pci/ngene/ngene-core.c | 28 ++--
 drivers/media/pci/ngene/ngene-i2c.c  |  6 +++---
 drivers/media/pci/ngene/ngene.h  |  6 +++---
 3 files changed, 20 insertions(+), 20 deletions(-)

-- 
Binoy Jayan

[PATCH] net: ethernet: micrel: ksz884x: Replace semaphore proc_sem with mutex

[Binoy Jayan]

The semaphore 'proc_sem' is used as a simple mutex, so
it should be written as one. Semaphores are going away in the future.

Signed-off-by: Binoy Jayan <binoy.ja...@linaro.org>
---

This patch is part of a bigger effort to eliminate unwanted
semaphores from the linux kernel.

 drivers/net/ethernet/micrel/ksz884x.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/micrel/ksz884x.c 
b/drivers/net/ethernet/micrel/ksz884x.c
index ee1c78a..9664666 100644
--- a/drivers/net/ethernet/micrel/ksz884x.c
+++ b/drivers/net/ethernet/micrel/ksz884x.c
@@ -1456,7 +1456,7 @@ struct dev_info {
  * @adapter:   Adapter device information.
  * @port:  Port information.
  * @monitor_time_info: Timer to monitor ports.
- * @proc_sem:  Semaphore for proc accessing.
+ * @proc_mutex:Mutex for proc accessing.
  * @id:Device ID.
  * @mii_if:MII interface information.
  * @advertising:   Temporary variable to store advertised settings.
@@ -1470,7 +1470,7 @@ struct dev_priv {
struct ksz_port port;
struct ksz_timer_info monitor_timer_info;
 
-   struct semaphore proc_sem;
+   struct mutex proc_mutex;
int id;
 
struct mii_if_info mii_if;
@@ -5842,7 +5842,7 @@ static int netdev_ioctl(struct net_device *dev, struct 
ifreq *ifr, int cmd)
int result = 0;
struct mii_ioctl_data *data = if_mii(ifr);
 
-   if (down_interruptible(>proc_sem))
+   if (mutex_lock_interruptible(>proc_mutex))
return -ERESTARTSYS;
 
switch (cmd) {
@@ -5876,7 +5876,7 @@ static int netdev_ioctl(struct net_device *dev, struct 
ifreq *ifr, int cmd)
result = -EOPNOTSUPP;
}
 
-   up(>proc_sem);
+   mutex_unlock(>proc_mutex);
 
return result;
 }
@@ -6805,7 +6805,7 @@ static int __init netdev_init(struct net_device *dev)
 
dev->features |= dev->hw_features;
 
-   sema_init(>proc_sem, 1);
+   mutex_init(>proc_mutex);
 
priv->mii_if.phy_id_mask = 0x1;
priv->mii_if.reg_num_mask = 0x7;
-- 
Binoy Jayan

[PATCH] net: ethernet: micrel: ksz884x: Replace semaphore proc_sem with mutex

[Binoy Jayan]

The semaphore 'proc_sem' is used as a simple mutex, so
it should be written as one. Semaphores are going away in the future.

Signed-off-by: Binoy Jayan 
---

This patch is part of a bigger effort to eliminate unwanted
semaphores from the linux kernel.

 drivers/net/ethernet/micrel/ksz884x.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/micrel/ksz884x.c 
b/drivers/net/ethernet/micrel/ksz884x.c
index ee1c78a..9664666 100644
--- a/drivers/net/ethernet/micrel/ksz884x.c
+++ b/drivers/net/ethernet/micrel/ksz884x.c
@@ -1456,7 +1456,7 @@ struct dev_info {
  * @adapter:   Adapter device information.
  * @port:  Port information.
  * @monitor_time_info: Timer to monitor ports.
- * @proc_sem:  Semaphore for proc accessing.
+ * @proc_mutex:Mutex for proc accessing.
  * @id:Device ID.
  * @mii_if:MII interface information.
  * @advertising:   Temporary variable to store advertised settings.
@@ -1470,7 +1470,7 @@ struct dev_priv {
struct ksz_port port;
struct ksz_timer_info monitor_timer_info;
 
-   struct semaphore proc_sem;
+   struct mutex proc_mutex;
int id;
 
struct mii_if_info mii_if;
@@ -5842,7 +5842,7 @@ static int netdev_ioctl(struct net_device *dev, struct 
ifreq *ifr, int cmd)
int result = 0;
struct mii_ioctl_data *data = if_mii(ifr);
 
-   if (down_interruptible(>proc_sem))
+   if (mutex_lock_interruptible(>proc_mutex))
return -ERESTARTSYS;
 
switch (cmd) {
@@ -5876,7 +5876,7 @@ static int netdev_ioctl(struct net_device *dev, struct 
ifreq *ifr, int cmd)
result = -EOPNOTSUPP;
}
 
-   up(>proc_sem);
+   mutex_unlock(>proc_mutex);
 
return result;
 }
@@ -6805,7 +6805,7 @@ static int __init netdev_init(struct net_device *dev)
 
dev->features |= dev->hw_features;
 
-   sema_init(>proc_sem, 1);
+   mutex_init(>proc_mutex);
 
priv->mii_if.phy_id_mask = 0x1;
priv->mii_if.reg_num_mask = 0x7;
-- 
Binoy Jayan

[PATCH] mwifiex: Replace semaphore async_sem with mutex

[Binoy Jayan]

The semaphore 'async_sem' is used as a simple mutex, so
it should be written as one. Semaphores are going away in the future.

Signed-off-by: Binoy Jayan <binoy.ja...@linaro.org>
---

This patch is part of a bigger effort to eliminate unwanted
semaphores from the linux kernel.

 drivers/net/wireless/marvell/mwifiex/cfg80211.c | 2 +-
 drivers/net/wireless/marvell/mwifiex/main.h | 2 +-
 drivers/net/wireless/marvell/mwifiex/scan.c | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c 
b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index 7ec06bf..9e0d638 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -3059,7 +3059,7 @@ struct wireless_dev *mwifiex_add_virtual_intf(struct 
wiphy *wiphy,
INIT_DELAYED_WORK(>dfs_chan_sw_work,
  mwifiex_dfs_chan_sw_work_queue);
 
-   sema_init(>async_sem, 1);
+   mutex_init(>async_mutex);
 
mwifiex_dbg(adapter, INFO,
"info: %s: Marvell 802.11 Adapter\n", dev->name);
diff --git a/drivers/net/wireless/marvell/mwifiex/main.h 
b/drivers/net/wireless/marvell/mwifiex/main.h
index bb2a467..9c2cb33 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.h
+++ b/drivers/net/wireless/marvell/mwifiex/main.h
@@ -628,7 +628,7 @@ struct mwifiex_private {
struct dentry *dfs_dev_dir;
 #endif
u16 current_key_index;
-   struct semaphore async_sem;
+   struct mutex async_mutex;
struct cfg80211_scan_request *scan_request;
u8 cfg_bssid[6];
struct wps wps;
diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c 
b/drivers/net/wireless/marvell/mwifiex/scan.c
index ce6936d..ae9630b 100644
--- a/drivers/net/wireless/marvell/mwifiex/scan.c
+++ b/drivers/net/wireless/marvell/mwifiex/scan.c
@@ -2809,7 +2809,7 @@ int mwifiex_request_scan(struct mwifiex_private *priv,
 {
int ret;
 
-   if (down_interruptible(>async_sem)) {
+   if (mutex_lock_interruptible(>async_mutex)) {
mwifiex_dbg(priv->adapter, ERROR,
"%s: acquire semaphore fail\n",
__func__);
@@ -2825,7 +2825,7 @@ int mwifiex_request_scan(struct mwifiex_private *priv,
/* Normal scan */
ret = mwifiex_scan_networks(priv, NULL);
 
-   up(>async_sem);
+   mutex_unlock(>async_mutex);
 
return ret;
 }
-- 
Binoy Jayan

[PATCH] mwifiex: Replace semaphore async_sem with mutex

[Binoy Jayan]

The semaphore 'async_sem' is used as a simple mutex, so
it should be written as one. Semaphores are going away in the future.

Signed-off-by: Binoy Jayan 
---

This patch is part of a bigger effort to eliminate unwanted
semaphores from the linux kernel.

 drivers/net/wireless/marvell/mwifiex/cfg80211.c | 2 +-
 drivers/net/wireless/marvell/mwifiex/main.h | 2 +-
 drivers/net/wireless/marvell/mwifiex/scan.c | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c 
b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index 7ec06bf..9e0d638 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -3059,7 +3059,7 @@ struct wireless_dev *mwifiex_add_virtual_intf(struct 
wiphy *wiphy,
INIT_DELAYED_WORK(>dfs_chan_sw_work,
  mwifiex_dfs_chan_sw_work_queue);
 
-   sema_init(>async_sem, 1);
+   mutex_init(>async_mutex);
 
mwifiex_dbg(adapter, INFO,
"info: %s: Marvell 802.11 Adapter\n", dev->name);
diff --git a/drivers/net/wireless/marvell/mwifiex/main.h 
b/drivers/net/wireless/marvell/mwifiex/main.h
index bb2a467..9c2cb33 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.h
+++ b/drivers/net/wireless/marvell/mwifiex/main.h
@@ -628,7 +628,7 @@ struct mwifiex_private {
struct dentry *dfs_dev_dir;
 #endif
u16 current_key_index;
-   struct semaphore async_sem;
+   struct mutex async_mutex;
struct cfg80211_scan_request *scan_request;
u8 cfg_bssid[6];
struct wps wps;
diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c 
b/drivers/net/wireless/marvell/mwifiex/scan.c
index ce6936d..ae9630b 100644
--- a/drivers/net/wireless/marvell/mwifiex/scan.c
+++ b/drivers/net/wireless/marvell/mwifiex/scan.c
@@ -2809,7 +2809,7 @@ int mwifiex_request_scan(struct mwifiex_private *priv,
 {
int ret;
 
-   if (down_interruptible(>async_sem)) {
+   if (mutex_lock_interruptible(>async_mutex)) {
mwifiex_dbg(priv->adapter, ERROR,
"%s: acquire semaphore fail\n",
__func__);
@@ -2825,7 +2825,7 @@ int mwifiex_request_scan(struct mwifiex_private *priv,
/* Normal scan */
ret = mwifiex_scan_networks(priv, NULL);
 
-   up(>async_sem);
+   mutex_unlock(>async_mutex);
 
return ret;
 }
-- 
Binoy Jayan

[PATCH] HID: Replace semaphore driver_lock with mutex

[Binoy Jayan]

The semaphore 'driver_lock' is used as a simple mutex, so
it should be written as one. Semaphores are going away in the future.

Signed-off-by: Binoy Jayan <binoy.ja...@linaro.org>
---

This patch is part of a bigger effort to eliminate unwanted
semaphores from the linux kernel.

 drivers/hid/hid-core.c | 10 +-
 include/linux/hid.h|  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 37084b6..bf21dac 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -,7 +,7 @@ static int hid_device_probe(struct device *dev)
const struct hid_device_id *id;
int ret = 0;
 
-   if (down_interruptible(>driver_lock))
+   if (mutex_lock_interruptible(>driver_lock))
return -EINTR;
if (down_interruptible(>driver_input_lock)) {
ret = -EINTR;
@@ -2254,7 +2254,7 @@ static int hid_device_probe(struct device *dev)
if (!hdev->io_started)
up(>driver_input_lock);
 unlock_driver_lock:
-   up(>driver_lock);
+   mutex_unlock(>driver_lock);
return ret;
 }
 
@@ -2264,7 +2264,7 @@ static int hid_device_remove(struct device *dev)
struct hid_driver *hdrv;
int ret = 0;
 
-   if (down_interruptible(>driver_lock))
+   if (mutex_lock_interruptible(>driver_lock))
return -EINTR;
if (down_interruptible(>driver_input_lock)) {
ret = -EINTR;
@@ -2285,7 +2285,7 @@ static int hid_device_remove(struct device *dev)
if (!hdev->io_started)
up(>driver_input_lock);
 unlock_driver_lock:
-   up(>driver_lock);
+   mutex_unlock(>driver_lock);
return ret;
 }
 
@@ -2742,7 +2742,7 @@ struct hid_device *hid_allocate_device(void)
init_waitqueue_head(>debug_wait);
INIT_LIST_HEAD(>debug_list);
spin_lock_init(>debug_list_lock);
-   sema_init(>driver_lock, 1);
+   mutex_init(>driver_lock);
sema_init(>driver_input_lock, 1);
 
return hdev;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 5be325d..1add2b3 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -516,7 +516,7 @@ struct hid_device { 
/* device report descriptor */
struct hid_report_enum report_enum[HID_REPORT_TYPES];
struct work_struct led_work;/* 
delayed LED worker */
 
-   struct semaphore driver_lock;   /* 
protects the current driver, except during input */
+   struct mutex driver_lock;   /* 
protects the current driver, except during input */
struct semaphore driver_input_lock; /* 
protects the current driver */
struct device dev;      /* 
device */
struct hid_driver *driver;
-- 
Binoy Jayan

[PATCH] HID: Replace semaphore driver_lock with mutex

[Binoy Jayan]

The semaphore 'driver_lock' is used as a simple mutex, so
it should be written as one. Semaphores are going away in the future.

Signed-off-by: Binoy Jayan 
---

This patch is part of a bigger effort to eliminate unwanted
semaphores from the linux kernel.

 drivers/hid/hid-core.c | 10 +-
 include/linux/hid.h|  2 +-
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index 37084b6..bf21dac 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -,7 +,7 @@ static int hid_device_probe(struct device *dev)
const struct hid_device_id *id;
int ret = 0;
 
-   if (down_interruptible(>driver_lock))
+   if (mutex_lock_interruptible(>driver_lock))
return -EINTR;
if (down_interruptible(>driver_input_lock)) {
ret = -EINTR;
@@ -2254,7 +2254,7 @@ static int hid_device_probe(struct device *dev)
if (!hdev->io_started)
up(>driver_input_lock);
 unlock_driver_lock:
-   up(>driver_lock);
+   mutex_unlock(>driver_lock);
return ret;
 }
 
@@ -2264,7 +2264,7 @@ static int hid_device_remove(struct device *dev)
struct hid_driver *hdrv;
int ret = 0;
 
-   if (down_interruptible(>driver_lock))
+   if (mutex_lock_interruptible(>driver_lock))
return -EINTR;
if (down_interruptible(>driver_input_lock)) {
ret = -EINTR;
@@ -2285,7 +2285,7 @@ static int hid_device_remove(struct device *dev)
if (!hdev->io_started)
up(>driver_input_lock);
 unlock_driver_lock:
-   up(>driver_lock);
+   mutex_unlock(>driver_lock);
return ret;
 }
 
@@ -2742,7 +2742,7 @@ struct hid_device *hid_allocate_device(void)
init_waitqueue_head(>debug_wait);
INIT_LIST_HEAD(>debug_list);
spin_lock_init(>debug_list_lock);
-   sema_init(>driver_lock, 1);
+   mutex_init(>driver_lock);
sema_init(>driver_input_lock, 1);
 
return hdev;
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 5be325d..1add2b3 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -516,7 +516,7 @@ struct hid_device { 
/* device report descriptor */
struct hid_report_enum report_enum[HID_REPORT_TYPES];
struct work_struct led_work;/* 
delayed LED worker */
 
-   struct semaphore driver_lock;   /* 
protects the current driver, except during input */
+   struct mutex driver_lock;   /* 
protects the current driver, except during input */
struct semaphore driver_input_lock; /* 
protects the current driver */
struct device dev;      /* 
device */
struct hid_driver *driver;
-- 
Binoy Jayan

[ACTIVITY] (Binoy Jayan) 2017-05-08 to 2017-05-15

[Binoy Jayan]

=== Binoy Jayan ===

=== Highlights ===

dm-crypt optimization
 - No progress

dm-crypt:store keys in secure storage
 - Read OPTEE ClientAPI specification to figure out how to
   communicate with optee from a client application
 - No examples available for using client api from kernel
   so need to figure that out using the offline patch
   
https://github.com/linaro-swg/linux/commit/860c46087c99c24073cc722b12c0017bb0ce0a79
 - Suspending work until key framework is available
   instead of using the secure storage API

=== Plans ===

y2038: security/keys
  - Investigate y2038 issues identified in the keys subsystem (security/keys)

=== Miscellaneous ===

 - OOO 24th May - 31st May

[ACTIVITY] (Binoy Jayan) 2017-05-08 to 2017-05-15

[Binoy Jayan]

=== Binoy Jayan ===

=== Highlights ===

dm-crypt optimization
 - No progress

dm-crypt:store keys in secure storage
 - Read OPTEE ClientAPI specification to figure out how to
   communicate with optee from a client application
 - No examples available for using client api from kernel
   so need to figure that out using the offline patch
   
https://github.com/linaro-swg/linux/commit/860c46087c99c24073cc722b12c0017bb0ce0a79
 - Suspending work until key framework is available
   instead of using the secure storage API

=== Plans ===

y2038: security/keys
  - Investigate y2038 issues identified in the keys subsystem (security/keys)

=== Miscellaneous ===

 - OOO 24th May - 31st May

Re: [RFC PATCH v5] IV Generation algorithms for dm-crypt

[Binoy Jayan]

Hi Milan,

On 10 April 2017 at 19:30, Milan Broz  wrote:

Thank you for the reply.

> Well, it is good that there is no performance degradation but it
> would be nice to have some user of it that proves it is really
> working for your hw.

I have been able to get access to a hardware with IV generation support
a few days back. The hardware I was having before did not have IV
generation support. Will be able to come up with numbers after making
it work with the new one.

> FYI - with patch that increases dmcrypt sector size to 4k
> I can see improvement in speed usually in 5-15% with sync AES-NI
> (depends on access pattern), with dmcrypt mapped to memory
> it is even close to 20% speed up (but such a configuration is
> completely artificial).
>
> I wonder why increased dmcrypt sector size does not work for your hw,
> it should help as well (and can be combiuned later with this IV approach).
> (For native 4k drives this should be used in future anyway...)

I think it should work well too with backward incompatibility.

Thanks,
Binoy

Re: [RFC PATCH v5] IV Generation algorithms for dm-crypt

[Binoy Jayan]

Hi Milan,

On 10 April 2017 at 19:30, Milan Broz  wrote:

Thank you for the reply.

> Well, it is good that there is no performance degradation but it
> would be nice to have some user of it that proves it is really
> working for your hw.

I have been able to get access to a hardware with IV generation support
a few days back. The hardware I was having before did not have IV
generation support. Will be able to come up with numbers after making
it work with the new one.

> FYI - with patch that increases dmcrypt sector size to 4k
> I can see improvement in speed usually in 5-15% with sync AES-NI
> (depends on access pattern), with dmcrypt mapped to memory
> it is even close to 20% speed up (but such a configuration is
> completely artificial).
>
> I wonder why increased dmcrypt sector size does not work for your hw,
> it should help as well (and can be combiuned later with this IV approach).
> (For native 4k drives this should be used in future anyway...)

I think it should work well too with backward incompatibility.

Thanks,
Binoy

[RFC PATCH v5] crypto: Add IV generation algorithms

[Binoy Jayan]

Currently, the iv generation algorithms are implemented in dm-crypt.c.
The goal is to move these algorithms from the dm layer to the kernel
crypto layer by implementing them as template ciphers so they can be
implemented in hardware for performance. As part of this patchset, the
iv-generation code is moved from the dm layer to the crypto layer and
adapt the dm-layer to send a whole 'bio' (as defined in the block layer)
at a time. Each bio contains an in memory representation of physically
contiguous disk blocks. The dm layer sets up a chained scatterlist of
these blocks split into physically contiguous segments in memory so that
DMA can be performed. Also, the key management code is moved from dm layer
to the cryto layer since the key selection for encrypting neighboring
sectors depend on the keycount.

Synchronous crypto requests to encrypt/decrypt a sector are processed
sequentially. Asynchronous requests if processed in parallel, are freed
in the async callback. The dm layer allocates space for iv. The hardware
implementations can choose to make use of this space to generate their IVs
sequentially or allocate it on their own.
Interface to the crypto layer - include/crypto/geniv.h

Signed-off-by: Binoy Jayan <binoy.ja...@linaro.org>
---
 drivers/md/dm-crypt.c  | 1916 ++--
 include/crypto/geniv.h |   47 ++
 2 files changed, 1424 insertions(+), 539 deletions(-)
 create mode 100644 include/crypto/geniv.h

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 389a363..ce2bb80 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -32,170 +32,113 @@
 #include 
 #include 
 #include 
-
 #include 
-
-#define DM_MSG_PREFIX "crypt"
-
-/*
- * context holding the current state of a multi-part conversion
- */
-struct convert_context {
-   struct completion restart;
-   struct bio *bio_in;
-   struct bio *bio_out;
-   struct bvec_iter iter_in;
-   struct bvec_iter iter_out;
-   sector_t cc_sector;
-   atomic_t cc_pending;
-   struct skcipher_request *req;
+#include 
+#include 
+#include 
+#include 
+
+#define DM_MSG_PREFIX  "crypt"
+#define MAX_SG_LIST(BIO_MAX_PAGES * 8)
+#define MIN_IOS64
+#define LMK_SEED_SIZE  64 /* hash + 0 */
+#define TCW_WHITENING_SIZE 16
+
+struct geniv_ctx;
+struct geniv_req_ctx;
+
+/* Sub request for each of the skcipher_request's for a segment */
+struct geniv_subreq {
+   struct scatterlist src;
+   struct scatterlist dst;
+   struct geniv_req_ctx *rctx;
+   struct skcipher_request req CRYPTO_MINALIGN_ATTR;
 };
 
-/*
- * per bio private data
- */
-struct dm_crypt_io {
-   struct crypt_config *cc;
-   struct bio *base_bio;
-   struct work_struct work;
-
-   struct convert_context ctx;
-
-   atomic_t io_pending;
-   int error;
-   sector_t sector;
-
-   struct rb_node rb_node;
-} CRYPTO_MINALIGN_ATTR;
-
-struct dm_crypt_request {
-   struct convert_context *ctx;
-   struct scatterlist sg_in;
-   struct scatterlist sg_out;
+struct geniv_req_ctx {
+   struct geniv_subreq *subreq;
+   int is_write;
sector_t iv_sector;
+   unsigned int nents;
+   u8 *iv;
+   struct completion restart;
+   atomic_t req_pending;
+   struct skcipher_request *req;
 };
 
-struct crypt_config;
-
 struct crypt_iv_operations {
-   int (*ctr)(struct crypt_config *cc, struct dm_target *ti,
-  const char *opts);
-   void (*dtr)(struct crypt_config *cc);
-   int (*init)(struct crypt_config *cc);
-   int (*wipe)(struct crypt_config *cc);
-   int (*generator)(struct crypt_config *cc, u8 *iv,
-struct dm_crypt_request *dmreq);
-   int (*post)(struct crypt_config *cc, u8 *iv,
-   struct dm_crypt_request *dmreq);
+   int (*ctr)(struct geniv_ctx *ctx);
+   void (*dtr)(struct geniv_ctx *ctx);
+   int (*init)(struct geniv_ctx *ctx);
+   int (*wipe)(struct geniv_ctx *ctx);
+   int (*generator)(struct geniv_ctx *ctx,
+struct geniv_req_ctx *rctx,
+struct geniv_subreq *subreq);
+   int (*post)(struct geniv_ctx *ctx,
+   struct geniv_req_ctx *rctx,
+   struct geniv_subreq *subreq);
 };
 
-struct iv_essiv_private {
+struct geniv_essiv_private {
struct crypto_ahash *hash_tfm;
u8 *salt;
 };
 
-struct iv_benbi_private {
+struct geniv_benbi_private {
int shift;
 };
 
-#define LMK_SEED_SIZE 64 /* hash + 0 */
-struct iv_lmk_private {
+struct geniv_lmk_private {
struct crypto_shash *hash_tfm;
u8 *seed;
 };
 
-#define TCW_WHITENING_SIZE 16
-struct iv_tcw_private {
+struct geniv_tcw_private {
struct crypto_shash *crc32_tfm;
u8 *iv_seed;
u8 *whitening;
 };
 
-/*
- * Crypt: maps a linear range of a block device
- * and encrypts /

[RFC PATCH v5] crypto: Add IV generation algorithms

[Binoy Jayan]

Currently, the iv generation algorithms are implemented in dm-crypt.c.
The goal is to move these algorithms from the dm layer to the kernel
crypto layer by implementing them as template ciphers so they can be
implemented in hardware for performance. As part of this patchset, the
iv-generation code is moved from the dm layer to the crypto layer and
adapt the dm-layer to send a whole 'bio' (as defined in the block layer)
at a time. Each bio contains an in memory representation of physically
contiguous disk blocks. The dm layer sets up a chained scatterlist of
these blocks split into physically contiguous segments in memory so that
DMA can be performed. Also, the key management code is moved from dm layer
to the cryto layer since the key selection for encrypting neighboring
sectors depend on the keycount.

Synchronous crypto requests to encrypt/decrypt a sector are processed
sequentially. Asynchronous requests if processed in parallel, are freed
in the async callback. The dm layer allocates space for iv. The hardware
implementations can choose to make use of this space to generate their IVs
sequentially or allocate it on their own.
Interface to the crypto layer - include/crypto/geniv.h

Signed-off-by: Binoy Jayan 
---
 drivers/md/dm-crypt.c  | 1916 ++--
 include/crypto/geniv.h |   47 ++
 2 files changed, 1424 insertions(+), 539 deletions(-)
 create mode 100644 include/crypto/geniv.h

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 389a363..ce2bb80 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -32,170 +32,113 @@
 #include 
 #include 
 #include 
-
 #include 
-
-#define DM_MSG_PREFIX "crypt"
-
-/*
- * context holding the current state of a multi-part conversion
- */
-struct convert_context {
-   struct completion restart;
-   struct bio *bio_in;
-   struct bio *bio_out;
-   struct bvec_iter iter_in;
-   struct bvec_iter iter_out;
-   sector_t cc_sector;
-   atomic_t cc_pending;
-   struct skcipher_request *req;
+#include 
+#include 
+#include 
+#include 
+
+#define DM_MSG_PREFIX  "crypt"
+#define MAX_SG_LIST(BIO_MAX_PAGES * 8)
+#define MIN_IOS64
+#define LMK_SEED_SIZE  64 /* hash + 0 */
+#define TCW_WHITENING_SIZE 16
+
+struct geniv_ctx;
+struct geniv_req_ctx;
+
+/* Sub request for each of the skcipher_request's for a segment */
+struct geniv_subreq {
+   struct scatterlist src;
+   struct scatterlist dst;
+   struct geniv_req_ctx *rctx;
+   struct skcipher_request req CRYPTO_MINALIGN_ATTR;
 };
 
-/*
- * per bio private data
- */
-struct dm_crypt_io {
-   struct crypt_config *cc;
-   struct bio *base_bio;
-   struct work_struct work;
-
-   struct convert_context ctx;
-
-   atomic_t io_pending;
-   int error;
-   sector_t sector;
-
-   struct rb_node rb_node;
-} CRYPTO_MINALIGN_ATTR;
-
-struct dm_crypt_request {
-   struct convert_context *ctx;
-   struct scatterlist sg_in;
-   struct scatterlist sg_out;
+struct geniv_req_ctx {
+   struct geniv_subreq *subreq;
+   int is_write;
sector_t iv_sector;
+   unsigned int nents;
+   u8 *iv;
+   struct completion restart;
+   atomic_t req_pending;
+   struct skcipher_request *req;
 };
 
-struct crypt_config;
-
 struct crypt_iv_operations {
-   int (*ctr)(struct crypt_config *cc, struct dm_target *ti,
-  const char *opts);
-   void (*dtr)(struct crypt_config *cc);
-   int (*init)(struct crypt_config *cc);
-   int (*wipe)(struct crypt_config *cc);
-   int (*generator)(struct crypt_config *cc, u8 *iv,
-struct dm_crypt_request *dmreq);
-   int (*post)(struct crypt_config *cc, u8 *iv,
-   struct dm_crypt_request *dmreq);
+   int (*ctr)(struct geniv_ctx *ctx);
+   void (*dtr)(struct geniv_ctx *ctx);
+   int (*init)(struct geniv_ctx *ctx);
+   int (*wipe)(struct geniv_ctx *ctx);
+   int (*generator)(struct geniv_ctx *ctx,
+struct geniv_req_ctx *rctx,
+struct geniv_subreq *subreq);
+   int (*post)(struct geniv_ctx *ctx,
+   struct geniv_req_ctx *rctx,
+   struct geniv_subreq *subreq);
 };
 
-struct iv_essiv_private {
+struct geniv_essiv_private {
struct crypto_ahash *hash_tfm;
u8 *salt;
 };
 
-struct iv_benbi_private {
+struct geniv_benbi_private {
int shift;
 };
 
-#define LMK_SEED_SIZE 64 /* hash + 0 */
-struct iv_lmk_private {
+struct geniv_lmk_private {
struct crypto_shash *hash_tfm;
u8 *seed;
 };
 
-#define TCW_WHITENING_SIZE 16
-struct iv_tcw_private {
+struct geniv_tcw_private {
struct crypto_shash *crc32_tfm;
u8 *iv_seed;
u8 *whitening;
 };
 
-/*
- * Crypt: maps a linear range of a block device
- * and encrypts / decrypts at the same ti

[RFC PATCH v5] IV Generation algorithms for dm-crypt

[Binoy Jayan]

===
dm-crypt optimization for larger block sizes
===

Currently, the iv generation algorithms are implemented in dm-crypt.c. The goal
is to move these algorithms from the dm layer to the kernel crypto layer by
implementing them as template ciphers so they can be used in relation with
algorithms like aes, and with multiple modes like cbc, ecb etc. As part of this
patchset, the iv-generation code is moved from the dm layer to the crypto layer
and adapt the dm-layer to send a whole 'bio' (as defined in the block layer)
at a time. Each bio contains the in memory representation of physically
contiguous disk blocks. Since the bio itself may not be contiguous in main
memory, the dm layer sets up a chained scatterlist of these blocks split into
physically contiguous segments in memory so that DMA can be performed.

One challenge in doing so is that the IVs are generated based on a 512-byte
sector number. This infact limits the block sizes to 512 bytes. But this should
not be a problem if a hardware with iv generation support is used. The geniv
itself splits the segments into sectors so it could choose the IV based on
sector number. But it could be modelled in hardware effectively by not
splitting up the segments in the bio.

Another challenge faced is that dm-crypt has an option to use multiple keys.
The key selection is done based on the sector number. If the whole bio is
encrypted / decrypted with the same key, the encrypted volumes will not be
compatible with the original dm-crypt [without the changes]. So, the key
selection code is moved to crypto layer so the neighboring sectors are
encrypted with a different key.

The dm layer allocates space for iv. The hardware drivers can choose to make
use of this space to generate their IVs sequentially or allocate it on their
own. This can be moved to crypto layer too. Postponing this decision until
the requirement to integrate milan's changes are clear.

Interface to the crypto layer - include/crypto/geniv.h

More information on test procedure can be found in v1.

---
Peformance comparison [Tests on 1 GiB Volume] on db410c
Test script:
https://github.com/binoyjayan/utilities/blob/master/utils/dmtest
dmtest -d  -o out.log -s 1024 -r 384 -f 768
---

This includes tests done with dd, fio and bonnie++ with the original dm-crypt
and the proposed solution with algorithm 'essiv(cbc(aes-arm))' implemented
in software. The hardware is yet to be evaluated. These tests are to make sure
there is no drastic performance degradation on systems without hw crypto.

Tests with dd [direct i/o]

Sequential read-0.134 %
Sequential Write   +0.091 %

Tests with fio [Aggregate bandwidth - aggrb]

Random Read+0.358 %
Random Write   +0.010 %

Tests with bonnie++ [768 MiB File, 384 MiB Ram]
after mounting dm-crypt target as ext4

Sequential o/p [per-char]  -2.876 %
Sequential o/p [per-blk]   +0.992 %
Sequential o/p [re-write]  +4.465 %

Sequential i/p [per-char] -0.453 %
Sequential i/p [per-blk]  -0.740 %

Sequential create -0.255 %
Sequential delete +0.042 %
Random create -0.007 %
Random delete +0.454 %

NB: The '+' sign shows improvement and '-' shows degradation.
The tests were performed with minimal cpu load.
Tests with higher cpu load to be done

Revisions:
--

v1: https://patchwork.kernel.org/patch/9439175
v2: https://patchwork.kernel.org/patch/9471923
v3: https://lkml.org/lkml/2017/1/18/170
v4: https://patchwork.kernel.org/patch/9559665

v4 --> v5
--

1. Fix for the multiple instance issue in /proc/crypto
2. Few cosmetic changes including struct alignment
3. Simplified 'struct geniv_req_info'

v3 --> v4
--
Fix for the bug reported by Gilad Ben-Yossef.
The element '__ctx' in 'struct skcipher_request req' overflowed into the
element 'struct scatterlist src' which immediately follows 'req' in
'struct geniv_subreq' and corrupted src.

v2 --> v3
--

1. Moved iv algorithms in dm-crypt.c for control
2. Key management code moved from dm layer to cryto layer
   so that cipher instance selection can be made depending on key_index
3. The revision v2 had scatterlist nodes created for every sector in the bio.
   It is modified to create only once scatterlist node to reduce memory
   foot print. Synchronous requests are processed sequentially. Asynchronous
   requests are processed in parallel and is freed in the async callback.
4. Changed allocation for sub-requests using mempool

v1 --> v2
--

1. dm-crypt changes to process larger block sizes (one segment in a bio)
2. Incorporated changes w.r.t. comments from Herbert.

Binoy Jayan (1):
  crypto: Add I

[RFC PATCH v5] IV Generation algorithms for dm-crypt

[Binoy Jayan]

===
dm-crypt optimization for larger block sizes
===

Currently, the iv generation algorithms are implemented in dm-crypt.c. The goal
is to move these algorithms from the dm layer to the kernel crypto layer by
implementing them as template ciphers so they can be used in relation with
algorithms like aes, and with multiple modes like cbc, ecb etc. As part of this
patchset, the iv-generation code is moved from the dm layer to the crypto layer
and adapt the dm-layer to send a whole 'bio' (as defined in the block layer)
at a time. Each bio contains the in memory representation of physically
contiguous disk blocks. Since the bio itself may not be contiguous in main
memory, the dm layer sets up a chained scatterlist of these blocks split into
physically contiguous segments in memory so that DMA can be performed.

One challenge in doing so is that the IVs are generated based on a 512-byte
sector number. This infact limits the block sizes to 512 bytes. But this should
not be a problem if a hardware with iv generation support is used. The geniv
itself splits the segments into sectors so it could choose the IV based on
sector number. But it could be modelled in hardware effectively by not
splitting up the segments in the bio.

Another challenge faced is that dm-crypt has an option to use multiple keys.
The key selection is done based on the sector number. If the whole bio is
encrypted / decrypted with the same key, the encrypted volumes will not be
compatible with the original dm-crypt [without the changes]. So, the key
selection code is moved to crypto layer so the neighboring sectors are
encrypted with a different key.

The dm layer allocates space for iv. The hardware drivers can choose to make
use of this space to generate their IVs sequentially or allocate it on their
own. This can be moved to crypto layer too. Postponing this decision until
the requirement to integrate milan's changes are clear.

Interface to the crypto layer - include/crypto/geniv.h

More information on test procedure can be found in v1.

---
Peformance comparison [Tests on 1 GiB Volume] on db410c
Test script:
https://github.com/binoyjayan/utilities/blob/master/utils/dmtest
dmtest -d  -o out.log -s 1024 -r 384 -f 768
---

This includes tests done with dd, fio and bonnie++ with the original dm-crypt
and the proposed solution with algorithm 'essiv(cbc(aes-arm))' implemented
in software. The hardware is yet to be evaluated. These tests are to make sure
there is no drastic performance degradation on systems without hw crypto.

Tests with dd [direct i/o]

Sequential read-0.134 %
Sequential Write   +0.091 %

Tests with fio [Aggregate bandwidth - aggrb]

Random Read+0.358 %
Random Write   +0.010 %

Tests with bonnie++ [768 MiB File, 384 MiB Ram]
after mounting dm-crypt target as ext4

Sequential o/p [per-char]  -2.876 %
Sequential o/p [per-blk]   +0.992 %
Sequential o/p [re-write]  +4.465 %

Sequential i/p [per-char] -0.453 %
Sequential i/p [per-blk]  -0.740 %

Sequential create -0.255 %
Sequential delete +0.042 %
Random create -0.007 %
Random delete +0.454 %

NB: The '+' sign shows improvement and '-' shows degradation.
The tests were performed with minimal cpu load.
Tests with higher cpu load to be done

Revisions:
--

v1: https://patchwork.kernel.org/patch/9439175
v2: https://patchwork.kernel.org/patch/9471923
v3: https://lkml.org/lkml/2017/1/18/170
v4: https://patchwork.kernel.org/patch/9559665

v4 --> v5
--

1. Fix for the multiple instance issue in /proc/crypto
2. Few cosmetic changes including struct alignment
3. Simplified 'struct geniv_req_info'

v3 --> v4
--
Fix for the bug reported by Gilad Ben-Yossef.
The element '__ctx' in 'struct skcipher_request req' overflowed into the
element 'struct scatterlist src' which immediately follows 'req' in
'struct geniv_subreq' and corrupted src.

v2 --> v3
--

1. Moved iv algorithms in dm-crypt.c for control
2. Key management code moved from dm layer to cryto layer
   so that cipher instance selection can be made depending on key_index
3. The revision v2 had scatterlist nodes created for every sector in the bio.
   It is modified to create only once scatterlist node to reduce memory
   foot print. Synchronous requests are processed sequentially. Asynchronous
   requests are processed in parallel and is freed in the async callback.
4. Changed allocation for sub-requests using mempool

v1 --> v2
--

1. dm-crypt changes to process larger block sizes (one segment in a bio)
2. Incorporated changes w.r.t. comments from Herbert.

Binoy Jayan (1):
  crypto: Add I

Re: [RFC PATCH v4] IV Generation algorithms for dm-crypt

[Binoy Jayan]

On 6 March 2017 at 20:08, Gilad Ben-Yossef  wrote:
>
> I gave it a spin on a x86_64 with 8 CPUs with AES-NI using cryptd and
> on Arm  using CryptoCell hardware accelerator.
>
> There was no difference in performance between 512 and 4096 bytes
> cluster size on the x86_64 (800 MB loop file system)
>
> There was an improvement in latency of 3.2% between 512 and 4096 bytes
> cluster size on the Arm. I expect the performance benefits for this
> test for Binoy's patch to be the same.
>
> In both cases the very naive test was a simple dd with block size of
> 4096 bytes or the raw block device.
>
> I do not know what effect having a bigger cluster size would have on
> have on other more complex file system operations.
> Is there any specific benchmark worth testing with?


The multiple instances issue in /proc/crypto is fixed. It was because of
the IV code itself modifying the algorithm name inadvertently in the
global crypto algorithm lookup table when it was splitting up
"plain(cbc(aes))" into "plain" and "cbc(aes)" so as to invoke the child
algorithm.

I ran a few tests with dd, bonnie and FIO under Qemu - x86 using the
automated script [1] that I wrote to make the testing easy.
The tests were done on software implementations of the algorithms
as the real hardware was not available with me. According to the test,
I found that the sequential reads and writes have a good improvement
(5.7 %) in the data rate with the proposed solution while the random
reads shows a very little improvement. When tested with FIO, the
random writes also shows a small improvement (2.2%) but the random
reads show a little deterioration in performance (4 %).

When tested in arm hardware, only the sequential writes with bonnie
shows improvement (5.6%). All other tests shows degraded performance
in the absence of crypto hardware.

[1] https://github.com/binoyjayan/utilities/blob/master/utils/dmtest
Dependencies: dd [Full version], bonnie, fio

Thanks,
Binoy

Re: [RFC PATCH v4] IV Generation algorithms for dm-crypt

[Binoy Jayan]

On 6 March 2017 at 20:08, Gilad Ben-Yossef  wrote:
>
> I gave it a spin on a x86_64 with 8 CPUs with AES-NI using cryptd and
> on Arm  using CryptoCell hardware accelerator.
>
> There was no difference in performance between 512 and 4096 bytes
> cluster size on the x86_64 (800 MB loop file system)
>
> There was an improvement in latency of 3.2% between 512 and 4096 bytes
> cluster size on the Arm. I expect the performance benefits for this
> test for Binoy's patch to be the same.
>
> In both cases the very naive test was a simple dd with block size of
> 4096 bytes or the raw block device.
>
> I do not know what effect having a bigger cluster size would have on
> have on other more complex file system operations.
> Is there any specific benchmark worth testing with?


The multiple instances issue in /proc/crypto is fixed. It was because of
the IV code itself modifying the algorithm name inadvertently in the
global crypto algorithm lookup table when it was splitting up
"plain(cbc(aes))" into "plain" and "cbc(aes)" so as to invoke the child
algorithm.

I ran a few tests with dd, bonnie and FIO under Qemu - x86 using the
automated script [1] that I wrote to make the testing easy.
The tests were done on software implementations of the algorithms
as the real hardware was not available with me. According to the test,
I found that the sequential reads and writes have a good improvement
(5.7 %) in the data rate with the proposed solution while the random
reads shows a very little improvement. When tested with FIO, the
random writes also shows a small improvement (2.2%) but the random
reads show a little deterioration in performance (4 %).

When tested in arm hardware, only the sequential writes with bonnie
shows improvement (5.6%). All other tests shows degraded performance
in the absence of crypto hardware.

[1] https://github.com/binoyjayan/utilities/blob/master/utils/dmtest
Dependencies: dd [Full version], bonnie, fio

Thanks,
Binoy

Re: [RFC PATCH v4] IV Generation algorithms for dm-crypt

[Binoy Jayan]

Hi Herbert,

On 8 February 2017 at 13:02, Gilad Ben-Yossef <gi...@benyossef.com> wrote:
> On Tue, Feb 7, 2017 at 12:35 PM, Binoy Jayan <binoy.ja...@linaro.org> wrote:
>> ===
>> dm-crypt optimization for larger block sizes
>> ===
>>
>> Currently, the iv generation algorithms are implemented in dm-crypt.c. The 
>> goal
>> is to move these algorithms from the dm layer to the kernel crypto layer by
>> implementing them as template ciphers so they can be used in relation with
>> algorithms like aes, and with multiple modes like cbc, ecb etc. As part of 
>> this
>> patchset, the iv-generation code is moved from the dm layer to the crypto 
>> layer
>> and adapt the dm-layer to send a whole 'bio' (as defined in the block layer)
>> at a time. Each bio contains the in memory representation of physically
>> contiguous disk blocks. Since the bio itself may not be contiguous in main
>> memory, the dm layer sets up a chained scatterlist of these blocks split into
>> physically contiguous segments in memory so that DMA can be performed.

> Ran Bonnie++ on it last night  (Luks mode, plain64, Qemu Virt platform
> Arm64) and it works just fine.
>
> Tested-by: Gilad Ben-Yossef <gi...@benyossef.com>

I was wondering if this is near to be ready for submission (apart from
the testmgr.c
changes) or I need to make some changes to make it similar to the IPSec offload?

Thanks,
Binoy

Re: [RFC PATCH v4] IV Generation algorithms for dm-crypt

[Binoy Jayan]

Hi Herbert,

On 8 February 2017 at 13:02, Gilad Ben-Yossef  wrote:
> On Tue, Feb 7, 2017 at 12:35 PM, Binoy Jayan  wrote:
>> ===
>> dm-crypt optimization for larger block sizes
>> ===
>>
>> Currently, the iv generation algorithms are implemented in dm-crypt.c. The 
>> goal
>> is to move these algorithms from the dm layer to the kernel crypto layer by
>> implementing them as template ciphers so they can be used in relation with
>> algorithms like aes, and with multiple modes like cbc, ecb etc. As part of 
>> this
>> patchset, the iv-generation code is moved from the dm layer to the crypto 
>> layer
>> and adapt the dm-layer to send a whole 'bio' (as defined in the block layer)
>> at a time. Each bio contains the in memory representation of physically
>> contiguous disk blocks. Since the bio itself may not be contiguous in main
>> memory, the dm layer sets up a chained scatterlist of these blocks split into
>> physically contiguous segments in memory so that DMA can be performed.

> Ran Bonnie++ on it last night  (Luks mode, plain64, Qemu Virt platform
> Arm64) and it works just fine.
>
> Tested-by: Gilad Ben-Yossef 

I was wondering if this is near to be ready for submission (apart from
the testmgr.c
changes) or I need to make some changes to make it similar to the IPSec offload?

Thanks,
Binoy

Re: [RFC PATCH v4] IV Generation algorithms for dm-crypt

[Binoy Jayan]

On 8 February 2017 at 13:02, Gilad Ben-Yossef  wrote:
>
> Ran Bonnie++ on it last night  (Luks mode, plain64, Qemu Virt platform
> Arm64) and it works just fine.
>
> Tested-by: Gilad Ben-Yossef 
>

Hi Gilad,

Thank you for testing it. Do you have access to a device having crypto
hardware with IV generation capability and associated drivers for let
say, aes with cbc or any another mode? I was wondering if I can customize
it to work with dm-crypt by generating IVs automatically. Please let
me know your thoughts.

Thanks,
Binoy

Re: [RFC PATCH v4] IV Generation algorithms for dm-crypt

[Binoy Jayan]

On 8 February 2017 at 13:02, Gilad Ben-Yossef  wrote:
>
> Ran Bonnie++ on it last night  (Luks mode, plain64, Qemu Virt platform
> Arm64) and it works just fine.
>
> Tested-by: Gilad Ben-Yossef 
>

Hi Gilad,

Thank you for testing it. Do you have access to a device having crypto
hardware with IV generation capability and associated drivers for let
say, aes with cbc or any another mode? I was wondering if I can customize
it to work with dm-crypt by generating IVs automatically. Please let
me know your thoughts.

Thanks,
Binoy

[RFC PATCH v4] crypto: Add IV generation algorithms

[Binoy Jayan]

Currently, the iv generation algorithms are implemented in dm-crypt.c.
The goal is to move these algorithms from the dm layer to the kernel
crypto layer by implementing them as template ciphers so they can be
implemented in hardware for performance. As part of this patchset, the
iv-generation code is moved from the dm layer to the crypto layer and
adapt the dm-layer to send a whole 'bio' (as defined in the block layer)
at a time. Each bio contains an in memory representation of physically
contiguous disk blocks. The dm layer sets up a chained scatterlist of
these blocks split into physically contiguous segments in memory so that
DMA can be performed. Also, the key management code is moved from dm layer
to the cryto layer since the key selection for encrypting neighboring
sectors depend on the keycount.

Synchronous crypto requests to encrypt/decrypt a sector are processed
sequentially. Asynchronous requests if processed in parallel, are freed
in the async callback. The dm layer allocates space for iv. The hardware
implementations can choose to make use of this space to generate their IVs
sequentially or allocate it on their own.
Interface to the crypto layer - include/crypto/geniv.h

Signed-off-by: Binoy Jayan <binoy.ja...@linaro.org>
---
 drivers/md/dm-crypt.c  | 1894 ++--
 include/crypto/geniv.h |   47 ++
 2 files changed, 1402 insertions(+), 539 deletions(-)
 create mode 100644 include/crypto/geniv.h

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 7c6c572..8540c0f 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -32,170 +32,113 @@
 #include 
 #include 
 #include 
-
 #include 
-
-#define DM_MSG_PREFIX "crypt"
-
-/*
- * context holding the current state of a multi-part conversion
- */
-struct convert_context {
-   struct completion restart;
-   struct bio *bio_in;
-   struct bio *bio_out;
-   struct bvec_iter iter_in;
-   struct bvec_iter iter_out;
-   sector_t cc_sector;
-   atomic_t cc_pending;
-   struct skcipher_request *req;
+#include 
+#include 
+#include 
+#include 
+
+#define DM_MSG_PREFIX  "crypt"
+#define MAX_SG_LIST(BIO_MAX_PAGES * 8)
+#define MIN_IOS64
+#define LMK_SEED_SIZE  64 /* hash + 0 */
+#define TCW_WHITENING_SIZE 16
+
+struct geniv_ctx;
+struct geniv_req_ctx;
+
+/* Sub request for each of the skcipher_request's for a segment */
+struct geniv_subreq {
+   struct scatterlist src;
+   struct scatterlist dst;
+   int n;
+   struct geniv_req_ctx *rctx;
+   struct skcipher_request req CRYPTO_MINALIGN_ATTR;
 };
 
-/*
- * per bio private data
- */
-struct dm_crypt_io {
-   struct crypt_config *cc;
-   struct bio *base_bio;
-   struct work_struct work;
-
-   struct convert_context ctx;
-
-   atomic_t io_pending;
-   int error;
-   sector_t sector;
-
-   struct rb_node rb_node;
-} CRYPTO_MINALIGN_ATTR;
-
-struct dm_crypt_request {
-   struct convert_context *ctx;
-   struct scatterlist sg_in;
-   struct scatterlist sg_out;
+struct geniv_req_ctx {
+   struct geniv_subreq *subreq;
+   bool is_write;
sector_t iv_sector;
+   unsigned int nents;
+   u8 *iv;
+   struct completion restart;
+   atomic_t req_pending;
+   struct skcipher_request *req;
 };
 
-struct crypt_config;
-
 struct crypt_iv_operations {
-   int (*ctr)(struct crypt_config *cc, struct dm_target *ti,
-  const char *opts);
-   void (*dtr)(struct crypt_config *cc);
-   int (*init)(struct crypt_config *cc);
-   int (*wipe)(struct crypt_config *cc);
-   int (*generator)(struct crypt_config *cc, u8 *iv,
-struct dm_crypt_request *dmreq);
-   int (*post)(struct crypt_config *cc, u8 *iv,
-   struct dm_crypt_request *dmreq);
+   int (*ctr)(struct geniv_ctx *ctx);
+   void (*dtr)(struct geniv_ctx *ctx);
+   int (*init)(struct geniv_ctx *ctx);
+   int (*wipe)(struct geniv_ctx *ctx);
+   int (*generator)(struct geniv_ctx *ctx,
+struct geniv_req_ctx *rctx,
+struct geniv_subreq *subreq);
+   int (*post)(struct geniv_ctx *ctx,
+   struct geniv_req_ctx *rctx,
+   struct geniv_subreq *subreq);
 };
 
-struct iv_essiv_private {
+struct geniv_essiv_private {
struct crypto_ahash *hash_tfm;
u8 *salt;
 };
 
-struct iv_benbi_private {
+struct geniv_benbi_private {
int shift;
 };
 
-#define LMK_SEED_SIZE 64 /* hash + 0 */
-struct iv_lmk_private {
+struct geniv_lmk_private {
struct crypto_shash *hash_tfm;
u8 *seed;
 };
 
-#define TCW_WHITENING_SIZE 16
-struct iv_tcw_private {
+struct geniv_tcw_private {
struct crypto_shash *crc32_tfm;
u8 *iv_seed;
u8 *whitening;
 };
 
-/*
- * Crypt: maps a linear range of a block device
-

[RFC PATCH v4] crypto: Add IV generation algorithms

[Binoy Jayan]

Currently, the iv generation algorithms are implemented in dm-crypt.c.
The goal is to move these algorithms from the dm layer to the kernel
crypto layer by implementing them as template ciphers so they can be
implemented in hardware for performance. As part of this patchset, the
iv-generation code is moved from the dm layer to the crypto layer and
adapt the dm-layer to send a whole 'bio' (as defined in the block layer)
at a time. Each bio contains an in memory representation of physically
contiguous disk blocks. The dm layer sets up a chained scatterlist of
these blocks split into physically contiguous segments in memory so that
DMA can be performed. Also, the key management code is moved from dm layer
to the cryto layer since the key selection for encrypting neighboring
sectors depend on the keycount.

Synchronous crypto requests to encrypt/decrypt a sector are processed
sequentially. Asynchronous requests if processed in parallel, are freed
in the async callback. The dm layer allocates space for iv. The hardware
implementations can choose to make use of this space to generate their IVs
sequentially or allocate it on their own.
Interface to the crypto layer - include/crypto/geniv.h

Signed-off-by: Binoy Jayan 
---
 drivers/md/dm-crypt.c  | 1894 ++--
 include/crypto/geniv.h |   47 ++
 2 files changed, 1402 insertions(+), 539 deletions(-)
 create mode 100644 include/crypto/geniv.h

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 7c6c572..8540c0f 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -32,170 +32,113 @@
 #include 
 #include 
 #include 
-
 #include 
-
-#define DM_MSG_PREFIX "crypt"
-
-/*
- * context holding the current state of a multi-part conversion
- */
-struct convert_context {
-   struct completion restart;
-   struct bio *bio_in;
-   struct bio *bio_out;
-   struct bvec_iter iter_in;
-   struct bvec_iter iter_out;
-   sector_t cc_sector;
-   atomic_t cc_pending;
-   struct skcipher_request *req;
+#include 
+#include 
+#include 
+#include 
+
+#define DM_MSG_PREFIX  "crypt"
+#define MAX_SG_LIST(BIO_MAX_PAGES * 8)
+#define MIN_IOS64
+#define LMK_SEED_SIZE  64 /* hash + 0 */
+#define TCW_WHITENING_SIZE 16
+
+struct geniv_ctx;
+struct geniv_req_ctx;
+
+/* Sub request for each of the skcipher_request's for a segment */
+struct geniv_subreq {
+   struct scatterlist src;
+   struct scatterlist dst;
+   int n;
+   struct geniv_req_ctx *rctx;
+   struct skcipher_request req CRYPTO_MINALIGN_ATTR;
 };
 
-/*
- * per bio private data
- */
-struct dm_crypt_io {
-   struct crypt_config *cc;
-   struct bio *base_bio;
-   struct work_struct work;
-
-   struct convert_context ctx;
-
-   atomic_t io_pending;
-   int error;
-   sector_t sector;
-
-   struct rb_node rb_node;
-} CRYPTO_MINALIGN_ATTR;
-
-struct dm_crypt_request {
-   struct convert_context *ctx;
-   struct scatterlist sg_in;
-   struct scatterlist sg_out;
+struct geniv_req_ctx {
+   struct geniv_subreq *subreq;
+   bool is_write;
sector_t iv_sector;
+   unsigned int nents;
+   u8 *iv;
+   struct completion restart;
+   atomic_t req_pending;
+   struct skcipher_request *req;
 };
 
-struct crypt_config;
-
 struct crypt_iv_operations {
-   int (*ctr)(struct crypt_config *cc, struct dm_target *ti,
-  const char *opts);
-   void (*dtr)(struct crypt_config *cc);
-   int (*init)(struct crypt_config *cc);
-   int (*wipe)(struct crypt_config *cc);
-   int (*generator)(struct crypt_config *cc, u8 *iv,
-struct dm_crypt_request *dmreq);
-   int (*post)(struct crypt_config *cc, u8 *iv,
-   struct dm_crypt_request *dmreq);
+   int (*ctr)(struct geniv_ctx *ctx);
+   void (*dtr)(struct geniv_ctx *ctx);
+   int (*init)(struct geniv_ctx *ctx);
+   int (*wipe)(struct geniv_ctx *ctx);
+   int (*generator)(struct geniv_ctx *ctx,
+struct geniv_req_ctx *rctx,
+struct geniv_subreq *subreq);
+   int (*post)(struct geniv_ctx *ctx,
+   struct geniv_req_ctx *rctx,
+   struct geniv_subreq *subreq);
 };
 
-struct iv_essiv_private {
+struct geniv_essiv_private {
struct crypto_ahash *hash_tfm;
u8 *salt;
 };
 
-struct iv_benbi_private {
+struct geniv_benbi_private {
int shift;
 };
 
-#define LMK_SEED_SIZE 64 /* hash + 0 */
-struct iv_lmk_private {
+struct geniv_lmk_private {
struct crypto_shash *hash_tfm;
u8 *seed;
 };
 
-#define TCW_WHITENING_SIZE 16
-struct iv_tcw_private {
+struct geniv_tcw_private {
struct crypto_shash *crc32_tfm;
u8 *iv_seed;
u8 *whitening;
 };
 
-/*
- * Crypt: maps a linear range of a block device
- * and encrypts / decrypts at the s

[RFC PATCH v4] IV Generation algorithms for dm-crypt

[Binoy Jayan]

===
dm-crypt optimization for larger block sizes
===

Currently, the iv generation algorithms are implemented in dm-crypt.c. The goal
is to move these algorithms from the dm layer to the kernel crypto layer by
implementing them as template ciphers so they can be used in relation with
algorithms like aes, and with multiple modes like cbc, ecb etc. As part of this
patchset, the iv-generation code is moved from the dm layer to the crypto layer
and adapt the dm-layer to send a whole 'bio' (as defined in the block layer)
at a time. Each bio contains the in memory representation of physically
contiguous disk blocks. Since the bio itself may not be contiguous in main
memory, the dm layer sets up a chained scatterlist of these blocks split into
physically contiguous segments in memory so that DMA can be performed.

One challenge in doing so is that the IVs are generated based on a 512-byte
sector number. This infact limits the block sizes to 512 bytes. But this should
not be a problem if a hardware with iv generation support is used. The geniv
itself splits the segments into sectors so it could choose the IV based on
sector number. But it could be modelled in hardware effectively by not
splitting up the segments in the bio.

Another challenge faced is that dm-crypt has an option to use multiple keys.
The key selection is done based on the sector number. If the whole bio is
encrypted / decrypted with the same key, the encrypted volumes will not be
compatible with the original dm-crypt [without the changes]. So, the key
selection code is moved to crypto layer so the neighboring sectors are
encrypted with a different key.

The dm layer allocates space for iv. The hardware drivers can choose to make
use of this space to generate their IVs sequentially or allocate it on their
own. This can be moved to crypto layer too. Postponing this decision until
the requirement to integrate milan's changes are clear.

Interface to the crypto layer - include/crypto/geniv.h

Revisions:
--

v1: https://patchwork.kernel.org/patch/9439175
v2: https://patchwork.kernel.org/patch/9471923
v3: https://lkml.org/lkml/2017/1/18/170

v3 --> v4
--
Fix for the bug reported by Gilad Ben-Yossef.
The element '__ctx' in 'struct skcipher_request req' overflowed into the
element 'struct scatterlist src' which immediately follows 'req' in
'struct geniv_subreq' and corrupted src.

v2 --> v3
--

1. Moved iv algorithms in dm-crypt.c for control
2. Key management code moved from dm layer to cryto layer
   so that cipher instance selection can be made depending on key_index
3. The revision v2 had scatterlist nodes created for every sector in the bio.
   It is modified to create only once scatterlist node to reduce memory
   foot print. Synchronous requests are processed sequentially. Asynchronous
   requests are processed in parallel and is freed in the async callback.
4. Changed allocation for sub-requests using mempool

v1 --> v2
--

1. dm-crypt changes to process larger block sizes (one segment in a bio)
2. Incorporated changes w.r.t. comments from Herbert.

Binoy Jayan (1):
  crypto: Add IV generation algorithms

 drivers/md/dm-crypt.c  | 1894 ++--
 include/crypto/geniv.h |   47 ++
 2 files changed, 1402 insertions(+), 539 deletions(-)
 create mode 100644 include/crypto/geniv.h

-- 
Binoy Jayan

[RFC PATCH v4] IV Generation algorithms for dm-crypt

[Binoy Jayan]

===
dm-crypt optimization for larger block sizes
===

Currently, the iv generation algorithms are implemented in dm-crypt.c. The goal
is to move these algorithms from the dm layer to the kernel crypto layer by
implementing them as template ciphers so they can be used in relation with
algorithms like aes, and with multiple modes like cbc, ecb etc. As part of this
patchset, the iv-generation code is moved from the dm layer to the crypto layer
and adapt the dm-layer to send a whole 'bio' (as defined in the block layer)
at a time. Each bio contains the in memory representation of physically
contiguous disk blocks. Since the bio itself may not be contiguous in main
memory, the dm layer sets up a chained scatterlist of these blocks split into
physically contiguous segments in memory so that DMA can be performed.

One challenge in doing so is that the IVs are generated based on a 512-byte
sector number. This infact limits the block sizes to 512 bytes. But this should
not be a problem if a hardware with iv generation support is used. The geniv
itself splits the segments into sectors so it could choose the IV based on
sector number. But it could be modelled in hardware effectively by not
splitting up the segments in the bio.

Another challenge faced is that dm-crypt has an option to use multiple keys.
The key selection is done based on the sector number. If the whole bio is
encrypted / decrypted with the same key, the encrypted volumes will not be
compatible with the original dm-crypt [without the changes]. So, the key
selection code is moved to crypto layer so the neighboring sectors are
encrypted with a different key.

The dm layer allocates space for iv. The hardware drivers can choose to make
use of this space to generate their IVs sequentially or allocate it on their
own. This can be moved to crypto layer too. Postponing this decision until
the requirement to integrate milan's changes are clear.

Interface to the crypto layer - include/crypto/geniv.h

Revisions:
--

v1: https://patchwork.kernel.org/patch/9439175
v2: https://patchwork.kernel.org/patch/9471923
v3: https://lkml.org/lkml/2017/1/18/170

v3 --> v4
--
Fix for the bug reported by Gilad Ben-Yossef.
The element '__ctx' in 'struct skcipher_request req' overflowed into the
element 'struct scatterlist src' which immediately follows 'req' in
'struct geniv_subreq' and corrupted src.

v2 --> v3
--

1. Moved iv algorithms in dm-crypt.c for control
2. Key management code moved from dm layer to cryto layer
   so that cipher instance selection can be made depending on key_index
3. The revision v2 had scatterlist nodes created for every sector in the bio.
   It is modified to create only once scatterlist node to reduce memory
   foot print. Synchronous requests are processed sequentially. Asynchronous
   requests are processed in parallel and is freed in the async callback.
4. Changed allocation for sub-requests using mempool

v1 --> v2
--

1. dm-crypt changes to process larger block sizes (one segment in a bio)
2. Incorporated changes w.r.t. comments from Herbert.

Binoy Jayan (1):
  crypto: Add IV generation algorithms

 drivers/md/dm-crypt.c  | 1894 ++--
 include/crypto/geniv.h |   47 ++
 2 files changed, 1402 insertions(+), 539 deletions(-)
 create mode 100644 include/crypto/geniv.h

-- 
Binoy Jayan

Re: [RFC PATCH v3] crypto: Add IV generation algorithms

[Binoy Jayan]

Hi Gilad,

On 19 January 2017 at 15:17, Gilad Ben-Yossef  wrote:
> I tried adding sg_init_table() where I thought it was appropriate and
> it didn't resolve the issue.
>
> For what it's worth, my guess is that the difference between our
> setups is not related to Arm but to other options or the storage I'm
> using.

I was able to reproduce this again on my qemu setup with a 1GB virtual
disk. That is the same thing I do with the x86 setup as well.

> Are you using cryptd?

You mean config CRYPTO_CRYPTD?

-Binoy

Re: [RFC PATCH v3] crypto: Add IV generation algorithms

[Binoy Jayan]

Hi Gilad,

On 19 January 2017 at 15:17, Gilad Ben-Yossef  wrote:
> I tried adding sg_init_table() where I thought it was appropriate and
> it didn't resolve the issue.
>
> For what it's worth, my guess is that the difference between our
> setups is not related to Arm but to other options or the storage I'm
> using.

I was able to reproduce this again on my qemu setup with a 1GB virtual
disk. That is the same thing I do with the x86 setup as well.

> Are you using cryptd?

You mean config CRYPTO_CRYPTD?

-Binoy

Re: [RFC PATCH v3] crypto: Add IV generation algorithms

[Binoy Jayan]

Hi Gilad,

On 18 January 2017 at 20:51, Gilad Ben-Yossef  wrote:
> I have some review comments and a bug report -

Thank you very much for testing this on ARM and for the comments.

> I'm pretty sure this needs to be
>
>  n2 = bio_segments(ctx->bio_out);

Yes you are right, that was a typo :)

>> +
>> +   rinfo.is_write = bio_data_dir(ctx->bio_in) == WRITE;
>
> Please consider wrapping the above boolean expression in parenthesis.

Well, I can do that to enhance the clarity.

>> +   rinfo.iv_sector = ctx->cc_sector;
>> +   rinfo.nents = nents;
>> +   rinfo.iv = iv;
>> +
>> +   skcipher_request_set_crypt(req, dmreq->sg_in, dmreq->sg_out,
>
> Also, where do the scatterlist src2 and dst2 that you use
> sg_set_page() get sg_init_table() called on?
> I couldn't figure it out...

Thank you pointing this out. I missed out to add sg_init_table(src2, 1)
and sg_init_table(dst2, 1), but sg_set_page is used in geniv_iter_block.
This is probably the reason for the panic on ARM platform. However it
ran fine under qemu-x86. May be I should setup an arm platform too
for testing.

Regards,
Binoy

Re: [RFC PATCH v3] crypto: Add IV generation algorithms

[Binoy Jayan]

Hi Gilad,

On 18 January 2017 at 20:51, Gilad Ben-Yossef  wrote:
> I have some review comments and a bug report -

Thank you very much for testing this on ARM and for the comments.

> I'm pretty sure this needs to be
>
>  n2 = bio_segments(ctx->bio_out);

Yes you are right, that was a typo :)

>> +
>> +   rinfo.is_write = bio_data_dir(ctx->bio_in) == WRITE;
>
> Please consider wrapping the above boolean expression in parenthesis.

Well, I can do that to enhance the clarity.

>> +   rinfo.iv_sector = ctx->cc_sector;
>> +   rinfo.nents = nents;
>> +   rinfo.iv = iv;
>> +
>> +   skcipher_request_set_crypt(req, dmreq->sg_in, dmreq->sg_out,
>
> Also, where do the scatterlist src2 and dst2 that you use
> sg_set_page() get sg_init_table() called on?
> I couldn't figure it out...

Thank you pointing this out. I missed out to add sg_init_table(src2, 1)
and sg_init_table(dst2, 1), but sg_set_page is used in geniv_iter_block.
This is probably the reason for the panic on ARM platform. However it
ran fine under qemu-x86. May be I should setup an arm platform too
for testing.

Regards,
Binoy

[RFC PATCH v3] crypto: Add IV generation algorithms

[Binoy Jayan]

Currently, the iv generation algorithms are implemented in dm-crypt.c.
The goal is to move these algorithms from the dm layer to the kernel
crypto layer by implementing them as template ciphers so they can be
implemented in hardware for performance. As part of this patchset, the
iv-generation code is moved from the dm layer to the crypto layer and
adapt the dm-layer to send a whole 'bio' (as defined in the block layer)
at a time. Each bio contains an in memory representation of physically
contiguous disk blocks. The dm layer sets up a chained scatterlist of
these blocks split into physically contiguous segments in memory so that
DMA can be performed. Also, the key management code is moved from dm layer
to the cryto layer since the key selection for encrypting neighboring
sectors depend on the keycount.

Synchronous crypto requests to encrypt/decrypt a sector are processed
sequentially. Asynchronous requests if processed in parallel, are freed
in the async callback. The dm layer allocates space for iv. The hardware
implementations can choose to make use of this space to generate their IVs
sequentially or allocate it on their own.
Interface to the crypto layer - include/crypto/geniv.h

Signed-off-by: Binoy Jayan <binoy.ja...@linaro.org>
---
 drivers/md/dm-crypt.c  | 1891 ++--
 include/crypto/geniv.h |   47 ++
 2 files changed, 1399 insertions(+), 539 deletions(-)
 create mode 100644 include/crypto/geniv.h

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 7c6c572..7275b0f 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -32,170 +32,113 @@
 #include 
 #include 
 #include 
-
 #include 
-
-#define DM_MSG_PREFIX "crypt"
-
-/*
- * context holding the current state of a multi-part conversion
- */
-struct convert_context {
-   struct completion restart;
-   struct bio *bio_in;
-   struct bio *bio_out;
-   struct bvec_iter iter_in;
-   struct bvec_iter iter_out;
-   sector_t cc_sector;
-   atomic_t cc_pending;
-   struct skcipher_request *req;
+#include 
+#include 
+#include 
+#include 
+
+#define DM_MSG_PREFIX  "crypt"
+#define MAX_SG_LIST(BIO_MAX_PAGES * 8)
+#define MIN_IOS64
+#define LMK_SEED_SIZE  64 /* hash + 0 */
+#define TCW_WHITENING_SIZE 16
+
+struct geniv_ctx;
+struct geniv_req_ctx;
+
+/* Sub request for each of the skcipher_request's for a segment */
+struct geniv_subreq {
+   struct skcipher_request req CRYPTO_MINALIGN_ATTR;
+   struct scatterlist src;
+   struct scatterlist dst;
+   int n;
+   struct geniv_req_ctx *rctx;
 };
 
-/*
- * per bio private data
- */
-struct dm_crypt_io {
-   struct crypt_config *cc;
-   struct bio *base_bio;
-   struct work_struct work;
-
-   struct convert_context ctx;
-
-   atomic_t io_pending;
-   int error;
-   sector_t sector;
-
-   struct rb_node rb_node;
-} CRYPTO_MINALIGN_ATTR;
-
-struct dm_crypt_request {
-   struct convert_context *ctx;
-   struct scatterlist sg_in;
-   struct scatterlist sg_out;
+struct geniv_req_ctx {
+   struct geniv_subreq *subreq;
+   bool is_write;
sector_t iv_sector;
+   unsigned int nents;
+   u8 *iv;
+   struct completion restart;
+   atomic_t req_pending;
+   struct skcipher_request *req;
 };
 
-struct crypt_config;
-
 struct crypt_iv_operations {
-   int (*ctr)(struct crypt_config *cc, struct dm_target *ti,
-  const char *opts);
-   void (*dtr)(struct crypt_config *cc);
-   int (*init)(struct crypt_config *cc);
-   int (*wipe)(struct crypt_config *cc);
-   int (*generator)(struct crypt_config *cc, u8 *iv,
-struct dm_crypt_request *dmreq);
-   int (*post)(struct crypt_config *cc, u8 *iv,
-   struct dm_crypt_request *dmreq);
+   int (*ctr)(struct geniv_ctx *ctx);
+   void (*dtr)(struct geniv_ctx *ctx);
+   int (*init)(struct geniv_ctx *ctx);
+   int (*wipe)(struct geniv_ctx *ctx);
+   int (*generator)(struct geniv_ctx *ctx,
+struct geniv_req_ctx *rctx,
+struct geniv_subreq *subreq);
+   int (*post)(struct geniv_ctx *ctx,
+   struct geniv_req_ctx *rctx,
+   struct geniv_subreq *subreq);
 };
 
-struct iv_essiv_private {
+struct geniv_essiv_private {
struct crypto_ahash *hash_tfm;
u8 *salt;
 };
 
-struct iv_benbi_private {
+struct geniv_benbi_private {
int shift;
 };
 
-#define LMK_SEED_SIZE 64 /* hash + 0 */
-struct iv_lmk_private {
+struct geniv_lmk_private {
struct crypto_shash *hash_tfm;
u8 *seed;
 };
 
-#define TCW_WHITENING_SIZE 16
-struct iv_tcw_private {
+struct geniv_tcw_private {
struct crypto_shash *crc32_tfm;
u8 *iv_seed;
u8 *whitening;
 };
 
-/*
- * Crypt: maps a linear range of a block device
-

[RFC PATCH v3] crypto: Add IV generation algorithms

[Binoy Jayan]

Currently, the iv generation algorithms are implemented in dm-crypt.c.
The goal is to move these algorithms from the dm layer to the kernel
crypto layer by implementing them as template ciphers so they can be
implemented in hardware for performance. As part of this patchset, the
iv-generation code is moved from the dm layer to the crypto layer and
adapt the dm-layer to send a whole 'bio' (as defined in the block layer)
at a time. Each bio contains an in memory representation of physically
contiguous disk blocks. The dm layer sets up a chained scatterlist of
these blocks split into physically contiguous segments in memory so that
DMA can be performed. Also, the key management code is moved from dm layer
to the cryto layer since the key selection for encrypting neighboring
sectors depend on the keycount.

Synchronous crypto requests to encrypt/decrypt a sector are processed
sequentially. Asynchronous requests if processed in parallel, are freed
in the async callback. The dm layer allocates space for iv. The hardware
implementations can choose to make use of this space to generate their IVs
sequentially or allocate it on their own.
Interface to the crypto layer - include/crypto/geniv.h

Signed-off-by: Binoy Jayan 
---
 drivers/md/dm-crypt.c  | 1891 ++--
 include/crypto/geniv.h |   47 ++
 2 files changed, 1399 insertions(+), 539 deletions(-)
 create mode 100644 include/crypto/geniv.h

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 7c6c572..7275b0f 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -32,170 +32,113 @@
 #include 
 #include 
 #include 
-
 #include 
-
-#define DM_MSG_PREFIX "crypt"
-
-/*
- * context holding the current state of a multi-part conversion
- */
-struct convert_context {
-   struct completion restart;
-   struct bio *bio_in;
-   struct bio *bio_out;
-   struct bvec_iter iter_in;
-   struct bvec_iter iter_out;
-   sector_t cc_sector;
-   atomic_t cc_pending;
-   struct skcipher_request *req;
+#include 
+#include 
+#include 
+#include 
+
+#define DM_MSG_PREFIX  "crypt"
+#define MAX_SG_LIST(BIO_MAX_PAGES * 8)
+#define MIN_IOS64
+#define LMK_SEED_SIZE  64 /* hash + 0 */
+#define TCW_WHITENING_SIZE 16
+
+struct geniv_ctx;
+struct geniv_req_ctx;
+
+/* Sub request for each of the skcipher_request's for a segment */
+struct geniv_subreq {
+   struct skcipher_request req CRYPTO_MINALIGN_ATTR;
+   struct scatterlist src;
+   struct scatterlist dst;
+   int n;
+   struct geniv_req_ctx *rctx;
 };
 
-/*
- * per bio private data
- */
-struct dm_crypt_io {
-   struct crypt_config *cc;
-   struct bio *base_bio;
-   struct work_struct work;
-
-   struct convert_context ctx;
-
-   atomic_t io_pending;
-   int error;
-   sector_t sector;
-
-   struct rb_node rb_node;
-} CRYPTO_MINALIGN_ATTR;
-
-struct dm_crypt_request {
-   struct convert_context *ctx;
-   struct scatterlist sg_in;
-   struct scatterlist sg_out;
+struct geniv_req_ctx {
+   struct geniv_subreq *subreq;
+   bool is_write;
sector_t iv_sector;
+   unsigned int nents;
+   u8 *iv;
+   struct completion restart;
+   atomic_t req_pending;
+   struct skcipher_request *req;
 };
 
-struct crypt_config;
-
 struct crypt_iv_operations {
-   int (*ctr)(struct crypt_config *cc, struct dm_target *ti,
-  const char *opts);
-   void (*dtr)(struct crypt_config *cc);
-   int (*init)(struct crypt_config *cc);
-   int (*wipe)(struct crypt_config *cc);
-   int (*generator)(struct crypt_config *cc, u8 *iv,
-struct dm_crypt_request *dmreq);
-   int (*post)(struct crypt_config *cc, u8 *iv,
-   struct dm_crypt_request *dmreq);
+   int (*ctr)(struct geniv_ctx *ctx);
+   void (*dtr)(struct geniv_ctx *ctx);
+   int (*init)(struct geniv_ctx *ctx);
+   int (*wipe)(struct geniv_ctx *ctx);
+   int (*generator)(struct geniv_ctx *ctx,
+struct geniv_req_ctx *rctx,
+struct geniv_subreq *subreq);
+   int (*post)(struct geniv_ctx *ctx,
+   struct geniv_req_ctx *rctx,
+   struct geniv_subreq *subreq);
 };
 
-struct iv_essiv_private {
+struct geniv_essiv_private {
struct crypto_ahash *hash_tfm;
u8 *salt;
 };
 
-struct iv_benbi_private {
+struct geniv_benbi_private {
int shift;
 };
 
-#define LMK_SEED_SIZE 64 /* hash + 0 */
-struct iv_lmk_private {
+struct geniv_lmk_private {
struct crypto_shash *hash_tfm;
u8 *seed;
 };
 
-#define TCW_WHITENING_SIZE 16
-struct iv_tcw_private {
+struct geniv_tcw_private {
struct crypto_shash *crc32_tfm;
u8 *iv_seed;
u8 *whitening;
 };
 
-/*
- * Crypt: maps a linear range of a block device
- * and encrypts / decrypts at the s

[RFC PATCH v3] IV Generation algorithms for dm-crypt

[Binoy Jayan]

===
GENIV Template cipher
===

Currently, the iv generation algorithms are implemented in dm-crypt.c. The goal
is to move these algorithms from the dm layer to the kernel crypto layer by
implementing them as template ciphers so they can be used in relation with
algorithms like aes, and with multiple modes like cbc, ecb etc. As part of this
patchset, the iv-generation code is moved from the dm layer to the crypto layer
and adapt the dm-layer to send a whole 'bio' (as defined in the block layer)
at a time. Each bio contains the in memory representation of physically
contiguous disk blocks. Since the bio itself may not be contiguous in main
memory, the dm layer sets up a chained scatterlist of these blocks split into
physically contiguous segments in memory so that DMA can be performed.

One challenge in doing so is that the IVs are generated based on a 512-byte
sector number. This infact limits the block sizes to 512 bytes. But this should
not be a problem if a hardware with iv generation support is used. The geniv
itself splits the segments into sectors so it could choose the IV based on
sector number. But it could be modelled in hardware effectively by not
splitting up the segments in the bio.

Another challenge faced is that dm-crypt has an option to use multiple keys.
The key selection is done based on the sector number. If the whole bio is
encrypted / decrypted with the same key, the encrypted volumes will not be
compatible with the original dm-crypt [without the changes]. So, the key
selection code is moved to crypto layer so the neighboring sectors are
encrypted with a different key.

The dm layer allocates space for iv. The hardware drivers can choose to make
use of this space to generate their IVs sequentially or allocate it on their
own. This can be moved to crypto layer too. Postponing this decision until
the requirement to integrate milan's changes are clear.

Interface to the crypto layer - include/crypto/geniv.h

Revisions:

v1: https://patchwork.kernel.org/patch/9439175
v2: https://patchwork.kernel.org/patch/9471923

v2 --> v3
--

1. Moved iv algorithms in dm-crypt.c for control
2. Key management code moved from dm layer to cryto layer
   so that cipher instance selection can be made depending on key_index
3. The revision v2 had scatterlist nodes created for every sector in the bio.
   It is modified to create only once scatterlist node to reduce memory
   foot print. Synchronous requests are processed sequentially. Asynchronous
   requests are processed in parallel and is freed in the async callback.
4. Changed allocation for sub-requests using mempool

v1 --> v2
--

1. dm-crypt changes to process larger block sizes (one segment in a bio)
2. Incorporated changes w.r.t. comments from Herbert.

Binoy Jayan (1):
  crypto: Add IV generation algorithms

 drivers/md/dm-crypt.c  | 1891 ++--
 include/crypto/geniv.h |   47 ++
 2 files changed, 1399 insertions(+), 539 deletions(-)
 create mode 100644 include/crypto/geniv.h

-- 
Binoy Jayan

[RFC PATCH v3] IV Generation algorithms for dm-crypt

[Binoy Jayan]

===
GENIV Template cipher
===

Currently, the iv generation algorithms are implemented in dm-crypt.c. The goal
is to move these algorithms from the dm layer to the kernel crypto layer by
implementing them as template ciphers so they can be used in relation with
algorithms like aes, and with multiple modes like cbc, ecb etc. As part of this
patchset, the iv-generation code is moved from the dm layer to the crypto layer
and adapt the dm-layer to send a whole 'bio' (as defined in the block layer)
at a time. Each bio contains the in memory representation of physically
contiguous disk blocks. Since the bio itself may not be contiguous in main
memory, the dm layer sets up a chained scatterlist of these blocks split into
physically contiguous segments in memory so that DMA can be performed.

One challenge in doing so is that the IVs are generated based on a 512-byte
sector number. This infact limits the block sizes to 512 bytes. But this should
not be a problem if a hardware with iv generation support is used. The geniv
itself splits the segments into sectors so it could choose the IV based on
sector number. But it could be modelled in hardware effectively by not
splitting up the segments in the bio.

Another challenge faced is that dm-crypt has an option to use multiple keys.
The key selection is done based on the sector number. If the whole bio is
encrypted / decrypted with the same key, the encrypted volumes will not be
compatible with the original dm-crypt [without the changes]. So, the key
selection code is moved to crypto layer so the neighboring sectors are
encrypted with a different key.

The dm layer allocates space for iv. The hardware drivers can choose to make
use of this space to generate their IVs sequentially or allocate it on their
own. This can be moved to crypto layer too. Postponing this decision until
the requirement to integrate milan's changes are clear.

Interface to the crypto layer - include/crypto/geniv.h

Revisions:

v1: https://patchwork.kernel.org/patch/9439175
v2: https://patchwork.kernel.org/patch/9471923

v2 --> v3
--

1. Moved iv algorithms in dm-crypt.c for control
2. Key management code moved from dm layer to cryto layer
   so that cipher instance selection can be made depending on key_index
3. The revision v2 had scatterlist nodes created for every sector in the bio.
   It is modified to create only once scatterlist node to reduce memory
   foot print. Synchronous requests are processed sequentially. Asynchronous
   requests are processed in parallel and is freed in the async callback.
4. Changed allocation for sub-requests using mempool

v1 --> v2
--

1. dm-crypt changes to process larger block sizes (one segment in a bio)
2. Incorporated changes w.r.t. comments from Herbert.

Binoy Jayan (1):
  crypto: Add IV generation algorithms

 drivers/md/dm-crypt.c  | 1891 ++--
 include/crypto/geniv.h |   47 ++
 2 files changed, 1399 insertions(+), 539 deletions(-)
 create mode 100644 include/crypto/geniv.h

-- 
Binoy Jayan

Re: [RFC PATCH v2] crypto: Add IV generation algorithms

[Binoy Jayan]

Hi Herbert,

On 2 January 2017 at 12:23, Herbert Xu <herb...@gondor.apana.org.au> wrote:
> On Mon, Jan 02, 2017 at 12:16:45PM +0530, Binoy Jayan wrote:
>
> Right.  The actual number of underlying tfms that do the work
> won't change compared to the status quo.  We're just structuring
> it such that if the overall scheme is supported by the hardware
> then we can feed more than one sector at a time to it.

I was thinking of continuing to have the iv generation algorithms as template
ciphers instead of regular 'skcipher' as it is easier to inherit the parameters
from the underlying cipher (e.g. aes) like cra_blocksize, cra_alignmask,
ivsize, chunksize etc.

Usually, the underlying cipher for the template ciphers are instantiated
in the following function:

skcipher_instance:skcipher_alg:init()

Since the number of such cipher instances depend on the key count, which is
not known at the time of creation of the cipher (it's passed to as an argument
to the setkey api), the creation of those have to be delayed until the setkey
operation of the template cipher. But as Mark pointed out, the users of this
cipher may get confused if the creation of the underlying cipher fails while
trying to do a 'setkey' on the template cipher. I was wondering if I can create
a single instance of the cipher and assign it to tfms[0] and allocate the
remaining instances when the setkey operation is called later with the encoded
key_count so that errors during cipher creation are uncovered earlier.

Thanks,
Binoy

Re: [RFC PATCH v2] crypto: Add IV generation algorithms

[Binoy Jayan]

Hi Herbert,

On 2 January 2017 at 12:23, Herbert Xu  wrote:
> On Mon, Jan 02, 2017 at 12:16:45PM +0530, Binoy Jayan wrote:
>
> Right.  The actual number of underlying tfms that do the work
> won't change compared to the status quo.  We're just structuring
> it such that if the overall scheme is supported by the hardware
> then we can feed more than one sector at a time to it.

I was thinking of continuing to have the iv generation algorithms as template
ciphers instead of regular 'skcipher' as it is easier to inherit the parameters
from the underlying cipher (e.g. aes) like cra_blocksize, cra_alignmask,
ivsize, chunksize etc.

Usually, the underlying cipher for the template ciphers are instantiated
in the following function:

skcipher_instance:skcipher_alg:init()

Since the number of such cipher instances depend on the key count, which is
not known at the time of creation of the cipher (it's passed to as an argument
to the setkey api), the creation of those have to be delayed until the setkey
operation of the template cipher. But as Mark pointed out, the users of this
cipher may get confused if the creation of the underlying cipher fails while
trying to do a 'setkey' on the template cipher. I was wondering if I can create
a single instance of the cipher and assign it to tfms[0] and allocate the
remaining instances when the setkey operation is called later with the encoded
key_count so that errors during cipher creation are uncovered earlier.

Thanks,
Binoy

Re: [RFC PATCH v2] crypto: Add IV generation algorithms

[Binoy Jayan]

Hi Gilad,

On 3 January 2017 at 19:53, Gilad Ben-Yossef  wrote:
> Good idea. I wanted to test the patch but alas it does not apply cleanly.
> You seem to have a blank line at the end of files and other small
> transgressions that makes checkpatch grumpy.

I think that is because there were some key structure changes in dm-crypt
after I sent out v2. I have resolved them while working on v3. Please wait for
the next version of the patchset. I'll send it probably by next week.
I wanted to incorporate a few changes suggested by Herbert before sending them.

> What is Not-signed-off-by ? :-)

It was just an RFC patch, not ready for merging.

Thanks,
Binoy

Re: [RFC PATCH v2] crypto: Add IV generation algorithms

[Binoy Jayan]

Hi Gilad,

On 3 January 2017 at 19:53, Gilad Ben-Yossef  wrote:
> Good idea. I wanted to test the patch but alas it does not apply cleanly.
> You seem to have a blank line at the end of files and other small
> transgressions that makes checkpatch grumpy.

I think that is because there were some key structure changes in dm-crypt
after I sent out v2. I have resolved them while working on v3. Please wait for
the next version of the patchset. I'll send it probably by next week.
I wanted to incorporate a few changes suggested by Herbert before sending them.

> What is Not-signed-off-by ? :-)

It was just an RFC patch, not ready for merging.

Thanks,
Binoy

Re: [RFC PATCH v2] crypto: Add IV generation algorithms

[Binoy Jayan]

On 2 January 2017 at 12:23, Herbert Xu <herb...@gondor.apana.org.au> wrote:
> On Mon, Jan 02, 2017 at 12:16:45PM +0530, Binoy Jayan wrote:
>>
>> Even if ciphers are allocated this way, all the encryption requests
>> for cbc should still go through IV generators? So that should mean,
>> create one instance of IV generator using 'crypto_alloc_skcipher'
>> and create tfms_count instances of the generator depending on the
>> number of keys.
>
> Right.  The actual number of underlying tfms that do the work
> won't change compared to the status quo.  We're just structuring
> it such that if the overall scheme is supported by the hardware
> then we can feed more than one sector at a time to it.

Thank you Herbert.

Re: [RFC PATCH v2] crypto: Add IV generation algorithms

[Binoy Jayan]

On 2 January 2017 at 12:23, Herbert Xu  wrote:
> On Mon, Jan 02, 2017 at 12:16:45PM +0530, Binoy Jayan wrote:
>>
>> Even if ciphers are allocated this way, all the encryption requests
>> for cbc should still go through IV generators? So that should mean,
>> create one instance of IV generator using 'crypto_alloc_skcipher'
>> and create tfms_count instances of the generator depending on the
>> number of keys.
>
> Right.  The actual number of underlying tfms that do the work
> won't change compared to the status quo.  We're just structuring
> it such that if the overall scheme is supported by the hardware
> then we can feed more than one sector at a time to it.

Thank you Herbert.

Re: [RFC PATCH v2] crypto: Add IV generation algorithms

[Binoy Jayan]

Hi Herbert,

On 30 December 2016 at 15:57, Herbert Xu  wrote:

> This is just a matter of structuring the key for the IV generator.
> The IV generator's key in this case should be a combination of the
> key to the underlying CBC plus the set of all keys for the IV
> generator itself.  It should then allocate the required number of
> tfms as is currently done by crypt_alloc_tfms in dm-crypt.

Since I used template ciphers for the iv algorithms, I use
crypto_spawn_skcipher_alg and skcipher_register_instance
for creating the underlying cbc algorithm. I guess you suggest
to change that to make use of crypto_alloc_skcipher.

Even if ciphers are allocated this way, all the encryption requests
for cbc should still go through IV generators? So that should mean,
create one instance of IV generator using 'crypto_alloc_skcipher'
and create tfms_count instances of the generator depending on the
number of keys.

Thanks,
Binoy

Re: [RFC PATCH v2] crypto: Add IV generation algorithms

[Binoy Jayan]

Hi Herbert,

On 30 December 2016 at 15:57, Herbert Xu  wrote:

> This is just a matter of structuring the key for the IV generator.
> The IV generator's key in this case should be a combination of the
> key to the underlying CBC plus the set of all keys for the IV
> generator itself.  It should then allocate the required number of
> tfms as is currently done by crypt_alloc_tfms in dm-crypt.

Since I used template ciphers for the iv algorithms, I use
crypto_spawn_skcipher_alg and skcipher_register_instance
for creating the underlying cbc algorithm. I guess you suggest
to change that to make use of crypto_alloc_skcipher.

Even if ciphers are allocated this way, all the encryption requests
for cbc should still go through IV generators? So that should mean,
create one instance of IV generator using 'crypto_alloc_skcipher'
and create tfms_count instances of the generator depending on the
number of keys.

Thanks,
Binoy

Re: [RFC PATCH v2] crypto: Add IV generation algorithms

[Binoy Jayan]

Hi Herbert,

Sorry for the delayed response, I was busy with testing dm-crypt
with bonnie++ for regressions. I tried to find some alternative
way to keep the IV algorithms' registration in the dm-crypt.
Also there were some changes done in dm-crypt keys structure too
recently.

c538f6e dm crypt: add ability to use keys from the kernel key retention service

On Thu, Dec 22, 2016 at 04:25:12PM +0530, Binoy Jayan wrote:
>
> > It doesn't have to live outside of dm-crypt.  You can register
> > these IV generators from there if you really want.
>
> Sorry, but I didn't understand this part.

What I mean is that moving the IV generators into the crypto API
does not mean the dm-crypt team giving up control over them.  You
could continue to keep them within the dm-crypt code base and
still register them through the normal crypto API mechanism

When we keep these in dm-crypt and if more than one key is used
(it is actually more than one parts of the original key),
there are more than one cipher instance created - one for each
unique part of the key. Since the crypto requests are modelled
to go through the template ciphers in the order:

"essiv -> cbc -> aes"

a particular cipher instance of the IV (essiv in this example) is
responsible to encrypt an entire bigger block. If this bigger block
is to be later split into 512 bytes blocks and then encrypted using
the other cipher instance depending on the following formula:

key_index = sector & (key_count - 1)

it is not possible as the cipher instances do not have access to
each other's instances. So, number of keys used is crucial while
performing encryption.

If there was only a single key, it should not have been a problem.
But if there are more than one key, then encrypting a bigger block
with a single key would cause backward incompatibility.
I was wondering if this is acceptable.

bigger block: What I mean by bigger block here is the set of 512-byte
blocks that dm-crypt can be optimized to process at once.

Thanks,
Binoy

Re: [RFC PATCH v2] crypto: Add IV generation algorithms

[Binoy Jayan]

Hi Herbert,

Sorry for the delayed response, I was busy with testing dm-crypt
with bonnie++ for regressions. I tried to find some alternative
way to keep the IV algorithms' registration in the dm-crypt.
Also there were some changes done in dm-crypt keys structure too
recently.

c538f6e dm crypt: add ability to use keys from the kernel key retention service

On Thu, Dec 22, 2016 at 04:25:12PM +0530, Binoy Jayan wrote:
>
> > It doesn't have to live outside of dm-crypt.  You can register
> > these IV generators from there if you really want.
>
> Sorry, but I didn't understand this part.

What I mean is that moving the IV generators into the crypto API
does not mean the dm-crypt team giving up control over them.  You
could continue to keep them within the dm-crypt code base and
still register them through the normal crypto API mechanism

When we keep these in dm-crypt and if more than one key is used
(it is actually more than one parts of the original key),
there are more than one cipher instance created - one for each
unique part of the key. Since the crypto requests are modelled
to go through the template ciphers in the order:

"essiv -> cbc -> aes"

a particular cipher instance of the IV (essiv in this example) is
responsible to encrypt an entire bigger block. If this bigger block
is to be later split into 512 bytes blocks and then encrypted using
the other cipher instance depending on the following formula:

key_index = sector & (key_count - 1)

it is not possible as the cipher instances do not have access to
each other's instances. So, number of keys used is crucial while
performing encryption.

If there was only a single key, it should not have been a problem.
But if there are more than one key, then encrypting a bigger block
with a single key would cause backward incompatibility.
I was wondering if this is acceptable.

bigger block: What I mean by bigger block here is the set of 512-byte
blocks that dm-crypt can be optimized to process at once.

Thanks,
Binoy

Re: [RFC PATCH v2] crypto: Add IV generation algorithms

[Binoy Jayan]

Hi Herbert,

On 22 December 2016 at 14:25, Herbert Xu  wrote:
> On Tue, Dec 13, 2016 at 11:01:08AM +0100, Milan Broz wrote:
>>
>> By the move everything to cryptoAPI we are basically introducing some 
>> strange mix
>> of IV and modes there, I wonder how this is going to be maintained.
>> Anyway, Herbert should say if it is ok...
>
> Well there is precedent in how do the IPsec IV generation.  In
> that case the IV generators too are completely specific to that
> application, i.e., IPsec.  However, the way structured it allowed
> us to have one single entry path from the IPsec stack into the
> crypto layer regardless of whether you are using AEAD or traditional
> encryption/hashing algorithms.
>
> For IPsec we make the IV generators behave like normal AEAD
> algorithms, except that they take the sequence number as the IV.
>
> The goal here are obviously different.  However, by employing
> the same method as we do in IPsec, it appears to me that you
> can effectively process multiple blocks at once instead of having
> to supply one block at a time due to the IV generation issue.

Thank you for clarifying that part.:)
So, I hope we can consider algorithms like lmk and tcw too as IV generation
algorithms, even though they manipulate encrypted data directly?

>> I really do not think the disk encryption key management should be moved
>> outside of dm-crypt. We cannot then change key structure later easily.

I agree with this too, only problem with this is when multiple keys are involved
(where the master key is split into 2 or more), and the key selection is made
with a modular division of the (512-byte) sector number by the number of keys.

> It doesn't have to live outside of dm-crypt.  You can register
> these IV generators from there if you really want.

Sorry, but I didn't understand this part.

Thanks,
Binoy

Re: [RFC PATCH v2] crypto: Add IV generation algorithms

[Binoy Jayan]

Hi Herbert,

On 22 December 2016 at 14:25, Herbert Xu  wrote:
> On Tue, Dec 13, 2016 at 11:01:08AM +0100, Milan Broz wrote:
>>
>> By the move everything to cryptoAPI we are basically introducing some 
>> strange mix
>> of IV and modes there, I wonder how this is going to be maintained.
>> Anyway, Herbert should say if it is ok...
>
> Well there is precedent in how do the IPsec IV generation.  In
> that case the IV generators too are completely specific to that
> application, i.e., IPsec.  However, the way structured it allowed
> us to have one single entry path from the IPsec stack into the
> crypto layer regardless of whether you are using AEAD or traditional
> encryption/hashing algorithms.
>
> For IPsec we make the IV generators behave like normal AEAD
> algorithms, except that they take the sequence number as the IV.
>
> The goal here are obviously different.  However, by employing
> the same method as we do in IPsec, it appears to me that you
> can effectively process multiple blocks at once instead of having
> to supply one block at a time due to the IV generation issue.

Thank you for clarifying that part.:)
So, I hope we can consider algorithms like lmk and tcw too as IV generation
algorithms, even though they manipulate encrypted data directly?

>> I really do not think the disk encryption key management should be moved
>> outside of dm-crypt. We cannot then change key structure later easily.

I agree with this too, only problem with this is when multiple keys are involved
(where the master key is split into 2 or more), and the key selection is made
with a modular division of the (512-byte) sector number by the number of keys.

> It doesn't have to live outside of dm-crypt.  You can register
> these IV generators from there if you really want.

Sorry, but I didn't understand this part.

Thanks,
Binoy

Re: dm-crypt optimization

[Binoy Jayan]

Hi Milan,

On 21 December 2016 at 18:17, Milan Broz  wrote:

> So the core problem is that your crypto accelerator can operate efficiently 
> only
> with bigger batch sizes.

Thank you for the reply. Yes, that would be rather an improvement when having
bigger block sizes.

> How big blocks your crypto hw need to be able to operate more efficiently?
> What about 4k blocks (no batches), could it be usable trade-off?

The benchmark results for Qualcomm Snapdragon SoC's (mentioned below) show
significant improvement with 4K blocks but in batches of all such contiguous
segments in the block layer's request queue in the form of a chained
scatterlist.
However, it uses the algorithm 'aes-xts' instead of the conventional
'essiv-cbc-aes'
used in dm-crypt. Also, it uses the device mapper dm-req-crypt instead
of dm-cypt.

http://nelenkov.blogspot.in/2015/05/hardware-accelerated-disk-encryption-in.html
Section : 'Performance'

Its reports and IO rate of 46.3MB/s compared to an IO rate of 25.1MB/s while
using a software-based FDE (based on dm-crypt).  But I am not sure how genuine
this data is or how it was tested.

Since qualcomm SoC's use hardware backed keystore for managing keys and since
there is no easy way to make dm-crypt work with qualcomm's engines, I do not
have solid benchmark data to show an improved performance when using 4k blocks.

> With some (backward incompatible) changes in LUKS format I would like to see 
> support
> for encryption blocks equivalent to sectors size, so it basically means for 
> 4k drive 4k
> encryption block.
> (This should decrease overhead, now is everything processed on 512 blocks 
> only.)
>
> Support of bigger block sizes would be unsafe without additional mechanism 
> that provides
> atomic writes of multiple sectors. Maybe it applies to 4k as well on some 
> devices though...)

Did you mean write to the crypto output buffers or the actual disk write?
I didn't quite understand how the block size for encryption affects atomic
writes as it is the block layer which handles them. As far as dm-crypt is,
concerned it just encrypts/decrypts a 'struct bio' instance and submits the IO
operation to the block layer.

> The above is not going against your proposal, I am just curious if this is 
> enough
> to provide better performance on your hw accelerator or not.

May be I should be able to procure an open crypto board and get back to you with
some results. Or may be show even a marginal improvement while using software
algorithm by avoiding the crypto overhead for every 512 bytes.

-Binoy

Re: dm-crypt optimization

[Binoy Jayan]

Hi Milan,

On 21 December 2016 at 18:17, Milan Broz  wrote:

> So the core problem is that your crypto accelerator can operate efficiently 
> only
> with bigger batch sizes.

Thank you for the reply. Yes, that would be rather an improvement when having
bigger block sizes.

> How big blocks your crypto hw need to be able to operate more efficiently?
> What about 4k blocks (no batches), could it be usable trade-off?

The benchmark results for Qualcomm Snapdragon SoC's (mentioned below) show
significant improvement with 4K blocks but in batches of all such contiguous
segments in the block layer's request queue in the form of a chained
scatterlist.
However, it uses the algorithm 'aes-xts' instead of the conventional
'essiv-cbc-aes'
used in dm-crypt. Also, it uses the device mapper dm-req-crypt instead
of dm-cypt.

http://nelenkov.blogspot.in/2015/05/hardware-accelerated-disk-encryption-in.html
Section : 'Performance'

Its reports and IO rate of 46.3MB/s compared to an IO rate of 25.1MB/s while
using a software-based FDE (based on dm-crypt).  But I am not sure how genuine
this data is or how it was tested.

Since qualcomm SoC's use hardware backed keystore for managing keys and since
there is no easy way to make dm-crypt work with qualcomm's engines, I do not
have solid benchmark data to show an improved performance when using 4k blocks.

> With some (backward incompatible) changes in LUKS format I would like to see 
> support
> for encryption blocks equivalent to sectors size, so it basically means for 
> 4k drive 4k
> encryption block.
> (This should decrease overhead, now is everything processed on 512 blocks 
> only.)
>
> Support of bigger block sizes would be unsafe without additional mechanism 
> that provides
> atomic writes of multiple sectors. Maybe it applies to 4k as well on some 
> devices though...)

Did you mean write to the crypto output buffers or the actual disk write?
I didn't quite understand how the block size for encryption affects atomic
writes as it is the block layer which handles them. As far as dm-crypt is,
concerned it just encrypts/decrypts a 'struct bio' instance and submits the IO
operation to the block layer.

> The above is not going against your proposal, I am just curious if this is 
> enough
> to provide better performance on your hw accelerator or not.

May be I should be able to procure an open crypto board and get back to you with
some results. Or may be show even a marginal improvement while using software
algorithm by avoiding the crypto overhead for every 512 bytes.

-Binoy

dm-crypt optimization

[Binoy Jayan]

At a high level the goal is to maximize the size of data blocks that get passed
to hardware accelerators, minimizing the overhead from setting up and tearing
down operations in the hardware. Currently dm-crypt itself is a big blocker as
it manually implements ESSIV and similar algorithms which allow per-block
encryption of the data so the low level operations from the crypto API can
only operate on a single block. This is done because currently the crypto API
doesn't have software implementations of these algorithms itself so dm-crypt
can't rely on it being able to provide the functionality. The plan to address
this was to provide some software implementations in the crypto API, then
update dm-crypt to rely on those. Even for a pure software implementation
with no hardware acceleration that should hopefully provide a small
optimization as we need to call into the crypto API less often but it's likely
to be marginal given the overhead of crypto, the real win would be on a system
that has an accelerator that can replace the software implementation.

Currently dm-crypt handles data only in single blocks. This means that it can't
make good use of hardware cryptography engines since there is an overhead to
each transaction with the engine but transfers must be split into block sized
chunks. Allowing the transfer of larger blocks e.g. 'struct bio', could
mitigate against these costs and could improve performance in operating systems
with encrypted filesystems. Although qualcomm chipsets support another variant
of the device-mapper dm-req-crypt, it is not something generic and in
mainline-able state. Also, it only supports 'XTS-AES' mode of encryption and
is not compatible with other modes supported by dm-crypt.

However, there are some challenges and a few possibilities to address this. I
request you to provide your suggestions on whether the points mentioned below
makes sense and if it could be done differently.

1. Move the 'real' IV generation algorithms to crypto layer (e.g. essiv)
2. Increase the 'length' of the scatterlist nodes used in the crypto api. It
   can be made equal to the size of a main memory segment (as defined in
   'struct bio') as they are physcially contiguous.
3. Multiple segments in 'struct bio' can be represented as scatterlist of all
   segments in a 'struct bio'.

4. Move algorithms 'lmk' and 'tcw' (which are IV combined with hacks to the
   cbc mode) to create a customized cbc algorithm, implemented in a seperate
   file (e.g. cbc_lmk.c/cbc_tcw.c). As Milan suggested, these can't be treated
   as real IVs as these include hacks to the cbc mode (and directly manipulate
   encrypted data).

5. Move key selection logic to user space or always assume keycount as '1'
   (as mentioned in the dm-crypt param format below) so that the key selection
   logic does not have to be dependent on the sector number. This is necessary
   as the key is selected otherwise based on sector number:

   key_index = sector & (key_count - 1)

   If block size for scatterlist nodes are increased beyond sector boundary
   (which is what we plan to achieve, for performance), the key set for every
   cipher operation cannot be changed at the sector level.

   dm-crypt param format : cipher[:keycount]-mode-iv:ivopts
   Example   : aes:2-cbc-essiv:sha256

   Also as Milan suggested, it is not wise to move the key selection logic to
   the crypto layer as it will prevent any changes to the key structure later.

The following is a reference to an earlier patchset. It had the cipher mode
'cbc' mixed up with the IV algorithms and is usually not the preferred way.

Reference:
https://lkml.org/lkml/2016/12/13/65
https://lkml.org/lkml/2016/12/13/66

dm-crypt optimization

[Binoy Jayan]

At a high level the goal is to maximize the size of data blocks that get passed
to hardware accelerators, minimizing the overhead from setting up and tearing
down operations in the hardware. Currently dm-crypt itself is a big blocker as
it manually implements ESSIV and similar algorithms which allow per-block
encryption of the data so the low level operations from the crypto API can
only operate on a single block. This is done because currently the crypto API
doesn't have software implementations of these algorithms itself so dm-crypt
can't rely on it being able to provide the functionality. The plan to address
this was to provide some software implementations in the crypto API, then
update dm-crypt to rely on those. Even for a pure software implementation
with no hardware acceleration that should hopefully provide a small
optimization as we need to call into the crypto API less often but it's likely
to be marginal given the overhead of crypto, the real win would be on a system
that has an accelerator that can replace the software implementation.

Currently dm-crypt handles data only in single blocks. This means that it can't
make good use of hardware cryptography engines since there is an overhead to
each transaction with the engine but transfers must be split into block sized
chunks. Allowing the transfer of larger blocks e.g. 'struct bio', could
mitigate against these costs and could improve performance in operating systems
with encrypted filesystems. Although qualcomm chipsets support another variant
of the device-mapper dm-req-crypt, it is not something generic and in
mainline-able state. Also, it only supports 'XTS-AES' mode of encryption and
is not compatible with other modes supported by dm-crypt.

However, there are some challenges and a few possibilities to address this. I
request you to provide your suggestions on whether the points mentioned below
makes sense and if it could be done differently.

1. Move the 'real' IV generation algorithms to crypto layer (e.g. essiv)
2. Increase the 'length' of the scatterlist nodes used in the crypto api. It
   can be made equal to the size of a main memory segment (as defined in
   'struct bio') as they are physcially contiguous.
3. Multiple segments in 'struct bio' can be represented as scatterlist of all
   segments in a 'struct bio'.

4. Move algorithms 'lmk' and 'tcw' (which are IV combined with hacks to the
   cbc mode) to create a customized cbc algorithm, implemented in a seperate
   file (e.g. cbc_lmk.c/cbc_tcw.c). As Milan suggested, these can't be treated
   as real IVs as these include hacks to the cbc mode (and directly manipulate
   encrypted data).

5. Move key selection logic to user space or always assume keycount as '1'
   (as mentioned in the dm-crypt param format below) so that the key selection
   logic does not have to be dependent on the sector number. This is necessary
   as the key is selected otherwise based on sector number:

   key_index = sector & (key_count - 1)

   If block size for scatterlist nodes are increased beyond sector boundary
   (which is what we plan to achieve, for performance), the key set for every
   cipher operation cannot be changed at the sector level.

   dm-crypt param format : cipher[:keycount]-mode-iv:ivopts
   Example   : aes:2-cbc-essiv:sha256

   Also as Milan suggested, it is not wise to move the key selection logic to
   the crypto layer as it will prevent any changes to the key structure later.

The following is a reference to an earlier patchset. It had the cipher mode
'cbc' mixed up with the IV algorithms and is usually not the preferred way.

Reference:
https://lkml.org/lkml/2016/12/13/65
https://lkml.org/lkml/2016/12/13/66

Re: [RFC PATCH v2] crypto: Add IV generation algorithms

[Binoy Jayan]

Hi Milan,

On 13 December 2016 at 15:31, Milan Broz  wrote:

> I think that IV generators should not modify or read encrypted data directly,
> it should only generate IV.

I was trying to find more information about what you said and how a
iv generator should be written. I saw two examples of IV generators
too used with AEAD ciphers (crypto/seqiv.c and crypto/echainiv.c)

Excerpt from crypto api doc:
http://www.chronox.de/crypto-API/crypto/architecture.html#crypto-api-cipher-references-and-priority

2. Now, SEQIV uses the AEAD API function calls to invoke the associated
AEAD cipher. In our case, during the instantiation of SEQIV, the cipher
handle for GCM is provided to SEQIV. This means that SEQIV invokes
AEAD cipher operations with the GCM cipher handle.

Here, it says seqiv invokes cipher operations. However the code crypto/seqiv.c
does not look similar to how the modes are implemented which is confusing. I
was looking for an example of an IV generator used with a regular block cipher
and not a AEAD cipher. Could you point me out to some?

Thanks,
Binoy

Re: [RFC PATCH v2] crypto: Add IV generation algorithms

[Binoy Jayan]

Hi Milan,

On 13 December 2016 at 15:31, Milan Broz  wrote:

> I think that IV generators should not modify or read encrypted data directly,
> it should only generate IV.

I was trying to find more information about what you said and how a
iv generator should be written. I saw two examples of IV generators
too used with AEAD ciphers (crypto/seqiv.c and crypto/echainiv.c)

Excerpt from crypto api doc:
http://www.chronox.de/crypto-API/crypto/architecture.html#crypto-api-cipher-references-and-priority

2. Now, SEQIV uses the AEAD API function calls to invoke the associated
AEAD cipher. In our case, during the instantiation of SEQIV, the cipher
handle for GCM is provided to SEQIV. This means that SEQIV invokes
AEAD cipher operations with the GCM cipher handle.

Here, it says seqiv invokes cipher operations. However the code crypto/seqiv.c
does not look similar to how the modes are implemented which is confusing. I
was looking for an example of an IV generator used with a regular block cipher
and not a AEAD cipher. Could you point me out to some?

Thanks,
Binoy

Re: [RFC PATCH v2] crypto: Add IV generation algorithms

[Binoy Jayan]

Hi Milan,

Thank you for the reply.

On 13 December 2016 at 15:31, Milan Broz  wrote:

> I really do not think the disk encryption key management should be moved
> outside of dm-crypt. We cannot then change key structure later easily.

Yes, I agree. but the key selection based on sector number restricts the
option of having a larger block size used for encryption.

>> + unsigned int key_size;
>> + unsigned int key_extra_size;
>> + unsigned int key_parts;  /* independent parts in key buffer */
>
> ^^^ these key sizes you probably mean by key management.

Yes, I mean splitting the keys into subkeys based on the keycount
parameter (as mentioned below) to the dm-crypt.

cipher[:keycount]-mode-iv:ivopts
aes:2-cbc-essiv:sha256

> It is based on way how the key is currently sent into kernel
> (one hexa string in ioctl that needs to be split) and have to be changed in 
> future.

-Binoy

Re: [RFC PATCH v2] crypto: Add IV generation algorithms

[Binoy Jayan]

Hi Milan,

Thank you for the reply.

On 13 December 2016 at 15:31, Milan Broz  wrote:

> I really do not think the disk encryption key management should be moved
> outside of dm-crypt. We cannot then change key structure later easily.

Yes, I agree. but the key selection based on sector number restricts the
option of having a larger block size used for encryption.

>> + unsigned int key_size;
>> + unsigned int key_extra_size;
>> + unsigned int key_parts;  /* independent parts in key buffer */
>
> ^^^ these key sizes you probably mean by key management.

Yes, I mean splitting the keys into subkeys based on the keycount
parameter (as mentioned below) to the dm-crypt.

cipher[:keycount]-mode-iv:ivopts
aes:2-cbc-essiv:sha256

> It is based on way how the key is currently sent into kernel
> (one hexa string in ioctl that needs to be split) and have to be changed in 
> future.

-Binoy

[RFC PATCH v2] crypto: Add IV generation algorithms

[Binoy Jayan]

Currently, the iv generation algorithms are implemented in dm-crypt.c.
The goal is to move these algorithms from the dm layer to the kernel
crypto layer by implementing them as template ciphers so they can be
implemented in hardware for performance. As part of this patchset, the
iv-generation code is moved from the dm layer to the crypto layer and
adapt the dm-layer to send a whole 'bio' (as defined in the block layer)
at a time. Each bio contains the in memory representation of physically
contiguous disk blocks. The dm layer sets up a chained scatterlist of
these blocks split into physically contiguous segments in memory so that
DMA can be performed. The iv generation algorithms implemented in geniv.c
include plain, plain64, essiv, benbi, null, lmk and tcw.

When using multiple keys with the original dm-crypt, the key selection is
made based on the sector number as:

key_index = sector & (key_count - 1)

This restricts the usage of the same key for encrypting/decrypting a
single bio. One way to solve this is to move the key management code from
dm-crypt to cryto layer. But this seems tricky when using template ciphers
because, when multiple ciphers are instantiated from dm layer, each cipher
instance set with a unique subkey (part of the bigger master key) and
these instances themselves do not have access to each other's instances
or contexts. This way, a single instance cannot encryt/decrypt a whole bio.
This has to be fixed.

Not-signed-off-by: Binoy Jayan <binoy.ja...@linaro.org>
---
 crypto/Kconfig |   10 +
 crypto/Makefile|1 +
 crypto/geniv.c | 1294 
 drivers/md/dm-crypt.c  |  894 +
 include/crypto/geniv.h |   60 +++
 5 files changed, 1499 insertions(+), 760 deletions(-)
 create mode 100644 crypto/geniv.c
 create mode 100644 include/crypto/geniv.h

diff --git a/crypto/Kconfig b/crypto/Kconfig
index 84d7148..dc33a33 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -326,6 +326,16 @@ config CRYPTO_CTS
  This mode is required for Kerberos gss mechanism support
  for AES encryption.
 
+config CRYPTO_GENIV
+   tristate "IV Generation for dm-crypt"
+   select CRYPTO_BLKCIPHER
+   help
+ GENIV: IV Generation for dm-crypt
+ Algorithms to generate Initialization Vector for ciphers
+ used by dm-crypt.  The iv generation algorithms implemented
+ as part of geniv include plain, plain64, essiv, benbi, null,
+ lmk and tcw.
+
 config CRYPTO_ECB
tristate "ECB support"
select CRYPTO_BLKCIPHER
diff --git a/crypto/Makefile b/crypto/Makefile
index bd6a029..627ec76 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -75,6 +75,7 @@ obj-$(CONFIG_CRYPTO_TGR192) += tgr192.o
 obj-$(CONFIG_CRYPTO_GF128MUL) += gf128mul.o
 obj-$(CONFIG_CRYPTO_ECB) += ecb.o
 obj-$(CONFIG_CRYPTO_CBC) += cbc.o
+obj-$(CONFIG_CRYPTO_GENIV) += geniv.o
 obj-$(CONFIG_CRYPTO_PCBC) += pcbc.o
 obj-$(CONFIG_CRYPTO_CTS) += cts.o
 obj-$(CONFIG_CRYPTO_LRW) += lrw.o
diff --git a/crypto/geniv.c b/crypto/geniv.c
new file mode 100644
index 000..ac81a49
--- /dev/null
+++ b/crypto/geniv.c
@@ -0,0 +1,1294 @@
+/*
+ * geniv: IV generation algorithms
+ *
+ * Copyright (c) 2016, Linaro Ltd.
+ * Copyright (C) 2006-2015 Red Hat, Inc. All rights reserved.
+ * Copyright (C) 2013 Milan Broz <gmazyl...@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)
+ * any later version.
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+struct geniv_ctx;
+struct crypto_geniv_req_ctx;
+
+/* Sub request for each of the skcipher_request's for a segment */
+struct crypto_geniv_subreq {
+   struct skcipher_request req CRYPTO_MINALIGN_ATTR;
+   struct crypto_geniv_req_ctx *rctx;
+};
+
+struct crypto_geniv_req_ctx {
+   struct crypto_geniv_subreq *subreqs;
+   struct scatterlist *src;
+   struct scatterlist *dst;
+   bool is_write;
+   sector_t iv_sector;
+   unsigned int nents;
+   u8 *iv;
+   struct completion restart;
+   atomic_t req_pending;
+   struct skcipher_request *req;
+};
+
+struct geniv_operations {
+   int (*ctr)(struct geniv_ctx *ctx);
+   void (*dtr)(struct geniv_ctx *ctx);
+   int (*init)(struct geniv_ctx *ctx);
+   int (*wipe)(struct geniv_ctx *ctx);
+   int (*generator)(struct geniv_ctx *ctx,
+struct crypto_geniv_req_ctx *rctx, int n);
+   int (*post)(struct geniv_ctx *ctx,
+   struct crypto_geniv_req_ctx *rctx, int n);
+};
+
+struct geniv_essiv_p

[RFC PATCH v2] IV Generation algorithms for dm-crypt

[Binoy Jayan]

 number of tests with the aes cipher.
# The IV generation algorithms may also be tested with other ciphers as well.

cryptsetup luksDump --dump-master-key /dev/sdb

# create a luks volume and open the device
cryptsetup luksOpen /dev/sdb crypt_fun
dmsetup table --showkeys

# Write some data to the device
cat data.txt > /dev/mapper/crypt_fun

# Read 100 bytes back
dd if=/dev/mapper/crypt_fun of=out.txt bs=100 count=1
cat out.txt

mkfs.ext4 -j /dev/mapper/crypt_fun

# Mount if fs creation succeeds
mount -t ext4 /dev/mapper/crypt_fun /mnt

<-- Use the encrypted file system -->

umount /mnt
cryptsetup luksClose crypt_fun
cryptsetup luksRemoveKey /dev/sdb

This seems to work well. The file system mounts successfully and the files
written to in the file system remain persistent across reboots.

Binoy Jayan (1):
  crypto: Add IV generation algorithms

 crypto/Kconfig |   10 +
 crypto/Makefile|1 +
 crypto/geniv.c | 1294 
 drivers/md/dm-crypt.c  |  894 +
 include/crypto/geniv.h |   60 +++
 5 files changed, 1499 insertions(+), 760 deletions(-)
 create mode 100644 crypto/geniv.c
 create mode 100644 include/crypto/geniv.h

-- 
Binoy Jayan