Hello James,
I'm currently looking at the crypto API and considering adding support for
new hardware instructions implemented in the IBM z990 architecture. Since I
found your name in most of the files I find it appropriate to ask for your
opinion on how to integrate the new code (from a design point of view).
z990 provides hardware support for AES, DES and SHA. The problem is, that
the respective instructions might not be implemented on all z990 systems
(export restrictions etc). Hence, a check must be run to test whether the
instruction set is present, and if not, a fall-back to the current software
implementation must be taken. I basically have two solutions in mind: (1)
to integrate the new code into the current crypto files; add some #ifdef s
to prevent the code from being compiled when building a non-z990 kernel;
add some ifs for runtime check. (2) include the new code into an
arch/s390/crypto directory.
The advantage of (1) is that there are no seperate crypto directories, the
code doesn't drift apart. Furthermore, it's probably the best solution with
respect to the kernel module loader. On the other hand, the hardware
support is very arch-specific, which would fit in option (2). (2) however
has the disadvantage that there are multiple crypto modules; the user has
to select one to load -> must have different names for one algorithm.
What is your opinion on this subject?
Regards,
Thomas Spatzier
--------------------------------------------------
+49-7031-16-1219
[email protected]
On Wed, 2 Jul 2003, Thomas Spatzier wrote:
> Hello James,
>
> I'm currently looking at the crypto API and considering adding support for
> new hardware instructions implemented in the IBM z990 architecture. Since I
> found your name in most of the files I find it appropriate to ask for your
> opinion on how to integrate the new code (from a design point of view).
> z990 provides hardware support for AES, DES and SHA. The problem is, that
> the respective instructions might not be implemented on all z990 systems
> (export restrictions etc). Hence, a check must be run to test whether the
> instruction set is present, and if not, a fall-back to the current software
> implementation must be taken.
Are there any details available on how all of this is implemented? Are
these instructions synchronous?
> I basically have two solutions in mind: (1)
> to integrate the new code into the current crypto files; add some #ifdef s
> to prevent the code from being compiled when building a non-z990 kernel;
> add some ifs for runtime check.
No, the core crypto code should not be altered with #ifdefs to handle some
arch specific issue.
> (2) include the new code into an arch/s390/crypto directory. The
> advantage of (1) is that there are no seperate crypto directories, the
> code doesn't drift apart. Furthermore, it's probably the best solution
> with respect to the kernel module loader. On the other hand, the
> hardware support is very arch-specific, which would fit in option (2).
> (2) however has the disadvantage that there are multiple crypto modules;
> the user has to select one to load -> must have different names for one
> algorithm. What is your opinion on this subject?
The plan is to provide crypto/arch/ subdirectories where arch optimized
versions of the crypto algorithms are implemented, and built automatically
(via configuration defaults) instead of the generic C versions.
So, there might be:
crypto/aes.c
crypto/arch/i386/aes.s
where on i386, aes.s would be built into aes.o and aes.c would not be
built.
The simple solution for you might be something like:
crypto/aes.c -> aes.o
crypto/arch/s390/aes_z990.c -> aes_z990.o
and the administrator of the system could configure modprobe.conf to alias
aes to aes_z990 if the latter is supported in hardware.
- James
--
James Morris
<[email protected]>
> Are there any details available on how all of this is implemented? Are
> these instructions synchronous?
Yes, the instructions are sunchronous.
> The plan is to provide crypto/arch/ subdirectories where arch optimized
> versions of the crypto algorithms are implemented, and built
automatically
> (via configuration defaults) instead of the generic C versions.
>
> So, there might be:
>
> crypto/aes.c
> crypto/arch/i386/aes.s
>
> where on i386, aes.s would be built into aes.o and aes.c would not be
> built.
>
> The simple solution for you might be something like:
>
> crypto/aes.c -> aes.o
> crypto/arch/s390/aes_z990.c -> aes_z990.o
>
> and the administrator of the system could configure modprobe.conf to
alias
> aes to aes_z990 if the latter is supported in hardware.
I agree on having arch-specific implementations in crypto/arch directories.
However, I've got some problems with having this kind of 'static' aliasing
in modules.conf. In my case I do not know beforehand, whether a specific
crypto instruction is enabled. I query some processor status flags during
runime. If the check fails, I'd like to switch back to the original
software implementation.
I read in your autoconf.c file that a more sophisticated version of
crypto_alg_autoload is planned. I would suggest first trying to load the
arch-specific aes_z990.ko in crypto_alg_autoload. In my init_module
function i could query the processor. If init_module for my implementation
fails -> request_module fails -> load the standard module aes.ko.
Something similar to this:
#ifndef CRYPTO_ARCH //defined in arch makefile as "_z990"
#define CRYPTO_ARCH ""
#endif
void crypto_alg_autoload(const char *name)
{
if (request_module("%s%s", name, CRYPTO_ARCH) != 0){
request_module("%s", name);
}
}
Regards,
Thomas Spatzier
--------------------------------------------------
+49-7031-16-1219
[email protected]
On Wed, 2 Jul 2003, Thomas Spatzier wrote:
> I agree on having arch-specific implementations in crypto/arch directories.
> However, I've got some problems with having this kind of 'static' aliasing
> in modules.conf. In my case I do not know beforehand, whether a specific
> crypto instruction is enabled. I query some processor status flags during
> runime. If the check fails, I'd like to switch back to the original
> software implementation.
I did say it was the simple solution :-)
> I read in your autoconf.c file that a more sophisticated version of
> crypto_alg_autoload is planned. I would suggest first trying to load the
> arch-specific aes_z990.ko in crypto_alg_autoload. In my init_module
> function i could query the processor. If init_module for my implementation
> fails -> request_module fails -> load the standard module aes.ko.
> Something similar to this:
>
> #ifndef CRYPTO_ARCH //defined in arch makefile as "_z990"
> #define CRYPTO_ARCH ""
> #endif
>
> void crypto_alg_autoload(const char *name)
> {
> if (request_module("%s%s", name, CRYPTO_ARCH) != 0){
> request_module("%s", name);
> }
> }
I'd like to avoid these kind of macros, and make it a general case
solution (e.g. which can be used for various hardware implementations).
One possibility would be to allow registration with an alias list in
crypto API with attributes indicating whether the module is hardware,
arch-specific etc.
So, during init, the s390 arch could register an alias like:
name .= "aes_z990"
algorithm .= "aes"
attributes .= CRYPTO_ATTR_ARCH|CRYPTO_ATTR_HW
Then, when a caller specifies "aes", crypto_alg_autoload() would first
check the alias list, giving preference to CRYPTO_ALG_ARCH by default.
In this case, it would find aes_z990 and try and load it. If this fails,
it continues along the alias list then ultimately falls back to the
current behavior.
This would allow subsystems and hardware drivers to dynamically provide
information to the API. Callers of crypto_alloc_tfm() could then pass
flags specifying preferences for algorithm implementations e.g. generic
sw, arch sw, arch hw, misc hw.
This would also need to take into account modprobe.conf configuration and
compiled in modules.
- James
--
James Morris
<[email protected]>
James Morris wrote:
>Are there any details available on how all of this is implemented? Are
>these instructions synchronous?
FYI, the relevant instructions are documented in the z/Architecture
Principles of Operation, available on the Web at:
http://publibfp.boulder.ibm.com/cgi-bin/bookmgr/BOOKS/dz9zr002
Check Chapter 7 for the instructions:
CHIPER MESSAGE (KM)
CHIPER MESSAGE WITH CHAINING (KMC)
COMPUTE INTERMEDIATE MESSAGE DIGEST (KIMD)
COMPUTE LAST MESSAGE DIGEST (KLMD)
COMPUTE MESSAGE AUTHENTICATION CODE (KMAC)
Bye,
Ulrich
--
Dr. Ulrich Weigand
[email protected]
James Morris wrote:
> On Wed, 2 Jul 2003, Thomas Spatzier wrote:
>
> I'd like to avoid these kind of macros, and make it a general case
> solution (e.g. which can be used for various hardware implementations).
Right.
> One possibility would be to allow registration with an alias list in
> crypto API with attributes indicating whether the module is hardware,
> arch-specific etc.
Still too complicated imho. The built-in code should not need to know
about which modules exist. For example that would prevent autoloading
of third-party hardware crypto drivers. In most cases, a
first-come-first-serve approach is sufficient and works without
changes to the current code. Modules implementing the same algorithm
can just set .cra_name to the same value.
For built-in drivers, link order decides which implementation is
preferred. Consequently, hardware crypto drivers need to come before
software implementations and must not register themselves if the
hardware is not found at initcall time.
For the module case, the aes-z990.o module could declare
'MODULE_ALIAS(aes-hw);', the simple patch below makes sure
that any aes-hw module is preferred to the software aes
module. If there is more than one hardware implementation
available for an architecture, either the autoloader can be
extended further, or modprobe has to be configured
appropriately.
Arnd <><
===== crypto/autoload.c 1.7 vs edited =====
--- 1.7/crypto/autoload.c Sat May 17 21:39:13 2003
+++ edited/crypto/autoload.c Wed Jul 2 23:48:10 2003
@@ -21,16 +21,15 @@
* A far more intelligent version of this is planned. For now, just
* try an exact match on the name of the algorithm.
*/
-void crypto_alg_autoload(const char *name)
-{
- request_module("%s", name);
-}
-
struct crypto_alg *crypto_alg_mod_lookup(const char *name)
{
struct crypto_alg *alg = crypto_alg_lookup(name);
if (alg == NULL) {
- crypto_alg_autoload(name);
+ request_module("%s-hw", name);
+ alg = crypto_alg_lookup(name);
+ }
+ if (alg == NULL) {
+ request_module("%s", name);
alg = crypto_alg_lookup(name);
}
return alg;
On Thu, 3 Jul 2003, Arnd Bergmann wrote:
> For built-in drivers, link order decides which implementation is
> preferred. Consequently, hardware crypto drivers need to come before
> software implementations and must not register themselves if the
> hardware is not found at initcall time.
>
> For the module case, the aes-z990.o module could declare
> 'MODULE_ALIAS(aes-hw);', the simple patch below makes sure
> that any aes-hw module is preferred to the software aes
> module. If there is more than one hardware implementation
> available for an architecture, either the autoloader can be
> extended further, or modprobe has to be configured
> appropriately.
While this looks like it will work fine for the z990, it is a special case
which does not address other requirements for hardware support (some
initial requirements are listed at
http://www.intercode.com.au/jamesm/crypto/hardware_notes.txt).
I'm not enthusiastic about adding infrastructure which is really just a
hack for some quaint hardware, and probably does not work towards
addressing more common hardware requirements.
- James
--
James Morris
<[email protected]>
On Sunday 06 July 2003 16:08, James Morris wrote:
> While this looks like it will work fine for the z990, it is a special case
> which does not address other requirements for hardware support (some
> initial requirements are listed at
> http://www.intercode.com.au/jamesm/crypto/hardware_notes.txt).
>
> I'm not enthusiastic about adding infrastructure which is really just a
> hack for some quaint hardware, and probably does not work towards
> addressing more common hardware requirements.
Ok, then I guess the module will simply have to declare MODULE_ALIAS("aes")
and live in arch/s390/crypto/, which means that the common code
is not touched at all, but building both the z990 assembler as well
as the C implementation as modules requires editing /etc/modprobe.conf
to get the right one.
As soon as you have the new API for crypto cards, we can move to that
for autoprobing the CPU features and reliably using the right
implementation.
Maybe you can add to your list something like the following items:
Requirements:
- Support for CPU specific optimized algorithms:
- autodetection of CPU features (e.g. Pentium MMX or z990 crypto)
- selection of different implementations. A high priority job
probably wants to use the CPU while another job offloads crypto
to an asynchronous add-on card.
Hardware Documentation status:
- IBM zSeries cryptographic instructions:
http://publibfp.boulder.ibm.com/cgi-bin/bookmgr/BOOKS/dz9zr002/7.5.25
GPL Driver status:
- IBM PCICC and PCICA cards (incompatible API):
Robert Burroughs <[email protected]>
http://oss.software.ibm.com/developerworks/opensource/linux390/june2003_recommended.shtml
- IBM zSeries cryptographic instructions:
Thomas Spatzier (work in progress)
Arnd <><
On Wed, Jul 02, 2003 at 07:35:16PM +1000, James Morris wrote:
> The plan is to provide crypto/arch/ subdirectories where arch optimized
> versions of the crypto algorithms are implemented, and built automatically
> (via configuration defaults) instead of the generic C versions.
>
> So, there might be:
>
> crypto/aes.c
> crypto/arch/i386/aes.s
crypto/arch/ sounds like a bad idea. We really should avoid arch code
outside arch/ and include/asm*. So arch/<foo>/crypto/ as suggested by
Thomas is much better.
> where on i386, aes.s would be built into aes.o and aes.c would not be
> built.
That's a really bad idea. Think of a i586/i686 optimized assembler
implementaion e.g. using MMX or SSE or whatever. You'll always need
the generic version as fallback.
> The simple solution for you might be something like:
>
> crypto/aes.c -> aes.o
> crypto/arch/s390/aes_z990.c -> aes_z990.o
>
> and the administrator of the system could configure modprobe.conf to alias
> aes to aes_z990 if the latter is supported in hardware.
Right. And IMHO this should happen with all optimized version - putting
policy in the kernel to select them sounds like a bad idea, especially
as it could get rather complicated when it involves multiple optimized
and / or hardware implementations.
On Thu, Jul 03, 2003 at 02:57:30AM +1000, James Morris wrote:
> Then, when a caller specifies "aes", crypto_alg_autoload() would first
> check the alias list, giving preference to CRYPTO_ALG_ARCH by default.
> In this case, it would find aes_z990 and try and load it. If this fails,
> it continues along the alias list then ultimately falls back to the
> current behavior.
This sounds like the right way to do it. The question is just whether we
want to put that complicated policy into the kernel or into some userspace
helper.
On Mon, Jul 07, 2003 at 12:08:37AM +1000, James Morris wrote:
> I'm not enthusiastic about adding infrastructure which is really just a
> hack for some quaint hardware, and probably does not work towards
> addressing more common hardware requirements.
The z990 isn't really crypto hw in the traditional sense, it has special
instructions in the CPU so from Linux POV it's more an extremly optimized
SW implementation..
And we have to solve this anyway even iof support for external crypto
hw is delayed. Think of integrating Jari's x86 assembly aes implementation
or similar things.
On Mon, 7 Jul 2003, Christoph Hellwig wrote:
> This sounds like the right way to do it. The question is just whether we
> want to put that complicated policy into the kernel or into some userspace
> helper.
A userspace helper sounds interesting, as we also need a way to allow
unprivileged users to invoke kernel crypto, which may require modules to
be loaded.
- James
--
James Morris
<[email protected]>
From: Christoph Hellwig <[email protected]>
Date: Mon, 7 Jul 2003 08:09:29 +0100
On Wed, Jul 02, 2003 at 07:35:16PM +1000, James Morris wrote:
> So, there might be:
>
> crypto/aes.c
> crypto/arch/i386/aes.s
crypto/arch/ sounds like a bad idea. We really should avoid arch code
outside arch/ and include/asm*. So arch/<foo>/crypto/ as suggested by
Thomas is much better.
I totally disagree. I think the way we do things today is _STUPID_.
We put arch code far away from the generic version which makes finding
stuff very difficult for people inspecting the code for the first time.
For example, the fact that I have to go groveling in
arch/foo/lib/whoknowswhatfile.whoknowswhatextension to look at
the memcpy/checksum/whatever implementation is completely busted.
So, I totally support making crypto/arch/ directories.
Christoph> crypto/arch/ sounds like a bad idea. We really should
Christoph> avoid arch code outside arch/ and include/asm*. So
Christoph> arch/<foo>/crypto/ as suggested by Thomas is much
Christoph> better.
David> I totally disagree. I think the way we do things today is
David> _STUPID_. We put arch code far away from the generic
David> version which makes finding stuff very difficult for people
David> inspecting the code for the first time.
David> For example, the fact that I have to go groveling in
David> arch/foo/lib/whoknowswhatfile.whoknowswhatextension to look
David> at the memcpy/checksum/whatever implementation is
David> completely busted.
I see your point. Still, I think there is a lot to be said for
keeping arch code in arch/xxx and include/asm-xxx. It means that
someone working on a new port (I don't necessarily mean a totally new
arch, but also adding support for some new CPU model or platform) has
a well-defined set of directories to look at.
It's also nice that the xxx-arch maintainers can say "we are the rulers
of arch/xxx and include/asm-xxx" and know that any changes outside of
those directories have to go through lkml.
By the way, I agree that it would be good if <asm/string.h> had
something like
/* See arch-xxx/lib/string.S for implementation of these */
Still, I don't think I would like it if we had
alpha/ arm/ arm26/ cris/ h8300/ i386/ ia64/ m68k/ m68knommu/
mips/ mips64/ parisc ppc/ ppc64/ s390/ sh/ sparc/ sparc64/ um/
v850/ x86_64/ generic/
directories scattered all over the source tree.
- Roland
From: Roland Dreier <[email protected]>
Date: 07 Jul 2003 20:37:09 -0700
Still, I think there is a lot to be said for keeping arch code in
arch/xxx and include/asm-xxx. It means that someone working on a
new port (I don't necessarily mean a totally new arch, but also
adding support for some new CPU model or platform) has a
well-defined set of directories to look at.
I again disagree. We're talking about things here where
the default you get is _working_.
Only if you want to enhance or _optimize_ your port do you
need to modify any of this crap.
In this way it's fundamentally different from things that
one normally finds under arch/foo and include/asm-foo
It's also nice that the xxx-arch maintainers can say "we are the rulers
of arch/xxx and include/asm-xxx" and know that any changes outside of
those directories have to go through lkml.
This isn't nice, it's rather bad for this case.
I think it'd be great that the "crypto maintainer" can be the one
by which "crypto changes" need to go through. So again, I totally
disagree with your assesment.
Still, I don't think I would like it if we had
alpha/ arm/ arm26/ cris/ h8300/ i386/ ia64/ m68k/ m68knommu/
mips/ mips64/ parisc ppc/ ppc64/ s390/ sh/ sparc/ sparc64/ um/
v850/ x86_64/ generic/
directories scattered all over the source tree.
I see no problem with this at all. In fact, I wish we had a much
higher directory to file ratio in the kernel tree.
And hey, if I went "find crypto -type d -name sparc" I'd know if there
are sparc optimizations for the crypto library. How might you do this
with the current "everything and it's mother under arch/" scheme?
Answer: you can't.
David S. Miller wrote:
> I totally disagree. I think the way we do things today is _STUPID_.
> We put arch code far away from the generic version which makes finding
> stuff very difficult for people inspecting the code for the first time.
>
> For example, the fact that I have to go groveling in
> arch/foo/lib/whoknowswhatfile.whoknowswhatextension to look at
> the memcpy/checksum/whatever implementation is completely busted.
Hear ! Hear ! Maybe I could also get you interested in the idea
of moving headers with inline functions to the only spot where
they are actually used ?
E.g. most of include/net/tcp.h pretty much only matters for
net/ipv4/. It would be so nice if a grep -w thing *.[ch] in
net/ipv4/ would really find all uses of "thing".
- Werner
--
_________________________________________________________________________
/ Werner Almesberger, Buenos Aires, Argentina [email protected] /
/_http://www.almesberger.net/____________________________________________/
From: Werner Almesberger <[email protected]>
Date: Wed, 9 Jul 2003 22:08:25 -0300
E.g. most of include/net/tcp.h pretty much only matters for
net/ipv4/. It would be so nice if a grep -w thing *.[ch] in
net/ipv4/ would really find all uses of "thing".
Bad example, I'd say that %65 of that file is used in net/ipv6/*.c
files too.
David S. Miller wrote:
> Bad example, I'd say that %65 of that file is used in net/ipv6/*.c
> files too.
Considering only inline functions, 64 of the 89 functions my script
below finds in my 2.5.72 tree are only used in net/ipv4 (or tcp.h
itself). That's almost 72%. Only 22 of the functions (25%) are used
in net/ipv6.
A lot of the code there is basically an extension of net/ipv4/tcp*.c,
so it seems odd to put it at a completely different location, only
because it happens to be inlined.
#!/bin/sh
SRC=include/net/tcp.h
for n in `sed '/^.*inline.* \([a-zA-Z_0-9]*\)([^;]*$/s//\1/p;d' <$SRC`; do
echo -n "$n: "
echo `find . -name '*.[ch]' | fgrep -vx ./$SRC | xargs grep -lrw $n`
done
File is "out":
wc -l out
89 out
sed 's|\./net/ipv4/[^/]*\>||g' <out | fgrep -v . | wc -l
64
sed 's|\./net/ipv4/[^/]*\>||g' <out | grep -c net/ipv6
22
- Werner
--
_________________________________________________________________________
/ Werner Almesberger, Buenos Aires, Argentina [email protected] /
/_http://www.almesberger.net/____________________________________________/
From: Werner Almesberger <[email protected]>
Date: Wed, 9 Jul 2003 23:06:37 -0300
Considering only inline functions, 64 of the 89 functions my script
below finds in my 2.5.72 tree are only used in net/ipv4 (or tcp.h
itself). That's almost 72%. Only 22 of the functions (25%) are used
in net/ipv6.
I was considering structure definitions, macros, and whatnot
as well.
David S. Miller wrote:
> I was considering structure definitions, macros, and whatnot
> as well.
Yes, structure definitions and the like are fine. Sorry, I wasn't
quite clear: it's the code living in include/* for apparently no
good reason (i.e. it's only used at one place, is unlikely to be
used by modules, and doesn't define an interface) that I find
irritating.
BTW, even of the macros with arguments, about 66% seem to be of
the "local use only" type.
#!/bin/sh
SRC=include/net/tcp.h
for n in `sed '/^# *define[^!-~]*\([a-zA-Z_0-9]*\)(.*/s//\1/p;d' \
<$SRC | sort | uniq`; do
echo -n "$n: "
echo `find . -name '*.[ch]' | fgrep -vx ./$SRC | xargs grep -lrw $n`
done
wc -l out2
21 out2
sed 's|\./net/ipv4/[^/]*\>||g' <out2 | fgrep -v . | wc -l
14
- Werner
--
_________________________________________________________________________
/ Werner Almesberger, Buenos Aires, Argentina [email protected] /
/_http://www.almesberger.net/____________________________________________/
>> I totally disagree. I think the way we do things today is _STUPID_.
>> We put arch code far away from the generic version which makes finding
>> stuff very difficult for people inspecting the code for the first time.
>>
>> For example, the fact that I have to go groveling in
>> arch/foo/lib/whoknowswhatfile.whoknowswhatextension to look at
>> the memcpy/checksum/whatever implementation is completely busted.
> E.g. most of include/net/tcp.h pretty much only matters for
> net/ipv4/. It would be so nice if a grep -w thing *.[ch] in
> net/ipv4/ would really find all uses of "thing".
I always do this:
cd linux
find . \( -name 'Make*' -o -name '*.[hcS]' \) > src.list
cat src.list| LANG=C xargs grep foo
It's only a CPU time, really.
-- Pete
From: Werner Almesberger <[email protected]>
Date: Wed, 9 Jul 2003 23:37:07 -0300
BTW, even of the macros with arguments, about 66% seem to be of
the "local use only" type.
Ok.
In the first stage, I'd be happy to take patches that extract out
inline functions that get used in only one TCP source file.
Those should be easy to find especially for all of the congestion
control etc. stuff going on in tcp_input.c