Re: [PATCH] crypto:caam - Modify width of few read only registers
On Tue, 29 Apr 2014 15:34:37 +0530 Ruchika Gupta wrote: > Few read only registers like CHAVID, CTPR etc were wrongly defined > as 64 bit registers. This functioned properly on the powerpc platforms. > However ARM SoC's wouldn't function correctly if these registers > are defined as 64 bit. So correcting the definition to two 32 bit registers. please rewrite, adding the details of the problem posted toward the end of this thread, e.g., what registers are affected, and how that renders MCFGR:DWT ineffective in this case. > /* Check to see if QI present. If so, enable */ > - ctrlpriv->qi_present = !!(rd_reg64(&topregs->ctrl.perfmon.comp_parms) & > - CTPR_QI_MASK); > + ctrlpriv->qi_present = > + !!(rd_reg32(&topregs->ctrl.perfmon.comp_parms_ms) & > + CTPR_MS_QI_MASK); alignment > /* Report "alive" for developer to see */ > - dev_info(dev, "device ID = 0x%016llx (Era %d)\n", caam_id, > + dev_info(dev, "device ID = 0x%08x (Era %d)\n", caam_id, >caam_get_era()); Why are we dropping the upper 32 bits here? Kim -- 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] crypto:caam - Modify width of few read only registers
On Thu, 12 Jun 2014 04:56:14 -0500 Gupta Ruchika-R66431 wrote: > > From: Kim Phillips [mailto:kim.phill...@freescale.com] > > Sent: Thursday, June 12, 2014 4:23 AM > > > /* Check to see if QI present. If so, enable */ > > > - ctrlpriv->qi_present = !!(rd_reg64(&topregs->ctrl.perfmon.comp_parms) & > > > - CTPR_QI_MASK); > > > + ctrlpriv->qi_present = > > > + !!(rd_reg32(&topregs->ctrl.perfmon.comp_parms_ms) & > > > + CTPR_MS_QI_MASK); > > > > alignment > Ok. I will correct it. > > > > > /* Report "alive" for developer to see */ > > > - dev_info(dev, "device ID = 0x%016llx (Era %d)\n", caam_id, > > > + dev_info(dev, "device ID = 0x%08x (Era %d)\n", caam_id, > > >caam_get_era()); > > > > Why are we dropping the upper 32 bits here? > The upper 32 bit contain the IP ID of SEC, the major number and the minor > number while the lower 32 bits have the details of the compile option, > integration and configuration options of SEC. So device ID is actually > contained only in the most significant 32 bits which are being printed here. > that may be true, but you're changing the driver to not display information it previously did, in a seemingly totally unrelated manner. This is a regression IMO - if you want the compile options specifically labelled in the display, then do that, but don't start hiding information from the user just because the h/w didn't pass an endianness change properly. Kim -- 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][v2] crypto: caam - Correct definition of registers in memory map
On Thu, 12 Jun 2014 23:52:06 +0530 Ruchika Gupta wrote: > Some registers like SECVID, CHAVID, CHA Revision Number, > CTPR were defined as 64 bit resgisters. The IP provides > a DWT bit(Double word Transpose) to transpose the two words when > a double word register is accessed. However setting this bit > would also affect the operation of job descriptors as well as > other registers which are truly double word in nature. > So, for the IP to work correctly on big-endian as well as > little-endian SoC's, change is required to access all 32 bit > registers as 32 bit quantities. > > Signed-off-by: Ruchika Gupta > --- > Changed in v2: > 1. Review comments incorporated only 2 out of 3...see previous version's thread. Also, please start using scripts/get_maintainer.pl. Thanks, Kim -- 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] crypto: caam - remove duplicate FIFOST_CONT_MASK define
On Mon, 9 Jun 2014 18:19:41 +0300 Dan Carpenter wrote: > The FIFOST_CONT_MASK define is cut and pasted twice so we can delete the > second instance. > > Signed-off-by: Dan Carpenter Acked-by: Kim Phillips Thanks, Kim -- 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 v2] crypto: caam - fix memleak in caam_jr module
On Thu, 3 Jul 2014 15:07:50 +0300 Cristian Stoica wrote: > This patch fixes a memory leak that appears when caam_jr module is unloaded. > > Cc: # 3.13+ > Signed-off-by: Cristian Stoica > --- > drivers/crypto/caam/jr.c | 6 ++ > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c > index 1d80bd3..e0b91fc 100644 > --- a/drivers/crypto/caam/jr.c > +++ b/drivers/crypto/caam/jr.c > @@ -453,7 +453,7 @@ static int caam_jr_probe(struct platform_device *pdev) > int error; > > jrdev = &pdev->dev; > - jrpriv = kmalloc(sizeof(struct caam_drv_private_jr), > + jrpriv = devm_kzalloc(jrdev, sizeof(struct caam_drv_private_jr), >GFP_KERNEL); alignment. Also, why are we replacing a _m_alloc with a _z_alloc? Please start using get_maintainer.pl. Thanks, Kim -- 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 0/9] crypto: caam - Add RTA descriptor creation library
On Fri, 18 Jul 2014 19:37:17 +0300 Horia Geanta wrote: > 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 - which can only mean it's slower and more susceptible to bugs - and AFAICT for no superior technical advantage: NACK from me. Kim -- 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 00/10] CAAM - DMA API fixes
On Fri, 11 Jul 2014 15:34:45 +0300 Horia Geanta wrote: Hi Horia, > Enabling DMA-API debugging reveals quite a lot of problems in CAAM module. > Patches below fix them - tested on P3041DS QorIQ platform. Please apply. In an attempt to try and test these patches on a t4240qds, I get: caam ffe30.crypto: failed to acquire DECO 0 even on the existing cryptodev master. Any ideas? Kim -- 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 0/9] crypto: caam - Add RTA descriptor creation library
On Sat, 19 Jul 2014 02:51:30 +0300 Horia Geantă wrote: > On 7/19/2014 1:13 AM, Kim Phillips wrote: > > On Fri, 18 Jul 2014 19:37:17 +0300 > > Horia Geanta wrote: > > > >> 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. > > 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 -- 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 0/9] crypto: caam - Add RTA descriptor creation library
On Mon, 21 Jul 2014 10:47:49 +0300 Horia Geantă wrote: > On 7/19/2014 4:23 AM, Kim Phillips wrote: > > On Sat, 19 Jul 2014 02:51:30 +0300 > > Horia Geantă wrote: > > > >> On 7/19/2014 1:13 AM, Kim Phillips wrote: > >>> On Fri, 18 Jul 2014 19:37:17 +0300 > >>> Horia Geanta wrote: > >>> > >>>> This patch set adds Run Time Assembler (RTA) SEC descriptor library. > >>>> > >>> 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! > > You are invited to comment on the code at hand. I am pretty sure it's > not perfect. I can see RTA is responsible for the code size increase, not the features. And just because RTA has - or has plans for - those features don't justify the kernel driver adopting RTA over inline-append. > >> 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 :) > > This is just like saying a toolchain isn't performing any checks and > lets the user generate invalid machine code and deal with HW errors. > > Beside this, there are (quite a few) cases when SEC won't emit any > error, but still the results are different than expected. > SEC HW is complex enough to deserve descriptor error checking. if part of RTA's objective is to cater to new SEC programmers, great, but that doesn't mean it belongs in the crypto API driver's limited input parameter set, and fixed descriptors operating environment: it's not the place to host an entire SEC toolchain. > >> 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. > > This is relevant, since having a piece of SW running in multiple > environments should lead to better testing, more exposure, finding bugs > faster. that doesn't defeat the fact that more lines of code to do the same thing is always going to be a more bug-prone way of doing it. > inline append *could run* elsewhere , but it doesn't AFAICT. Last time > I checked, USDPAA and AIOP use RTA. inline append runs in ASF, and has been available for all upstream for years. > >> 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. > > New platform support is not redundant. No, RTA is. > Error checking is not redundant, as explained above. It is: the kernel has fixed descriptors. > kernel-doc is always helpful. it doesn't matter how much you decorate it. > Coding Style can be fixed. inline append isn't broken. Kim -- 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] crypto: caam - fix DECO RSR polling
On Mon, 21 Jul 2014 16:03:21 +0300 Horia Geanta wrote: > RSR (Request Source Register) is not used when > virtualization is disabled, thus don't poll for Valid bit. > > Besides this, if used, timeout has to be reinitialized. > > Signed-off-by: Horia Geanta > --- > Only compile-tested. > Ruchika / Kim, please review / test. Acked-by: Kim Phillips fwiw, it would be nice if virt_en were a bool...can you also please start using get_maintainer.pl? Thanks, Kim -- 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 01/10] crypto: testmgr - avoid DMA mapping from text, rodata, stack
On Fri, 11 Jul 2014 15:34:46 +0300 Horia Geanta wrote: > +++ b/crypto/testmgr.c > @@ -198,13 +198,20 @@ static int __test_hash(struct crypto_ahash *tfm, struct > hash_testvec *template, > const char *algo = crypto_tfm_alg_driver_name(crypto_ahash_tfm(tfm)); > unsigned int i, j, k, temp; > struct scatterlist sg[8]; > - char result[64]; > + char *result = NULL; > + char *key = NULL; these needn't be initialized, here and elsewhere. > struct ahash_request *req; > struct tcrypt_result tresult; > void *hash_buff; > char *xbuf[XBUFSIZE]; > int ret = -ENOMEM; > > + result = kmalloc(64, GFP_KERNEL); s/64/MAX_DIGEST_SIZE > +++ b/crypto/testmgr.h > @@ -32,7 +32,7 @@ > #define MAX_DIGEST_SIZE 64 > #define MAX_TAP 8 > > -#define MAX_KEYLEN 56 > +#define MAX_KEYLEN 160 > #define MAX_IVLEN32 this change could use a blurb in the commit message. Other than that, this series gets my: Acked-by: Kim Phillips Thanks! Kim -- 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 v2 00/12] crypto: caam - Add RTA descriptor creation library
On Thu, 14 Aug 2014 15:54:22 +0300
Horia Geanta 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.
Thanks,
Kim
--
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 v2 00/12] crypto: caam - Add RTA descriptor creation library
On Wed, 3 Sep 2014 12:59:34 +0300
Horia Geantă wrote:
> On 8/16/2014 2:16 PM, Kim Phillips wrote:
> > On Thu, 14 Aug 2014 15:54:22 +0300
> > Horia Geanta 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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] crypto: talitos: Avoid excessive loops in softirq context
On Wed, 10 Sep 2014 10:34:47 +0200 Helmut Schaa wrote: > The talitos driver can cause starvation of other softirqs and as such > it can also cause rcu stalls like: ... > Work around this by processing a maximum amount of 16 finished requests > and rescheduling the done-tasklet if any work is left. > This allows other softirqs to run. 16 sounds rather arbitrary, and application-dependent - talitos' FIFO size is 24. IIRC, netdev's NAPI can be refactored out of just being able to work on network devices, and be made to apply to crypto devices, too. In fact, some old Freescale hacks of this nature have improved performance. Can we do something like refactor NAPI instead? Thanks, Kim -- 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] crypto: talitos: Avoid excessive loops in softirq context
[adding Sandeep, Horia and netdev] On Fri, 12 Sep 2014 09:39:12 +0200 Helmut Schaa wrote: > On Fri, Sep 12, 2014 at 2:49 AM, Kim Phillips > wrote: > > On Wed, 10 Sep 2014 10:34:47 +0200 > > Helmut Schaa wrote: > > > >> The talitos driver can cause starvation of other softirqs and as such > >> it can also cause rcu stalls like: > > ... > >> Work around this by processing a maximum amount of 16 finished requests > >> and rescheduling the done-tasklet if any work is left. > >> This allows other softirqs to run. > > > > 16 sounds rather arbitrary, and application-dependent - talitos' > > FIFO size is 24. > > Yep, 16 is arbitrary, I can also do "fifo_len" if you prefer? > > > IIRC, netdev's NAPI can be refactored out of just being able to work > > on network devices, and be made to apply to crypto devices, too. In > > fact, some old Freescale hacks of this nature have improved > > performance. Can we do something like refactor NAPI instead? > > That would indeed be nice but sounds like quite some more work and > I won't have time to do so. Especially since my system was taken down > completely by the talitos tasklet under some circumstances. If there is > any work going on in that regard I'd be fine with just dropping that patch > (and carrying it myself until the refactoring is done). I'm not aware of any, but to prove whether NAPI actually fixes the issue, can you try applying this patch: http://patchwork.ozlabs.org/patch/146094/ For it to be upstream-acceptable, I believe it would have to port NAPI in such a way that all crypto drivers could use it. The difference between net NAPI and crypto NAPI would be that the crypto version would be reduced to only using the weight code, since that's the only source of the performance benefit. Thanks, Kim -- 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 1/2] crypto: caam - add support for gcm(aes)
On Thu, 9 Oct 2014 17:54:09 +0300
Tudor Ambarus wrote:
> + /*
> + * Job Descriptor and Shared Descriptors
> + * must all fit into the 64-word Descriptor h/w Buffer
> + */
> + if (DESC_GCM_DEC_LEN + DESC_JOB_IO_LEN +
> + ctx->enckeylen <= CAAM_DESC_BYTES_MAX)
> + keys_fit_inline = true;
we need to reset the encrypt descriptor's keys_fit_inline setting to
false before re-evaluating for decrypt.
> + /* Galois Counter Mode */
> + {
> + .name = "gcm(aes)",
> + .driver_name = "gcm-aes-caam",
> + .blocksize = 1,
> + .type = CRYPTO_ALG_TYPE_AEAD,
> + .template_aead = {
> + .setkey = gcm_setkey,
> + .setauthsize = gcm_setauthsize,
> + .encrypt = aead_encrypt,
> + .decrypt = aead_decrypt,
> + .givencrypt = NULL,
> + .geniv = "",
> + .ivsize = 12,
> + .maxauthsize = 16,
AES_BLOCK_SIZE
Thanks,
Kim
--
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 2/2] crypto: caam - add support for rfc4106(gcm(aes))
On Thu, 9 Oct 2014 17:54:10 +0300
Tudor Ambarus wrote:
> +static int rfc4106_set_sh_desc(struct crypto_aead *aead)
...
> + /*
> + * Job Descriptor and Shared Descriptors
> + * must all fit into the 64-word Descriptor h/w Buffer
> + */
> + if (DESC_RFC4106_DEC_LEN + DESC_JOB_IO_LEN +
> + ctx->enckeylen <= CAAM_DESC_BYTES_MAX)
> + key_fit_inline = true;
we need to reset encrypt descriptor's keys_fit_inline setting to
false before doing this.
Also, the singular of "keys_fit_inline" is "key_fits_inline", but
I'd prefer we not gratuitously rename the variable from the rest of
the driver's keys_fit_inline for consistency's sake, thanks.
> + /*
> + * Job Descriptor and Shared Descriptors
> + * must all fit into the 64-word Descriptor h/w Buffer
> + */
> + if (DESC_RFC4106_GIVENC_LEN + DESC_JOB_IO_LEN +
> + ctx->split_key_pad_len + ctx->enckeylen <=
> + CAAM_DESC_BYTES_MAX)
> + key_fit_inline = true;
we need to reset the variable here too.
> +static int rfc4106_setauthsize(struct crypto_aead *authenc,
> +unsigned int authsize)
> +{
> + struct caam_ctx *ctx = crypto_aead_ctx(authenc);
> +
> + switch (authsize) {
> + case 8:
> + case 12:
> + case 16:
> + break;
> + default:
> + return -EINVAL;
> + }
the h/w can handle more authsizes than that, so we
shouldn't be blocking it from doing so here.
> @@ -2601,6 +2986,23 @@ static struct caam_alg_template driver_algs[] = {
> OP_ALG_AAI_HMAC_PRECOMP,
> .alg_op = OP_ALG_ALGSEL_SHA512 | OP_ALG_AAI_HMAC,
> },
> + {
> + .name = "rfc4106(gcm(aes))",
> + .driver_name = "rfc4106-gcm-aes-caam",
> + .blocksize = 1,
> + .type = CRYPTO_ALG_TYPE_AEAD,
> + .template_aead = {
> + .setkey = rfc4106_setkey,
> + .setauthsize = rfc4106_setauthsize,
> + .encrypt = aead_encrypt,
> + .decrypt = aead_decrypt,
> + .givencrypt = aead_givencrypt,
> + .geniv = "",
> + .ivsize = 8,
> + .maxauthsize = 16,
AES_BLOCK_SIZE
Thanks,
Kim
--
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 2/2] crypto: caam - add support for rfc4106(gcm(aes))
On Fri, 10 Oct 2014 05:10:55 -0500
Ambarus Tudor-Dan-B38632 wrote:
> On Thu, 9 Oct 2014 17:54:10 +0300
> Tudor Ambarus wrote:
> > +static int rfc4106_setauthsize(struct crypto_aead *authenc,
> > + unsigned int authsize)
> > +{
> > + struct caam_ctx *ctx = crypto_aead_ctx(authenc);
> > +
> > + switch (authsize) {
> > + case 8:
> > + case 12:
> > + case 16:
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
>
> the h/w can handle more authsizes than that, so we
> shouldn't be blocking it from doing so here.
>
> [TA] rfc4106 says that "Implementations MUST support a full-length 16-octet
> ICV, and MAY support 8 or 12 octet ICVs, and MUST NOT support other ICV
> lengths."
> Do we want to support other ICV lengths?
how much the linux kernel wants to enforce the RFC is up to the
higher levels in its crypto API and other front end interfaces such
as XFRM. So those checks for RFC compliance should be performed
before they reach the implementation (driver).
Meanwhile, drivers themselves should accurately represent the h/w
capabilities, whether they meet and/or exceed the RFC's constraints
or not, IMO. This way, if/when the RFC gets extended to become
open to a wider range of values, the driver needs no changes.
> > @@ -2601,6 +2986,23 @@ static struct caam_alg_template driver_algs[] = {
> >OP_ALG_AAI_HMAC_PRECOMP,
> > .alg_op = OP_ALG_ALGSEL_SHA512 | OP_ALG_AAI_HMAC,
> > },
> > + {
> > + .name = "rfc4106(gcm(aes))",
> > + .driver_name = "rfc4106-gcm-aes-caam",
> > + .blocksize = 1,
> > + .type = CRYPTO_ALG_TYPE_AEAD,
> > + .template_aead = {
> > + .setkey = rfc4106_setkey,
> > + .setauthsize = rfc4106_setauthsize,
> > + .encrypt = aead_encrypt,
> > + .decrypt = aead_decrypt,
> > + .givencrypt = aead_givencrypt,
> > + .geniv = "",
> > + .ivsize = 8,
> > + .maxauthsize = 16,
>
> AES_BLOCK_SIZE
>
> [TA] I don't think we should change the blocksize value to AES_BLOCK_SIZE.
sorry, I meant just .maxauthsize = AES_BLOCK_SIZE.
Thanks,
Kim
--
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 1/2] crypto: caam - add support for gcm(aes)
On Fri, 10 Oct 2014 03:47:18 -0500
Ambarus Tudor-Dan-B38632 wrote:
> On Thu, 9 Oct 2014 17:54:09 +0300
> Tudor Ambarus wrote:
> > + /* Galois Counter Mode */
> > + {
> > + .name = "gcm(aes)",
> > + .driver_name = "gcm-aes-caam",
> > + .blocksize = 1,
> > + .type = CRYPTO_ALG_TYPE_AEAD,
> > + .template_aead = {
> > + .setkey = gcm_setkey,
> > + .setauthsize = gcm_setauthsize,
> > + .encrypt = aead_encrypt,
> > + .decrypt = aead_decrypt,
> > + .givencrypt = NULL,
> > + .geniv = "",
> > + .ivsize = 12,
> > + .maxauthsize = 16,
>
> AES_BLOCK_SIZE
> [TA] I think we shall not change the blocksize value to AES_BLOCK_SIZE.
> GCM uses a block cipher as a stream cipher. It generates encryption blocks,
> which are then XORed with the plaintext blocks to get the ciphertext. Just as
> with other stream ciphers, flipping a bit in the ciphertext produces a
> flipped bit in the plaintext at the same location.
>
Sorry, I meant just .maxauthsize = AES_BLOCK_SIZE.
Kim
--
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] crypto: caam: fix error reporting
On Fri, 31 Oct 2014 18:57:33 +0200
Cristian Stoica wrote:
> The error code returned by hardware is four bits wide with an expected
> zero MSB. A hardware error condition where the error code can get between
> 0x8 and 0xf will trigger an out of bound array access on the error
> message table.
If this issue was brought up by h/w, the appropriate new error codes
should be being introduced.
Otherwise, I'm assuming it was brought up by a static code analyser,
which technically could be ignored, but...
> - /*
> - * If there is no further error handling function, just
> - * print the error code, error string and exit. Otherwise
> - * call the handler function.
> - */
why remove the comment? It's still valid.
> - if (!status_src[ssrc].report_ssed)
> - dev_err(jrdev, "%08x: %s: \n", status, status_src[ssrc].error);
> - else
> + if (status_src[ssrc].report_ssed)
> status_src[ssrc].report_ssed(jrdev, status, error);
> + else if (error)
> + dev_err(jrdev, "%d: %s\n", ssrc, error);
> + else
> + dev_err(jrdev, "%d: unknown error code\n", ssrc);
This is simpler:
diff --git a/drivers/crypto/caam/error.c b/drivers/crypto/caam/error.c
index 6531054..6f4a148 100644
--- a/drivers/crypto/caam/error.c
+++ b/drivers/crypto/caam/error.c
@@ -224,7 +224,12 @@ void caam_jr_strstatus(struct device *jrdev, u32 status)
{ report_cond_code_status, "Condition Code" },
};
u32 ssrc = status >> JRSTA_SSRC_SHIFT;
- const char *error = status_src[ssrc].error;
+ const char *error;
+
+ if (ssrc >= ARRAY_SIZE(status_src)) {
+ dev_err(jrdev, "unknown error status source %d\n", ssrc);
+ return;
+ }
/*
* If there is no further error handling function, just
Kim
--
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] crypto: caam: fix error reporting
On Mon, 3 Nov 2014 11:18:36 +0200
Cristian Stoica wrote:
> On 10/31/2014 08:22 PM, Kim Phillips wrote:
> > On Fri, 31 Oct 2014 18:57:33 +0200
> > Cristian Stoica wrote:
> >
> > If this issue was brought up by h/w, the appropriate new error codes
> > should be being introduced.
>
> If you have the new error codes please send them to me and I'll make an
> update.
I was mainly inquiring as to the motive of the patch. fwiw, I don't
see error codes with the most significant bit set in the latest
documentation (which is available from STC).
> > Otherwise, I'm assuming it was brought up by a static code analyser,
> > which technically could be ignored, but...
>
> Actually, our static code analyzer did not see this one.
ok, so the patch technically isn't fixing anything broken, then.
> >> - /*
> >> - * If there is no further error handling function, just
> >> - * print the error code, error string and exit. Otherwise
> >> - * call the handler function.
> >> - */
> >
> > why remove the comment? It's still valid.
>
> The comment was disagreeing with the new code, so I just removed it.
the new code just added a new condition, which doesn't invalidate
the comment. And simply removing the comment as opposed to amending
it is a bit overkill.
> >> - if (!status_src[ssrc].report_ssed)
> >> - dev_err(jrdev, "%08x: %s: \n", status, status_src[ssrc].error);
> >> - else
> >> + if (status_src[ssrc].report_ssed)
> >>status_src[ssrc].report_ssed(jrdev, status, error);
> >> + else if (error)
> >> + dev_err(jrdev, "%d: %s\n", ssrc, error);
> >> + else
> >> + dev_err(jrdev, "%d: unknown error code\n", ssrc);
> >
> > This is simpler:
> >
> > diff --git a/drivers/crypto/caam/error.c b/drivers/crypto/caam/error.c
> > index 6531054..6f4a148 100644
> > --- a/drivers/crypto/caam/error.c
> > +++ b/drivers/crypto/caam/error.c
> > @@ -224,7 +224,12 @@ void caam_jr_strstatus(struct device *jrdev, u32
> > status)
> > { report_cond_code_status, "Condition Code" },
> > };
> > u32 ssrc = status >> JRSTA_SSRC_SHIFT;
> > - const char *error = status_src[ssrc].error;
> > + const char *error;
> > +
> > + if (ssrc >= ARRAY_SIZE(status_src)) {
> > + dev_err(jrdev, "unknown error status source %d\n", ssrc);
> > + return;
> > + }
>
> It is indeed simpler but does it consider also the missing error codes
> at index 1 and 5? Just checking for an upper bound is not enough.
no, the existing code already handles that. Note that newer
documentation fills the 1 and 5 slots, too.
> On the other hand, if the error field is only three bits wide instead of
> four as stated by the documentation, a better fix means using a three
> bit mask instead of reporting an invalid error code.
true, but then we'd introduce a direct discrepancy with the
documentation, and thus h/w.
Kim
--
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] crypto: caam: fix error reporting
On Tue, 4 Nov 2014 10:57:57 +0200 Cristian Stoica wrote: > Hi Kim, > > >> Actually, our static code analyzer did not see this one. > > > > ok, so the patch technically isn't fixing anything broken, then. > > Are you saying the code isn't broken _because_ a static tool analyser > did not see anything wrong here? no: I'm saying there was no symptom - something I'd expect to gather from the original commit message. > > the new code just added a new condition, which doesn't invalidate > > the comment. And simply removing the comment as opposed to amending > > it is a bit overkill. > > You are partially right, but will the staggering lack of comments in the > caam driver be fixed by duplicating a cascade of three ifs into english? well, given that preamble, it would be better than removing the existing two :), but the simpler version makes it a moot point. > >> It is indeed simpler but does it consider also the missing error codes > >> at index 1 and 5? Just checking for an upper bound is not enough. > > > > no, the existing code already handles that. Note that newer > > documentation fills the 1 and 5 slots, too. > > If you have the new error codes please send them to me for an update. surely you have access to the documentation, if not, let me know. > >> On the other hand, if the error field is only three bits wide instead of > >> four as stated by the documentation, a better fix means using a three > >> bit mask instead of reporting an invalid error code. > > > > true, but then we'd introduce a direct discrepancy with the > > documentation, and thus h/w. > > You basically ask me to agree that if there are no _documented_ error > codes between 0x8 and 0xf then I should trust that they will never come > up on a 4 bit field. they may, depending on future revs of the h/w, but that's not what this patch is about. > Do you want me to drop the patch and pretend there is nothing to see? no, fixing potential bugs preemptively is fine; I'd just like to know that's the case: it wasn't clear from the original commit message whether the problem occurred on a new h/w revision, in which case I'd have like to have seen the driver updated with support for the new error codes. Kim -- 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 v2] crypto: caam: fix error reporting
On Wed, 5 Nov 2014 11:21:24 +0200 Cristian Stoica wrote: > The error code returned by hardware is four bits wide with an expected > zero MSB. A hardware error condition where the error code can get between > 0x8 and 0xf will trigger an out of bound array access on the error > message table. > This patch fixes the invalid array access following such an error and > reports the condition. > > Signed-off-by: Cristian Stoica > --- no v2 change info? All I noticed is the additional string for "queue manager interface", which, without its implementation fn, intoduces an inconsistency wrt NULL checking, so this comment: http://www.mail-archive.com/linux-crypto@vger.kernel.org/msg12105.html still applies. Thanks, Kim -- 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 01/16] crypto: caam - Remove unnecessary smp_read_barrier_depends()
On Thu, 13 Nov 2014 16:51:12 -0500
Pranith Kumar wrote:
> On 11/13/2014 03:10 PM, Paul E. McKenney wrote:
> > On Thu, Nov 13, 2014 at 02:24:07PM -0500, Pranith Kumar wrote:
> >> Recently lockless_dereference() was added which can be used in place of
> >> hard-coding smp_read_barrier_depends(). The following PATCH makes the
> >> change.
> >>
> >> Signed-off-by: Pranith Kumar
> >> ---
> >> drivers/crypto/caam/jr.c | 3 ---
> >> 1 file changed, 3 deletions(-)
> >>
> >> diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
> >> index bae20d8..9b3ef1bc 100644
> >> --- a/drivers/crypto/caam/jr.c
> >> +++ b/drivers/crypto/caam/jr.c
> >> @@ -181,8 +181,6 @@ static void caam_jr_dequeue(unsigned long devarg)
> >>for (i = 0; CIRC_CNT(head, tail + i, JOBR_DEPTH) >= 1; i++) {
> >>sw_idx = (tail + i) & (JOBR_DEPTH - 1);
> >>
> >> - smp_read_barrier_depends();
> >> -
> >
> > Did you mean to add a lockless_dereference() somewhere? I would guess
> > that one is required on the load into sw_idx before the "for" loop,
> > but I cannot claim to be familiar with this code.
>
> No, I could not understand why the barrier was here in the first place. The
> change log does not reflect the subject line.
>
> sw_idx is a local variable and is used to index the entinfo array. It does
> not seem like the barrier is needed. If not a comment saying why could be
> added I guess.
I likely misinterpreted the "read index before reading contents at
that index" instructions in early circular buffer documentation, and
left it in for the benefit of the doubt. Now it looks to me it's not
necessary, given both sw_idx and tail are just being computed within
a lock, and removing both barriers doesn't affect the compiler
output, so:
Reviewed-by: Kim Phillips
Thanks,
Kim
--
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] crypto: caam - add support for rfc4543(gcm(aes))
On Thu, 6 Nov 2014 23:17:14 +0800 Herbert Xu wrote: > On Thu, Oct 30, 2014 at 06:55:07PM +0200, Tudor Ambarus wrote: > > Add AES-GMAC as an IPSec ESP mechanism to provide > > data origin authentication, but not confidentiality. > > This method is referred as ENCR_NULL_AUTH_AES_GMAC. > > > > Signed-off-by: Tudor Ambarus > > Applied. this commit gets this when running the tcrypt module: caam_jr ffe302000.jr: 2000221a: CCB: desc idx 34: AES: ICV check failed. caam_jr ffe302000.jr: 2000221a: CCB: desc idx 34: AES: ICV check failed. caam_jr ffe302000.jr: 2000221a: CCB: desc idx 34: AES: ICV check failed. Briefly looking at testmgr's aes_gcm_rfc4543_dec_tv_template, there are only two decryption vectors, and, according to the comment, only the latter should fail? Thanks, Kim -- 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] crypto: caam - add support for rfc4543(gcm(aes))
On Thu, 4 Dec 2014 18:22:45 -0600 Kim Phillips wrote: > On Thu, 6 Nov 2014 23:17:14 +0800 > Herbert Xu wrote: > > > On Thu, Oct 30, 2014 at 06:55:07PM +0200, Tudor Ambarus wrote: > > > Add AES-GMAC as an IPSec ESP mechanism to provide > > > data origin authentication, but not confidentiality. > > > This method is referred as ENCR_NULL_AUTH_AES_GMAC. > > > > > > Signed-off-by: Tudor Ambarus > > > > Applied. > > this commit gets this when running the tcrypt module: > > caam_jr ffe302000.jr: 2000221a: CCB: desc idx 34: AES: ICV check failed. > caam_jr ffe302000.jr: 2000221a: CCB: desc idx 34: AES: ICV check failed. > caam_jr ffe302000.jr: 2000221a: CCB: desc idx 34: AES: ICV check failed. > > Briefly looking at testmgr's aes_gcm_rfc4543_dec_tv_template, there > are only two decryption vectors, and, according to the comment, only > the latter should fail? nm, the test runs three times total: twice more for the different destination and alignment test variations. Kim -- 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
[PATCH] crypto: caam - don't emit ICV check failures to dmesg
ICV check failures are part of normal operation; leave user notification up to the higher levels, as is done in s/w algorithm implementations. Signed-off-by: Kim Phillips --- drivers/crypto/caam/error.c | 13 + 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/drivers/crypto/caam/error.c b/drivers/crypto/caam/error.c index 66d73bf..33e41ea 100644 --- a/drivers/crypto/caam/error.c +++ b/drivers/crypto/caam/error.c @@ -151,10 +151,15 @@ static void report_ccb_status(struct device *jrdev, const u32 status, else snprintf(err_err_code, sizeof(err_err_code), "%02x", err_id); - dev_err(jrdev, "%08x: %s: %s %d: %s%s: %s%s\n", - status, error, idx_str, idx, - cha_str, cha_err_code, - err_str, err_err_code); + /* +* CCB ICV check failures are part of normal operation life; +* we leave the upper layers to do what they want with them. +*/ + if (err_id != JRSTA_CCBERR_ERRID_ICVCHK) + dev_err(jrdev, "%08x: %s: %s %d: %s%s: %s%s\n", + status, error, idx_str, idx, + cha_str, cha_err_code, + err_str, err_err_code); } static void report_jump_status(struct device *jrdev, const u32 status, -- 2.2.2 -- 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 2/3] crypto: caam_rng: fix rng_unmap_ctx's DMA_UNMAP size problem
On Tue, 3 Mar 2015 14:50:52 +0800 wrote: > - dma_unmap_single(jrdev, ctx->sh_desc_dma, DESC_RNG_LEN, > - DMA_TO_DEVICE); > + dma_unmap_single(jrdev, ctx->sh_desc_dma, > + desc_bytes(ctx->sh_desc), DMA_TO_DEVICE); alignment: the 'd' in desc_bytes should fall directly under the 'j' in jrdev. Also, DESC_RNG_LEN should be corrected to (4 * CAAM_CMD_SZ). Kim -- 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 1/3] crypto: caam: fix some compile warnings
On Tue, 3 Mar 2015 14:50:51 +0800 wrote: > This commit is to avoid the below warnings: > > drivers/crypto/caam/sg_sw_sec4.h:88:12: warning: > 'dma_map_sg_chained' defined but not used [-Wunused-function] > static int dma_map_sg_chained(struct device *dev, struct scatterlist *sg, > ^ > drivers/crypto/caam/sg_sw_sec4.h:104:12: warning: > 'dma_unmap_sg_chained' defined but not used [-Wunused-function] > static int dma_unmap_sg_chained(struct device *dev, > ^ I'm not seeing these warnings - both caamalg.c and caamhash.c use those functions fine. > -static int dma_map_sg_chained(struct device *dev, struct scatterlist *sg, > +static inline int dma_map_sg_chained(struct device *dev, struct scatterlist > *sg, > unsigned int nents, enum dma_data_direction dir, > bool chained) not to mention this isn't how to fix a defined but not used warning: marking the functions inline results in different compiler output. NACK from me. Kim -- 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 v2 5/5] crypto: talitos: Add software backlog queue handling
On Tue, 3 Mar 2015 08:21:37 -0500 Martin Hicks wrote: > @@ -1170,6 +1237,8 @@ static struct talitos_edesc *talitos_edesc_alloc(struct > device *dev, >edesc->dma_len, >DMA_BIDIRECTIONAL); > edesc->req.desc = &edesc->desc; > + /* A copy of the crypto_async_request to use the crypto_queue backlog */ > + memcpy(&edesc->req.base, areq, sizeof(struct crypto_async_request)); this seems backward, or, at least can be done more efficiently IMO: talitos_cra_init should set the tfm's reqsize so the rest of the driver can wholly embed its talitos_edesc (which should also wholly encapsulate its talitos_request (i.e., not via a pointer)) into the crypto API's request handle allocation. This would absorb and eliminate the talitos_edesc kmalloc and frees, the above memcpy, and replace the container_of after the crypto_dequeue_request with an offset_of, right? When scatter-gather buffers are needed, we can assume a slower-path and make them do their own allocations, since their sizes vary depending on each request. Of course, a pointer to those allocations would need to be retained somewhere in the request handle. Only potential problem is getting the crypto API to set the GFP_DMA flag in the allocation request, but presumably a CRYPTO_TFM_REQ_DMA crt_flag can be made to handle that. Kim -- 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 v2 3/5] crypto: talitos: Fix off-by-one and use all hardware slots
On Tue, 3 Mar 2015 08:21:35 -0500 Martin Hicks wrote: > The submission count was off by one. > > Signed-off-by: Martin Hicks > --- sadly, this directly contradicts: commit 4b24ea971a93f5d0bec34bf7bfd0939f70cfaae6 Author: Vishnu Suresh Date: Mon Oct 20 21:06:18 2008 +0800 crypto: talitos - Preempt overflow interrupts off-by-one fix My guess is your request submission pattern differs from that of Vishnu's (probably IPSec and/or tcrypt), or later h/w versions have gotten better about dealing with channel near-overflow conditions. Either way, I'd prefer we not do this: it might break others, and I'm guessing doesn't improve performance _that_ much? If it does, we could risk it and restrict it to SEC versions 3.3 and above maybe? Not sure what to do here exactly, barring digging up and old 2.x SEC and testing. Kim p.s. I checked, Vishnu isn't with Freescale anymore, so I can't cc him. -- 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 1/3] crypto: caam: fix some compile warnings
On Wed, 4 Mar 2015 11:03:28 +0200 Cristian Stoica wrote: > On 03/04/2015 06:57 AM, yjin wrote: > > An alternative is moving the definitions to a ".c" file, but I don't > > think it will be fundamental different. > > I know I am fixing a potential error which doesn't exist now, it seems > > useless for the current upstream version, we can abandon my patch. But I > > still think the current implementation adds unnecessary restrictions for > > its users. > > I think that both dma_map_sg_chained and dma_unmap_sg_chained should go > away. They were added to support chained scatterlists, but as far as I > verified, dma_map_sg should handle that case as well. > > Kim, can you confirm this? I don't see how, e.g., for one, dma_map_sg is I/O TLB implementation-dependent. Kim -- 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 2/3] crypto: caam_rng: fix rng_unmap_ctx's DMA_UNMAP size problem
On Wed, 4 Mar 2015 13:33:22 +0800 yjin wrote: > On 2015年03月04日 03:31, Kim Phillips wrote: > > On Tue, 3 Mar 2015 14:50:52 +0800 > > wrote: > > > >> - dma_unmap_single(jrdev, ctx->sh_desc_dma, DESC_RNG_LEN, > >> - DMA_TO_DEVICE); > >> + dma_unmap_single(jrdev, ctx->sh_desc_dma, > >> + desc_bytes(ctx->sh_desc), DMA_TO_DEVICE); > > alignment: the 'd' in desc_bytes should fall directly under the 'j' > > in jrdev. > > > > Also, DESC_RNG_LEN should be corrected to (4 * CAAM_CMD_SZ). > > Hi Kim, > > I can't find the obvious limitation for the RNG descriptor length in > Freescale documents, could you point out it? ? rng_create_sh_desc() creates a fixed descriptor of exactly 4 command-lengths. Kim -- 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: IPSec hmac(sha256) truncation bits length
On Wed, 4 Mar 2015 20:28:26 +0200 Nicolae Rosia wrote: > On Wed, Mar 4, 2015 at 7:13 PM, Nicolae Rosia wrote: > > I'm trying to understand why icv_truncbits is set to 96 for > > hmac(sha256) in xfrm_algo.c because > > RFC4868 [1] says that the truncation length for HMAC-SHA256 should be 128. See http://comments.gmane.org/gmane.linux.kernel.cryptoapi/6767 Kim -- 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 v2 2/5] crypto: talitos: Remove MD5_BLOCK_SIZE
On Tue, 3 Mar 2015 08:21:34 -0500 Martin Hicks wrote: > This is properly defined in the md5 header file. > > Signed-off-by: Martin Hicks > --- Acked-by: Kim Phillips Kim -- 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 v2 1/5] crypto: talitos: Simplify per-channel initialization
On Tue, 3 Mar 2015 08:21:33 -0500 Martin Hicks wrote: > There were multiple loops in a row, for each separate step of the > initialization of the channels. Simplify to a single loop. > > Signed-off-by: Martin Hicks > --- Acked-by: Kim Phillips Kim -- 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 2/2] crypto: talitos: Add AES-XTS Support
On Fri, 20 Feb 2015 12:00:10 -0500 Martin Hicks wrote: > The newer talitos hardware has support for AES in XTS mode. Assuming it's the same thing, AES-XCBC gets added with SEC v3.0 h/w. Assuming hw_supports() doesn't already support this algorithm combination (technically via the mode bit), this needs to be reflected in the patch so the driver doesn't think SEC 2.x devices can do XTS, too. > + .desc_hdr_template = DESC_HDR_TYPE_COMMON_NONSNOOP_NO_AFEU | > + DESC_HDR_SEL0_AESU | > + DESC_HDR_MODE0_AESU_XTS, ... > /* primary execution unit mode (MODE0) and derivatives */ > #define DESC_HDR_MODE0_ENCRYPT cpu_to_be32(0x0010) > #define DESC_HDR_MODE0_AESU_CBC cpu_to_be32(0x0020) > +#define DESC_HDR_MODE0_AESU_XTS cpu_to_be32(0x0420) I'd prefer these defines be kept as single bit definitions, and keep their names from the manual. An XTS-specific definition can be composed from them either after this, or manually in the desc_hdr_template assignment above. Thanks, Kim -- 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 0/17] crypto: talitos - Add support for SEC1
On Thu, 5 Mar 2015 17:46:05 +0100 Christophe Leroy wrote: > [15/17] crypto: talitos - Implementation of SEC1 ... > [16/17] crypto: talitos - SEC1 bugs on 0 data hash > [17/17] crypto: talitos - Update DT bindings with SEC1 This patchseries doesn't apply, at least on top of Herbert's cryptodev-2.6 tree, as of today: Applying: crypto: talitos - Implementation of SEC1 error: patch failed: drivers/crypto/talitos.c:655 error: drivers/crypto/talitos.c: patch does not apply Kim -- 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 2/3] crypto: caam_rng: fix rng_unmap_ctx's DMA_UNMAP size problem
On Thu, 5 Mar 2015 10:52:37 +0800 yjin wrote: > On 2015年03月05日 02:36, Kim Phillips wrote: > > On Wed, 4 Mar 2015 13:33:22 +0800 > > yjin wrote: > > > >> On 2015年03月04日 03:31, Kim Phillips wrote: > >>> On Tue, 3 Mar 2015 14:50:52 +0800 > >>> wrote: > >>> > >>>> -dma_unmap_single(jrdev, ctx->sh_desc_dma, DESC_RNG_LEN, > >>>> - DMA_TO_DEVICE); > >>>> +dma_unmap_single(jrdev, ctx->sh_desc_dma, > >>>> +desc_bytes(ctx->sh_desc), > >>>> DMA_TO_DEVICE); > >>> alignment: the 'd' in desc_bytes should fall directly under the 'j' > >>> in jrdev. > >>> > >>> Also, DESC_RNG_LEN should be corrected to (4 * CAAM_CMD_SZ). > >> > >> I can't find the obvious limitation for the RNG descriptor length in > >> Freescale documents, could you point out it? > > ? rng_create_sh_desc() creates a fixed descriptor of exactly 4 > > command-lengths. > > Do you mean that the code itself limits the descriptor length? Not a > hardware limitation. the code writes descriptors such that they don't reach h/w limitations. > If so, I prefer to dma_unmap with desc_bytes(ctx->sh_desc) as my > previous patch, and correct DESC_RNG_LEN to (4 * CAAM_CMD_SZ). please. Kim -- 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 v2 5/5] crypto: talitos: Add software backlog queue handling
On Thu, 5 Mar 2015 11:35:23 +0200 Horia Geantă wrote: > On 3/4/2015 2:23 AM, Kim Phillips wrote: > > On Tue, 3 Mar 2015 08:21:37 -0500 > > Martin Hicks wrote: > > > >> @@ -1170,6 +1237,8 @@ static struct talitos_edesc > >> *talitos_edesc_alloc(struct device *dev, > >> edesc->dma_len, > >> DMA_BIDIRECTIONAL); > >>edesc->req.desc = &edesc->desc; > >> + /* A copy of the crypto_async_request to use the crypto_queue backlog */ > >> + memcpy(&edesc->req.base, areq, sizeof(struct crypto_async_request)); > > > > this seems backward, or, at least can be done more efficiently IMO: > > talitos_cra_init should set the tfm's reqsize so the rest of > > the driver can wholly embed its talitos_edesc (which should also > > wholly encapsulate its talitos_request (i.e., not via a pointer)) > > into the crypto API's request handle allocation. This > > would absorb and eliminate the talitos_edesc kmalloc and frees, the > > above memcpy, and replace the container_of after the > > crypto_dequeue_request with an offset_of, right? > > > > When scatter-gather buffers are needed, we can assume a slower-path > > and make them do their own allocations, since their sizes vary > > depending on each request. Of course, a pointer to those > > allocations would need to be retained somewhere in the request > > handle. > > Unfortunately talitos_edesc structure size is most of the times > variable. Its exact size can only be established at "request time", and > not at "tfm init time". yes, I was suggesting a common minimum should be set in cra_init. > Fixed size would be sizeof(talitos_edesc). > Below are factors that influence the variable part, i.e. link_tbl in > talitos_edesc: > - whether any assoc / src / dst data is scattered > - icv_stashing (case when ICV checking is done in SW) both being slow(er) paths, IMO. > Still we'd be better with: > -crypto API allocates request + request context (i.e. > sizeof(talitos_edesc) + any alignment required) > -talitos driver allocates variable part of talitos_edesc (if needed) > > instead of: > -crypto API allocates request > -talitos driver allocates talitos_edesc (fixed + variable) > -memcopy of the req.base (crypto_async_request) into talitos_edesc > > both in terms of performance and readability. indeed. > At first look, the driver wouldn't change that much: > -talitos_cra_init() callback would have to set tfm.reqsize to > sizeof(talitos_edesc) + padding and also add the CRYPTO_TFM_REQ_DMA > indication in tfm.crt_flags > -talitos_edesc_alloc() logic would be pretty much the same, but would > allocate memory only for the link_tbl > > I'm willing to do these changes if needed. Please coordinate with Martin. fwiw, caam could use this, too. Kim -- 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 1/3] crypto: caam: fix some compile warnings
On Thu, 5 Mar 2015 09:12:13 +0200 Cristian Stoica wrote: > On 03/04/2015 08:34 PM, Kim Phillips wrote: > > > I don't see how, e.g., for one, dma_map_sg is I/O TLB > > implementation-dependent. > > I'll need some remedial classes on this topic, but for the moment I > don't see how this matters for mapping sg chains. Can you share some > pointers? I think it's better the driver not depend on whether the dma_map_sg implementation supports chains, but you're welcome to try... Kim -- 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 1/2] crypto: caamhash: - fix uninitialized edesc->sec4_sg_bytes field
On Fri, 6 Mar 2015 10:34:41 +0800 wrote: > From: Yanjiang Jin > > sec4_sg_bytes not being properly initialized causes ahash_done > to try to free unallocated DMA memory: > > caam_jr ffe301000.jr: DMA-API: device driver tries to free DMA memory it has > not allocated [device address=0xdeadbeefdeadbeef] [size=3735928559 bytes] > [ cut here ] > WARNING: at lib/dma-debug.c:1093 > Modules linked in: > CPU: 1 PID: 0 Comm: swapper/1 Not tainted 4.0.0-rc1+ #6 > task: e9598c00 ti: effca000 task.ti: e95a2000 > NIP: c04ef24c LR: c04ef24c CTR: c0549730 > REGS: effcbd40 TRAP: 0700 Not tainted (4.0.0-rc1+) > MSR: 00029002 CR: 22008084 XER: 2000 > > GPR00: c04ef24c effcbdf0 e9598c00 0096 c08f7424 c00ab2b0 0001 > GPR08: c0fe7510 effca000 01c3 22008082 c1048e77 c105 > GPR16: c0c36700 493c0040 002c e690e4a0 c1054fb4 c18bac40 00029002 c18b0788 > GPR24: 0014 e690e480 effcbe48 c0fde128 e6ffac10 deadbeef deadbeef > NIP [c04ef24c] check_unmap+0x93c/0xb40 > LR [c04ef24c] check_unmap+0x93c/0xb40 > Call Trace: > [effcbdf0] [c04ef24c] check_unmap+0x93c/0xb40 (unreliable) > [effcbe40] [c04ef4f4] debug_dma_unmap_page+0xa4/0xc0 > [effcbec0] [c070cda8] ahash_done+0x128/0x1a0 > [effcbef0] [c0700070] caam_jr_dequeue+0x1d0/0x290 > [effcbf40] [c0045f40] tasklet_action+0x110/0x1f0 > [effcbf80] [c0044bc8] __do_softirq+0x188/0x700 > [effcbfe0] [c00455d8] irq_exit+0x108/0x120 > [effcbff0] [c000f520] call_do_irq+0x24/0x3c > [e95a3e20] [c00059b8] do_IRQ+0xc8/0x170 > [e95a3e50] [c0011bc8] ret_from_except+0x0/0x18 > > Signed-off-by: Yanjiang Jin > --- Acked-by: Kim Phillips Thanks, Kim -- 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
[PATCH] crypto: powerpc - move files to fix build error
The current cryptodev-2.6 tree commits:
d9850fc529ef ("crypto: powerpc/sha1 - kernel config")
50ba29aaa7b0 ("crypto: powerpc/sha1 - glue")
failed to properly place files under arch/powerpc/crypto, which
leads to build errors:
make[1]: *** No rule to make target 'arch/powerpc/crypto/sha1-spe-asm.o',
needed by 'arch/powerpc/crypto/sha1-ppc-spe.o'. Stop.
make[1]: *** No rule to make target 'arch/powerpc/crypto/sha1_spe_glue.o',
needed by 'arch/powerpc/crypto/sha1-ppc-spe.o'. Stop.
Makefile:947: recipe for target 'arch/powerpc/crypto' failed
Move the two sha1 spe files under crypto/, and whilst there, rename
other powerpc crypto files with underscores to use dashes for
consistency.
Cc: Markus Stockhausen
Signed-off-by: Kim Phillips
---
applies to today's cryptodev-2.6.
arch/powerpc/crypto/Makefile | 8
arch/powerpc/crypto/{aes_spe_glue.c => aes-spe-glue.c} | 0
arch/powerpc/crypto/{md5_glue.c => md5-glue.c} | 0
arch/powerpc/{ => crypto}/sha1-spe-asm.S | 0
arch/powerpc/{sha1_spe_glue.c => crypto/sha1-spe-glue.c} | 0
arch/powerpc/crypto/{sha256_spe_glue.c => sha256-spe-glue.c} | 0
6 files changed, 4 insertions(+), 4 deletions(-)
rename arch/powerpc/crypto/{aes_spe_glue.c => aes-spe-glue.c} (100%)
rename arch/powerpc/crypto/{md5_glue.c => md5-glue.c} (100%)
rename arch/powerpc/{ => crypto}/sha1-spe-asm.S (100%)
rename arch/powerpc/{sha1_spe_glue.c => crypto/sha1-spe-glue.c} (100%)
rename arch/powerpc/crypto/{sha256_spe_glue.c => sha256-spe-glue.c} (100%)
diff --git a/arch/powerpc/crypto/Makefile b/arch/powerpc/crypto/Makefile
index c6b25cba..9c221b6 100644
--- a/arch/powerpc/crypto/Makefile
+++ b/arch/powerpc/crypto/Makefile
@@ -10,8 +10,8 @@ obj-$(CONFIG_CRYPTO_SHA1_PPC) += sha1-powerpc.o
obj-$(CONFIG_CRYPTO_SHA1_PPC_SPE) += sha1-ppc-spe.o
obj-$(CONFIG_CRYPTO_SHA256_PPC_SPE) += sha256-ppc-spe.o
-aes-ppc-spe-y := aes-spe-core.o aes-spe-keys.o aes-tab-4k.o aes-spe-modes.o
aes_spe_glue.o
-md5-ppc-y := md5-asm.o md5_glue.o
+aes-ppc-spe-y := aes-spe-core.o aes-spe-keys.o aes-tab-4k.o aes-spe-modes.o
aes-spe-glue.o
+md5-ppc-y := md5-asm.o md5-glue.o
sha1-powerpc-y := sha1-powerpc-asm.o sha1.o
-sha1-ppc-spe-y := sha1-spe-asm.o sha1_spe_glue.o
-sha256-ppc-spe-y := sha256-spe-asm.o sha256_spe_glue.o
+sha1-ppc-spe-y := sha1-spe-asm.o sha1-spe-glue.o
+sha256-ppc-spe-y := sha256-spe-asm.o sha256-spe-glue.o
diff --git a/arch/powerpc/crypto/aes_spe_glue.c
b/arch/powerpc/crypto/aes-spe-glue.c
similarity index 100%
rename from arch/powerpc/crypto/aes_spe_glue.c
rename to arch/powerpc/crypto/aes-spe-glue.c
diff --git a/arch/powerpc/crypto/md5_glue.c b/arch/powerpc/crypto/md5-glue.c
similarity index 100%
rename from arch/powerpc/crypto/md5_glue.c
rename to arch/powerpc/crypto/md5-glue.c
diff --git a/arch/powerpc/sha1-spe-asm.S b/arch/powerpc/crypto/sha1-spe-asm.S
similarity index 100%
rename from arch/powerpc/sha1-spe-asm.S
rename to arch/powerpc/crypto/sha1-spe-asm.S
diff --git a/arch/powerpc/sha1_spe_glue.c b/arch/powerpc/crypto/sha1-spe-glue.c
similarity index 100%
rename from arch/powerpc/sha1_spe_glue.c
rename to arch/powerpc/crypto/sha1-spe-glue.c
diff --git a/arch/powerpc/crypto/sha256_spe_glue.c
b/arch/powerpc/crypto/sha256-spe-glue.c
similarity index 100%
rename from arch/powerpc/crypto/sha256_spe_glue.c
rename to arch/powerpc/crypto/sha256-spe-glue.c
--
2.3.1
--
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 2/2] crypto: caam_rng: fix rng_unmap_ctx's DMA_UNMAP size problem
On Fri, 6 Mar 2015 10:34:42 +0800 wrote: > From: Yanjiang Jin > > Fix rng_unmap_ctx's DMA_UNMAP size problem for caam_rng, else system would > report the below calltrace during cleanup caam_rng. > Since rng_create_sh_desc() creates a fixed descriptor of exactly 4 > command-lengths now, also update DESC_RNG_LEN to (4 * CAAM_CMD_SZ). > > caam_jr ffe301000.jr: DMA-API: device driver frees DMA memory with different > size [device address=0x7f080010] [map size=16 bytes] [unmap size=40 > bytes] > [ cut here ] > WARNING: at lib/dma-debug.c:887 > Modules linked in: > task: c000f7cdaa80 ti: c000e534 task.ti: c000e534 > NIP: c04f5bc8 LR: c04f5bc4 CTR: c05f69b0 > REGS: c000e53433c0 TRAP: 0700 Not tainted > MSR: 80029000 CR: 24088482 XER: > SOFTE: 0 > > GPR00: c04f5bc4 c000e5343640 c12af360 009f > GPR04: 00a0 c0d02070 c00015980660 > GPR08: c0cff360 c12da018 > GPR12: 01e3 c1fff780 100f 0001 > GPR16: 0002 > GPR20: 0001 > GPR24: 0001 0001 0001 > GPR28: c1556b90 c1565b80 c000e5343750 c000f9427480 > NIP [c04f5bc8] .check_unmap+0x538/0x9c0 > LR [c04f5bc4] .check_unmap+0x534/0x9c0 > Call Trace: > [c000e5343640] [c04f5bc4] .check_unmap+0x534/0x9c0 (unreliable) > [c000e53436e0] [c04f60d4] .debug_dma_unmap_page+0x84/0xb0 > [c000e5343810] [c082f9d4] .caam_cleanup+0x1d4/0x240 > [c000e53438a0] [c056cc88] .hwrng_unregister+0xd8/0x1c0 > Instruction dump: > 7c641b78 41de0410 e8a90050 2fa5 419e0484 e8de0028 e8ff0030 3c62ff90 > e91e0030 38638388 48546ed9 6000 <0fe0> 3c62ff8f 38637fc8 48546ec5 > ---[ end trace e43fd1734d6600df ]--- > > Signed-off-by: Yanjiang Jin > --- Acked-by: Kim Phillips Thanks, Kim -- 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 2/2] crypto: talitos: Add AES-XTS Support
On Fri, 6 Mar 2015 11:49:43 -0500 Martin Hicks wrote: > On Thu, Mar 5, 2015 at 7:16 PM, Kim Phillips > wrote: > > On Fri, 20 Feb 2015 12:00:10 -0500 > > Martin Hicks wrote: > > > >> The newer talitos hardware has support for AES in XTS mode. > > > > Assuming it's the same thing, AES-XCBC gets added with SEC v3.0 > > h/w. Assuming hw_supports() doesn't already support this algorithm > > AES-XCBC isn't the same thing as AES-XTS. Thanks. > > combination (technically via the mode bit), this needs to be > > reflected in the patch so the driver doesn't think SEC 2.x devices > > can do XTS, too. > > Right. I hadn't looked into how exactly hw_supports() works. It only > indicates which execution units are present (in this case the AES > unit). I actually think XTS gets introduced in SEC v3.3.2. I also > have an MPC8379 (sec3.3) and it does not have XTS. > > Can you look internally to find out in which hardware it was > introduced? Is there a SEC 3.3.1 that also has XTS? later MPC8315Es had a SEC v3.3.1, but AFAICT, it doesn't support XTS, so, yes, it's likely v3.3.2 and above (if any). > I guess I just need a ->features flag to conditionally register the > XTS algorithm for 3.3.x and newer. Adding anything more complicated > doesn't seem warranted at this time, although that could change if > someone came along and needed to add a whole whack more of the AES > modes that were introduced at various times in the different SEC > revisions. OK. Note: there's some SEC node fixup code in u-boot that could be used for this job, too. > >> + .desc_hdr_template = DESC_HDR_TYPE_COMMON_NONSNOOP_NO_AFEU | > >> + DESC_HDR_SEL0_AESU | > >> + DESC_HDR_MODE0_AESU_XTS, > > > > ... > > > >> /* primary execution unit mode (MODE0) and derivatives */ > >> #define DESC_HDR_MODE0_ENCRYPT cpu_to_be32(0x0010) > >> #define DESC_HDR_MODE0_AESU_CBC cpu_to_be32(0x0020) > >> +#define DESC_HDR_MODE0_AESU_XTS cpu_to_be32(0x0420) > > > > I'd prefer these defines be kept as single bit definitions, and keep > > their names from the manual. An XTS-specific definition can be > > composed from them either after this, or manually in the > > desc_hdr_template assignment above. > > It doesn't look like there are divisible single-bit definitions for > the MODE0 bits. The AES Cipher modes are composed of 5 bits called > CipherMode, Extended CipherMode and Aux2. Individually they don't > seem to mean anything. Unless you want something like: > > #define AES_MODE(cm, ecm, aux2) cpu_to_be32(((ecm<<6) | (aux2<<5) | > (cm<<1)) << 20) > > #define DESC_HDR_MODE0_AESU_CBC AES_MODE(1, 0, 0) > #define DESC_HDR_MODE0_AESU_XTS AES_MODE(1, 1, 0) the way you had it seems to be ok for now. Kim -- 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 v2 15/17] crypto: talitos - Implementation of SEC1
On Fri, 6 Mar 2015 17:42:26 +0100
Christophe Leroy wrote:
> This patch adds talitos1.c and talitos1.h with all specificities needed
> to handle the SEC1 security engine found in MPC885 and MPC8272.
>
> The SEC1 has several differences with its younger brother SEC2:
> * Several bits in registers have different locations
> * Many bits are missing
> * Some bits are in addition
> * SEC1 doesn't support scatter/gather
> * SEC1 has a different descriptor structure
>
> We add a helper function for clearing the desc field in the descriptor as
> that field needs to be cleared on SEC1 and doesn't exist on SEC2.
>
> Signed-off-by: Christophe Leroy
Hi Christophe,
Thanks for rebasing on cryptodev-2.6, now I can easily see the
differences for review.
> +++ b/drivers/crypto/Kconfig
> @@ -211,7 +211,8 @@ config CRYPTO_DEV_TALITOS
> select CRYPTO_ALGAPI
> select CRYPTO_AUTHENC
> select HW_RANDOM
> - select CRYPTO_DEV_TALITOS2
> + select CRYPTO_DEV_TALITOS1 if PPC_8xx || PPC_82xx
> + select CRYPTO_DEV_TALITOS2 if !CRYPTO_DEV_TALITOS1
This will set CONFIG_CRYPTO_DEV_TALITOS1=y on a kernel with both
PPC_82xx and PPC_83xx set, and will therefore break talitos when run
on an 83xx part.
The driver needs to work on both v1 and v2/3 SECs dynamically,
and behave accordingly depending on what version h/w is described in
the device tree ("fsl,secX").
So, barring making a completely new driver, a couple of points:
- A h/w version 1 vs 2 compatible priv variable can be allocated for
parts of the driver that need quicker access than calling
of_device_is_compatible(). Other parts that do better as complete
function rewrites (the scatter-gather mapping fns?) can be
abstracted away using pointers to the v1 vs. v2+ base functions.
The pointers would live in the device's priv struct, and be assigned
at module initialization time.
- instead of rewriting the structs talitos_ptr, talitos_desc, either
use the v2-named member as-is, or make unions.
E.g., instead of having a new struct talitos_ptr, either use the v2
h/w version as-is and shift the length into place, or, add a v1 len
via a union, and make to_talitos_ptr a pointer to a function, with
the v1 version of the function updating only the v1 len.
For struct talitos_desc, how about:
struct talitos_desc {
__be32 hdr;
struct talitos_ptr[8];
}
and have the desc->ptr[X] assigning code add 1 via a new macro iff
on a SEC2 compatible (the hdr_lo code would have to be hardcoded to
refer to ptr[0] instead of hdr_lo directly, and next_desc would be
ptr[7]).
talitos_edesc shouldn't need the extra ptr_{in,out}, rather allocate
the extra space when they're needed, and refer to them via aliases
to defines that cast and use link_tbl[0] as a base reference.
For IO register accesses, either a lookup table can be used, or,
since they're not frequent, define v1-specific defines in addition
to the v2 ones, and reference using the compatible variable.
Other than that, please make sure to use checkpatch.pl --strict,
sparse -D__CHECK_ENDIAN__, and that building as a module doesn't
break.
Thanks,
Kim
--
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 v2 5/5] crypto: talitos: Add software backlog queue handling
On Mon, 16 Mar 2015 12:02:51 +0200
Horia Geantă wrote:
> On 3/4/2015 2:23 AM, Kim Phillips wrote:
> > Only potential problem is getting the crypto API to set the GFP_DMA
> > flag in the allocation request, but presumably a
> > CRYPTO_TFM_REQ_DMA crt_flag can be made to handle that.
>
> Seems there are quite a few places that do not use the
> {aead,ablkcipher_ahash}_request_alloc() API to allocate crypto requests.
> Among them, IPsec and dm-crypt.
> I've looked at the code and I don't think it can be converted to use
> crypto API.
why not?
> This means that the CRYPTO_TFM_REQ_DMA would be visible to all of these
> places. Some of the maintainers do not agree, as you've seen.
would modifying the crypto API to either have a different
*_request_alloc() API, and/or adding calls to negotiate the GFP mask
between crypto users and drivers, e.g., get/set_gfp_mask, work?
> An alternative would be for talitos to use the page allocator to get 1 /
> 2 pages at probe time (4 channels x 32 entries/channel x 64B/descriptor
> = 8 kB), dma_map_page the area and manage it internally for talitos_desc
> hw descriptors.
> What do you think?
There's a comment in esp_alloc_tmp(): "Use spare space in skb for
this where possible," which is ideally where we'd want to be (esp.
because that memory could already be DMA-able). Your above
suggestion would be in the opposite direction of that.
Kim
--
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 v2 5/5] crypto: talitos: Add software backlog queue handling
On Tue, 17 Mar 2015 19:58:55 +0200
Horia Geantă wrote:
> On 3/17/2015 2:19 AM, Kim Phillips wrote:
> > On Mon, 16 Mar 2015 12:02:51 +0200
> > Horia Geantă wrote:
> >
> >> On 3/4/2015 2:23 AM, Kim Phillips wrote:
> >>> Only potential problem is getting the crypto API to set the GFP_DMA
> >>> flag in the allocation request, but presumably a
> >>> CRYPTO_TFM_REQ_DMA crt_flag can be made to handle that.
> >>
> >> Seems there are quite a few places that do not use the
> >> {aead,ablkcipher_ahash}_request_alloc() API to allocate crypto requests.
> >> Among them, IPsec and dm-crypt.
> >> I've looked at the code and I don't think it can be converted to use
> >> crypto API.
> >
> > why not?
>
> It would imply having 2 memory allocations, one for crypto request and
> the other for the rest of the data bundled with the request (for IPsec
> that would be ESN + space for IV + sg entries for authenticated-only
> data and sk_buff extension, if needed).
>
> Trying to have a single allocation by making ESN, IV etc. part of the
> request private context requires modifying tfm.reqsize on the fly.
> This won't work without adding some kind of locking for the tfm.
can't a common minimum tfm.reqsize be co-established up front, at
least for the fast path?
> >> This means that the CRYPTO_TFM_REQ_DMA would be visible to all of these
> >> places. Some of the maintainers do not agree, as you've seen.
> >
> > would modifying the crypto API to either have a different
> > *_request_alloc() API, and/or adding calls to negotiate the GFP mask
> > between crypto users and drivers, e.g., get/set_gfp_mask, work?
>
> I think what DaveM asked for was the change to be transparent.
>
> Besides converting to *_request_alloc(), seems that all other options
> require some extra awareness from the user.
> Could you elaborate on the idea above?
was merely suggesting communicating GFP flags anonymously across the
API, i.e., GFP_DMA wouldn't appear in user code.
> >> An alternative would be for talitos to use the page allocator to get 1 /
> >> 2 pages at probe time (4 channels x 32 entries/channel x 64B/descriptor
> >> = 8 kB), dma_map_page the area and manage it internally for talitos_desc
> >> hw descriptors.
> >> What do you think?
> >
> > There's a comment in esp_alloc_tmp(): "Use spare space in skb for
> > this where possible," which is ideally where we'd want to be (esp.
>
> Ok, I'll check that. But note the "where possible" - finding room in the
> skb to avoid the allocation won't always be the case, and then we're
> back to square one.
>
> > because that memory could already be DMA-able). Your above
> > suggestion would be in the opposite direction of that.
>
> The proposal:
> -removes dma (un)mapping on the fast path
sure, but at the expense of additional complexity.
> -avoids requesting dma mappable memory for more than it's actually
> needed (CRYPTO_TFM_REQ_DMA forces entire request to be mappable, not
> only its private context)
compared to the payload? Plus, we have plenty of DMA space these
days.
> -for caam it has the added benefit of speeding the below search for the
> offending descriptor in the SW ring from O(n) to O(1):
> for (i = 0; CIRC_CNT(head, tail + i, JOBR_DEPTH) >= 1; i++) {
> sw_idx = (tail + i) & (JOBR_DEPTH - 1);
>
> if (jrp->outring[hw_idx].desc ==
> jrp->entinfo[sw_idx].desc_addr_dma)
> break; /* found */
> }
> (drivers/crypto/caam/jr.c - caam_dequeue)
how? The job ring h/w will still be spitting things out
out-of-order.
Plus, like I said, it's taking the problem in the wrong direction:
we need to strive to merge the allocation and mapping with the upper
layers as much as possible.
Kim
--
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 v2 5/5] crypto: talitos: Add software backlog queue handling
On Thu, 19 Mar 2015 17:56:57 +0200
Horia Geantă wrote:
> On 3/18/2015 12:03 AM, Kim Phillips wrote:
> > On Tue, 17 Mar 2015 19:58:55 +0200
> > Horia Geantă wrote:
> >
> >> On 3/17/2015 2:19 AM, Kim Phillips wrote:
> >>> On Mon, 16 Mar 2015 12:02:51 +0200
> >>> Horia Geantă wrote:
> >>>
> >>>> On 3/4/2015 2:23 AM, Kim Phillips wrote:
> >>>>> Only potential problem is getting the crypto API to set the GFP_DMA
> >>>>> flag in the allocation request, but presumably a
> >>>>> CRYPTO_TFM_REQ_DMA crt_flag can be made to handle that.
> >>>>
> >>>> Seems there are quite a few places that do not use the
> >>>> {aead,ablkcipher_ahash}_request_alloc() API to allocate crypto requests.
> >>>> Among them, IPsec and dm-crypt.
> >>>> I've looked at the code and I don't think it can be converted to use
> >>>> crypto API.
> >>>
> >>> why not?
> >>
> >> It would imply having 2 memory allocations, one for crypto request and
> >> the other for the rest of the data bundled with the request (for IPsec
> >> that would be ESN + space for IV + sg entries for authenticated-only
> >> data and sk_buff extension, if needed).
> >>
> >> Trying to have a single allocation by making ESN, IV etc. part of the
> >> request private context requires modifying tfm.reqsize on the fly.
> >> This won't work without adding some kind of locking for the tfm.
> >
> > can't a common minimum tfm.reqsize be co-established up front, at
> > least for the fast path?
>
> Indeed, for IPsec at tfm allocation time - esp_init_state() -
> tfm.reqsize could be increased to account for what is known for a given
> flow: ESN, IV and asg (S/G entries for authenticated-only data).
> The layout would be:
> aead request (fixed part)
> private ctx of backend algorithm
> seq_no_hi (if ESN)
> IV
> asg
> sg <-- S/G table for skb_to_sgvec; how many entries is the question
>
> Do you have a suggestion for how many S/G entries to preallocate for
> representing the sk_buff data to be encrypted?
> An ancient esp4.c used ESP_NUM_FAST_SG, set to 4.
> Btw, currently maximum number of fragments supported by the net stack
> (MAX_SKB_FRAGS) is 16 or more.
>
> >>>> This means that the CRYPTO_TFM_REQ_DMA would be visible to all of these
> >>>> places. Some of the maintainers do not agree, as you've seen.
> >>>
> >>> would modifying the crypto API to either have a different
> >>> *_request_alloc() API, and/or adding calls to negotiate the GFP mask
> >>> between crypto users and drivers, e.g., get/set_gfp_mask, work?
> >>
> >> I think what DaveM asked for was the change to be transparent.
> >>
> >> Besides converting to *_request_alloc(), seems that all other options
> >> require some extra awareness from the user.
> >> Could you elaborate on the idea above?
> >
> > was merely suggesting communicating GFP flags anonymously across the
> > API, i.e., GFP_DMA wouldn't appear in user code.
>
> Meaning user would have to get_gfp_mask before allocating a crypto
> request - i.e. instead of kmalloc(..., GFP_ATOMIC) to have
> kmalloc(GFP_ATOMIC | get_gfp_mask(aead))?
>
> >>>> An alternative would be for talitos to use the page allocator to get 1 /
> >>>> 2 pages at probe time (4 channels x 32 entries/channel x 64B/descriptor
> >>>> = 8 kB), dma_map_page the area and manage it internally for talitos_desc
> >>>> hw descriptors.
> >>>> What do you think?
> >>>
> >>> There's a comment in esp_alloc_tmp(): "Use spare space in skb for
> >>> this where possible," which is ideally where we'd want to be (esp.
> >>
> >> Ok, I'll check that. But note the "where possible" - finding room in the
> >> skb to avoid the allocation won't always be the case, and then we're
> >> back to square one.
>
> So the skb cb is out of the question, being too small (48B).
> Any idea what was the intention of the "TODO" - maybe to use the
> tailroom in the skb data area?
>
> >>> because that memory could already be DMA-able). Your above
> >>> suggestion would be in the opposite direction of that.
> >>
> >> The proposal:
> >> -removes dma (un)mapping on the fast path
> >
> > sure, but at the expense of additional complexity.
>
> Right, there'
Re: [PATCH RESEND 1/4] crypto: caam: add caam-dma node to SEC4.0 device tree binding
On Mon, 30 Oct 2017 15:46:51 +0200
Horia Geantă wrote:
> +=
> +CAAM DMA Node
> +
> +Child node of the crypto node that enables the use of the DMA
> capabilities
> +of the CAAM by a stand-alone driver. The only required property is the
> +"compatible" property. All the other properties are determined from
> +the job rings on which the CAAM DMA driver depends (ex: the number of
> +dma-channels is equal to the number of defined job rings).
> +
> + - compatible
> + Usage: required
> + Value type:
> + Definition: Must include "fsl,sec-v4.0-dma"
> +
> +EXAMPLE
> + caam-dma {
> +compatible = "fsl,sec-v5.4-dma",
> + "fsl,sec-v5.0-dma",
> + "fsl,sec-v4.0-dma";
> + }
If this isn't describing an aspect of the hardware, then what is it
doing in the device tree?
Kim
Re: [PATCH RESEND 1/4] crypto: caam: add caam-dma node to SEC4.0 device tree binding
On Thu, 9 Nov 2017 11:54:13 +
Radu Andrei Alexe wrote:
> On 10/30/2017 4:24 PM, Kim Phillips wrote:
> > On Mon, 30 Oct 2017 15:46:51 +0200
> > Horia Geantă wrote:
> >
> >> +=
> >> +CAAM DMA Node
> >> +
> >> +Child node of the crypto node that enables the use of the DMA
> >> capabilities
> >> +of the CAAM by a stand-alone driver. The only required property is the
> >> +"compatible" property. All the other properties are determined from
> >> +the job rings on which the CAAM DMA driver depends (ex: the number of
> >> +dma-channels is equal to the number of defined job rings).
> >> +
> >> + - compatible
> >> + Usage: required
> >> + Value type:
> >> + Definition: Must include "fsl,sec-v4.0-dma"
> >> +
> >> +EXAMPLE
> >> + caam-dma {
> >> +compatible = "fsl,sec-v5.4-dma",
> >> + "fsl,sec-v5.0-dma",
> >> + "fsl,sec-v4.0-dma";
> >> + }
> >
> > If this isn't describing an aspect of the hardware, then what is it
> > doing in the device tree?
> >
> > Kim
> >
>
> Because the caam_dma driver is a platform driver I needed to create a
> platform device to activate the driver. My thought was that it was
> simpler to implement it using device tree.
> The next patch version will create the platform device dynamically at
> run time.
Why create a new device when that h/w already has one?
Why doesn't the existing crypto driver register dma capabilities with
the dma driver subsystem?
Kim
Re: [PATCH RESEND 1/4] crypto: caam: add caam-dma node to SEC4.0 device tree binding
On Fri, 10 Nov 2017 08:02:01 + Radu Andrei Alexe wrote: > On 11/9/2017 6:34 PM, Kim Phillips wrote: > > On Thu, 9 Nov 2017 11:54:13 + > > Radu Andrei Alexe wrote: > >> The next patch version will create the platform device dynamically at > >> run time. > > > > Why create a new device when that h/w already has one? > > > > Why doesn't the existing crypto driver register dma capabilities with > > the dma driver subsystem? > > > I can think of two reasons: > > 1. The code that this driver introduces has nothing to do with crypto > and everything to do with dma. I would think that at least a crypto "null" algorithm implementation would share code. > Placing the code in the same directory as > the caam subsystem would only create confusion for the reader of an > already complex driver. this different directory argument seems to be identical to your 2 below: > 2. I wanted this driver to be tracked by the dma engine team. They have > the right expertise to provide adequate feedback. If all the code was in > the crypto directory they wouldn't know about this driver or any > subsequent changes to it. dma subsystem bits could still be put in the dma area if deemed necessary but I don't think it is: I see drivers/crypto/ccp/ccp-dmaengine.c calls dma_async_device_register for example. I also don't see how that complicates things much further. What is the rationale for using the crypto h/w as a dma engine anyway? Are there supporting performance figures? Kim
Re: [PATCH RESEND 1/4] crypto: caam: add caam-dma node to SEC4.0 device tree binding
On Mon, 13 Nov 2017 08:32:24 + Horia Geantă wrote: > On 11/10/2017 6:44 PM, Kim Phillips wrote: > > On Fri, 10 Nov 2017 08:02:01 + > > Radu Andrei Alexe wrote: > [snip]>> 2. I wanted this driver to be tracked by the dma engine team. > They have > >> the right expertise to provide adequate feedback. If all the code was in > >> the crypto directory they wouldn't know about this driver or any > >> subsequent changes to it. > > > > dma subsystem bits could still be put in the dma area if deemed > > necessary but I don't think it is: I see > > drivers/crypto/ccp/ccp-dmaengine.c calls dma_async_device_register for > > example. > > > Please see previous discussion with Vinod: > https://www.mail-archive.com/linux-crypto@vger.kernel.org/msg21468.html Vinod says: "If the dma controller is internal to crypto, then it might be okay to be inside the crypto driver." Is that the case for the CCP driver? Isn't it the case here? In any case, I don't care that much about that, this all begat from new *devices* coming out of nowhere. > > What is the rationale for using the crypto h/w as a dma engine anyway? > SoCs that don't have a system DMA, for e.g. LS1012A. OK. Kim
Re: [PATCH RESEND 1/4] crypto: caam: add caam-dma node to SEC4.0 device tree binding
On Mon, 13 Nov 2017 09:44:06 + Radu Andrei Alexe wrote: > On 11/10/2017 6:44 PM, Kim Phillips wrote: > > On Fri, 10 Nov 2017 08:02:01 + > > Radu Andrei Alexe wrote: > > > >> On 11/9/2017 6:34 PM, Kim Phillips wrote: > >>> On Thu, 9 Nov 2017 11:54:13 + > >>> Radu Andrei Alexe wrote: > >>>> The next patch version will create the platform device dynamically at > >>>> run time. > >>> > >>> Why create a new device when that h/w already has one? > >>> > >>> Why doesn't the existing crypto driver register dma capabilities with > >>> the dma driver subsystem? > >>> > >> I can think of two reasons: > >> > >> 1. The code that this driver introduces has nothing to do with crypto > >> and everything to do with dma. > > > > I would think that at least a crypto "null" algorithm implementation > > would share code. > > > >> Placing the code in the same directory as > >> the caam subsystem would only create confusion for the reader of an > >> already complex driver. > > > > this different directory argument seems to be identical to your 2 below: > > > >> 2. I wanted this driver to be tracked by the dma engine team. They have > >> the right expertise to provide adequate feedback. If all the code was in > >> the crypto directory they wouldn't know about this driver or any > >> subsequent changes to it. > > > > dma subsystem bits could still be put in the dma area if deemed > > necessary but I don't think it is: I see > > drivers/crypto/ccp/ccp-dmaengine.c calls dma_async_device_register for > > example. > > > > I also don't see how that complicates things much further. > > > > So who made their review? The guys from crypto? Don't see how that's relevant here, but people applying patches should solicit acks from the appropriate sources, esp. if a patch is across multiple subsystems. > If someone wants to enable only the DMA functionality of the CCP and not > the crypto part how do they do it? Look for it in the crypto submenu? Why would they want to do that? In any case, I suspect you're thinking about cross-subsystem Kconfig entries, which is common, but something like that can be a module parameter, too. I would say that maybe CRYPTO_DEV_FSL_CAAM should be made to not depend on CRYPTO_HW, but I think that's overkill for the addition of this minor feature. > > What is the rationale for using the crypto h/w as a dma engine anyway? > > Are there supporting performance figures? > > We have a platform that doesn't have a dedicated DMA controller but has > the CAAM hardware block that can perform dma transfers. We have a OK, please mention that next time. > use-case where we need to issue large transfers (hundred of MBs) > asynchronously, without using the core. Curious: what subsystem does that? Thanks, Kim
Re: AEAD in TALITOS SEC1 versus TALITOS SEC2
On Thu, 21 Apr 2016 13:31:47 + Horia Ioan Geanta Neag wrote: > On 4/20/2016 3:04 PM, Christophe Leroy wrote: > > Today, in Talitos driver crypto alg registration is based on predefined > > templates with a predefined descriptor type and verification against the > > descriptors supported by the HW. This works well for ALG that require a > > unique descriptor. But for IPsec this is slightly different: > > * IPsec can be performed with both DESC_HDR_TYPE_IPSEC_ESP and > > DESC_HDR_TYPE_HMAC_SNOOP_NO_AFEU > > * DESC_HDR_TYPE_IPSEC_ESP is supported only by SEC2 > > * DESC_HDR_TYPE_HMAC_SNOOP_NO_AFEU is supported by both SEC1 and SEC2 > > * DESC_HDR_TYPE_HMAC_SNOOP_NO_AFEU is less performant than > > DESC_HDR_TYPE_IPSEC_ESP > > So it is natural to use DESC_HDR_TYPE_IPSEC_ESP when it is supported and > > use DESC_HDR_TYPE_HMAC_SNOOP_NO_AFEU otherwise ? > > > > What's the best way to implement the selection of the proper descriptor > > type ? > > * We can duplicate the templates but it means that when both types are > > supported the driver with try to register each AEAD alg twice > > * We can "on the fly" change the DESC_HDR_TYPE_IPSEC_ESP type into > > DESC_HDR_TYPE_HMAC_SNOOP_NO_AFEU type ? > > * We can alter the templates at startup when we know we are on a SEC1, > > changing all templates based on DESC_HDR_TYPE_IPSEC_ESP into templates > > based on DESC_HDR_TYPE_HMAC_SNOOP_NO_AFEU > > > > What would be the best approach from your point of view ? > > > I would go with altering the relevant entries in the template array, of > course before the hw_supports() check. > > IIUC, the "on the fly" option won't work. There has to be a valid > descriptor type for each template entry before hw_supports(). > > Regards, > Horia > -- > 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 > IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. -- 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: AEAD in TALITOS SEC1 versus TALITOS SEC2
On Thu, 21 Apr 2016 13:31:47 + Horia Ioan Geanta Neag wrote: > On 4/20/2016 3:04 PM, Christophe Leroy wrote: > > What's the best way to implement the selection of the proper descriptor > > type ? > > * We can duplicate the templates but it means that when both types are > > supported the driver with try to register each AEAD alg twice > > * We can "on the fly" change the DESC_HDR_TYPE_IPSEC_ESP type into > > DESC_HDR_TYPE_HMAC_SNOOP_NO_AFEU type ? > > * We can alter the templates at startup when we know we are on a SEC1, > > changing all templates based on DESC_HDR_TYPE_IPSEC_ESP into templates > > based on DESC_HDR_TYPE_HMAC_SNOOP_NO_AFEU > > > > What would be the best approach from your point of view ? > > > I would go with altering the relevant entries in the template array, of > course before the hw_supports() check. was wondering whether assigning a different cra_priority were another option? Kim IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. -- 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] crypto: talitos: fix driver init
On Thu, 28 Apr 2016 17:15:30 +0300
Alexandru Ardelean wrote:
> From: Alexandru Ardelean
>
> Crypto hash algorithms must provide the statesize sometime
> from kernel 4.2 onwards.
> Since commit 8996eafdcbad149ac0f772fb1649fbb75c482a6a
>
> Signed-off-by: Alexandru Ardelean
> ---
This should already have been fixed here:
www.spinics.net/lists/linux-crypto/msg19225.html
> @@ -2458,6 +2458,7 @@ static struct talitos_alg_template driver_algs[] = {
> { .type = CRYPTO_ALG_TYPE_AHASH,
> .alg.hash = {
> .halg.digestsize = MD5_DIGEST_SIZE,
> + .halg.statesize = sizeof(struct talitos_ahash_req_ctx),
although I'm not sure why these statesize assignments aren't being
done in talitos_alg_alloc() there either
Thanks,
Kim
IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended recipient,
please notify the sender immediately and do not disclose the contents to any
other person, use it for any purpose, or store or copy the information in any
medium. Thank you.
--
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
[PATCH 2/3] crypto: talitos - fix checkpatch warning
WARNING: kfree(NULL) is safe this check is probably not required + if (priv->chan[i].fifo) + kfree(priv->chan[i].fifo); Signed-off-by: Kim Phillips --- drivers/crypto/talitos.c |3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c index d4c1fc2..f22666c 100644 --- a/drivers/crypto/talitos.c +++ b/drivers/crypto/talitos.c @@ -2333,8 +2333,7 @@ static int talitos_remove(struct platform_device *ofdev) talitos_unregister_rng(dev); for (i = 0; i < priv->num_channels; i++) - if (priv->chan[i].fifo) - kfree(priv->chan[i].fifo); + kfree(priv->chan[i].fifo); kfree(priv->chan); -- 1.7.1 -- 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
[PATCH 1/3] crypto: talitos - fix warning: 'alg' may be used uninitialized in this function
drivers/crypto/talitos.c: In function 'talitos_probe': drivers/crypto/talitos.c:2363: warning: 'alg' may be used uninitialized in this function drivers/crypto/talitos.c:2363: note: 'alg' was declared here Signed-off-by: Kim Phillips --- drivers/crypto/talitos.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c index 4bcd825..d4c1fc2 100644 --- a/drivers/crypto/talitos.c +++ b/drivers/crypto/talitos.c @@ -2389,6 +2389,9 @@ static struct talitos_crypto_alg *talitos_alg_alloc(struct device *dev, DESC_HDR_MODE0_MDEU_SHA256; } break; + default: + dev_err(dev, "unknown algorithm type %d\n", t_alg->algt.type); + return ERR_PTR(-EINVAL); } alg->cra_module = THIS_MODULE; -- 1.7.1 -- 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
[PATCH 3/3] crypto: talitos - sparse check endian fixes
Signed-off-by: Kim Phillips
---
drivers/crypto/talitos.c | 23 +++
1 files changed, 11 insertions(+), 12 deletions(-)
diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index f22666c..b879c3f 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -161,7 +161,7 @@ struct talitos_private {
static void to_talitos_ptr(struct talitos_ptr *talitos_ptr, dma_addr_t
dma_addr)
{
talitos_ptr->ptr = cpu_to_be32(lower_32_bits(dma_addr));
- talitos_ptr->eptr = cpu_to_be32(upper_32_bits(dma_addr));
+ talitos_ptr->eptr = upper_32_bits(dma_addr);
}
/*
@@ -332,10 +332,9 @@ static int talitos_submit(struct device *dev, struct
talitos_desc *desc,
/* GO! */
wmb();
- out_be32(priv->reg + TALITOS_FF(ch),
-cpu_to_be32(upper_32_bits(request->dma_desc)));
+ out_be32(priv->reg + TALITOS_FF(ch), upper_32_bits(request->dma_desc));
out_be32(priv->reg + TALITOS_FF_LO(ch),
-cpu_to_be32(lower_32_bits(request->dma_desc)));
+lower_32_bits(request->dma_desc));
spin_unlock_irqrestore(&priv->chan[ch].head_lock, flags);
@@ -1751,14 +1750,14 @@ static int ahash_init_sha224_swinit(struct
ahash_request *areq)
ahash_init(areq);
req_ctx->swinit = 1;/* prevent h/w initting context with sha256 values*/
- req_ctx->hw_context[0] = cpu_to_be32(SHA224_H0);
- req_ctx->hw_context[1] = cpu_to_be32(SHA224_H1);
- req_ctx->hw_context[2] = cpu_to_be32(SHA224_H2);
- req_ctx->hw_context[3] = cpu_to_be32(SHA224_H3);
- req_ctx->hw_context[4] = cpu_to_be32(SHA224_H4);
- req_ctx->hw_context[5] = cpu_to_be32(SHA224_H5);
- req_ctx->hw_context[6] = cpu_to_be32(SHA224_H6);
- req_ctx->hw_context[7] = cpu_to_be32(SHA224_H7);
+ req_ctx->hw_context[0] = SHA224_H0;
+ req_ctx->hw_context[1] = SHA224_H1;
+ req_ctx->hw_context[2] = SHA224_H2;
+ req_ctx->hw_context[3] = SHA224_H3;
+ req_ctx->hw_context[4] = SHA224_H4;
+ req_ctx->hw_context[5] = SHA224_H5;
+ req_ctx->hw_context[6] = SHA224_H6;
+ req_ctx->hw_context[7] = SHA224_H7;
/* init 64-bit count */
req_ctx->hw_context[8] = 0;
--
1.7.1
--
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] picoxcell_crypto: add support for the picoxcell crypto engines
On Tue, 8 Feb 2011 15:56:16 +
Jamie Iles wrote:
> Picochip picoXcell devices have two crypto engines, one targeted
> at IPSEC offload and the other at WCDMA layer 2 ciphering.
>
> Cc: Herbert Xu
> Signed-off-by: Jamie Iles
> ---
nice driver ;). Have a couple of comments though.
> + help
> + This option enables support for the hardware offload engines in the
> + Picochip picoXcell SoC devices. Select this for IPSEC ESP offload
> + and for 3gpp Layer 2 ciphering support.
it'd be nice to mention what name the module will have.
> +#define SPACC_CRYPTO_AES_MAX_KEY_LEN 32
> +#define SPACC_CRYPTO_AES_IV_LEN 16
> +#define SPACC_CRYPTO_DES_IV_LEN 8
these are identical to algorithm-generic symbolic constants
AES_MAX_KEY_SIZE, [AD]ES_BLOCK_SIZE - why not use them instead?
> +struct spacc_generic_ctx;
this declaration isn't used prior to its definition, so it's not needed.
> +/* DDT format. This must match the hardware DDT format exactly. */
> +struct spacc_ddt {
> + u32 p, len;
type-consistency: p should be a dma_addr_t
> + /* AEAD specifc bits. */
specific
> +static inline struct spacc_ablk_ctx *
> +to_spacc_ablk_ctx(struct spacc_generic_ctx *ctx)
> +{
> + return ctx ? container_of(ctx, struct spacc_ablk_ctx, generic) : NULL;
> +}
> +
> +static inline struct spacc_aead_ctx *
> +to_spacc_aead_ctx(struct spacc_generic_ctx *ctx)
> +{
> + return ctx ? container_of(ctx, struct spacc_aead_ctx, generic) : NULL;
> +}
these aren't being used anywhere.
> +static inline struct spacc_alg *to_spacc_alg(struct crypto_alg *alg);
define it here - forward declarations should only be necessary when
dealing with circular dependencies.
> +/*
> + * Take a crypto request and scatterlists for the data and turn them into
> DDTs
> + * for passing to the crypto engines. This also DMA maps the data so that the
> + * crypto engines can DMA to/from them.
> + */
> +static struct spacc_ddt *spacc_sg_to_ddt(struct spacc_engine *engine,
> + struct scatterlist *payload,
> + unsigned nbytes,
> + enum dma_data_direction dir,
> + dma_addr_t *ddt_phys)
> +{
> + unsigned nents, mapped_ents;
> + struct scatterlist *cur;
> + struct spacc_ddt *ddt;
> + int i;
> +
> + nents = sg_count(payload, nbytes);
> + mapped_ents = dma_map_sg(engine->dev, payload, nents, dir);
> +
> + if (mapped_ents + 1 > MAX_DDT_LEN) {
> + dma_unmap_sg(engine->dev, payload, nents, dir);
> + return NULL;
> + }
> +
> + ddt = dma_pool_alloc(engine->req_pool, GFP_ATOMIC, ddt_phys);
> + if (ddt) {
> + for_each_sg(payload, cur, mapped_ents, i) {
> + ddt[i].p = sg_dma_address(cur);
> + ddt[i].len = sg_dma_len(cur);
> + }
> +
> + ddt[mapped_ents].p = 0;
> + ddt[mapped_ents].len = 0;
> + } else {
> + dma_unmap_sg(engine->dev, payload, nents, dir);
> + ddt = NULL;
> + }
> +
> + return ddt;
> +}
easier to read would be:
if (mapped_ents + 1 > MAX_DDT_LEN)
goto out;
ddt = dma_pool_alloc(engine->req_pool, GFP_ATOMIC, ddt_phys);
if (!ddt)
goto out;
for_each_sg(payload, cur, mapped_ents, i) {
ddt[i].p = sg_dma_address(cur);
ddt[i].len = sg_dma_len(cur);
}
ddt[mapped_ents].p = 0;
ddt[mapped_ents].len = 0;
return ddt;
out:
dma_unmap_sg(engine->dev, payload, nents, dir);
return NULL;
}
even more so by moving ddt_set() above it, and then using ddt_set() to
assign the p, len pairs.
> +static inline void ddt_set(struct spacc_ddt *ddt, unsigned long phys,
phys should be dma_addr_t
> +static int spacc_aead_make_ddts(struct spacc_req *req, u8 *giv)
> +{
> + struct aead_request *areq = container_of(req->req, struct aead_request,
> + base);
> + struct spacc_alg *alg = to_spacc_alg(req->req->tfm->__crt_alg);
> + struct spacc_engine *engine = req->engine;
> + struct spacc_ddt *src_ddt, *dst_ddt;
> + unsigned ivsize = alg->alg.cra_aead.ivsize;
no need to go through all those hoops to get to the ivsize - use helper
fns crypto_aead_reqtfm() and crypto_aead_ivsize(), as is done at the
callsite, or just pass it in from there.
> +static int spacc_aead_des_setkey(struct crypto_aead *aead, const u8 *key,
> + unsigned int len)
> +{
> + struct crypto_tfm *tfm = crypto_aead_tfm(aead);
> + struct spacc_aead_ctx *ctx = crypto_tfm_ctx(tfm);
> + int err = 0;
> + u32 tmp[DES_EXPKEY_WORDS];
> +
> + err = des_ekey(tmp, key);
> + if (unlikely(!err) &&
might want to change the name of the variable err here to something
like ret or is_weak so as
[PATCH 0/3] crypto: add support for the Freescale SEC4/CAAM
splitting and resending due to apparent 100KB message limit imposed by linux-crypto and devicetree-discuss mailing lists. No content has been changed from the original post that made it through linuxppc-dev's 400KB limit, available here: http://patchwork.ozlabs.org/patch/86051/ .../devicetree/bindings/crypto/fsl-sec4.txt| 409 + arch/powerpc/boot/dts/p4080ds.dts | 95 ++- drivers/crypto/Kconfig |2 + drivers/crypto/Makefile|1 + drivers/crypto/caam/Kconfig| 72 + drivers/crypto/caam/Makefile |8 + drivers/crypto/caam/caamalg.c | 1163 ++ drivers/crypto/caam/compat.h | 35 + drivers/crypto/caam/ctrl.c | 270 drivers/crypto/caam/desc.h | 1605 drivers/crypto/caam/desc_constr.h | 204 +++ drivers/crypto/caam/error.c| 248 +++ drivers/crypto/caam/error.h| 10 + drivers/crypto/caam/intern.h | 113 ++ drivers/crypto/caam/jr.c | 523 +++ drivers/crypto/caam/jr.h | 21 + drivers/crypto/caam/regs.h | 663 17 files changed, 5441 insertions(+), 1 deletions(-) create mode 100644 Documentation/devicetree/bindings/crypto/fsl-sec4.txt create mode 100644 drivers/crypto/caam/Kconfig create mode 100644 drivers/crypto/caam/Makefile create mode 100644 drivers/crypto/caam/caamalg.c create mode 100644 drivers/crypto/caam/compat.h create mode 100644 drivers/crypto/caam/ctrl.c create mode 100644 drivers/crypto/caam/desc.h create mode 100644 drivers/crypto/caam/desc_constr.h create mode 100644 drivers/crypto/caam/error.c create mode 100644 drivers/crypto/caam/error.h create mode 100644 drivers/crypto/caam/intern.h create mode 100644 drivers/crypto/caam/jr.c create mode 100644 drivers/crypto/caam/jr.h create mode 100644 drivers/crypto/caam/regs.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
[PATCH 1/3] crypto: add device tree bindings for the Freescale SEC4/CAAM
Add SEC4 device tree binding documentation and add a SEC4
device node to the P4080's dts.
Signed-off-by: Steve Cornelius
Signed-off-by: Kim Phillips
---
.../devicetree/bindings/crypto/fsl-sec4.txt| 409
arch/powerpc/boot/dts/p4080ds.dts | 95 +-
2 files changed, 503 insertions(+), 1 deletions(-)
create mode 100644 Documentation/devicetree/bindings/crypto/fsl-sec4.txt
diff --git a/Documentation/devicetree/bindings/crypto/fsl-sec4.txt
b/Documentation/devicetree/bindings/crypto/fsl-sec4.txt
new file mode 100644
index 000..fce16a8
--- /dev/null
+++ b/Documentation/devicetree/bindings/crypto/fsl-sec4.txt
@@ -0,0 +1,409 @@
+=
+SEC 4 Device Tree Binding
+Copyright (C) 2008-2011 Freescale Semiconductor Inc.
+
+ CONTENTS
+ -Overview
+ -SEC 4 Node
+ -Job Ring Node
+ -Run Time Integrity Check (RTIC) Node
+ -Run Time Integrity Check (RTIC) Memory Node
+ -Secure Non-Volatile Storage (SNVS) Node
+ -Full Example
+
+NOTE: the SEC 4 is also known as Freescale's Cryptographic Accelerator
+Accelerator and Assurance Module (CAAM).
+
+=
+Overview
+
+DESCRIPTION
+
+SEC 4 h/w can process requests from 2 types of sources.
+1. DPAA Queue Interface (HW interface between Queue Manager & SEC 4).
+2. Job Rings (HW interface between cores & SEC 4 registers).
+
+High Speed Data Path Configuration:
+
+HW interface between QM & SEC 4 and also BM & SEC 4, on DPAA-enabled parts
+such as the P4080. The number of simultaneous dequeues the QI can make is
+equal to the number of Descriptor Controller (DECO) engines in a particular
+SEC version. E.g., the SEC 4.0 in the P4080 has 5 DECOs and can thus
+dequeue from 5 subportals simultaneously.
+
+Job Ring Data Path Configuration:
+
+Each JR is located on a separate 4k page, they may (or may not) be made visible
+in the memory partition devoted to a particular core. The P4080 has 4 JRs, so
+up to 4 JRs can be configured; and all 4 JRs process requests in parallel.
+
+=
+P4080 SEC 4 Node
+
+Description
+
+Node defines the base address of the SEC 4 block.
+This block specifies the address range of all global
+configuration registers for the SEC 4 block. It
+also receives interrupts from the Run Time Integrity Check
+(RTIC) function within the SEC 4 block.
+
+PROPERTIES
+
+ - compatible
+ Usage: required
+ Value type:
+ Definition: Must include "fsl,p4080-sec4.0","fsl,sec-4.0"
+
+ - #address-cells
+ Usage: required
+ Value type:
+ Definition: A standard property. Defines the number of cells
+ for representing physical addresses in child nodes.
+
+ - #size-cells
+ Usage: required
+ Value type:
+ Definition: A standard property. Defines the number of cells
+ for representing the size of physical addresses in
+ child nodes.
+
+ - reg
+ Usage: required
+ Value type:
+ Definition: A standard property. Specifies the physical
+ address and length of the SEC4.0 configuration registers.
+ registers
+
+ - ranges
+ Usage: required
+ Value type:
+ Definition: A standard property. Specifies the physical address
+ range of the SEC 4.0 register space (-SNVS not included). A
+ triplet that includes the child address, parent address, &
+ length.
+
+ - interrupts
+ Usage: required
+ Value type:
+ Definition: Specifies the interrupts generated by this
+ device. The value of the interrupts property
+ consists of one interrupt specifier. The format
+ of the specifier is defined by the binding document
+ describing the node's interrupt parent.
+
+ - interrupt-parent
+ Usage: (required if interrupt property is defined)
+ Value type:
+ Definition: A single value that points
+ to the interrupt parent to which the child domain
+ is being mapped.
+
+ Note: All other standard properties (see the ePAPR) are allowed
+ but are optional.
+
+
+EXAMPLE
+ crypto@30 {
+ compatible = "fsl,p4080-sec4.0", "fsl,sec4.0";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ reg = <0x30 0x1>;
+ ranges = <0 0x30 0x1>;
+ interrupt-parent = <&mpic>;
+ interrupts = <92 2>;
+ };
+
+=
+P4080 Job Ring (JR) Node
+
+Child of the crypto node defines data processing interface to SEC 4
+across the peripheral bus for purposes of processing
+cryptographic descr
[PATCH] crypto: caam - standardize device tree naming convention to utilize '-vX.Y'
Help clarify that the number trailing in compatible nomenclature
is the version number of the device, i.e., change:
"fsl,p4080-sec4.0", "fsl,sec4.0";
to:
"fsl,p4080-sec-v4.0", "fsl,sec-v4.0";
Signed-off-by: Kim Phillips
Cc: Kumar Gala
Cc: Steve Cornelius
---
.../devicetree/bindings/crypto/fsl-sec4.txt| 68 ++--
arch/powerpc/boot/dts/p4080ds.dts | 41 ++--
drivers/crypto/caam/caamalg.c |4 +-
drivers/crypto/caam/ctrl.c |6 +-
4 files changed, 60 insertions(+), 59 deletions(-)
diff --git a/Documentation/devicetree/bindings/crypto/fsl-sec4.txt
b/Documentation/devicetree/bindings/crypto/fsl-sec4.txt
index fce16a8..568aa3c 100644
--- a/Documentation/devicetree/bindings/crypto/fsl-sec4.txt
+++ b/Documentation/devicetree/bindings/crypto/fsl-sec4.txt
@@ -53,7 +53,7 @@ PROPERTIES
- compatible
Usage: required
Value type:
- Definition: Must include "fsl,p4080-sec4.0","fsl,sec-4.0"
+ Definition: Must include "fsl,p4080-sec-v4.0","fsl,sec-v4.0"
- #address-cells
Usage: required
@@ -72,7 +72,7 @@ PROPERTIES
Usage: required
Value type:
Definition: A standard property. Specifies the physical
- address and length of the SEC4.0 configuration registers.
+ address and length of the SEC4 configuration registers.
registers
- ranges
@@ -105,7 +105,7 @@ PROPERTIES
EXAMPLE
crypto@30 {
- compatible = "fsl,p4080-sec4.0", "fsl,sec4.0";
+ compatible = "fsl,p4080-sec-v4.0", "fsl,sec-v4.0";
#address-cells = <1>;
#size-cells = <1>;
reg = <0x30 0x1>;
@@ -127,7 +127,7 @@ P4080 Job Ring (JR) Node
- compatible
Usage: required
Value type:
- Definition: Must include
"fsl,p4080-sec4.0-job-ring","fsl,sec4.0-job-ring"
+ Definition: Must include
"fsl,p4080-sec-v4.0-job-ring","fsl,sec-v4.0-job-ring"
- reg
Usage: required
@@ -163,8 +163,8 @@ P4080 Job Ring (JR) Node
EXAMPLE
jr@1000 {
- compatible = "fsl,p4080-sec4.0-job-ring",
-"fsl,sec4.0-job-ring";
+ compatible = "fsl,p4080-sec-v4.0-job-ring",
+"fsl,sec-v4.0-job-ring";
reg = <0x1000 0x1000>;
fsl,liodn = <0x081>;
interrupt-parent = <&mpic>;
@@ -186,7 +186,7 @@ P4080 Run Time Integrity Check (RTIC) Node
- compatible
Usage: required
Value type:
- Definition: Must include "fsl,p4080-sec4.0-rtic","fsl,sec4.0-rtic".
+ Definition: Must include "fsl,p4080-sec-v4.0-rtic","fsl,sec-v4.0-rtic".
- #address-cells
Usage: required
@@ -219,8 +219,8 @@ P4080 Run Time Integrity Check (RTIC) Node
EXAMPLE
rtic@6000 {
- compatible = "fsl,p4080-sec4.0-rtic",
-"fsl,sec4.0-rtic";
+ compatible = "fsl,p4080-sec-v4.0-rtic",
+"fsl,sec-v4.0-rtic";
#address-cells = <1>;
#size-cells = <1>;
reg = <0x6000 0x100>;
@@ -238,7 +238,7 @@ P4080 Run Time Integrity Check (RTIC) Memory Node
- compatible
Usage: required
Value type:
- Definition: Must include
"fsl,p4080-sec4.0-rtic-memory","fsl,sec4.0-rtic-memory".
+ Definition: Must include
"fsl,p4080-sec-v4.0-rtic-memory","fsl,sec-v4.0-rtic-memory".
- reg
Usage: required
@@ -270,8 +270,8 @@ P4080 Run Time Integrity Check (RTIC) Memory Node
EXAMPLE
rtic-a@0 {
- compatible = "fsl,p4080-sec4.0-rtic-memory",
-"fsl,sec4.0-rtic-memory";
+ compatible = "fsl,p4080-sec-v4.0-rtic-memory",
+"fsl,sec-v4.0-rtic-memory";
reg = <0x00 0x20 0x100 0x80>;
fsl,liodn = <0x03c>;
fsl,rtic-region = <0x12345678 0x12345678 0x12345678>;
@@ -288,7 +288,7 @@ P4080 Secure Non-Volatile Storage (SNVS) Node
- compatible
Usage: required
Value type:
- Definition: Must include "fsl,p4080-sec4.0-mon", "fsl,sec4.0-mon".
+ Definition: Must include "fsl,p4080-sec-v4.0-mon", "fsl,sec-v4.0-mon".
- reg
Usage: required
@@ -315,7 +315,7 @@ P4080 Secure Non-Volatile Storage (SNVS) Node
EXAMPLE
sec_mon@314000 {
- compatible = &quo
[PATCH] crypto: caam - de-CHIP-ify device tree compatibles
- all the integration parameters have been captured by the binding.
- the block name really uniquely identifies this hardware.
Some advocate putting SoC names everywhere in case software needs
to work around some chip-specific bug, but more precise SoC
information already exists in SVR, and board information already
exists in the top-level device tree node.
Note that sometimes the SoC name is a worse identifier than the
block version, as the block version can change between revisions
of the same SoC.
As a matter of historical reference, neither SEC versions 2.x
nor 3.x (driven by talitos) ever needed CHIP references.
Signed-off-by: Kim Phillips
Cc: Kumar Gala
Cc: Scott Wood
---
.../devicetree/bindings/crypto/fsl-sec4.txt| 64
arch/powerpc/boot/dts/p4080ds.dts | 32 ---
2 files changed, 37 insertions(+), 59 deletions(-)
diff --git a/Documentation/devicetree/bindings/crypto/fsl-sec4.txt
b/Documentation/devicetree/bindings/crypto/fsl-sec4.txt
index 568aa3c..bf57ecd 100644
--- a/Documentation/devicetree/bindings/crypto/fsl-sec4.txt
+++ b/Documentation/devicetree/bindings/crypto/fsl-sec4.txt
@@ -38,7 +38,7 @@ in the memory partition devoted to a particular core. The
P4080 has 4 JRs, so
up to 4 JRs can be configured; and all 4 JRs process requests in parallel.
=
-P4080 SEC 4 Node
+SEC 4 Node
Description
@@ -53,7 +53,7 @@ PROPERTIES
- compatible
Usage: required
Value type:
- Definition: Must include "fsl,p4080-sec-v4.0","fsl,sec-v4.0"
+ Definition: Must include "fsl,sec-v4.0"
- #address-cells
Usage: required
@@ -105,7 +105,7 @@ PROPERTIES
EXAMPLE
crypto@30 {
- compatible = "fsl,p4080-sec-v4.0", "fsl,sec-v4.0";
+ compatible = "fsl,sec-v4.0";
#address-cells = <1>;
#size-cells = <1>;
reg = <0x30 0x1>;
@@ -115,7 +115,7 @@ EXAMPLE
};
=
-P4080 Job Ring (JR) Node
+Job Ring (JR) Node
Child of the crypto node defines data processing interface to SEC 4
across the peripheral bus for purposes of processing
@@ -127,7 +127,7 @@ P4080 Job Ring (JR) Node
- compatible
Usage: required
Value type:
- Definition: Must include
"fsl,p4080-sec-v4.0-job-ring","fsl,sec-v4.0-job-ring"
+ Definition: Must include "fsl,sec-v4.0-job-ring"
- reg
Usage: required
@@ -163,8 +163,7 @@ P4080 Job Ring (JR) Node
EXAMPLE
jr@1000 {
- compatible = "fsl,p4080-sec-v4.0-job-ring",
-"fsl,sec-v4.0-job-ring";
+ compatible = "fsl,sec-v4.0-job-ring";
reg = <0x1000 0x1000>;
fsl,liodn = <0x081>;
interrupt-parent = <&mpic>;
@@ -173,7 +172,7 @@ EXAMPLE
=
-P4080 Run Time Integrity Check (RTIC) Node
+Run Time Integrity Check (RTIC) Node
Child node of the crypto node. Defines a register space that
contains up to 5 sets of addresses and their lengths (sizes) that
@@ -186,7 +185,7 @@ P4080 Run Time Integrity Check (RTIC) Node
- compatible
Usage: required
Value type:
- Definition: Must include "fsl,p4080-sec-v4.0-rtic","fsl,sec-v4.0-rtic".
+ Definition: Must include "fsl,sec-v4.0-rtic".
- #address-cells
Usage: required
@@ -219,8 +218,7 @@ P4080 Run Time Integrity Check (RTIC) Node
EXAMPLE
rtic@6000 {
- compatible = "fsl,p4080-sec-v4.0-rtic",
-"fsl,sec-v4.0-rtic";
+ compatible = "fsl,sec-v4.0-rtic";
#address-cells = <1>;
#size-cells = <1>;
reg = <0x6000 0x100>;
@@ -228,7 +226,7 @@ EXAMPLE
};
=
-P4080 Run Time Integrity Check (RTIC) Memory Node
+Run Time Integrity Check (RTIC) Memory Node
A child node that defines individual RTIC memory regions that are used to
perform run-time integrity check of memory areas that should not modified.
The node defines a register that contains the memory address &
@@ -238,7 +236,7 @@ P4080 Run Time Integrity Check (RTIC) Memory Node
- compatible
Usage: required
Value type:
- Definition: Must include
"fsl,p4080-sec-v4.0-rtic-memory","fsl,sec-v4.0-rtic-memory".
+ Definition: Must include "fsl,sec-v4.0-rtic-memory".
- reg
Usage: required
@@ -270,15 +268,14 @@ P4080 Run Time Integrity Che
Re: [patch] crypto: caam - ARRAY_SIZE() vs sizeof()
On Tue, 15 Mar 2011 09:59:55 +0300 Dan Carpenter wrote: > ARRAY_SIZE() was intended here instead of sizeof(). sizeof() is four > times larger than ARRAY_SIZE(). outstr is normally 256 chars so > printing garbage to it could overfill the buffer and corrupt memory. > > Signed-off-by: Dan Carpenter Acked-by: Kim Phillips Thanks Dan, Kim -- 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] crypto: caam - dereferencing ERR_PTR on allocation failure
On Tue, 15 Mar 2011 09:57:47 +0300 Dan Carpenter wrote: > t_alg is an ERR_PTR here so we can't dereference it. > > Signed-off-by: Dan Carpenter > --- Acked-by: Kim Phillips Thanks, Kim -- 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 2/6] drivers/crypto/caam/caamalg.c: introduce missing kfree
On Fri, 1 Apr 2011 16:23:43 +0200 Julia Lawall wrote: > Error handling code following a kmalloc should free the allocated data. > Signed-off-by: Julia Lawall > --- Acked-by: Kim Phillips Kim -- 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: aead: driver side documentation
On Mon, 4 Apr 2011 19:03:37 +0200 Phil Sutter wrote: > I would like to enhance drivers/crypto/mv_cesa.c by an AEAD algorithm > (at least authenc(hmac(sha1),cbc(aes))), since the driver is able to do > both operations in one go. > > Unfortunately, I have found little information about this task in > Documentation/ or the web. Am I missing something? It would be really > great if you could point me to the right direction here. use existing drivers for guidance. The following drivers implement those types of algorithms: drivers/crypto/caam/caamalg.c* drivers/crypto/ixp4xx_crypto.c drivers/crypto/picoxcell_crypto.c drivers/crypto/talitos.c Kim * only available in Herbert's cryptodev-2.6 tree -- 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: aead: driver side documentation
On Tue, 5 Apr 2011 15:04:35 +0200
Phil Sutter wrote:
> Hi,
>
> On Mon, Apr 04, 2011 at 08:35:43PM -0500, Kim Phillips wrote:
> > On Mon, 4 Apr 2011 19:03:37 +0200
> > Phil Sutter wrote:
> >
> > > I would like to enhance drivers/crypto/mv_cesa.c by an AEAD algorithm
> > > (at least authenc(hmac(sha1),cbc(aes))), since the driver is able to do
> > > both operations in one go.
> > >
> > > Unfortunately, I have found little information about this task in
> > > Documentation/ or the web. Am I missing something? It would be really
> > > great if you could point me to the right direction here.
> >
> > use existing drivers for guidance. The following drivers implement
> > those types of algorithms:
>
> Thanks for the hint, although I've already found the "sample code". ;)
> Was rather looking for something telling me what is crucial and what
> options there are. Concrete code tends to just show the solution of a
> specific problem. (My five cents to the question why code is often, but
> seldomly good documentation.)
>
> Grepping reveals IPSec (i.e., esp{4,6}.c) as the only user of AEAD so
> far. Is this correct?
yep.
I started to add test vectors from [1] to crypto/testmgr.c, but it
required that drivers not assume associated data, iv, and cipher data
were contiguous in memory, and since some of them do, I don't know if
such a contribution would be acceptable upstream.
then there's the rtnetlink dependencies I mention in [2] that
also make drivers fail AEAD testmgr tests in their setkey()
implementations, IIRC, because testmgr keys aren't RTA_OK.
Kim
[1] http://grouper.ieee.org/groups/1619/email/msg01966.html
[2] http://www.mail-archive.com/linux-crypto@vger.kernel.org/msg05533.html
--
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: aead: driver side documentation
On Fri, 8 Apr 2011 08:55:33 +0800 Herbert Xu wrote: > Kim Phillips wrote: > > > > I started to add test vectors from [1] to crypto/testmgr.c, but it > > required that drivers not assume associated data, iv, and cipher data > > were contiguous in memory, and since some of them do, I don't know if > > such a contribution would be acceptable upstream. > > Such drivers are broken and should be fixed. ok. > > then there's the rtnetlink dependencies I mention in [2] that > > also make drivers fail AEAD testmgr tests in their setkey() > > implementations, IIRC, because testmgr keys aren't RTA_OK. > > Can you elaborate on this problem? the following conditions in a driver setkey fail: if (!RTA_OK(rta, keylen)) goto badkey; if (rta->rta_type != CRYPTO_AUTHENC_KEYA_PARAM) goto badkey; because testmgr keys are plain strings, not rtattr structs. Kim -- 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
[PATCH 1/4] crypto: caam - handle interrupt lines shared across rings
- add IRQF_SHARED to request_irq flags to support parts such as
the p1023 that has one IRQ line per couple of rings.
- resetting a job ring triggers an interrupt, so move request_irq
prior to jr_reset to avoid 'got IRQ but nobody cared' messages.
- disable IRQs in h/w to avoid contention between reset and
interrupt status
- delete invalid comment - if there were incomplete jobs,
module would be in use, preventing an unload.
Signed-off-by: Kim Phillips
---
this, and the remaining patches in this series, tested on p1023, p3041,
p4080, and 32- and 64-bit p5020.
drivers/crypto/caam/jr.c | 46 --
1 files changed, 20 insertions(+), 26 deletions(-)
diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
index 68cb9af..340fa32 100644
--- a/drivers/crypto/caam/jr.c
+++ b/drivers/crypto/caam/jr.c
@@ -292,10 +292,10 @@ static int caam_reset_hw_jr(struct device *dev)
unsigned int timeout = 10;
/*
-* FIXME: disabling IRQs here inhibits proper job completion
-* and error propagation
+* mask interrupts since we are going to poll
+* for reset completion status
*/
- disable_irq(jrp->irq);
+ setbits32(&jrp->rregs->rconfig_lo, JRCFG_IMSK);
/* initiate flush (required prior to reset) */
wr_reg32(&jrp->rregs->jrcommand, JRCR_RESET);
@@ -320,7 +320,8 @@ static int caam_reset_hw_jr(struct device *dev)
return -EIO;
}
- enable_irq(jrp->irq);
+ /* unmask interrupts */
+ clrbits32(&jrp->rregs->rconfig_lo, JRCFG_IMSK);
return 0;
}
@@ -336,6 +337,21 @@ static int caam_jr_init(struct device *dev)
jrp = dev_get_drvdata(dev);
+ /* Connect job ring interrupt handler. */
+ for_each_possible_cpu(i)
+ tasklet_init(&jrp->irqtask[i], caam_jr_dequeue,
+(unsigned long)dev);
+
+ error = request_irq(jrp->irq, caam_jr_interrupt, IRQF_SHARED,
+ "caam-jobr", dev);
+ if (error) {
+ dev_err(dev, "can't connect JobR %d interrupt (%d)\n",
+ jrp->ridx, jrp->irq);
+ irq_dispose_mapping(jrp->irq);
+ jrp->irq = 0;
+ return -EINVAL;
+ }
+
error = caam_reset_hw_jr(dev);
if (error)
return error;
@@ -404,28 +420,6 @@ static int caam_jr_init(struct device *dev)
(JOBR_INTC_COUNT_THLD << JRCFG_ICDCT_SHIFT) |
(JOBR_INTC_TIME_THLD << JRCFG_ICTT_SHIFT));
- /* Connect job ring interrupt handler. */
- for_each_possible_cpu(i)
- tasklet_init(&jrp->irqtask[i], caam_jr_dequeue,
-(unsigned long)dev);
-
- error = request_irq(jrp->irq, caam_jr_interrupt, 0,
- "caam-jobr", dev);
- if (error) {
- dev_err(dev, "can't connect JobR %d interrupt (%d)\n",
- jrp->ridx, jrp->irq);
- irq_dispose_mapping(jrp->irq);
- jrp->irq = 0;
- dma_unmap_single(dev, inpbusaddr, sizeof(u32 *) * JOBR_DEPTH,
-DMA_BIDIRECTIONAL);
- dma_unmap_single(dev, outbusaddr, sizeof(u32 *) * JOBR_DEPTH,
-DMA_BIDIRECTIONAL);
- kfree(jrp->inpring);
- kfree(jrp->outring);
- kfree(jrp->entinfo);
- return -EINVAL;
- }
-
jrp->assign = JOBR_UNASSIGNED;
return 0;
}
--
1.7.4.3.dirty
--
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
[PATCH 2/4] crypto: caam - fix queue interface detection
The presence of a h/w Queue Interface would fail due to this
cut-n-paste snafu.
Signed-off-by: Kim Phillips
---
drivers/crypto/caam/regs.h |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
index d063a26..aee394e 100644
--- a/drivers/crypto/caam/regs.h
+++ b/drivers/crypto/caam/regs.h
@@ -131,7 +131,7 @@ struct caam_perfmon {
/* CAAM Hardware Instantiation Parameters fa0-fbf */
u64 cha_rev;/* CRNR - CHA Revision Number */
#define CTPR_QI_SHIFT 57
-#define CTPR_QI_MASK (0x1ull << CHA_NUM_DECONUM_SHIFT)
+#define CTPR_QI_MASK (0x1ull << CTPR_QI_SHIFT)
u64 comp_parms; /* CTPR - Compile Parameters Register */
u64 rsvd1[2];
--
1.7.4.3.dirty
--
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
[PATCH 4/4] crypto: caam - remove duplicate dev_err
keep the hex error value reporting version (a) to be consistent
with decrypt_done(), and (b) to keep our hardware guys happy.
Signed-off-by: Kim Phillips
---
drivers/crypto/caam/caamalg.c |1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index 8eb84d3..245cfe4 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -418,7 +418,6 @@ static void ipsec_esp_encrypt_done(struct device *jrdev,
u32 *desc, u32 err,
if (err) {
char tmp[256];
- dev_err(jrdev, "%s\n", caam_jr_strstatus(tmp, err));
dev_err(jrdev, "%08x: %s\n", err, caam_jr_strstatus(tmp, err));
}
--
1.7.4.3.dirty
--
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
[PATCH 3/4] crypto: caam - remove WAIT-FOR-COMPLETIONs from givencrypt descriptor
remains from descriptor debugging - not required for normal operation. Signed-off-by: Kim Phillips --- drivers/crypto/caam/caamalg.c |6 ++ 1 files changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c index 20e1215..8eb84d3 100644 --- a/drivers/crypto/caam/caamalg.c +++ b/drivers/crypto/caam/caamalg.c @@ -828,15 +828,13 @@ static int aead_authenc_givencrypt(struct aead_givcrypt_request *req) append_cmd(desc, CMD_LOAD | DISABLE_AUTO_INFO_FIFO); /* MOVE DECO Alignment -> C1 Context 16 bytes */ - append_move(desc, MOVE_WAITCOMP | MOVE_SRC_INFIFO | - MOVE_DEST_CLASS1CTX | ivsize); + append_move(desc, MOVE_SRC_INFIFO | MOVE_DEST_CLASS1CTX | ivsize); /* re-enable info fifo entries */ append_cmd(desc, CMD_LOAD | ENABLE_AUTO_INFO_FIFO); /* MOVE C1 Context -> OFIFO 16 bytes */ - append_move(desc, MOVE_WAITCOMP | MOVE_SRC_CLASS1CTX | - MOVE_DEST_OUTFIFO | ivsize); + append_move(desc, MOVE_SRC_CLASS1CTX | MOVE_DEST_OUTFIFO | ivsize); append_fifo_store(desc, iv_dma, ivsize, FIFOST_TYPE_MESSAGE_DATA); -- 1.7.4.3.dirty -- 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 1/4] crypto: caam - handle interrupt lines shared across rings
On Fri, 15 Apr 2011 17:50:49 +0800 Herbert Xu wrote: > On Mon, Apr 11, 2011 at 07:15:16PM -0500, Kim Phillips wrote: > > - add IRQF_SHARED to request_irq flags to support parts such as > > the p1023 that has one IRQ line per couple of rings. > > > > - resetting a job ring triggers an interrupt, so move request_irq > > prior to jr_reset to avoid 'got IRQ but nobody cared' messages. > > > > - disable IRQs in h/w to avoid contention between reset and > > interrupt status > > > > - delete invalid comment - if there were incomplete jobs, > > module would be in use, preventing an unload. > > > > Signed-off-by: Kim Phillips > > --- > > this, and the remaining patches in this series, tested on p1023, p3041, > > p4080, and 32- and 64-bit p5020. > > All four patches applied. Thanks Kim! I don't see them in cryptodev-2.6 - did you forget to push them? Thanks, Kim -- 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
[PATCH 1/2] crypto: caam - remove unused keylen from session context
Signed-off-by: Kim Phillips
---
drivers/crypto/caam/caamalg.c |2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index 97d6dcc..719ad06 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -82,7 +82,6 @@ struct caam_ctx {
u32 alg_op;
u8 *key;
dma_addr_t key_phys;
- unsigned int keylen;
unsigned int enckeylen;
unsigned int authkeylen;
unsigned int split_key_len;
@@ -330,7 +329,6 @@ static int aead_authenc_setkey(struct crypto_aead *aead,
ctx->split_key_pad_len + enckeylen, 1);
#endif
- ctx->keylen = keylen;
ctx->enckeylen = enckeylen;
ctx->authkeylen = authkeylen;
--
1.7.5
--
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
[PATCH 2/2] crypto: caam - fix printk recursion for long error texts
during recent descriptor development, an Invalid Sequence Command
error triggered a:
BUG: recent printk recursion!
due to insufficient memory allocated for the error text.
The Invalid Sequence Command error text is the longest.
The length of the maximum error string is computed as
the sum of:
"DECO: ": 6
"jump tgt desc idx 255: ": 23
Invalid Sequence Command text: 272
zero termination character: 1
i.e, 302 characters.
Define this maximum error string length in error.h and fix
caam_jr_strstatus callsites.
Signed-off-by: Kim Phillips
---
drivers/crypto/caam/caamalg.c |6 +++---
drivers/crypto/caam/error.h |1 +
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index 719ad06..449f5b6 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -113,7 +113,7 @@ static void split_key_done(struct device *dev, u32 *desc,
u32 err,
dev_err(dev, "%s %d: err 0x%x\n", __func__, __LINE__, err);
#endif
if (err) {
- char tmp[256];
+ char tmp[CAAM_ERROR_STR_MAX];
dev_err(dev, "%08x: %s\n", err, caam_jr_strstatus(tmp, err));
}
@@ -414,7 +414,7 @@ static void ipsec_esp_encrypt_done(struct device *jrdev,
u32 *desc, u32 err,
offsetof(struct ipsec_esp_edesc, hw_desc));
if (err) {
- char tmp[256];
+ char tmp[CAAM_ERROR_STR_MAX];
dev_err(jrdev, "%08x: %s\n", err, caam_jr_strstatus(tmp, err));
}
@@ -454,7 +454,7 @@ static void ipsec_esp_decrypt_done(struct device *jrdev,
u32 *desc, u32 err,
offsetof(struct ipsec_esp_edesc, hw_desc));
if (err) {
- char tmp[256];
+ char tmp[CAAM_ERROR_STR_MAX];
dev_err(jrdev, "%08x: %s\n", err, caam_jr_strstatus(tmp, err));
}
diff --git a/drivers/crypto/caam/error.h b/drivers/crypto/caam/error.h
index 067afc1..02c7baa 100644
--- a/drivers/crypto/caam/error.h
+++ b/drivers/crypto/caam/error.h
@@ -6,5 +6,6 @@
#ifndef CAAM_ERROR_H
#define CAAM_ERROR_H
+#define CAAM_ERROR_STR_MAX 302
extern char *caam_jr_strstatus(char *outstr, u32 status);
#endif /* CAAM_ERROR_H */
--
1.7.5
--
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
[PATCH 1/4] crypto: caam - platform_bus_type migration
this fixes a build error since cryptodev-2.6 got rebased
to include commit d714d1979d7b4df7e2c127407f4014ce71f73cd0
"dt: eliminate of_platform_driver shim code".
Signed-off-by: Kim Phillips
---
drivers/crypto/caam/ctrl.c |9 -
1 files changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index 59aae4e..9009713 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -44,8 +44,7 @@ static int caam_remove(struct platform_device *pdev)
}
/* Probe routine for CAAM top (controller) level */
-static int caam_probe(struct platform_device *pdev,
- const struct of_device_id *devmatch)
+static int caam_probe(struct platform_device *pdev)
{
int d, ring, rspec;
struct device *dev;
@@ -242,7 +241,7 @@ static struct of_device_id caam_match[] = {
};
MODULE_DEVICE_TABLE(of, caam_match);
-static struct of_platform_driver caam_driver = {
+static struct platform_driver caam_driver = {
.driver = {
.name = "caam",
.owner = THIS_MODULE,
@@ -254,12 +253,12 @@ static struct of_platform_driver caam_driver = {
static int __init caam_base_init(void)
{
- return of_register_platform_driver(&caam_driver);
+ return platform_driver_register(&caam_driver);
}
static void __exit caam_base_exit(void)
{
- return of_unregister_platform_driver(&caam_driver);
+ return platform_driver_unregister(&caam_driver);
}
module_init(caam_base_init);
--
1.7.5.1
--
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
[PATCH 2/4] crypto: caam - fix decryption shared vs. non-shared key setting
Key sharing is enabled by default in the shared descriptor.
Using CBC decrypt, AES has to alter the key in order to decrypt.
During high traffic decryption rates, i.e, when sharing starts to
take place, we need to use a different OPERATION option to tell AES
that the key was already altered by the PRIOR descriptor - we need
the following kind of logic:
if ( shared )
operation where AES uses decryption key (DK=1)
else
operation where AES uses encryption key (DK=0)
this patch implements this logic using a conditional and
a non-conditional local jump within the decryption job
descriptor.
Signed-off-by: Kim Phillips
---
drivers/crypto/caam/caamalg.c | 26 ++
1 files changed, 22 insertions(+), 4 deletions(-)
diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index b97575e..4c69ba7 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -571,9 +571,27 @@ static int ipsec_esp(struct ipsec_esp_edesc *edesc, struct
aead_request *areq,
/* copy iv from cipher/class1 input context to class2 infifo */
append_move(desc, MOVE_SRC_CLASS1CTX | MOVE_DEST_CLASS2INFIFO | ivsize);
- /* start class 1 (cipher) operation */
- append_operation(desc, ctx->class1_alg_type | OP_ALG_AS_INITFINAL |
-encrypt);
+ if (!encrypt) {
+ u32 *jump_cmd, *uncond_jump_cmd;
+
+ /* JUMP if shared */
+ jump_cmd = append_jump(desc, JUMP_TEST_ALL | JUMP_COND_SHRD);
+
+ /* start class 1 (cipher) operation, non-shared version */
+ append_operation(desc, ctx->class1_alg_type |
+OP_ALG_AS_INITFINAL);
+
+ uncond_jump_cmd = append_jump(desc, 0);
+
+ set_jump_tgt_here(desc, jump_cmd);
+
+ /* start class 1 (cipher) operation, shared version */
+ append_operation(desc, ctx->class1_alg_type |
+OP_ALG_AS_INITFINAL | OP_ALG_AAI_DK);
+ set_jump_tgt_here(desc, uncond_jump_cmd);
+ } else
+ append_operation(desc, ctx->class1_alg_type |
+OP_ALG_AS_INITFINAL | encrypt);
/* load payload & instruct to class2 to snoop class 1 if encrypting */
options = 0;
@@ -762,7 +780,7 @@ static int aead_authenc_decrypt(struct aead_request *req)
req->cryptlen -= ctx->authsize;
/* allocate extended descriptor */
- edesc = ipsec_esp_edesc_alloc(req, 21 * sizeof(u32));
+ edesc = ipsec_esp_edesc_alloc(req, 24 * sizeof(u32));
if (IS_ERR(edesc))
return PTR_ERR(edesc);
--
1.7.5.1
--
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
[PATCH 3/4] crypto: caam - remove unused authkeylen from caam_ctx
Signed-off-by: Kim Phillips
---
drivers/crypto/caam/caamalg.c |2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index 4c69ba7..672abf3 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -83,7 +83,6 @@ struct caam_ctx {
u8 *key;
dma_addr_t key_phys;
unsigned int enckeylen;
- unsigned int authkeylen;
unsigned int split_key_len;
unsigned int split_key_pad_len;
unsigned int authsize;
@@ -330,7 +329,6 @@ static int aead_authenc_setkey(struct crypto_aead *aead,
#endif
ctx->enckeylen = enckeylen;
- ctx->authkeylen = authkeylen;
ret = build_sh_desc_ipsec(ctx);
if (ret) {
--
1.7.5.1
--
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
[PATCH 4/4] crypto: caam - add support for sha512 variants of existing AEAD algorithms
In doing so, sha512 sized keys would not fit with the current
descriptor inlining mechanism, so we now calculate whether keys
should be referenced instead by pointers in the shared descriptor.
also, use symbols for descriptor text lengths, and, ahem, unmap and
free key i/o memory in cra_exit.
Signed-off-by: Kim Phillips
---
drivers/crypto/caam/caamalg.c | 119 +
drivers/crypto/caam/desc_constr.h |1 +
2 files changed, 107 insertions(+), 13 deletions(-)
diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index 672abf3..d0e65d6 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -61,6 +61,12 @@
/* max IV is max of AES_BLOCK_SIZE, DES3_EDE_BLOCK_SIZE */
#define CAAM_MAX_IV_LENGTH 16
+/* length of descriptors text */
+#define DESC_AEAD_SHARED_TEXT_LEN 4
+#define DESC_AEAD_ENCRYPT_TEXT_LEN 21
+#define DESC_AEAD_DECRYPT_TEXT_LEN 24
+#define DESC_AEAD_GIVENCRYPT_TEXT_LEN 27
+
#ifdef DEBUG
/* for print_hex_dumps with line references */
#define xstr(s) str(s)
@@ -219,10 +225,22 @@ static int build_sh_desc_ipsec(struct caam_ctx *ctx)
struct device *jrdev = ctx->jrdev;
u32 *sh_desc;
u32 *jump_cmd;
+ bool keys_fit_inline = 0;
+
+ /*
+* largest Job Descriptor and its Shared Descriptor
+* must both fit into the 64-word Descriptor h/w Buffer
+*/
+ if ((DESC_AEAD_GIVENCRYPT_TEXT_LEN +
+DESC_AEAD_SHARED_TEXT_LEN) * CAAM_CMD_SZ +
+ ctx->split_key_pad_len + ctx->enckeylen <= CAAM_DESC_BYTES_MAX)
+ keys_fit_inline = 1;
/* build shared descriptor for this session */
- sh_desc = kmalloc(CAAM_CMD_SZ * 4 + ctx->split_key_pad_len +
- ctx->enckeylen, GFP_DMA | GFP_KERNEL);
+ sh_desc = kmalloc(CAAM_CMD_SZ * DESC_AEAD_SHARED_TEXT_LEN +
+ keys_fit_inline ?
+ ctx->split_key_pad_len + ctx->enckeylen :
+ CAAM_PTR_SZ * 2, GFP_DMA | GFP_KERNEL);
if (!sh_desc) {
dev_err(jrdev, "could not allocate shared descriptor\n");
return -ENOMEM;
@@ -233,14 +251,23 @@ static int build_sh_desc_ipsec(struct caam_ctx *ctx)
jump_cmd = append_jump(sh_desc, CLASS_BOTH | JUMP_TEST_ALL |
JUMP_COND_SHRD | JUMP_COND_SELF);
- /* process keys, starting with class 2/authentication */
- append_key_as_imm(sh_desc, ctx->key, ctx->split_key_pad_len,
- ctx->split_key_len,
- CLASS_2 | KEY_DEST_MDHA_SPLIT | KEY_ENC);
-
- append_key_as_imm(sh_desc, (void *)ctx->key + ctx->split_key_pad_len,
- ctx->enckeylen, ctx->enckeylen,
- CLASS_1 | KEY_DEST_CLASS_REG);
+ /*
+* process keys, starting with class 2/authentication.
+*/
+ if (keys_fit_inline) {
+ append_key_as_imm(sh_desc, ctx->key, ctx->split_key_pad_len,
+ ctx->split_key_len,
+ CLASS_2 | KEY_DEST_MDHA_SPLIT | KEY_ENC);
+
+ append_key_as_imm(sh_desc, (void *)ctx->key +
+ ctx->split_key_pad_len, ctx->enckeylen,
+ ctx->enckeylen, CLASS_1 | KEY_DEST_CLASS_REG);
+ } else {
+ append_key(sh_desc, ctx->key_phys, ctx->split_key_len, CLASS_2 |
+ KEY_DEST_MDHA_SPLIT | KEY_ENC);
+ append_key(sh_desc, ctx->key_phys + ctx->split_key_pad_len,
+ ctx->enckeylen, CLASS_1 | KEY_DEST_CLASS_REG);
+ }
/* update jump cmd now that we are at the jump target */
set_jump_tgt_here(sh_desc, jump_cmd);
@@ -746,7 +773,8 @@ static int aead_authenc_encrypt(struct aead_request *areq)
dma_addr_t iv_dma;
/* allocate extended descriptor */
- edesc = ipsec_esp_edesc_alloc(areq, 21 * sizeof(u32));
+ edesc = ipsec_esp_edesc_alloc(areq, DESC_AEAD_ENCRYPT_TEXT_LEN *
+ CAAM_CMD_SZ);
if (IS_ERR(edesc))
return PTR_ERR(edesc);
@@ -778,7 +806,8 @@ static int aead_authenc_decrypt(struct aead_request *req)
req->cryptlen -= ctx->authsize;
/* allocate extended descriptor */
- edesc = ipsec_esp_edesc_alloc(req, 24 * sizeof(u32));
+ edesc = ipsec_esp_edesc_alloc(req, DESC_AEAD_DECRYPT_TEXT_LEN *
+ CAAM_CMD_SZ);
if (IS_ERR(edesc))
return PTR_ERR(edesc);
@@ -813,7 +842,8 @@ static int aead_authenc_givencrypt(struct
aead_givcrypt_request *req)
debug("%s: giv %p\n", __func__, req->giv);
/* allocate extended descripto
[PATCH] crypto: caam - fix operator precedence in shared descriptor allocation
setkey allocates 16 bytes (CAAM_CMD_SZ *
DESC_AEAD_SHARED_TEXT_LEN) shy of what is needed to
store the shared descriptor, resulting in memory
corruption. Fix this.
Signed-off-by: Kim Phillips
---
Bug introduced in commit 4427b1b - "crypto: caam - add support for
sha512 variants of existing AEAD algorithms".
drivers/crypto/caam/caamalg.c |6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index d0e65d6..676d957 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -238,9 +238,9 @@ static int build_sh_desc_ipsec(struct caam_ctx *ctx)
/* build shared descriptor for this session */
sh_desc = kmalloc(CAAM_CMD_SZ * DESC_AEAD_SHARED_TEXT_LEN +
- keys_fit_inline ?
- ctx->split_key_pad_len + ctx->enckeylen :
- CAAM_PTR_SZ * 2, GFP_DMA | GFP_KERNEL);
+ (keys_fit_inline ?
+ ctx->split_key_pad_len + ctx->enckeylen :
+ CAAM_PTR_SZ * 2), GFP_DMA | GFP_KERNEL);
if (!sh_desc) {
dev_err(jrdev, "could not allocate shared descriptor\n");
return -ENOMEM;
--
1.7.5.1
--
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
[PATCH] crypto: caam - fix build warning when DEBUG_FS not configured
drivers/crypto/caam/ctrl.c: In function 'caam_probe': drivers/crypto/caam/ctrl.c:55:23: warning: unused variable 'perfmon' Signed-off-by: Kim Phillips --- drivers/crypto/caam/ctrl.c |4 +++- 1 files changed, 3 insertions(+), 1 deletions(-) diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c index 9009713..fc2d9ed 100644 --- a/drivers/crypto/caam/ctrl.c +++ b/drivers/crypto/caam/ctrl.c @@ -52,9 +52,11 @@ static int caam_probe(struct platform_device *pdev) struct caam_ctrl __iomem *ctrl; struct caam_full __iomem *topregs; struct caam_drv_private *ctrlpriv; - struct caam_perfmon *perfmon; struct caam_deco **deco; u32 deconum; +#ifdef CONFIG_DEBUG_FS + struct caam_perfmon *perfmon; +#endif ctrlpriv = kzalloc(sizeof(struct caam_drv_private), GFP_KERNEL); if (!ctrlpriv) -- 1.7.5.4 -- 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] hwrng: ppc4xx - add support for ppc4xx TRNG
[adding linux-crypto]
On Tue, 21 Jun 2011 10:56:02 -0500
Matt Mackall wrote:
> On Tue, 2011-06-21 at 08:19 -0400, Josh Boyer wrote:
> > +static struct hwrng ppc4xx_rng = {
> > + .name = MODULE_NAME,
> > + .data_present = ppc4xx_rng_data_present,
> > + .data_read = ppc4xx_rng_data_read,
> > +};
under what criteria should new hwrng drivers use the new hwrng API?
See commit bb347d9 "hwrng: virtio-rng - Convert to new API".
Kim
--
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] crypto: caam - fix operator precedence in shared descriptor allocation
On Thu, 26 May 2011 13:30:44 +1000 Herbert Xu wrote: > On Mon, May 23, 2011 at 06:45:23PM -0500, Kim Phillips wrote: > > setkey allocates 16 bytes (CAAM_CMD_SZ * > > DESC_AEAD_SHARED_TEXT_LEN) shy of what is needed to > > store the shared descriptor, resulting in memory > > corruption. Fix this. > > > > Signed-off-by: Kim Phillips > > Applied to cryptodev. Thanks Kim! Herbert, this patch fixes a memory corruption bug introduced in commit 4427b1b - "crypto: caam - add support for sha512 variants of existing AEAD algorithms", which is currently the last commit for caam in Linus' tree. Can you please push this commit (c5bf900 in cryptodev) to Linus before he releases 3.0? btw, it would be good if commit af56dea "crypto: caam - fix build warning when DEBUG_FS not configured" were also pushed to avoid the build-bot messages, but it's not nearly as important as this one. Thanks, Kim -- 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
[PATCH 1/8] crypto: talitos - ensure request ordering within a single tfm
Assign single target channel per tfm in talitos_cra_init instead of
performing channel scheduling dynamically during the encryption request.
This changes the talitos_submit interface to accept a new channel
number argument. Without this, rapid bursts of misc. sized requests
could make it possible for IPsec packets to be encrypted out-of-order,
which would result in packet drops due to sequence numbers falling
outside the anti-reply window on a peer gateway.
Signed-off-by: Kim Phillips
---
drivers/crypto/talitos.c | 23 ++-
1 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 854e263..b8ca583 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -1,7 +1,7 @@
/*
* talitos - Freescale Integrated Security Engine (SEC) device driver
*
- * Copyright (c) 2008-2010 Freescale Semiconductor, Inc.
+ * Copyright (c) 2008-2011 Freescale Semiconductor, Inc.
*
* Scatterlist Crypto API glue code copied from files with the following:
* Copyright (c) 2006-2007 Herbert Xu
@@ -282,6 +282,7 @@ static int init_device(struct device *dev)
/**
* talitos_submit - submits a descriptor to the device for processing
* @dev: the SEC device to be used
+ * @ch:the SEC device channel to be used
* @desc: the descriptor to be processed by the device
* @callback: whom to call when processing is complete
* @context: a handle for use by caller (optional)
@@ -290,7 +291,7 @@ static int init_device(struct device *dev)
* callback must check err and feedback in descriptor header
* for device processing status.
*/
-static int talitos_submit(struct device *dev, struct talitos_desc *desc,
+static int talitos_submit(struct device *dev, int ch, struct talitos_desc
*desc,
void (*callback)(struct device *dev,
struct talitos_desc *desc,
void *context, int error),
@@ -298,15 +299,12 @@ static int talitos_submit(struct device *dev, struct
talitos_desc *desc,
{
struct talitos_private *priv = dev_get_drvdata(dev);
struct talitos_request *request;
- unsigned long flags, ch;
+ unsigned long flags;
int head;
/* select done notification */
desc->hdr |= DESC_HDR_DONE_NOTIFY;
- /* emulate SEC's round-robin channel fifo polling scheme */
- ch = atomic_inc_return(&priv->last_chan) & (priv->num_channels - 1);
-
spin_lock_irqsave(&priv->chan[ch].head_lock, flags);
if (!atomic_inc_not_zero(&priv->chan[ch].submit_count)) {
@@ -706,6 +704,7 @@ static void talitos_unregister_rng(struct device *dev)
struct talitos_ctx {
struct device *dev;
+ int ch;
__be32 desc_hdr_template;
u8 key[TALITOS_MAX_KEY_SIZE];
u8 iv[TALITOS_MAX_IV_LENGTH];
@@ -1117,7 +1116,7 @@ static int ipsec_esp(struct talitos_edesc *edesc, struct
aead_request *areq,
map_single_talitos_ptr(dev, &desc->ptr[6], ivsize, ctx->iv, 0,
DMA_FROM_DEVICE);
- ret = talitos_submit(dev, desc, callback, areq);
+ ret = talitos_submit(dev, ctx->ch, desc, callback, areq);
if (ret != -EINPROGRESS) {
ipsec_esp_unmap(dev, edesc, areq);
kfree(edesc);
@@ -1524,7 +1523,7 @@ static int common_nonsnoop(struct talitos_edesc *edesc,
to_talitos_ptr(&desc->ptr[6], 0);
desc->ptr[6].j_extent = 0;
- ret = talitos_submit(dev, desc, callback, areq);
+ ret = talitos_submit(dev, ctx->ch, desc, callback, areq);
if (ret != -EINPROGRESS) {
common_nonsnoop_unmap(dev, edesc, areq);
kfree(edesc);
@@ -1703,7 +1702,7 @@ static int common_nonsnoop_hash(struct talitos_edesc
*edesc,
/* last DWORD empty */
desc->ptr[6] = zero_entry;
- ret = talitos_submit(dev, desc, callback, areq);
+ ret = talitos_submit(dev, ctx->ch, desc, callback, areq);
if (ret != -EINPROGRESS) {
common_nonsnoop_hash_unmap(dev, edesc, areq);
kfree(edesc);
@@ -2244,6 +2243,7 @@ static int talitos_cra_init(struct crypto_tfm *tfm)
struct crypto_alg *alg = tfm->__crt_alg;
struct talitos_crypto_alg *talitos_alg;
struct talitos_ctx *ctx = crypto_tfm_ctx(tfm);
+ struct talitos_private *priv;
if ((alg->cra_flags & CRYPTO_ALG_TYPE_MASK) == CRYPTO_ALG_TYPE_AHASH)
talitos_alg = container_of(__crypto_ahash_alg(alg),
@@ -2256,6 +2256,11 @@ static int talitos_cra_init(struct crypto_tfm *tfm)
/* update context with ptr to dev */
ctx->dev = talitos_alg->dev;
+ /* assign SEC channel to tfm in round-robin fashion */
+ priv = dev_get_drvdata(ctx->dev);
+ ctx->ch = atomic_inc_return(&a
[PATCH 2/8] crypto: talitos - don't set done notification in hot path
IRQ done notification is always set. Remove its explicit
assignment from the hot path by including it in the
descriptor header template assignment in talitos_cra_init.
Signed-off-by: Kim Phillips
---
drivers/crypto/talitos.c |6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index b8ca583..bd9e2ca 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -302,9 +302,6 @@ static int talitos_submit(struct device *dev, int ch,
struct talitos_desc *desc,
unsigned long flags;
int head;
- /* select done notification */
- desc->hdr |= DESC_HDR_DONE_NOTIFY;
-
spin_lock_irqsave(&priv->chan[ch].head_lock, flags);
if (!atomic_inc_not_zero(&priv->chan[ch].submit_count)) {
@@ -2264,6 +2261,9 @@ static int talitos_cra_init(struct crypto_tfm *tfm)
/* copy descriptor header template value */
ctx->desc_hdr_template = talitos_alg->algt.desc_hdr_template;
+ /* select done notification */
+ ctx->desc_hdr_template |= DESC_HDR_DONE_NOTIFY;
+
return 0;
}
--
1.7.6
--
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
[PATCH 3/8] crypto: talitos - remove unused giv from ablkcipher methods
Signed-off-by: Kim Phillips --- drivers/crypto/talitos.c |7 +++ 1 files changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c index bd9e2ca..521244e 100644 --- a/drivers/crypto/talitos.c +++ b/drivers/crypto/talitos.c @@ -1429,7 +1429,6 @@ static void ablkcipher_done(struct device *dev, static int common_nonsnoop(struct talitos_edesc *edesc, struct ablkcipher_request *areq, - u8 *giv, void (*callback) (struct device *dev, struct talitos_desc *desc, void *context, int error)) @@ -1449,7 +1448,7 @@ static int common_nonsnoop(struct talitos_edesc *edesc, /* cipher iv */ ivsize = crypto_ablkcipher_ivsize(cipher); - map_single_talitos_ptr(dev, &desc->ptr[1], ivsize, giv ?: areq->info, 0, + map_single_talitos_ptr(dev, &desc->ptr[1], ivsize, areq->info, 0, DMA_TO_DEVICE); /* cipher key */ @@ -1552,7 +1551,7 @@ static int ablkcipher_encrypt(struct ablkcipher_request *areq) /* set encrypt */ edesc->desc.hdr = ctx->desc_hdr_template | DESC_HDR_MODE0_ENCRYPT; - return common_nonsnoop(edesc, areq, NULL, ablkcipher_done); + return common_nonsnoop(edesc, areq, ablkcipher_done); } static int ablkcipher_decrypt(struct ablkcipher_request *areq) @@ -1568,7 +1567,7 @@ static int ablkcipher_decrypt(struct ablkcipher_request *areq) edesc->desc.hdr = ctx->desc_hdr_template | DESC_HDR_DIR_INBOUND; - return common_nonsnoop(edesc, areq, NULL, ablkcipher_done); + return common_nonsnoop(edesc, areq, ablkcipher_done); } static void common_nonsnoop_hash_unmap(struct device *dev, -- 1.7.6 -- 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
[PATCH 6/8] crypto: caam - structure renaming
From: Yuan Kang
caam_ctx.key_phys to key_dma
caam_alg_template supports multiple algorithm types
listed in union, which requires cases for
different types in function caam_alg_alloc
Signed-off-by: Yuan Kang
Signed-off-by: Kim Phillips
---
drivers/crypto/caam/caamalg.c | 64 +++-
1 files changed, 43 insertions(+), 21 deletions(-)
diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index 4786a20..403b293 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -87,7 +87,7 @@ struct caam_ctx {
u32 class2_alg_type;
u32 alg_op;
u8 *key;
- dma_addr_t key_phys;
+ dma_addr_t key_dma;
unsigned int enckeylen;
unsigned int split_key_len;
unsigned int split_key_pad_len;
@@ -263,9 +263,9 @@ static int build_sh_desc_ipsec(struct caam_ctx *ctx)
ctx->split_key_pad_len, ctx->enckeylen,
ctx->enckeylen, CLASS_1 | KEY_DEST_CLASS_REG);
} else {
- append_key(sh_desc, ctx->key_phys, ctx->split_key_len, CLASS_2 |
+ append_key(sh_desc, ctx->key_dma, ctx->split_key_len, CLASS_2 |
KEY_DEST_MDHA_SPLIT | KEY_ENC);
- append_key(sh_desc, ctx->key_phys + ctx->split_key_pad_len,
+ append_key(sh_desc, ctx->key_dma + ctx->split_key_pad_len,
ctx->enckeylen, CLASS_1 | KEY_DEST_CLASS_REG);
}
@@ -342,9 +342,9 @@ static int aead_setkey(struct crypto_aead *aead,
/* postpend encryption key to auth split key */
memcpy(ctx->key + ctx->split_key_pad_len, key + authkeylen, enckeylen);
- ctx->key_phys = dma_map_single(jrdev, ctx->key, ctx->split_key_pad_len +
+ ctx->key_dma = dma_map_single(jrdev, ctx->key, ctx->split_key_pad_len +
enckeylen, DMA_TO_DEVICE);
- if (dma_mapping_error(jrdev, ctx->key_phys)) {
+ if (dma_mapping_error(jrdev, ctx->key_dma)) {
dev_err(jrdev, "unable to map key i/o memory\n");
kfree(ctx->key);
return -ENOMEM;
@@ -359,7 +359,7 @@ static int aead_setkey(struct crypto_aead *aead,
ret = build_sh_desc_ipsec(ctx);
if (ret) {
- dma_unmap_single(jrdev, ctx->key_phys, ctx->split_key_pad_len +
+ dma_unmap_single(jrdev, ctx->key_dma, ctx->split_key_pad_len +
enckeylen, DMA_TO_DEVICE);
kfree(ctx->key);
}
@@ -884,11 +884,20 @@ static int aead_givencrypt(struct aead_givcrypt_request
*areq)
return init_aead_job(edesc, req, OP_ALG_ENCRYPT, aead_encrypt_done);
}
+#define template_aead template_u.aead
struct caam_alg_template {
char name[CRYPTO_MAX_ALG_NAME];
char driver_name[CRYPTO_MAX_ALG_NAME];
unsigned int blocksize;
- struct aead_alg aead;
+ u32 type;
+ union {
+ struct ablkcipher_alg ablkcipher;
+ struct aead_alg aead;
+ struct blkcipher_alg blkcipher;
+ struct cipher_alg cipher;
+ struct compress_alg compress;
+ struct rng_alg rng;
+ } template_u;
u32 class1_alg_type;
u32 class2_alg_type;
u32 alg_op;
@@ -900,7 +909,8 @@ static struct caam_alg_template driver_algs[] = {
.name = "authenc(hmac(sha1),cbc(aes))",
.driver_name = "authenc-hmac-sha1-cbc-aes-caam",
.blocksize = AES_BLOCK_SIZE,
- .aead = {
+ .type = CRYPTO_ALG_TYPE_AEAD,
+ .template_aead = {
.setkey = aead_setkey,
.setauthsize = aead_setauthsize,
.encrypt = aead_encrypt,
@@ -918,7 +928,8 @@ static struct caam_alg_template driver_algs[] = {
.name = "authenc(hmac(sha256),cbc(aes))",
.driver_name = "authenc-hmac-sha256-cbc-aes-caam",
.blocksize = AES_BLOCK_SIZE,
- .aead = {
+ .type = CRYPTO_ALG_TYPE_AEAD,
+ .template_aead = {
.setkey = aead_setkey,
.setauthsize = aead_setauthsize,
.encrypt = aead_encrypt,
@@ -937,7 +948,8 @@ static struct caam_alg_template driver_algs[] = {
.name = "authenc(hmac(sha512),cbc(aes))",
.driver_name = "authenc-hmac-sha512-cbc-aes-caam",
.blocksize = AES_BLOCK_SIZE,
- .aead = {
+ .type = CRYPTO_ALG_TYPE_AEAD,
+ .template_aead = {
.setkey = aead_setkey,
.setauthsize = aead_setauthsize,
[PATCH 4/8] crypto: talitos - don't bad_key in ablkcipher setkey
crypto/ablkcipher.c's setkey() has already checked against the min, max
key sizes before it calls here, and all max_keysize assignments in the
algorithm template array do not exceed TALITOS_MAX_KEY_SIZE.
Signed-off-by: Kim Phillips
---
drivers/crypto/talitos.c | 11 ---
1 files changed, 0 insertions(+), 11 deletions(-)
diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 521244e..8a0bb41 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -1378,22 +1378,11 @@ static int ablkcipher_setkey(struct crypto_ablkcipher
*cipher,
const u8 *key, unsigned int keylen)
{
struct talitos_ctx *ctx = crypto_ablkcipher_ctx(cipher);
- struct ablkcipher_alg *alg = crypto_ablkcipher_alg(cipher);
-
- if (keylen > TALITOS_MAX_KEY_SIZE)
- goto badkey;
-
- if (keylen < alg->min_keysize || keylen > alg->max_keysize)
- goto badkey;
memcpy(&ctx->key, key, keylen);
ctx->keylen = keylen;
return 0;
-
-badkey:
- crypto_ablkcipher_set_flags(cipher, CRYPTO_TFM_RES_BAD_KEY_LEN);
- return -EINVAL;
}
static void common_nonsnoop_unmap(struct device *dev,
--
1.7.6
--
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
[PATCH 5/8] crypto: caam - shorter names
From: Yuan Kang
"aead_authenc" and "ipsec_esp" changed to "aead,"
except for function "ipsec_esp," which is changed
to "init_aead_job."
Variable name of aead_request structures changed
to "req" and name of aead_givcrypt_request structure
changed to "areq"
Signed-off-by: Yuan Kang
Signed-off-by: Kim Phillips
---
drivers/crypto/caam/caamalg.c | 274
1 files changed, 137 insertions(+), 137 deletions(-)
diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index 676d957..4786a20 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -94,7 +94,7 @@ struct caam_ctx {
unsigned int authsize;
};
-static int aead_authenc_setauthsize(struct crypto_aead *authenc,
+static int aead_setauthsize(struct crypto_aead *authenc,
unsigned int authsize)
{
struct caam_ctx *ctx = crypto_aead_ctx(authenc);
@@ -286,7 +286,7 @@ static int build_sh_desc_ipsec(struct caam_ctx *ctx)
return 0;
}
-static int aead_authenc_setkey(struct crypto_aead *aead,
+static int aead_setkey(struct crypto_aead *aead,
const u8 *key, unsigned int keylen)
{
/* Sizes for MDHA pads (*not* keys): MD5, SHA1, 224, 256, 384, 512 */
@@ -379,7 +379,7 @@ struct link_tbl_entry {
};
/*
- * ipsec_esp_edesc - s/w-extended ipsec_esp descriptor
+ * aead_edesc - s/w-extended ipsec_esp descriptor
* @src_nents: number of segments in input scatterlist
* @dst_nents: number of segments in output scatterlist
* @assoc_nents: number of segments in associated data (SPI+Seq) scatterlist
@@ -388,7 +388,7 @@ struct link_tbl_entry {
* @link_tbl_dma: bus physical mapped address of h/w link table
* @hw_desc: the h/w job descriptor followed by any referenced link tables
*/
-struct ipsec_esp_edesc {
+struct aead_edesc {
int assoc_nents;
int src_nents;
int dst_nents;
@@ -398,19 +398,19 @@ struct ipsec_esp_edesc {
u32 hw_desc[0];
};
-static void ipsec_esp_unmap(struct device *dev,
- struct ipsec_esp_edesc *edesc,
- struct aead_request *areq)
+static void aead_unmap(struct device *dev,
+ struct aead_edesc *edesc,
+ struct aead_request *req)
{
- dma_unmap_sg(dev, areq->assoc, edesc->assoc_nents, DMA_TO_DEVICE);
+ dma_unmap_sg(dev, req->assoc, edesc->assoc_nents, DMA_TO_DEVICE);
- if (unlikely(areq->dst != areq->src)) {
- dma_unmap_sg(dev, areq->src, edesc->src_nents,
+ if (unlikely(req->dst != req->src)) {
+ dma_unmap_sg(dev, req->src, edesc->src_nents,
DMA_TO_DEVICE);
- dma_unmap_sg(dev, areq->dst, edesc->dst_nents,
+ dma_unmap_sg(dev, req->dst, edesc->dst_nents,
DMA_FROM_DEVICE);
} else {
- dma_unmap_sg(dev, areq->src, edesc->src_nents,
+ dma_unmap_sg(dev, req->src, edesc->src_nents,
DMA_BIDIRECTIONAL);
}
@@ -423,20 +423,20 @@ static void ipsec_esp_unmap(struct device *dev,
/*
* ipsec_esp descriptor callbacks
*/
-static void ipsec_esp_encrypt_done(struct device *jrdev, u32 *desc, u32 err,
+static void aead_encrypt_done(struct device *jrdev, u32 *desc, u32 err,
void *context)
{
- struct aead_request *areq = context;
- struct ipsec_esp_edesc *edesc;
+ struct aead_request *req = context;
+ struct aead_edesc *edesc;
#ifdef DEBUG
- struct crypto_aead *aead = crypto_aead_reqtfm(areq);
+ struct crypto_aead *aead = crypto_aead_reqtfm(req);
int ivsize = crypto_aead_ivsize(aead);
struct caam_ctx *ctx = crypto_aead_ctx(aead);
dev_err(jrdev, "%s %d: err 0x%x\n", __func__, __LINE__, err);
#endif
- edesc = (struct ipsec_esp_edesc *)((char *)desc -
-offsetof(struct ipsec_esp_edesc, hw_desc));
+ edesc = (struct aead_edesc *)((char *)desc -
+offsetof(struct aead_edesc, hw_desc));
if (err) {
char tmp[CAAM_ERROR_STR_MAX];
@@ -444,39 +444,39 @@ static void ipsec_esp_encrypt_done(struct device *jrdev,
u32 *desc, u32 err,
dev_err(jrdev, "%08x: %s\n", err, caam_jr_strstatus(tmp, err));
}
- ipsec_esp_unmap(jrdev, edesc, areq);
+ aead_unmap(jrdev, edesc, req);
#ifdef DEBUG
print_hex_dump(KERN_ERR, "assoc @"xstr(__LINE__)": ",
- DUMP_PREFIX_ADDRESS, 16, 4, sg_virt(areq->assoc),
- areq->assoclen , 1);
+ DUMP_PREFIX_ADDRESS, 16, 4, sg_virt(req->assoc),
+
[PATCH 8/8] crypto: caam - ablkcipher support
From: Yuan Kang
caam now supports encrypt and decrypt
for aes, des and 3des
Signed-off-by: Yuan Kang
Signed-off-by: Kim Phillips
---
drivers/crypto/caam/caamalg.c | 510 +
drivers/crypto/caam/compat.h |1 +
2 files changed, 511 insertions(+), 0 deletions(-)
diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index ed7d59d..4159265 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -69,6 +69,12 @@
#define DESC_AEAD_DEC_LEN (DESC_AEAD_BASE + 21 * CAAM_CMD_SZ)
#define DESC_AEAD_GIVENC_LEN (DESC_AEAD_ENC_LEN + 7 * CAAM_CMD_SZ)
+#define DESC_ABLKCIPHER_BASE (3 * CAAM_CMD_SZ)
+#define DESC_ABLKCIPHER_ENC_LEN(DESC_ABLKCIPHER_BASE + \
+20 * CAAM_CMD_SZ)
+#define DESC_ABLKCIPHER_DEC_LEN(DESC_ABLKCIPHER_BASE + \
+15 * CAAM_CMD_SZ)
+
#define DESC_MAX_USED_BYTES(DESC_AEAD_GIVENC_LEN + \
CAAM_MAX_KEY_SIZE)
#define DESC_MAX_USED_LEN (DESC_MAX_USED_BYTES / CAAM_CMD_SZ)
@@ -132,6 +138,19 @@ static inline void aead_append_ld_iv(u32 *desc, int ivsize)
}
/*
+ * For ablkcipher encrypt and decrypt, read from req->src and
+ * write to req->dst
+ */
+static inline void ablkcipher_append_src_dst(u32 *desc)
+{
+ append_math_add(desc, VARSEQOUTLEN, SEQINLEN, REG0, CAAM_CMD_SZ); \
+ append_math_add(desc, VARSEQINLEN, SEQINLEN, REG0, CAAM_CMD_SZ); \
+ append_seq_fifo_load(desc, 0, FIFOLD_CLASS_CLASS1 | \
+KEY_VLF | FIFOLD_TYPE_MSG | FIFOLD_TYPE_LAST1); \
+ append_seq_fifo_store(desc, 0, FIFOST_TYPE_MESSAGE_DATA | KEY_VLF); \
+}
+
+/*
* If all data, including src (with assoc and iv) or dst (with iv only) are
* contiguous
*/
@@ -625,6 +644,119 @@ badkey:
return -EINVAL;
}
+static int ablkcipher_setkey(struct crypto_ablkcipher *ablkcipher,
+const u8 *key, unsigned int keylen)
+{
+ struct caam_ctx *ctx = crypto_ablkcipher_ctx(ablkcipher);
+ struct ablkcipher_tfm *tfm = &ablkcipher->base.crt_ablkcipher;
+ struct device *jrdev = ctx->jrdev;
+ int ret = 0;
+ u32 *key_jump_cmd, *jump_cmd;
+ u32 *desc;
+
+#ifdef DEBUG
+ print_hex_dump(KERN_ERR, "key in @"xstr(__LINE__)": ",
+ DUMP_PREFIX_ADDRESS, 16, 4, key, keylen, 1);
+#endif
+
+ memcpy(ctx->key, key, keylen);
+ ctx->key_dma = dma_map_single(jrdev, ctx->key, keylen,
+ DMA_TO_DEVICE);
+ if (dma_mapping_error(jrdev, ctx->key_dma)) {
+ dev_err(jrdev, "unable to map key i/o memory\n");
+ return -ENOMEM;
+ }
+ ctx->enckeylen = keylen;
+
+ /* ablkcipher_encrypt shared descriptor */
+ desc = ctx->sh_desc_enc;
+ init_sh_desc(desc, HDR_SHARE_WAIT);
+ /* Skip if already shared */
+ key_jump_cmd = append_jump(desc, JUMP_JSL | JUMP_TEST_ALL |
+ JUMP_COND_SHRD);
+
+ /* Load class1 key only */
+ append_key_as_imm(desc, (void *)ctx->key, ctx->enckeylen,
+ ctx->enckeylen, CLASS_1 |
+ KEY_DEST_CLASS_REG);
+
+ set_jump_tgt_here(desc, key_jump_cmd);
+
+ /* Propagate errors from shared to job descriptor */
+ append_cmd(desc, SET_OK_PROP_ERRORS | CMD_LOAD);
+
+ /* Load iv */
+ append_cmd(desc, CMD_SEQ_LOAD | LDST_SRCDST_BYTE_CONTEXT |
+ LDST_CLASS_1_CCB | tfm->ivsize);
+
+ /* Load operation */
+ append_operation(desc, ctx->class1_alg_type |
+OP_ALG_AS_INITFINAL | OP_ALG_ENCRYPT);
+
+ /* Perform operation */
+ ablkcipher_append_src_dst(desc);
+
+ ctx->sh_desc_enc_dma = dma_map_single(jrdev, desc,
+ desc_bytes(desc),
+ DMA_TO_DEVICE);
+ if (dma_mapping_error(jrdev, ctx->sh_desc_enc_dma)) {
+ dev_err(jrdev, "unable to map shared descriptor\n");
+ return -ENOMEM;
+ }
+#ifdef DEBUG
+ print_hex_dump(KERN_ERR, "ablkcipher enc shdesc@"xstr(__LINE__)": ",
+ DUMP_PREFIX_ADDRESS, 16, 4, desc,
+ desc_bytes(desc), 1);
+#endif
+ /* ablkcipher_decrypt shared descriptor */
+ desc = ctx->sh_desc_dec;
+
+ init_sh_desc(desc, HDR_SHARE_WAIT);
+ /* Skip if already shared */
+ key_jump_cmd = append_jump(desc, JUMP_JSL | JUMP_TEST_ALL |
+ JUMP_COND_SHRD);
+
+ /* Load class1 key only */
+ append_key_as_imm(desc, (void *)ctx->key, ctx->enckeylen,
+ ctx-
[PATCH 7/8] crypto: caam - faster aead implementation
From: Yuan Kang
Job descriptors only contain header and seq pointers.
Other commands are stored in separate shared descriptors
for encrypt, decrypt and givencrypt, stored as arrays
in caam_ctx.
This requires additional macros to create math commands
to calculate assoclen and cryptlen.
Signed-off-by: Yuan Kang
Signed-off-by: Kim Phillips
---
drivers/crypto/caam/caamalg.c | 1104 ++---
drivers/crypto/caam/desc_constr.h | 58 ++-
2 files changed, 832 insertions(+), 330 deletions(-)
diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index 403b293..ed7d59d 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -62,10 +62,16 @@
#define CAAM_MAX_IV_LENGTH 16
/* length of descriptors text */
-#define DESC_AEAD_SHARED_TEXT_LEN 4
-#define DESC_AEAD_ENCRYPT_TEXT_LEN 21
-#define DESC_AEAD_DECRYPT_TEXT_LEN 24
-#define DESC_AEAD_GIVENCRYPT_TEXT_LEN 27
+#define DESC_JOB_IO_LEN(CAAM_CMD_SZ * 3 + CAAM_PTR_SZ
* 3)
+
+#define DESC_AEAD_BASE (4 * CAAM_CMD_SZ)
+#define DESC_AEAD_ENC_LEN (DESC_AEAD_BASE + 16 * CAAM_CMD_SZ)
+#define DESC_AEAD_DEC_LEN (DESC_AEAD_BASE + 21 * CAAM_CMD_SZ)
+#define DESC_AEAD_GIVENC_LEN (DESC_AEAD_ENC_LEN + 7 * CAAM_CMD_SZ)
+
+#define DESC_MAX_USED_BYTES(DESC_AEAD_GIVENC_LEN + \
+CAAM_MAX_KEY_SIZE)
+#define DESC_MAX_USED_LEN (DESC_MAX_USED_BYTES / CAAM_CMD_SZ)
#ifdef DEBUG
/* for print_hex_dumps with line references */
@@ -76,17 +82,77 @@
#define debug(format, arg...)
#endif
+/* Set DK bit in class 1 operation if shared */
+static inline void append_dec_op1(u32 *desc, u32 type)
+{
+ u32 *jump_cmd, *uncond_jump_cmd;
+
+ jump_cmd = append_jump(desc, JUMP_TEST_ALL | JUMP_COND_SHRD);
+ append_operation(desc, type | OP_ALG_AS_INITFINAL |
+OP_ALG_DECRYPT);
+ uncond_jump_cmd = append_jump(desc, JUMP_TEST_ALL);
+ set_jump_tgt_here(desc, jump_cmd);
+ append_operation(desc, type | OP_ALG_AS_INITFINAL |
+OP_ALG_DECRYPT | OP_ALG_AAI_DK);
+ set_jump_tgt_here(desc, uncond_jump_cmd);
+}
+
+/*
+ * Wait for completion of class 1 key loading before allowing
+ * error propagation
+ */
+static inline void append_dec_shr_done(u32 *desc)
+{
+ u32 *jump_cmd;
+
+ jump_cmd = append_jump(desc, JUMP_CLASS_CLASS1 | JUMP_TEST_ALL);
+ set_jump_tgt_here(desc, jump_cmd);
+ append_cmd(desc, SET_OK_PROP_ERRORS | CMD_LOAD);
+}
+
+/*
+ * For aead functions, read payload and write payload,
+ * both of which are specified in req->src and req->dst
+ */
+static inline void aead_append_src_dst(u32 *desc, u32 msg_type)
+{
+ append_seq_fifo_load(desc, 0, FIFOLD_CLASS_BOTH |
+KEY_VLF | msg_type | FIFOLD_TYPE_LASTBOTH);
+ append_seq_fifo_store(desc, 0, FIFOST_TYPE_MESSAGE_DATA | KEY_VLF);
+}
+
+/*
+ * For aead encrypt and decrypt, read iv for both classes
+ */
+static inline void aead_append_ld_iv(u32 *desc, int ivsize)
+{
+ append_cmd(desc, CMD_SEQ_LOAD | LDST_SRCDST_BYTE_CONTEXT |
+ LDST_CLASS_1_CCB | ivsize);
+ append_move(desc, MOVE_SRC_CLASS1CTX | MOVE_DEST_CLASS2INFIFO | ivsize);
+}
+
+/*
+ * If all data, including src (with assoc and iv) or dst (with iv only) are
+ * contiguous
+ */
+#define GIV_SRC_CONTIG 1
+#define GIV_DST_CONTIG (1 << 1)
+
/*
* per-session context
*/
struct caam_ctx {
struct device *jrdev;
- u32 *sh_desc;
- dma_addr_t shared_desc_phys;
+ u32 sh_desc_enc[DESC_MAX_USED_LEN];
+ u32 sh_desc_dec[DESC_MAX_USED_LEN];
+ u32 sh_desc_givenc[DESC_MAX_USED_LEN];
+ dma_addr_t sh_desc_enc_dma;
+ dma_addr_t sh_desc_dec_dma;
+ dma_addr_t sh_desc_givenc_dma;
u32 class1_alg_type;
u32 class2_alg_type;
u32 alg_op;
- u8 *key;
+ u8 key[CAAM_MAX_KEY_SIZE];
dma_addr_t key_dma;
unsigned int enckeylen;
unsigned int split_key_len;
@@ -94,12 +160,275 @@ struct caam_ctx {
unsigned int authsize;
};
+static void append_key_aead(u32 *desc, struct caam_ctx *ctx,
+ int keys_fit_inline)
+{
+ if (keys_fit_inline) {
+ append_key_as_imm(desc, ctx->key, ctx->split_key_pad_len,
+ ctx->split_key_len, CLASS_2 |
+ KEY_DEST_MDHA_SPLIT | KEY_ENC);
+ append_key_as_imm(desc, (void *)ctx->key +
+ ctx->split_key_pad_len, ctx->enckeylen,
+ ctx->enckeylen, CLASS_1 | KEY_DEST_CLASS_REG);
+ } else {
+ append_key(desc, ctx->key_dma, ctx->split_key_len, CLASS_2 |
+ KEY_DEST_MDHA_SPLIT | KEY_ENC);
+
Re: Kernel OOPS with Freescale talitos driver on ppc
On Mon, 19 Sep 2011 13:33:57 +0200
Sven Schnelle wrote:
> Hi Kim,
>
> i'm seeing the following oops on ppc (Freescale P1020):
>
> talitos ffe3.crypto: cur_desc: (channel 1)
> Unable to handle kernel paging request for data at address 0x
You should have seen the "couldn't locate current descriptor" error
message right before it crashed.
> This happens due to the fact that the SEC engine returns NULL in the
> CDPR register, so current_desc() returns NULL, which gets dereferenced
> in report_eu_error().
> I've fixed that with the attached patch, however, i have no idea why the
> SEC engine doesn't return a descriptor pointer in CDPR. As i understand
> the documentation, it should put the 'currently processed' descriptor
> there. Can someone shed some light on this?
I believe the CDPR may be overwritten by the time the core gets to
it if there is another descriptor queued in the channel's fetch
FIFO. You may be able to glean more information about what happened
by looking at the ISR execution units done/error bits in isr_lo, the
descriptor buffer (displayed at the end of report_eu_error), or the
EU ISRs themselves (see below).
fwiw, I just booted a vanilla 3.1-rc6 kernel on a p1020 with your
config and CONFIG_CRYPTO_MANAGER_DISABLE_TESTS not set, and the
selftests passed. IPSec isn't enabled in your config. Are you
implementing a new algorithm? If not, how to reproduce?
Kim
diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 8a0bb41..568847e 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -442,64 +442,55 @@ static void report_eu_error(struct device *dev, int ch,
struct talitos_desc *desc)
{
struct talitos_private *priv = dev_get_drvdata(dev);
+ u32 eu = desc ? desc->hdr : ~0;
int i;
- switch (desc->hdr & DESC_HDR_SEL0_MASK) {
+ switch (eu & DESC_HDR_SEL0_MASK) {
case DESC_HDR_SEL0_AFEU:
dev_err(dev, "AFEUISR 0x%08x_%08x\n",
in_be32(priv->reg + TALITOS_AFEUISR),
in_be32(priv->reg + TALITOS_AFEUISR_LO));
- break;
case DESC_HDR_SEL0_DEU:
dev_err(dev, "DEUISR 0x%08x_%08x\n",
in_be32(priv->reg + TALITOS_DEUISR),
in_be32(priv->reg + TALITOS_DEUISR_LO));
- break;
case DESC_HDR_SEL0_MDEUA:
case DESC_HDR_SEL0_MDEUB:
dev_err(dev, "MDEUISR 0x%08x_%08x\n",
in_be32(priv->reg + TALITOS_MDEUISR),
in_be32(priv->reg + TALITOS_MDEUISR_LO));
- break;
case DESC_HDR_SEL0_RNG:
dev_err(dev, "RNGUISR 0x%08x_%08x\n",
in_be32(priv->reg + TALITOS_RNGUISR),
in_be32(priv->reg + TALITOS_RNGUISR_LO));
- break;
case DESC_HDR_SEL0_PKEU:
dev_err(dev, "PKEUISR 0x%08x_%08x\n",
in_be32(priv->reg + TALITOS_PKEUISR),
in_be32(priv->reg + TALITOS_PKEUISR_LO));
- break;
case DESC_HDR_SEL0_AESU:
dev_err(dev, "AESUISR 0x%08x_%08x\n",
in_be32(priv->reg + TALITOS_AESUISR),
in_be32(priv->reg + TALITOS_AESUISR_LO));
- break;
case DESC_HDR_SEL0_CRCU:
dev_err(dev, "CRCUISR 0x%08x_%08x\n",
in_be32(priv->reg + TALITOS_CRCUISR),
in_be32(priv->reg + TALITOS_CRCUISR_LO));
- break;
case DESC_HDR_SEL0_KEU:
dev_err(dev, "KEUISR 0x%08x_%08x\n",
in_be32(priv->reg + TALITOS_KEUISR),
in_be32(priv->reg + TALITOS_KEUISR_LO));
- break;
}
- switch (desc->hdr & DESC_HDR_SEL1_MASK) {
+ switch (eu & DESC_HDR_SEL1_MASK) {
case DESC_HDR_SEL1_MDEUA:
case DESC_HDR_SEL1_MDEUB:
dev_err(dev, "MDEUISR 0x%08x_%08x\n",
in_be32(priv->reg + TALITOS_MDEUISR),
in_be32(priv->reg + TALITOS_MDEUISR_LO));
- break;
case DESC_HDR_SEL1_CRCU:
dev_err(dev, "CRCUISR 0x%08x_%08x\n",
in_be32(priv->reg + TALITOS_CRCUISR),
in_be32(priv->reg + TALITOS_CRCUISR_LO));
- break;
}
for (i = 0; i < 8; i++)
--
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
[PATCH] talitos: handle descriptor not found in error path
The CDPR (Current Descriptor Pointer Register) can be unreliable
when trying to locate an offending descriptor. Handle that case by
(a) not OOPSing, and (b) reverting to the machine internal copy of
the descriptor header in order to report the correct execution unit
error.
Note: printing all execution units' ISRs is not effective because it
results in an internal time out (ITO) error and the EU resetting its
ISR value (at least when specifying an invalid key length on an SEC
2.2/MPC8313E).
Reported-by: Sven Schnelle
Signed-off-by: Kim Phillips
---
please test, as it seems I cannot reproduce the descriptor not found
case.
drivers/crypto/talitos.c | 18 ++
1 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index 8a0bb41..dbe76b5 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -416,7 +416,7 @@ static void talitos_done(unsigned long data)
/*
* locate current (offending) descriptor
*/
-static struct talitos_desc *current_desc(struct device *dev, int ch)
+static u32 current_desc_hdr(struct device *dev, int ch)
{
struct talitos_private *priv = dev_get_drvdata(dev);
int tail = priv->chan[ch].tail;
@@ -428,23 +428,25 @@ static struct talitos_desc *current_desc(struct device
*dev, int ch)
tail = (tail + 1) & (priv->fifo_len - 1);
if (tail == priv->chan[ch].tail) {
dev_err(dev, "couldn't locate current descriptor\n");
- return NULL;
+ return 0;
}
}
- return priv->chan[ch].fifo[tail].desc;
+ return priv->chan[ch].fifo[tail].desc->hdr;
}
/*
* user diagnostics; report root cause of error based on execution unit status
*/
-static void report_eu_error(struct device *dev, int ch,
- struct talitos_desc *desc)
+static void report_eu_error(struct device *dev, int ch, u32 desc_hdr)
{
struct talitos_private *priv = dev_get_drvdata(dev);
int i;
- switch (desc->hdr & DESC_HDR_SEL0_MASK) {
+ if (!desc_hdr)
+ desc_hdr = in_be32(priv->reg + TALITOS_DESCBUF(ch));
+
+ switch (desc_hdr & DESC_HDR_SEL0_MASK) {
case DESC_HDR_SEL0_AFEU:
dev_err(dev, "AFEUISR 0x%08x_%08x\n",
in_be32(priv->reg + TALITOS_AFEUISR),
@@ -488,7 +490,7 @@ static void report_eu_error(struct device *dev, int ch,
break;
}
- switch (desc->hdr & DESC_HDR_SEL1_MASK) {
+ switch (desc_hdr & DESC_HDR_SEL1_MASK) {
case DESC_HDR_SEL1_MDEUA:
case DESC_HDR_SEL1_MDEUB:
dev_err(dev, "MDEUISR 0x%08x_%08x\n",
@@ -550,7 +552,7 @@ static void talitos_error(unsigned long data, u32 isr, u32
isr_lo)
if (v_lo & TALITOS_CCPSR_LO_IEU)
dev_err(dev, "invalid execution unit error\n");
if (v_lo & TALITOS_CCPSR_LO_EU)
- report_eu_error(dev, ch, current_desc(dev, ch));
+ report_eu_error(dev, ch, current_desc_hdr(dev, ch));
if (v_lo & TALITOS_CCPSR_LO_GB)
dev_err(dev, "gather boundary error\n");
if (v_lo & TALITOS_CCPSR_LO_GRL)
--
1.7.6
--
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] talitos: handle descriptor not found in error path
On Tue, 18 Oct 2011 09:36:18 +0200 Herbert Xu wrote: > Kim Phillips wrote: > > The CDPR (Current Descriptor Pointer Register) can be unreliable > > when trying to locate an offending descriptor. Handle that case by > > (a) not OOPSing, and (b) reverting to the machine internal copy of > > the descriptor header in order to report the correct execution unit > > error. > > > > Note: printing all execution units' ISRs is not effective because it > > results in an internal time out (ITO) error and the EU resetting its > > ISR value (at least when specifying an invalid key length on an SEC > > 2.2/MPC8313E). > > > > Reported-by: Sven Schnelle > > Signed-off-by: Kim Phillips > > --- > > please test, as it seems I cannot reproduce the descriptor not found > > case. > > So what's the verdict Kim, should I take this patch or not? sure - I've verified it does at least satisfy Sven's shouldn't-oops comment. btw, can you take a look at applying this also?: http://www.mail-archive.com/linux-crypto@vger.kernel.org/msg05996.html It makes IPSec AH work for async crypto implementations. Thanks, Kim -- 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] crypto: driver for tegra AES hardware
On Fri, 4 Nov 2011 16:44:16 +0530
wrote:
> +/* Define sub-commands */
> +enum {
> + SUBCMD_VRAM_SEL = 0x1,
> + SUBCMD_CRYPTO_TABLESEL = 0x3,
SUBCMD_CRYPTO_TABLE_SEL, to match VRAM_SEL for a more consistent
naming convention
> + SUBCMD_KEYTABLESEL = 0x8,
SUBCMD_KEY_TABLE_SEL?
> +/* memdma_vd command */
> +#define MEMDMA_DIR_DTOVRAM 0 /* sdram -> vram */
> +#define MEMDMA_DIR_VTODRAM 1 /* vram -> sdram */
> +#define MEMDMABITSHIFT_DIR 25
> +#define MEMDMABITSHIFT_NUM_WORDS 12
MEMDMA_DIR_SHIFT, MEMDMA_NUM_WORDS_SHIFT
> +
> +/* command queue bit shifts */
> +enum {
> + CMDQBITSHIFT_KEYTABLEADDR = 0,
> + CMDQBITSHIFT_KEYTABLEID = 17,
> + CMDQBITSHIFT_VRAMSEL = 23,
> + CMDQBITSHIFT_TABLESEL = 24,
> + CMDQBITSHIFT_OPCODE = 26,
same here. Seems to better suit existing naming strategy in this
driver.
> +static int aes_start_crypt(struct tegra_aes_dev *dd, u32 in_addr, u32
> out_addr,
> + int nblocks, int mode, bool upd_iv)
> +{
> + u32 cmdq[AES_HW_MAX_ICQ_LENGTH];
> + int qlen = 0, i, eng_busy, icq_empty, ret;
> + u32 value;
> +
> + /* reset all the interrupt bits */
> + aes_writel(eng, 0x, INTR_STATUS);
> +
> + /* enable error, dma xfer complete interrupts */
> + aes_writel(dd, 0x33, INT_ENB);
> + enable_irq(INT_VDE_BSE_V);
is it really necessary to manually control the enabling and
disabling of IRQs? If so, add a comment explaining why.
> + cmdq[qlen++] = CMD_DMASETUP << CMDQBITSHIFT_OPCODE;
> + cmdq[qlen++] = in_addr;
> + cmdq[qlen++] = CMD_BLKSTARTENGINE << CMDQBITSHIFT_OPCODE | (nblocks-1);
> + cmdq[qlen++] = CMD_DMACOMPLETE << CMDQBITSHIFT_OPCODE;
qlen appears to be AES_HW_MAX_ICQ_LENGTH -> would this be simpler?:
cmdq[0] = ..
cmdq[1] = ..
..and later qlen references be replaced with AES_HW_MAX_ICQ_LENGTH.
> + value = aes_readl(dd, CMDQUE_CONTROL);
> + /* access SDRAM through AHB */
> + value &= ~CMDQ_CTRL_SRC_STM_SEL_FIELD;
> + value &= ~CMDQ_CTRL_DST_STM_SEL_FIELD;
> + value |= (CMDQ_CTRL_SRC_STM_SEL_FIELD | CMDQ_CTRL_DST_STM_SEL_FIELD |
> + CMDQ_CTRL_ICMDQEN_FIELD);
arm arch could really use some set/clr/clrsetbits helpers..
> + aes_writel(dd, value, CMDQUE_CONTROL);
> + dev_dbg(dd->dev, "cmd_q_ctrl=0x%x", value);
> +
> + value = (0x1 << SECURE_INPUT_ALG_SEL_SHIFT) |
> + ((dd->ctx->keylen * 8) << SECURE_INPUT_KEY_LEN_SHIFT) |
> + ((u32)upd_iv << SECURE_IV_SELECT_SHIFT);
> +
> + if (mode & FLAGS_CBC) {
> + value |= mode & FLAGS_ENCRYPT) ? 2 : 3)
> + << SECURE_XOR_POS_SHIFT) |
> + (0 << SECURE_INPUT_SEL_SHIFT) |
> + (((mode & FLAGS_ENCRYPT) ? 2 : 3)
> + << SECURE_VCTRAM_SEL_SHIFT) |
> + ((mode & FLAGS_ENCRYPT) ? 1 : 0)
> + << SECURE_CORE_SEL_SHIFT |
> + (0 << SECURE_RNG_ENB_SHIFT) |
> + (0 << SECURE_HASH_ENB_SHIFT));
simpler and easier to read if we don't assign 0 to
context-irrelevant bitfields - I'm assuming such fields are
non-overlapping and already guaranteed to be 0.
> + for (i = 0; i < qlen - 1; i++) {
> + do {
> + value = aes_readl(dd, INTR_STATUS);
> + eng_busy = value & BIT(0);
> + icq_empty = value & BIT(3);
name BIT(0) and BIT(3)?
> +static struct tegra_aes_slot *aes_find_key_slot(struct tegra_aes_dev *dd)
> + spin_unlock(&list_lock);
> + return found ? slot : NULL;
leave blank lines before return statements
> +static int aes_set_key(struct tegra_aes_dev *dd)
> +{
> + u32 value, cmdq[2];
> + struct tegra_aes_ctx *ctx = dd->ctx;
> + int i, eng_busy, icq_empty, dma_busy;
> + bool use_ssk = false;
> +
> + if (!ctx) {
> + dev_err(dd->dev, "%s: context invalid\n", __func__);
> + return -EINVAL;
> + }
when would this condition be met?
> + if (use_ssk)
> + goto out;
return 0;
> +
> + /* copy the key table from sdram to vram */
> + cmdq[0] = 0;
not needed
> + cmdq[0] = CMD_MEMDMAVD << CMDQBITSHIFT_OPCODE |
> + (MEMDMA_DIR_DTOVRAM << MEMDMABITSHIFT_DIR) |
> + (AES_HW_KEY_TABLE_LENGTH_BYTES/sizeof(u32))
> + << MEMDMABITSHIFT_NUM_WORDS;
alignment, unnecessary parens, operators at end of prior line:
cmdq[0] = CMD_MEMDMAVD << CMDQBITSHIFT_OPCODE |
MEMDMA_DIR_DTOVRAM << MEMDMABITSHIFT_DIR |
AES_HW_KEY_TABLE_LENGTH_BYTES / sizeof(u32) <<
MEMDMABITSHIFT_NUM_WORDS;
> + cmdq[1] = (u32)dd->ivkey_phys_base;
> +
> + for (i = 0; i < ARRAY_SIZE(cmdq); i++)
> + aes_writel(dd, cmdq[i], ICMDQUE_WR);
ARRAY_SIZE is 2 here - why not use a single temporary variable and
two individual aes_writel()s?
> + v
Re: [PATCH v1] crypto: driver for tegra AES hardware
On Sat, 5 Nov 2011 16:12:14 +0530
wrote:
> +config CRYPTO_DEV_TEGRA_AES
> + tristate "Support for TEGRA AES hw engine"
> + depends on ARCH_TEGRA
> + select CRYPTO_AES
> + help
> + TEGRA processors have AES module accelerator. Select this if you
> + want to use the TEGRA module for AES algorithms.
> +
"To compile this driver as a module, choose M here: the module
will be called tegra-aes."
> +static int aes_start_crypt(struct tegra_aes_dev *dd, u32 in_addr, u32
> out_addr,
> + int nblocks, int mode, bool upd_iv)
> +{
> + u32 cmdq[AES_HW_MAX_ICQ_LENGTH];
> + int i, eng_busy, icq_empty, ret;
> + u32 value;
> +
> + /* reset all the interrupt bits */
> + aes_writel(dd, 0x, TEGRA_AES_INTR_STATUS);
> +
> + /* enable error, dma xfer complete interrupts */
> + aes_writel(dd, 0x33, TEGRA_AES_INT_ENB);
> +
> + /* this module is shared with the other hardware blocks
> + * and there have been cases where another user of the VDE
> + * has caused this irq to trigger */
> + enable_irq(dd->irq);
do the other users of the VDE cause this IRQ to trigger in error?
If so, they should be fixed. If not, and the IRQ line is shared by
h/w, then all users of the IRQ should request_irq with IRQF_SHARED,
and return IRQ_NONE if the IRQ wasn't for them. Either way, the IRQ
should be left enabled.
> + value = aes_readl(dd, TEGRA_AES_CMDQUE_CONTROL);
> + /* access SDRAM through AHB */
> + value &= ~TEGRA_AES_CMDQ_CTRL_SRC_STM_SEL_FIELD;
> + value &= ~TEGRA_AES_CMDQ_CTRL_DST_STM_SEL_FIELD;
> + value |= (TEGRA_AES_CMDQ_CTRL_SRC_STM_SEL_FIELD |
> + TEGRA_AES_CMDQ_CTRL_DST_STM_SEL_FIELD |
> + TEGRA_AES_CMDQ_CTRL_ICMDQEN_FIELD);
unnecessary parens
> + ret = wait_for_completion_timeout(&dd->op_complete,
> + msecs_to_jiffies(150));
alignment
> + total = dd->total;
> + rctx = ablkcipher_request_ctx(req);
> + ctx = crypto_ablkcipher_ctx(crypto_ablkcipher_reqtfm(req));
> + rctx->mode &= FLAGS_MODE_MASK;
> + dd->flags = (dd->flags & ~FLAGS_MODE_MASK) | rctx->mode;
> +
> + dd->iv = (u8 *)req->info;
> + dd->ivlen = crypto_ablkcipher_ivsize(crypto_ablkcipher_reqtfm(req));
cleaner:
tfm = crypto_ablkcipher_reqtfm(req);
...
ctx = crypto_ablkcipher_ctx(tfm);
...
dd->ivlen = crypto_ablkcipher_ivsize(tfm);
> + /* assign new context to device */
> + ctx->dd = dd;
> + dd->ctx = ctx;
> +
> + if (ctx->flags & FLAGS_NEW_KEY) {
> + /* copy the key */
> + memset(dd->ivkey_base, 0, AES_HW_KEY_TABLE_LENGTH_BYTES);
> + memcpy(dd->ivkey_base, ctx->key, ctx->keylen);
these really should be writes to mutually exclusive addresses.
> + addr_in = sg_dma_address(in_sg);
> + addr_out = sg_dma_address(out_sg);
> + dd->flags |= FLAGS_FAST;
> + count = min((int)sg_dma_len(in_sg), (int)dma_max);
use min_t
> +static irqreturn_t aes_irq(int irq, void *dev_id)
> +{
> + struct tegra_aes_dev *dd = (struct tegra_aes_dev *)dev_id;
> + u32 value = aes_readl(dd, TEGRA_AES_INTR_STATUS);
> +
> + dev_dbg(dd->dev, "irq_stat: 0x%x", value);
> + if (value & TEGRA_AES_INT_ERROR_MASK)
> + aes_writel(dd, TEGRA_AES_INT_ERROR_MASK, TEGRA_AES_INTR_STATUS);
> +
> + if (!(value & TEGRA_AES_ENGINE_BUSY_FIELD))
> + complete(&dd->op_complete);
> +
> + return IRQ_HANDLED;
return IRQ_NONE if there was no error and (value &
TEGRA_AES_ENGINE_BUSY_FIELD).
> + ret = aes_start_crypt(dd, (u32)dd->dma_buf_in,
> + (u32)dd->dma_buf_out, 1, dd->flags, true);
alignment
> +static int tegra_aes_rng_reset(struct crypto_rng *tfm, u8 *seed,
> + unsigned int slen)
alignment
> + /* copy the key to the key slot */
> + memset(dd->ivkey_base, 0, AES_HW_KEY_TABLE_LENGTH_BYTES);
> + memcpy(dd->ivkey_base, seed + DEFAULT_RNG_BLK_SZ, AES_KEYSIZE_128);
should be to mutually exclusive addresses
> + /* set seed to the aes hw slot */
> + memcpy(dd->buf_in, dd->iv, DEFAULT_RNG_BLK_SZ);
> + ret = aes_start_crypt(dd, (u32)dd->dma_buf_in,
> + dd->dma_buf_out, 1, FLAGS_CBC, false);
alignment
> +static int __devexit tegra_aes_remove(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct tegra_aes_dev *dd = platform_get_drvdata(pdev);
> + int i;
> +
> + if (!dd)
> + return -ENODEV;
> +
when would this condition be met?
> +/* init vector select */
> +#define TEGRA_AES_SECURE_IV_SELECT_SHIFT (10)
no parens
Kim
--
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
[PATCH] crypto: talitos - fix descriptor buffer access code
commit 3e721ae (crypto: talitos - handle descriptor not found in
error path) used the wrong offset method to access a channel's
descriptor buffer registers - the TALITOS_DESCBUF macro
doesn't take a channel argument. Fix it to use the
base chan[x].reg offset instead.
Signed-off-by: Kim Phillips
---
drivers/crypto/talitos.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index dbe76b5..98ffcc1 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -444,7 +444,7 @@ static void report_eu_error(struct device *dev, int ch, u32
desc_hdr)
int i;
if (!desc_hdr)
- desc_hdr = in_be32(priv->reg + TALITOS_DESCBUF(ch));
+ desc_hdr = in_be32(priv->chan[ch].reg + TALITOS_DESCBUF);
switch (desc_hdr & DESC_HDR_SEL0_MASK) {
case DESC_HDR_SEL0_AFEU:
--
1.7.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
