Skip to contentSkip to conversation

crypto: Remove pointless padlock module

PATCH In review Simon Arlott 4 participants 9 messages Apr 28, 2007 – May 20, 2007 Crypto API

lore
Share & tools
lore

Simon Arlott

author
#0 lore raw

PATCH 1/1 crypto: Remove pointless padlock module

When this is compiled in it is run too early to do anything useful:

[    6.052000] padlock: No VIA PadLock drivers have been loaded.
[    6.052000] padlock: Using VIA PadLock ACE for AES algorithm.
[    6.052000] padlock: Using VIA PadLock ACE for SHA1/SHA256 algorithms.

When it's a module it isn't doing anything special, the same functionality can be provided in userspace by "probeall padlock padlock-aes padlock-sha" in modules.conf if it is required.

Signed-off-by Simon Arlott <[email protected]>
Cc Herbert Xu <[email protected]>
Cc Michal Ludvig <[email protected]>

---

drivers/crypto/Kconfig   |   16 ++----------
drivers/crypto/Makefile  |    1 -
drivers/crypto/padlock.c |   58 ----------------------------------------------
3 files changed, 3 insertions(+), 72 deletions(-)

delete mode 100644 drivers/crypto/padlock.c

drivers/crypto/Kconfig +0 -0
index ff8c4be..f85cecc 100644
@@ -1,10 +1,10 @@

menu "Hardware crypto devices"

config CRYPTO_DEV_PADLOCK
- tristate "Support for VIA PadLock ACE"
- depends on X86_32
+ bool "Support for VIA PadLock ACE"
+ depends on CRYPTO && X86_32
+ default y
select CRYPTO_ALGAPI
- default m
help
Some VIA processors come with an integrated crypto engine (so called VIA PadLock ACE, Advanced Cryptography Engine)
@@ -14,16 +14,6 @@ config CRYPTO_DEV_PADLOCK
The instructions are used only when the CPU supports them. Otherwise software encryption is used.

-	  Selecting M for this option will compile a helper module
-	  padlock.ko that should autoload all below configured
-	  algorithms. Don't worry if your hardware does not support
-	  some or all of them. In such case padlock.ko will
-	  simply write a single line into the kernel log informing
-	  about its failure but everything will keep working fine.

-
- If you are unsure, say M. The compiled module will be
- called padlock.ko
-
config CRYPTO_DEV_PADLOCK_AES
tristate "PadLock driver for AES algorithm" depends on CRYPTO_DEV_PADLOCK

drivers/crypto/Makefile +0 -1
index 6059cf8..d070030 100644
@@ -1,4 +1,3 @@
-obj-$(CONFIG_CRYPTO_DEV_PADLOCK) += padlock.o

obj-$(CONFIG_CRYPTO_DEV_PADLOCK_AES) += padlock-aes.o obj-$(CONFIG_CRYPTO_DEV_PADLOCK_SHA) += padlock-sha.o obj-$(CONFIG_CRYPTO_DEV_GEODE) += geode-aes.o

drivers/crypto/padlock.c deleted +0 -58

58 lines not on this page · raw message

Signature · 4 lines
1.5.0.1

-- 
Simon Arlott
#1 lore raw
Quoted 26 lines · Simon Arlott

When this is compiled in it is run too early to do anything useful:

[    6.052000] padlock: No VIA PadLock drivers have been loaded.
[    6.052000] padlock: Using VIA PadLock ACE for AES algorithm.
[    6.052000] padlock: Using VIA PadLock ACE for SHA1/SHA256 algorithms.

When it's a module it isn't doing anything special, the same functionality can be provided in userspace by "probeall padlock padlock-aes padlock-sha" in modules.conf if it is required.

Signed-off-by Simon Arlott <[email protected]>
Cc Herbert Xu <[email protected]>
Cc Michal Ludvig <[email protected]>

---

drivers/crypto/Kconfig   |   16 ++----------
drivers/crypto/Makefile  |    1 -
drivers/crypto/padlock.c |   58 
----------------------------------------------
3 files changed, 3 insertions(+), 72 deletions(-)

delete mode 100644 drivers/crypto/padlock.c

drivers/crypto/Kconfig +0 -0
index ff8c4be..f85cecc 100644
@@ -1,10 +1,10 @@

menu "Hardware crypto devices"

config CRYPTO_DEV_PADLOCK

drivers/crypto/Kconfig

Quoted 4 lines
drivers/crypto/Kconfig +2 -2
-    tristate "Support for VIA PadLock ACE"
-    depends on X86_32
+    bool "Support for VIA PadLock ACE"
+    depends on CRYPTO && X86_32

All of drivers/crypto/Kconfig already depends on CRYPTO, so just

	depends on X86_32

should be enough.

Quoted 6 lines
drivers/crypto/Kconfig +1 -1
+    default y
    select CRYPTO_ALGAPI
-    default m
    help
      Some VIA processors come with an integrated crypto engine
      (so called VIA PadLock ACE, Advanced Cryptography Engine)
Signature · 2 lines
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
author
#2 lore raw

PATCH 1/1 crypto: Remove pointless padlock module

When this is compiled in it is run too early to do anything useful:

[    6.052000] padlock: No VIA PadLock drivers have been loaded.
[    6.052000] padlock: Using VIA PadLock ACE for AES algorithm.
[    6.052000] padlock: Using VIA PadLock ACE for SHA1/SHA256 algorithms.

When it's a module it isn't doing anything special, the same functionality can be provided in userspace by "probeall padlock padlock-aes padlock-sha" in modules.conf if it is required.

Signed-off-by Simon Arlott <[email protected]>
Cc Herbert Xu <[email protected]>
Cc Michal Ludvig <[email protected]>

---

Quoted 6 lines · Randy Dunlap
Quoted 1 line · Simon Arlott

+ depends on CRYPTO && X86_32

All of drivers/crypto/Kconfig already depends on CRYPTO, so just

    depends on X86_32

should be enough.

Ok, I've changed this for geode too.

Quoted 6 lines · Ingo Oeser
Quoted 1 line · Simon Arlott
Quoted 1 line

Ideally I'd just remove that module completely, all it does is trigger the loading of the other two modules when modules are used - so I'll submit a patch for that instead.

That's much better!

When you force a feature to be a module on a kernel without module support, it will effectivly be disabled.

Well that's mostly the point - it shouldn't get compiled in - ever, but it also has other modules depending on it in Kconfig that shouldn't need to be modules.

drivers/crypto/Kconfig   |   16 ++----------
drivers/crypto/Makefile  |    1 -
drivers/crypto/padlock.c |   58 ----------------------------------------------
3 files changed, 3 insertions(+), 72 deletions(-)

delete mode 100644 drivers/crypto/padlock.c

drivers/crypto/Kconfig +0 -0
index ff8c4be..f21fe66 100644
@@ -1,10 +1,10 @@

menu "Hardware crypto devices"

config CRYPTO_DEV_PADLOCK
- tristate "Support for VIA PadLock ACE"
+ bool "Support for VIA PadLock ACE"

	depends on X86_32
	select CRYPTO_ALGAPI

- default m
+ default y
help
Some VIA processors come with an integrated crypto engine (so called VIA PadLock ACE, Advanced Cryptography Engine)
@@ -14,16 +14,6 @@ config CRYPTO_DEV_PADLOCK
The instructions are used only when the CPU supports them. Otherwise software encryption is used.

-	  Selecting M for this option will compile a helper module
-	  padlock.ko that should autoload all below configured
-	  algorithms. Don't worry if your hardware does not support
-	  some or all of them. In such case padlock.ko will
-	  simply write a single line into the kernel log informing
-	  about its failure but everything will keep working fine.

-
- If you are unsure, say M. The compiled module will be
- called padlock.ko
-
config CRYPTO_DEV_PADLOCK_AES
tristate "PadLock driver for AES algorithm" depends on CRYPTO_DEV_PADLOCK
@@ -55,7 +45,7 @@ source "arch/s390/crypto/Kconfig"

config CRYPTO_DEV_GEODE
tristate "Support for the Geode LX AES engine"
- depends on CRYPTO && X86_32 && PCI
+ depends on X86_32 && PCI
select CRYPTO_ALGAPI
select CRYPTO_BLKCIPHER
default m

drivers/crypto/Makefile +0 -1
index 6059cf8..d070030 100644
@@ -1,4 +1,3 @@
-obj-$(CONFIG_CRYPTO_DEV_PADLOCK) += padlock.o

obj-$(CONFIG_CRYPTO_DEV_PADLOCK_AES) += padlock-aes.o obj-$(CONFIG_CRYPTO_DEV_PADLOCK_SHA) += padlock-sha.o obj-$(CONFIG_CRYPTO_DEV_GEODE) += geode-aes.o

drivers/crypto/padlock.c deleted +0 -58

58 lines not on this page · raw message

Signature · 4 lines
1.5.0.1

-- 
Simon Arlott
maintainer
#3 lore raw
Quoted 4 lines · Simon Arlott

Well that's mostly the point - it shouldn't get compiled in - ever, but it also has other modules depending on it in Kconfig that shouldn't need to be modules.

Patch applied. Thanks!

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
maintainer
#4 lore raw
In reply to Simon Arlott
Quoted 8 lines · Simon Arlott

When this is compiled in it is run too early to do anything useful:

[    6.052000] padlock: No VIA PadLock drivers have been loaded.
[    6.052000] padlock: Using VIA PadLock ACE for AES algorithm.
[    6.052000] padlock: Using VIA PadLock ACE for SHA1/SHA256 algorithms.

When it's a module it isn't doing anything special, the same functionality can be provided in userspace by "probeall padlock padlock-aes padlock-sha" in modules.conf if it is required.

BTW, I noticed that this prevented CRYPTO_ALGAPI from being marked as m since it was selected by CRYPTO_DEV_PADLOCK. So I'm turning it into a tristate again.

Cheers,

Signature · 6 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
d158325e407864793c5b0fbc85c74702753333b9
drivers/crypto/Kconfig +2 -2
index e678a33..bb90cbd 100644
@@ -1,10 +1,10 @@
 menu "Hardware crypto devices"
 
 config CRYPTO_DEV_PADLOCK
-	bool "Support for VIA PadLock ACE"
+	tristate "Support for VIA PadLock ACE"
 	depends on X86_32
 	select CRYPTO_ALGAPI
-	default y
+	default m
 	help
 	  Some VIA processors come with an integrated crypto engine
 	  (so called VIA PadLock ACE, Advanced Cryptography Engine)
author
#5 lore raw
Quoted 11 lines · Herbert Xu
Quoted 6 lines · Simon Arlott

When this is compiled in it is run too early to do anything useful:

[    6.052000] padlock: No VIA PadLock drivers have been loaded.
[    6.052000] padlock: Using VIA PadLock ACE for AES algorithm.
[    6.052000] padlock: Using VIA PadLock ACE for SHA1/SHA256 algorithms.

When it's a module it isn't doing anything special, the same functionality can be provided in userspace by "probeall padlock padlock-aes padlock-sha" in modules.conf if it is required.

BTW, I noticed that this prevented CRYPTO_ALGAPI from being marked as m since it was selected by CRYPTO_DEV_PADLOCK. So I'm turning it into a tristate again.

It should be a bool that doesn't select anything, the AES and SHA modules will select CRYPTO_ALGAPI. It could also depend on MVIAC3_2 || MVIA_C7 instead of X86_32.

Signature · 1 line
Simon Arlott
#6 lore raw
Quoted 7 lines · Simon Arlott
Quoted 3 lines

BTW, I noticed that this prevented CRYPTO_ALGAPI from being marked as m since it was selected by CRYPTO_DEV_PADLOCK. So I'm turning it into a tristate again.

It should be a bool that doesn't select anything, the AES and SHA modules will select CRYPTO_ALGAPI. It could also depend on MVIAC3_2 || MVIA_C7 instead of X86_32.

Don't forget it's possible to build 'generic' x86 kernels, which would also want this module.

Dave

Signature · 1 line
http://www.codemonkey.org.uk
maintainer
#7 lore raw
In reply to Simon Arlott
Quoted 4 lines · Simon Arlott

It should be a bool that doesn't select anything, the AES and SHA modules will select CRYPTO_ALGAPI. It could also depend on MVIAC3_2 || MVIA_C7 instead of X86_32.

Having it as a tristate means that we don't have to duplicate the dependencies and selects that each padlock algorithm would otherwise do. So is there actually a problem with it being a tristate?

Cheers,

Signature · 4 lines
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
author
#8 lore raw
Quoted 6 lines · Herbert Xu
Quoted 1 line · Simon Arlott

It should be a bool that doesn't select anything, the AES and SHA modules will select CRYPTO_ALGAPI. It could also depend on MVIAC3_2 || MVIA_C7 instead of X86_32.

Having it as a tristate means that we don't have to duplicate the dependencies and selects that each padlock algorithm would otherwise do. So is there actually a problem with it being a tristate?

It has nothing to compile as a module, so M makes no sense. Each algorithm already selects CRYPTO_ALGAPI indirectly.

Signature · 1 line
Simon Arlott