mbox series

[v4,00/20] Support more Amlogic SoC families in crypto driver

Message ID 20240212135108.549755-1-avromanov@salutedevices.com
Headers
Series Support more Amlogic SoC families in crypto driver |

Message

Alexey Romanov Feb. 12, 2024, 1:50 p.m. UTC 
  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.

---

Changes V1 -> V2 [2]:

- Rebased over linux-next.
- Adjusted device tree bindings description.
- A1 and S4 dts use their own compatible, which is a G12 fallback.

Changes V2 -> V3 [3]:

- Fix errors in dt-bindings and device tree.
- Add new field in platform data, which determines
whether clock controller should be used for crypto IP.
- Place back MODULE_DEVICE_TABLE.
- Correct commit messages.

Changes V3 -> V4 [4]:

- Update dt-bindings as per Krzysztof Kozlowski comments.
- Fix bisection: get rid of compiler errors in some patches.

Links:
  - [1] https://gist.github.com/mRrvz/3fb8943a7487ab7b943ec140706995e7
  - [2] https://lore.kernel.org/all/20240110201216.18016-1-avromanov@salutedevices.com/
  - [3] https://lore.kernel.org/all/20240123165831.970023-1-avromanov@salutedevices.com/
  - [4] https://lore.kernel.org/all/20240205155521.1795552-1-avromanov@salutedevices.com/

Alexey Romanov (20):
  drivers: crypto: meson: don't hardcode IRQ count
  drviers: crypto: meson: add platform data
  drivers: crypto: meson: make CLK controller optional
  drivers: crypto: meson: add MMIO helpers
  drivers: crypto: meson: move get_engine_number()
  drivers: crypto: meson: drop status field from meson_flow
  drivers: crypto: meson: move algs definition and cipher API to
    cipher.c
  drivers: crypto: meson: cleanup defines
  drivers: crypto: meson: process more than MAXDESCS descriptors
  drivers: crypto: meson: avoid kzalloc in engine thread
  drivers: crypto: meson: introduce hasher
  drivers: crypto: meson: add support for AES-CTR
  drivers: crypto: meson: use fallback for 192-bit keys
  drivers: crypto: meson: add support for G12-series
  drivers: crypto: meson: add support for AXG-series
  dt-bindings: crypto: meson: support new SoC's
  arch: arm64: dts: meson: a1: add crypto node
  arch: arm64: dts: meson: s4: add crypto node
  arch: arm64: dts: meson: g12: add crypto node
  arch: arm64: dts: meson: axg: add crypto node

 .../bindings/crypto/amlogic,gxl-crypto.yaml   |  43 +-
 arch/arm64/boot/dts/amlogic/meson-a1.dtsi     |   7 +
 arch/arm64/boot/dts/amlogic/meson-axg.dtsi    |   6 +
 .../boot/dts/amlogic/meson-g12-common.dtsi    |   6 +
 arch/arm64/boot/dts/amlogic/meson-s4.dtsi     |   6 +
 drivers/crypto/amlogic/Makefile               |   2 +-
 drivers/crypto/amlogic/amlogic-gxl-cipher.c   | 602 ++++++++++++------
 drivers/crypto/amlogic/amlogic-gxl-core.c     | 290 +++++----
 drivers/crypto/amlogic/amlogic-gxl-hasher.c   | 452 +++++++++++++
 drivers/crypto/amlogic/amlogic-gxl.h          | 117 +++-
 10 files changed, 1183 insertions(+), 348 deletions(-)
 create mode 100644 drivers/crypto/amlogic/amlogic-gxl-hasher.c

Comments

Neil Armstrong Feb. 12, 2024, 5:04 p.m. UTC | #1 
Hi,

On 12/02/2024 14:50, Alexey Romanov wrote:
> 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.
> 
> ---
> 
> Changes V1 -> V2 [2]:
> 
> - Rebased over linux-next.
> - Adjusted device tree bindings description.
> - A1 and S4 dts use their own compatible, which is a G12 fallback.
> 
> Changes V2 -> V3 [3]:
> 
> - Fix errors in dt-bindings and device tree.
> - Add new field in platform data, which determines
> whether clock controller should be used for crypto IP.
> - Place back MODULE_DEVICE_TABLE.
> - Correct commit messages.
> 
> Changes V3 -> V4 [4]:
> 
> - Update dt-bindings as per Krzysztof Kozlowski comments.
> - Fix bisection: get rid of compiler errors in some patches.
> 
> Links:
>    - [1] https://gist.github.com/mRrvz/3fb8943a7487ab7b943ec140706995e7
>    - [2] https://lore.kernel.org/all/20240110201216.18016-1-avromanov@salutedevices.com/
>    - [3] https://lore.kernel.org/all/20240123165831.970023-1-avromanov@salutedevices.com/
>    - [4] https://lore.kernel.org/all/20240205155521.1795552-1-avromanov@salutedevices.com/
> 
> Alexey Romanov (20):
>    drivers: crypto: meson: don't hardcode IRQ count
>    drviers: crypto: meson: add platform data
>    drivers: crypto: meson: make CLK controller optional
>    drivers: crypto: meson: add MMIO helpers
>    drivers: crypto: meson: move get_engine_number()
>    drivers: crypto: meson: drop status field from meson_flow
>    drivers: crypto: meson: move algs definition and cipher API to
>      cipher.c
>    drivers: crypto: meson: cleanup defines
>    drivers: crypto: meson: process more than MAXDESCS descriptors
>    drivers: crypto: meson: avoid kzalloc in engine thread
>    drivers: crypto: meson: introduce hasher
>    drivers: crypto: meson: add support for AES-CTR
>    drivers: crypto: meson: use fallback for 192-bit keys
>    drivers: crypto: meson: add support for G12-series
>    drivers: crypto: meson: add support for AXG-series
>    dt-bindings: crypto: meson: support new SoC's
>    arch: arm64: dts: meson: a1: add crypto node
>    arch: arm64: dts: meson: s4: add crypto node
>    arch: arm64: dts: meson: g12: add crypto node
>    arch: arm64: dts: meson: axg: add crypto node
> 
>   .../bindings/crypto/amlogic,gxl-crypto.yaml   |  43 +-
>   arch/arm64/boot/dts/amlogic/meson-a1.dtsi     |   7 +
>   arch/arm64/boot/dts/amlogic/meson-axg.dtsi    |   6 +
>   .../boot/dts/amlogic/meson-g12-common.dtsi    |   6 +
>   arch/arm64/boot/dts/amlogic/meson-s4.dtsi     |   6 +
>   drivers/crypto/amlogic/Makefile               |   2 +-
>   drivers/crypto/amlogic/amlogic-gxl-cipher.c   | 602 ++++++++++++------
>   drivers/crypto/amlogic/amlogic-gxl-core.c     | 290 +++++----
>   drivers/crypto/amlogic/amlogic-gxl-hasher.c   | 452 +++++++++++++
>   drivers/crypto/amlogic/amlogic-gxl.h          | 117 +++-
>   10 files changed, 1183 insertions(+), 348 deletions(-)
>   create mode 100644 drivers/crypto/amlogic/amlogic-gxl-hasher.c
> 

The overall looks fine, I'll wait for further comments on the code,
but the DT looks ok and I didn't find any issues code building
or DT bindings check.

Neil
Corentin Labbe Feb. 13, 2024, 7:21 a.m. UTC | #2 
Le Mon, Feb 12, 2024 at 04:50:48PM +0300, Alexey Romanov a écrit :
> 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.
> 
> ---
> 

I started to test on Lepotato board and added patchs up to  "drivers: crypto: meson: process more than MAXDESCS descriptors"
booting lead to:
[   18.559922] gxl-crypto c883e000.crypto: will run requests pump with realtime priority
[   18.562492] gxl-crypto c883e000.crypto: will run requests pump with realtime priority
[   18.570328] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000028
[   18.581135] Mem abort info:
[   18.581354]   ESR = 0x0000000096000006
[   18.585138]   EC = 0x25: DABT (current EL), IL = 32 bits
[   18.593005]   SET = 0, FnV = 0
[   18.593334]   EA = 0, S1PTW = 0
[   18.597329]   FSC = 0x06: level 2 translation fault
[   18.604250] Data abort info:
[   18.604282]   ISV = 0, ISS = 0x00000006, ISS2 = 0x00000000
[   18.612243]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[   18.614552]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[   18.624249] user pgtable: 4k pages, 48-bit VAs, pgdp=000000007b8ab000
[   18.626196] [0000000000000028] pgd=080000007b8ac003, p4d=080000007b8ac003, pud=080000007b8ad003, pmd=0000000000000000
[   18.640426] Internal error: Oops: 0000000096000006 [#1] PREEMPT SMP
[   18.642929] Modules linked in: of_mdio fixed_phy fwnode_mdio sm4_ce(-) sm4 meson_rng meson_canvas libphy rng_core meson_gxbb_wdt watchdog amlogic_gxl_crypto(+) ghash_generic gcm xctr xts cts essiv authenc cmac xcbc ccm
[   18.662164] CPU: 3 PID: 264 Comm: cryptomgr_test Not tainted 6.8.0-rc1-00052-gf70f2b0814a0 #11
[   18.670698] Hardware name: Libre Computer AML-S905X-CC (DT)
[   18.676220] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[   18.683118] pc : meson_get_engine_number+0x2c/0x50 [amlogic_gxl_crypto]
[   18.689674] lr : meson_skencrypt+0x38/0x8c [amlogic_gxl_crypto]
[   18.695539] sp : ffff800081393790
[   18.698816] x29: ffff800081393790 x28: 0000000000000400 x27: ffff800080874a80
[   18.705888] x26: ffff800081393830 x25: ffff800081393bd8 x24: ffff000001aaa000
[   18.712961] x23: 0000000000000001 x22: 0000000000000000 x21: ffff0000011b1c50
[   18.720033] x20: ffff00007bac8248 x19: ffff0000011b1c00 x18: ffffffffffffffff
[   18.727105] x17: 00000000000001a4 x16: ffff800078edc1f0 x15: ffff8000813938e0
[   18.734178] x14: ffff800101393bd7 x13: 0000000000000000 x12: 0000000000000000
[   18.741250] x11: 000000000000021c x10: fffffffff81213e0 x9 : 00000000000730d5
[   18.748323] x8 : ffff0000011b1ca8 x7 : fefefefefefefefe x6 : fffffc000007a302
[   18.755395] x5 : ffff800078eb4148 x4 : 0000000000000000 x3 : 0000000000000028
[   18.762468] x2 : ffff000001aaa040 x1 : 0000000000000000 x0 : 0000000000000000
[   18.769541] Call trace:
[   18.771956]  meson_get_engine_number+0x2c/0x50 [amlogic_gxl_crypto]
[   18.778167]  crypto_skcipher_encrypt+0xe0/0x124
[   18.782651]  test_skcipher_vec_cfg+0x2a8/0x6b0
[   18.787050]  test_skcipher_vec+0x80/0x1c4
[   18.791017]  alg_test_skcipher+0xbc/0x1fc
[   18.794985]  alg_test+0x140/0x628
[   18.798262]  cryptomgr_test+0x24/0x44
[   18.801885]  kthread+0x110/0x114
[   18.805076]  ret_from_fork+0x10/0x20
[   18.808617] Code: 1b008440 d65f03c0 9100a003 f9800071 (885f7c61) 
[   18.814651] ---[ end trace 0000000000000000 ]---
[   18.862270] meson8b-dwmac c9410000.ethernet: IRQ eth_wake_irq not found
[   18.863897] meson8b-dwmac c9410000.ethernet: IRQ eth_lpi not found
[   18.870349] meson8b-dwmac c9410000.ethernet: PTP uses main clock
[   18.880548] meson8b-dwmac c9410000.ethernet: User ID: 0x11, Synopsys ID: 0x37
[   18.882403] meson8b-dwmac c9410000.ethernet: 	DWMAC1000
[   18.887926] meson8b-dwmac c9410000.ethernet: DMA HW capability register supported
[   18.895215] meson8b-dwmac c9410000.ethernet: RX Checksum Offload Engine supported
[   18.902627] meson8b-dwmac c9410000.ethernet: COE Type 2
[   18.907756] meson8b-dwmac c9410000.ethernet: TX Checksum insertion supported
[   18.914750] meson8b-dwmac c9410000.ethernet: Wake-Up On Lan supported
[   18.921246] meson8b-dwmac c9410000.ethernet: Normal descriptors
[   18.927017] meson8b-dwmac c9410000.ethernet: Ring mode enabled
[   18.932782] meson8b-dwmac c9410000.ethernet: Enable RX Mitigation via HW Watchdog Timer
Alexey Romanov Feb. 15, 2024, 10:43 a.m. UTC | #3 
Hello,

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.
> > 
> > ---
> > 
> 
> I started to test on Lepotato board and added patchs up to  "drivers: crypto: meson: process more than MAXDESCS descriptors"
> booting lead to:

Can you please give me your test cases?
Which tool are you using or is it something custom?

> [   18.559922] gxl-crypto c883e000.crypto: will run requests pump with realtime priority
> [   18.562492] gxl-crypto c883e000.crypto: will run requests pump with realtime priority
> [   18.570328] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000028
> [   18.581135] Mem abort info:
> [   18.581354]   ESR = 0x0000000096000006
> [   18.585138]   EC = 0x25: DABT (current EL), IL = 32 bits
> [   18.593005]   SET = 0, FnV = 0
> [   18.593334]   EA = 0, S1PTW = 0
> [   18.597329]   FSC = 0x06: level 2 translation fault
> [   18.604250] Data abort info:
> [   18.604282]   ISV = 0, ISS = 0x00000006, ISS2 = 0x00000000
> [   18.612243]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
> [   18.614552]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
> [   18.624249] user pgtable: 4k pages, 48-bit VAs, pgdp=000000007b8ab000
> [   18.626196] [0000000000000028] pgd=080000007b8ac003, p4d=080000007b8ac003, pud=080000007b8ad003, pmd=0000000000000000
> [   18.640426] Internal error: Oops: 0000000096000006 [#1] PREEMPT SMP
> [   18.642929] Modules linked in: of_mdio fixed_phy fwnode_mdio sm4_ce(-) sm4 meson_rng meson_canvas libphy rng_core meson_gxbb_wdt watchdog amlogic_gxl_crypto(+) ghash_generic gcm xctr xts cts essiv authenc cmac xcbc ccm
> [   18.662164] CPU: 3 PID: 264 Comm: cryptomgr_test Not tainted 6.8.0-rc1-00052-gf70f2b0814a0 #11
> [   18.670698] Hardware name: Libre Computer AML-S905X-CC (DT)
> [   18.676220] pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> [   18.683118] pc : meson_get_engine_number+0x2c/0x50 [amlogic_gxl_crypto]
> [   18.689674] lr : meson_skencrypt+0x38/0x8c [amlogic_gxl_crypto]
> [   18.695539] sp : ffff800081393790
> [   18.698816] x29: ffff800081393790 x28: 0000000000000400 x27: ffff800080874a80
> [   18.705888] x26: ffff800081393830 x25: ffff800081393bd8 x24: ffff000001aaa000
> [   18.712961] x23: 0000000000000001 x22: 0000000000000000 x21: ffff0000011b1c50
> [   18.720033] x20: ffff00007bac8248 x19: ffff0000011b1c00 x18: ffffffffffffffff
> [   18.727105] x17: 00000000000001a4 x16: ffff800078edc1f0 x15: ffff8000813938e0
> [   18.734178] x14: ffff800101393bd7 x13: 0000000000000000 x12: 0000000000000000
> [   18.741250] x11: 000000000000021c x10: fffffffff81213e0 x9 : 00000000000730d5
> [   18.748323] x8 : ffff0000011b1ca8 x7 : fefefefefefefefe x6 : fffffc000007a302
> [   18.755395] x5 : ffff800078eb4148 x4 : 0000000000000000 x3 : 0000000000000028
> [   18.762468] x2 : ffff000001aaa040 x1 : 0000000000000000 x0 : 0000000000000000
> [   18.769541] Call trace:
> [   18.771956]  meson_get_engine_number+0x2c/0x50 [amlogic_gxl_crypto]
> [   18.778167]  crypto_skcipher_encrypt+0xe0/0x124
> [   18.782651]  test_skcipher_vec_cfg+0x2a8/0x6b0
> [   18.787050]  test_skcipher_vec+0x80/0x1c4
> [   18.791017]  alg_test_skcipher+0xbc/0x1fc
> [   18.794985]  alg_test+0x140/0x628
> [   18.798262]  cryptomgr_test+0x24/0x44
> [   18.801885]  kthread+0x110/0x114
> [   18.805076]  ret_from_fork+0x10/0x20
> [   18.808617] Code: 1b008440 d65f03c0 9100a003 f9800071 (885f7c61) 
> [   18.814651] ---[ end trace 0000000000000000 ]---
> [   18.862270] meson8b-dwmac c9410000.ethernet: IRQ eth_wake_irq not found
> [   18.863897] meson8b-dwmac c9410000.ethernet: IRQ eth_lpi not found
> [   18.870349] meson8b-dwmac c9410000.ethernet: PTP uses main clock
> [   18.880548] meson8b-dwmac c9410000.ethernet: User ID: 0x11, Synopsys ID: 0x37
> [   18.882403] meson8b-dwmac c9410000.ethernet: 	DWMAC1000
> [   18.887926] meson8b-dwmac c9410000.ethernet: DMA HW capability register supported
> [   18.895215] meson8b-dwmac c9410000.ethernet: RX Checksum Offload Engine supported
> [   18.902627] meson8b-dwmac c9410000.ethernet: COE Type 2
> [   18.907756] meson8b-dwmac c9410000.ethernet: TX Checksum insertion supported
> [   18.914750] meson8b-dwmac c9410000.ethernet: Wake-Up On Lan supported
> [   18.921246] meson8b-dwmac c9410000.ethernet: Normal descriptors
> [   18.927017] meson8b-dwmac c9410000.ethernet: Ring mode enabled
> [   18.932782] meson8b-dwmac c9410000.ethernet: Enable RX Mitigation via HW Watchdog Timer
Alexey Romanov Feb. 15, 2024, 10:47 a.m. UTC | #4 
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?
Corentin Labbe Feb. 19, 2024, 6:57 a.m. UTC | #5 
Le Thu, Feb 15, 2024 at 10:47:24AM +0000, Alexey Romanov a écrit :
> 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"
Alexey Romanov Feb. 28, 2024, 1:37 p.m. UTC | #6 
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?
Corentin Labbe Feb. 28, 2024, 8:19 p.m. UTC | #7 
Le Wed, Feb 28, 2024 at 01:37:02PM +0000, Alexey Romanov a écrit :
> 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.

Since you didnt hit the case, I suspect you didnt test the driver as module.

Regards
Alexey Romanov Feb. 29, 2024, 1:05 p.m. UTC | #8 
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.

> 
> Regards
Neil Armstrong Feb. 29, 2024, 3:14 p.m. UTC | #9 
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
>