On Tue, 26 Sep 2023 18:41:58 +0200 Halil Pasic <pasic@xxxxxxxxxxxxx> wrote: > > + local_bh_disable(); > > crypto_finalize_akcipher_request(vc_akcipher_req->base.dataq->engine, req, err); > > + local_bh_enable(); > > Thanks Gonglei! > > I did this a quick spin, and it does not seem to be sufficient on s390x. > Which does not come as a surprise to me, because > > #define lockdep_assert_in_softirq() \ > do { \ > WARN_ON_ONCE(__lockdep_enabled && \ > (!in_softirq() || in_irq() || in_nmi())); \ > } while (0) > > will still warn because in_irq() still evaluates to true (your patch > addresses the !in_softirq() part). > > I don't have any results on x86 yet. My current understanding is that the > virtio-pci transport code disables interrupts locally somewhere in the > call chain (actually in vp_vring_interrupt() via spin_lock_irqsave()) > and then x86 would be fine. But I will get that verified. [ 35.177962][ C0] WARNING: CPU: 0 PID: 152 at kernel/softirq.c:306 __local_bh_disable_ip (kernel/softirq.c:306 (discriminator 1)) [ 35.178551][ C0] Modules linked in: vmw_vsock_virtio_transport(+) vmw_vsock_virtio_transport_common virtio_crypto(+) crypto_engine vsock [ 35.179930][ C0] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.2-1.fc38 04/01/2014 [ 35.180548][ C0] RIP: 0010:__local_bh_disable_ip (kernel/softirq.c:306 (discriminator 1)) [ 35.180936][ C0] Code: eb 7d 65 8b 05 ef 90 eb 7d 31 f0 f6 c4 ff 74 13 9c 58 f6 c4 02 75 17 80 e7 02 74 01 fb 5b c3 cc cc cc cc e8 48 2f 15 00 eb e6 <0f> 0b eb ca e8 2d 88 03 03 eb e2 66 66 2e 0f 1f 84 00 00 00 00 00 All code ======== 0: eb 7d jmp 0x7f 2: 65 8b 05 ef 90 eb 7d mov %gs:0x7deb90ef(%rip),%eax # 0x7deb90f8 9: 31 f0 xor %esi,%eax b: f6 c4 ff test $0xff,%ah e: 74 13 je 0x23 10: 9c pushf 11: 58 pop %rax 12: f6 c4 02 test $0x2,%ah 15: 75 17 jne 0x2e 17: 80 e7 02 and $0x2,%bh 1a: 74 01 je 0x1d 1c: fb sti 1d: 5b pop %rbx 1e: c3 ret 1f: cc int3 20: cc int3 21: cc int3 22: cc int3 23: e8 48 2f 15 00 call 0x152f70 28: eb e6 jmp 0x10 2a:* 0f 0b ud2 <-- trapping instruction 2c: eb ca jmp 0xfffffffffffffff8 2e: e8 2d 88 03 03 call 0x3038860 33: eb e2 jmp 0x17 35: 66 66 2e 0f 1f 84 00 data16 cs nopw 0x0(%rax,%rax,1) 3c: 00 00 00 00 Code starting with the faulting instruction =========================================== 0: 0f 0b ud2 2: eb ca jmp 0xffffffffffffffce 4: e8 2d 88 03 03 call 0x3038836 9: eb e2 jmp 0xffffffffffffffed b: 66 66 2e 0f 1f 84 00 data16 cs nopw 0x0(%rax,%rax,1) 12: 00 00 00 00 [ 35.182237][ C0] RSP: 0018:ffffc90000007d88 EFLAGS: 00010006 [ 35.182637][ C0] RAX: 0000000080010003 RBX: ffff888108308538 RCX: ffffc90000007d50 [ 35.183186][ C0] RDX: ffff88811ae36300 RSI: 0000000000000200 RDI: ffffffffc02b16cc [ 35.183700][ C0] RBP: ffff8881083084e8 R08: 0000000000000000 R09: fffffbfff0d04f04 [ 35.184216][ C0] R10: ffffffff86827823 R11: ffffffff852013e6 R12: 0000000000000001 [ 35.184730][ C0] R13: 0000000000000000 R14: ffff888108308538 R15: dffffc0000000000 [ 35.185248][ C0] FS: 00007f06cb551800(0000) GS:ffff88811ae00000(0000) knlGS:0000000000000000 [ 35.185831][ C0] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 35.186271][ C0] CR2: 000055dc93010628 CR3: 0000000116b28000 CR4: 00000000000006f0 [ 35.186789][ C0] Call Trace: [ 35.187010][ C0] <IRQ> [ 35.187204][ C0] ? __warn (kernel/panic.c:673) [ 35.187505][ C0] ? __local_bh_disable_ip (kernel/softirq.c:306 (discriminator 1)) [ 35.187857][ C0] ? report_bug (lib/bug.c:180 lib/bug.c:219) [ 35.188197][ C0] ? handle_bug (arch/x86/kernel/traps.c:237 (discriminator 1)) [ 35.188483][ C0] ? exc_invalid_op (arch/x86/kernel/traps.c:258 (discriminator 1)) [ 35.188790][ C0] ? asm_exc_invalid_op (./arch/x86/include/asm/idtentry.h:568) [ 35.189120][ C0] ? asm_common_interrupt (./arch/x86/include/asm/idtentry.h:636) [ 35.189466][ C0] ? virtio_crypto_dataq_sym_callback (drivers/crypto/virtio/virtio_crypto_skcipher_algs.c:567 drivers/crypto/virtio/virtio_crypto_skcipher_algs.c:81 drivers/crypto/virtio/virtio_crypto_skcipher_algs.c:55) virtio_crypto [ 35.189983][ C0] ? __local_bh_disable_ip (kernel/softirq.c:306 (discriminator 1)) [ 35.190336][ C0] virtio_crypto_dataq_sym_callback (drivers/crypto/virtio/virtio_crypto_skcipher_algs.c:570 drivers/crypto/virtio/virtio_crypto_skcipher_algs.c:81 drivers/crypto/virtio/virtio_crypto_skcipher_algs.c:55) virtio_crypto [ 35.190837][ C0] virtcrypto_dataq_callback (drivers/crypto/virtio/virtio_crypto_core.c:91) virtio_crypto [ 35.191304][ C0] ? __pfx_virtcrypto_dataq_callback (drivers/crypto/virtio/virtio_crypto_core.c:76) virtio_crypto [ 35.191796][ C0] ? __pfx_do_raw_spin_lock (kernel/locking/spinlock_debug.c:113) [ 35.192154][ C0] vring_interrupt (drivers/virtio/virtio_ring.c:2598) [ 35.192536][ C0] vp_vring_interrupt (drivers/virtio/virtio_pci_common.c:67 (discriminator 2)) [ 35.193064][ C0] ? __pfx_vp_vring_interrupt (drivers/virtio/virtio_pci_common.c:60) [ 35.193793][ C0] __handle_irq_event_percpu (kernel/irq/handle.c:158) [ 35.194272][ C0] handle_irq_event (kernel/irq/handle.c:195 kernel/irq/handle.c:210) [ 35.194587][ C0] handle_edge_irq (kernel/irq/chip.c:833) [ 35.194903][ C0] __common_interrupt (arch/x86/kernel/irq.c:271) [ 35.195232][ C0] common_interrupt (arch/x86/kernel/irq.c:247 (discriminator 47)) So I was wrong, this patch is not sufficient, not on x86 nor on s390x. And the problem is that we are in hardirq context. For some reason, I was under the impression that disabling interrupts in a hardirq context somehow takes you out of hardirq context and makes in_irq() return false. Silly me! (I was assuming the fix works on x86 and hallucinated based on that assumption and any differences I have found between virtio-ccw and virtio-pci.) Currently I don't see a need to fix anything in virtio-ccw. Regards, Halil