Re: [PATCH] crypto: remove double execution of the same test suite

2013-07-19 Thread Horia Geantă

On 7/18/2013 6:57 PM, Cristian Stoica wrote:

This patch removes redundant execution of the same test suite in cases
where alg and driver variables are the same (e.g. when alg_test is
called from tcrypt_test)

Signed-off-by: Cristian Stoica cristian.sto...@freescale.com


Reviewed-by: Horia Geanta horia.gea...@freescale.com


---
  crypto/testmgr.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 2f00607..e091ef6 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -3234,7 +3234,7 @@ int alg_test(const char *driver, const char *alg, u32 
type, u32 mask)
if (i = 0)
rc |= alg_test_descs[i].test(alg_test_descs + i, driver,
 type, mask);
-   if (j = 0)
+   if (j = 0  j != i)
rc |= alg_test_descs[j].test(alg_test_descs + j, driver,
 type, mask);






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


Re: [PATCH 3.11-rc1] crypto: Fix boot failure due to module dependency.

2013-07-19 Thread Rafael J. Wysocki
On Thursday, July 18, 2013 04:44:20 PM H. Peter Anvin wrote:
 On 07/18/2013 03:17 PM, Rafael J. Wysocki wrote:
 
  alias x86cpu:vendor:*:family:*:model:*:feature:*0081* crct10dif_pclmul
 
  This should cause udev to load the crct10dif_pclml module when cpu
  support the PCLMULQDQ (feature code 0081).  I did my testing during
  development on 3.10 and the module was indeed loaded.
 
  However, I found that the following commit under 3.11-rc1 broke
  the mechanism after some bisection.
 
  commit ac212b6980d8d5eda705864fc5a8ecddc6d6eacc
  Author: Rafael J. Wysocki rafael.j.wyso...@intel.com
  Date:   Fri May 3 00:26:22 2013 +0200
 
   ACPI / processor: Use common hotplug infrastructure
Split the ACPI processor driver into two parts, one that is
   non-modular, resides in the ACPI core and handles the enumeration
   and hotplug of processors and one that implements the rest of the
   existing processor driver functionality.
   Rafael, can you check and see if this can be fixed so those
  optimized
  crypto modules for Intel cpu that support them can be loaded?
  
  I think this is an ordering issue between udev startup and the time when
  devices are registered.
  
  I wonder what happens if you put those modules into the initramfs image?
  
 
 OK, this bothers me on some pretty deep level... a set of changes
 exclusively in drivers/acpi breaking functionality which had nothing to
 do with ACPI, specifically CPU-feature-based module loading.

Well, they are not exclusively in drivers/acpi, they are in drivers/base/cpu.c
too and that most likely is the responsible part.

 Please let me know what the investigation comes up with, or if I need to
 get more directly involved.

I will.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3.11-rc1] crypto: Fix boot failure due to module dependency.

2013-07-19 Thread Tim Chen
On Fri, 2013-07-19 at 16:49 +0200, Rafael J. Wysocki wrote:
 This should cause udev to load the crct10dif_pclml module when cpu
 support the PCLMULQDQ (feature code 0081).  I did my testing during
 development on 3.10 and the module was indeed loaded.

 However, I found that the following commit under 3.11-rc1 broke
 the mechanism after some bisection.

 commit ac212b6980d8d5eda705864fc5a8ecddc6d6eacc
 Author: Rafael J. Wysocki rafael.j.wyso...@intel.com
 Date:   Fri May 3 00:26:22 2013 +0200

  ACPI / processor: Use common hotplug infrastructure
  
  Split the ACPI processor driver into two parts, one that is
  non-modular, resides in the ACPI core and handles the enumeration
  and hotplug of processors and one that implements the rest of the
  existing processor driver functionality.
  
 Rafael, can you check and see if this can be fixed so those optimized
 crypto modules for Intel cpu that support them can be loaded?

I think this is an ordering issue between udev startup and the time 
when 
devices are registered.
   
   Something that can be fixed?
  
  I'm sure it can be fixes, but I'm not sure whether it's a udev bug or real
  breakage in the kernel yet.  I need to figure out that part.
 
 OK
 
 I wonder if the appended patch makes the issue go away for you?
 

Rafael, the patch did fix the cpu feature based module loading issue.
Thanks for your quick response.

Herbert, the patch below should fix the boot failure issue.  

I also added the change to rename crct10dif.c to crct10dif_generic.c 
and added MODULE_ALIAS(crct10dif) to  crct10dif_generic.c.  
This was according to our discussion:
https://lkml.org/lkml/2013/5/21/216  

However, when I have the

CONFIG_CRYPTO_CRCT10DIF=y
CONFIG_CRYPTO_CRCT10DIF_PCLMUL=m
CONFIG_CRC_T10DIF=y

combination, the crc-t10dif library function was still 
initialized before the crct10dif-pclmul.ko was loaded, 
leading to the generic version being used.  This
is also one of Tetsuo's concerns.  In theory, loading
the generic version should also load the pclmul version as
they have the same MODULE_ALIAS, before crc-t10dif library
try to allocate the crypto transform. 

Something else that I'm missing and need to be changed?

I will be away for a week without internet access so my
response will be slow.

Thanks.

Tim

Signed-off-by: Tim Chen tim.c.c...@linux.intel.com
---
Fix boot failure as crc-t10dif library may not cause crct10dif.ko
containing the generic algorithm to be loaded.

Rename crct10dif.c to crct10dif_generic.c and add module alias.
This should load all the modules supporting crct10dif algorithm
at the same time.
---
 crypto/Makefile | 2 +-
 crypto/{crct10dif.c = crct10dif_generic.c} | 1 +
 lib/crc-t10dif.c| 3 ++-
 3 files changed, 4 insertions(+), 2 deletions(-)
 rename crypto/{crct10dif.c = crct10dif_generic.c} (99%)

diff --git a/crypto/Makefile b/crypto/Makefile
index 2d5ed08..3fd76fa 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -83,7 +83,7 @@ obj-$(CONFIG_CRYPTO_ZLIB) += zlib.o
 obj-$(CONFIG_CRYPTO_MICHAEL_MIC) += michael_mic.o
 obj-$(CONFIG_CRYPTO_CRC32C) += crc32c.o
 obj-$(CONFIG_CRYPTO_CRC32) += crc32.o
-obj-$(CONFIG_CRYPTO_CRCT10DIF) += crct10dif.o
+obj-$(CONFIG_CRYPTO_CRCT10DIF) += crct10dif_generic.o
 obj-$(CONFIG_CRYPTO_AUTHENC) += authenc.o authencesn.o
 obj-$(CONFIG_CRYPTO_LZO) += lzo.o
 obj-$(CONFIG_CRYPTO_LZ4) += lz4.o
diff --git a/crypto/crct10dif.c b/crypto/crct10dif_generic.c
similarity index 99%
rename from crypto/crct10dif.c
rename to crypto/crct10dif_generic.c
index 92aca96..45a9acd 100644
--- a/crypto/crct10dif.c
+++ b/crypto/crct10dif_generic.c
@@ -176,3 +176,4 @@ module_exit(crct10dif_mod_fini);
 MODULE_AUTHOR(Tim Chen tim.c.c...@linux.intel.com);
 MODULE_DESCRIPTION(T10 DIF CRC calculation.);
 MODULE_LICENSE(GPL);
+MODULE_ALIAS(crct10dif);
diff --git a/lib/crc-t10dif.c b/lib/crc-t10dif.c
index d102d83..37b3eb4 100644
--- a/lib/crc-t10dif.c
+++ b/lib/crc-t10dif.c
@@ -15,7 +15,8 @@
 #include linux/init.h
 #include crypto/hash.h
 
-static struct crypto_shash *crct10dif_tfm;
+__u16 crc_t10dif_generic(__u16 crc, const unsigned char *buffer, size_t len);
+static struct crypto_shash *crct10dif_tfm = crc_t10dif_generic;
 
 __u16 crc_t10dif(const unsigned char *buffer, size_t len)
 {
-- 
1.7.11.7





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


Re: [PATCH 3.11-rc1] crypto: Fix boot failure due to module dependency.

2013-07-19 Thread Rafael J. Wysocki
On Friday, July 19, 2013 11:08:49 AM Tim Chen wrote:
 On Fri, 2013-07-19 at 16:49 +0200, Rafael J. Wysocki wrote:
  This should cause udev to load the crct10dif_pclml module when cpu
  support the PCLMULQDQ (feature code 0081).  I did my testing during
  development on 3.10 and the module was indeed loaded.
 
  However, I found that the following commit under 3.11-rc1 broke
  the mechanism after some bisection.
 
  commit ac212b6980d8d5eda705864fc5a8ecddc6d6eacc
  Author: Rafael J. Wysocki rafael.j.wyso...@intel.com
  Date:   Fri May 3 00:26:22 2013 +0200
 
   ACPI / processor: Use common hotplug infrastructure
   
   Split the ACPI processor driver into two parts, one that is
   non-modular, resides in the ACPI core and handles the 
  enumeration
   and hotplug of processors and one that implements the rest of 
  the
   existing processor driver functionality.
   
  Rafael, can you check and see if this can be fixed so those 
  optimized
  crypto modules for Intel cpu that support them can be loaded?
 
 I think this is an ordering issue between udev startup and the time 
 when 
 devices are registered.

Something that can be fixed?
   
   I'm sure it can be fixes, but I'm not sure whether it's a udev bug or real
   breakage in the kernel yet.  I need to figure out that part.
  
  OK
  
  I wonder if the appended patch makes the issue go away for you?
  
 
 Rafael, the patch did fix the cpu feature based module loading issue.
 Thanks for your quick response.

Alas, this is not the one I'd like to apply.

With that patch applied, new device objects are created to avoid binding the
processor driver directly to the cpu system device objects, because that
apparently confuses udev and it starts to ignore the cpu modalias once the
driver has been bound to any of those objects.

I've verified in the meantime that this indeed is the case.

A link to the patch in question: https://patchwork.kernel.org/patch/2830561/

Greg, I asked you some time ago whether or not it was possible for udev to stop
autoloading modules that matched the cpu modalias after a driver had been bound
to the cpu system device objects and you said no.  However, this time I can
say with certainty that that really is the case.  So, the question now is
whether or not we can do anything in the kernel to avoid that confusion in udev
instead of applying the patch linked above (which is beyond ugly in my not so
humble opinion)?

Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3.11-rc1] crypto: Fix boot failure due to module dependency.

2013-07-19 Thread Greg Kroah-Hartman
On Fri, Jul 19, 2013 at 11:38:04PM +0200, Rafael J. Wysocki wrote:
 Alas, this is not the one I'd like to apply.
 
 With that patch applied, new device objects are created to avoid binding the
 processor driver directly to the cpu system device objects, because that
 apparently confuses udev and it starts to ignore the cpu modalias once the
 driver has been bound to any of those objects.
 
 I've verified in the meantime that this indeed is the case.
 
 A link to the patch in question: https://patchwork.kernel.org/patch/2830561/
 
 Greg, I asked you some time ago whether or not it was possible for udev to 
 stop
 autoloading modules that matched the cpu modalias after a driver had been 
 bound
 to the cpu system device objects and you said no.  However, this time I can
 say with certainty that that really is the case.  So, the question now is
 whether or not we can do anything in the kernel to avoid that confusion in 
 udev
 instead of applying the patch linked above (which is beyond ugly in my not so
 humble opinion)?

udev isn't doing any module loading, 'modprobe' is just being called for
any new module alias that shows up in the system, and all of the drivers
that match it then get loaded.

How is it a problem if a module is attempted to be loaded that is
already loaded?  How is it a problem if a different module is loaded for
a device already bound to a driver?  Both of those should be total
no-ops for the kernel.

But, I don't know anything about the cpu code, how is loading a module
causing problems?  That sounds like it needs to be fixes, as any root
user can load modules whenever they want, you can't protect the kernel
from doing that.

thanks,

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


Re: [PATCH 3.11-rc1] crypto: Fix boot failure due to module dependency.

2013-07-19 Thread H. Peter Anvin
On 07/19/2013 04:16 PM, Greg Kroah-Hartman wrote:
 
 udev isn't doing any module loading, 'modprobe' is just being called for
 any new module alias that shows up in the system, and all of the drivers
 that match it then get loaded.
 
 How is it a problem if a module is attempted to be loaded that is
 already loaded?  How is it a problem if a different module is loaded for
 a device already bound to a driver?  Both of those should be total
 no-ops for the kernel.
 
 But, I don't know anything about the cpu code, how is loading a module
 causing problems?  That sounds like it needs to be fixes, as any root
 user can load modules whenever they want, you can't protect the kernel
 from doing that.
 

The issue here seems to be the dynamic binding nature of the crypto
subsystem.  When something needs crypto, it will request the appropriate
crypto module (e.g. crct10dif), which may race with detecting a specific
hardware accelerator based on CPUID or device information (e.g.
crct10dif_pclmul).

RAID has effectively the same issue, and we just solved it by
compiling in all the accelerators into the top-level module.

-hpa

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


Re: [PATCH 3.11-rc1] crypto: Fix boot failure due to module dependency.

2013-07-19 Thread Greg Kroah-Hartman
On Fri, Jul 19, 2013 at 04:21:09PM -0700, H. Peter Anvin wrote:
 On 07/19/2013 04:16 PM, Greg Kroah-Hartman wrote:
  
  udev isn't doing any module loading, 'modprobe' is just being called for
  any new module alias that shows up in the system, and all of the drivers
  that match it then get loaded.
  
  How is it a problem if a module is attempted to be loaded that is
  already loaded?  How is it a problem if a different module is loaded for
  a device already bound to a driver?  Both of those should be total
  no-ops for the kernel.
  
  But, I don't know anything about the cpu code, how is loading a module
  causing problems?  That sounds like it needs to be fixes, as any root
  user can load modules whenever they want, you can't protect the kernel
  from doing that.
  
 
 The issue here seems to be the dynamic binding nature of the crypto
 subsystem.  When something needs crypto, it will request the appropriate
 crypto module (e.g. crct10dif), which may race with detecting a specific
 hardware accelerator based on CPUID or device information (e.g.
 crct10dif_pclmul).
 
 RAID has effectively the same issue, and we just solved it by
 compiling in all the accelerators into the top-level module.

Then there's nothing to be done in udev or kmod, right?

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


Re: [PATCH 3.11-rc1] crypto: Fix boot failure due to module dependency.

2013-07-19 Thread Herbert Xu
On Fri, Jul 19, 2013 at 04:21:09PM -0700, H. Peter Anvin wrote:

 The issue here seems to be the dynamic binding nature of the crypto
 subsystem.  When something needs crypto, it will request the appropriate
 crypto module (e.g. crct10dif), which may race with detecting a specific
 hardware accelerator based on CPUID or device information (e.g.
 crct10dif_pclmul).
 
 RAID has effectively the same issue, and we just solved it by
 compiling in all the accelerators into the top-level module.

I think for crypto the simplest solution is to not do CPUID-based
loading.  Then crypto users will simply load the module alias which
causes modprobe to load all modules providing that alias.

Cheers,
-- 
Email: Herbert Xu herb...@gondor.apana.org.au
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3.11-rc1] crypto: Fix boot failure due to module dependency.

2013-07-19 Thread H. Peter Anvin
On 07/19/2013 04:26 PM, Greg Kroah-Hartman wrote:

 RAID has effectively the same issue, and we just solved it by
 compiling in all the accelerators into the top-level module.
 
 Then there's nothing to be done in udev or kmod, right?
 

I don't know.

-hpa


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


Re: [PATCH 3.11-rc1] crypto: Fix boot failure due to module dependency.

2013-07-19 Thread Tim Chen
On Sat, 2013-07-20 at 09:24 +1000, Herbert Xu wrote:
 On Fri, Jul 19, 2013 at 04:21:09PM -0700, H. Peter Anvin wrote:
 
  The issue here seems to be the dynamic binding nature of the crypto
  subsystem.  When something needs crypto, it will request the appropriate
  crypto module (e.g. crct10dif), which may race with detecting a specific
  hardware accelerator based on CPUID or device information (e.g.
  crct10dif_pclmul).
  
  RAID has effectively the same issue, and we just solved it by
  compiling in all the accelerators into the top-level module.
 
 I think for crypto the simplest solution is to not do CPUID-based
 loading.  Then crypto users will simply load the module alias which
 causes modprobe to load all modules providing that alias.
 
 Cheers,

Herbert,

I've tried the module alias approach (see my earlier mail with patch) 
but it didn't seem to load things properly.  Can you check to see if 
there's anything I did incorrectly.

Tim

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


Re: [PATCH 3.11-rc1] crypto: Fix boot failure due to module dependency.

2013-07-19 Thread Rafael J. Wysocki
On Friday, July 19, 2013 04:16:30 PM Greg Kroah-Hartman wrote:
 On Fri, Jul 19, 2013 at 11:38:04PM +0200, Rafael J. Wysocki wrote:
  Alas, this is not the one I'd like to apply.
  
  With that patch applied, new device objects are created to avoid binding the
  processor driver directly to the cpu system device objects, because that
  apparently confuses udev and it starts to ignore the cpu modalias once the
  driver has been bound to any of those objects.
  
  I've verified in the meantime that this indeed is the case.
  
  A link to the patch in question: https://patchwork.kernel.org/patch/2830561/
  
  Greg, I asked you some time ago whether or not it was possible for udev to 
  stop
  autoloading modules that matched the cpu modalias after a driver had been 
  bound
  to the cpu system device objects and you said no.  However, this time I 
  can
  say with certainty that that really is the case.  So, the question now is
  whether or not we can do anything in the kernel to avoid that confusion in 
  udev
  instead of applying the patch linked above (which is beyond ugly in my not 
  so
  humble opinion)?
 
 udev isn't doing any module loading, 'modprobe' is just being called for
 any new module alias that shows up in the system, and all of the drivers
 that match it then get loaded.

The problem is that that doesn't happen when a driver is bound to the
cpu system device objects.  modprobe is just not called for modules that
match the cpu modalias in that case.

If I call modprobe manually for any of the modules in question, it loads
and works no problem.

 How is it a problem if a module is attempted to be loaded that is
 already loaded?  How is it a problem if a different module is loaded for
 a device already bound to a driver?  Both of those should be total
 no-ops for the kernel.

Precisely, but that's not what happens in practice, hence my question.

The situation is this:

- With 3.11-rc1 modules that match the CPU modalias are not loaded
  automatically (that is, modprobe is not called for them by udev) after
  the processor module is loaded.
- With 3.10 they are loaded automatically at any time.
- With the patch at https://patchwork.kernel.org/patch/2830561/ on top of
  3.11-rc1 the situation is the same as for 3.10.

Therefore we have a kernel regression from 3.10 (with respect to the existing
user space which is udev) in 3.11-rc1 3.10 and the patch at
https://patchwork.kernel.org/patch/2830561/ makes that regression go away.

However, that patch is totally voodoo programming, so I wonder if there is any
saner alternative or we really need to do voodoo programming in the kernel to
work around udev's confusion.

 But, I don't know anything about the cpu code, how is loading a module
 causing problems?  That sounds like it needs to be fixes, as any root
 user can load modules whenever they want, you can't protect the kernel
 from doing that.

Yes, that needs to be fixed, but I don't know *why* it is a problem in the
first place.  I'd like to understand what's going on, because I don't
right now.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3.11-rc1] crypto: Fix boot failure due to module dependency.

2013-07-19 Thread Tim Chen
On Fri, 2013-07-19 at 16:37 -0700, Tim Chen wrote:
 On Sat, 2013-07-20 at 09:24 +1000, Herbert Xu wrote:
  On Fri, Jul 19, 2013 at 04:21:09PM -0700, H. Peter Anvin wrote:
  
   The issue here seems to be the dynamic binding nature of the crypto
   subsystem.  When something needs crypto, it will request the appropriate
   crypto module (e.g. crct10dif), which may race with detecting a specific
   hardware accelerator based on CPUID or device information (e.g.
   crct10dif_pclmul).
   
   RAID has effectively the same issue, and we just solved it by
   compiling in all the accelerators into the top-level module.
  
  I think for crypto the simplest solution is to not do CPUID-based
  loading.  Then crypto users will simply load the module alias which
  causes modprobe to load all modules providing that alias.
  
  Cheers,
 
 Herbert,
 
 I've tried the module alias approach (see my earlier mail with patch) 
 but it didn't seem to load things properly.  Can you check to see if 
 there's anything I did incorrectly.
 
 Tim

I fixed a missing request_module statement in crct10dif library.  
So now things work if I have the following config:

CONFIG_CRYPTO_CRCT10DIF=m
CONFIG_CRYPTO_CRCT10DIF_PCLMUL=m
CONFIG_CRC_T10DIF=m

However, when I have the library and generic algorithm compiled in,
I do not see the PCLMULQDQ version loaded.

CONFIG_CRYPTO_CRCT10DIF=y
CONFIG_CRYPTO_CRCT10DIF_PCLMUL=m
CONFIG_CRC_T10DIF=y

Perhaps I am initiating the crct10dif library at a really early
stage when things are compiled in, where the module is not in 
initramfs?  In that case, perhaps we should only allow 
PCLMUL version to be compiled in
and not exist as a module?

Here's the patch I tried:

Signed-off-by: Tim Chen tim.c.c...@linux.intel.com
---
 arch/x86/crypto/crct10dif-pclmul_glue.c | 8 +---
 crypto/Makefile | 2 +-
 crypto/{crct10dif.c = crct10dif_generic.c} | 1 +
 lib/crc-t10dif.c| 1 +
 4 files changed, 4 insertions(+), 8 deletions(-)
 rename crypto/{crct10dif.c = crct10dif_generic.c} (99%)

diff --git a/arch/x86/crypto/crct10dif-pclmul_glue.c 
b/arch/x86/crypto/crct10dif-pclmul_glue.c
index 7845d7f..7ad5f09 100644
--- a/arch/x86/crypto/crct10dif-pclmul_glue.c
+++ b/arch/x86/crypto/crct10dif-pclmul_glue.c
@@ -121,15 +121,9 @@ static struct shash_alg alg = {
}
 };
 
-static const struct x86_cpu_id crct10dif_cpu_id[] = {
-   X86_FEATURE_MATCH(X86_FEATURE_PCLMULQDQ),
-   {}
-};
-MODULE_DEVICE_TABLE(x86cpu, crct10dif_cpu_id);
-
 static int __init crct10dif_intel_mod_init(void)
 {
-   if (!x86_match_cpu(crct10dif_cpu_id))
+   if (!cpu_has_pclmulqdq)
return -ENODEV;
 
return crypto_register_shash(alg);
diff --git a/crypto/Makefile b/crypto/Makefile
index 2d5ed08..3fd76fa 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -83,7 +83,7 @@ obj-$(CONFIG_CRYPTO_ZLIB) += zlib.o
 obj-$(CONFIG_CRYPTO_MICHAEL_MIC) += michael_mic.o
 obj-$(CONFIG_CRYPTO_CRC32C) += crc32c.o
 obj-$(CONFIG_CRYPTO_CRC32) += crc32.o
-obj-$(CONFIG_CRYPTO_CRCT10DIF) += crct10dif.o
+obj-$(CONFIG_CRYPTO_CRCT10DIF) += crct10dif_generic.o
 obj-$(CONFIG_CRYPTO_AUTHENC) += authenc.o authencesn.o
 obj-$(CONFIG_CRYPTO_LZO) += lzo.o
 obj-$(CONFIG_CRYPTO_LZ4) += lz4.o
diff --git a/crypto/crct10dif.c b/crypto/crct10dif_generic.c
similarity index 99%
rename from crypto/crct10dif.c
rename to crypto/crct10dif_generic.c
index 92aca96..c960a95 100644
--- a/crypto/crct10dif.c
+++ b/crypto/crct10dif_generic.c
@@ -176,3 +176,4 @@ module_exit(crct10dif_mod_fini);
 MODULE_AUTHOR(Tim Chen tim.c.c...@linux.intel.com);
 MODULE_DESCRIPTION(T10 DIF CRC calculation.);
 MODULE_LICENSE(GPL);
+MODULE_ALIAS(crct10dif);
diff --git a/lib/crc-t10dif.c b/lib/crc-t10dif.c
index fe3428c..d8cd353 100644
--- a/lib/crc-t10dif.c
+++ b/lib/crc-t10dif.c
@@ -38,6 +38,7 @@ EXPORT_SYMBOL(crc_t10dif);
 
 static int __init crc_t10dif_mod_init(void)
 {
+   request_module(crct10dif);
crct10dif_tfm = crypto_alloc_shash(crct10dif, 0, 0);
return PTR_RET(crct10dif_tfm);
 }
-- 
1.7.11.7



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


Re: [PATCH 3.11-rc1] crypto: Fix boot failure due to moduledependency.

2013-07-19 Thread Tetsuo Handa
Tim Chen wrote:
 On Fri, 2013-07-19 at 16:37 -0700, Tim Chen wrote:
  Herbert,
  
  I've tried the module alias approach (see my earlier mail with patch) 
  but it didn't seem to load things properly.  Can you check to see if 
  there's anything I did incorrectly.
  
  Tim
 
 I fixed a missing request_module statement in crct10dif library.  
 So now things work if I have the following config:
 
 CONFIG_CRYPTO_CRCT10DIF=m
 CONFIG_CRYPTO_CRCT10DIF_PCLMUL=m
 CONFIG_CRC_T10DIF=m
 
 However, when I have the library and generic algorithm compiled in,
 I do not see the PCLMULQDQ version loaded.
 
 CONFIG_CRYPTO_CRCT10DIF=y
 CONFIG_CRYPTO_CRCT10DIF_PCLMUL=m
 CONFIG_CRC_T10DIF=y
 
 Perhaps I am initiating the crct10dif library at a really early
 stage when things are compiled in, where the module is not in 
 initramfs?  In that case, perhaps we should only allow 
 PCLMUL version to be compiled in
 and not exist as a module?

I think that use of request_module(crct10dif) does not help loading
crct10dif-pclmul.ko when CONFIG_CRC_T10DIF=y CONFIG_CRYPTO_CRCT10DIF_PCLMUL=m ,
for there is no / directory (note that the initramfs is not yet mounted as /
for loading modules which are not in vmlinux) when any module_init() functions
which are in vmlinux are called.
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3.11-rc1] crypto: Fix boot failure due to module dependency.

2013-07-19 Thread Rafael J. Wysocki
On Saturday, July 20, 2013 02:00:44 AM Rafael J. Wysocki wrote:
 On Friday, July 19, 2013 04:16:30 PM Greg Kroah-Hartman wrote:
  On Fri, Jul 19, 2013 at 11:38:04PM +0200, Rafael J. Wysocki wrote:
   Alas, this is not the one I'd like to apply.
   
   With that patch applied, new device objects are created to avoid binding 
   the
   processor driver directly to the cpu system device objects, because that
   apparently confuses udev and it starts to ignore the cpu modalias once the
   driver has been bound to any of those objects.
   
   I've verified in the meantime that this indeed is the case.
   
   A link to the patch in question: 
   https://patchwork.kernel.org/patch/2830561/
   
   Greg, I asked you some time ago whether or not it was possible for udev 
   to stop
   autoloading modules that matched the cpu modalias after a driver had been 
   bound
   to the cpu system device objects and you said no.  However, this time I 
   can
   say with certainty that that really is the case.  So, the question now is
   whether or not we can do anything in the kernel to avoid that confusion 
   in udev
   instead of applying the patch linked above (which is beyond ugly in my 
   not so
   humble opinion)?
  
  udev isn't doing any module loading, 'modprobe' is just being called for
  any new module alias that shows up in the system, and all of the drivers
  that match it then get loaded.
 
 The problem is that that doesn't happen when a driver is bound to the
 cpu system device objects.  modprobe is just not called for modules that
 match the cpu modalias in that case.
 
 If I call modprobe manually for any of the modules in question, it loads
 and works no problem.

OK, I know what's up.  udev has no rule that would allow it to load stuff on
the basis of MODALIAS if DRIVER is set in the event properties.

So, the following rule needs to be added to udev rules for things to work
as before:

DRIVER==processor, ENV{MODALIAS}==?*, IMPORT{builtin}=kmod load 
$env{MODALIAS}

To be precise, I added a file called 80-drivers.rules to /etc/udev/rules.d/
with the following contents:

ACTION==remove, GOTO=drivers_end

DRIVER==processor, ENV{MODALIAS}==?*, IMPORT{builtin}=kmod load 
$env{MODALIAS}

LABEL=drivers_end

but I'm not sure how to propagate this to the udev maintainers.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3.11-rc1] crypto: Fix boot failure due to module dependency.

2013-07-19 Thread Herbert Xu
On Fri, Jul 19, 2013 at 06:31:04PM -0700, Tim Chen wrote:

 However, when I have the library and generic algorithm compiled in,
 I do not see the PCLMULQDQ version loaded.
 
 CONFIG_CRYPTO_CRCT10DIF=y
 CONFIG_CRYPTO_CRCT10DIF_PCLMUL=m
 CONFIG_CRC_T10DIF=y

That is completely expected.  I don't really think we need to
do anything about this case.  After all, if the admin wants to
use the optimised version for CRC_T10DIF then they could simply
compile that in as well.

Cheers,
-- 
Email: Herbert Xu herb...@gondor.apana.org.au
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line unsubscribe linux-crypto in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3.11-rc1] crypto: Fix boot failure due to moduledependency.

2013-07-19 Thread Tetsuo Handa
Herbert Xu wrote:
 On Fri, Jul 19, 2013 at 06:31:04PM -0700, Tim Chen wrote:
 
  However, when I have the library and generic algorithm compiled in,
  I do not see the PCLMULQDQ version loaded.
  
  CONFIG_CRYPTO_CRCT10DIF=y
  CONFIG_CRYPTO_CRCT10DIF_PCLMUL=m
  CONFIG_CRC_T10DIF=y
 
 That is completely expected.  I don't really think we need to
 do anything about this case.  After all, if the admin wants to
 use the optimised version for CRC_T10DIF then they could simply
 compile that in as well.
 

Wow! ;-)

But I'd expect something like

 static int __init crc_t10dif_mod_init(void)
 {
+#if !defined(CONFIG_CRC_T10DIF_MODULE)  
defined(CONFIG_CRYPTO_CRCT10DIF_PCLMUL_MODULE)
+   printk(KERN_WARNING Consider CONFIG_CRYPTO_CRCT10DIF_PCLMUL=y for 
better performance\n);
+#endif
crct10dif_tfm = crypto_alloc_shash(crct10dif, 0, 0);
return PTR_RET(crct10dif_tfm);
 }

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