Re: [PATCH v2 2/2] hwrng: mxc-rnga - add driver support on boards with device tree

[Kim Phillips]

On Tue, 6 Mar 2018 00:21:00 +0200
Vladimir Zapolskiy <v...@mleia.com> wrote:

> The driver works well on i.MX31 powered boards with device description
> taken from board device tree, the only change to add to the driver is
> the missing OF device id, the affected list of included headers and
> indentation in platform driver struct are beautified a little.
> 
> Signed-off-by: Vladimir Zapolskiy <v...@mleia.com>
> ---
> Changes from v1 to v2:
> * a kernel for iMX boards is always built with multiplatform support,
>   thus CONFIG_OF guards were removed, thanks to Kim Phillips for review,

That's not necessarily the reason, e.g., of_match_table is available to
be assigned even if CONFIG_OF is not set.  Recall, I tested building
without CONFIG_OF by removing the SOC_IMX31 dependency in Kconfig, and
building with netwinder_defconfig as a base.

Nevertheless, this v2 is much easier to review without the ifdef
CONFIG_OF, so:

Reviewed-by: Kim Phillips <kim.phill...@arm.com>

Thanks,

Kim

p.s. my responses typically have lower latencies when I'm added to cc.

Re: [PATCH 2/2] hwrng: mxc-rnga - add driver support on boards with device tree

[Kim Phillips]

On Tue, 27 Feb 2018 18:53:08 +0200
Vladimir Zapolskiy <v...@mleia.com> wrote:

> On 02/27/2018 05:49 PM, Kim Phillips wrote:
> > On Mon, 26 Feb 2018 20:38:49 +0200
> > Vladimir Zapolskiy <v...@mleia.com> wrote:
> > 
> >> +#ifdef CONFIG_OF
> >> +static const struct of_device_id mxc_rnga_of_match[] = {
> >> +  { .compatible = "fsl,imx31-rnga", },
> >> +  { /* sentinel */ },
> >> +};
> >> +MODULE_DEVICE_TABLE(of, mxc_rnga_of_match);
> >> +#endif
> >> +
> >>  static struct platform_driver mxc_rnga_driver = {
> >>.driver = {
> >> - .name = "mxc_rnga",
> >> - },
> >> +  .name = "mxc_rnga",
> >> +  .of_match_table = of_match_ptr(mxc_rnga_of_match),
> > 
> > Does this build if CONFIG_OF is not set?
> 
> Definitely it is expected to be built, you can verify it directly or
> check of_match_ptr() macro definition from include/linux/of.h

Thanks, I verified it by removing the SOC_IMX31 dependency, and with
netwinder_defconfig as a base.  I also verified that the #ifdef
CONFIG_OF protecting the mxc_rnga_of_match definition is also not
needed.

Kim

Re: [PATCH 2/2] hwrng: mxc-rnga - add driver support on boards with device tree

[Kim Phillips]

On Mon, 26 Feb 2018 20:38:49 +0200
Vladimir Zapolskiy  wrote:

> +#ifdef CONFIG_OF
> +static const struct of_device_id mxc_rnga_of_match[] = {
> + { .compatible = "fsl,imx31-rnga", },
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, mxc_rnga_of_match);
> +#endif
> +
>  static struct platform_driver mxc_rnga_driver = {
>   .driver = {
> -.name = "mxc_rnga",
> -},
> + .name = "mxc_rnga",
> + .of_match_table = of_match_ptr(mxc_rnga_of_match),

Does this build if CONFIG_OF is not set?

Thanks,

Kim

Re: [PATCH RESEND 1/4] crypto: caam: add caam-dma node to SEC4.0 device tree binding

[Kim Phillips]

On Mon, 13 Nov 2017 09:44:06 +
Radu Andrei Alexe <radu.al...@nxp.com> wrote:

> On 11/10/2017 6:44 PM, Kim Phillips wrote:
> > On Fri, 10 Nov 2017 08:02:01 +
> > Radu Andrei Alexe <radu.al...@nxp.com> wrote:
> > 
> >> On 11/9/2017 6:34 PM, Kim Phillips wrote:
> >>> On Thu, 9 Nov 2017 11:54:13 +
> >>> Radu Andrei Alexe <radu.al...@nxp.com> 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: [PATCH RESEND 1/4] crypto: caam: add caam-dma node to SEC4.0 device tree binding

[Kim Phillips]

On Mon, 13 Nov 2017 08:32:24 +
Horia Geantă <horia.gea...@nxp.com> wrote:

> On 11/10/2017 6:44 PM, Kim Phillips wrote:
> > On Fri, 10 Nov 2017 08:02:01 +
> > Radu Andrei Alexe <radu.al...@nxp.com> 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

[Kim Phillips]

On Fri, 10 Nov 2017 08:02:01 +
Radu Andrei Alexe <radu.al...@nxp.com> wrote:

> On 11/9/2017 6:34 PM, Kim Phillips wrote:
> > On Thu, 9 Nov 2017 11:54:13 +
> > Radu Andrei Alexe <radu.al...@nxp.com> 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

[Kim Phillips]

On Thu, 9 Nov 2017 11:54:13 +
Radu Andrei Alexe <radu.al...@nxp.com> wrote:

> On 10/30/2017 4:24 PM, Kim Phillips wrote:
> > On Mon, 30 Oct 2017 15:46:51 +0200
> > Horia Geantă <horia.gea...@nxp.com> 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

[Kim Phillips]

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] crypto: talitos: fix driver init

[Kim Phillips]

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

Re: AEAD in TALITOS SEC1 versus TALITOS SEC2

[Kim Phillips]

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: AEAD in TALITOS SEC1 versus TALITOS SEC2

[Kim Phillips]

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: [PATCH v2 5/5] crypto: talitos: Add software backlog queue handling

[Kim Phillips]

On Thu, 19 Mar 2015 17:56:57 +0200
Horia Geantă horia.gea...@freescale.com wrote:

 On 3/18/2015 12:03 AM, Kim Phillips wrote:
  On Tue, 17 Mar 2015 19:58:55 +0200
  Horia Geantă horia.gea...@freescale.com wrote:
  
  On 3/17/2015 2:19 AM, Kim Phillips wrote:
  On Mon, 16 Mar 2015 12:02:51 +0200
  Horia Geantă horia.gea...@freescale.com 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's no free lunch. But it's cheaper.
 
  -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.
 
 jrp-outring[hw_idx].desc bus address can be used to find the sw_idx in
 O(1):
 
 dma_addr_t desc_base = dma_map_page(alloc_page(GFP_DMA),...);
 [...]
 sw_idx = (desc_base - jrp-outring[hw_idx].desc) / JD_SIZE;
 
 JD_SIZE would be 16 words (64B) - 13 words used for the h/w job
 descriptor, 3 words can be used

Re: [PATCH v2 5/5] crypto: talitos: Add software backlog queue handling

[Kim Phillips]

On Tue, 17 Mar 2015 19:58:55 +0200
Horia Geantă horia.gea...@freescale.com wrote:

 On 3/17/2015 2:19 AM, Kim Phillips wrote:
  On Mon, 16 Mar 2015 12:02:51 +0200
  Horia Geantă horia.gea...@freescale.com 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

[Kim Phillips]

On Mon, 16 Mar 2015 12:02:51 +0200
Horia Geantă horia.gea...@freescale.com 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 15/17] crypto: talitos - Implementation of SEC1

[Kim Phillips]

On Fri, 6 Mar 2015 17:42:26 +0100
Christophe Leroy christophe.le...@c-s.fr 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 christophe.le...@c-s.fr

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

[PATCH] crypto: powerpc - move files to fix build error

[Kim Phillips]

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 stockhau...@collogia.de
Signed-off-by: Kim Phillips kim.phill...@freescale.com
---
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: talitos: Add AES-XTS Support

[Kim Phillips]

On Fri, 6 Mar 2015 11:49:43 -0500
Martin Hicks m...@bork.org wrote:

 On Thu, Mar 5, 2015 at 7:16 PM, Kim Phillips kim.phill...@freescale.com 
 wrote:
  On Fri, 20 Feb 2015 12:00:10 -0500
  Martin Hicks m...@bork.org 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(((ecm6) | (aux25) |
 (cm1))  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 1/2] crypto: caamhash: - fix uninitialized edesc-sec4_sg_bytes field

[Kim Phillips]

On Fri, 6 Mar 2015 10:34:41 +0800
yanjiang@windriver.com wrote:

 From: Yanjiang Jin yanjiang@windriver.com
 
 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 CE,EE,ME  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 yanjiang@windriver.com
 ---

Acked-by: Kim Phillips kim.phill...@freescale.com

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 2/5] crypto: talitos: Remove MD5_BLOCK_SIZE

[Kim Phillips]

On Tue,  3 Mar 2015 08:21:34 -0500
Martin Hicks m...@bork.org wrote:

 This is properly defined in the md5 header file.
 
 Signed-off-by: Martin Hicks m...@bork.org
 ---

Acked-by: Kim Phillips kim.phill...@freescale.com

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

[Kim Phillips]

On Tue,  3 Mar 2015 08:21:33 -0500
Martin Hicks m...@bork.org 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 m...@bork.org
 ---

Acked-by: Kim Phillips kim.phill...@freescale.com

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

[Kim Phillips]

On Fri, 20 Feb 2015 12:00:10 -0500
Martin Hicks m...@bork.org 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 2/3] crypto: caam_rng: fix rng_unmap_ctx's DMA_UNMAP size problem

[Kim Phillips]

On Thu, 5 Mar 2015 10:52:37 +0800
yjin yanjiang@windriver.com wrote:

 On 2015年03月05日 02:36, Kim Phillips wrote:
  On Wed, 4 Mar 2015 13:33:22 +0800
  yjin yanjiang@windriver.com wrote:
 
  On 2015年03月04日 03:31, Kim Phillips wrote:
  On Tue, 3 Mar 2015 14:50:52 +0800
  yanjiang@windriver.com 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 1/3] crypto: caam: fix some compile warnings

[Kim Phillips]

On Thu, 5 Mar 2015 09:12:13 +0200
Cristian Stoica cristian.sto...@freescale.com 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 0/17] crypto: talitos - Add support for SEC1

[Kim Phillips]

On Thu, 5 Mar 2015 17:46:05 +0100
Christophe Leroy christophe.le...@c-s.fr 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 v2 5/5] crypto: talitos: Add software backlog queue handling

[Kim Phillips]

On Thu, 5 Mar 2015 11:35:23 +0200
Horia Geantă horia.gea...@freescale.com wrote:

 On 3/4/2015 2:23 AM, Kim Phillips wrote:
  On Tue,  3 Mar 2015 08:21:37 -0500
  Martin Hicks m...@bork.org 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: IPSec hmac(sha256) truncation bits length

[Kim Phillips]

On Wed, 4 Mar 2015 20:28:26 +0200
Nicolae Rosia nicolae.ro...@gmail.com wrote:

 On Wed, Mar 4, 2015 at 7:13 PM, Nicolae Rosia nicolae.ro...@gmail.com 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 2/3] crypto: caam_rng: fix rng_unmap_ctx's DMA_UNMAP size problem

[Kim Phillips]

On Wed, 4 Mar 2015 13:33:22 +0800
yjin yanjiang@windriver.com wrote:

 On 2015年03月04日 03:31, Kim Phillips wrote:
  On Tue, 3 Mar 2015 14:50:52 +0800
  yanjiang@windriver.com 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: [PATCH 1/3] crypto: caam: fix some compile warnings

[Kim Phillips]

On Wed, 4 Mar 2015 11:03:28 +0200
Cristian Stoica cristian.sto...@freescale.com 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

[Kim Phillips]

On Tue, 3 Mar 2015 14:50:52 +0800
yanjiang@windriver.com 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

[Kim Phillips]

On Tue, 3 Mar 2015 14:50:51 +0800
yanjiang@windriver.com 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

[Kim Phillips]

On Tue,  3 Mar 2015 08:21:37 -0500
Martin Hicks m...@bork.org 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

[PATCH] crypto: caam - don't emit ICV check failures to dmesg

[Kim Phillips]

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 kim.phill...@freescale.com
---
 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 01/16] crypto: caam - Remove unnecessary smp_read_barrier_depends()

[Kim Phillips]

On Thu, 13 Nov 2014 16:51:12 -0500
Pranith Kumar bobby.pr...@gmail.com 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 bobby.pr...@gmail.com
  ---
   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 kim.phill...@freescale.com

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 error reporting

[Kim Phillips]

On Wed, 5 Nov 2014 11:21:24 +0200
Cristian Stoica cristian.sto...@freescale.com 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 cristian.sto...@freescale.com
 ---

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] crypto: caam: fix error reporting

[Kim Phillips]

On Tue, 4 Nov 2014 10:57:57 +0200
Cristian Stoica cristian.sto...@freescale.com 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] crypto: caam: fix error reporting

[Kim Phillips]

On Mon, 3 Nov 2014 11:18:36 +0200
Cristian Stoica cristian.sto...@freescale.com wrote:

 On 10/31/2014 08:22 PM, Kim Phillips wrote:
  On Fri, 31 Oct 2014 18:57:33 +0200
  Cristian Stoica cristian.sto...@freescale.com 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

[Kim Phillips]

On Fri, 31 Oct 2014 18:57:33 +0200
Cristian Stoica cristian.sto...@freescale.com 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 2/2] crypto: caam - add support for rfc4106(gcm(aes))

[Kim Phillips]

On Fri, 10 Oct 2014 05:10:55 -0500
Ambarus Tudor-Dan-B38632 tudor.amba...@freescale.com wrote:

 On Thu, 9 Oct 2014 17:54:10 +0300
 Tudor Ambarus tudor.amba...@freescale.com 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 = built-in,
  +   .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)

[Kim Phillips]

On Fri, 10 Oct 2014 03:47:18 -0500
Ambarus Tudor-Dan-B38632 tudor.amba...@freescale.com wrote:

 On Thu, 9 Oct 2014 17:54:09 +0300
 Tudor Ambarus tudor.amba...@freescale.com 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 = built-in,
  +   .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 1/2] crypto: caam - add support for gcm(aes)

[Kim Phillips]

On Thu, 9 Oct 2014 17:54:09 +0300
Tudor Ambarus tudor.amba...@freescale.com 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 = built-in,
 + .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))

[Kim Phillips]

On Thu, 9 Oct 2014 17:54:10 +0300
Tudor Ambarus tudor.amba...@freescale.com 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 = built-in,
 + .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] crypto: talitos: Avoid excessive loops in softirq context

[Kim Phillips]

[adding Sandeep, Horia and netdev]

On Fri, 12 Sep 2014 09:39:12 +0200
Helmut Schaa helmut.sc...@googlemail.com wrote:

 On Fri, Sep 12, 2014 at 2:49 AM, Kim Phillips
 kim.phill...@freescale.com wrote:
  On Wed, 10 Sep 2014 10:34:47 +0200
  Helmut Schaa helmut.sc...@googlemail.com 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] crypto: talitos: Avoid excessive loops in softirq context

[Kim Phillips]

On Wed, 10 Sep 2014 10:34:47 +0200
Helmut Schaa helmut.sc...@googlemail.com 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 v2 00/12] crypto: caam - Add RTA descriptor creation library

[Kim Phillips]

On Wed, 3 Sep 2014 12:59:34 +0300
Horia Geantă horia.gea...@freescale.com wrote:

 On 8/16/2014 2:16 PM, Kim Phillips wrote:
  On Thu, 14 Aug 2014 15:54:22 +0300
  Horia Geanta horia.gea...@freescale.com 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: caam - fix DECO RSR polling

[Kim Phillips]

On Mon, 21 Jul 2014 16:03:21 +0300
Horia Geanta horia.gea...@freescale.com 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 horia.gea...@freescale.com
 ---
 Only compile-tested.
 Ruchika / Kim, please review / test.

Acked-by: Kim Phillips kim.phill...@freescale.com

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

[Kim Phillips]

On Fri, 11 Jul 2014 15:34:46 +0300
Horia Geanta horia.gea...@freescale.com 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 kim.phill...@freescale.com

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

[Kim Phillips]

On Mon, 21 Jul 2014 10:47:49 +0300
Horia Geantă horia.gea...@freescale.com wrote:

 On 7/19/2014 4:23 AM, Kim Phillips wrote:
  On Sat, 19 Jul 2014 02:51:30 +0300
  Horia Geantă horia.gea...@freescale.com wrote:
 
  On 7/19/2014 1:13 AM, Kim Phillips wrote:
  On Fri, 18 Jul 2014 19:37:17 +0300
  Horia Geanta horia.gea...@freescale.com 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 0/9] crypto: caam - Add RTA descriptor creation library

[Kim Phillips]

On Sat, 19 Jul 2014 02:51:30 +0300
Horia Geantă horia.gea...@freescale.com wrote:

 On 7/19/2014 1:13 AM, Kim Phillips wrote:
  On Fri, 18 Jul 2014 19:37:17 +0300
  Horia Geanta horia.gea...@freescale.com 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 v2] crypto: caam - fix memleak in caam_jr module

[Kim Phillips]

On Thu, 3 Jul 2014 15:07:50 +0300
Cristian Stoica cristian.sto...@freescale.com wrote:

 This patch fixes a memory leak that appears when caam_jr module is unloaded.
 
 Cc: sta...@vger.kernel.org # 3.13+
 Signed-off-by: Cristian Stoica cristian.sto...@freescale.com
 ---
  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] crypto:caam - Modify width of few read only registers

[Kim Phillips]

On Thu, 12 Jun 2014 04:56:14 -0500
Gupta Ruchika-R66431 ruchika.gu...@freescale.com 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

[Kim Phillips]

On Thu, 12 Jun 2014 23:52:06 +0530
Ruchika Gupta ruchika.gu...@freescale.com 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 ruchika.gu...@freescale.com
 ---
 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

[Kim Phillips]

On Mon, 9 Jun 2014 18:19:41 +0300
Dan Carpenter dan.carpen...@oracle.com wrote:

 The FIFOST_CONT_MASK define is cut and pasted twice so we can delete the
 second instance.
 
 Signed-off-by: Dan Carpenter dan.carpen...@oracle.com

Acked-by: Kim Phillips kim.phill...@freescale.com

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 - Modify width of few read only registers

[Kim Phillips]

On Tue, 29 Apr 2014 15:34:37 +0530
Ruchika Gupta ruchika.gu...@freescale.com 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

[PATCH] crypto: caam - reinitialize keys_fit_inline for decrypt and givencrypt

[Kim Phillips]

From: Vakul Garg va...@freescale.com

Re-initialize keys_fit_inline to avoid using its stale encrypt() shared
descriptor value prior to building descriptors for the decrypt() and
givencrypt() cases.

Signed-off-by: Vakul Garg va...@freescale.com
[reworded commit text, enhanced code readability]
Signed-off-by: Kim Phillips kim.phill...@freescale.com
---
 drivers/crypto/caam/caamalg.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index 5f89125..4bf7634 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -303,6 +303,7 @@ static int aead_null_set_sh_desc(struct crypto_aead *aead)
 * Job Descriptor and Shared Descriptors
 * must all fit into the 64-word Descriptor h/w Buffer
 */
+   keys_fit_inline = false;
if (DESC_AEAD_NULL_DEC_LEN + DESC_JOB_IO_LEN +
ctx-split_key_pad_len = CAAM_DESC_BYTES_MAX)
keys_fit_inline = true;
@@ -472,6 +473,7 @@ static int aead_set_sh_desc(struct crypto_aead *aead)
 * Job Descriptor and Shared Descriptors
 * must all fit into the 64-word Descriptor h/w Buffer
 */
+   keys_fit_inline = false;
if (DESC_AEAD_DEC_LEN + DESC_JOB_IO_LEN +
ctx-split_key_pad_len + ctx-enckeylen =
CAAM_DESC_BYTES_MAX)
@@ -527,6 +529,7 @@ static int aead_set_sh_desc(struct crypto_aead *aead)
 * Job Descriptor and Shared Descriptors
 * must all fit into the 64-word Descriptor h/w Buffer
 */
+   keys_fit_inline = false;
if (DESC_AEAD_GIVENC_LEN + DESC_JOB_IO_LEN +
ctx-split_key_pad_len + ctx-enckeylen =
CAAM_DESC_BYTES_MAX)
-- 
1.9.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] crypto:caam - Modify width of few read only registers

[Kim Phillips]

On Tue, 6 May 2014 23:09:15 -0500
Gupta Ruchika-R66431 ruchika.gu...@freescale.com wrote:

 Hi Kim,

Hi Ruchika,

  From: Kim Phillips [mailto:kim.phill...@freescale.com]
  Sent: Wednesday, May 07, 2014 2:02 AM
  
  On Tue, 6 May 2014 05:11:23 -0500
  Gupta Ruchika-R66431 ruchika.gu...@freescale.com wrote:
  
From: Kim Phillips [mailto:kim.phill...@freescale.com]
Sent: Friday, May 02, 2014 2:15 AM
   
On Tue, 29 Apr 2014 15:34:37 +0530
Ruchika Gupta ruchika.gu...@freescale.com 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.
   
why wouldn't they function correctly?
  
   The SEC IP guide states these registers as 2 32 bit registers. So
   register definition in
  
  I'm looking at LS2100A's SEC reference manual, it clearly has the CHAVID
  defined as one, single 64-bit register.  What are you looking at?
 
 In the first version of guide they were defined as 64 bit. They were later 
 changed to 32 bit once issue was reported while testing on emulator. Latest 
 guide of LS2100 has them modified. A register width column has also been 
 added in the memory map now.

I love how they try to cover up h/w bugs by amending the
documentation...

   crypto code should also have them defined as 32 bit registers. Defining
  them as 64 bit in this case would be incorrect.
  
   Endianness of the CAAM IP varies with core's endiannes. In ARM SoC's , 
   CAAM
  block is also little endian.  So in case the 2 - 32 bit registers are 
  treated
  as a 64 bit register, the result would be word swapped as compared to 
  powerpc
  platforms. As a result, the reads won't return the right result.
  
   For eg.
   For the 2 32 bit registers CHAVID_MS(at address 0x0) and
   CHAVID_LS(address 0x4) , if core reads them as 64 bit word,
  
   In powerpc (big endian) platform -
   CHAVID_MS would be available in most significant portion of the 64 bit
  word.
   CHAVID_LS would be the in least significant portion.
   This is as expected.
  
   In ARM (little endian) platform, 64 bit read would result in -
   CHAVID_MS in Least significant portion of the word and CHAVID_LS in
   the most significant location.
   This result is word swapped and  the value read wouldn't be correct.
  
  hmm, have you looked at using the DWT Double Word Transpose bit in the
  MCFGR?
 I am not able to locate this bit in MCFGR.

It's bit 19:  Double Word Transpose. Setting this bit affects
whether the two words within a Dword are transposed when a
double-word register is accessed, ...

 However there are few swapping options present in Job ring configuration and 
 QICTL registers. Are you referring to these ?

no.  Plus, those don't sound relevant to accessing CHAVID...

 Since these are 32 bit registers by nature, shouldn't we just treat them as 
 32 bit instead of enabling the swapping option .

depends on the definition of 'treat':  I'd rather still use the
superior 64-bit accessors on all possible arches, if we can get them
to work.

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 key inlining in AEAD shared descriptors

[Kim Phillips]

On Mon, 5 May 2014 22:39:09 -0500
Garg Vakul-B16394 va...@freescale.com wrote:

 Hi Kim

Hi Vakul,

  From: Kim Phillips [mailto:kim.phill...@freescale.com]
  Sent: Tuesday, May 06, 2014 12:07 AM
  
  On Sat, 3 May 2014 06:44:39 -0500
  Garg Vakul-B16394 va...@freescale.com wrote:
  
From: Kim Phillips [mailto:kim.phill...@freescale.com]
Sent: Saturday, May 03, 2014 5:40 AM
   
On Sun, 27 Apr 2014 11:26:14 -0400
Vakul Garg va...@freescale.com wrote:
   
 @@ -220,6 +220,8 @@ static int aead_null_set_sh_desc(struct
 crypto_aead
*aead)
   if (DESC_AEAD_NULL_ENC_LEN + DESC_JOB_IO_LEN +
   ctx-split_key_pad_len = CAAM_DESC_BYTES_MAX)
   keys_fit_inline = true;
 + else
 + keys_fit_inline = false;
   
Can we do the easier to read:
   
keys_fit_inline = false;
if (DESC_AEAD_NULL_ENC_LEN + DESC_JOB_IO_LEN +
ctx-split_key_pad_len = CAAM_DESC_BYTES_MAX)
keys_fit_inline = true;
   
?
  
   Why pre-init a variable with default value when it could be
  overwritten?
  
  why not?  compiler output doesn't differ in this regard.
 
 Agree that compiler output doesn't differ.
 But why depend upon compiler's optimization capability while writing code 
 when we can be explicit?

?  with optimizations turned off, the explicit 'else' *adds* an
extra CoF..

   I think that the form I submitted is equally easy to read.
  
  adding one line instead of two - less lines overall - more code on one
  screen - easier to read.
 
 I think that this is a matter of personal coding choice.
 Both the approaches are fine and compliant to kernel coding guidelines.

I disagree, for the reasons already mentioned above.

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

[Kim Phillips]

On Tue, 6 May 2014 05:11:23 -0500
Gupta Ruchika-R66431 ruchika.gu...@freescale.com wrote:

  From: Kim Phillips [mailto:kim.phill...@freescale.com]
  Sent: Friday, May 02, 2014 2:15 AM
  
  On Tue, 29 Apr 2014 15:34:37 +0530
  Ruchika Gupta ruchika.gu...@freescale.com 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.
  
  why wouldn't they function correctly?
 
 The SEC IP guide states these registers as 2 32 bit registers. So register 
 definition in

I'm looking at LS2100A's SEC reference manual, it clearly has the
CHAVID defined as one, single 64-bit register.  What are you looking
at?

 crypto code should also have them defined as 32 bit registers. Defining them 
 as 64 bit in this case would be incorrect.
 
 Endianness of the CAAM IP varies with core's endiannes. In ARM SoC's , CAAM 
 block is also little endian.  So in case the 2 - 32 bit registers are treated 
 as a 64 bit register, the result would be word swapped as compared to powerpc 
 platforms. As a result, the reads won't return the right result.
 
 For eg.
 For the 2 32 bit registers CHAVID_MS(at address 0x0) and CHAVID_LS(address 
 0x4) , if core reads them as 64 bit word, 
 
 In powerpc (big endian) platform -
 CHAVID_MS would be available in most significant portion of the 64 bit word.
 CHAVID_LS would be the in least significant portion.
 This is as expected.
 
 In ARM (little endian) platform, 64 bit read would result in -
 CHAVID_MS in Least significant portion of the word and 
 CHAVID_LS in the most significant location. 
 This result is word swapped and  the value read wouldn't be correct.

hmm, have you looked at using the DWT Double Word Transpose bit in
the MCFGR?

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 key inlining in AEAD shared descriptors

[Kim Phillips]

On Sun, 27 Apr 2014 11:26:14 -0400
Vakul Garg va...@freescale.com wrote:

 The variable 'keys_fit_inline' is initialised correctly to avoid using
 its stale value while creating shared descriptor for decryption and
 given-iv-encryption.

you mean givencrypt?  That's generate an IV and encrypt.

 @@ -220,6 +220,8 @@ static int aead_null_set_sh_desc(struct crypto_aead *aead)
   if (DESC_AEAD_NULL_ENC_LEN + DESC_JOB_IO_LEN +
   ctx-split_key_pad_len = CAAM_DESC_BYTES_MAX)
   keys_fit_inline = true;
 + else
 + keys_fit_inline = false;

Can we do the easier to read:

keys_fit_inline = false;
if (DESC_AEAD_NULL_ENC_LEN + DESC_JOB_IO_LEN +
ctx-split_key_pad_len = CAAM_DESC_BYTES_MAX)
keys_fit_inline = true;

?

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 - Modify width of few read only registers

[Kim Phillips]

On Tue, 29 Apr 2014 15:34:37 +0530
Ruchika Gupta ruchika.gu...@freescale.com 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. 

why wouldn't they function correctly?

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 - Define setbits32() and clrbits32() for ARM in the Freescale CAAM driver

[Kim Phillips]

On Tue, 29 Apr 2014 15:41:39 +0530
Ruchika Gupta ruchika.gu...@freescale.com wrote:

 The kernel defines setbits32() and clrbits32() macros only for
 Power-based architectures.  This patch modifies the Freescale CAAM
 driver to add macros for use on ARM architectures.
 
 Signed-off-by: Victoria Milhoan vicki.milh...@freescale.com
 Signed-off-by: Ruchika Gupta ruchika.gu...@freescale.com
 ---

Add these macros to arch/arm's io.h instead of duplicating their
definitions in drivers such as caam.

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: Parity Error on keys used for DES crypto test

[Kim Phillips]

On Wed, 23 Apr 2014 10:20:16 +0200
leroy christophe christophe.le...@c-s.fr wrote:

 I'm altering the Freescale Talitos Driver in order to support the SEC1 
 security engine, and I have a big issue with the DES test vectors in 
 testmgr.h:
 
 The Sec Engine reports key parity error.
 
 Looking at the keys defined in testmgr.h for DES3, it looks like there 
 is a real parity issue with the test vectors. A DES key is supposed to 
 have all bytes with an odd number of ones. It is not the case in the key 
 below. At least the second byte 0xC0 has an even number of ones.
 
 static struct cipher_testvec des3_ede_cbc_enc_tv_template[] = {
  { /* Generated from openssl */
  .key= \xE9\xC0\xFF\x2E\x76\x0B\x64\x24
\x44\x4D\x99\x5A\x12\xD6\x40\xC0
\xEA\xC2\x84\xE8\x14\x95\xDB\xE8,
 
 So, how can this test vector work ?

I'm not going to comment on the validity of the test key vector
other than to say that you can turn off key parity errors in the
SEC1 in the DEU Interrupt Control Register.

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: Does talitos driver support mpc8272 ?

[Kim Phillips]

On Tue, 1 Apr 2014 18:12:19 +0200
christophe leroy chler...@gmail.com wrote:

 Le 1 avr. 2014 00:55, Kim Phillips kim.phill...@freescale.com a écrit :
 
  On Sat, 29 Mar 2014 17:07:43 +
  christophe leroy chler...@gmail.com wrote:
 
   Hi
   Does the talitos driver support mpc 8272 or does any other driver do ?
 
  none upstream that I know of.
 
 But you said in a mail linked to the patch with which you introduced this
 sec1.0 compatible in the mpc8272ads.dts that there were drivers supporting
 it in the cryptodev-2.6 (
 http://linuxppc.10917.n7.nabble.com/PATCH-v2-update-crypto-node-definition-and-device-tree-instances-td36186.html
 )

I don't see where I say a SEC _1_ driver is in the upstream kernel
in that thread (incl. cryptodev-2.6).

 So does this exists or has existed in the past ?

I've never seen it upstream, no.

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: Does talitos driver support mpc8272 ?

[Kim Phillips]

On Sat, 29 Mar 2014 17:07:43 +
christophe leroy chler...@gmail.com wrote:

 Hi
 Does the talitos driver support mpc 8272 or does any other driver do ?

none upstream that I know of.

 i see that CONFIG_CRYPTO_DEV_TALITOS is not set in mpc8272ads config, and
 mpc8272ads dts does includeba compatible for fsl,sec1.0 while talitos
 driver match is fsl,sec2.0

my (dated) understanding was that talitos would need to be ported to
the SEC1, that e.g., uses a different ring buffer mechanism vs.
SEC2's Fetch FIFOs.

drivers/crypto/mxs-dcp.c and esp. the later drivers/crypto/sahara.c
share some bits; although since they're in i.MX parts, it's likely
the number of differences from a networking part like the 8272 is
higher than that of talitos.

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 ERA retrieval function

[Kim Phillips]

On Thu, 6 Feb 2014 11:41:28 +0200
Horia Geantă horia.gea...@freescale.com wrote:

 On 2/6/2014 10:27 AM, Alex Porosanu wrote:
  Signed-off-by: Alex Porosanu alexandru.poros...@freescale.com
  ---
drivers/crypto/caam/ctrl.c | 36 ++--
drivers/crypto/caam/ctrl.h |  2 +-
2 files changed, 11 insertions(+), 27 deletions(-)
 
 Reviewed-by: Horia Geanta horia.gea...@freescale.com
 
 fsl, sec-era has been in u-boot and in dt bindings for over a year.

yours maybe.  It's an optional property.

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 v4] crypto: caam - map src buffer before access

[Kim Phillips]

On Sat, 21 Sep 2013 14:26:35 +0530
Yashpal Dutta yashpal.du...@freescale.com wrote:

 KMap the buffers before copying trailing bytes during hmac into a session
 temporary buffer. This is required if pinned buffer from user-space is send
 during hmac and is safe even if hmac request is generated from within kernel.

it may be safe but it adversely affects performance for AF_ALG users,
no?

why does ocf-linux need this, and not AF_ALG?  Is a patch to ocf-linux
more appropriate here?

 Cc:sta...@vger.kernel.org

fyi, this violates the following rule in
Documentation/stable_kernel_rules.txt:

 - It or an equivalent fix must already exist in Linus' tree (upstream).

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: questions of crypto async api

[Kim Phillips]

On Mon, 8 Apr 2013 13:49:58 +
Hsieh, Che-Min chem...@qti.qualcomm.com wrote:

 Thanks for the answer.
 
 I have further question on the same subject.
 With regard to the commit in talitos.c, (attached at end of this mail),  
   the driver submits requests of same tfm to the same channel to ensure the 
 ordering.  
 
 Is it because the tfm context needs to be maintained from one operation to 
 next operation? Eg, aead_givencrypt() generates iv based on previous iv 
 result stored in tfm.

is that what the commit text says?

 If requests are sent to different channels dynamically. And driver at the 
 completion of a request from HW, reorders the requests completion callback, 
 what would happen? 

about the same thing as wrapping the driver with pcrypt?  why not
use the h/w to maintain ordering?

Kim

 Thanks in advance.
 
 Chemin
 
 
 
 commit 5228f0f79e983c2b39c202c75af901ceb0003fc1
 Author: Kim Phillips kim.phill...@freescale.com
 Date:   Fri Jul 15 11:21:38 2011 +0800
 
 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 kim.phill...@freescale.com
 Signed-off-by: Herbert Xu herb...@gondor.apana.org.au
 
 -Original Message-
 From: Kim Phillips [mailto:kim.phill...@freescale.com] 
 Sent: Friday, April 05, 2013 6:33 PM
 To: Hsieh, Che-Min
 Cc: linux-crypto@vger.kernel.org
 Subject: Re: questions of crypto async api
 
 On Thu, 4 Apr 2013 14:38:41 +
 Hsieh, Che-Min chem...@qti.qualcomm.com wrote:
 
  If a driver supports multiple instances of HW crypto engines, the order of 
  the request completion from HW can be different from the order of requests 
  submitted to different HW.  The 2nd request sent out to the 2nd HW instance 
  may take shorter time to complete than the first request for different HW 
  instance.  Is the driver responsible for re-ordering the completion 
  callout? Or the agents (such as IP protocol stack) are responsible for 
  reordering? How does pcrypt do it?
  
   Does it make sense for a transform to send multiple requests outstanding 
  to async crypto api?
 
 see:
 
 http://comments.gmane.org/gmane.linux.kernel.cryptoapi/5350
 
   Is scatterwalk_sg_next() preferred method over sg_next()?  Why?
 
 scatterwalk_* is the crypto subsystem's version of the function, so yes.
 
   sg_copy_to_buffer() and sg_copy_from_buffer() - 
  sg_copy_buffer()-sg_copy_buffer() - sg_miter_next()- sg_next() Sometimes 
  sg_copy_to_buffer() and sg_copy_from_buffer() in our driver do not copy the 
  whole list. We have to rewrite those functions by using 
  scattewalk_sg_next() to walk down the list. Is this the correct behavior?
 
 sounds like you're on the right track, although buffers shouldn't be being 
 copied that often, if at all.
 
 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: questions of crypto async api

[Kim Phillips]

On Thu, 4 Apr 2013 14:38:41 +
Hsieh, Che-Min chem...@qti.qualcomm.com wrote:

 If a driver supports multiple instances of HW crypto engines, the order of 
 the request completion from HW can be different from the order of requests 
 submitted to different HW.  The 2nd request sent out to the 2nd HW instance 
 may take shorter time to complete than the first request for different HW 
 instance.  Is the driver responsible for re-ordering the completion callout? 
 Or the agents (such as IP protocol stack) are responsible for reordering? How 
 does pcrypt do it?
 
  Does it make sense for a transform to send multiple requests outstanding to 
 async crypto api?

see:

http://comments.gmane.org/gmane.linux.kernel.cryptoapi/5350

  Is scatterwalk_sg_next() preferred method over sg_next()?  Why?

scatterwalk_* is the crypto subsystem's version of the function, so
yes.

  sg_copy_to_buffer() and sg_copy_from_buffer() - 
 sg_copy_buffer()-sg_copy_buffer() - sg_miter_next()- sg_next()
 Sometimes sg_copy_to_buffer() and sg_copy_from_buffer() in our driver do not 
 copy the whole list. We have to rewrite those functions by using 
 scattewalk_sg_next() to walk down the list. Is this the correct behavior?

sounds like you're on the right track, although buffers shouldn't be
being copied that often, if at all.

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 v2 2/2] v2 crypto: caam - static constify error data

[Kim Phillips]

checkstack reports report_deco_status(), report_ccb_status() as
particularly excessive stack users.  Move their lookup tables
off the stack and put them in .rodata.

Signed-off-by: Kim Phillips kim.phill...@freescale.com
---
v1 posted here:
http://article.gmane.org/gmane.linux.kernel.cryptoapi/7571
v2: made cha_id_list, err_id_list, and rng_err_id_list
static const char * const, instead of just static const.

 drivers/crypto/caam/error.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/crypto/caam/error.c b/drivers/crypto/caam/error.c
index 30b8f74..9f25f52 100644
--- a/drivers/crypto/caam/error.c
+++ b/drivers/crypto/caam/error.c
@@ -36,7 +36,7 @@ static void report_jump_idx(u32 status, char *outstr)
 
 static void report_ccb_status(u32 status, char *outstr)
 {
-   char *cha_id_list[] = {
+   static const char * const cha_id_list[] = {
,
AES,
DES,
@@ -51,7 +51,7 @@ static void report_ccb_status(u32 status, char *outstr)
ZUCE,
ZUCA,
};
-   char *err_id_list[] = {
+   static const char * const err_id_list[] = {
No error.,
Mode error.,
Data size error.,
@@ -69,7 +69,7 @@ static void report_ccb_status(u32 status, char *outstr)
Invalid CHA combination was selected,
Invalid CHA selected.,
};
-   char *rng_err_id_list[] = {
+   static const char * const rng_err_id_list[] = {
,
,
,
@@ -117,7 +117,7 @@ static void report_jump_status(u32 status, char *outstr)
 
 static void report_deco_status(u32 status, char *outstr)
 {
-   const struct {
+   static const struct {
u8 value;
char *error_text;
} desc_error_list[] = {
@@ -245,7 +245,7 @@ static void report_cond_code_status(u32 status, char 
*outstr)
 
 char *caam_jr_strstatus(char *outstr, u32 status)
 {
-   struct stat_src {
+   static const struct stat_src {
void (*report_ssed)(u32 status, char *outstr);
char *error;
} status_src[] = {
-- 
1.8.1.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

Re: authencesn compatibility problemn between software crypto and talitos driver

[Kim Phillips]

On Thu, 14 Mar 2013 12:21:20 +0200
Horia Geantă horia.gea...@freescale.com wrote:

 On 3/12/2013 10:57 PM, Chaoxing Lin wrote:
  The freescale crypto engine is still capable of doing AES-CBC + HMAC-SHAxxx 
  in one shot.
  DESC_HDR_TYPE_IPSEC_ESP may not able to achieve authencesn.
 
 Correct. And that's why I think reverting crypto: talitos - add IPsec 
 ESN support is the right thing to do.

This would need to happen for -stable anyway.

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 typo CRYPTO_AHASH

[Kim Phillips]

On Tue, 5 Mar 2013 14:33:16 +0100
Paul Bolle pebo...@tiscali.nl wrote:

 The Kconfig entry for CAAM's hash algorithm implementations has always
 selected CRYPTO_AHASH. But there's no corresponding Kconfig symbol.
 
 It seems it was intended to select CRYPTO_HASH, like other crypto
 drivers do. That would apparently (indirectly) select CRYPTO_HASH2,
 which would enable the ahash functionality this driver uses.
 
 Signed-off-by: Paul Bolle pebo...@tiscali.nl
 ---

Reviewed-by: Kim Phillips kim.phill...@freescale.com

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 - Added property fsl,sec-era in SEC4.0 device tree binding.

[Kim Phillips]

cc'ing devicetree-discuss.  Start using scripts/get_maintainers.pl.

On Wed, 23 Jan 2013 11:04:55 +0530
Vakul Garg va...@freescale.com wrote:

 +++ b/Documentation/devicetree/bindings/crypto/fsl-sec4.txt
 @@ -54,8 +54,13 @@ PROPERTIES
 - compatible
Usage: required
Value type: string
 -  Definition: Must include fsl,sec-v4.0. Also includes SEC
 -   ERA versions (optional) with which the device is compatible.
 +  Definition: Must include fsl,sec-v4.0
 +
 +   - fsl,sec-era
 +  Usage: required

what's the rationale for going from optional to required?

  EXAMPLE
   crypto@30 {
 - compatible = fsl,sec-v4.0, fsl,sec-era-v2.0;
 + compatible = fsl,sec-v4.0;
 + fsl,sec-era = 0x2;

use decimal notation

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][RFC] crypto: tcrypt - Ahash tests changed to run in parallel.

[Kim Phillips]

On Sat, 5 Jan 2013 17:14:13 +
Garg Vakul-B16394 b16...@freescale.com wrote:

  From: Jussi Kivilinna [mailto:jussi.kivili...@mbnet.fi]
  Sent: Saturday, January 05, 2013 9:56 PM
  
  Quoting Vakul Garg va...@freescale.com:
  
   This allows to test  run multiple parallel crypto ahash contexts.
   Each of the test vector under the ahash speed test template is started
   under a separate kthread.
  
  Why you want to do this? 
  
 In its current form, we cannot test multiple simultaneous crypto sessions 
 with tcrypt.
 Crypto offload hardware accelerators are usually capable of handling multiple 
 session in parallel.
 This patch allows to load such hardware.

tcrypt can currently test offload engines with multiple
single-session async requests.  So to test multiple _session_
requests, why isn't the change to tcrypt simply to alternate keys
in the request loop, as opposed to what this patch does, i.e.
adding separate kthreads?  The number of sessions/keys to alternate
can be added as a module parameter.

 Even if offload accelerators are not present, multiple crypto sessions can 
 execute in parallel on Multicore.

I think that's more in the area of PCRYPT.

  Does not this change make tcrypt give
  inconsistent results?
  
 
 Based on kernel scheduling of threads, this change can make tcrypt give 
 varying results in different runs.
 For consistent results, we can use existing synchronous mode crypto sessions.

see above.  I'm not sure, but I think multithreading tcrypt should
be left to the parallel crypto engine (PCRYPT) and Software async
crypto daemon (CRYPTD).

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 missing init of '.type' in AEAD algos.

[Kim Phillips]

On Mon, 7 Jan 2013 16:58:19 +0530
Vakul Garg va...@freescale.com wrote:

 Following AEAD algo templates are updated for '.type' initialization.
   (a) authenc(hmac(sha224),cbc(aes))
   (b) authenc(hmac(sha384),cbc(aes))
   (c) authenc(hmac(sha224),cbc(des3_ede))
   (d) authenc(hmac(sha384),cbc(des3_ede))
   (e) authenc(hmac(sha224),cbc(des))
   (f) authenc(hmac(sha384),cbc(des))
 
 Signed-off-by: Vakul Garg va...@freescale.com
 ---

Reviewed-by: Kim Phillips kim.phill...@freescale.com

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: Question about Talitos Linux driver for MPC885

[Kim Phillips]

On Fri, 23 Nov 2012 17:03:15 +0100
leroy christophe christophe.le...@c-s.fr wrote:

 Kim,
 
 Looking once more at talitos.h file, it looks like none of the values 
 corresponds to what is in the reference manuals.
 But it looks like in the talitos.h it is encoded as if it was with LSB 
 first. Hence for instance the TALITOS_MCR_SWR which is 0x01 instead of  
 0x0100.
 Then I'm wondering what/where the problem is.

the problem is they're not the same on all SECs - some SECs just
have different bitfield encodings.  One solution is to #ifdef
CONFIG_PPC_8xx for the values correct for the 885, and #else the
existing values.

Kim

 Christophe
 
 
 Le 23/11/2012 14:51, leroy christophe a écrit :
  Dear Kim,
 
  Thank you for you quick answer.
  I didn't have much time to look at it until this week unfortunatly.
 
  I have some more questions/observations below
 
  Le 26/09/2012 02:47, Kim Phillips a écrit :
  On Tue, 25 Sep 2012 10:45:17 +0200
  leroy christophe christophe.le...@c-s.fr wrote:
 
  I'm trying to use the Talitos crypto driver with the MPC885
  microcontroller. For the time being, it doesn't work.
  yes, they're not exactly compatible...
 
  The kernel startup blocks at the test of the DES function.
 
  I have added the following definition in the DTS file:
 
crypto@2 {
compatible = fsl,sec2.0;
  interesting, its called SEC Lite and its version register does
  indeed say 2.  I see it has a single channel FIFO instead of a ring,
  that the SEC v1.x (MPC185) used, so you probably don't have to
  rewrite talitos_submit.
  Good news, it was also my understanding.
 
reg = 0x2 0x8000;
interrupts = 1 1;
  I couldn't find the IRQ line in the MPC855RM - if there's no IRQ
  line, then that's a problem.
  Neither do I on the drawing, however in Table 52-1, there are 3 bits 
  in the CPTR register for defining the interrupt level of the SEC lite, 
  just like you do for the CPM and for the FEC.
  So I believe this should be ok ?
 
interrupt-parent = PIC;
fsl,num-channels = 1;
fsl,channel-fifo-len = 24;
fsl,exec-units-mask = 0x4c;
fsl,descriptor-types-mask = 0x301f;
  the descriptor type enumeration isn't uniform across into the mpc8xx
  SEC version, e.g., the SEC Lite doesn't support the ipsec_esp
  descriptor type, represented in mpc8xxx SEC versions as the second
  bit, so this descriptor-types-mask setting should be fixed to at
  least omit that since the driver checks for, and uses it if
  available.
 
  Is there anything wrong in what I did ? Or is there something else I
  should do ?
  might want to go through the defines in talitos.h, e.g,
  TALITOS_MCR_SWR is 0x1 on mpc8xxx vs. 0x1000 on
  mpc8xx (I suppose CONFIG_PPC_8xx can be used as the ifdef, btw).
  I'm surprised about this, I didn't check the talitos.h file, but had 
  checked the Reference Manual of the MPC 8272.
  I rechecked yesterday and the SWR bit is at the same place as on the 
  MPC885 which is different from what is defined in talitos.h
 
  Descriptor header and pointer formats, along with field locations,
  sizes, and enumerations may also be different.
 
  It also appears the SEC Lite doesn't support scatter-gather tables,
  which will make performance hurt for fragmented (large) packet sizes.
  Does it mean something has to be modified if the SW ?
 
  Kim
 
  Thanks,
  Christophe
 
 

--
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: Question about Talitos Linux driver for MPC885

[Kim Phillips]

On Fri, 23 Nov 2012 14:51:08 +0100
leroy christophe christophe.le...@c-s.fr wrote:

 Le 26/09/2012 02:47, Kim Phillips a écrit :
  On Tue, 25 Sep 2012 10:45:17 +0200
  leroy christophe christophe.le...@c-s.fr wrote:
 
  I'm trying to use the Talitos crypto driver with the MPC885
  microcontroller. For the time being, it doesn't work.
  yes, they're not exactly compatible...
 
  The kernel startup blocks at the test of the DES function.
 
  I have added the following definition in the DTS file:
 
crypto@2 {
compatible = fsl,sec2.0;
  interesting, its called SEC Lite and its version register does
  indeed say 2.  I see it has a single channel FIFO instead of a ring,
  that the SEC v1.x (MPC185) used, so you probably don't have to
  rewrite talitos_submit.
 Good news, it was also my understanding.
 
reg = 0x2 0x8000;
interrupts = 1 1;
  I couldn't find the IRQ line in the MPC855RM - if there's no IRQ
  line, then that's a problem.
 Neither do I on the drawing, however in Table 52-1, there are 3 bits in 
 the CPTR register for defining the interrupt level of the SEC lite, just 
 like you do for the CPM and for the FEC.

not sure how interrupt _level_ is relevant, but I'm going to assume
you're saying the SEC's IRQ line is integrated into that of the CPM
and/or FEC.

 So I believe this should be ok ?

Assuming the above, the 'interrupts' property above should
equal that of the CPM and/or FEC, and the IRQ_SHARED flag set in
request_irq (in both talitos and the CPM and/or FEC drivers).  The
talitos IRQ handler will return IRQ_NONE if none of the channel
done/error bits are set in the SEC.

interrupt-parent = PIC;
fsl,num-channels = 1;
fsl,channel-fifo-len = 24;
fsl,exec-units-mask = 0x4c;
fsl,descriptor-types-mask = 0x301f;
  the descriptor type enumeration isn't uniform across into the mpc8xx
  SEC version, e.g., the SEC Lite doesn't support the ipsec_esp
  descriptor type, represented in mpc8xxx SEC versions as the second
  bit, so this descriptor-types-mask setting should be fixed to at
  least omit that since the driver checks for, and uses it if
  available.
 
  Is there anything wrong in what I did ? Or is there something else I
  should do ?
  might want to go through the defines in talitos.h, e.g,
  TALITOS_MCR_SWR is 0x1 on mpc8xxx vs. 0x1000 on
  mpc8xx (I suppose CONFIG_PPC_8xx can be used as the ifdef, btw).
 I'm surprised about this, I didn't check the talitos.h file, but had 
 checked the Reference Manual of the MPC 8272.
 I rechecked yesterday and the SWR bit is at the same place as on the 
 MPC885 which is different from what is defined in talitos.h

right, for some odd reason the h/w designers decided to scramble
the bitfield definitions.

  Descriptor header and pointer formats, along with field locations,
  sizes, and enumerations may also be different.
 
  It also appears the SEC Lite doesn't support scatter-gather tables,
  which will make performance hurt for fragmented (large) packet sizes.
 Does it mean something has to be modified if the SW ?

yes, s/w will have to allocate a new contiguous memory buffer and
sg_copy_to_buffer() prior to crypto request submission to the h/w,
and sg_copy_from_buffer() after h/w completion.

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: Question about Talitos Linux driver for MPC885

[Kim Phillips]

On Tue, 25 Sep 2012 10:45:17 +0200
leroy christophe christophe.le...@c-s.fr wrote:

 I'm trying to use the Talitos crypto driver with the MPC885 
 microcontroller. For the time being, it doesn't work.

yes, they're not exactly compatible...

 The kernel startup blocks at the test of the DES function.
 
 I have added the following definition in the DTS file:
 
  crypto@2 {
  compatible = fsl,sec2.0;

interesting, its called SEC Lite and its version register does
indeed say 2.  I see it has a single channel FIFO instead of a ring,
that the SEC v1.x (MPC185) used, so you probably don't have to
rewrite talitos_submit.

  reg = 0x2 0x8000;
  interrupts = 1 1;

I couldn't find the IRQ line in the MPC855RM - if there's no IRQ
line, then that's a problem.

  interrupt-parent = PIC;
  fsl,num-channels = 1;
  fsl,channel-fifo-len = 24;
  fsl,exec-units-mask = 0x4c;
  fsl,descriptor-types-mask = 0x301f;

the descriptor type enumeration isn't uniform across into the mpc8xx
SEC version, e.g., the SEC Lite doesn't support the ipsec_esp
descriptor type, represented in mpc8xxx SEC versions as the second
bit, so this descriptor-types-mask setting should be fixed to at
least omit that since the driver checks for, and uses it if
available.

 Is there anything wrong in what I did ? Or is there something else I 
 should do ?

might want to go through the defines in talitos.h, e.g,
TALITOS_MCR_SWR is 0x1 on mpc8xxx vs. 0x1000 on
mpc8xx (I suppose CONFIG_PPC_8xx can be used as the ifdef, btw).

Descriptor header and pointer formats, along with field locations,
sizes, and enumerations may also be different.

It also appears the SEC Lite doesn't support scatter-gather tables,
which will make performance hurt for fragmented (large) packet sizes.

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 - Improved support of CAAM era retrieval.

[Kim Phillips]

missing patch version

On Wed, 19 Sep 2012 14:15:11 +0530
Vakul Garg va...@freescale.com wrote:

 CAAM era retrieval api caam_get_era() currently supports devices upto ERA-4.
 The CAAM era is looked up from a table mapping SECVID register to an ERA 
 number.
 Post ERA-6, era can be directly read from register CCBVID. This patch enhances
 api caam_get_era() to support additional pre ERA-6 devices in the mapping 
 table.
 For ERA-6 and later devices, it returns ERA directly by reading CCBVID.

wrap this text to ~75 cols please

 + if (ccb_vid-era)
 + return ccb_vid-era;
 + else

unnecessary else

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 - increase TRNG clocks per sample

[Kim Phillips]

we need to configure the TRNG to use more clocks per sample
to handle the two back-to-back 64KiB random descriptor requests
on higher frequency P5040s.

Signed-off-by: Kim Phillips kim.phill...@freescale.com
---
fixes booting on the p5040, but since p5040 support isn't in
3.6, this can wait for 3.7.

Also, Herbert, this:

http://www.mail-archive.com/linux-crypto@vger.kernel.org/msg07586.html

got dropped on the floor.

 drivers/crypto/caam/ctrl.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index 414ba20..bf20dd8 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -129,7 +129,7 @@ static int instantiate_rng(struct device *jrdev)
 
 /*
  * By default, the TRNG runs for 200 clocks per sample;
- * 800 clocks per sample generates better entropy.
+ * 1600 clocks per sample generates better entropy.
  */
 static void kick_trng(struct platform_device *pdev)
 {
@@ -144,9 +144,9 @@ static void kick_trng(struct platform_device *pdev)
 
/* put RNG4 into program mode */
setbits32(r4tst-rtmctl, RTMCTL_PRGM);
-   /* 800 clocks per sample */
+   /* 1600 clocks per sample */
val = rd_reg32(r4tst-rtsdctl);
-   val = (val  ~RTSDCTL_ENT_DLY_MASK) | (800  RTSDCTL_ENT_DLY_SHIFT);
+   val = (val  ~RTSDCTL_ENT_DLY_MASK) | (1600  RTSDCTL_ENT_DLY_SHIFT);
wr_reg32(r4tst-rtsdctl, val);
/* min. freq. count */
wr_reg32(r4tst-rtfrqmin, 400);
-- 
1.7.12


--
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 - change key gen functions to return signed int

[Kim Phillips]

The 'coccicheck fixes' commit added error return values yet
neglected to change the type from unsigned.

Signed-off-by: Kim Phillips kim.phill...@freescale.com
---
 drivers/crypto/caam/caamhash.c | 4 ++--
 drivers/crypto/caam/key_gen.c  | 2 +-
 drivers/crypto/caam/key_gen.h  | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index fbf3fe8..656cd4d 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -411,7 +411,7 @@ static int ahash_set_sh_desc(struct crypto_ahash *ahash)
return 0;
 }
 
-static u32 gen_split_hash_key(struct caam_hash_ctx *ctx, const u8 *key_in,
+static int gen_split_hash_key(struct caam_hash_ctx *ctx, const u8 *key_in,
  u32 keylen)
 {
return gen_split_key(ctx-jrdev, ctx-key, ctx-split_key_len,
@@ -420,7 +420,7 @@ static u32 gen_split_hash_key(struct caam_hash_ctx *ctx, 
const u8 *key_in,
 }
 
 /* Digest hash size if it is too large */
-static u32 hash_digest_key(struct caam_hash_ctx *ctx, const u8 *key_in,
+static int hash_digest_key(struct caam_hash_ctx *ctx, const u8 *key_in,
   u32 *keylen, u8 *key_out, u32 digestsize)
 {
struct device *jrdev = ctx-jrdev;
diff --git a/drivers/crypto/caam/key_gen.c b/drivers/crypto/caam/key_gen.c
index 0028881..2eb8e28 100644
--- a/drivers/crypto/caam/key_gen.c
+++ b/drivers/crypto/caam/key_gen.c
@@ -44,7 +44,7 @@ Split key 
generation---
 [06] 0x64260028fifostr: class2 mdsplit-jdk len=40
@0xffe04000
 */
-u32 gen_split_key(struct device *jrdev, u8 *key_out, int split_key_len,
+int gen_split_key(struct device *jrdev, u8 *key_out, int split_key_len,
  int split_key_pad_len, const u8 *key_in, u32 keylen,
  u32 alg_op)
 {
diff --git a/drivers/crypto/caam/key_gen.h b/drivers/crypto/caam/key_gen.h
index d95d290..c5588f6 100644
--- a/drivers/crypto/caam/key_gen.h
+++ b/drivers/crypto/caam/key_gen.h
@@ -12,6 +12,6 @@ struct split_key_result {
 
 void split_key_done(struct device *dev, u32 *desc, u32 err, void *context);
 
-u32 gen_split_key(struct device *jrdev, u8 *key_out, int split_key_len,
+int gen_split_key(struct device *jrdev, u8 *key_out, int split_key_len,
int split_key_pad_len, const u8 *key_in, u32 keylen,
u32 alg_op);
-- 
1.7.12


--
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 - coccicheck fixes

[Kim Phillips]

use true/false for bool, fix code alignment, and fix two allocs with
no test.

Signed-off-by: Kim Phillips kim.phill...@freescale.com
---
 drivers/crypto/caam/caamalg.c  | 16 
 drivers/crypto/caam/caamhash.c |  4 
 drivers/crypto/caam/key_gen.c  |  4 
 3 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index 6b48295..cf268b1 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -224,7 +224,7 @@ static int aead_set_sh_desc(struct crypto_aead *aead)
struct aead_tfm *tfm = aead-base.crt_aead;
struct caam_ctx *ctx = crypto_aead_ctx(aead);
struct device *jrdev = ctx-jrdev;
-   bool keys_fit_inline = 0;
+   bool keys_fit_inline = false;
u32 *key_jump_cmd, *jump_cmd;
u32 geniv, moveiv;
u32 *desc;
@@ -239,7 +239,7 @@ static int aead_set_sh_desc(struct crypto_aead *aead)
if (DESC_AEAD_ENC_LEN + DESC_JOB_IO_LEN +
ctx-split_key_pad_len + ctx-enckeylen =
CAAM_DESC_BYTES_MAX)
-   keys_fit_inline = 1;
+   keys_fit_inline = true;
 
/* aead_encrypt shared descriptor */
desc = ctx-sh_desc_enc;
@@ -297,7 +297,7 @@ static int aead_set_sh_desc(struct crypto_aead *aead)
if (DESC_AEAD_DEC_LEN + DESC_JOB_IO_LEN +
ctx-split_key_pad_len + ctx-enckeylen =
CAAM_DESC_BYTES_MAX)
-   keys_fit_inline = 1;
+   keys_fit_inline = true;
 
desc = ctx-sh_desc_dec;
 
@@ -365,7 +365,7 @@ static int aead_set_sh_desc(struct crypto_aead *aead)
if (DESC_AEAD_GIVENC_LEN + DESC_JOB_IO_LEN +
ctx-split_key_pad_len + ctx-enckeylen =
CAAM_DESC_BYTES_MAX)
-   keys_fit_inline = 1;
+   keys_fit_inline = true;
 
/* aead_givencrypt shared descriptor */
desc = ctx-sh_desc_givenc;
@@ -1354,10 +1354,10 @@ static struct aead_edesc *aead_giv_edesc_alloc(struct 
aead_givcrypt_request
contig = ~GIV_SRC_CONTIG;
if (dst_nents || iv_dma + ivsize != sg_dma_address(req-dst))
contig = ~GIV_DST_CONTIG;
-   if (unlikely(req-src != req-dst)) {
-   dst_nents = dst_nents ? : 1;
-   sec4_sg_len += 1;
-   }
+   if (unlikely(req-src != req-dst)) {
+   dst_nents = dst_nents ? : 1;
+   sec4_sg_len += 1;
+   }
if (!(contig  GIV_SRC_CONTIG)) {
assoc_nents = assoc_nents ? : 1;
src_nents = src_nents ? : 1;
diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index fbf3fe8..32aba7a 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -430,6 +430,10 @@ static u32 hash_digest_key(struct caam_hash_ctx *ctx, 
const u8 *key_in,
int ret = 0;
 
desc = kmalloc(CAAM_CMD_SZ * 6 + CAAM_PTR_SZ * 2, GFP_KERNEL | GFP_DMA);
+   if (!desc) {
+   dev_err(jrdev, unable to allocate key input memory\n);
+   return -ENOMEM;
+   }
 
init_job_desc(desc, 0);
 
diff --git a/drivers/crypto/caam/key_gen.c b/drivers/crypto/caam/key_gen.c
index 0028881..f5c0d56 100644
--- a/drivers/crypto/caam/key_gen.c
+++ b/drivers/crypto/caam/key_gen.c
@@ -54,6 +54,10 @@ u32 gen_split_key(struct device *jrdev, u8 *key_out, int 
split_key_len,
int ret = 0;
 
desc = kmalloc(CAAM_CMD_SZ * 6 + CAAM_PTR_SZ * 2, GFP_KERNEL | GFP_DMA);
+   if (!desc) {
+   dev_err(jrdev, unable to allocate key input memory\n);
+   return -ENOMEM;
+   }
 
init_job_desc(desc, 0);
 
-- 
1.7.12


--
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: Export gen_split_key symbol for other modules

[Kim Phillips]

On Thu, 23 Aug 2012 18:39:57 -0400
Ben Collins be...@servergy.com wrote:

 In 3.6-rc3, without this patch, the following error occurs with a modular 
 build:
 
 ERROR: gen_split_key [drivers/crypto/caam/caamhash.ko] undefined!
 ERROR: gen_split_key [drivers/crypto/caam/caamalg.ko] undefined!
 
 Signed-off-by: Ben Collins be...@servergy.com
 Cc: Herbert Xu herb...@gondor.apana.org.au
 Cc: Yuan Kang yuan.k...@freescale.com

Acked-by: Kim Phillips kim.phill...@freescale.com

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 - static constify error lookup tables

[Kim Phillips]

checkstack reports report_deco_status(), report_ccb_status() as
particularly excessive stack users.  Move error lookup tables
off the stack and put them in .rodata.

Signed-off-by: Kim Phillips kim.phill...@freescale.com
---
 drivers/crypto/caam/error.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/crypto/caam/error.c b/drivers/crypto/caam/error.c
index 9955ed9..497c1c2 100644
--- a/drivers/crypto/caam/error.c
+++ b/drivers/crypto/caam/error.c
@@ -36,7 +36,7 @@ static void report_jump_idx(u32 status, char *outstr)
 
 static void report_ccb_status(u32 status, char *outstr)
 {
-   char *cha_id_list[] = {
+   static const char *cha_id_list[] = {
,
AES,
DES,
@@ -51,7 +51,7 @@ static void report_ccb_status(u32 status, char *outstr)
ZUCE,
ZUCA,
};
-   char *err_id_list[] = {
+   static const char *err_id_list[] = {
No error.,
Mode error.,
Data size error.,
@@ -69,7 +69,7 @@ static void report_ccb_status(u32 status, char *outstr)
Invalid CHA combination was selected,
Invalid CHA selected.,
};
-   char *rng_err_id_list[] = {
+   static const char *rng_err_id_list[] = {
,
,
,
@@ -119,7 +119,7 @@ static void report_jump_status(u32 status, char *outstr)
 
 static void report_deco_status(u32 status, char *outstr)
 {
-   const struct {
+   static const struct {
u8 value;
char *error_text;
} desc_error_list[] = {
@@ -247,7 +247,7 @@ static void report_cond_code_status(u32 status, char 
*outstr)
 
 char *caam_jr_strstatus(char *outstr, u32 status)
 {
-   struct stat_src {
+   static const struct stat_src {
void (*report_ssed)(u32 status, char *outstr);
char *error;
} status_src[] = {
-- 
1.7.11.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 1/3 v2] crypto: caam - fix possible deadlock condition

[Kim Phillips]

On Mon, 30 Jul 2012 15:56:04 +0800
Herbert Xu herb...@gondor.apana.org.au wrote:

 On Fri, Jul 13, 2012 at 06:04:23PM -0500, Kim Phillips wrote:
  commit crypto: caam - use non-irq versions of spinlocks for job rings
  made two bad assumptions:
  
  (a) The caam_jr_enqueue lock isn't used in softirq context.
  Not true: jr_enqueue can be interrupted by an incoming net
  interrupt and the received packet may be sent for encryption,
  via caam_jr_enqueue in softirq context, thereby inducing a
  deadlock.
  
  This is evidenced when running netperf over an IPSec tunnel
  between two P4080's, with spinlock debugging turned on:
 
 All patches applied.  Thanks Kim.

Herbert, just wanted to make sure that at least the first patch in
this series were also applied to the crypto tree, i.e., for 3.6
release because it fixes a potential deadlock condition.  The second
patch should, too, but it's less important IMO.

These are the commits to cherry pick from cryptodev into crypto:

4a90507 crypto: caam - fix possible deadlock condition
95bcaa3 crypto: caam - add backward compatible string sec4.0 (optional)

The third is this one:

61bb86b crypto: caam - set descriptor sharing type to SERIAL

which technically is a performance improvement, and I'm ok with
leaving it out of v3.6.

Sorry for not making that clear from the start; sometimes it's tough
to tell what will make it into cryptodev prior to the merge window
opening (or rather as that time approaches).

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 IPsec ESN support

[Kim Phillips]

On Wed, 8 Aug 2012 18:46:45 +0300
Horia Geanta horia.gea...@freescale.com wrote:

 Support for ESNs (extended sequence numbers).
 Tested with strongswan on a P2020RDB back-to-back setup.
 Extracted from /etc/ipsec.conf:
 esp=aes-sha1-esn-modp4096!
 
 Signed-off-by: Horia Geanta horia.gea...@freescale.com
 ---

series:

Reviewed-by: Kim Phillips kim.phill...@freescale.com

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: talitos - consolidate cra_type assignments

[Kim Phillips]

lighten driver_algs[] by moving them to talitos_alg_alloc().

Signed-off-by: Kim Phillips kim.phill...@freescale.com
---
these two patches apply on top of all remaining talitos patches -
including XOR support - currently on linux-crypto (i.e., that
haven't been applied to the cryptodev tree).

 drivers/crypto/talitos.c | 29 +++--
 1 file changed, 3 insertions(+), 26 deletions(-)

diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index e0b097f..df4a266 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -2379,7 +2379,6 @@ static struct talitos_alg_template driver_algs[] = {
.cra_driver_name = authenc-hmac-sha1-cbc-aes-talitos,
.cra_blocksize = AES_BLOCK_SIZE,
.cra_flags = CRYPTO_ALG_TYPE_AEAD | CRYPTO_ALG_ASYNC,
-   .cra_type = crypto_aead_type,
.cra_aead = {
.setkey = aead_setkey,
.setauthsize = aead_setauthsize,
@@ -2405,7 +2404,6 @@ static struct talitos_alg_template driver_algs[] = {
.cra_driver_name = authenc-hmac-sha1-cbc-3des-talitos,
.cra_blocksize = DES3_EDE_BLOCK_SIZE,
.cra_flags = CRYPTO_ALG_TYPE_AEAD | CRYPTO_ALG_ASYNC,
-   .cra_type = crypto_aead_type,
.cra_aead = {
.setkey = aead_setkey,
.setauthsize = aead_setauthsize,
@@ -2432,7 +2430,6 @@ static struct talitos_alg_template driver_algs[] = {
.cra_driver_name = 
authenc-hmac-sha224-cbc-aes-talitos,
.cra_blocksize = AES_BLOCK_SIZE,
.cra_flags = CRYPTO_ALG_TYPE_AEAD | CRYPTO_ALG_ASYNC,
-   .cra_type = crypto_aead_type,
.cra_aead = {
.setkey = aead_setkey,
.setauthsize = aead_setauthsize,
@@ -2458,7 +2455,6 @@ static struct talitos_alg_template driver_algs[] = {
.cra_driver_name = 
authenc-hmac-sha224-cbc-3des-talitos,
.cra_blocksize = DES3_EDE_BLOCK_SIZE,
.cra_flags = CRYPTO_ALG_TYPE_AEAD | CRYPTO_ALG_ASYNC,
-   .cra_type = crypto_aead_type,
.cra_aead = {
.setkey = aead_setkey,
.setauthsize = aead_setauthsize,
@@ -2485,7 +2481,6 @@ static struct talitos_alg_template driver_algs[] = {
.cra_driver_name = 
authenc-hmac-sha256-cbc-aes-talitos,
.cra_blocksize = AES_BLOCK_SIZE,
.cra_flags = CRYPTO_ALG_TYPE_AEAD | CRYPTO_ALG_ASYNC,
-   .cra_type = crypto_aead_type,
.cra_aead = {
.setkey = aead_setkey,
.setauthsize = aead_setauthsize,
@@ -2511,7 +2506,6 @@ static struct talitos_alg_template driver_algs[] = {
.cra_driver_name = 
authenc-hmac-sha256-cbc-3des-talitos,
.cra_blocksize = DES3_EDE_BLOCK_SIZE,
.cra_flags = CRYPTO_ALG_TYPE_AEAD | CRYPTO_ALG_ASYNC,
-   .cra_type = crypto_aead_type,
.cra_aead = {
.setkey = aead_setkey,
.setauthsize = aead_setauthsize,
@@ -2538,7 +2532,6 @@ static struct talitos_alg_template driver_algs[] = {
.cra_driver_name = 
authenc-hmac-sha384-cbc-aes-talitos,
.cra_blocksize = AES_BLOCK_SIZE,
.cra_flags = CRYPTO_ALG_TYPE_AEAD | CRYPTO_ALG_ASYNC,
-   .cra_type = crypto_aead_type,
.cra_aead = {
.setkey = aead_setkey,
.setauthsize = aead_setauthsize,
@@ -2564,7 +2557,6 @@ static struct talitos_alg_template driver_algs[] = {
.cra_driver_name = 
authenc-hmac-sha384-cbc-3des-talitos,
.cra_blocksize = DES3_EDE_BLOCK_SIZE,
.cra_flags = CRYPTO_ALG_TYPE_AEAD | CRYPTO_ALG_ASYNC,
-   .cra_type = crypto_aead_type,
.cra_aead = {
.setkey = aead_setkey,
.setauthsize = aead_setauthsize,
@@ -2591,7 +2583,6 @@ static struct talitos_alg_template driver_algs[] = {
.cra_driver_name = 
authenc-hmac-sha512-cbc-aes-talitos,
.cra_blocksize = AES_BLOCK_SIZE,
.cra_flags = CRYPTO_ALG_TYPE_AEAD | CRYPTO_ALG_ASYNC,
-   .cra_type = crypto_aead_type,
.cra_aead

[PATCH 2/2] crypto: talitos - consolidate common cra_* assignments

[Kim Phillips]

the entry points and geniv definitions for all aead,
ablkcipher, and hash algorithms are all common; move them to a
single assignment in talitos_alg_alloc().

This assumes it's ok to assign a setkey() on non-hmac algs.

Signed-off-by: Kim Phillips kim.phill...@freescale.com
---

 drivers/crypto/talitos.c | 163 +--
 1 file changed, 17 insertions(+), 146 deletions(-)

diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c
index df4a266..f414983 100644
--- a/drivers/crypto/talitos.c
+++ b/drivers/crypto/talitos.c
@@ -2380,12 +2380,6 @@ static struct talitos_alg_template driver_algs[] = {
.cra_blocksize = AES_BLOCK_SIZE,
.cra_flags = CRYPTO_ALG_TYPE_AEAD | CRYPTO_ALG_ASYNC,
.cra_aead = {
-   .setkey = aead_setkey,
-   .setauthsize = aead_setauthsize,
-   .encrypt = aead_encrypt,
-   .decrypt = aead_decrypt,
-   .givencrypt = aead_givencrypt,
-   .geniv = built-in,
.ivsize = AES_BLOCK_SIZE,
.maxauthsize = SHA1_DIGEST_SIZE,
}
@@ -2405,12 +2399,6 @@ static struct talitos_alg_template driver_algs[] = {
.cra_blocksize = DES3_EDE_BLOCK_SIZE,
.cra_flags = CRYPTO_ALG_TYPE_AEAD | CRYPTO_ALG_ASYNC,
.cra_aead = {
-   .setkey = aead_setkey,
-   .setauthsize = aead_setauthsize,
-   .encrypt = aead_encrypt,
-   .decrypt = aead_decrypt,
-   .givencrypt = aead_givencrypt,
-   .geniv = built-in,
.ivsize = DES3_EDE_BLOCK_SIZE,
.maxauthsize = SHA1_DIGEST_SIZE,
}
@@ -2431,12 +2419,6 @@ static struct talitos_alg_template driver_algs[] = {
.cra_blocksize = AES_BLOCK_SIZE,
.cra_flags = CRYPTO_ALG_TYPE_AEAD | CRYPTO_ALG_ASYNC,
.cra_aead = {
-   .setkey = aead_setkey,
-   .setauthsize = aead_setauthsize,
-   .encrypt = aead_encrypt,
-   .decrypt = aead_decrypt,
-   .givencrypt = aead_givencrypt,
-   .geniv = built-in,
.ivsize = AES_BLOCK_SIZE,
.maxauthsize = SHA224_DIGEST_SIZE,
}
@@ -2456,12 +2438,6 @@ static struct talitos_alg_template driver_algs[] = {
.cra_blocksize = DES3_EDE_BLOCK_SIZE,
.cra_flags = CRYPTO_ALG_TYPE_AEAD | CRYPTO_ALG_ASYNC,
.cra_aead = {
-   .setkey = aead_setkey,
-   .setauthsize = aead_setauthsize,
-   .encrypt = aead_encrypt,
-   .decrypt = aead_decrypt,
-   .givencrypt = aead_givencrypt,
-   .geniv = built-in,
.ivsize = DES3_EDE_BLOCK_SIZE,
.maxauthsize = SHA224_DIGEST_SIZE,
}
@@ -2482,12 +2458,6 @@ static struct talitos_alg_template driver_algs[] = {
.cra_blocksize = AES_BLOCK_SIZE,
.cra_flags = CRYPTO_ALG_TYPE_AEAD | CRYPTO_ALG_ASYNC,
.cra_aead = {
-   .setkey = aead_setkey,
-   .setauthsize = aead_setauthsize,
-   .encrypt = aead_encrypt,
-   .decrypt = aead_decrypt,
-   .givencrypt = aead_givencrypt,
-   .geniv = built-in,
.ivsize = AES_BLOCK_SIZE,
.maxauthsize = SHA256_DIGEST_SIZE,
}
@@ -2507,12 +2477,6 @@ static struct talitos_alg_template driver_algs[] = {
.cra_blocksize = DES3_EDE_BLOCK_SIZE,
.cra_flags = CRYPTO_ALG_TYPE_AEAD | CRYPTO_ALG_ASYNC,
.cra_aead = {
-   .setkey = aead_setkey,
-   .setauthsize = aead_setauthsize,
-   .encrypt = aead_encrypt,
-   .decrypt = aead_decrypt,
-   .givencrypt = aead_givencrypt,
-   .geniv = built-in,
.ivsize = DES3_EDE_BLOCK_SIZE

Re: [PATCH 5/5] crypto: talitos - add IPsec ESN support

[Kim Phillips]

On Tue, 7 Aug 2012 08:34:07 -0500
Geanta Neag Horia Ioan-B05471 b05...@freescale.com wrote:

 On Tue, 7 Aug 2012 09:54:14 +0300, Geanta Neag Horia Ioan-B05471 
 b05...@freescale.com wrote:
  On Tue, 7 Aug 2012 04:47:51 +0300, Phillips Kim-R1AAHA 
  r1a...@freescale.com wrote:
 
  Also: if the algorithm name is really all that changes, can it be
  done more space-efficiently with some string manipulation in the
  algorithm registration loop?:  If registering an authenc
  algorithm, also register an authencesn version.
  
  Agree. Will come up with smth more elegant in v2.
  
 
 Looking closer at algorithm registration, talitos_alg_alloc uses shallow 
 copies
 of the templates in driver_algs[], so maybe sticking with the initial approach
 is safer ?

not sure what you mean by 'shallow copies', but the t_alg-algt =
*template assignment should be copying the entire struct crypto_alg.

There are lots more redundancies in driver_algs, e.g., all
.type == ALG_TYPE_AEADs have the same .cra_type and entry points.
So, ideally, as support for more algorithms are added, factoring out
the the common assignments into talitos_alg_alloc should keep the
size of driver_algs from exploding and becoming even more
unmanageable.

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 v6 0/8] Raid: enable talitos xor offload for improving performance

[Kim Phillips]

On Mon, 6 Aug 2012 18:10:15 +0800
qiang@freescale.com wrote:

 Changes in v6:
   - swap the order of original patch 3/6 and 4/6;
   - merge Ira's patch to reduce the size of original patch;
   - merge Ira's patch of carma in 8/8;
   - update documents and descriptions according to Ira's advice;

fwiw, I gave v5 a test-drive, setting up a RAID5 array on ramdisks
[1], and this patchseries, along with FSL_DMA  NET_DMA set seems
to be holding water, so this series gets my:

Tested-by: Kim Phillips kim.phill...@freescale.com

Thanks,

Kim

[1] mdadm --create --verbose --force /dev/md0 --level=raid5 --raid-devices=4 
/dev/ram[0123]

--
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 5/5] crypto: talitos - add IPsec ESN support

[Kim Phillips]

On Thu, 2 Aug 2012 17:16:41 +0300
Horia Geanta horia.gea...@freescale.com wrote:

 + .cra_name = authencesn(hmac(sha1),cbc(aes)),
 + .cra_driver_name = authenc-hmac-sha1-cbc-aes-talitos,

the driver name should be authencesn, too, no?

Also: if the algorithm name is really all that changes, can it be
done more space-efficiently with some string manipulation in the
algorithm registration loop?:  If registering an authenc
algorithm, also register an authencesn version.

The rest of the patchseries looks fine.

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 v3 0/4] Raid: enable talitos xor offload for improving performance

[Kim Phillips]

On Mon, 16 Jul 2012 12:07:16 +0800
Qiang Liu qiang@freescale.com wrote:

  drivers/crypto/Kconfig   |9 +
  drivers/crypto/talitos.c |  410 +++
  drivers/crypto/talitos.h |   53 ++
  drivers/dma/fsldma.c |  436 
 +-
  drivers/dma/fsldma.h |1 +
  5 files changed, 708 insertions(+), 201 deletions(-)

Given the pending talitos patches, this patchseries doesn't apply
cleanly:  can you rebase onto [1], which is based on Herbert's
cryptodev tree and contain's Horia's four patches?  They didn't get
any negative comments, so I assume eventually they will be applied,
and doing so will make Herbert's life easier.

I applied the series on Herbert's cryptodev, and while fsldma
already had this build warning:

drivers/dma/fsldma.c: In function 'fsl_dma_tx_submit':
drivers/dma/fsldma.c:636:2: warning: 'cookie' may be used uninitialized in this 
function [-Wuninitialized]

this patchseries introduces a new one:

drivers/dma/fsldma.c: In function 'dma_do_tasklet':
drivers/dma/fsldma.c:1134:16: warning: unused variable 'flags' 
[-Wunused-variable]

I'll wait for a re-post, after these and Ira's comments are
addressed, before trying to test again.

Thanks,

Kim

[1] git://git.freescale.com/crypto/cryptodev.git

--
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: caam - add backward compatible string sec4.0

[Kim Phillips]

From: Shengzhou Liu shengzhou@freescale.com

In some device trees of previous version, there were string fsl,sec4.0.
To be backward compatible with device trees, we first check fsl,sec-v4.0,
if it fails, then check for fsl,sec4.0.

Signed-off-by: Shengzhou Liu shengzhou@freescale.com

extended to include new hash and rng code, which was omitted from
the previous version of this patch during a rebase of the SDK
version.

Signed-off-by: Kim Phillips kim.phill...@freescale.com
---
 drivers/crypto/caam/caamhash.c | 14 ++
 drivers/crypto/caam/caamrng.c  |  7 +--
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index 895aaf2..dca3c19 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -1736,8 +1736,11 @@ static void __exit caam_algapi_hash_exit(void)
struct caam_hash_alg *t_alg, *n;
 
dev_node = of_find_compatible_node(NULL, NULL, fsl,sec-v4.0);
-   if (!dev_node)
-   return;
+   if (!dev_node) {
+   dev_node = of_find_compatible_node(NULL, NULL, fsl,sec4.0);
+   if (!dev_node)
+   return;
+   }
 
pdev = of_find_device_by_node(dev_node);
if (!pdev)
@@ -1812,8 +1815,11 @@ static int __init caam_algapi_hash_init(void)
int i = 0, err = 0;
 
dev_node = of_find_compatible_node(NULL, NULL, fsl,sec-v4.0);
-   if (!dev_node)
-   return -ENODEV;
+   if (!dev_node) {
+   dev_node = of_find_compatible_node(NULL, NULL, fsl,sec4.0);
+   if (!dev_node)
+   return -ENODEV;
+   }
 
pdev = of_find_device_by_node(dev_node);
if (!pdev)
diff --git a/drivers/crypto/caam/caamrng.c b/drivers/crypto/caam/caamrng.c
index e2bfe16..ccedb54 100644
--- a/drivers/crypto/caam/caamrng.c
+++ b/drivers/crypto/caam/caamrng.c
@@ -284,8 +284,11 @@ static int __init caam_rng_init(void)
struct caam_drv_private *priv;
 
dev_node = of_find_compatible_node(NULL, NULL, fsl,sec-v4.0);
-   if (!dev_node)
-   return -ENODEV;
+   if (!dev_node) {
+   dev_node = of_find_compatible_node(NULL, NULL, fsl,sec4.0);
+   if (!dev_node)
+   return -ENODEV;
+   }
 
pdev = of_find_device_by_node(dev_node);
if (!pdev)
-- 
1.7.11.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

[PATCH 1/3 v2] crypto: caam - fix possible deadlock condition

[Kim Phillips]

 context.
Since the lock is only used within softirq context, and this tasklet
is atomic, there is no need to do the additional work to disable
back halves.

This patch adds back-half disabling protection to caam_jr_enqueue
spin_locks to fix (a), and drops it from caam_jr_dequeue to fix (b).

Signed-off-by: Kim Phillips kim.phill...@freescale.com
---
The offending commit currently only resides in the cryptodev tree;
so this patch, and the rest in this series, only apply there.

Hopefully this makes sense.  Now where's that brown paper bag...

version 2: actually include the fix for dequeue/part (b).

 drivers/crypto/caam/jr.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
index 53c8c51..93d1407 100644
--- a/drivers/crypto/caam/jr.c
+++ b/drivers/crypto/caam/jr.c
@@ -63,7 +63,7 @@ static void caam_jr_dequeue(unsigned long devarg)
 
head = ACCESS_ONCE(jrp-head);
 
-   spin_lock_bh(jrp-outlock);
+   spin_lock(jrp-outlock);
 
sw_idx = tail = jrp-tail;
hw_idx = jrp-out_ring_read_index;
@@ -115,7 +115,7 @@ static void caam_jr_dequeue(unsigned long devarg)
jrp-tail = tail;
}
 
-   spin_unlock_bh(jrp-outlock);
+   spin_unlock(jrp-outlock);
 
/* Finally, execute user's callback */
usercall(dev, userdesc, userstatus, userarg);
@@ -236,14 +236,14 @@ int caam_jr_enqueue(struct device *dev, u32 *desc,
return -EIO;
}
 
-   spin_lock(jrp-inplock);
+   spin_lock_bh(jrp-inplock);
 
head = jrp-head;
tail = ACCESS_ONCE(jrp-tail);
 
if (!rd_reg32(jrp-rregs-inpring_avail) ||
CIRC_SPACE(head, tail, JOBR_DEPTH) = 0) {
-   spin_unlock(jrp-inplock);
+   spin_unlock_bh(jrp-inplock);
dma_unmap_single(dev, desc_dma, desc_size, DMA_TO_DEVICE);
return -EBUSY;
}
@@ -265,7 +265,7 @@ int caam_jr_enqueue(struct device *dev, u32 *desc,
 
wr_reg32(jrp-rregs-inpring_jobadd, 1);
 
-   spin_unlock(jrp-inplock);
+   spin_unlock_bh(jrp-inplock);
 
return 0;
 }
-- 
1.7.11.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 1/4] Talitos: move the data structure into header file

[Kim Phillips]

On Tue, 10 Jul 2012 13:56:46 +0800
Qiang Liu qiang@freescale.com wrote:

 Move the declaration of talitos data structure into talitos.h.
 
 Cc: Herbert Xu herb...@gondor.apana.org.au
 Cc: David S. Miller da...@davemloft.net
 Signed-off-by: Qiang Liu qiang@freescale.com
 ---

this patch has already been submitted [1].

Subsequent patches in this series also don't apply cleanly:  can
you rebase onto [2], which is based on Herbert's cryptodev tree and
contain's Horia's four patches, and re-send?

Also note that upstream talitos does not yet contain NAPI support
[3].

Thanks,

Kim

[1] http://www.mail-archive.com/linux-crypto@vger.kernel.org/msg07299.html
[2] git://git.freescale.com/crypto/cryptodev.git
[3] http://www.mail-archive.com/linux-crypto@vger.kernel.org/msg07289.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

[PATCH v2] ERA retrieval and printing for SEC device

[Kim Phillips]

From: Alex Porosanu alexandru.poros...@freescale.com

This patch adds support for retrieving and printing of
SEC ERA information. It is useful for knowing beforehand
what features exist from the SEC point of view on a
certain SoC. Only era-s 1 to 4 are currently supported;
other eras will appear as unknown.

Signed-off-by: Alex Porosanu alexandru.poros...@freescale.com

- rebased onto current cryptodev master
- made caam_eras static

Signed-off-by: Kim Phillips kim.phill...@freescale.com
---
 drivers/crypto/caam/ctrl.c | 41 +++--
 drivers/crypto/caam/ctrl.h | 13 +
 drivers/crypto/caam/regs.h |  6 ++
 3 files changed, 58 insertions(+), 2 deletions(-)
 create mode 100644 drivers/crypto/caam/ctrl.h

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index ac6abb3..414ba20 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -11,6 +11,7 @@
 #include jr.h
 #include desc_constr.h
 #include error.h
+#include ctrl.h
 
 static int caam_remove(struct platform_device *pdev)
 {
@@ -155,10 +156,44 @@ static void kick_trng(struct platform_device *pdev)
clrbits32(r4tst-rtmctl, RTMCTL_PRGM);
 }
 
+/**
+ * caam_get_era() - Return the ERA of the SEC on SoC, based
+ * on the SEC_VID register.
+ * Returns the ERA number (1..4) or -ENOTSUPP if the ERA is unknown.
+ * @caam_id - the value of the SEC_VID register
+ **/
+int caam_get_era(u64 caam_id)
+{
+   struct sec_vid *sec_vid = (struct sec_vid *)caam_id;
+   static const struct {
+   u16 ip_id;
+   u8 maj_rev;
+   u8 era;
+   } caam_eras[] = {
+   {0x0A10, 1, 1},
+   {0x0A10, 2, 2},
+   {0x0A12, 1, 3},
+   {0x0A14, 1, 3},
+   {0x0A14, 2, 4},
+   {0x0A16, 1, 4},
+   {0x0A11, 1, 4}
+   };
+   int i;
+
+   for (i = 0; i  ARRAY_SIZE(caam_eras); i++)
+   if (caam_eras[i].ip_id == sec_vid-ip_id 
+   caam_eras[i].maj_rev == sec_vid-maj_rev)
+   return caam_eras[i].era;
+
+   return -ENOTSUPP;
+}
+EXPORT_SYMBOL(caam_get_era);
+
 /* Probe routine for CAAM top (controller) level */
 static int caam_probe(struct platform_device *pdev)
 {
int ret, ring, rspec;
+   u64 caam_id;
struct device *dev;
struct device_node *nprop, *np;
struct caam_ctrl __iomem *ctrl;
@@ -276,9 +311,11 @@ static int caam_probe(struct platform_device *pdev)
/* Initialize queue allocator lock */
spin_lock_init(ctrlpriv-jr_alloc_lock);
 
+   caam_id = rd_reg64(topregs-ctrl.perfmon.caam_id);
+
/* Report alive for developer to see */
-   dev_info(dev, device ID = 0x%016llx\n,
-rd_reg64(topregs-ctrl.perfmon.caam_id));
+   dev_info(dev, device ID = 0x%016llx (Era %d)\n, caam_id,
+caam_get_era(caam_id));
dev_info(dev, job rings = %d, qi = %d\n,
 ctrlpriv-total_jobrs, ctrlpriv-qi_present);
 
diff --git a/drivers/crypto/caam/ctrl.h b/drivers/crypto/caam/ctrl.h
new file mode 100644
index 000..980d44e
--- /dev/null
+++ b/drivers/crypto/caam/ctrl.h
@@ -0,0 +1,13 @@
+/*
+ * CAAM control-plane driver backend public-level include definitions
+ *
+ * Copyright 2012 Freescale Semiconductor, Inc.
+ */
+
+#ifndef CTRL_H
+#define CTRL_H
+
+/* Prototypes for backend-level services exposed to APIs */
+int caam_get_era(u64 caam_id);
+
+#endif /* CTRL_H */
diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
index 6d9f1d9..3223fc6 100644
--- a/drivers/crypto/caam/regs.h
+++ b/drivers/crypto/caam/regs.h
@@ -117,6 +117,12 @@ struct jr_outentry {
 #define CHA_NUM_DECONUM_SHIFT  56
 #define CHA_NUM_DECONUM_MASK   (0xfull  CHA_NUM_DECONUM_SHIFT)
 
+struct sec_vid {
+   u16 ip_id;
+   u8 maj_rev;
+   u8 min_rev;
+};
+
 struct caam_perfmon {
/* Performance Monitor Registersf00-f9f */
u64 req_dequeued;   /* PC_REQ_DEQ - Dequeued Requests*/
-- 
1.7.11.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 01/20] crypto: caam - remove line continuations from ablkcipher_append_src_dst

[Kim Phillips]

presumably leftovers from possible macro development.

Signed-off-by: Kim Phillips kim.phill...@freescale.com
---
 drivers/crypto/caam/caamalg.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index 4eec389..5c10dc5 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -143,11 +143,11 @@ static inline void aead_append_ld_iv(u32 *desc, int 
ivsize)
  */
 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); \
+   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);
 }
 
 /*
-- 
1.7.11.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 02/20] crypto: caam - fix input job ring element dma mapping size

[Kim Phillips]

SEC4 h/w gets configured in 32- vs. 36-bit physical
addressing modes depending on the size of dma_addr_t,
which is not always equal to sizeof(u32 *).

Also fixed alignment of a dma_unmap call whilst in there.

Signed-off-by: Kim Phillips kim.phill...@freescale.com
---
 drivers/crypto/caam/jr.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
index 340fa32..6ce4c41 100644
--- a/drivers/crypto/caam/jr.c
+++ b/drivers/crypto/caam/jr.c
@@ -376,7 +376,7 @@ static int caam_jr_init(struct device *dev)
 
/* Setup rings */
inpbusaddr = dma_map_single(dev, jrp-inpring,
-   sizeof(u32 *) * JOBR_DEPTH,
+   sizeof(dma_addr_t) * JOBR_DEPTH,
DMA_BIDIRECTIONAL);
if (dma_mapping_error(dev, inpbusaddr)) {
dev_err(dev, caam_jr_init(): can't map input ring\n);
@@ -391,9 +391,9 @@ static int caam_jr_init(struct device *dev)
DMA_BIDIRECTIONAL);
if (dma_mapping_error(dev, outbusaddr)) {
dev_err(dev, caam_jr_init(): can't map output ring\n);
-   dma_unmap_single(dev, inpbusaddr,
-sizeof(u32 *) * JOBR_DEPTH,
-DMA_BIDIRECTIONAL);
+   dma_unmap_single(dev, inpbusaddr,
+sizeof(dma_addr_t) * JOBR_DEPTH,
+DMA_BIDIRECTIONAL);
kfree(jrp-inpring);
kfree(jrp-outring);
kfree(jrp-entinfo);
@@ -447,7 +447,7 @@ int caam_jr_shutdown(struct device *dev)
dma_unmap_single(dev, outbusaddr,
 sizeof(struct jr_outentry) * JOBR_DEPTH,
 DMA_BIDIRECTIONAL);
-   dma_unmap_single(dev, inpbusaddr, sizeof(u32 *) * JOBR_DEPTH,
+   dma_unmap_single(dev, inpbusaddr, sizeof(dma_addr_t) * JOBR_DEPTH,
 DMA_BIDIRECTIONAL);
kfree(jrp-outring);
kfree(jrp-inpring);
-- 
1.7.11.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 04/20] crypto: caam - fix descriptor length adjustments for protocol descriptors

[Kim Phillips]

init_desc, by always ORing with 1 for the descriptor header inclusion
into the descriptor length, and init_sh_desc_pdb, by always specifying
the descriptor length modification for the PDB via options, would not
allow for odd length PDBs to be embedded in the constructed descriptor
length.  Fix this by simply changing the OR to an addition.

also round-up pdb_bytes to the next SEC command unit size, to
allow for, e.g., optional packet header bytes that aren't a
multiple of CAAM_CMD_SZ.

Reported-by: Radu-Andrei BULIE radu.bu...@freescale.com
Signed-off-by: Kim Phillips kim.phill...@freescale.com
Cc: Yashpal Dutta yashpal.du...@freescale.com
---
 drivers/crypto/caam/desc_constr.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/caam/desc_constr.h 
b/drivers/crypto/caam/desc_constr.h
index 0d31e27..8e1056f 100644
--- a/drivers/crypto/caam/desc_constr.h
+++ b/drivers/crypto/caam/desc_constr.h
@@ -51,7 +51,7 @@ static inline void *sh_desc_pdb(u32 *desc)
 
 static inline void init_desc(u32 *desc, u32 options)
 {
-   *desc = options | HDR_ONE | 1;
+   *desc = (options | HDR_ONE) + 1;
 }
 
 static inline void init_sh_desc(u32 *desc, u32 options)
@@ -62,7 +62,7 @@ static inline void init_sh_desc(u32 *desc, u32 options)
 
 static inline void init_sh_desc_pdb(u32 *desc, u32 options, size_t pdb_bytes)
 {
-   u32 pdb_len = pdb_bytes / CAAM_CMD_SZ + 1;
+   u32 pdb_len = (pdb_bytes + CAAM_CMD_SZ - 1) / CAAM_CMD_SZ;
 
init_sh_desc(desc, (((pdb_len + 1)  HDR_START_IDX_SHIFT) + pdb_len) |
 options);
-- 
1.7.11.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 05/20] crypto: caam - add PDB (Protocol Descriptor Block) definitions

[Kim Phillips]

From: Hemant Agrawal hem...@freescale.com

Add a PDB header file to support building protocol descriptors.

Signed-off-by: Steve Cornelius s...@pobox.com
Signed-off-by: Hemant Agrawal hem...@freescale.com
Signed-off-by: Kim Phillips kim.phill...@freescale.com
---
 drivers/crypto/caam/desc.h |  16 --
 drivers/crypto/caam/pdb.h  | 401 +
 2 files changed, 401 insertions(+), 16 deletions(-)
 create mode 100644 drivers/crypto/caam/pdb.h

diff --git a/drivers/crypto/caam/desc.h b/drivers/crypto/caam/desc.h
index a17c295..af25e76 100644
--- a/drivers/crypto/caam/desc.h
+++ b/drivers/crypto/caam/desc.h
@@ -1585,20 +1585,4 @@
 #define NFIFOENTRY_PLEN_SHIFT  0
 #define NFIFOENTRY_PLEN_MASK   (0xFF  NFIFOENTRY_PLEN_SHIFT)
 
-/*
- * PDB internal definitions
- */
-
-/* IPSec ESP CBC Encap/Decap Options */
-#define PDBOPTS_ESPCBC_ARSNONE 0x00/* no antireplay window */
-#define PDBOPTS_ESPCBC_ARS32   0x40/* 32-entry antireplay window */
-#define PDBOPTS_ESPCBC_ARS64   0xc0/* 64-entry antireplay window */
-#define PDBOPTS_ESPCBC_IVSRC   0x20/* IV comes from internal random gen */
-#define PDBOPTS_ESPCBC_ESN 0x10/* extended sequence included */
-#define PDBOPTS_ESPCBC_OUTFMT  0x08/* output only decapsulation (decap) */
-#define PDBOPTS_ESPCBC_IPHDRSRC 0x08   /* IP header comes from PDB (encap) */
-#define PDBOPTS_ESPCBC_INCIPHDR 0x04   /* Prepend IP header to output frame */
-#define PDBOPTS_ESPCBC_IPVSN   0x02/* process IPv6 header */
-#define PDBOPTS_ESPCBC_TUNNEL  0x01/* tunnel mode next-header byte */
-
 #endif /* DESC_H */
diff --git a/drivers/crypto/caam/pdb.h b/drivers/crypto/caam/pdb.h
new file mode 100644
index 000..62950d2
--- /dev/null
+++ b/drivers/crypto/caam/pdb.h
@@ -0,0 +1,401 @@
+/*
+ * CAAM Protocol Data Block (PDB) definition header file
+ *
+ * Copyright 2008-2012 Freescale Semiconductor, Inc.
+ *
+ */
+
+#ifndef CAAM_PDB_H
+#define CAAM_PDB_H
+
+/*
+ * PDB- IPSec ESP Header Modification Options
+ */
+#define PDBHMO_ESP_DECAP_SHIFT 12
+#define PDBHMO_ESP_ENCAP_SHIFT 4
+/*
+ * Encap and Decap - Decrement TTL (Hop Limit) - Based on the value of the
+ * Options Byte IP version (IPvsn) field:
+ * if IPv4, decrement the inner IP header TTL field (byte 8);
+ * if IPv6 decrement the inner IP header Hop Limit field (byte 7).
+*/
+#define PDBHMO_ESP_DECAP_DEC_TTL   (0x02  PDBHMO_ESP_DECAP_SHIFT)
+#define PDBHMO_ESP_ENCAP_DEC_TTL   (0x02  PDBHMO_ESP_ENCAP_SHIFT)
+/*
+ * Decap - DiffServ Copy - Copy the IPv4 TOS or IPv6 Traffic Class byte
+ * from the outer IP header to the inner IP header.
+ */
+#define PDBHMO_ESP_DIFFSERV(0x01  PDBHMO_ESP_DECAP_SHIFT)
+/*
+ * Encap- Copy DF bit -if an IPv4 tunnel mode outer IP header is coming from
+ * the PDB, copy the DF bit from the inner IP header to the outer IP header.
+ */
+#define PDBHMO_ESP_DFBIT   (0x04  PDBHMO_ESP_ENCAP_SHIFT)
+
+/*
+ * PDB - IPSec ESP Encap/Decap Options
+ */
+#define PDBOPTS_ESP_ARSNONE0x00 /* no antireplay window */
+#define PDBOPTS_ESP_ARS32  0x40 /* 32-entry antireplay window */
+#define PDBOPTS_ESP_ARS64  0xc0 /* 64-entry antireplay window */
+#define PDBOPTS_ESP_IVSRC  0x20 /* IV comes from internal random gen */
+#define PDBOPTS_ESP_ESN0x10 /* extended sequence included */
+#define PDBOPTS_ESP_OUTFMT 0x08 /* output only decapsulation (decap) */
+#define PDBOPTS_ESP_IPHDRSRC   0x08 /* IP header comes from PDB (encap) */
+#define PDBOPTS_ESP_INCIPHDR   0x04 /* Prepend IP header to output frame */
+#define PDBOPTS_ESP_IPVSN  0x02 /* process IPv6 header */
+#define PDBOPTS_ESP_TUNNEL 0x01 /* tunnel mode next-header byte */
+#define PDBOPTS_ESP_IPV6   0x02 /* ip header version is V6 */
+#define PDBOPTS_ESP_DIFFSERV   0x40 /* copy TOS/TC from inner iphdr */
+#define PDBOPTS_ESP_UPDATE_CSUM 0x80 /* encap-update ip header checksum */
+#define PDBOPTS_ESP_VERIFY_CSUM 0x20 /* decap-validate ip header checksum */
+
+/*
+ * General IPSec encap/decap PDB definitions
+ */
+struct ipsec_encap_cbc {
+   u32 iv[4];
+};
+
+struct ipsec_encap_ctr {
+   u32 ctr_nonce;
+   u32 ctr_initial;
+   u32 iv[2];
+};
+
+struct ipsec_encap_ccm {
+   u32 salt; /* lower 24 bits */
+   u8 b0_flags;
+   u8 ctr_flags;
+   u16 ctr_initial;
+   u32 iv[2];
+};
+
+struct ipsec_encap_gcm {
+   u32 salt; /* lower 24 bits */
+   u32 rsvd1;
+   u32 iv[2];
+};
+
+struct ipsec_encap_pdb {
+   u8 hmo_rsvd;
+   u8 ip_nh;
+   u8 ip_nh_offset;
+   u8 options;
+   u32 seq_num_ext_hi;
+   u32 seq_num;
+   union {
+   struct ipsec_encap_cbc cbc;
+   struct ipsec_encap_ctr ctr;
+   struct ipsec_encap_ccm ccm;
+   struct ipsec_encap_gcm gcm;
+   };
+   u32 spi;
+   u16 rsvd1;
+   u16 ip_hdr_len;
+   u32 ip_hdr[0]; /* optional IP Header content */
+};
+
+struct ipsec_decap_cbc {
+   u32 rsvd

[PATCH 07/20] crypto: caam - remove jr register/deregister

[Kim Phillips]

From: Yuan Kang yuan.k...@freescale.com

remove caam_jr_register and caam_jr_deregister
to allow sharing of job rings.

Signed-off-by: Yuan Kang yuan.k...@freescale.com
Signed-off-by: Kim Phillips kim.phill...@freescale.com
---
 drivers/crypto/caam/caamalg.c | 30 ++
 drivers/crypto/caam/intern.h  |  2 --
 2 files changed, 2 insertions(+), 30 deletions(-)

diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg.c
index d0f8df1..a4e266f 100644
--- a/drivers/crypto/caam/caamalg.c
+++ b/drivers/crypto/caam/caamalg.c
@@ -2228,7 +2228,7 @@ static int caam_cra_init(struct crypto_tfm *tfm)
 * distribute tfms across job rings to ensure in-order
 * crypto request processing per tfm
 */
-   ctx-jrdev = priv-algapi_jr[(tgt_jr / 2) % priv-num_jrs_for_algapi];
+   ctx-jrdev = priv-jrdev[(tgt_jr / 2) % priv-total_jobrs];
 
/* copy descriptor header template value */
ctx-class1_alg_type = OP_TYPE_CLASS1_ALG | caam_alg-class1_alg_type;
@@ -2265,7 +2265,6 @@ static void __exit caam_algapi_exit(void)
struct device *ctrldev;
struct caam_drv_private *priv;
struct caam_crypto_alg *t_alg, *n;
-   int i, err;
 
dev_node = of_find_compatible_node(NULL, NULL, fsl,sec-v4.0);
if (!dev_node) {
@@ -2290,13 +2289,6 @@ static void __exit caam_algapi_exit(void)
list_del(t_alg-entry);
kfree(t_alg);
}
-
-   for (i = 0; i  priv-total_jobrs; i++) {
-   err = caam_jr_deregister(priv-algapi_jr[i]);
-   if (err  0)
-   break;
-   }
-   kfree(priv-algapi_jr);
 }
 
 static struct caam_crypto_alg *caam_alg_alloc(struct device *ctrldev,
@@ -2349,7 +2341,7 @@ static int __init caam_algapi_init(void)
 {
struct device_node *dev_node;
struct platform_device *pdev;
-   struct device *ctrldev, **jrdev;
+   struct device *ctrldev;
struct caam_drv_private *priv;
int i = 0, err = 0;
 
@@ -2370,24 +2362,6 @@ static int __init caam_algapi_init(void)
 
INIT_LIST_HEAD(priv-alg_list);
 
-   jrdev = kmalloc(sizeof(*jrdev) * priv-total_jobrs, GFP_KERNEL);
-   if (!jrdev)
-   return -ENOMEM;
-
-   for (i = 0; i  priv-total_jobrs; i++) {
-   err = caam_jr_register(ctrldev, jrdev[i]);
-   if (err  0)
-   break;
-   }
-   if (err  0  i == 0) {
-   dev_err(ctrldev, algapi error in job ring registration: %d\n,
-   err);
-   kfree(jrdev);
-   return err;
-   }
-
-   priv-num_jrs_for_algapi = i;
-   priv-algapi_jr = jrdev;
atomic_set(priv-tfm_count, -1);
 
/* register crypto algorithms the device supports */
diff --git a/drivers/crypto/caam/intern.h b/drivers/crypto/caam/intern.h
index a34be01..462be99 100644
--- a/drivers/crypto/caam/intern.h
+++ b/drivers/crypto/caam/intern.h
@@ -86,8 +86,6 @@ struct caam_drv_private {
 
/* which jr allocated to scatterlist crypto */
atomic_t tfm_count cacheline_aligned;
-   int num_jrs_for_algapi;
-   struct device **algapi_jr;
/* list of registered crypto algorithms (mk generic context handle?) */
struct list_head alg_list;
 
-- 
1.7.11.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 14/20] crypto: caam - assign 40-bit masks on SEC v5.0 and above

[Kim Phillips]

SEC v4.x were only 36-bit, SEC v5+ are 40-bit capable.
Also set a DMA mask for any job ring devices created.

Signed-off-by: Kim Phillips kim.phill...@freescale.com
---
 drivers/crypto/caam/ctrl.c | 9 +++--
 drivers/crypto/caam/jr.c   | 8 
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index 77557eb..9a2db9c 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -82,13 +82,18 @@ static int caam_probe(struct platform_device *pdev)
 
/*
 * Enable DECO watchdogs and, if this is a PHYS_ADDR_T_64BIT kernel,
-* 36-bit pointers in master configuration register
+* long pointers in master configuration register
 */
setbits32(topregs-ctrl.mcr, MCFGR_WDENABLE |
  (sizeof(dma_addr_t) == sizeof(u64) ? MCFGR_LONG_PTR : 0));
 
if (sizeof(dma_addr_t) == sizeof(u64))
-   dma_set_mask(dev, DMA_BIT_MASK(36));
+   if (of_device_is_compatible(nprop, fsl,sec-v5.0))
+   dma_set_mask(dev, DMA_BIT_MASK(40));
+   else
+   dma_set_mask(dev, DMA_BIT_MASK(36));
+   else
+   dma_set_mask(dev, DMA_BIT_MASK(32));
 
/*
 * Detect and enable JobRs
diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
index 6ce4c41..9f16b2c 100644
--- a/drivers/crypto/caam/jr.c
+++ b/drivers/crypto/caam/jr.c
@@ -503,6 +503,14 @@ int caam_jr_probe(struct platform_device *pdev, struct 
device_node *np,
dev_set_drvdata(jrdev, jrpriv);
ctrlpriv-jrdev[ring] = jrdev;
 
+   if (sizeof(dma_addr_t) == sizeof(u64))
+   if (of_device_is_compatible(np, fsl,sec-v5.0-job-ring))
+   dma_set_mask(jrdev, DMA_BIT_MASK(40));
+   else
+   dma_set_mask(jrdev, DMA_BIT_MASK(36));
+   else
+   dma_set_mask(jrdev, DMA_BIT_MASK(32));
+
/* Identify the interrupt */
jrpriv-irq = of_irq_to_resource(np, 0, NULL);
 
-- 
1.7.11.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