Linux cryptographic layer development
 help / color / mirror / Atom feed
* Re: [PATCH V4 02/15] asymmetric keys: implement EMSA_PKCS1-v1_5-ENCODE in rsa
From: joeyli @ 2013-09-18  9:08 UTC (permalink / raw)
  To: Dmitry Kasatkin
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA,
	linux-efi-u79uwXL29TY76Z2rM5mHXA, linux-pm-u79uwXL29TY76Z2rM5mHXA,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA,
	opensuse-kernel-stAJ6ESoqRxg9hUCZPvPmw, David Howells,
	Rafael J. Wysocki, Matthew Garrett, Len Brown, Pavel Machek,
	Josh Boyer, Vojtech Pavlik, Matt Fleming, James Bottomley,
	Greg KH, JKosina-IBi9RG/b67k, Rusty Russell, Herbert Xu,
	David S. Miller, H. Peter Anvin, Michal Marek, Gary Lin,
	Vivek Goyal
In-Reply-To: <CACE9dm-7HKz4VFR1bNTTFd-YpYhnkNVwiW81iXSJZbqjUTBR_Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hi Dmitry, 

First, thanks for your time to review my patches!

於 二,2013-09-17 於 16:51 -0500,Dmitry Kasatkin 提到:
> Hello,
> 
> 
> On Sat, Sep 14, 2013 at 7:56 PM, Lee, Chun-Yi <joeyli.kernel-Re5JQEeQqe8@public.gmane.orgm> wrote:
> > Implement EMSA_PKCS1-v1_5-ENCODE [RFC3447 sec 9.2] in rsa.c. It's the
> > first step of signature generation operation (RSASSA-PKCS1-v1_5-SIGN).
> >
> > This patch is temporary set emLen to pks->k, and temporary set EM to
> > pks->S for debugging. We will replace the above values to real signature
> > after implement RSASP1.
> >
> > The naming of EMSA_PKCS1_v1_5_ENCODE and the variables used in this function
> > accord PKCS#1 spec but not follow kernel naming convention, it useful when look
> > at them with spec.
> >
> > Reference: ftp://ftp.rsasecurity.com/pub/pkcs/pkcs-1v2/pkcs1ietffinal.txt
> > Reference: http://www.emc.com/collateral/white-papers/h11300-pkcs-1v2-2-rsa-cryptography-standard-wp.pdf
> >
> > V2:
> > - Clean up naming of variable: replace _EM by EM, replace EM by EM_tmp.
> > - Add comment to EMSA_PKCS1-v1_5-ENCODE function.
> >
> > Cc: Pavel Machek <pavel-+ZI9xUNit7I@public.gmane.org>
> > Reviewed-by: Jiri Kosina <jkosina-AlSwsSmVLrQ@public.gmane.org>
> > Signed-off-by: Lee, Chun-Yi <jlee-IBi9RG/b67k@public.gmane.org>
> > ---
> >  crypto/asymmetric_keys/rsa.c |  163 +++++++++++++++++++++++++++++++++++++++++-
> >  include/crypto/public_key.h  |    2 +
> >  2 files changed, 164 insertions(+), 1 deletions(-)
> >
> > diff --git a/crypto/asymmetric_keys/rsa.c b/crypto/asymmetric_keys/rsa.c
> > index 47f3be4..352ba45 100644
> > --- a/crypto/asymmetric_keys/rsa.c
> > +++ b/crypto/asymmetric_keys/rsa.c
> > @@ -13,6 +13,7 @@
> >  #include <linux/module.h>
> >  #include <linux/kernel.h>
> >  #include <linux/slab.h>
> > +#include <crypto/hash.h>
> >  #include "public_key.h"
> >  #include "private_key.h"
> >
> > @@ -152,6 +153,132 @@ static int RSA_I2OSP(MPI x, size_t xLen, u8 **_X)
> >  }
> >
> >  /*
> > + * EMSA_PKCS1-v1_5-ENCODE [RFC3447 sec 9.2]
> > + * @M: message to be signed, an octet string
> > + * @emLen: intended length in octets of the encoded message
> > + * @hash_algo: hash function (option)
> > + * @hash: true means hash M, otherwise M is already a digest
> > + * @EM: encoded message, an octet string of length emLen
> > + *
> > + * This function is a implementation of the EMSA-PKCS1-v1_5 encoding operation
> > + * in RSA PKCS#1 spec. It used by the signautre generation operation of
> > + * RSASSA-PKCS1-v1_5 to encode message M to encoded message EM.
> > + *
> > + * The variables used in this function accord PKCS#1 spec but not follow kernel
> > + * naming convention, it useful when look at them with spec.
> > + */
> > +static int EMSA_PKCS1_v1_5_ENCODE(const u8 *M, size_t emLen,
> > +               enum pkey_hash_algo hash_algo, const bool hash,
> > +               u8 **EM, struct public_key_signature *pks)
> > +{
> > +       u8 *digest;
> > +       struct crypto_shash *tfm;
> > +       struct shash_desc *desc;
> > +       size_t digest_size, desc_size;
> > +       size_t tLen;
> > +       u8 *T, *PS, *EM_tmp;
> > +       int i, ret;
> > +
> > +       pr_info("EMSA_PKCS1_v1_5_ENCODE start\n");
> > +
> > +       if (!RSA_ASN1_templates[hash_algo].data)
> 
> What about checking hash_algo against PKEY_HASH__LAST, or it relies on
> the caller?
> 

Yes, check PKEY_HASH__LAST is more easy and clear, I will change it.
Thanks!

> 
> > +               ret = -ENOTSUPP;
> > +       else
> > +               pks->pkey_hash_algo = hash_algo;
> > +
> > +       /* 1) Apply the hash function to the message M to produce a hash value H */
> > +       tfm = crypto_alloc_shash(pkey_hash_algo[hash_algo], 0, 0);
> > +       if (IS_ERR(tfm))
> > +               return (PTR_ERR(tfm) == -ENOENT) ? -ENOPKG : PTR_ERR(tfm);
> > +
> > +       desc_size = crypto_shash_descsize(tfm) + sizeof(*desc);
> > +       digest_size = crypto_shash_digestsize(tfm);
> > +
> > +       ret = -ENOMEM;
> > +
> > +       digest = kzalloc(digest_size + desc_size, GFP_KERNEL);
> > +       if (!digest)
> > +               goto error_digest;
> > +       pks->digest = digest;
> > +       pks->digest_size = digest_size;
> > +
> 
> Ok. You allocated tfm to get hash size, right?
> 
> But why do you allocate descriptor even it might not be needed?
> 

You are right, I should skip the code of allocate descriptor when the
hash is supported. I will modified it.
Thanks!

> > +       if (hash) {
> > +               desc = (void *) digest + digest_size;
> > +               desc->tfm = tfm;
> > +               desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
> > +
> > +               ret = crypto_shash_init(desc);
> > +               if (ret < 0)
> > +                       goto error_shash;
> > +               ret = crypto_shash_finup(desc, M, sizeof(M), pks->digest);
> 
> This is I completely fail to understand... You expect sizeof(M) to be
> the message length?????
> Have you ever tested it?
> 

Sigh!
I just checked my test program for this code path, this stupid problem
causes by my test program doesn't feed the right hash result that should
used to verify signature. So, I didn't capture this bug.

And, the hibernate signature check mechanism doesn't run into this code
path because the hash generation is done by hibernate code but not in
here. So, I also didn't find this problem when running hibernate check.

Appreciate for your point out! I just fix it in next patch version.

> > +               if (ret < 0)
> > +                       goto error_shash;
> > +       } else {
> > +               memcpy(pks->digest, M, pks->digest_size);
> > +               pks->digest_size = digest_size;
> > +       }
> 
> Does caller use pks->digest and pks->digest_size after return?
> I think it needs encoded value, not the hash...
> So why do you pass pks?
> 

I put the signature MPI to pks->rsa.s, and put the encoded signature to
pks->S. So caller can grab encoded signature.

I also pass the pks->digest and pks->digest_size to caller for
reference. Then caller can simply feed this pks to
RSA_verify_signature() for verify the signature result, don't need
generate hash again.

> 
> 
> > +       crypto_free_shash(tfm);
> > +
> > +       /* 2) Encode the algorithm ID for the hash function and the hash value into
> > +        * an ASN.1 value of type DigestInfo with the DER. Let T be the DER encoding of
> > +        * the DigestInfo value and let tLen be the length in octets of T.
> > +        */
> > +       tLen = RSA_ASN1_templates[hash_algo].size + pks->digest_size;
> > +       T = kmalloc(tLen, GFP_KERNEL);
> > +       if (!T)
> > +               goto error_T;
> > +
> 
> Why do you need T and PS memory allocations at all?
> You need only EM_tmp allocation and copy directly to the destination...
> 

OK, I will change the code to allocate T and PS size in EM_tmp.

> 
> > +       memcpy(T, RSA_ASN1_templates[hash_algo].data, RSA_ASN1_templates[hash_algo].size);
> > +       memcpy(T + RSA_ASN1_templates[hash_algo].size, pks->digest, pks->digest_size);
> > +
> > +       /* 3) check If emLen < tLen + 11, output "intended encoded message length too short" */
> > +       if (emLen < tLen + 11) {
> > +               ret = -EINVAL;
> > +               goto error_emLen;
> > +       }
> > +
> > +       /* 4) Generate an octet string PS consisting of emLen - tLen - 3 octets with 0xff. */
> > +       PS = kmalloc(emLen - tLen - 3, GFP_KERNEL);
> > +       if (!PS)
> > +               goto error_P;
> > +
> 
> ditto

OK, I will allocate PS with EM_tmp.
Thanks!

> 
> > +       for (i = 0; i < (emLen - tLen - 3); i++)
> > +               PS[i] = 0xff;
> > +
> > +       /* 5) Concatenate PS, the DER encoding T, and other padding to form the encoded
> > +        * message EM as EM = 0x00 || 0x01 || PS || 0x00 || T
> > +        */
> > +       EM_tmp = kmalloc(3 + emLen - tLen - 3 + tLen, GFP_KERNEL);
> > +       if (!EM_tmp)
> > +               goto error_EM;
> > +
> > +       EM_tmp[0] = 0x00;
> > +       EM_tmp[1] = 0x01;
> > +       memcpy(EM_tmp + 2, PS, emLen - tLen - 3);
> > +       EM_tmp[2 + emLen - tLen - 3] = 0x00;
> > +       memcpy(EM_tmp + 2 + emLen - tLen - 3 + 1, T, tLen);
> > +
> > +       *EM = EM_tmp;
> > +
> > +       kfree(PS);
> > +       kfree(T);
> 
> get rid of it...
> 

OK!

> 
> - Dmitry
> 
> > +
> > +       return 0;
> > +
> > +error_EM:
> > +       kfree(PS);
> > +error_P:
> > +error_emLen:
> > +       kfree(T);
> > +error_T:
> > +error_shash:
> > +       kfree(digest);
> > +error_digest:
> > +       crypto_free_shash(tfm);
> > +       return ret;
> > +}
> > +
> > +/*
> >   * Perform the RSA signature verification.
> >   * @H: Value of hash of data and metadata
> >   * @EM: The computed signature value
> > @@ -275,9 +402,43 @@ static struct public_key_signature *RSA_generate_signature(
> >                 const struct private_key *key, u8 *M,
> >                 enum pkey_hash_algo hash_algo, const bool hash)
> >  {
> > +       struct public_key_signature *pks;
> > +       u8 *EM = NULL;
> > +       size_t emLen;
> > +       int ret;
> > +
> >         pr_info("RSA_generate_signature start\n");
> >
> > -       return 0;
> > +       ret = -ENOMEM;
> > +       pks = kzalloc(sizeof(*pks), GFP_KERNEL);
> > +       if (!pks)
> > +               goto error_no_pks;
> > +
> > +       /* 1): EMSA-PKCS1-v1_5 encoding: */
> > +       /* Use the private key modulus size to be EM length */
> > +       emLen = mpi_get_nbits(key->rsa.n);
> > +       emLen = (emLen + 7) / 8;
> > +
> > +       ret = EMSA_PKCS1_v1_5_ENCODE(M, emLen, hash_algo, hash, &EM, pks);
> > +       if (ret < 0)
> > +               goto error_v1_5_encode;
> > +
> > +       /* TODO 2): m = OS2IP (EM) */
> > +
> > +       /* TODO 3): s = RSASP1 (K, m) */
> > +
> > +       /* TODO 4): S = I2OSP (s, k) */
> > +
> > +       /* TODO: signature S to a u8* S or set to sig->rsa.s? */
> > +       pks->S = EM;            /* TODO: temporary set S to EM */
> > +
> > +       return pks;
> > +
> > +error_v1_5_encode:
> > +       kfree(pks);
> > +error_no_pks:
> > +       pr_info("<==%s() = %d\n", __func__, ret);
> > +       return ERR_PTR(ret);
> >  }
> >
> >  const struct public_key_algorithm RSA_public_key_algorithm = {
> > diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
> > index d44b29f..1cdf457 100644
> > --- a/include/crypto/public_key.h
> > +++ b/include/crypto/public_key.h
> > @@ -110,6 +110,8 @@ extern void public_key_destroy(void *payload);
> >  struct public_key_signature {
> >         u8 *digest;
> >         u8 digest_size;                 /* Number of bytes in digest */
> > +       u8 *S;                          /* signature S of length k octets */
> > +       size_t k;                       /* length k of signature S */
> >         u8 nr_mpi;                      /* Occupancy of mpi[] */
> >         enum pkey_hash_algo pkey_hash_algo : 8;
> >         union {
> > --
> > 1.6.0.2
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
> > the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 

Thanks a lot!
Joey Lee

^ permalink raw reply

* Re: [PATCH V4 02/15] asymmetric keys: implement EMSA_PKCS1-v1_5-ENCODE in rsa
From: joeyli @ 2013-09-18  9:08 UTC (permalink / raw)
  To: Dmitry Kasatkin
  Cc: linux-kernel@vger.kernel.org, linux-security-module, linux-efi,
	linux-pm, linux-crypto, opensuse-kernel, David Howells,
	Rafael J. Wysocki, Matthew Garrett, Len Brown, Pavel Machek,
	Josh Boyer, Vojtech Pavlik, Matt Fleming, James Bottomley,
	Greg KH, JKosina, Rusty Russell, Herbert Xu, David S. Miller,
	H. Peter Anvin, Michal Marek, Gary Lin, Vivek Goyal
In-Reply-To: <CACE9dm-7HKz4VFR1bNTTFd-YpYhnkNVwiW81iXSJZbqjUTBR_Q@mail.gmail.com>

Hi Dmitry, 

First, thanks for your time to review my patches!

於 二,2013-09-17 於 16:51 -0500,Dmitry Kasatkin 提到:
> Hello,
> 
> 
> On Sat, Sep 14, 2013 at 7:56 PM, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote:
> > Implement EMSA_PKCS1-v1_5-ENCODE [RFC3447 sec 9.2] in rsa.c. It's the
> > first step of signature generation operation (RSASSA-PKCS1-v1_5-SIGN).
> >
> > This patch is temporary set emLen to pks->k, and temporary set EM to
> > pks->S for debugging. We will replace the above values to real signature
> > after implement RSASP1.
> >
> > The naming of EMSA_PKCS1_v1_5_ENCODE and the variables used in this function
> > accord PKCS#1 spec but not follow kernel naming convention, it useful when look
> > at them with spec.
> >
> > Reference: ftp://ftp.rsasecurity.com/pub/pkcs/pkcs-1v2/pkcs1ietffinal.txt
> > Reference: http://www.emc.com/collateral/white-papers/h11300-pkcs-1v2-2-rsa-cryptography-standard-wp.pdf
> >
> > V2:
> > - Clean up naming of variable: replace _EM by EM, replace EM by EM_tmp.
> > - Add comment to EMSA_PKCS1-v1_5-ENCODE function.
> >
> > Cc: Pavel Machek <pavel@ucw.cz>
> > Reviewed-by: Jiri Kosina <jkosina@suse.cz>
> > Signed-off-by: Lee, Chun-Yi <jlee@suse.com>
> > ---
> >  crypto/asymmetric_keys/rsa.c |  163 +++++++++++++++++++++++++++++++++++++++++-
> >  include/crypto/public_key.h  |    2 +
> >  2 files changed, 164 insertions(+), 1 deletions(-)
> >
> > diff --git a/crypto/asymmetric_keys/rsa.c b/crypto/asymmetric_keys/rsa.c
> > index 47f3be4..352ba45 100644
> > --- a/crypto/asymmetric_keys/rsa.c
> > +++ b/crypto/asymmetric_keys/rsa.c
> > @@ -13,6 +13,7 @@
> >  #include <linux/module.h>
> >  #include <linux/kernel.h>
> >  #include <linux/slab.h>
> > +#include <crypto/hash.h>
> >  #include "public_key.h"
> >  #include "private_key.h"
> >
> > @@ -152,6 +153,132 @@ static int RSA_I2OSP(MPI x, size_t xLen, u8 **_X)
> >  }
> >
> >  /*
> > + * EMSA_PKCS1-v1_5-ENCODE [RFC3447 sec 9.2]
> > + * @M: message to be signed, an octet string
> > + * @emLen: intended length in octets of the encoded message
> > + * @hash_algo: hash function (option)
> > + * @hash: true means hash M, otherwise M is already a digest
> > + * @EM: encoded message, an octet string of length emLen
> > + *
> > + * This function is a implementation of the EMSA-PKCS1-v1_5 encoding operation
> > + * in RSA PKCS#1 spec. It used by the signautre generation operation of
> > + * RSASSA-PKCS1-v1_5 to encode message M to encoded message EM.
> > + *
> > + * The variables used in this function accord PKCS#1 spec but not follow kernel
> > + * naming convention, it useful when look at them with spec.
> > + */
> > +static int EMSA_PKCS1_v1_5_ENCODE(const u8 *M, size_t emLen,
> > +               enum pkey_hash_algo hash_algo, const bool hash,
> > +               u8 **EM, struct public_key_signature *pks)
> > +{
> > +       u8 *digest;
> > +       struct crypto_shash *tfm;
> > +       struct shash_desc *desc;
> > +       size_t digest_size, desc_size;
> > +       size_t tLen;
> > +       u8 *T, *PS, *EM_tmp;
> > +       int i, ret;
> > +
> > +       pr_info("EMSA_PKCS1_v1_5_ENCODE start\n");
> > +
> > +       if (!RSA_ASN1_templates[hash_algo].data)
> 
> What about checking hash_algo against PKEY_HASH__LAST, or it relies on
> the caller?
> 

Yes, check PKEY_HASH__LAST is more easy and clear, I will change it.
Thanks!

> 
> > +               ret = -ENOTSUPP;
> > +       else
> > +               pks->pkey_hash_algo = hash_algo;
> > +
> > +       /* 1) Apply the hash function to the message M to produce a hash value H */
> > +       tfm = crypto_alloc_shash(pkey_hash_algo[hash_algo], 0, 0);
> > +       if (IS_ERR(tfm))
> > +               return (PTR_ERR(tfm) == -ENOENT) ? -ENOPKG : PTR_ERR(tfm);
> > +
> > +       desc_size = crypto_shash_descsize(tfm) + sizeof(*desc);
> > +       digest_size = crypto_shash_digestsize(tfm);
> > +
> > +       ret = -ENOMEM;
> > +
> > +       digest = kzalloc(digest_size + desc_size, GFP_KERNEL);
> > +       if (!digest)
> > +               goto error_digest;
> > +       pks->digest = digest;
> > +       pks->digest_size = digest_size;
> > +
> 
> Ok. You allocated tfm to get hash size, right?
> 
> But why do you allocate descriptor even it might not be needed?
> 

You are right, I should skip the code of allocate descriptor when the
hash is supported. I will modified it.
Thanks!

> > +       if (hash) {
> > +               desc = (void *) digest + digest_size;
> > +               desc->tfm = tfm;
> > +               desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
> > +
> > +               ret = crypto_shash_init(desc);
> > +               if (ret < 0)
> > +                       goto error_shash;
> > +               ret = crypto_shash_finup(desc, M, sizeof(M), pks->digest);
> 
> This is I completely fail to understand... You expect sizeof(M) to be
> the message length?????
> Have you ever tested it?
> 

Sigh!
I just checked my test program for this code path, this stupid problem
causes by my test program doesn't feed the right hash result that should
used to verify signature. So, I didn't capture this bug.

And, the hibernate signature check mechanism doesn't run into this code
path because the hash generation is done by hibernate code but not in
here. So, I also didn't find this problem when running hibernate check.

Appreciate for your point out! I just fix it in next patch version.

> > +               if (ret < 0)
> > +                       goto error_shash;
> > +       } else {
> > +               memcpy(pks->digest, M, pks->digest_size);
> > +               pks->digest_size = digest_size;
> > +       }
> 
> Does caller use pks->digest and pks->digest_size after return?
> I think it needs encoded value, not the hash...
> So why do you pass pks?
> 

I put the signature MPI to pks->rsa.s, and put the encoded signature to
pks->S. So caller can grab encoded signature.

I also pass the pks->digest and pks->digest_size to caller for
reference. Then caller can simply feed this pks to
RSA_verify_signature() for verify the signature result, don't need
generate hash again.

> 
> 
> > +       crypto_free_shash(tfm);
> > +
> > +       /* 2) Encode the algorithm ID for the hash function and the hash value into
> > +        * an ASN.1 value of type DigestInfo with the DER. Let T be the DER encoding of
> > +        * the DigestInfo value and let tLen be the length in octets of T.
> > +        */
> > +       tLen = RSA_ASN1_templates[hash_algo].size + pks->digest_size;
> > +       T = kmalloc(tLen, GFP_KERNEL);
> > +       if (!T)
> > +               goto error_T;
> > +
> 
> Why do you need T and PS memory allocations at all?
> You need only EM_tmp allocation and copy directly to the destination...
> 

OK, I will change the code to allocate T and PS size in EM_tmp.

> 
> > +       memcpy(T, RSA_ASN1_templates[hash_algo].data, RSA_ASN1_templates[hash_algo].size);
> > +       memcpy(T + RSA_ASN1_templates[hash_algo].size, pks->digest, pks->digest_size);
> > +
> > +       /* 3) check If emLen < tLen + 11, output "intended encoded message length too short" */
> > +       if (emLen < tLen + 11) {
> > +               ret = -EINVAL;
> > +               goto error_emLen;
> > +       }
> > +
> > +       /* 4) Generate an octet string PS consisting of emLen - tLen - 3 octets with 0xff. */
> > +       PS = kmalloc(emLen - tLen - 3, GFP_KERNEL);
> > +       if (!PS)
> > +               goto error_P;
> > +
> 
> ditto

OK, I will allocate PS with EM_tmp.
Thanks!

> 
> > +       for (i = 0; i < (emLen - tLen - 3); i++)
> > +               PS[i] = 0xff;
> > +
> > +       /* 5) Concatenate PS, the DER encoding T, and other padding to form the encoded
> > +        * message EM as EM = 0x00 || 0x01 || PS || 0x00 || T
> > +        */
> > +       EM_tmp = kmalloc(3 + emLen - tLen - 3 + tLen, GFP_KERNEL);
> > +       if (!EM_tmp)
> > +               goto error_EM;
> > +
> > +       EM_tmp[0] = 0x00;
> > +       EM_tmp[1] = 0x01;
> > +       memcpy(EM_tmp + 2, PS, emLen - tLen - 3);
> > +       EM_tmp[2 + emLen - tLen - 3] = 0x00;
> > +       memcpy(EM_tmp + 2 + emLen - tLen - 3 + 1, T, tLen);
> > +
> > +       *EM = EM_tmp;
> > +
> > +       kfree(PS);
> > +       kfree(T);
> 
> get rid of it...
> 

OK!

> 
> - Dmitry
> 
> > +
> > +       return 0;
> > +
> > +error_EM:
> > +       kfree(PS);
> > +error_P:
> > +error_emLen:
> > +       kfree(T);
> > +error_T:
> > +error_shash:
> > +       kfree(digest);
> > +error_digest:
> > +       crypto_free_shash(tfm);
> > +       return ret;
> > +}
> > +
> > +/*
> >   * Perform the RSA signature verification.
> >   * @H: Value of hash of data and metadata
> >   * @EM: The computed signature value
> > @@ -275,9 +402,43 @@ static struct public_key_signature *RSA_generate_signature(
> >                 const struct private_key *key, u8 *M,
> >                 enum pkey_hash_algo hash_algo, const bool hash)
> >  {
> > +       struct public_key_signature *pks;
> > +       u8 *EM = NULL;
> > +       size_t emLen;
> > +       int ret;
> > +
> >         pr_info("RSA_generate_signature start\n");
> >
> > -       return 0;
> > +       ret = -ENOMEM;
> > +       pks = kzalloc(sizeof(*pks), GFP_KERNEL);
> > +       if (!pks)
> > +               goto error_no_pks;
> > +
> > +       /* 1): EMSA-PKCS1-v1_5 encoding: */
> > +       /* Use the private key modulus size to be EM length */
> > +       emLen = mpi_get_nbits(key->rsa.n);
> > +       emLen = (emLen + 7) / 8;
> > +
> > +       ret = EMSA_PKCS1_v1_5_ENCODE(M, emLen, hash_algo, hash, &EM, pks);
> > +       if (ret < 0)
> > +               goto error_v1_5_encode;
> > +
> > +       /* TODO 2): m = OS2IP (EM) */
> > +
> > +       /* TODO 3): s = RSASP1 (K, m) */
> > +
> > +       /* TODO 4): S = I2OSP (s, k) */
> > +
> > +       /* TODO: signature S to a u8* S or set to sig->rsa.s? */
> > +       pks->S = EM;            /* TODO: temporary set S to EM */
> > +
> > +       return pks;
> > +
> > +error_v1_5_encode:
> > +       kfree(pks);
> > +error_no_pks:
> > +       pr_info("<==%s() = %d\n", __func__, ret);
> > +       return ERR_PTR(ret);
> >  }
> >
> >  const struct public_key_algorithm RSA_public_key_algorithm = {
> > diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
> > index d44b29f..1cdf457 100644
> > --- a/include/crypto/public_key.h
> > +++ b/include/crypto/public_key.h
> > @@ -110,6 +110,8 @@ extern void public_key_destroy(void *payload);
> >  struct public_key_signature {
> >         u8 *digest;
> >         u8 digest_size;                 /* Number of bytes in digest */
> > +       u8 *S;                          /* signature S of length k octets */
> > +       size_t k;                       /* length k of signature S */
> >         u8 nr_mpi;                      /* Occupancy of mpi[] */
> >         enum pkey_hash_algo pkey_hash_algo : 8;
> >         union {
> > --
> > 1.6.0.2
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 

Thanks a lot!
Joey Lee

^ permalink raw reply

* Re: [PATCH V4 02/15] asymmetric keys: implement EMSA_PKCS1-v1_5-ENCODE in rsa
From: joeyli @ 2013-09-18  9:08 UTC (permalink / raw)
  To: Dmitry Kasatkin
  Cc: linux-kernel@vger.kernel.org, linux-security-module, linux-efi,
	linux-pm, linux-crypto, opensuse-kernel, David Howells,
	Rafael J. Wysocki, Matthew Garrett, Len Brown, Pavel Machek,
	Josh Boyer, Vojtech Pavlik, Matt Fleming, James Bottomley,
	Greg KH, JKosina, Rusty Russell, Herbert Xu, David S. Miller,
	H. Peter Anvin, Michal Marek, Gary Lin, Vivek Goyal
In-Reply-To: <CACE9dm-7HKz4VFR1bNTTFd-YpYhnkNVwiW81iXSJZbqjUTBR_Q@mail.gmail.com>

Hi Dmitry, 

First, thanks for your time to review my patches!

於 二,2013-09-17 於 16:51 -0500,Dmitry Kasatkin 提到:
> Hello,
> 
> 
> On Sat, Sep 14, 2013 at 7:56 PM, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote:
> > Implement EMSA_PKCS1-v1_5-ENCODE [RFC3447 sec 9.2] in rsa.c. It's the
> > first step of signature generation operation (RSASSA-PKCS1-v1_5-SIGN).
> >
> > This patch is temporary set emLen to pks->k, and temporary set EM to
> > pks->S for debugging. We will replace the above values to real signature
> > after implement RSASP1.
> >
> > The naming of EMSA_PKCS1_v1_5_ENCODE and the variables used in this function
> > accord PKCS#1 spec but not follow kernel naming convention, it useful when look
> > at them with spec.
> >
> > Reference: ftp://ftp.rsasecurity.com/pub/pkcs/pkcs-1v2/pkcs1ietffinal.txt
> > Reference: http://www.emc.com/collateral/white-papers/h11300-pkcs-1v2-2-rsa-cryptography-standard-wp.pdf
> >
> > V2:
> > - Clean up naming of variable: replace _EM by EM, replace EM by EM_tmp.
> > - Add comment to EMSA_PKCS1-v1_5-ENCODE function.
> >
> > Cc: Pavel Machek <pavel@ucw.cz>
> > Reviewed-by: Jiri Kosina <jkosina@suse.cz>
> > Signed-off-by: Lee, Chun-Yi <jlee@suse.com>
> > ---
> >  crypto/asymmetric_keys/rsa.c |  163 +++++++++++++++++++++++++++++++++++++++++-
> >  include/crypto/public_key.h  |    2 +
> >  2 files changed, 164 insertions(+), 1 deletions(-)
> >
> > diff --git a/crypto/asymmetric_keys/rsa.c b/crypto/asymmetric_keys/rsa.c
> > index 47f3be4..352ba45 100644
> > --- a/crypto/asymmetric_keys/rsa.c
> > +++ b/crypto/asymmetric_keys/rsa.c
> > @@ -13,6 +13,7 @@
> >  #include <linux/module.h>
> >  #include <linux/kernel.h>
> >  #include <linux/slab.h>
> > +#include <crypto/hash.h>
> >  #include "public_key.h"
> >  #include "private_key.h"
> >
> > @@ -152,6 +153,132 @@ static int RSA_I2OSP(MPI x, size_t xLen, u8 **_X)
> >  }
> >
> >  /*
> > + * EMSA_PKCS1-v1_5-ENCODE [RFC3447 sec 9.2]
> > + * @M: message to be signed, an octet string
> > + * @emLen: intended length in octets of the encoded message
> > + * @hash_algo: hash function (option)
> > + * @hash: true means hash M, otherwise M is already a digest
> > + * @EM: encoded message, an octet string of length emLen
> > + *
> > + * This function is a implementation of the EMSA-PKCS1-v1_5 encoding operation
> > + * in RSA PKCS#1 spec. It used by the signautre generation operation of
> > + * RSASSA-PKCS1-v1_5 to encode message M to encoded message EM.
> > + *
> > + * The variables used in this function accord PKCS#1 spec but not follow kernel
> > + * naming convention, it useful when look at them with spec.
> > + */
> > +static int EMSA_PKCS1_v1_5_ENCODE(const u8 *M, size_t emLen,
> > +               enum pkey_hash_algo hash_algo, const bool hash,
> > +               u8 **EM, struct public_key_signature *pks)
> > +{
> > +       u8 *digest;
> > +       struct crypto_shash *tfm;
> > +       struct shash_desc *desc;
> > +       size_t digest_size, desc_size;
> > +       size_t tLen;
> > +       u8 *T, *PS, *EM_tmp;
> > +       int i, ret;
> > +
> > +       pr_info("EMSA_PKCS1_v1_5_ENCODE start\n");
> > +
> > +       if (!RSA_ASN1_templates[hash_algo].data)
> 
> What about checking hash_algo against PKEY_HASH__LAST, or it relies on
> the caller?
> 

Yes, check PKEY_HASH__LAST is more easy and clear, I will change it.
Thanks!

> 
> > +               ret = -ENOTSUPP;
> > +       else
> > +               pks->pkey_hash_algo = hash_algo;
> > +
> > +       /* 1) Apply the hash function to the message M to produce a hash value H */
> > +       tfm = crypto_alloc_shash(pkey_hash_algo[hash_algo], 0, 0);
> > +       if (IS_ERR(tfm))
> > +               return (PTR_ERR(tfm) == -ENOENT) ? -ENOPKG : PTR_ERR(tfm);
> > +
> > +       desc_size = crypto_shash_descsize(tfm) + sizeof(*desc);
> > +       digest_size = crypto_shash_digestsize(tfm);
> > +
> > +       ret = -ENOMEM;
> > +
> > +       digest = kzalloc(digest_size + desc_size, GFP_KERNEL);
> > +       if (!digest)
> > +               goto error_digest;
> > +       pks->digest = digest;
> > +       pks->digest_size = digest_size;
> > +
> 
> Ok. You allocated tfm to get hash size, right?
> 
> But why do you allocate descriptor even it might not be needed?
> 

You are right, I should skip the code of allocate descriptor when the
hash is supported. I will modified it.
Thanks!

> > +       if (hash) {
> > +               desc = (void *) digest + digest_size;
> > +               desc->tfm = tfm;
> > +               desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
> > +
> > +               ret = crypto_shash_init(desc);
> > +               if (ret < 0)
> > +                       goto error_shash;
> > +               ret = crypto_shash_finup(desc, M, sizeof(M), pks->digest);
> 
> This is I completely fail to understand... You expect sizeof(M) to be
> the message length?????
> Have you ever tested it?
> 

Sigh!
I just checked my test program for this code path, this stupid problem
causes by my test program doesn't feed the right hash result that should
used to verify signature. So, I didn't capture this bug.

And, the hibernate signature check mechanism doesn't run into this code
path because the hash generation is done by hibernate code but not in
here. So, I also didn't find this problem when running hibernate check.

Appreciate for your point out! I just fix it in next patch version.

> > +               if (ret < 0)
> > +                       goto error_shash;
> > +       } else {
> > +               memcpy(pks->digest, M, pks->digest_size);
> > +               pks->digest_size = digest_size;
> > +       }
> 
> Does caller use pks->digest and pks->digest_size after return?
> I think it needs encoded value, not the hash...
> So why do you pass pks?
> 

I put the signature MPI to pks->rsa.s, and put the encoded signature to
pks->S. So caller can grab encoded signature.

I also pass the pks->digest and pks->digest_size to caller for
reference. Then caller can simply feed this pks to
RSA_verify_signature() for verify the signature result, don't need
generate hash again.

> 
> 
> > +       crypto_free_shash(tfm);
> > +
> > +       /* 2) Encode the algorithm ID for the hash function and the hash value into
> > +        * an ASN.1 value of type DigestInfo with the DER. Let T be the DER encoding of
> > +        * the DigestInfo value and let tLen be the length in octets of T.
> > +        */
> > +       tLen = RSA_ASN1_templates[hash_algo].size + pks->digest_size;
> > +       T = kmalloc(tLen, GFP_KERNEL);
> > +       if (!T)
> > +               goto error_T;
> > +
> 
> Why do you need T and PS memory allocations at all?
> You need only EM_tmp allocation and copy directly to the destination...
> 

OK, I will change the code to allocate T and PS size in EM_tmp.

> 
> > +       memcpy(T, RSA_ASN1_templates[hash_algo].data, RSA_ASN1_templates[hash_algo].size);
> > +       memcpy(T + RSA_ASN1_templates[hash_algo].size, pks->digest, pks->digest_size);
> > +
> > +       /* 3) check If emLen < tLen + 11, output "intended encoded message length too short" */
> > +       if (emLen < tLen + 11) {
> > +               ret = -EINVAL;
> > +               goto error_emLen;
> > +       }
> > +
> > +       /* 4) Generate an octet string PS consisting of emLen - tLen - 3 octets with 0xff. */
> > +       PS = kmalloc(emLen - tLen - 3, GFP_KERNEL);
> > +       if (!PS)
> > +               goto error_P;
> > +
> 
> ditto

OK, I will allocate PS with EM_tmp.
Thanks!

> 
> > +       for (i = 0; i < (emLen - tLen - 3); i++)
> > +               PS[i] = 0xff;
> > +
> > +       /* 5) Concatenate PS, the DER encoding T, and other padding to form the encoded
> > +        * message EM as EM = 0x00 || 0x01 || PS || 0x00 || T
> > +        */
> > +       EM_tmp = kmalloc(3 + emLen - tLen - 3 + tLen, GFP_KERNEL);
> > +       if (!EM_tmp)
> > +               goto error_EM;
> > +
> > +       EM_tmp[0] = 0x00;
> > +       EM_tmp[1] = 0x01;
> > +       memcpy(EM_tmp + 2, PS, emLen - tLen - 3);
> > +       EM_tmp[2 + emLen - tLen - 3] = 0x00;
> > +       memcpy(EM_tmp + 2 + emLen - tLen - 3 + 1, T, tLen);
> > +
> > +       *EM = EM_tmp;
> > +
> > +       kfree(PS);
> > +       kfree(T);
> 
> get rid of it...
> 

OK!

> 
> - Dmitry
> 
> > +
> > +       return 0;
> > +
> > +error_EM:
> > +       kfree(PS);
> > +error_P:
> > +error_emLen:
> > +       kfree(T);
> > +error_T:
> > +error_shash:
> > +       kfree(digest);
> > +error_digest:
> > +       crypto_free_shash(tfm);
> > +       return ret;
> > +}
> > +
> > +/*
> >   * Perform the RSA signature verification.
> >   * @H: Value of hash of data and metadata
> >   * @EM: The computed signature value
> > @@ -275,9 +402,43 @@ static struct public_key_signature *RSA_generate_signature(
> >                 const struct private_key *key, u8 *M,
> >                 enum pkey_hash_algo hash_algo, const bool hash)
> >  {
> > +       struct public_key_signature *pks;
> > +       u8 *EM = NULL;
> > +       size_t emLen;
> > +       int ret;
> > +
> >         pr_info("RSA_generate_signature start\n");
> >
> > -       return 0;
> > +       ret = -ENOMEM;
> > +       pks = kzalloc(sizeof(*pks), GFP_KERNEL);
> > +       if (!pks)
> > +               goto error_no_pks;
> > +
> > +       /* 1): EMSA-PKCS1-v1_5 encoding: */
> > +       /* Use the private key modulus size to be EM length */
> > +       emLen = mpi_get_nbits(key->rsa.n);
> > +       emLen = (emLen + 7) / 8;
> > +
> > +       ret = EMSA_PKCS1_v1_5_ENCODE(M, emLen, hash_algo, hash, &EM, pks);
> > +       if (ret < 0)
> > +               goto error_v1_5_encode;
> > +
> > +       /* TODO 2): m = OS2IP (EM) */
> > +
> > +       /* TODO 3): s = RSASP1 (K, m) */
> > +
> > +       /* TODO 4): S = I2OSP (s, k) */
> > +
> > +       /* TODO: signature S to a u8* S or set to sig->rsa.s? */
> > +       pks->S = EM;            /* TODO: temporary set S to EM */
> > +
> > +       return pks;
> > +
> > +error_v1_5_encode:
> > +       kfree(pks);
> > +error_no_pks:
> > +       pr_info("<==%s() = %d\n", __func__, ret);
> > +       return ERR_PTR(ret);
> >  }
> >
> >  const struct public_key_algorithm RSA_public_key_algorithm = {
> > diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
> > index d44b29f..1cdf457 100644
> > --- a/include/crypto/public_key.h
> > +++ b/include/crypto/public_key.h
> > @@ -110,6 +110,8 @@ extern void public_key_destroy(void *payload);
> >  struct public_key_signature {
> >         u8 *digest;
> >         u8 digest_size;                 /* Number of bytes in digest */
> > +       u8 *S;                          /* signature S of length k octets */
> > +       size_t k;                       /* length k of signature S */
> >         u8 nr_mpi;                      /* Occupancy of mpi[] */
> >         enum pkey_hash_algo pkey_hash_algo : 8;
> >         union {
> > --
> > 1.6.0.2
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 

Thanks a lot!
Joey Lee


^ permalink raw reply

* Re: [PATCH V4 02/15] asymmetric keys: implement EMSA_PKCS1-v1_5-ENCODE in rsa
From: joeyli @ 2013-09-18  9:08 UTC (permalink / raw)
  To: Dmitry Kasatkin
  Cc: linux-kernel@vger.kernel.org, linux-security-module, linux-efi,
	linux-pm, linux-crypto, opensuse-kernel, David Howells,
	Rafael J. Wysocki, Matthew Garrett, Len Brown, Pavel Machek,
	Josh Boyer, Vojtech Pavlik, Matt Fleming, James Bottomley,
	Greg KH, JKosina, Rusty Russell, Herbert Xu, David S. Miller,
	H. Peter Anvin, Michal Marek, Gary Lin, Vivek Goyal
In-Reply-To: <CACE9dm-7HKz4VFR1bNTTFd-YpYhnkNVwiW81iXSJZbqjUTBR_Q@mail.gmail.com>

Hi Dmitry, 

First, thanks for your time to review my patches!

於 二,2013-09-17 於 16:51 -0500,Dmitry Kasatkin 提到:
> Hello,
> 
> 
> On Sat, Sep 14, 2013 at 7:56 PM, Lee, Chun-Yi <joeyli.kernel@gmail.com> wrote:
> > Implement EMSA_PKCS1-v1_5-ENCODE [RFC3447 sec 9.2] in rsa.c. It's the
> > first step of signature generation operation (RSASSA-PKCS1-v1_5-SIGN).
> >
> > This patch is temporary set emLen to pks->k, and temporary set EM to
> > pks->S for debugging. We will replace the above values to real signature
> > after implement RSASP1.
> >
> > The naming of EMSA_PKCS1_v1_5_ENCODE and the variables used in this function
> > accord PKCS#1 spec but not follow kernel naming convention, it useful when look
> > at them with spec.
> >
> > Reference: ftp://ftp.rsasecurity.com/pub/pkcs/pkcs-1v2/pkcs1ietffinal.txt
> > Reference: http://www.emc.com/collateral/white-papers/h11300-pkcs-1v2-2-rsa-cryptography-standard-wp.pdf
> >
> > V2:
> > - Clean up naming of variable: replace _EM by EM, replace EM by EM_tmp.
> > - Add comment to EMSA_PKCS1-v1_5-ENCODE function.
> >
> > Cc: Pavel Machek <pavel@ucw.cz>
> > Reviewed-by: Jiri Kosina <jkosina@suse.cz>
> > Signed-off-by: Lee, Chun-Yi <jlee@suse.com>
> > ---
> >  crypto/asymmetric_keys/rsa.c |  163 +++++++++++++++++++++++++++++++++++++++++-
> >  include/crypto/public_key.h  |    2 +
> >  2 files changed, 164 insertions(+), 1 deletions(-)
> >
> > diff --git a/crypto/asymmetric_keys/rsa.c b/crypto/asymmetric_keys/rsa.c
> > index 47f3be4..352ba45 100644
> > --- a/crypto/asymmetric_keys/rsa.c
> > +++ b/crypto/asymmetric_keys/rsa.c
> > @@ -13,6 +13,7 @@
> >  #include <linux/module.h>
> >  #include <linux/kernel.h>
> >  #include <linux/slab.h>
> > +#include <crypto/hash.h>
> >  #include "public_key.h"
> >  #include "private_key.h"
> >
> > @@ -152,6 +153,132 @@ static int RSA_I2OSP(MPI x, size_t xLen, u8 **_X)
> >  }
> >
> >  /*
> > + * EMSA_PKCS1-v1_5-ENCODE [RFC3447 sec 9.2]
> > + * @M: message to be signed, an octet string
> > + * @emLen: intended length in octets of the encoded message
> > + * @hash_algo: hash function (option)
> > + * @hash: true means hash M, otherwise M is already a digest
> > + * @EM: encoded message, an octet string of length emLen
> > + *
> > + * This function is a implementation of the EMSA-PKCS1-v1_5 encoding operation
> > + * in RSA PKCS#1 spec. It used by the signautre generation operation of
> > + * RSASSA-PKCS1-v1_5 to encode message M to encoded message EM.
> > + *
> > + * The variables used in this function accord PKCS#1 spec but not follow kernel
> > + * naming convention, it useful when look at them with spec.
> > + */
> > +static int EMSA_PKCS1_v1_5_ENCODE(const u8 *M, size_t emLen,
> > +               enum pkey_hash_algo hash_algo, const bool hash,
> > +               u8 **EM, struct public_key_signature *pks)
> > +{
> > +       u8 *digest;
> > +       struct crypto_shash *tfm;
> > +       struct shash_desc *desc;
> > +       size_t digest_size, desc_size;
> > +       size_t tLen;
> > +       u8 *T, *PS, *EM_tmp;
> > +       int i, ret;
> > +
> > +       pr_info("EMSA_PKCS1_v1_5_ENCODE start\n");
> > +
> > +       if (!RSA_ASN1_templates[hash_algo].data)
> 
> What about checking hash_algo against PKEY_HASH__LAST, or it relies on
> the caller?
> 

Yes, check PKEY_HASH__LAST is more easy and clear, I will change it.
Thanks!

> 
> > +               ret = -ENOTSUPP;
> > +       else
> > +               pks->pkey_hash_algo = hash_algo;
> > +
> > +       /* 1) Apply the hash function to the message M to produce a hash value H */
> > +       tfm = crypto_alloc_shash(pkey_hash_algo[hash_algo], 0, 0);
> > +       if (IS_ERR(tfm))
> > +               return (PTR_ERR(tfm) == -ENOENT) ? -ENOPKG : PTR_ERR(tfm);
> > +
> > +       desc_size = crypto_shash_descsize(tfm) + sizeof(*desc);
> > +       digest_size = crypto_shash_digestsize(tfm);
> > +
> > +       ret = -ENOMEM;
> > +
> > +       digest = kzalloc(digest_size + desc_size, GFP_KERNEL);
> > +       if (!digest)
> > +               goto error_digest;
> > +       pks->digest = digest;
> > +       pks->digest_size = digest_size;
> > +
> 
> Ok. You allocated tfm to get hash size, right?
> 
> But why do you allocate descriptor even it might not be needed?
> 

You are right, I should skip the code of allocate descriptor when the
hash is supported. I will modified it.
Thanks!

> > +       if (hash) {
> > +               desc = (void *) digest + digest_size;
> > +               desc->tfm = tfm;
> > +               desc->flags = CRYPTO_TFM_REQ_MAY_SLEEP;
> > +
> > +               ret = crypto_shash_init(desc);
> > +               if (ret < 0)
> > +                       goto error_shash;
> > +               ret = crypto_shash_finup(desc, M, sizeof(M), pks->digest);
> 
> This is I completely fail to understand... You expect sizeof(M) to be
> the message length?????
> Have you ever tested it?
> 

Sigh!
I just checked my test program for this code path, this stupid problem
causes by my test program doesn't feed the right hash result that should
used to verify signature. So, I didn't capture this bug.

And, the hibernate signature check mechanism doesn't run into this code
path because the hash generation is done by hibernate code but not in
here. So, I also didn't find this problem when running hibernate check.

Appreciate for your point out! I just fix it in next patch version.

> > +               if (ret < 0)
> > +                       goto error_shash;
> > +       } else {
> > +               memcpy(pks->digest, M, pks->digest_size);
> > +               pks->digest_size = digest_size;
> > +       }
> 
> Does caller use pks->digest and pks->digest_size after return?
> I think it needs encoded value, not the hash...
> So why do you pass pks?
> 

I put the signature MPI to pks->rsa.s, and put the encoded signature to
pks->S. So caller can grab encoded signature.

I also pass the pks->digest and pks->digest_size to caller for
reference. Then caller can simply feed this pks to
RSA_verify_signature() for verify the signature result, don't need
generate hash again.

> 
> 
> > +       crypto_free_shash(tfm);
> > +
> > +       /* 2) Encode the algorithm ID for the hash function and the hash value into
> > +        * an ASN.1 value of type DigestInfo with the DER. Let T be the DER encoding of
> > +        * the DigestInfo value and let tLen be the length in octets of T.
> > +        */
> > +       tLen = RSA_ASN1_templates[hash_algo].size + pks->digest_size;
> > +       T = kmalloc(tLen, GFP_KERNEL);
> > +       if (!T)
> > +               goto error_T;
> > +
> 
> Why do you need T and PS memory allocations at all?
> You need only EM_tmp allocation and copy directly to the destination...
> 

OK, I will change the code to allocate T and PS size in EM_tmp.

> 
> > +       memcpy(T, RSA_ASN1_templates[hash_algo].data, RSA_ASN1_templates[hash_algo].size);
> > +       memcpy(T + RSA_ASN1_templates[hash_algo].size, pks->digest, pks->digest_size);
> > +
> > +       /* 3) check If emLen < tLen + 11, output "intended encoded message length too short" */
> > +       if (emLen < tLen + 11) {
> > +               ret = -EINVAL;
> > +               goto error_emLen;
> > +       }
> > +
> > +       /* 4) Generate an octet string PS consisting of emLen - tLen - 3 octets with 0xff. */
> > +       PS = kmalloc(emLen - tLen - 3, GFP_KERNEL);
> > +       if (!PS)
> > +               goto error_P;
> > +
> 
> ditto

OK, I will allocate PS with EM_tmp.
Thanks!

> 
> > +       for (i = 0; i < (emLen - tLen - 3); i++)
> > +               PS[i] = 0xff;
> > +
> > +       /* 5) Concatenate PS, the DER encoding T, and other padding to form the encoded
> > +        * message EM as EM = 0x00 || 0x01 || PS || 0x00 || T
> > +        */
> > +       EM_tmp = kmalloc(3 + emLen - tLen - 3 + tLen, GFP_KERNEL);
> > +       if (!EM_tmp)
> > +               goto error_EM;
> > +
> > +       EM_tmp[0] = 0x00;
> > +       EM_tmp[1] = 0x01;
> > +       memcpy(EM_tmp + 2, PS, emLen - tLen - 3);
> > +       EM_tmp[2 + emLen - tLen - 3] = 0x00;
> > +       memcpy(EM_tmp + 2 + emLen - tLen - 3 + 1, T, tLen);
> > +
> > +       *EM = EM_tmp;
> > +
> > +       kfree(PS);
> > +       kfree(T);
> 
> get rid of it...
> 

OK!

> 
> - Dmitry
> 
> > +
> > +       return 0;
> > +
> > +error_EM:
> > +       kfree(PS);
> > +error_P:
> > +error_emLen:
> > +       kfree(T);
> > +error_T:
> > +error_shash:
> > +       kfree(digest);
> > +error_digest:
> > +       crypto_free_shash(tfm);
> > +       return ret;
> > +}
> > +
> > +/*
> >   * Perform the RSA signature verification.
> >   * @H: Value of hash of data and metadata
> >   * @EM: The computed signature value
> > @@ -275,9 +402,43 @@ static struct public_key_signature *RSA_generate_signature(
> >                 const struct private_key *key, u8 *M,
> >                 enum pkey_hash_algo hash_algo, const bool hash)
> >  {
> > +       struct public_key_signature *pks;
> > +       u8 *EM = NULL;
> > +       size_t emLen;
> > +       int ret;
> > +
> >         pr_info("RSA_generate_signature start\n");
> >
> > -       return 0;
> > +       ret = -ENOMEM;
> > +       pks = kzalloc(sizeof(*pks), GFP_KERNEL);
> > +       if (!pks)
> > +               goto error_no_pks;
> > +
> > +       /* 1): EMSA-PKCS1-v1_5 encoding: */
> > +       /* Use the private key modulus size to be EM length */
> > +       emLen = mpi_get_nbits(key->rsa.n);
> > +       emLen = (emLen + 7) / 8;
> > +
> > +       ret = EMSA_PKCS1_v1_5_ENCODE(M, emLen, hash_algo, hash, &EM, pks);
> > +       if (ret < 0)
> > +               goto error_v1_5_encode;
> > +
> > +       /* TODO 2): m = OS2IP (EM) */
> > +
> > +       /* TODO 3): s = RSASP1 (K, m) */
> > +
> > +       /* TODO 4): S = I2OSP (s, k) */
> > +
> > +       /* TODO: signature S to a u8* S or set to sig->rsa.s? */
> > +       pks->S = EM;            /* TODO: temporary set S to EM */
> > +
> > +       return pks;
> > +
> > +error_v1_5_encode:
> > +       kfree(pks);
> > +error_no_pks:
> > +       pr_info("<==%s() = %d\n", __func__, ret);
> > +       return ERR_PTR(ret);
> >  }
> >
> >  const struct public_key_algorithm RSA_public_key_algorithm = {
> > diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
> > index d44b29f..1cdf457 100644
> > --- a/include/crypto/public_key.h
> > +++ b/include/crypto/public_key.h
> > @@ -110,6 +110,8 @@ extern void public_key_destroy(void *payload);
> >  struct public_key_signature {
> >         u8 *digest;
> >         u8 digest_size;                 /* Number of bytes in digest */
> > +       u8 *S;                          /* signature S of length k octets */
> > +       size_t k;                       /* length k of signature S */
> >         u8 nr_mpi;                      /* Occupancy of mpi[] */
> >         enum pkey_hash_algo pkey_hash_algo : 8;
> >         union {
> > --
> > 1.6.0.2
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 
> 

Thanks a lot!
Joey Lee

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

^ permalink raw reply

* Re: [PATCH V4 13/15] Hibernate: introduced SNAPSHOT_SIG_HASH config for select hash algorithm
From: Pavel Machek @ 2013-09-18 13:45 UTC (permalink / raw)
  To: Lee, Chun-Yi
  Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	linux-security-module-u79uwXL29TY76Z2rM5mHXA,
	linux-efi-u79uwXL29TY76Z2rM5mHXA, linux-pm-u79uwXL29TY76Z2rM5mHXA,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA,
	opensuse-kernel-stAJ6ESoqRxg9hUCZPvPmw, David Howells,
	Rafael J. Wysocki, Matthew Garrett, Len Brown, Josh Boyer,
	Vojtech Pavlik, Matt Fleming, James Bottomley, Greg KH,
	JKosina-IBi9RG/b67k, Rusty Russell, Herbert Xu, David S. Miller,
	H. Peter Anvin, Michal Marek, Gary Lin, Vivek Goyal, Lee, Chun-Yi
In-Reply-To: <1379206621-18639-14-git-send-email-jlee-IBi9RG/b67k@public.gmane.org>

On Sun 2013-09-15 08:56:59, Lee, Chun-Yi wrote:
> This patch introduced SNAPSHOT_SIG_HASH config for user to select which
> hash algorithm will be used during signature generation of snapshot.

This series is big enough already... and who is going to test it?
There's no need to make hash configurable. Just select one that works.
	
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply

* Re: [PATCH] crypto_mem_not_equal: add constant-time equality testing of memory regions
From: James Yonan @ 2013-09-19  0:13 UTC (permalink / raw)
  To: Daniel Borkmann; +Cc: Florian Weimer, Marcelo Cerri, linux-crypto, herbert
In-Reply-To: <5238A860.2010404@redhat.com>

On 17/09/2013 13:07, Daniel Borkmann wrote:
> On 09/16/2013 07:10 PM, James Yonan wrote:
>> On 16/09/2013 01:56, Daniel Borkmann wrote:
>>> On 09/15/2013 06:59 PM, James Yonan wrote:
>>>> On 15/09/2013 09:45, Florian Weimer wrote:
>>>>> * James Yonan:
>>>>>
>>>>>> + * Constant-time equality testing of memory regions.
>>>>>> + * Returns 0 when data is equal, non-zero otherwise.
>>>>>> + * Fast path if size == 16.
>>>>>> + */
>>>>>> +noinline unsigned long crypto_mem_not_equal(const void *a, const
>>>>>> void *b, size_t size)
>>>>>
>>>>> I think this should really return unsigned or int, to reduce the risk
>>>>> that the upper bytes are truncated because the caller uses an
>>>>> inappropriate type, resulting in a bogus zero result.  Reducing the
>>>>> value to 0/1 probably doesn't hurt performance too much.  It also
>>>>> doesn't encode any information about the location of the difference in
>>>>> the result value, which helps if that ever leaks.
>>>>
>>>> The problem with returning 0/1 within the function body of
>>>> crypto_mem_not_equal is that it makes it easier for the compiler to
>>>> introduce a short-circuit optimization.
>>>>
>>>> It might be better to move the test where the result is compared
>>>> against 0 into an inline function:
>>>>
>>>> noinline unsigned long __crypto_mem_not_equal(const void *a, const
>>>> void *b, size_t size);
>>>>
>>>> static inline int crypto_mem_not_equal(const void *a, const void *b,
>>>> size_t size) {
>>>>      return __crypto_mem_not_equal(a, b, size) != 0UL ? 1 : 0;
>>>> }
>>>>
>>>> This hides the fact that we are only interested in a boolean result
>>>> from the compiler when it's compiling crypto_mem_not_equal.c, but also
>>>> ensures type safety when users test the return value.  It's also
>>>> likely to have little or no performance impact.
>>>
>>> Well, the code snippet I've provided from NaCl [1] is not really
>>> "fast-path"
>>> as you say, but rather to prevent the compiler from doing such
>>> optimizations
>>> by having a transformation of the "accumulated" bits into 0 and 1 as
>>> an end
>>> result (likely to prevent a short circuit), plus it has static size,
>>> so no
>>> loops applied here that could screw up.
>>>
>>> Variable size could be done under arch/ in asm, and if not available,
>>> that
>>> just falls back to normal memcmp that is being transformed into a same
>>> return
>>> value. By that, all other archs could easily migrate afterwards. What do
>>> you
>>> think?
>>>
>>>   [1] http://www.spinics.net/lists/linux-crypto/msg09558.html
>>
>> I'm not sure that the differentbits -> 0/-1 transform in NaCl really
>> buys us anything because
>  > we don't care very much about making the final test of differentbits
> != 0 constant-time.  An
>  > attacker already knows whether the test succeeded or failed -- we
> care more about making the
>  > failure cases constant-time.
>>
>> To do this, we need to make sure that the compiler doesn't insert one
>> or more early instructions
>  > to compare differentbits with 0xFF and then bypass the rest of the
> F(n) lines because it knows
>  > then that the value of differentbits cannot be changed by subsequent
> F(n) lines.  It seems that
>  > all of the approaches that use |= to build up the differentbits value
> suffer from this problem.
>>
>> My proposed solution is rather than trying to outsmart the compiler
>> with code that resists
>  > optimization, why not just turn optimization off directly with
> #pragma GCC optimize.  Or better
>  > yet, use an optimization setting that provides reasonable speed
> without introducing potential
>  > short-circuit optimizations.
>>
>> By optimizing for size ("Os"), the compiler will need to turn off
>> optimizations that add code
>  > for a possible speed benefit, and the kind of short-circuit
> optimizations that we are trying to
>  > avoid fall precisely into this class -- they add an instruction to
> check if the OR accumulator has
>  > all of its bits set, then if so, do an early exit.  So by using Os,
> we still benefit from
>  > optimizations that increase speed but don't increase code size.
>
> While I was looking a bit further into this, I thought using an
> attribute like this might be a
> more clean variant ...
>
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index 92669cd..2505b1b 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -351,6 +351,11 @@ void ftrace_likely_update(struct ftrace_branch_data
> *f, int val, int expect);
>    */
>   #define ACCESS_ONCE(x) (*(volatile typeof(x) *)&(x))
>
> +/* Tell GCC to turn off optimization for a particular function. */
> +#ifndef __do_not_optimize
> +#define __do_not_optimize      __attribute__((optimize("O0")))
> +#endif
> +
>   /* Ignore/forbid kprobes attach on very low level functions marked by
> this attribute: */
>   #ifdef CONFIG_KPROBES
>   # define __kprobes     __attribute__((__section__(".kprobes.text")))
>
> ... however, then I found on the GCC developer mailing list [1]: That
> said, I consider the
> optimize attribute code seriously broken and unmaintained (but sometimes
> useful for debugging -
> and only that). [...] Does "#pragma Gcc optimize" work more reliably?
> No, it uses the same
> mechanism internally. [...] And if these options are so broken, should
> they be marked as such
> in the manual? [...] Probably yes.
>
> Therefore, I still need to ask ... what about the option of an
> arch/*/crypto/... asm
> implementation? This still seems safer for something that crucial, imho.

We can easily specify -Os in the Makefile rather than depending on 
#pragma optimize or __attribute__ optimize if they are considered broken.

Re: arch/*/crypto/... asm, not sure it's worth it given the extra effort 
to develop, test, and maintain asm for all archs.  The two things we 
care about (constant time and performance) seem readily achievable in C.

Regarding O0 vs. Os, I would tend to prefer Os because it's much faster 
than O0, but still carries the desirable property that optimizations 
that increase code size are disabled.  It seems that short-circuit 
optimizations would be disabled by this, since by definition a 
short-circuit optimization requires the addition of a compare and branch.

James

^ permalink raw reply

* Re: [PATCH] crypto_mem_not_equal: add constant-time equality testing of memory regions
From: Daniel Borkmann @ 2013-09-19  8:37 UTC (permalink / raw)
  To: James Yonan; +Cc: Florian Weimer, Marcelo Cerri, linux-crypto, herbert
In-Reply-To: <523A41AE.9060105@openvpn.net>

On 09/19/2013 02:13 AM, James Yonan wrote:
[...]
> We can easily specify -Os in the Makefile rather than depending on #pragma optimize or __attribute__ optimize if they are considered broken.
>
> Re: arch/*/crypto/... asm, not sure it's worth it given the extra effort to develop, test, and maintain asm for all archs.  The two things we care about (constant time and performance) seem readily achievable in C.
>
> Regarding O0 vs. Os, I would tend to prefer Os because it's much faster than O0, but still carries the desirable property that optimizations that increase code size are disabled.  It seems that short-circuit optimizations would be disabled by this, since by definition a short-circuit optimization requires the addition of a compare and branch.

Ok, if we can make sure that this would overwrite global defaults in any circumstances,
then that approach should be fine, imho.

I would suggest that you use the crypto_mem_not_equal() function that you originally had
or that I was proposing, and still allow the possibility for an arch optimized version,
if people want to.

In that way, it can be kept simple and stupid and easy to review, just like all other
util functions such as memcmp etc is implemented in [1].

  [1] http://lingrok.org/xref/linux-net-next/lib/string.c#643

^ permalink raw reply

* Re: [PATCH v2] crypto: caam - map src buffer before access
From: Horia Geantă @ 2013-09-19 12:39 UTC (permalink / raw)
  To: Yashpal Dutta; +Cc: linux-crypto
In-Reply-To: <1378927446-21209-1-git-send-email-yashpal.dutta@freescale.com>

On 9/11/2013 10:24 PM, Yashpal Dutta wrote:
> KMap the buffers before copying trailing bytes during hmac into a session
> temporary buffer. This is required if pinned buffer from user-space is send
> during hmac and is safe even if hmac request is generated from within kernel.
>
> Signed-off-by: Yashpal Dutta <yashpal.dutta@freescale.com>
Reviewed-by: Horia Geanta <horia.geanta@freescale.com>

Why haven't you Cc-ed stable?

^ permalink raw reply

* Re: crypto: GCM API usage
From: Marcelo Cerri @ 2013-09-19 20:33 UTC (permalink / raw)
  To: Dominik Paulus; +Cc: linux-crypto, herbert, davem, tobias.polzer, linux-kernel
In-Reply-To: <20130916183411.GC3380@d-paulus.de>

On Mon, Sep 16, 2013 at 08:34:11PM +0200, Dominik Paulus wrote:
> Hi,
> 
> On Mon, Sep 16, 2013 at 12:58:40PM +0200, dominik.d.paulus@studium.uni-erlangen.de wrote:
> > We are currently trying to add encryption support to the usbip kernel
> > driver. Unfortunately, there is almost no documentation for the kernel
> > crypto API. So far, we couldn't figure out how to use the GCM encryption
> > mode in the kernel. There seems to be infrastructure for IV generation
> > in place (e.g. seqiv.c, the geniv stuff and the RFC 4106 implementation),
> > but no code directly using it.
> > 
> > What's the recommended way to use the IV generators with a "high-level"
> > API?
> 
> Sorry, that mail probably got a bit too short. To explain our problem a bit
> more: We are currently using a 64-bit counter to generate IVs. As the
> keys are randomly generated for each session and thus never reused,
> that's probably a not too bad idea (if it is, please tell us why ;)),
> assuming this counter is never going to overflow. We pass the IVs
> directly to aead_request_set_crypt for each message. This currently
> works quite fine.

It's usual the use of random IVs but I don't think that any known
pattern (as a counter) in the IV generation should cause any reduction
in the algorithm strength (and if it does, it's probably a weakness in
the algorithm itself). The only thing that should be avoided for sure is
the reuse of the same IV.

> 
> However, we would expect that IV generation is at least partially handled
> by the crypto API. As I said, there seems to be infrastructure for that,
> that abstracts the sequence number quite nicely. The seqiv generator
> seems to provide a high-level interface to the AEAD crypto, including an 
> abstraction for the sequence number generation. However, due to the lack
> of documentation and/or reference code using the API, we couldn't find
> out how to use it yet.

I haven't used the IV generation facility of the Crypto API, but it
seems to be very straightforward although there's no documentation
about that.

You should use aead_givcrypt_set_callback(), aead_givcrypt_set_assoc()
and aead_givcrypt_set_crypt() as you would use the regular aead
functions, that includes that you have to provide a buffer with length
equals to the algorithm block size for the IV. And then you should call
aead_givcrypt_set_giv() passing a counter and another IV buffer.

The difference between the two IV buffers that you have to provide to
aead_givcrypt_set_crypt() and aead_givcrypt_set_giv() is that the first
one will be updated by the algorithm during the encryption of each block
and the second one will contain the generated IV that you will have to
use to decrypt data.

The last step is to call crypto_aead_givencrypt() as you would call
crypto_aead_encrypt().

Under the cover the Crypto API will generate a random salt in the first
use of each request. This salt will be xor'd with the given counter to
create the new IV. That has an advantage over a simple count, that
usually will start with the same value every time the system is
rebooted.

> 
> Any help on this would be appreciated. If we feel competent enough to do 
> so after finishing this project, we would also volunteer to extend the
> introduction in Documentation/crypto/api-intro.txt a bit.
> 

That is probably a very good idea! I also want to improve the
documentation as soon I have some time to do it :)

> Regards,
>         Tobias Polzer and Dominik Paulus
> --
> To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply

* [PATCH 00/51] DMA mask changes
From: Russell King - ARM Linux @ 2013-09-19 21:22 UTC (permalink / raw)
  To: alsa-devel, b43-dev, devel, devicetree, dri-devel, e1000-devel,
	linux-arm-kernel, linux-crypto, linux-doc, linux-fbdev, linux-ide,
	linux-media, linux-mmc, linux-nvme, linux-omap, linuxppc-dev,
	linux-samsung-soc, linux-scsi, linux-tegra, linux-usb,
	linux-wireless, netdev, Solarflare linux maintainers,
	uclinux-dist-devel

This started out as a request to look at the DMA mask situation, and how
to solve the issues which we have on ARM - notably how the DMA mask
should be setup.

However, I started off reviewing how the dma_mask and coherent_dma_mask
was being used, and what I found was rather messy, and in some cases
rather buggy.  I tried to get some of the bug fixes in before the last
merge window, but it seems that the maintainers preferred to have the
full solution rather than a simple -rc suitable bug fix.

So, this is an attempt to clean things up.

The first point here is that drivers performing DMA should be calling
dma_set_mask()/dma_set_coherent_mask() in their probe function to verify
that DMA can be performed.  Lots of ARM drivers omit this step; please
refer to the DMA API documentation on this subject.

What this means is that the DMA mask provided by bus code is a default
value - nothing more.  It doesn't have to accurately reflect what the
device is actually capable of.  Apart from the storage for dev->dma_mask
being initialised for any device which is DMA capable, there is no other
initialisation which is strictly necessary at device creation time.

Now, these cleanups address two major areas:
1. The setting of DMA masks, particularly when both the coherent and
   streaming DMA masks are set together.

2. The initialisation of DMA masks by drivers - this seems to be becoming
   a popular habbit, one which may not be entirely the right solution.
   Rather than having this scattered throughout the tree, I've pulled
   that into a central location (and called it coercing the DMA mask -
   because it really is about forcing the DMA mask to be that value.)

3. Finally, addressing the long held misbelief that DMA masks somehow
   correspond with physical addresses.  We already have established
   long ago that dma_addr_t values returned from the DMA API are the
   values which you program into the DMA controller, and so are the
   bus addresses.  It is _only_ sane that DMA masks are also bus
   related too, and not related to physical address spaces.

(3) is a very important point for LPAE systems, which may still have
less than 4GB of memory, but this memory is all located above the 4GB
physical boundary.  This means with the current model, any device
using a 32-bit DMA mask fails - even though the DMA controller is
still only a 32-bit DMA controller but the 32-bit bus addresses map
to system memory.  To put it another way, the bus addresses have a
4GB physical offset on them.

This email is only being sent to the mailing lists in question, not to
anyone personally.  The list of individuals is far to great to do that.
I'm hoping no mailing lists reject the patches based on the number of
recipients.

Patches based on v3.12-rc1.

 Documentation/DMA-API-HOWTO.txt                   |   37 +++++++++------
 Documentation/DMA-API.txt                         |    8 +++
 arch/arm/include/asm/dma-mapping.h                |    8 +++
 arch/arm/mm/dma-mapping.c                         |   49 ++++++++++++++++++--
 arch/arm/mm/init.c                                |   12 +++---
 arch/arm/mm/mm.h                                  |    2 +
 arch/powerpc/kernel/vio.c                         |    3 +-
 block/blk-settings.c                              |    8 ++--
 drivers/amba/bus.c                                |    6 +--
 drivers/ata/pata_ixp4xx_cf.c                      |    5 ++-
 drivers/ata/pata_octeon_cf.c                      |    5 +-
 drivers/block/nvme-core.c                         |   10 ++---
 drivers/crypto/ixp4xx_crypto.c                    |   48 ++++++++++----------
 drivers/dma/amba-pl08x.c                          |    5 ++
 drivers/dma/dw/platform.c                         |    8 +--
 drivers/dma/edma.c                                |    6 +--
 drivers/dma/pl330.c                               |    4 ++
 drivers/firmware/dcdbas.c                         |   23 +++++-----
 drivers/firmware/google/gsmi.c                    |   13 +++--
 drivers/gpu/drm/exynos/exynos_drm_drv.c           |    6 ++-
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c          |    5 +-
 drivers/media/platform/omap3isp/isp.c             |    6 +-
 drivers/media/platform/omap3isp/isp.h             |    3 -
 drivers/mmc/card/queue.c                          |    3 +-
 drivers/mmc/host/sdhci-acpi.c                     |    5 +-
 drivers/net/ethernet/broadcom/b44.c               |    3 +-
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c  |    8 +---
 drivers/net/ethernet/brocade/bna/bnad.c           |   13 ++----
 drivers/net/ethernet/emulex/benet/be_main.c       |   12 +----
 drivers/net/ethernet/intel/e1000/e1000_main.c     |    9 +---
 drivers/net/ethernet/intel/e1000e/netdev.c        |   18 +++-----
 drivers/net/ethernet/intel/igb/igb_main.c         |   18 +++-----
 drivers/net/ethernet/intel/igbvf/netdev.c         |   18 +++-----
 drivers/net/ethernet/intel/ixgb/ixgb_main.c       |   16 ++-----
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c     |   15 ++----
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |   15 ++----
 drivers/net/ethernet/nxp/lpc_eth.c                |    6 ++-
 drivers/net/ethernet/octeon/octeon_mgmt.c         |    5 +-
 drivers/net/ethernet/sfc/efx.c                    |   12 +-----
 drivers/net/wireless/b43/dma.c                    |    9 +---
 drivers/net/wireless/b43legacy/dma.c              |    9 +---
 drivers/of/platform.c                             |    3 -
 drivers/parport/parport_pc.c                      |    8 +++-
 drivers/scsi/scsi_lib.c                           |    2 +-
 drivers/staging/dwc2/platform.c                   |    5 +-
 drivers/staging/et131x/et131x.c                   |   17 +------
 drivers/staging/imx-drm/imx-drm-core.c            |    8 +++-
 drivers/staging/imx-drm/ipuv3-crtc.c              |    4 +-
 drivers/staging/media/dt3155v4l/dt3155v4l.c       |    5 +--
 drivers/usb/chipidea/ci_hdrc_imx.c                |    7 +--
 drivers/usb/dwc3/dwc3-exynos.c                    |    7 +--
 drivers/usb/gadget/lpc32xx_udc.c                  |    4 +-
 drivers/usb/host/bcma-hcd.c                       |    3 +-
 drivers/usb/host/ehci-atmel.c                     |    7 +--
 drivers/usb/host/ehci-octeon.c                    |    4 +-
 drivers/usb/host/ehci-omap.c                      |   10 ++--
 drivers/usb/host/ehci-orion.c                     |    7 +--
 drivers/usb/host/ehci-platform.c                  |   10 ++--
 drivers/usb/host/ehci-s5p.c                       |    7 +--
 drivers/usb/host/ehci-spear.c                     |    7 +--
 drivers/usb/host/ehci-tegra.c                     |    7 +--
 drivers/usb/host/ohci-at91.c                      |    9 ++--
 drivers/usb/host/ohci-exynos.c                    |    7 +--
 drivers/usb/host/ohci-nxp.c                       |    5 +-
 drivers/usb/host/ohci-octeon.c                    |    5 +-
 drivers/usb/host/ohci-omap3.c                     |   10 ++--
 drivers/usb/host/ohci-pxa27x.c                    |    8 ++--
 drivers/usb/host/ohci-sa1111.c                    |    6 +++
 drivers/usb/host/ohci-spear.c                     |    7 +--
 drivers/usb/host/ssb-hcd.c                        |    3 +-
 drivers/usb/host/uhci-platform.c                  |    7 +--
 drivers/usb/musb/am35x.c                          |   50 +++++++--------------
 drivers/usb/musb/da8xx.c                          |   49 +++++++-------------
 drivers/usb/musb/davinci.c                        |   48 +++++++-------------
 drivers/usb/musb/tusb6010.c                       |   49 +++++++-------------
 drivers/video/amba-clcd.c                         |    5 ++
 include/linux/amba/bus.h                          |    2 -
 include/linux/dma-mapping.h                       |   31 +++++++++++++
 sound/arm/pxa2xx-pcm.c                            |    9 +---
 sound/soc/atmel/atmel-pcm.c                       |   11 ++---
 sound/soc/blackfin/bf5xx-ac97-pcm.c               |   11 ++---
 sound/soc/blackfin/bf5xx-i2s-pcm.c                |   10 ++---
 sound/soc/davinci/davinci-pcm.c                   |    9 +---
 sound/soc/fsl/fsl_dma.c                           |    9 +---
 sound/soc/fsl/mpc5200_dma.c                       |   10 ++---
 sound/soc/jz4740/jz4740-pcm.c                     |   12 ++---
 sound/soc/kirkwood/kirkwood-dma.c                 |    9 +---
 sound/soc/nuc900/nuc900-pcm.c                     |    9 ++--
 sound/soc/omap/omap-pcm.c                         |   11 ++---
 sound/soc/pxa/pxa2xx-pcm.c                        |   11 ++---
 sound/soc/s6000/s6000-pcm.c                       |    9 +---
 sound/soc/samsung/dma.c                           |   11 ++---
 sound/soc/samsung/idma.c                          |   11 ++---
 93 files changed, 493 insertions(+), 566 deletions(-)

^ permalink raw reply

* [PATCH 01/51] DMA-API: provide a helper to set both DMA and coherent DMA masks
From: Russell King @ 2013-09-19 21:25 UTC (permalink / raw)
  To: alsa-devel, b43-dev, devel, devicetree, dri-devel, e1000-devel,
	linux-arm-kernel, linux-crypto, linux-doc, linux-fbdev, linux-ide,
	linux-media, linux-mmc, linux-nvme, linux-omap, linuxppc-dev,
	linux-samsung-soc, linux-scsi, linux-tegra, linux-usb,
	linux-wireless, netdev, Solarflare linux maintainers,
	uclinux-dist-devel
  Cc: Vinod Koul, Dan Williams, Rob Landley
In-Reply-To: <20130919212235.GD12758@n2100.arm.linux.org.uk>

Provide a helper to set both the DMA and coherent DMA masks to the
same value - this avoids duplicated code in a number of drivers,
sometimes with buggy error handling, and also allows us identify
which drivers do things differently.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 Documentation/DMA-API-HOWTO.txt |   37 ++++++++++++++++++++++---------------
 Documentation/DMA-API.txt       |    8 ++++++++
 include/linux/dma-mapping.h     |   14 ++++++++++++++
 3 files changed, 44 insertions(+), 15 deletions(-)

diff --git a/Documentation/DMA-API-HOWTO.txt b/Documentation/DMA-API-HOWTO.txt
index 14129f1..5e98303 100644
--- a/Documentation/DMA-API-HOWTO.txt
+++ b/Documentation/DMA-API-HOWTO.txt
@@ -101,14 +101,23 @@ style to do this even if your device holds the default setting,
 because this shows that you did think about these issues wrt. your
 device.
 
-The query is performed via a call to dma_set_mask():
+The query is performed via a call to dma_set_mask_and_coherent():
 
-	int dma_set_mask(struct device *dev, u64 mask);
+	int dma_set_mask_and_coherent(struct device *dev, u64 mask);
 
-The query for consistent allocations is performed via a call to
-dma_set_coherent_mask():
+which will query the mask for both streaming and coherent APIs together.
+If you have some special requirements, then the following two separate
+queries can be used instead:
 
-	int dma_set_coherent_mask(struct device *dev, u64 mask);
+	The query for streaming mappings is performed via a call to
+	dma_set_mask():
+
+		int dma_set_mask(struct device *dev, u64 mask);
+
+	The query for consistent allocations is performed via a call
+	to dma_set_coherent_mask():
+
+		int dma_set_coherent_mask(struct device *dev, u64 mask);
 
 Here, dev is a pointer to the device struct of your device, and mask
 is a bit mask describing which bits of an address your device
@@ -137,7 +146,7 @@ exactly why.
 
 The standard 32-bit addressing device would do something like this:
 
-	if (dma_set_mask(dev, DMA_BIT_MASK(32))) {
+	if (dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32))) {
 		printk(KERN_WARNING
 		       "mydev: No suitable DMA available.\n");
 		goto ignore_this_device;
@@ -171,22 +180,20 @@ If a card is capable of using 64-bit consistent allocations as well,
 
 	int using_dac, consistent_using_dac;
 
-	if (!dma_set_mask(dev, DMA_BIT_MASK(64))) {
+	if (!dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64))) {
 		using_dac = 1;
 	   	consistent_using_dac = 1;
-		dma_set_coherent_mask(dev, DMA_BIT_MASK(64));
-	} else if (!dma_set_mask(dev, DMA_BIT_MASK(32))) {
+	} else if (!dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32))) {
 		using_dac = 0;
 		consistent_using_dac = 0;
-		dma_set_coherent_mask(dev, DMA_BIT_MASK(32));
 	} else {
 		printk(KERN_WARNING
 		       "mydev: No suitable DMA available.\n");
 		goto ignore_this_device;
 	}
 
-dma_set_coherent_mask() will always be able to set the same or a
-smaller mask as dma_set_mask(). However for the rare case that a
+The coherent coherent mask will always be able to set the same or a
+smaller mask as the streaming mask. However for the rare case that a
 device driver only uses consistent allocations, one would have to
 check the return value from dma_set_coherent_mask().
 
@@ -199,9 +206,9 @@ Finally, if your device can only drive the low 24-bits of
 		goto ignore_this_device;
 	}
 
-When dma_set_mask() is successful, and returns zero, the kernel saves
-away this mask you have provided.  The kernel will use this
-information later when you make DMA mappings.
+When dma_set_mask() or dma_set_mask_and_coherent() is successful, and
+returns zero, the kernel saves away this mask you have provided.  The
+kernel will use this information later when you make DMA mappings.
 
 There is a case which we are aware of at this time, which is worth
 mentioning in this documentation.  If your device supports multiple
diff --git a/Documentation/DMA-API.txt b/Documentation/DMA-API.txt
index 78a6c56..e865279 100644
--- a/Documentation/DMA-API.txt
+++ b/Documentation/DMA-API.txt
@@ -142,6 +142,14 @@ internal API for use by the platform than an external API for use by
 driver writers.
 
 int
+dma_set_mask_and_coherent(struct device *dev, u64 mask)
+
+Checks to see if the mask is possible and updates the device
+streaming and coherent DMA mask parameters if it is.
+
+Returns: 0 if successful and a negative error if not.
+
+int
 dma_set_mask(struct device *dev, u64 mask)
 
 Checks to see if the mask is possible and updates the device
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 3a8d0a2..ec951f9 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -97,6 +97,20 @@ static inline int dma_set_coherent_mask(struct device *dev, u64 mask)
 }
 #endif
 
+/*
+ * Set both the DMA mask and the coherent DMA mask to the same thing.
+ * Note that we don't check the return value from dma_set_coherent_mask()
+ * as the DMA API guarantees that the coherent DMA mask can be set to
+ * the same or smaller than the streaming DMA mask.
+ */
+static inline int dma_set_mask_and_coherent(struct device *dev, u64 mask)
+{
+	int rc = dma_set_mask(dev, mask);
+	if (rc == 0)
+		dma_set_coherent_mask(dev, mask);
+	return rc;
+}
+
 extern u64 dma_get_required_mask(struct device *dev);
 
 static inline unsigned int dma_get_max_seg_size(struct device *dev)
-- 
1.7.4.4

^ permalink raw reply related

* [PATCH 02/51] DMA-API: net: brocade/bna/bnad.c: fix 32-bit DMA mask handling
From: Russell King @ 2013-09-19 21:26 UTC (permalink / raw)
  To: alsa-devel, b43-dev, devel, devicetree, dri-devel, e1000-devel,
	linux-arm-kernel, linux-crypto, linux-doc, linux-fbdev, linux-ide,
	linux-media, linux-mmc, linux-nvme, linux-omap, linuxppc-dev,
	linux-samsung-soc, linux-scsi, linux-tegra, linux-usb,
	linux-wireless, netdev, Solarflare linux maintainers,
	uclinux-dist-devel
  Cc: Rasesh Mody
In-Reply-To: <20130919212235.GD12758@n2100.arm.linux.org.uk>

The fallback to 32-bit DMA mask is rather odd:
	if (!dma_set_mask(&pdev->dev, DMA_BIT_MASK(64)) &&
	    !dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64))) {
		*using_dac = true;
	} else {
		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
		if (err) {
			err = dma_set_coherent_mask(&pdev->dev,
						    DMA_BIT_MASK(32));
			if (err)
				goto release_regions;
		}

This means we only try and set the coherent DMA mask if we failed to
set a 32-bit DMA mask, and only if both fail do we fail the driver.
Adjust this so that if either setting fails, we fail the driver - and
thereby end up properly setting both the DMA mask and the coherent
DMA mask in the fallback case.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/brocade/bna/bnad.c |   13 ++++---------
 1 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/brocade/bna/bnad.c b/drivers/net/ethernet/brocade/bna/bnad.c
index b78e69e..45ce6e2 100644
--- a/drivers/net/ethernet/brocade/bna/bnad.c
+++ b/drivers/net/ethernet/brocade/bna/bnad.c
@@ -3300,17 +3300,12 @@ bnad_pci_init(struct bnad *bnad,
 	err = pci_request_regions(pdev, BNAD_NAME);
 	if (err)
 		goto disable_device;
-	if (!dma_set_mask(&pdev->dev, DMA_BIT_MASK(64)) &&
-	    !dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64))) {
+	if (!dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64))) {
 		*using_dac = true;
 	} else {
-		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
-		if (err) {
-			err = dma_set_coherent_mask(&pdev->dev,
-						    DMA_BIT_MASK(32));
-			if (err)
-				goto release_regions;
-		}
+		err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
+		if (err)
+			goto release_regions;
 		*using_dac = false;
 	}
 	pci_set_master(pdev);
-- 
1.7.4.4

^ permalink raw reply related

* [PATCH 03/51] DMA-API: net: intel/e1000e: fix 32-bit DMA mask handling
From: Russell King @ 2013-09-19 21:27 UTC (permalink / raw)
  To: alsa-devel, b43-dev, devel, devicetree, dri-devel, e1000-devel,
	linux-arm-kernel, linux-crypto, linux-doc, linux-fbdev, linux-ide,
	linux-media, linux-mmc, linux-nvme, linux-omap, linuxppc-dev,
	linux-samsung-soc, linux-scsi, linux-tegra, linux-usb,
	linux-wireless, netdev, Solarflare linux maintainers,
	uclinux-dist-devel
  Cc: Jeff Kirsher, Jesse Brandeburg, Bruce Allan, Carolyn Wyborny,
	Don Skidmore, Greg Rose, Peter P Waskiewicz Jr, Alex Duyck,
	John Ronciak, Tushar Dave
In-Reply-To: <20130919212235.GD12758@n2100.arm.linux.org.uk>

The fallback to 32-bit DMA mask is rather odd:
	err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
	if (!err) {
		err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64));
		if (!err)
			pci_using_dac = 1;
	} else {
		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
		if (err) {
			err = dma_set_coherent_mask(&pdev->dev,
						    DMA_BIT_MASK(32));
			if (err) {
				dev_err(&pdev->dev,
					"No usable DMA configuration, aborting\n");
				goto err_dma;
			}
		}
	}
This means we only set the coherent DMA mask in the fallback path if
the DMA mask set failed, which is silly.  This fixes it to set the
coherent DMA mask only if dma_set_mask() succeeded, and to error out
if either fails.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/intel/e1000e/netdev.c |   18 ++++++------------
 1 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index e87e9b0..519e293 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -6553,21 +6553,15 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		return err;
 
 	pci_using_dac = 0;
-	err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
+	err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
 	if (!err) {
-		err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64));
-		if (!err)
-			pci_using_dac = 1;
+		pci_using_dac = 1;
 	} else {
-		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
+		err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
 		if (err) {
-			err = dma_set_coherent_mask(&pdev->dev,
-						    DMA_BIT_MASK(32));
-			if (err) {
-				dev_err(&pdev->dev,
-					"No usable DMA configuration, aborting\n");
-				goto err_dma;
-			}
+			dev_err(&pdev->dev,
+				"No usable DMA configuration, aborting\n");
+			goto err_dma;
 		}
 	}
 
-- 
1.7.4.4


^ permalink raw reply related

* [PATCH 04/51] DMA-API: net: intel/igb: fix 32-bit DMA mask handling
From: Russell King @ 2013-09-19 21:28 UTC (permalink / raw)
  To: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	b43-dev-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	e1000-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-ide-u79uwXL29TY76Z2rM5mHXA,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, Solarflare linux maintainers,
	uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b
  Cc: Jeff Kirsher, Jesse Brandeburg, Bruce Allan, Carolyn Wyborny,
	Don Skidmore, Greg Rose, Peter P Waskiewicz Jr, Alex Duyck,
	John Ronciak, Tushar Dave
In-Reply-To: <20130919212235.GD12758-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>

The fallback to 32-bit DMA mask is rather odd:
	err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
	if (!err) {
		err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64));
		if (!err)
			pci_using_dac = 1;
	} else {
		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
		if (err) {
			err = dma_set_coherent_mask(&pdev->dev,
						    DMA_BIT_MASK(32));
			if (err) {
				dev_err(&pdev->dev,
					"No usable DMA configuration, aborting\n");
				goto err_dma;
			}
		}
	}
This means we only set the coherent DMA mask in the fallback path if
the DMA mask set failed, which is silly.  This fixes it to set the
coherent DMA mask only if dma_set_mask() succeeded, and to error out
if either fails.

Signed-off-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
---
 drivers/net/ethernet/intel/igb/igb_main.c |   18 ++++++------------
 1 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 8cf44f2..7579383 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -2034,21 +2034,15 @@ static int igb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		return err;
 
 	pci_using_dac = 0;
-	err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
+	err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
 	if (!err) {
-		err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64));
-		if (!err)
-			pci_using_dac = 1;
+		pci_using_dac = 1;
 	} else {
-		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
+		err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
 		if (err) {
-			err = dma_set_coherent_mask(&pdev->dev,
-						    DMA_BIT_MASK(32));
-			if (err) {
-				dev_err(&pdev->dev,
-					"No usable DMA configuration, aborting\n");
-				goto err_dma;
-			}
+			dev_err(&pdev->dev,
+				"No usable DMA configuration, aborting\n");
+			goto err_dma;
 		}
 	}
 
-- 
1.7.4.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 05/51] DMA-API: net: intel/igbvf: fix 32-bit DMA mask handling
From: Russell King @ 2013-09-19 21:29 UTC (permalink / raw)
  To: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	b43-dev-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	e1000-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-ide-u79uwXL29TY76Z2rM5mHXA,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, Solarflare linux maintainers,
	uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b
  Cc: Jeff Kirsher, Jesse Brandeburg, Bruce Allan, Carolyn Wyborny,
	Don Skidmore, Greg Rose, Peter P Waskiewicz Jr, Alex Duyck,
	John Ronciak, Tushar Dave
In-Reply-To: <20130919212235.GD12758-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>

The fallback to 32-bit DMA mask is rather odd:
	err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
	if (!err) {
		err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64));
		if (!err)
			pci_using_dac = 1;
	} else {
		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
		if (err) {
			err = dma_set_coherent_mask(&pdev->dev,
						    DMA_BIT_MASK(32));
			if (err) {
				dev_err(&pdev->dev, "No usable DMA "
					"configuration, aborting\n");
				goto err_dma;
			}
		}
	}
This means we only set the coherent DMA mask in the fallback path if
the DMA mask set failed, which is silly.  This fixes it to set the
coherent DMA mask only if dma_set_mask() succeeded, and to error out
if either fails.

Signed-off-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
---
 drivers/net/ethernet/intel/igbvf/netdev.c |   18 ++++++------------
 1 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/intel/igbvf/netdev.c b/drivers/net/ethernet/intel/igbvf/netdev.c
index 93eb7ee..4e6b02f 100644
--- a/drivers/net/ethernet/intel/igbvf/netdev.c
+++ b/drivers/net/ethernet/intel/igbvf/netdev.c
@@ -2638,21 +2638,15 @@ static int igbvf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		return err;
 
 	pci_using_dac = 0;
-	err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
+	err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
 	if (!err) {
-		err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64));
-		if (!err)
-			pci_using_dac = 1;
+		pci_using_dac = 1;
 	} else {
-		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
+		err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
 		if (err) {
-			err = dma_set_coherent_mask(&pdev->dev,
-						    DMA_BIT_MASK(32));
-			if (err) {
-				dev_err(&pdev->dev, "No usable DMA "
-				        "configuration, aborting\n");
-				goto err_dma;
-			}
+			dev_err(&pdev->dev, "No usable DMA "
+			        "configuration, aborting\n");
+			goto err_dma;
 		}
 	}
 
-- 
1.7.4.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 06/51] DMA-API: net: intel/ixgb: fix 32-bit DMA mask handling
From: Russell King @ 2013-09-19 21:30 UTC (permalink / raw)
  To: alsa-devel, b43-dev, devel, devicetree, dri-devel, e1000-devel,
	linux-arm-kernel, linux-crypto, linux-doc, linux-fbdev, linux-ide,
	linux-media, linux-mmc, linux-nvme, linux-omap, linuxppc-dev,
	linux-samsung-soc, linux-scsi, linux-tegra, linux-usb,
	linux-wireless, netdev, Solarflare linux maintainers,
	uclinux-dist-devel
  Cc: Jeff Kirsher, Jesse Brandeburg, Bruce Allan, Carolyn Wyborny,
	Don Skidmore, Greg Rose, Peter P Waskiewicz Jr, Alex Duyck,
	John Ronciak, Tushar Dave
In-Reply-To: <20130919212235.GD12758@n2100.arm.linux.org.uk>

The fallback to 32-bit DMA mask is rather odd:
	err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
	if (!err) {
		err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64));
		if (!err)
			pci_using_dac = 1;
	} else {
		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
		if (err) {
			err = dma_set_coherent_mask(&pdev->dev,
						    DMA_BIT_MASK(32));
			if (err) {
				pr_err("No usable DMA configuration, aborting\n");
				goto err_dma_mask;
			}
		}
	}
This means we only set the coherent DMA mask in the fallback path if
the DMA mask set failed, which is silly.  This fixes it to set the
coherent DMA mask only if dma_set_mask() succeeded, and to error out
if either fails.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/intel/ixgb/ixgb_main.c |   16 +++++-----------
 1 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgb/ixgb_main.c b/drivers/net/ethernet/intel/ixgb/ixgb_main.c
index 9f6b236..57e390c 100644
--- a/drivers/net/ethernet/intel/ixgb/ixgb_main.c
+++ b/drivers/net/ethernet/intel/ixgb/ixgb_main.c
@@ -408,20 +408,14 @@ ixgb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 		return err;
 
 	pci_using_dac = 0;
-	err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
+	err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
 	if (!err) {
-		err = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64));
-		if (!err)
-			pci_using_dac = 1;
+		pci_using_dac = 1;
 	} else {
-		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
+		err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
 		if (err) {
-			err = dma_set_coherent_mask(&pdev->dev,
-						    DMA_BIT_MASK(32));
-			if (err) {
-				pr_err("No usable DMA configuration, aborting\n");
-				goto err_dma_mask;
-			}
+			pr_err("No usable DMA configuration, aborting\n");
+			goto err_dma_mask;
 		}
 	}
 
-- 
1.7.4.4

^ permalink raw reply related

* [PATCH 07/51] DMA-API: net: intel/ixgbe: fix 32-bit DMA mask handling
From: Russell King @ 2013-09-19 21:31 UTC (permalink / raw)
  To: alsa-devel, b43-dev, devel, devicetree, dri-devel, e1000-devel,
	linux-arm-kernel, linux-crypto, linux-doc, linux-fbdev, linux-ide,
	linux-media, linux-mmc, linux-nvme, linux-omap, linuxppc-dev,
	linux-samsung-soc, linux-scsi, linux-tegra, linux-usb,
	linux-wireless, netdev, Solarflare linux maintainers,
	uclinux-dist-devel
  Cc: Jeff Kirsher, Jesse Brandeburg, Bruce Allan, Carolyn Wyborny,
	Don Skidmore, Greg Rose, Peter P Waskiewicz Jr, Alex Duyck,
	John Ronciak, Tushar Dave
In-Reply-To: <20130919212235.GD12758@n2100.arm.linux.org.uk>

The fallback to 32-bit DMA mask is rather odd:
	if (!dma_set_mask(&pdev->dev, DMA_BIT_MASK(64)) &&
	    !dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64))) {
		pci_using_dac = 1;
	} else {
		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
		if (err) {
			err = dma_set_coherent_mask(&pdev->dev,
						    DMA_BIT_MASK(32));
			if (err) {
				dev_err(&pdev->dev,
					"No usable DMA configuration, aborting\n");
				goto err_dma;
			}
		}
		pci_using_dac = 0;
	}
This means we only set the coherent DMA mask in the fallback path if
the DMA mask set failed, which is silly.  This fixes it to set the
coherent DMA mask only if dma_set_mask() succeeded, and to error out
if either fails.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c |   15 +++++----------
 1 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index 7aba452..b1dc844 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -7475,19 +7475,14 @@ static int ixgbe_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (err)
 		return err;
 
-	if (!dma_set_mask(&pdev->dev, DMA_BIT_MASK(64)) &&
-	    !dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64))) {
+	if (!dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64))) {
 		pci_using_dac = 1;
 	} else {
-		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
+		err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
 		if (err) {
-			err = dma_set_coherent_mask(&pdev->dev,
-						    DMA_BIT_MASK(32));
-			if (err) {
-				dev_err(&pdev->dev,
-					"No usable DMA configuration, aborting\n");
-				goto err_dma;
-			}
+			dev_err(&pdev->dev,
+				"No usable DMA configuration, aborting\n");
+			goto err_dma;
 		}
 		pci_using_dac = 0;
 	}
-- 
1.7.4.4


^ permalink raw reply related

* [PATCH 08/51] DMA-API: net: intel/ixgbevf: fix 32-bit DMA mask handling
From: Russell King @ 2013-09-19 21:32 UTC (permalink / raw)
  To: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	b43-dev-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	e1000-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-ide-u79uwXL29TY76Z2rM5mHXA,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, Solarflare linux maintainers,
	uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b
  Cc: Jeff Kirsher, Jesse Brandeburg, Bruce Allan, Carolyn Wyborny,
	Don Skidmore, Greg Rose, Peter P Waskiewicz Jr, Alex Duyck,
	John Ronciak, Tushar Dave
In-Reply-To: <20130919212235.GD12758-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>

The fallback to 32-bit DMA mask is rather odd:
	if (!dma_set_mask(&pdev->dev, DMA_BIT_MASK(64)) &&
	    !dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64))) {
		pci_using_dac = 1;
	} else {
		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
		if (err) {
			err = dma_set_coherent_mask(&pdev->dev,
						    DMA_BIT_MASK(32));
			if (err) {
				dev_err(&pdev->dev, "No usable DMA "
					"configuration, aborting\n");
				goto err_dma;
			}
		}
		pci_using_dac = 0;
	}
This means we only set the coherent DMA mask in the fallback path if
the DMA mask set failed, which is silly.  This fixes it to set the
coherent DMA mask only if dma_set_mask() succeeded, and to error out
if either fails.

Signed-off-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
---
 drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c |   15 +++++----------
 1 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
index 59a62bb..e34c2da 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
@@ -3326,19 +3326,14 @@ static int ixgbevf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if (err)
 		return err;
 
-	if (!dma_set_mask(&pdev->dev, DMA_BIT_MASK(64)) &&
-	    !dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64))) {
+	if (!dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64))) {
 		pci_using_dac = 1;
 	} else {
-		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
+		err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
 		if (err) {
-			err = dma_set_coherent_mask(&pdev->dev,
-						    DMA_BIT_MASK(32));
-			if (err) {
-				dev_err(&pdev->dev, "No usable DMA "
-					"configuration, aborting\n");
-				goto err_dma;
-			}
+			dev_err(&pdev->dev, "No usable DMA "
+				"configuration, aborting\n");
+			goto err_dma;
 		}
 		pci_using_dac = 0;
 	}
-- 
1.7.4.4

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related

* [PATCH 09/51] DMA-API: net: broadcom/b44: replace dma_set_mask()+dma_set_coherent_mask() with new helper
From: Russell King @ 2013-09-19 21:33 UTC (permalink / raw)
  To: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	b43-dev-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	e1000-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-ide-u79uwXL29TY76Z2rM5mHXA,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, Solarflare linux maintainers,
	uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b
  Cc: Gary Zambrano
In-Reply-To: <20130919212235.GD12758-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>

Replace the following sequence:

	dma_set_mask(dev, mask);
	dma_set_coherent_mask(dev, mask);

with a call to the new helper dma_set_mask_and_coherent().

Signed-off-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
---
 drivers/net/ethernet/broadcom/b44.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/b44.c b/drivers/net/ethernet/broadcom/b44.c
index 9b017d9..b4d2018 100644
--- a/drivers/net/ethernet/broadcom/b44.c
+++ b/drivers/net/ethernet/broadcom/b44.c
@@ -2183,8 +2183,7 @@ static int b44_init_one(struct ssb_device *sdev,
 		goto err_out_free_dev;
 	}
 
-	if (dma_set_mask(sdev->dma_dev, DMA_BIT_MASK(30)) ||
-	    dma_set_coherent_mask(sdev->dma_dev, DMA_BIT_MASK(30))) {
+	if (dma_set_mask_and_coherent(sdev->dma_dev, DMA_BIT_MASK(30))) {
 		dev_err(sdev->dev,
 			"Required 30BIT DMA mask unsupported by the system\n");
 		goto err_out_powerdown;
-- 
1.7.4.4

^ permalink raw reply related

* [PATCH 10/51] DMA-API: net: broadcom/bnx2x: replace dma_set_mask()+dma_set_coherent_mask() with new helper
From: Russell King @ 2013-09-19 21:34 UTC (permalink / raw)
  To: alsa-devel, b43-dev, devel, devicetree, dri-devel, e1000-devel,
	linux-arm-kernel, linux-crypto, linux-doc, linux-fbdev, linux-ide,
	linux-media, linux-mmc, linux-nvme, linux-omap, linuxppc-dev,
	linux-samsung-soc, linux-scsi, linux-tegra, linux-usb,
	linux-wireless, netdev, Solarflare linux maintainers,
	uclinux-dist-devel
  Cc: Eilon Greenstein
In-Reply-To: <20130919212235.GD12758@n2100.arm.linux.org.uk>

Replace the following sequence:

	dma_set_mask(dev, mask);
	dma_set_coherent_mask(dev, mask);

with a call to the new helper dma_set_mask_and_coherent().

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c |    8 ++------
 1 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
index 2f8dbbb..e6c3e66 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_main.c
@@ -12072,13 +12072,9 @@ static int bnx2x_set_coherency_mask(struct bnx2x *bp)
 {
 	struct device *dev = &bp->pdev->dev;
 
-	if (dma_set_mask(dev, DMA_BIT_MASK(64)) == 0) {
+	if (dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64)) == 0) {
 		bp->flags |= USING_DAC_FLAG;
-		if (dma_set_coherent_mask(dev, DMA_BIT_MASK(64)) != 0) {
-			dev_err(dev, "dma_set_coherent_mask failed, aborting\n");
-			return -EIO;
-		}
-	} else if (dma_set_mask(dev, DMA_BIT_MASK(32)) != 0) {
+	} else if (dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32)) != 0) {
 		dev_err(dev, "System does not support DMA, aborting\n");
 		return -EIO;
 	}
-- 
1.7.4.4

^ permalink raw reply related

* [PATCH 11/51] DMA-API: net: emulex/benet: replace dma_set_mask()+dma_set_coherent_mask() with new helper
From: Russell King @ 2013-09-19 21:36 UTC (permalink / raw)
  To: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
	b43-dev-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
	devicetree-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	e1000-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-crypto-u79uwXL29TY76Z2rM5mHXA,
	linux-doc-u79uwXL29TY76Z2rM5mHXA,
	linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
	linux-ide-u79uwXL29TY76Z2rM5mHXA,
	linux-media-u79uwXL29TY76Z2rM5mHXA,
	linux-mmc-u79uwXL29TY76Z2rM5mHXA,
	linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-omap-u79uwXL29TY76Z2rM5mHXA,
	linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
	linux-scsi-u79uwXL29TY76Z2rM5mHXA,
	linux-tegra-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA, Solarflare linux maintainers,
	uclinux-dist-devel-ZG0+EudsQA8dtHy/vicBwGD2FQJk+8+b
  Cc: Sathya Perla, Subbu Seetharaman, Ajit Khaparde
In-Reply-To: <20130919212235.GD12758-l+eeeJia6m9vn6HldHNs0ANdhmdF6hFW@public.gmane.org>

Replace the following sequence:

	dma_set_mask(dev, mask);
	dma_set_coherent_mask(dev, mask);

with a call to the new helper dma_set_mask_and_coherent().

Signed-off-by: Russell King <rmk+kernel-lFZ/pmaqli7XmaaqVzeoHQ@public.gmane.org>
---
 drivers/net/ethernet/emulex/benet/be_main.c |   12 ++----------
 1 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 3224d28..2084151 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -4335,19 +4335,11 @@ static int be_probe(struct pci_dev *pdev, const struct pci_device_id *pdev_id)
 	adapter->netdev = netdev;
 	SET_NETDEV_DEV(netdev, &pdev->dev);
 
-	status = dma_set_mask(&pdev->dev, DMA_BIT_MASK(64));
+	status = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64));
 	if (!status) {
-		status = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64));
-		if (status < 0) {
-			dev_err(&pdev->dev, "dma_set_coherent_mask failed\n");
-			goto free_netdev;
-		}
 		netdev->features |= NETIF_F_HIGHDMA;
 	} else {
-		status = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
-		if (!status)
-			status = dma_set_coherent_mask(&pdev->dev,
-						       DMA_BIT_MASK(32));
+		status = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
 		if (status) {
 			dev_err(&pdev->dev, "Could not set PCI DMA Mask\n");
 			goto free_netdev;
-- 
1.7.4.4

^ permalink raw reply related

* [PATCH 12/51] DMA-API: net: intel/e1000: replace dma_set_mask()+dma_set_coherent_mask() with new helper
From: Russell King @ 2013-09-19 21:37 UTC (permalink / raw)
  To: alsa-devel, b43-dev, devel, devicetree, dri-devel, e1000-devel,
	linux-arm-kernel, linux-crypto, linux-doc, linux-fbdev, linux-ide,
	linux-media, linux-mmc, linux-nvme, linux-omap, linuxppc-dev,
	linux-samsung-soc, linux-scsi, linux-tegra, linux-usb,
	linux-wireless, netdev, Solarflare linux maintainers,
	uclinux-dist-devel
  Cc: Alex Duyck, Don Skidmore, Peter P Waskiewicz Jr, Bruce Allan,
	Jesse Brandeburg, Greg Rose, John Ronciak, Jeff Kirsher,
	Carolyn Wyborny, Tushar Dave
In-Reply-To: <20130919212235.GD12758@n2100.arm.linux.org.uk>

Replace the following sequence:

	dma_set_mask(dev, mask);
	dma_set_coherent_mask(dev, mask);

with a call to the new helper dma_set_mask_and_coherent().

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/intel/e1000/e1000_main.c |    9 ++-------
 1 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/e1000/e1000_main.c b/drivers/net/ethernet/intel/e1000/e1000_main.c
index 59ad007..34672f8 100644
--- a/drivers/net/ethernet/intel/e1000/e1000_main.c
+++ b/drivers/net/ethernet/intel/e1000/e1000_main.c
@@ -1018,19 +1018,14 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	 */
 	pci_using_dac = 0;
 	if ((hw->bus_type == e1000_bus_type_pcix) &&
-	    !dma_set_mask(&pdev->dev, DMA_BIT_MASK(64))) {
-		/* according to DMA-API-HOWTO, coherent calls will always
-		 * succeed if the set call did
-		 */
-		dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(64));
+	    !dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(64))) {
 		pci_using_dac = 1;
 	} else {
-		err = dma_set_mask(&pdev->dev, DMA_BIT_MASK(32));
+		err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
 		if (err) {
 			pr_err("No usable DMA config, aborting\n");
 			goto err_dma;
 		}
-		dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32));
 	}
 
 	netdev->netdev_ops = &e1000_netdev_ops;
-- 
1.7.4.4

^ permalink raw reply related

* [PATCH 13/51] DMA-API: net: sfc/efx.c: replace dma_set_mask()+dma_set_coherent_mask() with new helper
From: Russell King @ 2013-09-19 21:38 UTC (permalink / raw)
  To: alsa-devel, b43-dev, devel, devicetree, dri-devel, e1000-devel,
	linux-arm-kernel, linux-crypto, linux-doc, linux-fbdev, linux-ide,
	linux-media, linux-mmc, linux-nvme, linux-omap, linuxppc-dev,
	linux-samsung-soc, linux-scsi, linux-tegra, linux-usb,
	linux-wireless, netdev, Solarflare linux maintainers,
	uclinux-dist-devel
  Cc: Ben Hutchings
In-Reply-To: <20130919212235.GD12758@n2100.arm.linux.org.uk>

Replace the following sequence:

	dma_set_mask(dev, mask);
	dma_set_coherent_mask(dev, mask);

with a call to the new helper dma_set_mask_and_coherent().

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/ethernet/sfc/efx.c |   12 +-----------
 1 files changed, 1 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/sfc/efx.c b/drivers/net/ethernet/sfc/efx.c
index 07c9bc4..2e27837 100644
--- a/drivers/net/ethernet/sfc/efx.c
+++ b/drivers/net/ethernet/sfc/efx.c
@@ -1121,7 +1121,7 @@ static int efx_init_io(struct efx_nic *efx)
 	 */
 	while (dma_mask > 0x7fffffffUL) {
 		if (dma_supported(&pci_dev->dev, dma_mask)) {
-			rc = dma_set_mask(&pci_dev->dev, dma_mask);
+			rc = dma_set_mask_and_coherent(&pci_dev->dev, dma_mask);
 			if (rc == 0)
 				break;
 		}
@@ -1134,16 +1134,6 @@ static int efx_init_io(struct efx_nic *efx)
 	}
 	netif_dbg(efx, probe, efx->net_dev,
 		  "using DMA mask %llx\n", (unsigned long long) dma_mask);
-	rc = dma_set_coherent_mask(&pci_dev->dev, dma_mask);
-	if (rc) {
-		/* dma_set_coherent_mask() is not *allowed* to
-		 * fail with a mask that dma_set_mask() accepted,
-		 * but just in case...
-		 */
-		netif_err(efx, probe, efx->net_dev,
-			  "failed to set consistent DMA mask\n");
-		goto fail2;
-	}
 
 	efx->membase_phys = pci_resource_start(efx->pci_dev, EFX_MEM_BAR);
 	rc = pci_request_region(pci_dev, EFX_MEM_BAR, "sfc");
-- 
1.7.4.4

^ permalink raw reply related

* [PATCH 14/51] DMA-API: net: b43: replace dma_set_mask()+dma_set_coherent_mask() with new helper
From: Russell King @ 2013-09-19 21:39 UTC (permalink / raw)
  To: alsa-devel, b43-dev, devel, devicetree, dri-devel, e1000-devel,
	linux-arm-kernel, linux-crypto, linux-doc, linux-fbdev, linux-ide,
	linux-media, linux-mmc, linux-nvme, linux-omap, linuxppc-dev,
	linux-samsung-soc, linux-scsi, linux-tegra, linux-usb,
	linux-wireless, netdev, Solarflare linux maintainers,
	uclinux-dist-devel
  Cc: John W. Linville, Stefano Brivio
In-Reply-To: <20130919212235.GD12758@n2100.arm.linux.org.uk>

Replace the following sequence:

	dma_set_mask(dev, mask);
	dma_set_coherent_mask(dev, mask);

with a call to the new helper dma_set_mask_and_coherent().

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/wireless/b43/dma.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/b43/dma.c b/drivers/net/wireless/b43/dma.c
index c51d2dc..1d7982a 100644
--- a/drivers/net/wireless/b43/dma.c
+++ b/drivers/net/wireless/b43/dma.c
@@ -1065,12 +1065,9 @@ static int b43_dma_set_mask(struct b43_wldev *dev, u64 mask)
 	/* Try to set the DMA mask. If it fails, try falling back to a
 	 * lower mask, as we can always also support a lower one. */
 	while (1) {
-		err = dma_set_mask(dev->dev->dma_dev, mask);
-		if (!err) {
-			err = dma_set_coherent_mask(dev->dev->dma_dev, mask);
-			if (!err)
-				break;
-		}
+		err = dma_set_mask_and_coherent(dev->dev->dma_dev, mask);
+		if (!err)
+			break;
 		if (mask == DMA_BIT_MASK(64)) {
 			mask = DMA_BIT_MASK(32);
 			fallback = true;
-- 
1.7.4.4

^ permalink raw reply related

* [PATCH 15/51] DMA-API: net: b43legacy: replace dma_set_mask()+dma_set_coherent_mask() with new helper
From: Russell King @ 2013-09-19 21:40 UTC (permalink / raw)
  To: alsa-devel, b43-dev, devel, devicetree, dri-devel, e1000-devel,
	linux-arm-kernel, linux-crypto, linux-doc, linux-fbdev, linux-ide,
	linux-media, linux-mmc, linux-nvme, linux-omap, linuxppc-dev,
	linux-samsung-soc, linux-scsi, linux-tegra, linux-usb,
	linux-wireless, netdev, Solarflare linux maintainers,
	uclinux-dist-devel
  Cc: John W. Linville, Stefano Brivio, Larry Finger
In-Reply-To: <20130919212235.GD12758@n2100.arm.linux.org.uk>

Replace the following sequence:

	dma_set_mask(dev, mask);
	dma_set_coherent_mask(dev, mask);

with a call to the new helper dma_set_mask_and_coherent().

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 drivers/net/wireless/b43legacy/dma.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/b43legacy/dma.c b/drivers/net/wireless/b43legacy/dma.c
index 42eb26c..b2ed179 100644
--- a/drivers/net/wireless/b43legacy/dma.c
+++ b/drivers/net/wireless/b43legacy/dma.c
@@ -806,12 +806,9 @@ static int b43legacy_dma_set_mask(struct b43legacy_wldev *dev, u64 mask)
 	/* Try to set the DMA mask. If it fails, try falling back to a
 	 * lower mask, as we can always also support a lower one. */
 	while (1) {
-		err = dma_set_mask(dev->dev->dma_dev, mask);
-		if (!err) {
-			err = dma_set_coherent_mask(dev->dev->dma_dev, mask);
-			if (!err)
-				break;
-		}
+		err = dma_set_mask_and_coherent(dev->dev->dma_dev, mask);
+		if (!err)
+			break;
 		if (mask == DMA_BIT_MASK(64)) {
 			mask = DMA_BIT_MASK(32);
 			fallback = true;
-- 
1.7.4.4

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox