[PATCH 0/6] crypto: img-hash - fixes and interface changes

[PATCH 0/6] crypto: img-hash - fixes and interface changes

[Will Thomas]

This patch set includes small stability fixes, power management
and import/export interface functions for the img-hash driver.

Govindraj Raja (1):
  crypto: img-hash - Add suspend resume hooks for img hash

James Hartley (2):
  crypto: img-hash - Add support for export and import
  crypto: img-hash - log a successful probe

Will Thomas (3):
  crypto: img-hash - Fix null pointer exception
  crypto: img-hash - Fix hash request context
  crypto: img-hash - Reconfigure DMA Burst length

 drivers/crypto/img-hash.c | 76 ++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 72 insertions(+), 4 deletions(-)

-- 
2.1.4

[PATCH 1/6] crypto: img-hash - Fix null pointer exception

[Will Thomas]

Sporadic null pointer exceptions came from here. Fix them.

Signed-off-by: Will Thomas <will.thomas@imgtec.com>
Reviewed-by: James Hartley <james.hartley@imgtec.com>
---
 drivers/crypto/img-hash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/img-hash.c b/drivers/crypto/img-hash.c
index 68e8aa9..a5f270b 100644
--- a/drivers/crypto/img-hash.c
+++ b/drivers/crypto/img-hash.c
@@ -361,7 +361,7 @@ static void img_hash_dma_task(unsigned long d)
 	size_t nbytes, bleft, wsend, len, tbc;
 	struct scatterlist tsg;
 
-	if (!ctx->sg)
+	if (!ctx || !ctx->sg)
 		return;
 
 	addr = sg_virt(ctx->sg);
-- 
2.1.4

Re: [PATCH 1/6] crypto: img-hash - Fix null pointer exception

[Herbert Xu]

Will Thomas <will.thomas@imgtec.com> wrote:
> Sporadic null pointer exceptions came from here. Fix them.
> 
> Signed-off-by: Will Thomas <will.thomas@imgtec.com>
> Reviewed-by: James Hartley <james.hartley@imgtec.com>
> ---
> drivers/crypto/img-hash.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/crypto/img-hash.c b/drivers/crypto/img-hash.c
> index 68e8aa9..a5f270b 100644
> --- a/drivers/crypto/img-hash.c
> +++ b/drivers/crypto/img-hash.c
> @@ -361,7 +361,7 @@ static void img_hash_dma_task(unsigned long d)
>        size_t nbytes, bleft, wsend, len, tbc;
>        struct scatterlist tsg;
> 
> -       if (!ctx->sg)
> +       if (!ctx || !ctx->sg)
>                return;

This makes no sense.  ctx comes from ahash_request_ctx which can
never be NULL.  Perhaps you meant !hdev->req?

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[PATCH 2/6] crypto: img-hash - Fix hash request context

[Will Thomas]

Move 0 length buffer to end of structure to stop overwriting
fallback request data. This doesn't cause a bug itself as the
buffer is never used alongside the fallback but should be
changed.

Signed-off-by: Will Thomas <will.thomas@imgtec.com>
Reviewed-by: James Hartley <james.hartley@imgtec.com>
---
 drivers/crypto/img-hash.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/img-hash.c b/drivers/crypto/img-hash.c
index a5f270b..7fa6aa4 100644
--- a/drivers/crypto/img-hash.c
+++ b/drivers/crypto/img-hash.c
@@ -102,8 +102,10 @@ struct img_hash_request_ctx {
 	unsigned long		op;
 
 	size_t			bufcnt;
-	u8 buffer[0] __aligned(sizeof(u32));
 	struct ahash_request	fallback_req;
+
+	/* Zero length buffer must remain last member of struct */
+	u8 buffer[0] __aligned(sizeof(u32));
 };
 
 struct img_hash_ctx {
-- 
2.1.4

Re: [PATCH 2/6] crypto: img-hash - Fix hash request context

[Herbert Xu]

Will Thomas <will.thomas@imgtec.com> wrote:
> Move 0 length buffer to end of structure to stop overwriting
> fallback request data. This doesn't cause a bug itself as the
> buffer is never used alongside the fallback but should be
> changed.
> 
> Signed-off-by: Will Thomas <will.thomas@imgtec.com>
> Reviewed-by: James Hartley <james.hartley@imgtec.com>

This points out another bug, the fallback can have its own ctx
following the request.

So the set_reqsize call needs to be adjusted to take that into
account.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 2/6] crypto: img-hash - Fix hash request context

[Will Thomas]

On 12/07/16 08:43, Herbert Xu wrote:
> Will Thomas <will.thomas@imgtec.com> wrote:
>> Move 0 length buffer to end of structure to stop overwriting
>> fallback request data. This doesn't cause a bug itself as the
>> buffer is never used alongside the fallback but should be
>> changed.
>>
>> Signed-off-by: Will Thomas <will.thomas@imgtec.com>
>> Reviewed-by: James Hartley <james.hartley@imgtec.com>
> 
> This points out another bug, the fallback can have its own ctx
> following the request.
> 
> So the set_reqsize call needs to be adjusted to take that into
> account.

Will be an additional patch in v2 to fix this.

Thanks,
Will

[PATCH 3/6] crypto: img-hash - Reconfigure DMA Burst length

[Will Thomas]

Burst length of 16 drives the hash accelerator out of spec
and causes stability issues in some cases. Reduce this to
stop data being lost.

Signed-off-by: Will Thomas <will.thomas@imgtec.com>
Reviewed-by: James Hartley <james.hartley@imgtec.com>
---
 drivers/crypto/img-hash.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/img-hash.c b/drivers/crypto/img-hash.c
index 7fa6aa4..fbed47a 100644
--- a/drivers/crypto/img-hash.c
+++ b/drivers/crypto/img-hash.c
@@ -71,6 +71,7 @@
 #define DRIVER_FLAGS_MD5		BIT(21)
 
 #define IMG_HASH_QUEUE_LENGTH		20
+#define IMG_HASH_DMA_BURST		4
 #define IMG_HASH_DMA_THRESHOLD		64
 
 #ifdef __LITTLE_ENDIAN
@@ -342,7 +343,7 @@ static int img_hash_dma_init(struct img_hash_dev *hdev)
 	dma_conf.direction = DMA_MEM_TO_DEV;
 	dma_conf.dst_addr = hdev->bus_addr;
 	dma_conf.dst_addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
-	dma_conf.dst_maxburst = 16;
+	dma_conf.dst_maxburst = IMG_HASH_DMA_BURST;
 	dma_conf.device_fc = false;
 
 	err = dmaengine_slave_config(hdev->dma_lch,  &dma_conf);
-- 
2.1.4

[PATCH 4/6] crypto: img-hash - Add suspend resume hooks for img hash

[Will Thomas]

From: Govindraj Raja <Govindraj.Raja@imgtec.com>

Current img hash claims sys and periph gate clocks
and this can be gated in system suspend scenarios.

Add support for Device pm ops for img hash to gate
the clocks claimed by img hash.

Signed-off-by: Govindraj Raja <Govindraj.Raja@imgtec.com>
Reviewed-by: Will Thomas <will.thomas@imgtec.com>
---
 drivers/crypto/img-hash.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/crypto/img-hash.c b/drivers/crypto/img-hash.c
index fbed47a..ed4408a 100644
--- a/drivers/crypto/img-hash.c
+++ b/drivers/crypto/img-hash.c
@@ -1016,11 +1016,38 @@ static int img_hash_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int img_hash_suspend(struct device *dev)
+{
+	struct img_hash_dev *hdev = dev_get_drvdata(dev);
+
+	clk_disable_unprepare(hdev->hash_clk);
+	clk_disable_unprepare(hdev->sys_clk);
+
+	return 0;
+}
+
+static int img_hash_resume(struct device *dev)
+{
+	struct img_hash_dev *hdev = dev_get_drvdata(dev);
+
+	clk_prepare_enable(hdev->hash_clk);
+	clk_prepare_enable(hdev->sys_clk);
+
+	return 0;
+}
+#endif /* CONFIG_PM_SLEEP */
+
+static const struct dev_pm_ops img_hash_pm_ops = {
+	SET_SYSTEM_SLEEP_PM_OPS(img_hash_suspend, img_hash_resume)
+};
+
 static struct platform_driver img_hash_driver = {
 	.probe		= img_hash_probe,
 	.remove		= img_hash_remove,
 	.driver		= {
 		.name	= "img-hash-accelerator",
+		.pm	= &img_hash_pm_ops,
 		.of_match_table	= of_match_ptr(img_hash_match),
 	}
 };
-- 
2.1.4

[PATCH 5/6] crypto: img-hash - Add support for export and import

[Will Thomas]

From: James Hartley <james.hartley@imgtec.com>

Currently the img-hash accelerator does not probe
successfully due to a change in the checks made during
registration with the crypto framework. This is due to
import and export functions not being defined. Correct
this.

Signed-off-by: James Hartley <james.hartley@imgtec.com>
Reviewed-by: Will Thomas <will.thomas@imgtec.com>
---
 drivers/crypto/img-hash.c | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/drivers/crypto/img-hash.c b/drivers/crypto/img-hash.c
index ed4408a..c2fdc35 100644
--- a/drivers/crypto/img-hash.c
+++ b/drivers/crypto/img-hash.c
@@ -590,6 +590,32 @@ static int img_hash_finup(struct ahash_request *req)
 	return crypto_ahash_finup(&rctx->fallback_req);
 }
 
+static int img_hash_import(struct ahash_request *req, const void *in)
+{
+	struct img_hash_request_ctx *rctx = ahash_request_ctx(req);
+	struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
+	struct img_hash_ctx *ctx = crypto_ahash_ctx(tfm);
+
+	ahash_request_set_tfm(&rctx->fallback_req, ctx->fallback);
+	rctx->fallback_req.base.flags = req->base.flags
+		& CRYPTO_TFM_REQ_MAY_SLEEP;
+
+	return crypto_ahash_import(&rctx->fallback_req, in);
+}
+
+static int img_hash_export(struct ahash_request *req, void *out)
+{
+	struct img_hash_request_ctx *rctx = ahash_request_ctx(req);
+	struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
+	struct img_hash_ctx *ctx = crypto_ahash_ctx(tfm);
+
+	ahash_request_set_tfm(&rctx->fallback_req, ctx->fallback);
+	rctx->fallback_req.base.flags = req->base.flags
+		& CRYPTO_TFM_REQ_MAY_SLEEP;
+
+	return crypto_ahash_export(&rctx->fallback_req, out);
+}
+
 static int img_hash_digest(struct ahash_request *req)
 {
 	struct crypto_ahash *tfm = crypto_ahash_reqtfm(req);
@@ -714,9 +740,12 @@ static struct ahash_alg img_algs[] = {
 		.update = img_hash_update,
 		.final = img_hash_final,
 		.finup = img_hash_finup,
+		.export = img_hash_export,
+		.import = img_hash_import,
 		.digest = img_hash_digest,
 		.halg = {
 			.digestsize = MD5_DIGEST_SIZE,
+			.statesize = sizeof(struct md5_state),
 			.base = {
 				.cra_name = "md5",
 				.cra_driver_name = "img-md5",
@@ -737,9 +766,12 @@ static struct ahash_alg img_algs[] = {
 		.update = img_hash_update,
 		.final = img_hash_final,
 		.finup = img_hash_finup,
+		.export = img_hash_export,
+		.import = img_hash_import,
 		.digest = img_hash_digest,
 		.halg = {
 			.digestsize = SHA1_DIGEST_SIZE,
+			.statesize = sizeof(struct sha1_state),
 			.base = {
 				.cra_name = "sha1",
 				.cra_driver_name = "img-sha1",
@@ -760,9 +792,12 @@ static struct ahash_alg img_algs[] = {
 		.update = img_hash_update,
 		.final = img_hash_final,
 		.finup = img_hash_finup,
+		.export = img_hash_export,
+		.import = img_hash_import,
 		.digest = img_hash_digest,
 		.halg = {
 			.digestsize = SHA224_DIGEST_SIZE,
+			.statesize = sizeof(struct sha256_state),
 			.base = {
 				.cra_name = "sha224",
 				.cra_driver_name = "img-sha224",
@@ -783,9 +818,12 @@ static struct ahash_alg img_algs[] = {
 		.update = img_hash_update,
 		.final = img_hash_final,
 		.finup = img_hash_finup,
+		.export = img_hash_export,
+		.import = img_hash_import,
 		.digest = img_hash_digest,
 		.halg = {
 			.digestsize = SHA256_DIGEST_SIZE,
+			.statesize = sizeof(struct sha256_state),
 			.base = {
 				.cra_name = "sha256",
 				.cra_driver_name = "img-sha256",
-- 
2.1.4

Re: [PATCH 5/6] crypto: img-hash - Add support for export and import

[Herbert Xu]

Will Thomas <will.thomas@imgtec.com> wrote:
>
> @@ -714,9 +740,12 @@ static struct ahash_alg img_algs[] = {
>                .update = img_hash_update,
>                .final = img_hash_final,
>                .finup = img_hash_finup,
> +               .export = img_hash_export,
> +               .import = img_hash_import,
>                .digest = img_hash_digest,
>                .halg = {
>                        .digestsize = MD5_DIGEST_SIZE,
> +                       .statesize = sizeof(struct md5_state),

This is wrong.  The fallback state size is not guaranteed to be
the same as the generic MD5.  I suppose the easiest fix is to
explicitly request for md5-generic/sha1-generic/etc. when you
allocate the fallback.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH 5/6] crypto: img-hash - Add support for export and import

[Will Thomas]

Hi Herbert,

I don't see any other drivers explicitly requesting a fallback
driver by name. Anyways, should this be done using  "sha1-generic"
in the crypto_alloc_ahash call when setting up the fallback tfm?

Thanks,
Will

On 12/07/16 08:49, Herbert Xu wrote:
> Will Thomas <will.thomas@imgtec.com> wrote:
>>
>> @@ -714,9 +740,12 @@ static struct ahash_alg img_algs[] = {
>>                .update = img_hash_update,
>>                .final = img_hash_final,
>>                .finup = img_hash_finup,
>> +               .export = img_hash_export,
>> +               .import = img_hash_import,
>>                .digest = img_hash_digest,
>>                .halg = {
>>                        .digestsize = MD5_DIGEST_SIZE,
>> +                       .statesize = sizeof(struct md5_state),
> 
> This is wrong.  The fallback state size is not guaranteed to be
> the same as the generic MD5.  I suppose the easiest fix is to
> explicitly request for md5-generic/sha1-generic/etc. when you
> allocate the fallback.
> 
> Thanks,
> 

Re: [PATCH 5/6] crypto: img-hash - Add support for export and import

[Herbert Xu]

On Wed, Jul 13, 2016 at 03:01:38PM +0100, Will Thomas wrote:
> 
> I don't see any other drivers explicitly requesting a fallback
> driver by name. Anyways, should this be done using  "sha1-generic"

The problem is that you're relying on the fallback to implement
import/export.  So if the fallback changes, then your algorithm
parameters will change which is not allowed.

> in the crypto_alloc_ahash call when setting up the fallback tfm?

Yes that should be sufficient.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

[PATCH 6/6] crypto: img-hash - log a successful probe

[Will Thomas]

From: James Hartley <james.hartley@imgtec.com>

Currently the probe function only emits an output on success
when debug is specifically enabled. It would be more useful
if this happens by default.

Signed-off-by: James Hartley <james.hartley@imgtec.com>
Reviewed-by: Will Thomas <will.thomas@imgtec.com>
---
 drivers/crypto/img-hash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/img-hash.c b/drivers/crypto/img-hash.c
index c2fdc35..f90fdf3 100644
--- a/drivers/crypto/img-hash.c
+++ b/drivers/crypto/img-hash.c
@@ -1012,7 +1012,7 @@ static int img_hash_probe(struct platform_device *pdev)
 	err = img_register_algs(hdev);
 	if (err)
 		goto err_algs;
-	dev_dbg(dev, "Img MD5/SHA1/SHA224/SHA256 Hardware accelerator initialized\n");
+	dev_info(dev, "Img MD5/SHA1/SHA224/SHA256 Hardware accelerator initialized\n");
 
 	return 0;
 
-- 
2.1.4