On Tue, 26 Sep 2023 18:41:58 +0200
Halil Pasic <pa...@linux.ibm.com> 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


_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Reply via email to