On Sat, 19 Jul 2014 02:51:30 +0300
Horia Geantă [off-list ref] wrote:

On 7/19/2014 1:13 AM, Kim Phillips wrote:

quoted

On Fri, 18 Jul 2014 19:37:17 +0300
Horia Geanta [off-list ref] wrote:

quoted

This patch set adds Run Time Assembler (RTA) SEC descriptor library.

The main reason of replacing incumbent "inline append" is
to have a single code base both for user space and kernel space.

that's orthogonal to what this patchseries is doing from the kernel
maintainer's perspective:  it's polluting the driver with a
CodingStyle-violating (see, e.g., Chapter 12) 6000+ lines of code -

Regarding coding style - AFAICT that's basically:
ERROR: Macros with complex values should be enclosed in parenthesis
and I am wiling to find a different approach.

There's that, too.

quoted

which can only mean it's slower and more susceptible to bugs - and
AFAICT for no superior technical advantage:  NACK from me.

The fact that the code size is bigger doesn't necessarily mean a bad thing:
1-code is better documented - cloc reports ~ 1000 more lines of 
comments; patch 09 even adds support for generating a docbook
2-pure code (i.e. no comments, white spaces) - cloc reports ~ 5000 more 
lines; this reflects two things, AFAICT:
2.1-more features: options (for e.g. new SEC instructions, little endian 
env. support), platform support includes Era 7 and Era 8, i.e. 
Layerscape LS1 and LS2; this is important to note, since plans are to 
run the very same CAAM driver on ARM platforms

um, *those* features should not cost any driver *that many* lines of
code!

2.2-more error-checking - from this perspective, I'd say driver is less 
susceptible to bugs, especially subtle ones in CAAM descriptors that are 
hard to identify / debug; RTA will complain when generating descriptors 
using features (say a new bit in an instruction opcode) that are not 
supported on the SEC on device

?  The hardware does the error checking.  This just tells me RTA is
slow, inflexible, and requires unnecessary maintenance by design:
all the more reason to keep it out of the kernel :)

RTA currently runs on:
-QorIQ platforms - userspace (USDPAA)
-Layerscape platforms - AIOP accelerator
(obviously, plans are to run also on QorIQ/PowerPC and LS/ARM kernels)

inline append runs elsewhere, too, but I don't see how this is
related to the subject I'm bringing up.

Combined with:
-comprehensive unit testing suite
-RTA kernel port is bit-exact in terms of SEC descriptors hex dumps with 
inline append; besides this, it was tested with tcrypt and in IPsec 
scenarios
I would say that RTA is tested more than inline append. In the end, this 
is a side effect of having a single code base.

inline append has been proven stable for years now.  RTA just adds
redundant code and violates CodingStyle AFAICT.

Kim