| Message ID | 20240212135108.549755-1-avromanov@salutedevices.com |
|---|---|
| Headers | show |
| Series | Support more Amlogic SoC families in crypto driver | expand |
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
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?
On Mon, Feb 12, 2024 at 04:51:04PM +0300, Alexey Romanov wrote: > Now crypto module available at G12A/G12B/S4/A1/SM1/AXG. > > 1. Add new compatibles: > - amlogic,g12a-crypto > - amlogic,s4-crypto (uses g12a-crypto as fallback) > - amlogic,a1-crypto (uses g12a-crypto as fallback) > - amlogic,axg-crypto > > 2. All SoC's, exclude GXL, doesn't take a clock input for > Crypto IP. Make it required only for amlogic,gxl-crypto. > > 3. All SoC's, exclude GXL, uses only one interrupt flow > for Crypto IP. > > 4. Add power-domains in schema. > > Signed-off-by: Alexey Romanov <avromanov@salutedevices.com> > --- > .../bindings/crypto/amlogic,gxl-crypto.yaml | 43 ++++++++++++++++--- > 1 file changed, 36 insertions(+), 7 deletions(-) > > diff --git a/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml b/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml > index 948e11ebe4ee..41f0153d58c8 100644 > --- a/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml > +++ b/Documentation/devicetree/bindings/crypto/amlogic,gxl-crypto.yaml > @@ -11,20 +11,30 @@ maintainers: > > properties: > compatible: > - items: > - - const: amlogic,gxl-crypto > + oneOf: > + - items: > + - enum: > + - amlogic,a1-crypto > + - amlogic,s4-crypto > + - const: amlogic,g12a-crypto > + - enum: > + - amlogic,gxl-crypto > + - amlogic,axg-crypto > + - amlogic,g12a-crypto > > reg: > maxItems: 1 > > interrupts: > - items: > - - description: Interrupt for flow 0 > - - description: Interrupt for flow 1 > + minItems: 1 > + maxItems: 2 Just keep the descrptions here and add 'minItems: 1'. Then in the if/then schema, you only need minItems and maxItems to require 1 or 2 interrupts. > > clocks: > maxItems: 1 > > + power-domains: > + maxItems: 1 > + > clock-names: > const: blkmv > > @@ -32,8 +42,27 @@ required: > - compatible > - reg > - interrupts > - - clocks > - - clock-names New chips work without clocks? Cool! Rob
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
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 >
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