On Wed, Mar 8, 2023 at 4:40 AM Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> wrote: > On Wed, Mar 08, 2023 at 11:23:24AM +0800, Herbert Xu wrote: > > > > Nevermind, I know why it doesn't work. It's specific to ux500 > > where you're polling the DCAL bit. But the DCAL bit only works > > for the final hash, it doesn't work for the intermediate state. > > > > Let me check how the old ux500 handled this case. > > Hmm, it seems to use the same bit. I guess the meaning must be > different with ux500. Not sure if it's that or if we were just lucky, or the tests were not as extensive back on the old driver :/ > Could you check for me which wait_busy() call is actually failing? > Is it the one I added right before we save the state, or is it > something else? It times out - always - before writing the HMAC key in stm32_hash_xmit_cpu(). So this: stm32_hash_set_nblw(hdev, length); reg = stm32_hash_read(hdev, HASH_STR); reg |= HASH_STR_DCAL; stm32_hash_write(hdev, HASH_STR, reg); if (hdev->flags & HASH_FLAGS_HMAC) { - if (stm32_hash_wait_busy(hdev)) + if (stm32_hash_wait_busy(hdev)) { + dev_err(hdev->dev, + "timeout before writing key in stm32_hash_xmit_cpu()\n"); return -ETIMEDOUT; + } stm32_hash_write_key(hdev); } return -EINPROGRESS; Gives: [ 4.812106] stm32-hash a03c2000.hash: allocated hmac(sha256) fallback [ 5.008829] stm32-hash a03c2000.hash: timeout before writing key in stm32_hash_xmit_cpu() [ 5.017167] alg: ahash: stm32-hmac-sha256 final() failed with err -110 on test vector "random: psize=0 ksize=70", cfg="random: may_sleep use_final src_divs=[<fl" [ 5.041034] alg: self-tests for hmac(sha256) using stm32-hmac-sha256 failed (rc=-110) I put error messages at all other timeouts too but they do not trigger. This is why non-HMAC works fine all of the time. > If it's something perhaps we aren't restoring the state in the > right way, because the stm32 state restoring code is quite different > compared to the ux500 code. It's just the HMAC that is failing so I'm puzzled, because until this point it is essentially a clean SHA sum. I also noticed that it only happens with long keys (more than 64 bytes) that need to set bit 16 (HASH_CR_LKEY) in the control register. > Could you also confirm that the old ux500 driver actually passes > all the extra tests on your hardware? It literally saves and > restores the state every 256 bytes :) I had this old patch set where I tried to clean up the old driver: https://lore.kernel.org/linux-crypto/20220816140049.102306-1-linus.walleij@xxxxxxxxxx/ I put this old patch set on top of v6.0. First I enabled the old crypto driver without extended tests: all works fine. Then I enabled the extended tests. It does seem like it has the same problem! [ 25.486878] CPU: 1 PID: 91 Comm: cryptomgr_test Not tainted 6.0.0-00016-g23791565aac5 #3 [ 25.494967] Hardware name: ST-Ericsson Ux5x0 platform (Device Tree Support) [ 25.501932] unwind_backtrace from show_stack+0x10/0x14 [ 25.507175] show_stack from dump_stack_lvl+0x40/0x4c [ 25.512236] dump_stack_lvl from nmi_cpu_backtrace+0xd4/0x104 [ 25.517988] nmi_cpu_backtrace from nmi_trigger_cpumask_backtrace+0xdc/0x128 [ 25.525034] nmi_trigger_cpumask_backtrace from trigger_single_cpu_backtrace+0x24/0x2c [ 25.532952] trigger_single_cpu_backtrace from rcu_dump_cpu_stacks+0x10c/0x150 [ 25.540173] rcu_dump_cpu_stacks from print_cpu_stall+0x14c/0x1d0 [ 25.546268] print_cpu_stall from check_cpu_stall+0x1cc/0x26c [ 25.552013] check_cpu_stall from rcu_sched_clock_irq+0x74/0x16c [ 25.558017] rcu_sched_clock_irq from update_process_times+0x68/0x94 [ 25.564377] update_process_times from tick_sched_timer+0x60/0xec [ 25.570470] tick_sched_timer from __hrtimer_run_queues+0x15c/0x218 [ 25.576737] __hrtimer_run_queues from hrtimer_interrupt+0x124/0x2b0 [ 25.583090] hrtimer_interrupt from twd_handler+0x34/0x3c [ 25.588489] twd_handler from handle_percpu_devid_irq+0x78/0x134 [ 25.594498] handle_percpu_devid_irq from generic_handle_domain_irq+0x24/0x34 [ 25.601640] generic_handle_domain_irq from gic_handle_irq+0x74/0x88 [ 25.608000] gic_handle_irq from generic_handle_arch_irq+0x34/0x44 [ 25.614187] generic_handle_arch_irq from call_with_stack+0x18/0x20 [ 25.620462] call_with_stack from __irq_svc+0x98/0xb0 [ 25.625514] Exception stack(0xf10ad970 to 0xf10ad9b8) [ 25.630563] d960: c1d59a00 40000013 c056bbfc 00005f32 [ 25.638734] d980: 00000000 00000008 f10ad9e0 00000020 c1f69bc0 c1f69bc0 c2364140 f10adb34 [ 25.646904] d9a0: 00000000 f10ad9c0 c056da50 c099d1ec 20000013 ffffffff [ 25.653512] __irq_svc from _raw_spin_unlock_irqrestore+0x1c/0x20 [ 25.659605] _raw_spin_unlock_irqrestore from regmap_read+0x50/0x60 [ 25.665882] regmap_read from hash_hw_write_key+0xd8/0x100 [ 25.671377] hash_hw_write_key from init_hash_hw+0xd8/0xfc [ 25.676862] init_hash_hw from hash_hw_final+0x88/0x36c [ 25.682089] hash_hw_final from ahash_final+0x58/0x9c [ 25.687138] ahash_final from do_ahash_op+0x20/0x98 [ 25.692019] do_ahash_op from test_ahash_vec_cfg+0x68c/0x984 [ 25.697677] test_ahash_vec_cfg from test_hash_vec+0x64/0x168 [ 25.703421] test_hash_vec from __alg_test_hash+0x158/0x2d8 [ 25.708991] __alg_test_hash from alg_test_hash+0xc0/0x170 [ 25.714474] alg_test_hash from alg_test.part.0+0x378/0x4b8 [ 25.720044] alg_test.part.0 from cryptomgr_test+0x24/0x44 [ 25.725537] cryptomgr_test from kthread+0xc0/0xc4 [ 25.730334] kthread from ret_from_fork+0x14/0x2c [ 25.735036] Exception stack(0xf10adfb0 to 0xf10adff8) [ 25.740081] dfa0: 00000000 00000000 00000000 00000000 [ 25.748252] dfc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 [ 25.756422] dfe0: 00000000 00000000 00000000 00000000 00000013 00000000 I can't see 100% if it is the same issue but it does hang on something related to writing keys. So for Ux500 at least I suppose it would be best to inhibit .import and .export when using HMAC with long keys unless I can figure out exactly what the issue is here. I wonder if that is possible? Or do I have to remove it from the HMAC algos altogether? Yours, Linus Walleij