On Wed, 3 Sep 2014 12:59:34 +0300
Horia Geantă <horia.geanta@xxxxxxxxxxxxx> wrote:
> On 8/16/2014 2:16 PM, Kim Phillips wrote:
> > On Thu, 14 Aug 2014 15:54:22 +0300
> > Horia Geanta <horia.geanta@xxxxxxxxxxxxx> wrote:
> >
> >> This patch set adds Run Time Assembler (RTA) SEC descriptor library.
> >> RTA is a replacement for incumbent "inline append".
> >>
> >> The library is intended to be a single code base for SEC descriptors creation
> >> for all Freescale products. This comes with a series of advantages, such as
> >> library being maintained / kept up-to-date with latest platforms, i.e. SEC
> >> functionalities (for e.g. SEC incarnations present in Layerscape LS1 and LS2).
> >>
> >> RTA detects options in SEC descriptors that are not supported
> >> by a SEC HW revision ("Era") and reports this back.
> >> Say a descriptor uses Sequence Out Pointer (SOP) option for the SEQINPTR
> >> command, which is supported starting from SEC Era 5. If the descriptor would
> >> be built on a P4080R3 platform (which has SEC Era 4), RTA would report
> >> "SEQ IN PTR: Flag(s) not supported by SEC Era 4".
> >> This is extremely useful and saves a lot of time wasted on debugging.
> >> SEC HW detects only *some* of these problems, leaving user wonder what causes
> >> a "DECO Watchdog Timeout". And when it prints something more useful, sometimes
> >> it does not point to the exact opcode.
> >
> > again, RTA just adds bloat to the kernel driver - the kernel driver
> > is supposed to generate the appropriate descriptor for its target
> > running SEC version no matter what, not "report back" what is/is not
> > supported. This is a flaw at the RTA design level, as far as the
> > kernel driver is concerned.
>
> What is your understanding of developing a descriptor?
> First it needs to be written, then tested - within the kernel driver.
> Having no error checking in the code that generates descriptors
> increases testing / debugging time significantly. Again, SEC HW provides
> some error reporting, but in many cases this is a clueless Watchdog Timeout.
> SEC descriptors development is complex enough to deserve a few
> indications along the way.
AFAICT, RTA doesn't address the Watchdog Timeout issue (which
pretty much always means some part of the SEC has timed out waiting
for more input data). This is something learnt pretty quickly by
SEC developers, and something best to leave to the h/w anyway, since
it would be too cumbersome to add to descriptor construction. So
instead of using RTA, we can discuss enhancing error.c messages to
address cluing in users wrt what the error means, but that change
should probably start with the SEC documentation itself (which is
what error.c messages are based on, and what developers should be
referencing in the first place :).
So RTA tells you what command flags are supported in which SEC
versions. I'm pretty sure h/w behaviour covers that case, too, but
the premise doesn't apply to the kernel driver, since adding support
to the existing descriptors for a new feature flag implies knowing
which SEC version it was introduced in, because the kernel must
work on all versions of the SEC. This promotes a more constructive
implementation design, i.e., the upper levels of the descriptor
construction code shouldn't use the flag if the h/w doesn't support
it in the first place: none of this 'reporting back' business.
FWIW, I doubt that out-of-kernel users would want this feature
either - it's bloat to them too, assuming they are working under the
same constraints as the kernel. If a user experiences the 'flag
unsupported' error, hypothetically they'd go back to the upper level
descriptor construction code and adjust it accordingly, rendering
the original check a waste of runtime after that point.
This reinforces my notion that RTA was not well thought-out from the
beginning. Probably a better place to do these basic checks is in
the form of an external disassembler.
In any case, most of the kernel's crypto API algorithm descriptors
are already fixed, written and constantly get backward-compatibility
tested on newer h/w via the kernel's testing infrastructure: RTA
adds nothing but bloat, and for that reason, I consider it a
regression, and therefore unacceptable for upstream inclusion.
Sorry.
Kim
--
To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
