2019-11-05 15:14:34

by Andrey Smirnov

[permalink] [raw]
Subject: [PATCH 0/5] CAAM JR lifecycle

Everyone:

This series is a different approach to addressing the issues brought
up in [discussion]. This time the proposition is to get away from
creating per-JR platfrom device, move all of the underlying code into
caam.ko and disable manual binding/unbinding of the CAAM device via
sysfs. Note that this series is a rough cut intented to gauge if this
approach could be acceptable for upstreaming.

Thanks,
Andrey Smirnov

[discussion] lore.kernel.org/lkml/[email protected]

Andrey Smirnov (5):
crypto: caam - use static initialization
crypto: caam - introduce caam_jr_cbk
crypto: caam - convert JR API to use struct caam_drv_private_jr
crypto: caam - do not create a platform devices for JRs
crypto: caam - disable CAAM's bind/unbind attributes

drivers/crypto/caam/Kconfig | 5 +-
drivers/crypto/caam/Makefile | 15 +--
drivers/crypto/caam/caamalg.c | 114 ++++++++++----------
drivers/crypto/caam/caamalg_qi.c | 12 +--
drivers/crypto/caam/caamhash.c | 117 +++++++++++----------
drivers/crypto/caam/caampkc.c | 67 ++++++------
drivers/crypto/caam/caampkc.h | 2 +-
drivers/crypto/caam/caamrng.c | 41 ++++----
drivers/crypto/caam/ctrl.c | 16 ++-
drivers/crypto/caam/intern.h | 3 +-
drivers/crypto/caam/jr.c | 173 ++++++++-----------------------
drivers/crypto/caam/jr.h | 14 ++-
drivers/crypto/caam/key_gen.c | 11 +-
drivers/crypto/caam/key_gen.h | 5 +-
14 files changed, 275 insertions(+), 320 deletions(-)

--
2.21.0


2019-11-05 15:15:00

by Andrey Smirnov

[permalink] [raw]
Subject: [PATCH 5/5] crypto: caam - disable CAAM's bind/unbind attributes

Exposing bind/unbind attributes for CAAM device allows user to
circumvent module use counter and remove underlying device even while
it is still in use by crypto API. The problem can be easily reproduce
using the following sinppiet:

$ openssl speed -evp aes-128-cbc -engine afalg &
$ echo 30900000.crypto > /sys/bus/platform/drivers/caam/unbind
[ 164.797687] ------------[ cut here ]------------
[ 164.802320] kernel BUG at crypto/algapi.c:412!
[ 164.806771] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
[ 164.812260] Modules linked in: crct10dif_ce caam caamhash_desc caamalg_desc error btusb btbcm btintel
[ 164.821506] CPU: 1 PID: 2170 Comm: sh Not tainted 5.4.0-rc1 #30
[ 164.827428] Hardware name: ZII i.MX8MQ Ultra Zest Board (DT)
[ 164.833091] pstate: 20000005 (nzCv daif -PAN -UAO)
[ 164.837897] pc : crypto_unregister_alg+0xe4/0xf0
[ 164.842520] lr : crypto_unregister_alg+0x8c/0xf0
[ 164.847138] sp : ffff8000130f3b20
[ 164.850454] x29: ffff8000130f3b20 x28: ffff0000f1131a80
[ 164.855771] x27: 0000000000000000 x26: 0000000000000000
[ 164.861087] x25: ffff0000fa147ea0 x24: 0000000000000020
[ 164.866404] x23: ffff8000130f3c58 x22: ffff8000130f3b58
[ 164.871721] x21: ffff800012b787c8 x20: ffff800012be7ef0
[ 164.877037] x19: ffff800008ad7300 x18: 000000000000002b
[ 164.882353] x17: 0000000000000000 x16: 0000000000000000
[ 164.887670] x15: ffff800012b8f4d0 x14: 55980d468eb0c075
[ 164.892987] x13: 4375a0958c16498f x12: 27cb4484db878b3d
[ 164.898304] x11: c3bdc615f6902956 x10: e030849201295489
[ 164.903620] x9 : 00a97e1a31855afa x8 : 00000000000014a5
[ 164.908937] x7 : ffff800008ad7310 x6 : ffff8000130f3a60
[ 164.914253] x5 : ffff8000130f3af8 x4 : ffff800008ad7310
[ 164.919570] x3 : 0000000000000000 x2 : 0000000000000000
[ 164.924886] x1 : ffffffffffffffff x0 : 0000000000000002
[ 164.930202] Call trace:
[ 164.932656] crypto_unregister_alg+0xe4/0xf0
[ 164.936932] crypto_unregister_skcipher+0x20/0x30
[ 164.941662] caam_algapi_exit+0x84/0xa0 [caam]
[ 164.946124] caam_jr_remove+0x54/0xd0 [caam]
[ 164.950401] devm_action_release+0x20/0x30
[ 164.954501] release_nodes+0x1c8/0x240
[ 164.958255] devres_release_all+0x3c/0x60
[ 164.962272] device_release_driver_internal+0x10c/0x1c0
[ 164.967501] device_driver_detach+0x28/0x40
[ 164.971689] unbind_store+0x94/0x100
[ 164.975269] drv_attr_store+0x40/0x60
[ 164.978938] sysfs_kf_write+0x5c/0x70
[ 164.982605] kernfs_fop_write+0xf4/0x1f0
[ 164.986534] __vfs_write+0x48/0x90
[ 164.989941] vfs_write+0xb8/0x1d0
[ 164.993261] ksys_write+0x74/0x100
[ 164.996668] __arm64_sys_write+0x24/0x30
[ 165.000598] el0_svc_handler+0x94/0x100
[ 165.004439] el0_svc+0x8/0xc
[ 165.007329] Code: aa1403e0 97f2d52e 12800020 17fffff5 (d4210000)
[ 165.013428] ---[ end trace 11587fd1ef597dd6 ]---
[ 165.018138] note: sh[2170] exited with preempt_count 1
[ 165.024146] ------------[ cut here ]------------
[ 165.028786] WARNING: CPU: 1 PID: 0 at kernel/rcu/tree.c:569 rcu_idle_enter+0x7c/0x90
[ 165.048977] Hardware name: ZII i.MX8MQ Ultra Zest Board (DT)
[ 165.054640] pstate: 200003c5 (nzCv DAIF -PAN -UAO)
[ 165.059435] pc : rcu_idle_enter+0x7c/0x90
[ 165.063450] lr : do_idle+0x218/0x2b0
[ 165.067027] sp : ffff800012e1bf20
[ 165.070343] x29: ffff800012e1bf20 x28: 0000000000000000
[ 165.075663] x27: 0000000000000000 x26: 0000000000000000
[ 165.080983] x25: 0000000000000000 x24: ffff800012b78884
[ 165.089045] x21: ffff800012b78860 x20: 0000000000000002
[ 165.094362] x19: ffff800012b787e8 x18: 0000000000000010
[ 165.099678] x17: 0000000000000000 x16: 0000000000000001
[ 165.104995] x15: ffff0000ff789170 x14: 0000000000000001
[ 165.110311] x13: ffff0000ff7a8170 x12: ffff0000fa996cd4
[ 165.115628] x11: ffff0000fa996cd4 x10: 0000000000000970
[ 165.120945] x9 : ffff800012e1bea0 x8 : ffff0000fa9aa450
[ 165.126261] x7 : 0000000000000001 x6 : ffff800012e1bee0
[ 165.131577] x5 : 0000000000000001 x4 : ffff800012cc61a8
[ 165.136894] x3 : 4000000000000002 x2 : 4000000000000000
[ 165.142210] x1 : ffff800012b6edc0 x0 : ffff0000ff789dc0
[ 165.147526] Call trace:
[ 165.149978] rcu_idle_enter+0x7c/0x90
[ 165.153644] do_idle+0x218/0x2b0
[ 165.156876] cpu_startup_entry+0x2c/0x50
[ 165.160806] secondary_start_kernel+0x164/0x180
[ 165.165339] ---[ end trace 11587fd1ef597dd7 ]---

Remove bind/unbind attributes of CAAM device, so that the only way to
remove it during runtime would be to remove underlying kernel module.

Signed-off-by: Andrey Smirnov <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Horia Geantă <[email protected]>
Cc: Herbert Xu <[email protected]>
Cc: Iuliana Prodan <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/crypto/caam/ctrl.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c
index 0fb39bcf638a..e0c16cd2ce1a 100644
--- a/drivers/crypto/caam/ctrl.c
+++ b/drivers/crypto/caam/ctrl.c
@@ -920,6 +920,7 @@ static struct platform_driver caam_driver = {
.driver = {
.name = "caam",
.of_match_table = caam_match,
+ .suppress_bind_attrs = true,
},
.probe = caam_probe,
};
--
2.21.0

2019-11-05 15:17:59

by Andrey Smirnov

[permalink] [raw]
Subject: [PATCH 2/5] crypto: caam - introduce caam_jr_cbk

Coalesce multiple ad-hoc definitions of the same function pointer into
a dedicated type to avoid repetition.

Signed-off-by: Andrey Smirnov <[email protected]>
Cc: Chris Healy <[email protected]>
Cc: Lucas Stach <[email protected]>
Cc: Horia Geantă <[email protected]>
Cc: Herbert Xu <[email protected]>
Cc: Iuliana Prodan <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
---
drivers/crypto/caam/intern.h | 3 ++-
drivers/crypto/caam/jr.c | 9 +++------
drivers/crypto/caam/jr.h | 7 ++++---
3 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/crypto/caam/intern.h b/drivers/crypto/caam/intern.h
index c7c10c90464b..fe2ca2ad6ff0 100644
--- a/drivers/crypto/caam/intern.h
+++ b/drivers/crypto/caam/intern.h
@@ -11,6 +11,7 @@
#define INTERN_H

#include "ctrl.h"
+#include "jr.h"

/* Currently comes from Kconfig param as a ^2 (driver-required) */
#define JOBR_DEPTH (1 << CONFIG_CRYPTO_DEV_FSL_CAAM_RINGSIZE)
@@ -31,7 +32,7 @@
* Each entry on an output ring needs one of these
*/
struct caam_jrentry_info {
- void (*callbk)(struct device *dev, u32 *desc, u32 status, void *arg);
+ caam_jr_cbk callbk;
void *cbkarg; /* Argument per ring entry */
u32 *desc_addr_virt; /* Stored virt addr for postprocessing */
dma_addr_t desc_addr_dma; /* Stored bus addr for done matching */
diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c
index 49c98a7f6723..3e78fedeea30 100644
--- a/drivers/crypto/caam/jr.c
+++ b/drivers/crypto/caam/jr.c
@@ -197,7 +197,7 @@ static void caam_jr_dequeue(unsigned long devarg)
int hw_idx, sw_idx, i, head, tail;
struct device *dev = (struct device *)devarg;
struct caam_drv_private_jr *jrp = dev_get_drvdata(dev);
- void (*usercall)(struct device *dev, u32 *desc, u32 status, void *arg);
+ caam_jr_cbk usercall;
u32 *userdesc, userstatus;
void *userarg;
u32 outring_used = 0;
@@ -354,10 +354,7 @@ EXPORT_SYMBOL(caam_jr_free);
* @areq: optional pointer to a user argument for use at callback
* time.
**/
-int caam_jr_enqueue(struct device *dev, u32 *desc,
- void (*cbk)(struct device *dev, u32 *desc,
- u32 status, void *areq),
- void *areq)
+int caam_jr_enqueue(struct device *dev, u32 *desc, caam_jr_cbk cbk, void *areq)
{
struct caam_drv_private_jr *jrp = dev_get_drvdata(dev);
struct caam_jrentry_info *head_entry;
@@ -386,7 +383,7 @@ int caam_jr_enqueue(struct device *dev, u32 *desc,
head_entry = &jrp->entinfo[head];
head_entry->desc_addr_virt = desc;
head_entry->desc_size = desc_size;
- head_entry->callbk = (void *)cbk;
+ head_entry->callbk = cbk;
head_entry->cbkarg = areq;
head_entry->desc_addr_dma = desc_dma;

diff --git a/drivers/crypto/caam/jr.h b/drivers/crypto/caam/jr.h
index eab611530f36..81acc6a6909f 100644
--- a/drivers/crypto/caam/jr.h
+++ b/drivers/crypto/caam/jr.h
@@ -9,11 +9,12 @@
#define JR_H

/* Prototypes for backend-level services exposed to APIs */
+typedef void (*caam_jr_cbk)(struct device *dev, u32 *desc, u32 status,
+ void *areq);
+
struct device *caam_jr_alloc(void);
void caam_jr_free(struct device *rdev);
-int caam_jr_enqueue(struct device *dev, u32 *desc,
- void (*cbk)(struct device *dev, u32 *desc, u32 status,
- void *areq),
+int caam_jr_enqueue(struct device *dev, u32 *desc, caam_jr_cbk cbk,
void *areq);

#endif /* JR_H */
--
2.21.0

2019-11-06 15:20:53

by Andrey Smirnov

[permalink] [raw]
Subject: Re: [PATCH 0/5] CAAM JR lifecycle

On Tue, Nov 5, 2019 at 11:27 PM Vakul Garg <[email protected]> wrote:
>
>
>
> > -----Original Message-----
> > From: [email protected] <linux-crypto-
> > [email protected]> On Behalf Of Andrey Smirnov
> > Sent: Tuesday, November 5, 2019 8:44 PM
> > To: [email protected]
> > Cc: Andrey Smirnov <[email protected]>; Chris Healy
> > <[email protected]>; Lucas Stach <[email protected]>; Horia Geanta
> > <[email protected]>; Herbert Xu <[email protected]>;
> > Iuliana Prodan <[email protected]>; dl-linux-imx <linux-
> > [email protected]>; [email protected]
> > Subject: [PATCH 0/5] CAAM JR lifecycle
> >
> > Everyone:
> >
> > This series is a different approach to addressing the issues brought up in
> > [discussion]. This time the proposition is to get away from creating per-JR
> > platfrom device, move all of the underlying code into caam.ko and disable
> > manual binding/unbinding of the CAAM device via sysfs. Note that this series
> > is a rough cut intented to gauge if this approach could be acceptable for
> > upstreaming.
> >
> > Thanks,
> > Andrey Smirnov
> >
> > [discussion] lore.kernel.org/lkml/20190904023515.7107-13-
> > [email protected]
> >
> > Andrey Smirnov (5):
> > crypto: caam - use static initialization
> > crypto: caam - introduce caam_jr_cbk
> > crypto: caam - convert JR API to use struct caam_drv_private_jr
> > crypto: caam - do not create a platform devices for JRs
> > crypto: caam - disable CAAM's bind/unbind attributes
> >
>
> To access caam jobrings from DPDK (user space drivers), we unbind job-ring's platform device from the kernel.
> What would be the alternate way to enable job ring drivers in user space?
>

Wouldn't either building your kernel with
CONFIG_CRYPTO_DEV_FSL_CAAM_JR=n (this series doesn't handle that right
currently due to being a rough cut) or disabling specific/all JRs via
DT accomplish the same goal?

Thanks,
Andrey Smirnov

2019-11-13 19:25:59

by Horia Geanta

[permalink] [raw]
Subject: Re: [PATCH 0/5] CAAM JR lifecycle

On 11/6/2019 5:19 PM, Andrey Smirnov wrote:
> On Tue, Nov 5, 2019 at 11:27 PM Vakul Garg <[email protected]> wrote:
>>
>>
>>
>>> -----Original Message-----
>>> From: [email protected] <linux-crypto-
>>> [email protected]> On Behalf Of Andrey Smirnov
>>> Sent: Tuesday, November 5, 2019 8:44 PM
>>> To: [email protected]
>>> Cc: Andrey Smirnov <[email protected]>; Chris Healy
>>> <[email protected]>; Lucas Stach <[email protected]>; Horia Geanta
>>> <[email protected]>; Herbert Xu <[email protected]>;
>>> Iuliana Prodan <[email protected]>; dl-linux-imx <linux-
>>> [email protected]>; [email protected]
>>> Subject: [PATCH 0/5] CAAM JR lifecycle
>>>
>>> Everyone:
>>>
>>> This series is a different approach to addressing the issues brought up in
>>> [discussion]. This time the proposition is to get away from creating per-JR
>>> platfrom device, move all of the underlying code into caam.ko and disable
>>> manual binding/unbinding of the CAAM device via sysfs. Note that this series
>>> is a rough cut intented to gauge if this approach could be acceptable for
>>> upstreaming.
>>>
>>> Thanks,
>>> Andrey Smirnov
>>>
>>> [discussion] lore.kernel.org/lkml/20190904023515.7107-13-
>>> [email protected]
>>>
>>> Andrey Smirnov (5):
>>> crypto: caam - use static initialization
>>> crypto: caam - introduce caam_jr_cbk
>>> crypto: caam - convert JR API to use struct caam_drv_private_jr
>>> crypto: caam - do not create a platform devices for JRs
>>> crypto: caam - disable CAAM's bind/unbind attributes
>>>
>>
>> To access caam jobrings from DPDK (user space drivers), we unbind job-ring's platform device from the kernel.
>> What would be the alternate way to enable job ring drivers in user space?
>>
>
> Wouldn't either building your kernel with
> CONFIG_CRYPTO_DEV_FSL_CAAM_JR=n (this series doesn't handle that right
> currently due to being a rough cut) or disabling specific/all JRs via
> DT accomplish the same goal?
>
It's not a 1:1 match, the ability to move a ring to user space / VM etc.
*dynamically* goes away.

Horia

2019-11-13 19:30:00

by Andrey Smirnov

[permalink] [raw]
Subject: Re: [PATCH 0/5] CAAM JR lifecycle

On Wed, Nov 13, 2019 at 10:57 AM Horia Geanta <[email protected]> wrote:
>
> On 11/6/2019 5:19 PM, Andrey Smirnov wrote:
> > On Tue, Nov 5, 2019 at 11:27 PM Vakul Garg <[email protected]> wrote:
> >>
> >>
> >>
> >>> -----Original Message-----
> >>> From: [email protected] <linux-crypto-
> >>> [email protected]> On Behalf Of Andrey Smirnov
> >>> Sent: Tuesday, November 5, 2019 8:44 PM
> >>> To: [email protected]
> >>> Cc: Andrey Smirnov <[email protected]>; Chris Healy
> >>> <[email protected]>; Lucas Stach <[email protected]>; Horia Geanta
> >>> <[email protected]>; Herbert Xu <[email protected]>;
> >>> Iuliana Prodan <[email protected]>; dl-linux-imx <linux-
> >>> [email protected]>; [email protected]
> >>> Subject: [PATCH 0/5] CAAM JR lifecycle
> >>>
> >>> Everyone:
> >>>
> >>> This series is a different approach to addressing the issues brought up in
> >>> [discussion]. This time the proposition is to get away from creating per-JR
> >>> platfrom device, move all of the underlying code into caam.ko and disable
> >>> manual binding/unbinding of the CAAM device via sysfs. Note that this series
> >>> is a rough cut intented to gauge if this approach could be acceptable for
> >>> upstreaming.
> >>>
> >>> Thanks,
> >>> Andrey Smirnov
> >>>
> >>> [discussion] lore.kernel.org/lkml/20190904023515.7107-13-
> >>> [email protected]
> >>>
> >>> Andrey Smirnov (5):
> >>> crypto: caam - use static initialization
> >>> crypto: caam - introduce caam_jr_cbk
> >>> crypto: caam - convert JR API to use struct caam_drv_private_jr
> >>> crypto: caam - do not create a platform devices for JRs
> >>> crypto: caam - disable CAAM's bind/unbind attributes
> >>>
> >>
> >> To access caam jobrings from DPDK (user space drivers), we unbind job-ring's platform device from the kernel.
> >> What would be the alternate way to enable job ring drivers in user space?
> >>
> >
> > Wouldn't either building your kernel with
> > CONFIG_CRYPTO_DEV_FSL_CAAM_JR=n (this series doesn't handle that right
> > currently due to being a rough cut) or disabling specific/all JRs via
> > DT accomplish the same goal?
> >
> It's not a 1:1 match, the ability to move a ring to user space / VM etc.
> *dynamically* goes away.
>

Wouldn't it be possible to do that dynamically using DT overlays? That
is "modprobe -r caam; <apply overlay>; modprobe caam"?

Thanks,
Andrey Smirnov

2019-11-17 06:17:57

by Andrey Smirnov

[permalink] [raw]
Subject: Re: [PATCH 0/5] CAAM JR lifecycle

On Wed, Nov 13, 2019 at 11:25 AM Andrey Smirnov
<[email protected]> wrote:
>
> On Wed, Nov 13, 2019 at 10:57 AM Horia Geanta <[email protected]> wrote:
> >
> > On 11/6/2019 5:19 PM, Andrey Smirnov wrote:
> > > On Tue, Nov 5, 2019 at 11:27 PM Vakul Garg <[email protected]> wrote:
> > >>
> > >>
> > >>
> > >>> -----Original Message-----
> > >>> From: [email protected] <linux-crypto-
> > >>> [email protected]> On Behalf Of Andrey Smirnov
> > >>> Sent: Tuesday, November 5, 2019 8:44 PM
> > >>> To: [email protected]
> > >>> Cc: Andrey Smirnov <[email protected]>; Chris Healy
> > >>> <[email protected]>; Lucas Stach <[email protected]>; Horia Geanta
> > >>> <[email protected]>; Herbert Xu <[email protected]>;
> > >>> Iuliana Prodan <[email protected]>; dl-linux-imx <linux-
> > >>> [email protected]>; [email protected]
> > >>> Subject: [PATCH 0/5] CAAM JR lifecycle
> > >>>
> > >>> Everyone:
> > >>>
> > >>> This series is a different approach to addressing the issues brought up in
> > >>> [discussion]. This time the proposition is to get away from creating per-JR
> > >>> platfrom device, move all of the underlying code into caam.ko and disable
> > >>> manual binding/unbinding of the CAAM device via sysfs. Note that this series
> > >>> is a rough cut intented to gauge if this approach could be acceptable for
> > >>> upstreaming.
> > >>>
> > >>> Thanks,
> > >>> Andrey Smirnov
> > >>>
> > >>> [discussion] lore.kernel.org/lkml/20190904023515.7107-13-
> > >>> [email protected]
> > >>>
> > >>> Andrey Smirnov (5):
> > >>> crypto: caam - use static initialization
> > >>> crypto: caam - introduce caam_jr_cbk
> > >>> crypto: caam - convert JR API to use struct caam_drv_private_jr
> > >>> crypto: caam - do not create a platform devices for JRs
> > >>> crypto: caam - disable CAAM's bind/unbind attributes
> > >>>
> > >>
> > >> To access caam jobrings from DPDK (user space drivers), we unbind job-ring's platform device from the kernel.
> > >> What would be the alternate way to enable job ring drivers in user space?
> > >>
> > >
> > > Wouldn't either building your kernel with
> > > CONFIG_CRYPTO_DEV_FSL_CAAM_JR=n (this series doesn't handle that right
> > > currently due to being a rough cut) or disabling specific/all JRs via
> > > DT accomplish the same goal?
> > >
> > It's not a 1:1 match, the ability to move a ring to user space / VM etc.
> > *dynamically* goes away.
> >
>
> Wouldn't it be possible to do that dynamically using DT overlays? That
> is "modprobe -r caam; <apply overlay>; modprobe caam"?
>

Or, alternatively, could adding a module parameter, say "jr_mask", to
limit JRs controlled by the driver cover dynamic use case?

Thanks,
Andrey Smirnov

2019-11-20 07:12:19

by Horia Geanta

[permalink] [raw]
Subject: Re: [PATCH 2/5] crypto: caam - introduce caam_jr_cbk

On 11/5/2019 5:14 PM, Andrey Smirnov wrote:
> Coalesce multiple ad-hoc definitions of the same function pointer into
> a dedicated type to avoid repetition.
>
> Signed-off-by: Andrey Smirnov <[email protected]>
> Cc: Chris Healy <[email protected]>
> Cc: Lucas Stach <[email protected]>
> Cc: Horia Geant? <[email protected]>
> Cc: Herbert Xu <[email protected]>
> Cc: Iuliana Prodan <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
Reviewed-by: Horia Geant? <[email protected]>

Note that there will be a conflict with the patch set adding
backlogging support:
https://lore.kernel.org/linux-crypto/[email protected]/

Thanks,
Horia

2020-01-08 16:08:59

by Andrey Smirnov

[permalink] [raw]
Subject: Re: [PATCH 0/5] CAAM JR lifecycle

On Sat, Nov 16, 2019 at 10:15 PM Andrey Smirnov
<[email protected]> wrote:
>
> On Wed, Nov 13, 2019 at 11:25 AM Andrey Smirnov
> <[email protected]> wrote:
> >
> > On Wed, Nov 13, 2019 at 10:57 AM Horia Geanta <[email protected]> wrote:
> > >
> > > On 11/6/2019 5:19 PM, Andrey Smirnov wrote:
> > > > On Tue, Nov 5, 2019 at 11:27 PM Vakul Garg <[email protected]> wrote:
> > > >>
> > > >>
> > > >>
> > > >>> -----Original Message-----
> > > >>> From: [email protected] <linux-crypto-
> > > >>> [email protected]> On Behalf Of Andrey Smirnov
> > > >>> Sent: Tuesday, November 5, 2019 8:44 PM
> > > >>> To: [email protected]
> > > >>> Cc: Andrey Smirnov <[email protected]>; Chris Healy
> > > >>> <[email protected]>; Lucas Stach <[email protected]>; Horia Geanta
> > > >>> <[email protected]>; Herbert Xu <[email protected]>;
> > > >>> Iuliana Prodan <[email protected]>; dl-linux-imx <linux-
> > > >>> [email protected]>; [email protected]
> > > >>> Subject: [PATCH 0/5] CAAM JR lifecycle
> > > >>>
> > > >>> Everyone:
> > > >>>
> > > >>> This series is a different approach to addressing the issues brought up in
> > > >>> [discussion]. This time the proposition is to get away from creating per-JR
> > > >>> platfrom device, move all of the underlying code into caam.ko and disable
> > > >>> manual binding/unbinding of the CAAM device via sysfs. Note that this series
> > > >>> is a rough cut intented to gauge if this approach could be acceptable for
> > > >>> upstreaming.
> > > >>>
> > > >>> Thanks,
> > > >>> Andrey Smirnov
> > > >>>
> > > >>> [discussion] lore.kernel.org/lkml/20190904023515.7107-13-
> > > >>> [email protected]
> > > >>>
> > > >>> Andrey Smirnov (5):
> > > >>> crypto: caam - use static initialization
> > > >>> crypto: caam - introduce caam_jr_cbk
> > > >>> crypto: caam - convert JR API to use struct caam_drv_private_jr
> > > >>> crypto: caam - do not create a platform devices for JRs
> > > >>> crypto: caam - disable CAAM's bind/unbind attributes
> > > >>>
> > > >>
> > > >> To access caam jobrings from DPDK (user space drivers), we unbind job-ring's platform device from the kernel.
> > > >> What would be the alternate way to enable job ring drivers in user space?
> > > >>
> > > >
> > > > Wouldn't either building your kernel with
> > > > CONFIG_CRYPTO_DEV_FSL_CAAM_JR=n (this series doesn't handle that right
> > > > currently due to being a rough cut) or disabling specific/all JRs via
> > > > DT accomplish the same goal?
> > > >
> > > It's not a 1:1 match, the ability to move a ring to user space / VM etc.
> > > *dynamically* goes away.
> > >
> >
> > Wouldn't it be possible to do that dynamically using DT overlays? That
> > is "modprobe -r caam; <apply overlay>; modprobe caam"?
> >
>
> Or, alternatively, could adding a module parameter, say "jr_mask", to
> limit JRs controlled by the driver cover dynamic use case?
>

Horia, could you please comment on the above? I think getting rid of
struct device for JRs is the best approach to dealing with described
corner case problems + it will allows us to get rid of this custom JR
users lifecycle management
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/crypto/caam/jr.c?h=v5.4.8#n26
since it can be just done as a part for caam_probe(), so I'd like to
either move forward on this series or close this discussion.

Thanks,
Andrey Smirnov