RE: virtcrypto_dataq_callback calls crypto_finalize_request() from irq context
> -Original Message- > From: Michael S. Tsirkin [mailto:m...@redhat.com] > Sent: Thursday, November 2, 2023 9:17 PM > To: Gonglei (Arei) > Cc: Halil Pasic ; Herbert Xu > ; Jason Wang ; > virtualization@lists.linux-foundation.org; linux-ker...@vger.kernel.org; > linux-cry...@vger.kernel.org; Marc Hartmayer > Subject: Re: virtcrypto_dataq_callback calls crypto_finalize_request() from > irq > context > > On Thu, Nov 02, 2023 at 01:04:07PM +, Gonglei (Arei) wrote: > > > > > > > -Original Message- > > > From: Michael S. Tsirkin [mailto:m...@redhat.com] > > > Sent: Wednesday, November 1, 2023 9:26 PM > > > To: Halil Pasic > > > Cc: Gonglei (Arei) ; Herbert Xu > > > ; Jason Wang ; > > > virtualization@lists.linux-foundation.org; > > > linux-ker...@vger.kernel.org; linux-cry...@vger.kernel.org; Marc > > > Hartmayer > > > Subject: Re: virtcrypto_dataq_callback calls > > > crypto_finalize_request() from irq context > > > > > > On Sun, Sep 24, 2023 at 07:39:41PM +0200, Halil Pasic wrote: > > > > On Sun, 24 Sep 2023 11:56:25 + "Gonglei (Arei)" > > > > wrote: > > > > > > > > > Hi Halil, > > > > > > > > > > Commit 4058cf08945 introduced a check for detecting crypto > > > > > completion function called with enable BH, and indeed the > > > > > virtio-crypto driver didn't disable BH, which needs a patch to fix it. > > > > > > > > > > P.S.: > > > > > https://lore.kernel.org/lkml/20220221120833.2618733-5-clabbe@bay > > > > > libr > > > > > e.com/T/ > > > > > > > > > > Regards, > > > > > -Gonglei > > > > > > > > Thanks Gonglei! > > > > > > > > Thanks! I would be glad to test that fix on s390x. Are you about > > > > to send one? > > > > > > > > Regards, > > > > Halil > > > > > > > > > Gonglei did you intend to send a fix? > > > > Actually I sent a patch a month ago, pls see another thread. > > > > > > Regards, > > -Gonglei > > And I think there was an issue with that patch that you wanted to fix? > config changed callback got fixed but this still didn't. > Now my concern is whether or not the judgement (commit 4058cf08945c1) is reasonable. Regards, -Gonglei ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
RE: virtcrypto_dataq_callback calls crypto_finalize_request() from irq context
> -Original Message- > From: Michael S. Tsirkin [mailto:m...@redhat.com] > Sent: Wednesday, November 1, 2023 9:26 PM > To: Halil Pasic > Cc: Gonglei (Arei) ; Herbert Xu > ; Jason Wang ; > virtualization@lists.linux-foundation.org; linux-ker...@vger.kernel.org; > linux-cry...@vger.kernel.org; Marc Hartmayer > Subject: Re: virtcrypto_dataq_callback calls crypto_finalize_request() from > irq > context > > On Sun, Sep 24, 2023 at 07:39:41PM +0200, Halil Pasic wrote: > > On Sun, 24 Sep 2023 11:56:25 + > > "Gonglei (Arei)" wrote: > > > > > Hi Halil, > > > > > > Commit 4058cf08945 introduced a check for detecting crypto > > > completion function called with enable BH, and indeed the > > > virtio-crypto driver didn't disable BH, which needs a patch to fix it. > > > > > > P.S.: > > > https://lore.kernel.org/lkml/20220221120833.2618733-5-clabbe@baylibr > > > e.com/T/ > > > > > > Regards, > > > -Gonglei > > > > Thanks Gonglei! > > > > Thanks! I would be glad to test that fix on s390x. Are you about to > > send one? > > > > Regards, > > Halil > > > Gonglei did you intend to send a fix? Actually I sent a patch a month ago, pls see another thread. Regards, -Gonglei ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: virtcrypto_dataq_callback calls crypto_finalize_request() from irq context
On Thu, Nov 02, 2023 at 01:04:07PM +, Gonglei (Arei) wrote: > > > > -Original Message- > > From: Michael S. Tsirkin [mailto:m...@redhat.com] > > Sent: Wednesday, November 1, 2023 9:26 PM > > To: Halil Pasic > > Cc: Gonglei (Arei) ; Herbert Xu > > ; Jason Wang ; > > virtualization@lists.linux-foundation.org; linux-ker...@vger.kernel.org; > > linux-cry...@vger.kernel.org; Marc Hartmayer > > Subject: Re: virtcrypto_dataq_callback calls crypto_finalize_request() from > > irq > > context > > > > On Sun, Sep 24, 2023 at 07:39:41PM +0200, Halil Pasic wrote: > > > On Sun, 24 Sep 2023 11:56:25 + > > > "Gonglei (Arei)" wrote: > > > > > > > Hi Halil, > > > > > > > > Commit 4058cf08945 introduced a check for detecting crypto > > > > completion function called with enable BH, and indeed the > > > > virtio-crypto driver didn't disable BH, which needs a patch to fix it. > > > > > > > > P.S.: > > > > https://lore.kernel.org/lkml/20220221120833.2618733-5-clabbe@baylibr > > > > e.com/T/ > > > > > > > > Regards, > > > > -Gonglei > > > > > > Thanks Gonglei! > > > > > > Thanks! I would be glad to test that fix on s390x. Are you about to > > > send one? > > > > > > Regards, > > > Halil > > > > > > Gonglei did you intend to send a fix? > > Actually I sent a patch a month ago, pls see another thread. > > > Regards, > -Gonglei And I think there was an issue with that patch that you wanted to fix? config changed callback got fixed but this still didn't. -- MST ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: virtcrypto_dataq_callback calls crypto_finalize_request() from irq context
On Sun, Sep 24, 2023 at 07:39:41PM +0200, Halil Pasic wrote: > On Sun, 24 Sep 2023 11:56:25 + > "Gonglei (Arei)" wrote: > > > Hi Halil, > > > > Commit 4058cf08945 introduced a check for detecting crypto completion > > function > > called with enable BH, and indeed the virtio-crypto driver didn't disable > > BH, which needs > > a patch to fix it. > > > > P.S.: > > https://lore.kernel.org/lkml/20220221120833.2618733-5-cla...@baylibre.com/T/ > > > > Regards, > > -Gonglei > > Thanks Gonglei! > > Thanks! I would be glad to test that fix on s390x. Are you about to send > one? > > Regards, > Halil Gonglei did you intend to send a fix? ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
Re: virtcrypto_dataq_callback calls crypto_finalize_request() from irq context
On Sun, 24 Sep 2023 11:56:25 + "Gonglei (Arei)" wrote: > Hi Halil, > > Commit 4058cf08945 introduced a check for detecting crypto completion > function > called with enable BH, and indeed the virtio-crypto driver didn't disable BH, > which needs > a patch to fix it. > > P.S.: > https://lore.kernel.org/lkml/20220221120833.2618733-5-cla...@baylibre.com/T/ > > Regards, > -Gonglei Thanks Gonglei! Thanks! I would be glad to test that fix on s390x. Are you about to send one? Regards, Halil ___ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
RE: virtcrypto_dataq_callback calls crypto_finalize_request() from irq context
Hi Halil,
Commit 4058cf08945 introduced a check for detecting crypto completion function
called with enable BH, and indeed the virtio-crypto driver didn't disable BH,
which needs
a patch to fix it.
P.S.:
https://lore.kernel.org/lkml/20220221120833.2618733-5-cla...@baylibre.com/T/
Regards,
-Gonglei
> -Original Message-
> From: Halil Pasic [mailto:pa...@linux.ibm.com]
> Sent: Friday, September 22, 2023 9:46 PM
> To: Gonglei (Arei)
> Cc: Halil Pasic ; Herbert Xu
> ; Michael S. Tsirkin ;
> Jason Wang ;
> virtualization@lists.linux-foundation.org; linux-ker...@vger.kernel.org;
> linux-cry...@vger.kernel.org; Marc Hartmayer
> Subject: BUG: virtcrypto_dataq_callback calls crypto_finalize_request() from
> irq
> context
>
> Hi Gonglei!
>
> Our CI has found that virtio-crypto does not honor the requirement of
> crypto_finalize_request() being called in softirq context which is asserted in
> that function via lockdep_assert_in_softirq() since commit 4058cf08945c
> ("crypto: engine - check if BH is disabled during completion").
>
> The problem was originally found on s390x but Marc Hartmayer was so kind to
> reproduce it on amd64. Please find the corresponding kernel messages at the
> end of this email.
>
> The call chain looks like this.
> interrupt handler for queue notification --> virtcrypto_dataq_callback() -->
> via vc_req->alg_cb either virtio_crypto_skcipher_finalize_req()
> or virtio_crypto_akcipher_finalize_req()
> --> crypto_finalize_skcipher_request()
> or crypto_finalize_akcipher_request()
> --> crypto_finalize_request()
>
> Everything above is happening in the interrupt handler (and in "hard" irq
> context).
>
> I'm not really familiar with the implementation of virtio_crypto or with the
> crypto_engine interfaces. I assume the problem is on the side of virtio-crypto
> so I would like to kindly ask you as the maintainer of virtio-crypt to have a
> look
> at it. But if you think it is rather in the crypto_engine, please clarify
> that with
> Herbert. I have no strong opinion on this issue.
>
> Regards,
> Halil
>
> [ 31.033415][ C0] WARNING: CPU: 0 PID: 136 at crypto/crypto_engine.c:58
> crypto_finalize_request (crypto/crypto_engine.c:58 (discriminator 23))
> crypto_engine
> [ 31.034131][C0] Modules linked in: virtio_crypto(+)
> vmw_vsock_virtio_transport_common(+) crypto_engine vsock
> [ 31.035326][C0] Hardware name: QEMU Standard PC (i440FX + PIIX,
> 1996), BIOS 1.16.2-1.fc38 04/01/2014
> [ 31.035917][ C0] RIP: 0010:crypto_finalize_request (crypto/crypto_engine.c:58
> (discriminator 23)) crypto_engine [ 31.036398][ C0] Code: 08 5b 5d 41 5c 41 5d
> e9 bf 88 1c c1 65 8b 05 b0 36 01 40 f6 c4 ff 74 12 a9 00 00 0f 00 75 0b a9 00
> 00
> f0 00 0f 84 54 ff ff ff <0f> 0b e9 4d ff ff ff 4c 8d 6b 38 4c 89 ef e8 8e 47
> 1b c4 48
> 8d bb All code
>0: 08 5b 5dor %bl,0x5d(%rbx)
>3: 41 5c pop%r12
>5: 41 5d pop%r13
>7: e9 bf 88 1c c1 jmp0xc11c88cb
>c: 65 8b 05 b0 36 01 40mov%gs:0x400136b0(%rip),%eax
> # 0x400136c3
> 13: f6 c4 fftest $0xff,%ah
> 16: 74 12 je 0x2a
> 18: a9 00 00 0f 00 test $0xf,%eax
> 1d: 75 0b jne0x2a
> 1f: a9 00 00 f0 00 test $0xf0,%eax
> 24: 0f 84 54 ff ff ff je 0xff7e
> 2a:*0f 0b ud2 <-- trapping instruction
> 2c: e9 4d ff ff ff jmp0xff7e
> 31: 4c 8d 6b 38 lea0x38(%rbx),%r13
> 35: 4c 89 efmov%r13,%rdi
> 38: e8 8e 47 1b c4 call 0xc41b47cb
> 3d: 48 rex.W
> 3e: 8d .byte 0x8d
> 3f: bb .byte 0xbb
>
> Code starting with the faulting instruction
> ===
>0: 0f 0b ud2
>2: e9 4d ff ff ff jmp0xff54
>7: 4c 8d 6b 38 lea0x38(%rbx),%r13
>b: 4c 89 efmov%r13,%rdi
>e: e8 8e 47 1b c4 call 0xc41b47a1
> 13: 48 rex.W
> 14: 8d .byte 0x8d
> 15: bb .byte 0xbb
> [ 31.037591][C0] RSP: 0018:c9007da0 EFLAGS: 00010046
> [ 31.037976][C0] RAX: 80010002 RBX: 888006c87428 RCX:
> 10c0e523
> [ 31.038471][C0] RDX: RSI: 88810d0819e8 RDI:
> 888006c87449
> [ 31.038967][C0] RBP: 88810d0819e8 R08: R09:
> fbfff0b04f04
> [ 31.039463][C0] R10: 85827823 R11: 842013e6 R12:
>
> [ 31.039963][C0] R13: 0001 R14: 88810d081a18 R15:
> dc00
> [ 31.040475][C0] FS: 7f80c0cc6800()
> GS:88811ae0() knlGS:
> [ 31.041058][C0]
