Re: [PATCH v4 00/20] Support more Amlogic SoC families in crypto driver

[neil.armstrong@xxxxxxxxxx]

On 29/02/2024 14:05, Alexey Romanov wrote:
> Hello!
>
> On Wed, Feb 28, 2024 at 09:19:32PM +0100, Corentin Labbe wrote:
> > Le Wed, Feb 28, 2024 at 01:37:02PM +0000, Alexey Romanov a 'ecrit :
> > > Hello,
> > >
> > > On Mon, Feb 19, 2024 at 07:57:27AM +0100, Corentin Labbe wrote:
> > > > Le Thu, Feb 15, 2024 at 10:47:24AM +0000, Alexey Romanov a 'ecrit :
> > > > > On Tue, Feb 13, 2024 at 08:21:12AM +0100, Corentin Labbe wrote:
> > > > > > Le Mon, Feb 12, 2024 at 04:50:48PM +0300, Alexey Romanov a 'ecrit :
> > > > > > > Hello!
> > > > > > >
> > > > > > > This patchset expand the funcionality of the Amlogic
> > > > > > > crypto driver by adding support for more SoC families:
> > > > > > > AXG, G12A, G12B, SM1, A1, S4.
> > > > > > >
> > > > > > > Also specify and enable crypto node in device tree
> > > > > > > for reference Amlogic devices.
> > > > > > >
> > > > > > > Tested on AXG, G12A/B, SM1, A1 and S4 devices via
> > > > > > > custom tests [1] and tcrypt module.
> > > > > > >
> > > > > > > ---
> > > > > >
> > > > > > added patchs up to  "drivers: crypto: meson: process more than MAXDESCS descriptors"
> > > > >
> > > > > Including this patch or not?
> > > >
> > > > The crash start with "drivers: crypto: meson: move algs definition and cipher API to cipher.c"
> > >
> > > Unfortunately I was unable to reproduce this. I use Khadas Vim1 board
> > > and my custom tests (https://gist.github.com/mRrvz/3fb8943a7487ab7b943ec140706995e7).
> > > Tried both build as module and built-in.
> > >
> > > Can you, please, give more information? Maybe your test cases?
> >
> > My test case is simple, simply load the driver.
> >
> > The problem is that you moved the algs[i].mc = mc after the register of algs (in drivers: crypto: meson: move algs definition and cipher API to cipher.c)
> > Test could happen as soon the register is done and so mc is deferenced.
>
> Yeah, you are right. Will fix it. Thank you.
>
> > Since you didnt hit the case, I suspect you didnt test the driver as module.
>
> No, I test the driver as module.
> I think the problem is that on my system no one uses this crypto backend
> outside of my tests module, unlike your system.

I reproduced the issue, testing each commit with enabled runtime cryto tests:

I get the following when applying "drivers: crypto: meson: introduce hasher":
[    4.514031] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
[    4.517193] Mem abort info:
[    4.519934]   ESR = 0x0000000086000004
[    4.523641]   EC = 0x21: IABT (current EL), IL = 32 bits
[    4.528903]   SET = 0, FnV = 0
[    4.531920]   EA = 0, S1PTW = 0
[    4.535025]   FSC = 0x04: level 0 translation fault
[    4.539856] [0000000000000000] user address but active_mm is swapper
[    4.546155] Internal error: Oops: 0000000086000004 [#1] PREEMPT SMP
[    4.552359] Modules linked in:
[    4.555376] CPU: 2 PID: 77 Comm: cryptomgr_test Not tainted 6.8.0-rc6-next-20240229-g2296d426c21a #105
[    4.564608] Hardware name: Libre Computer AML-S905X-CC (DT)
[    4.570125] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    4.577025] pc : 0x0
[    4.579178] lr : ahash_def_finup+0x12c/0x14c
[    4.583406] sp : ffff8000825037a0
[    4.586683] x29: ffff8000825037a0 x28: 0000000000000001 x27: ffff000007ab8060
[    4.593758] x26: ffff800082503958 x25: 0000000000000400 x24: ffff800082503918
[    4.600830] x23: 0000000000000820 x22: ffff0000048ec390 x21: ffff000003d9e200
[    4.607903] x20: ffff0000048ee300 x19: ffff000003d9e400 x18: 0000000000000020
[    4.614976] x17: 00000000b5f74d5b x16: ffff800081bbbc60 x15: ffffffffffffffff
[    4.622048] x14: ffff0000049fb380 x13: 0000000000000000 x12: 0000000000000000
[    4.629120] x11: ffff800082503778 x10: ffff800082503780 x9 : 0000000000001388
[    4.636193] x8 : ffff800082503988 x7 : fefefefefefefefe x6 : 0101010101010101
[    4.643265] x5 : ffff0000049fb300 x4 : ffff00007f2d5830 x3 : 0000000000000000
[    4.650338] x2 : 0000000000000000 x1 : ffff0000048ee300 x0 : ffff000003d9e200
[    4.657411] Call trace:
[    4.659823]  0x0
[    4.661633]  crypto_ahash_finup+0x38/0x44
[    4.665602]  test_ahash_vec_cfg+0x680/0x7dc
[    4.669742]  test_hash_vec+0xac/0x1ec
[    4.673364]  __alg_test_hash+0x158/0x308
[    4.677246]  alg_test_hash+0xfc/0x1a0
[    4.680868]  alg_test.part.0+0x51c/0x524
[    4.684750]  alg_test+0x20/0x64
[    4.687854]  cryptomgr_test+0x24/0x44
[    4.691477]  kthread+0x118/0x11c
[    4.694668]  ret_from_fork+0x10/0x20
[    4.698214] Code: ???????? ???????? ???????? ???????? (????????)
[    4.704247] ---[ end trace 0000000000000000 ]---

Please make sure every single commit builds without error and doesn't regress at runtime on existing platforms.

Thanks,
Neil

> > Regards