Re: [PATCH] crypto: tcrypt - set assoc in sg_init_aead()
On 11/14/2017 4:59 PM, Tudor Ambarus wrote: > Results better code readability. ^^ *in* better > > Signed-off-by: Tudor Ambarus Reviewed-by: Horia Geantă Horia
Hello Dear...
Hello Dear... I know that this message will come to you as a surprise. I hoped that you will not expose or betray this trust and confident that I am about to repose on you, my name is M, Shakour Rosarita. I am 19 years old Girl, female, from Tartu Syria, (never married) 61 kg, white in complexion, and I am currently living in the refugee camp here in Abidjan Ivory Coast, My late beloved father M,Shakour Hassin was a renowned businessman and owner of Natour Petrol Station in Syria, he was killed in a stampede riot in Tartu, Syria. When I got the news about my parents. I fled to a nearby country Jordan before I joined a ferry to Africa and came to Abidjan capital city Ivory Coast West Africa find safety here. I came in 2015 to Abidjan and I now live in refugee camps here as refugees are allowed freely to enter here without, My late father deposited (US$6.200.000.00m) My late father kept the money at the bank of Africa, I want you to help me transfer the money to your hand so that you will help me bring me into your country for my continue education. I sent my pictures here for you to see. Who I am seriously looking for a good-person in my life, so I want to hear from you soon and learn more about you. I am an open-minded and friendly girl to share a good time with you and have fun and enjoy on the go, bird watching, the rest of our lives. My Hobbies, tourism books, dance, music, soccer, tennis, swimming and cinema. I would just think about you, including your dose and doesn’t .I believe we will do well together, and live like one family. Thank you and God bless you, for you in your promise to help me here, looking forward to your reply by the grace of God and have a good day. I hope you send me your photos as well? I await your next reply in faith please reply through my private email at (mshakourrosarit...@gmail.com): Thanks. Ms Rosarita
[PATCH] crypto: cavium: fix memory leak on info
From: Colin Ian King
The object info is being leaked on an error return path, fix this
by setting ret to -ENOMEM and exiting via the request_cleanup path
that will free info.
Detected by CoverityScan, CID#1408439 ("Resource Leak")
Fixes: c694b233295b ("crypto: cavium - Add the Virtual Function driver for CPT")
Signed-off-by: Colin Ian King
---
drivers/crypto/cavium/cpt/cptvf_reqmanager.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/crypto/cavium/cpt/cptvf_reqmanager.c
b/drivers/crypto/cavium/cpt/cptvf_reqmanager.c
index 169e66231bcf..b0ba4331944b 100644
--- a/drivers/crypto/cavium/cpt/cptvf_reqmanager.c
+++ b/drivers/crypto/cavium/cpt/cptvf_reqmanager.c
@@ -459,7 +459,8 @@ int process_request(struct cpt_vf *cptvf, struct
cpt_request_info *req)
info->completion_addr = kzalloc(sizeof(union cpt_res_s), GFP_KERNEL);
if (unlikely(!info->completion_addr)) {
dev_err(&pdev->dev, "Unable to allocate memory for
completion_addr\n");
- return -ENOMEM;
+ ret = -ENOMEM;
+ goto request_cleanup;
}
result = (union cpt_res_s *)info->completion_addr;
--
2.14.1
[PATCH] dt-bindings: add device tree binding for Arm TrustZone CryptoCell crypto engine
The Arm TrustZone CryptoCell is a hardware security engine. This patch
adds DT bindings for its Rich Execution Environment crypto engine.
A driver supporting this device is already present in the staging tree.
Signed-off-by: Gilad Ben-Yossef
---
.../devicetree/bindings/crypto/arm-cryptocell.txt | 22 ++
1 file changed, 22 insertions(+)
create mode 100644 Documentation/devicetree/bindings/crypto/arm-cryptocell.txt
diff --git a/Documentation/devicetree/bindings/crypto/arm-cryptocell.txt
b/Documentation/devicetree/bindings/crypto/arm-cryptocell.txt
new file mode 100644
index 000..ccf8a101
--- /dev/null
+++ b/Documentation/devicetree/bindings/crypto/arm-cryptocell.txt
@@ -0,0 +1,22 @@
+Arm TrustZone CryptoCell cryptographic engine
+
+Required properties:
+- compatible: Should be "arm,cryptocell-712-ree".
+- reg: Base physical address of the engine and length of memory mapped region.
+- interrupts: Interrupt number for the device.
+
+Optional properties:
+- interrupt-parent: The phandle for the interrupt controller that services
+ interrupts for this device.
+- clocks: Reference to the crypto engine clock.
+- dma-coherent: Present if dma operations are coherent.
+
+Examples:
+
+ arm_cc712: arm_cc712@8000 {
+ compatible = "arm,cryptocell-712-ree";
+ interrupt-parent = <&intc>;
+ interrupts = < 0 30 4 >;
+ reg = < 0x8000 0x1 >;
+
+ };
--
2.7.4
[PATCH] crypto: keywrap - Add missing ULL suffixes for 64-bit constants
On 32-bit (e.g. with m68k-linux-gnu-gcc-4.1):
crypto/keywrap.c: In function ‘crypto_kw_decrypt’:
crypto/keywrap.c:191: warning: integer constant is too large for ‘long’ type
crypto/keywrap.c: In function ‘crypto_kw_encrypt’:
crypto/keywrap.c:224: warning: integer constant is too large for ‘long’ type
Fixes: 9e49451d7a15365d ("crypto: keywrap - simplify code")
Signed-off-by: Geert Uytterhoeven
---
crypto/keywrap.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/crypto/keywrap.c b/crypto/keywrap.c
index 744e35134c45f300..ec5c6a087c901501 100644
--- a/crypto/keywrap.c
+++ b/crypto/keywrap.c
@@ -188,7 +188,7 @@ static int crypto_kw_decrypt(struct blkcipher_desc *desc,
}
/* Perform authentication check */
- if (block.A != cpu_to_be64(0xa6a6a6a6a6a6a6a6))
+ if (block.A != cpu_to_be64(0xa6a6a6a6a6a6a6a6ULL))
ret = -EBADMSG;
memzero_explicit(&block, sizeof(struct crypto_kw_block));
@@ -221,7 +221,7 @@ static int crypto_kw_encrypt(struct blkcipher_desc *desc,
* Place the predefined IV into block A -- for encrypt, the caller
* does not need to provide an IV, but he needs to fetch the final IV.
*/
- block.A = cpu_to_be64(0xa6a6a6a6a6a6a6a6);
+ block.A = cpu_to_be64(0xa6a6a6a6a6a6a6a6ULL);
/*
* src scatterlist is read-only. dst scatterlist is r/w. During the
--
2.7.4
Re: [PATCH] crypto: keywrap - Add missing ULL suffixes for 64-bit constants
Am Mittwoch, 15. November 2017, 11:44:28 CET schrieb Geert Uytterhoeven:
Hi Geert,
> On 32-bit (e.g. with m68k-linux-gnu-gcc-4.1):
>
> crypto/keywrap.c: In function ‘crypto_kw_decrypt’:
> crypto/keywrap.c:191: warning: integer constant is too large for ‘long’
> type crypto/keywrap.c: In function ‘crypto_kw_encrypt’:
> crypto/keywrap.c:224: warning: integer constant is too large for ‘long’
> type
>
> Fixes: 9e49451d7a15365d ("crypto: keywrap - simplify code")
> Signed-off-by: Geert Uytterhoeven
Thank you
Reviewed-by: Stephan Mueller
Ciao
Stephan
Re: [PATCH] iommu/vt-d: Fix scatterlist offset handling
Hi Alex and all, Just wondering if you could merge Robin's patch for the next rc. From all our testing, this seems to be a solid fix and should be included in the stable releases as well. Thanks, Jacob On Mon, 6 Nov 2017 10:47:09 -0800 Jacob Pan wrote: > On Fri, 6 Oct 2017 16:43:09 +0200 > Joerg Roedel wrote: > > > On Tue, Oct 03, 2017 at 07:05:17PM +0100, Robin Murphy wrote: > > > Now, there are indeed plenty of drivers and subsystems which do > > > work on lists of explicitly single pages - anything doing some > > > variant of "addr = kmap_atomic(sg_page(sg)) + sg->offset;" is easy > > > to spot - but I don't think DMA API implementations are in a > > > position to make any kind of assumption; nearly all of them just > > > shut up and handle sg->length bytes from sg_phys(sg) without > > > questioning the caller, and I reckon that's exactly what they > > > should be doing. > > > > I agree with that, it is not explicitly forbidden to have an > > sg->offset > PAGE_SIZE and most IOMMU drivers handle this case. > > > > So this is a problem I'd like to see resolved in the VT-d driver > > too. If nobody comes up with a correct fix soon I'll apply this one > > and rip out the large-page support from __domain_mapping() to make > > it work. > > > Hi All, > > Just to give an update on the offline debugging of this issue. With > Robin's patch applied, I was able to reproduce the failure with > similar configuration that Jain helped to set up. > > I added trace prints just to see the map/unmap activities leading to > the DMAR fault. When fault occurs, the trace shows there is an unmap > to the offending iova pfn. So I think this is a separate problem than > Robin's patch is fixing. I think we should move forward to merge this > patch upstream and stable. The remaining problem is likely a race > condition between unmap and DMA activities. > > Here a brief extracted log, ee3d7 is the iova pfn in question. > #1. map sg pfn ee3d7 > -0 [076] 74124.154254: bprint: > __domain_mapping: vpfn:ee3d7, pgoff=2126, np:1, da:ee3d784e, > len:1464 , > ppfn:1849c9c > > > #2. unmap ee3d7000 > -0 [054] 74124.154301: bprint: > intel_unmap: Device :18:00.4 unmapping: pfn ee3d7-ee3d7 > -0 [076] 74124.154301: bprint: > __domain_mapping: lvlpg:1, nrpg 0, vpfn:ec2ff, ppfn:183221a, sg_res:0 > -0 [059] 74124.154302: bprint: > __domain_mapping: lvlpg:1, nrpg 0, vpfn:ee719, ppfn:c3e4dd, sg_res:0 > -0 [076] 74124.154302: bprint: > __domain_mapping: vpfn:f183b, pgoff=78, np:1, da:f183b04e, len:1464, > > #3. DMA to unmapped address ee3d7000, DMAR fault raised. > +2.952861] dmar_fault: 6 callbacks > suppressed +0.02] DMAR: DRHD: handling fault status reg > 2 +0.005588] turning tracing > off +0.003592] DMAR: [DMA Write] Request device [18:00.4] fault addr > ee3d7000 [fault reason 05] PTE Write access is not set > > > > > -0 [000] 74124.156906: bputs: > 0xb259916bs: turning tracing off > > > Thanks, > > Jacob > > > Speaking of __domain_mapping(), this function is a big > > unmaintainable mess which should be split and rewritten. A clean > > and maintainable rewrite can alse re-add the large-page support. > > > > > > Regards, > > > > Joerg > > > > ___ > > iommu mailing list > > io...@lists.linux-foundation.org > > https://lists.linuxfoundation.org/mailman/listinfo/iommu > > [Jacob Pan] [Jacob Pan]
Re: [PATCH] dt-bindings: add device tree binding for Arm TrustZone CryptoCell crypto engine
On Wed, Nov 15, 2017 at 01:05:17PM +, Gilad Ben-Yossef wrote:
> The Arm TrustZone CryptoCell is a hardware security engine. This patch
> adds DT bindings for its Rich Execution Environment crypto engine.
>
> A driver supporting this device is already present in the staging tree.
>
> Signed-off-by: Gilad Ben-Yossef
> ---
> .../devicetree/bindings/crypto/arm-cryptocell.txt | 22
> ++
> 1 file changed, 22 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/crypto/arm-cryptocell.txt
>
> diff --git a/Documentation/devicetree/bindings/crypto/arm-cryptocell.txt
> b/Documentation/devicetree/bindings/crypto/arm-cryptocell.txt
> new file mode 100644
> index 000..ccf8a101
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/crypto/arm-cryptocell.txt
> @@ -0,0 +1,22 @@
> +Arm TrustZone CryptoCell cryptographic engine
> +
> +Required properties:
> +- compatible: Should be "arm,cryptocell-712-ree".
> +- reg: Base physical address of the engine and length of memory mapped
> region.
> +- interrupts: Interrupt number for the device.
> +
> +Optional properties:
> +- interrupt-parent: The phandle for the interrupt controller that services
> + interrupts for this device.
> +- clocks: Reference to the crypto engine clock.
> +- dma-coherent: Present if dma operations are coherent.
> +
> +Examples:
> +
> + arm_cc712: arm_cc712@8000 {
crypto@...
With that,
Acked-by: Rob Herring
> + compatible = "arm,cryptocell-712-ree";
> + interrupt-parent = <&intc>;
> + interrupts = < 0 30 4 >;
> + reg = < 0x8000 0x1 >;
> +
> + };
> --
> 2.7.4
>
Re: [PATCH v2 2/2] chcr: Add support for Inline IPSec
Hi Atul,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on cryptodev/master]
[also build test ERROR on next-20171115]
[cannot apply to v4.14]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Atul-Gupta/cxgb4-Add-support-for-Inline-IPSec-Tx/20171112-012558
base:
https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
config: x86_64-randconfig-g0-11160917 (attached as .config)
compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
All errors (new ones prefixed by >>):
drivers/crypto/chelsio/chcr_core.o: In function `chcr_uld_tx_handler':
>> drivers/crypto/chelsio/chcr_core.c:195: undefined reference to
>> `chcr_ipsec_xmit'
drivers/crypto/chelsio/chcr_core.o: In function `chcr_uld_add':
>> drivers/crypto/chelsio/chcr_core.c:169: undefined reference to
>> `chcr_add_xfrmops'
vim +195 drivers/crypto/chelsio/chcr_core.c
152
153 static void *chcr_uld_add(const struct cxgb4_lld_info *lld)
154 {
155 struct uld_ctx *u_ctx;
156
157 /* Create the device and add it in the device list */
158 if (!(lld->ulp_crypto & ULP_CRYPTO_LOOKASIDE))
159 return ERR_PTR(-EOPNOTSUPP);
160
161 /* Create the device and add it in the device list */
162 u_ctx = kzalloc(sizeof(*u_ctx), GFP_KERNEL);
163 if (!u_ctx) {
164 u_ctx = ERR_PTR(-ENOMEM);
165 goto out;
166 }
167 u_ctx->lldi = *lld;
168 if (lld->crypto & ULP_CRYPTO_IPSEC_INLINE)
> 169 chcr_add_xfrmops(lld);
170 out:
171 return u_ctx;
172 }
173
174 int chcr_uld_rx_handler(void *handle, const __be64 *rsp,
175 const struct pkt_gl *pgl)
176 {
177 struct uld_ctx *u_ctx = (struct uld_ctx *)handle;
178 struct chcr_dev *dev = u_ctx->dev;
179 const struct cpl_fw6_pld *rpl = (struct cpl_fw6_pld *)rsp;
180
181 if (rpl->opcode != CPL_FW6_PLD) {
182 pr_err("Unsupported opcode\n");
183 return 0;
184 }
185
186 if (!pgl)
187 work_handlers[rpl->opcode](dev, (unsigned char
*)&rsp[1]);
188 else
189 work_handlers[rpl->opcode](dev, pgl->va);
190 return 0;
191 }
192
193 int chcr_uld_tx_handler(struct sk_buff *skb, struct net_device *dev)
194 {
> 195 return chcr_ipsec_xmit(skb, dev);
196 }
197
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
Re: [PATCH RESEND 4/4] dma: caam: add dma memcpy driver
On Wed, Nov 08, 2017 at 02:36:31PM +, Radu Andrei Alexe wrote:
> On 10/31/2017 2:01 PM, Vinod Koul wrote:
> > On Mon, Oct 30, 2017 at 03:46:54PM +0200, Horia Geantă wrote:
> >> +/*
> >> + * caam support for SG DMA
> >> + *
> >> + * Copyright 2016 Freescale Semiconductor, Inc
> >> + * Copyright 2017 NXP
> >> + */
> >
> > that is interesting, no license text?
> >
>
> Thanks for the catch. The next patch will contain the full license text.
or as is the "new" practice, you may used SPDX tags
> >> +#include "../crypto/caam/regs.h"
> >> +#include "../crypto/caam/jr.h"
> >> +#include "../crypto/caam/error.h"
> >> +#include "../crypto/caam/intern.h"
> >> +#include "../crypto/caam/desc_constr.h"
> >
> > ah that puts very hard assumptions on locations of the subsystems. Please do
> > not do that and move the required stuff into common header location in
> > include/
> >
>
> Unfortunately this is not possible. The functionality used by CAAM DMA
> from the CAAM subsystem should not be exported to be used by other
> modules. It is because this driver is so tightly coupled with the CAAM
> driver(s) that it needs access to it's 'internal' functionality (that
> should not be otherwise shared with anyone).
Which other driver would be interested in your CAM implementation except
you. Realistically speaking none, so it is perfectly fine to do so in a
header at a right place!
> >> +struct caam_dma_sh_desc {
> >
> > sh?
> >
>
> It means "shared". It is obvious to anyone who is familiar with the CAAM
> system.
Unfortunately I am not. As a patch writer you have the onus to explain it to
people who live outside the CAAM world
> >> +static struct dma_device *dma_dev;
> >> +static struct caam_dma_sh_desc *dma_sh_desc;
> >> +static LIST_HEAD(dma_ctx_list);
> >
> > why do you need so many globals?
> >
>
> How many globals are too many? I can group them in a common structure.
> But I'm not sure how would that help.
I prefer none. Make sure the struct instance is not global and allocated in
your driver probe routine.
> >> +static dma_cookie_t caam_dma_tx_submit(struct dma_async_tx_descriptor *tx)
> >> +{
> >> + struct caam_dma_edesc *edesc = NULL;
> >> + struct caam_dma_ctx *ctx = NULL;
> >> + dma_cookie_t cookie;
> >> +
> >> + edesc = container_of(tx, struct caam_dma_edesc, async_tx);
> >> + ctx = container_of(tx->chan, struct caam_dma_ctx, chan);
> >> +
> >> + spin_lock_bh(&ctx->edesc_lock);
> >
> > why _bh, i didnt see any irqs or tasklets here which is actually odd, so
> > what is going on
> >
>
> The tasklet is hidden inside the JR driver from the CAAM subsystem (see
> drivers/crypto/caam/jr.c). The function caam_dma_done(), which is called
> from the JR tasklet handler, also uses the spin_lock. I need to disable
> bottom half to make sure that I don't run into a deadlock when I'm
> preempted.
This would be the right time to explain why. I understand that your subsystem
has tightly coupled DMA but that doesn't really mean the DMA controller should
not have its own IRQ and bh. This makes things harder to review and follow.
> >> + cookie = dma_cookie_assign(tx);
> >> + list_add_tail(&edesc->node, &ctx->submit_q);
> >> +
> >> + spin_unlock_bh(&ctx->edesc_lock);
> >> +
> >> + return cookie;
> >> +}
> >
> > we have a virtual channel wrapper where we do the same stuff as above, so
> > consider reusing that
> >
>
> Some of the functionality that the virtual channel might provide cannot
> be extracted because it is embedded into the JR driver (e.g. the
> tasklet). Therefore the use of the virtual channel is inefficient at
> best if not even impossible.
Which itself is problematic in first place and no explanation provided on
why. Yes you have a special hardware, so does every second person!
> >> +static void caam_dma_issue_pending(struct dma_chan *chan)
> >> +{
> >> + struct caam_dma_ctx *ctx = container_of(chan, struct caam_dma_ctx,
> >> + chan);
> >> + struct caam_dma_edesc *edesc, *_edesc;
> >> +
> >> + spin_lock_bh(&ctx->edesc_lock);
> >> + list_for_each_entry_safe(edesc, _edesc, &ctx->submit_q, node) {
> >> + if (caam_jr_enqueue(ctx->jrdev, edesc->jd,
> >> + caam_dma_done, edesc) < 0)
> >
> > what does the caam_jr_enqueue() do?
> >
>
> Enqueues a job descriptor (jd) into a job ring (jr). Additionally the
> "caam_dma_done()" function is registered to be used as a callback when
> the job finishes. For more details see drivers/crypto/caam/jr.c.
Sorry I am going to refuse to look at other references. A patch should
provide enough explanation on its own for me to review.
> >> + err = dma_async_device_register(dma_dev);
> >> + if (err) {
> >> + dev_err(dev, "Failed to register CAAM DMA engine\n");
> >> + goto jr_bind_err;
> >> + }
> >> +
> >> + dev_info(dev, "caam dma support with %d job rings\n", bonds);
> >
> > that is very noisy
> >
>
> I want to print a line that in
[PATCHi v2] dt-bindings: add device tree binding for Arm TrustZone CryptoCell crypto engine
The Arm TrustZone CryptoCell is a hardware security engine. This patch
adds DT bindings for its Rich Execution Environment crypto engine.
A driver supporting this device is already present in the staging tree.
Signed-off-by: Gilad Ben-Yossef
Acked-by: Rob Herring
---
Changes from v1:
- Change node name to reflect type of device in example as pointed out by
Rob Herring.
.../devicetree/bindings/crypto/arm-cryptocell.txt | 22 ++
1 file changed, 22 insertions(+)
create mode 100644 Documentation/devicetree/bindings/crypto/arm-cryptocell.txt
diff --git a/Documentation/devicetree/bindings/crypto/arm-cryptocell.txt
b/Documentation/devicetree/bindings/crypto/arm-cryptocell.txt
new file mode 100644
index 000..cec8d5d
--- /dev/null
+++ b/Documentation/devicetree/bindings/crypto/arm-cryptocell.txt
@@ -0,0 +1,22 @@
+Arm TrustZone CryptoCell cryptographic engine
+
+Required properties:
+- compatible: Should be "arm,cryptocell-712-ree".
+- reg: Base physical address of the engine and length of memory mapped region.
+- interrupts: Interrupt number for the device.
+
+Optional properties:
+- interrupt-parent: The phandle for the interrupt controller that services
+ interrupts for this device.
+- clocks: Reference to the crypto engine clock.
+- dma-coherent: Present if dma operations are coherent.
+
+Examples:
+
+ arm_cc712: crypto@8000 {
+ compatible = "arm,cryptocell-712-ree";
+ interrupt-parent = <&intc>;
+ interrupts = < 0 30 4 >;
+ reg = < 0x8000 0x1 >;
+
+ };
--
2.7.4
Re: [PATCH RESEND 1/4] crypto: caam: add caam-dma node to SEC4.0 device tree binding
On Fri, Nov 10, 2017 at 08:02:01AM +, Radu Andrei Alexe wrote:
> On 11/9/2017 6:34 PM, Kim Phillips wrote:
> > On Thu, 9 Nov 2017 11:54:13 +
> > Radu Andrei Alexe wrote:
> >
> >> On 10/30/2017 4:24 PM, Kim Phillips wrote:
> >>> On Mon, 30 Oct 2017 15:46:51 +0200
> >>> Horia Geantă wrote:
> >>>
> +=
> +CAAM DMA Node
> +
> +Child node of the crypto node that enables the use of the DMA
> capabilities
> +of the CAAM by a stand-alone driver. The only required property is
> the
> +"compatible" property. All the other properties are determined from
> +the job rings on which the CAAM DMA driver depends (ex: the number
> of
> +dma-channels is equal to the number of defined job rings).
> +
> + - compatible
> + Usage: required
> + Value type:
> + Definition: Must include "fsl,sec-v4.0-dma"
> +
> +EXAMPLE
> + caam-dma {
> +compatible = "fsl,sec-v5.4-dma",
> + "fsl,sec-v5.0-dma",
> + "fsl,sec-v4.0-dma";
> + }
> >>>
> >>> If this isn't describing an aspect of the hardware, then what is it
> >>> doing in the device tree?
> >>>
> >>> Kim
> >>>
> >>
> >> Because the caam_dma driver is a platform driver I needed to create a
> >> platform device to activate the driver. My thought was that it was
> >> simpler to implement it using device tree.
> >> The next patch version will create the platform device dynamically at
> >> run time.
> >
> > Why create a new device when that h/w already has one?
> >
> > Why doesn't the existing crypto driver register dma capabilities with
> > the dma driver subsystem?
> >
> > Kim
> >
>
>
> 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. Placing the code in the same directory as
> the caam subsystem would only create confusion for the reader of an
> already complex driver.
>
> 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.
These are very good reasons.
We already have one DMA implementation drivers/crypto/ccp/ccp-dmaengine.c
which was sneaked past us and put into kernel with proper review!
On the other hand we have *bunch* of dmaengine users in crypto subsystem
which use dmaengine drivers and APIs and are good citizens. It allows folks
to share code with other usages of dmaengine and the usual arguments for
common code and frameworks...
If there are enough users we can add up a crypto-dmaengine lib which
programs dma controller for crypto users and avoid open coding for everyone.
for example sound-dmaengine layers does that...
HTH
--
~Vinod
Re: [PATCH RESEND 1/4] crypto: caam: add caam-dma node to SEC4.0 device tree binding
On Fri, Nov 10, 2017 at 10:44:30AM -0600, Kim Phillips wrote: > On Fri, 10 Nov 2017 08:02:01 + > Radu Andrei Alexe wrote: > > > On 11/9/2017 6:34 PM, Kim Phillips wrote: > > > On Thu, 9 Nov 2017 11:54:13 + > > > Radu Andrei Alexe wrote: > > >> The next patch version will create the platform device dynamically at > > >> run time. > > > > > > Why create a new device when that h/w already has one? > > > > > > Why doesn't the existing crypto driver register dma capabilities with > > > the dma driver subsystem? > > > > > I can think of two reasons: > > > > 1. The code that this driver introduces has nothing to do with crypto > > and everything to do with dma. > > I would think that at least a crypto "null" algorithm implementation > would share code. > > > Placing the code in the same directory as > > the caam subsystem would only create confusion for the reader of an > > already complex driver. > > this different directory argument seems to be identical to your 2 below: > > > 2. I wanted this driver to be tracked by the dma engine team. They have > > the right expertise to provide adequate feedback. If all the code was in > > the crypto directory they wouldn't know about this driver or any > > subsequent changes to it. > > dma subsystem bits could still be put in the dma area if deemed > necessary but I don't think it is: I see > drivers/crypto/ccp/ccp-dmaengine.c calls dma_async_device_register for > example. which is a shame as it was sneaked past the dmaengine community!! This is the *only* example and there and other examples where people use dmaengine: $ grep -rl dmaengine_prep* drivers/crypto/* |uniq drivers/crypto/atmel-aes.c drivers/crypto/atmel-sha.c drivers/crypto/atmel-tdes.c drivers/crypto/img-hash.c drivers/crypto/omap-aes.c drivers/crypto/omap-des.c drivers/crypto/omap-sham.c drivers/crypto/qce/dma.c drivers/crypto/stm32/stm32-hash.c drivers/crypto/ux500/cryp/cryp_core.c drivers/crypto/ux500/hash/hash_core.c > > 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? that is a very good question, perf does matter. Given that we have many folks using it, I think it would help, but yes nothing better than numbers speak for themselves. -- ~Vinod
Re: [PATCH v2 2/2] chcr: Add support for Inline IPSec
On Thu, Nov 16, 2017 at 01:19:52PM +0800, kbuild test robot wrote: > Hi Atul, > > Thank you for the patch! Yet something to improve: > > [auto build test ERROR on cryptodev/master] > [also build test ERROR on next-20171115] > [cannot apply to v4.14] > [if your patch is applied to the wrong git tree, please drop us a note to > help improve the system] > > url: > https://github.com/0day-ci/linux/commits/Atul-Gupta/cxgb4-Add-support-for-Inline-IPSec-Tx/20171112-012558 > base: > https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git > master > config: x86_64-randconfig-g0-11160917 (attached as .config) > compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4 > reproduce: > # save the attached .config to linux build tree > make ARCH=x86_64 > > All errors (new ones prefixed by >>): > >drivers/crypto/chelsio/chcr_core.o: In function `chcr_uld_tx_handler': > >> drivers/crypto/chelsio/chcr_core.c:195: undefined reference to > >> `chcr_ipsec_xmit' >drivers/crypto/chelsio/chcr_core.o: In function `chcr_uld_add': > >> drivers/crypto/chelsio/chcr_core.c:169: undefined reference to > >> `chcr_add_xfrmops' Atul, can you fix this and resubmit your patches please? Thanks! -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
