Re: [PATCH] irqbypass: fix error handle when irq_bypass_register_producer() return fails
Hi Alex
According to your suggestion, the final patch is as follows:
diff --git a/drivers/vfio/pci/vfio_pci_intrs.c
b/drivers/vfio/pci/vfio_pci_intrs.c
index 1d9fb25..869dce5 100644
--- a/drivers/vfio/pci/vfio_pci_intrs.c
+++ b/drivers/vfio/pci/vfio_pci_intrs.c
@@ -352,11 +352,13 @@ static int vfio_msi_set_vector_signal(struct
vfio_pci_device *vdev,
vdev->ctx[vector].producer.token = trigger;
vdev->ctx[vector].producer.irq = irq;
ret = irq_bypass_register_producer(>ctx[vector].producer);
- if (unlikely(ret))
+ if (unlikely(ret)) {
dev_info(>dev,
"irq bypass producer (token %p) registration fails: %d\n",
vdev->ctx[vector].producer.token, ret);
+ vdev->ctx[vector].producer.token = NULL;
+ }
vdev->ctx[vector].trigger = trigger;
return 0;
--
I applied this patch, after several days of testing, it is now normal
and the BUG no longer occurs.
Now the kernel will only print the log of registration failure,
Thanks Alex.
Alex Williamson 于2020年10月10日周六 下午10:26写道：
>
> On Sat, 10 Oct 2020 19:01:30 +0800
> gchen chen wrote:
>
> > Alex Williamson 于2020年10月10日周六 上午2:44写道：
> > >
> > > On Fri, 9 Oct 2020 12:30:04 +0800
> > > gchen chen wrote:
> > >
> > > > Alex Williamson 于2020年9月30日周三 下午10:09写道：
> > > > >
> > > > >
> > > > > Please version your postings so we know which one to consider as the
> > > > > current proposal.
> > > > >
> > > > > On Wed, 30 Sep 2020 20:54:39 +0800
> > > > > guomin_c...@sina.com wrote:
> > > > >
> > > > > > From: guomin chen
> > > > > >
> > > > > > When the producer object registration fails,In the future, due to
> > > > > > incorrect matching when unregistering, list_del(>node)
> > > > > > may still be called, then trigger a BUG:
> > > > > >
> > > > > > vfio-pci :db:00.0: irq bypass producer (token
> > > > > > 60c8cda5) registration fails: -16
> > > > > > vfio-pci :db:00.0: irq bypass producer (token
> > > > > > 60c8cda5) registration fails: -16
> > > > > > vfio-pci :db:00.0: irq bypass producer (token
> > > > > > 60c8cda5) registration fails: -16
> > > > > > ...
> > > > > > list_del corruption, 8f7fb8ba0828->next is LIST_POISON1
> > > > > > (dead0100)
> > > > > > [ cut here ]
> > > > > > kernel BUG at lib/list_debug.c:47!
> > > > > > invalid opcode: [#1] SMP NOPTI
> > > > > > CPU: 29 PID: 3914 Comm: qemu-kvm Kdump: loaded Tainted: G E
> > > > > > - -4.18.0-193.6.3.el8.x86_64 #1
> > > > > > Hardware name: Lenovo ThinkSystem SR650
> > > > > > -[7X06CTO1WW]-/-[7X06CTO1WW]-,
> > > > > > BIOS -[IVE636Z-2.13]- 07/18/2019
> > > > > > RIP: 0010:__list_del_entry_valid.cold.1+0x12/0x4c
> > > > > > Code: ce ff 0f 0b 48 89 c1 4c 89 c6 48 c7 c7 40 85 4d 88 e8 8c
> > > > > > bc
> > > > > > ce ff 0f 0b 48 89 fe 48 89 c2 48 c7 c7 d0 85 4d 88 e8 78
> > > > > > bc
> > > > > > ce ff <0f> 0b 48 c7 c7 80 86 4d 88 e8 6a bc ce ff 0f 0b 48
> > > > > > 89 f2 48 89 fe
> > > > > > RSP: 0018:aa9d60197d20 EFLAGS: 00010246
> > > > > > RAX: 004e RBX: 8f7fb8ba0828 RCX:
> > > > > >
> > > > > > RDX: RSI: 8f7fbf4d6a08 RDI:
> > > > > > 8f7fbf4d6a08
> > > > > > RBP: R08: 084b R09:
> > > > > > 005d
> > > > > > R10: R11: aa9d60197bd0 R12:
> > > > > > 8f4fbe863000
> > > > > > R13: 00c2 R14: R15:
> > > > > >
> > > > > > FS: 7f7cb97fa700() GS:8f7fbf4c()
> > > > > > knlGS:
> > > > > > CS: 0010 DS: ES: CR0: 80050033
> > > > > > CR2: 7fcf31da4000 CR3: 005f6d404001 CR4:
> > > > > > 007626e0
> > > > > > DR0: DR1: DR2:
> > > > > >
> > > > > > DR3: DR6: fffe0ff0 DR7:
> > > > > > 0400
> > > > > > PKRU: 5554
> > > > > > Call Trace:
> > > > > > irq_bypass_unregister_producer+0x9b/0xf0 [irqbypass]
> > > > > > vfio_msi_set_vector_signal+0x8c/0x290 [vfio_pci]
> > > > > > ? load_fixmap_gdt+0x22/0x30
> > > > > > vfio_msi_set_block+0x6e/0xd0 [vfio_pci]
> > > > > > vfio_pci_ioctl+0x218/0xbe0 [vfio_pci]
> > > > > > ? kvm_vcpu_ioctl+0xf2/0x5f0 [kvm]
> > > > > > do_vfs_ioctl+0xa4/0x630
> > > > > > ? syscall_trace_enter+0x1d3/0x2c0
> > > > > > ksys_ioctl+0x60/0x90
> > > > > > __x64_sys_ioctl+0x16/0x20
> > > > > > do_syscall_64+0x5b/0x1a0
> > > > > > entry_SYSCALL_64_after_hwframe+0x65/0xca
> > > > > >
> > > > > > Cc: Alex Williamson
> > > > > > Cc: Cornelia Huck
> > > > > > Cc: Jiang Yi
> > > > > > Cc: Marc Zyngier
> > > > > > Cc: Peter Xu
> > > > > >
Re: [PATCH] irqbypass: fix error handle when irq_bypass_register_producer() return fails
On Sat, 10 Oct 2020 19:01:30 +0800
gchen chen wrote:
> Alex Williamson 于2020年10月10日周六 上午2:44写道：
> >
> > On Fri, 9 Oct 2020 12:30:04 +0800
> > gchen chen wrote:
> >
> > > Alex Williamson 于2020年9月30日周三 下午10:09写道：
> > > >
> > > >
> > > > Please version your postings so we know which one to consider as the
> > > > current proposal.
> > > >
> > > > On Wed, 30 Sep 2020 20:54:39 +0800
> > > > guomin_c...@sina.com wrote:
> > > >
> > > > > From: guomin chen
> > > > >
> > > > > When the producer object registration fails,In the future, due to
> > > > > incorrect matching when unregistering, list_del(>node)
> > > > > may still be called, then trigger a BUG:
> > > > >
> > > > > vfio-pci :db:00.0: irq bypass producer (token
> > > > > 60c8cda5) registration fails: -16
> > > > > vfio-pci :db:00.0: irq bypass producer (token
> > > > > 60c8cda5) registration fails: -16
> > > > > vfio-pci :db:00.0: irq bypass producer (token
> > > > > 60c8cda5) registration fails: -16
> > > > > ...
> > > > > list_del corruption, 8f7fb8ba0828->next is LIST_POISON1
> > > > > (dead0100)
> > > > > [ cut here ]
> > > > > kernel BUG at lib/list_debug.c:47!
> > > > > invalid opcode: [#1] SMP NOPTI
> > > > > CPU: 29 PID: 3914 Comm: qemu-kvm Kdump: loaded Tainted: G E
> > > > > - -4.18.0-193.6.3.el8.x86_64 #1
> > > > > Hardware name: Lenovo ThinkSystem SR650
> > > > > -[7X06CTO1WW]-/-[7X06CTO1WW]-,
> > > > > BIOS -[IVE636Z-2.13]- 07/18/2019
> > > > > RIP: 0010:__list_del_entry_valid.cold.1+0x12/0x4c
> > > > > Code: ce ff 0f 0b 48 89 c1 4c 89 c6 48 c7 c7 40 85 4d 88 e8 8c bc
> > > > > ce ff 0f 0b 48 89 fe 48 89 c2 48 c7 c7 d0 85 4d 88 e8 78 bc
> > > > > ce ff <0f> 0b 48 c7 c7 80 86 4d 88 e8 6a bc ce ff 0f 0b 48
> > > > > 89 f2 48 89 fe
> > > > > RSP: 0018:aa9d60197d20 EFLAGS: 00010246
> > > > > RAX: 004e RBX: 8f7fb8ba0828 RCX:
> > > > > RDX: RSI: 8f7fbf4d6a08 RDI: 8f7fbf4d6a08
> > > > > RBP: R08: 084b R09: 005d
> > > > > R10: R11: aa9d60197bd0 R12: 8f4fbe863000
> > > > > R13: 00c2 R14: R15:
> > > > > FS: 7f7cb97fa700() GS:8f7fbf4c()
> > > > > knlGS:
> > > > > CS: 0010 DS: ES: CR0: 80050033
> > > > > CR2: 7fcf31da4000 CR3: 005f6d404001 CR4: 007626e0
> > > > > DR0: DR1: DR2:
> > > > > DR3: DR6: fffe0ff0 DR7: 0400
> > > > > PKRU: 5554
> > > > > Call Trace:
> > > > > irq_bypass_unregister_producer+0x9b/0xf0 [irqbypass]
> > > > > vfio_msi_set_vector_signal+0x8c/0x290 [vfio_pci]
> > > > > ? load_fixmap_gdt+0x22/0x30
> > > > > vfio_msi_set_block+0x6e/0xd0 [vfio_pci]
> > > > > vfio_pci_ioctl+0x218/0xbe0 [vfio_pci]
> > > > > ? kvm_vcpu_ioctl+0xf2/0x5f0 [kvm]
> > > > > do_vfs_ioctl+0xa4/0x630
> > > > > ? syscall_trace_enter+0x1d3/0x2c0
> > > > > ksys_ioctl+0x60/0x90
> > > > > __x64_sys_ioctl+0x16/0x20
> > > > > do_syscall_64+0x5b/0x1a0
> > > > > entry_SYSCALL_64_after_hwframe+0x65/0xca
> > > > >
> > > > > Cc: Alex Williamson
> > > > > Cc: Cornelia Huck
> > > > > Cc: Jiang Yi
> > > > > Cc: Marc Zyngier
> > > > > Cc: Peter Xu
> > > > > Cc: Eric Auger
> > > > > Cc: "Michael S. Tsirkin"
> > > > > Cc: Jason Wang
> > > > > Cc: k...@vger.kernel.org
> > > > > Cc: linux-kernel@vger.kernel.org
> > > > > Signed-off-by: guomin chen
> > > > > ---
> > > > > drivers/vfio/pci/vfio_pci_intrs.c | 13 +++--
> > > > > drivers/vhost/vdpa.c | 7 +++
> > > > > 2 files changed, 18 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/vfio/pci/vfio_pci_intrs.c
> > > > > b/drivers/vfio/pci/vfio_pci_intrs.c
> > > > > index 1d9fb25..c371943 100644
> > > > > --- a/drivers/vfio/pci/vfio_pci_intrs.c
> > > > > +++ b/drivers/vfio/pci/vfio_pci_intrs.c
> > > > > @@ -352,12 +352,21 @@ static int vfio_msi_set_vector_signal(struct
> > > > > vfio_pci_device *vdev,
> > > > > vdev->ctx[vector].producer.token = trigger;
> > > > > vdev->ctx[vector].producer.irq = irq;
> > > > > ret = irq_bypass_register_producer(>ctx[vector].producer);
> > > > > - if (unlikely(ret))
> > > > > + if (unlikely(ret)) {
> > > > > dev_info(>dev,
> > > > > "irq bypass producer (token %p) registration fails:
> > > > > %d\n",
> > > > > vdev->ctx[vector].producer.token, ret);
> > > > >
> > > > > - vdev->ctx[vector].trigger = trigger;
> > > > > + kfree(vdev->ctx[vector].name);
> > > > > +
Re: [PATCH] irqbypass: fix error handle when irq_bypass_register_producer() return fails
Alex Williamson 于2020年10月10日周六 上午2:44写道：
>
> On Fri, 9 Oct 2020 12:30:04 +0800
> gchen chen wrote:
>
> > Alex Williamson 于2020年9月30日周三 下午10:09写道：
> > >
> > >
> > > Please version your postings so we know which one to consider as the
> > > current proposal.
> > >
> > > On Wed, 30 Sep 2020 20:54:39 +0800
> > > guomin_c...@sina.com wrote:
> > >
> > > > From: guomin chen
> > > >
> > > > When the producer object registration fails,In the future, due to
> > > > incorrect matching when unregistering, list_del(>node)
> > > > may still be called, then trigger a BUG:
> > > >
> > > > vfio-pci :db:00.0: irq bypass producer (token 60c8cda5)
> > > > registration fails: -16
> > > > vfio-pci :db:00.0: irq bypass producer (token 60c8cda5)
> > > > registration fails: -16
> > > > vfio-pci :db:00.0: irq bypass producer (token 60c8cda5)
> > > > registration fails: -16
> > > > ...
> > > > list_del corruption, 8f7fb8ba0828->next is LIST_POISON1
> > > > (dead0100)
> > > > [ cut here ]
> > > > kernel BUG at lib/list_debug.c:47!
> > > > invalid opcode: [#1] SMP NOPTI
> > > > CPU: 29 PID: 3914 Comm: qemu-kvm Kdump: loaded Tainted: G E
> > > > - -4.18.0-193.6.3.el8.x86_64 #1
> > > > Hardware name: Lenovo ThinkSystem SR650
> > > > -[7X06CTO1WW]-/-[7X06CTO1WW]-,
> > > > BIOS -[IVE636Z-2.13]- 07/18/2019
> > > > RIP: 0010:__list_del_entry_valid.cold.1+0x12/0x4c
> > > > Code: ce ff 0f 0b 48 89 c1 4c 89 c6 48 c7 c7 40 85 4d 88 e8 8c bc
> > > > ce ff 0f 0b 48 89 fe 48 89 c2 48 c7 c7 d0 85 4d 88 e8 78 bc
> > > > ce ff <0f> 0b 48 c7 c7 80 86 4d 88 e8 6a bc ce ff 0f 0b 48
> > > > 89 f2 48 89 fe
> > > > RSP: 0018:aa9d60197d20 EFLAGS: 00010246
> > > > RAX: 004e RBX: 8f7fb8ba0828 RCX:
> > > > RDX: RSI: 8f7fbf4d6a08 RDI: 8f7fbf4d6a08
> > > > RBP: R08: 084b R09: 005d
> > > > R10: R11: aa9d60197bd0 R12: 8f4fbe863000
> > > > R13: 00c2 R14: R15:
> > > > FS: 7f7cb97fa700() GS:8f7fbf4c()
> > > > knlGS:
> > > > CS: 0010 DS: ES: CR0: 80050033
> > > > CR2: 7fcf31da4000 CR3: 005f6d404001 CR4: 007626e0
> > > > DR0: DR1: DR2:
> > > > DR3: DR6: fffe0ff0 DR7: 0400
> > > > PKRU: 5554
> > > > Call Trace:
> > > > irq_bypass_unregister_producer+0x9b/0xf0 [irqbypass]
> > > > vfio_msi_set_vector_signal+0x8c/0x290 [vfio_pci]
> > > > ? load_fixmap_gdt+0x22/0x30
> > > > vfio_msi_set_block+0x6e/0xd0 [vfio_pci]
> > > > vfio_pci_ioctl+0x218/0xbe0 [vfio_pci]
> > > > ? kvm_vcpu_ioctl+0xf2/0x5f0 [kvm]
> > > > do_vfs_ioctl+0xa4/0x630
> > > > ? syscall_trace_enter+0x1d3/0x2c0
> > > > ksys_ioctl+0x60/0x90
> > > > __x64_sys_ioctl+0x16/0x20
> > > > do_syscall_64+0x5b/0x1a0
> > > > entry_SYSCALL_64_after_hwframe+0x65/0xca
> > > >
> > > > Cc: Alex Williamson
> > > > Cc: Cornelia Huck
> > > > Cc: Jiang Yi
> > > > Cc: Marc Zyngier
> > > > Cc: Peter Xu
> > > > Cc: Eric Auger
> > > > Cc: "Michael S. Tsirkin"
> > > > Cc: Jason Wang
> > > > Cc: k...@vger.kernel.org
> > > > Cc: linux-kernel@vger.kernel.org
> > > > Signed-off-by: guomin chen
> > > > ---
> > > > drivers/vfio/pci/vfio_pci_intrs.c | 13 +++--
> > > > drivers/vhost/vdpa.c | 7 +++
> > > > 2 files changed, 18 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/vfio/pci/vfio_pci_intrs.c
> > > > b/drivers/vfio/pci/vfio_pci_intrs.c
> > > > index 1d9fb25..c371943 100644
> > > > --- a/drivers/vfio/pci/vfio_pci_intrs.c
> > > > +++ b/drivers/vfio/pci/vfio_pci_intrs.c
> > > > @@ -352,12 +352,21 @@ static int vfio_msi_set_vector_signal(struct
> > > > vfio_pci_device *vdev,
> > > > vdev->ctx[vector].producer.token = trigger;
> > > > vdev->ctx[vector].producer.irq = irq;
> > > > ret = irq_bypass_register_producer(>ctx[vector].producer);
> > > > - if (unlikely(ret))
> > > > + if (unlikely(ret)) {
> > > > dev_info(>dev,
> > > > "irq bypass producer (token %p) registration fails: %d\n",
> > > > vdev->ctx[vector].producer.token, ret);
> > > >
> > > > - vdev->ctx[vector].trigger = trigger;
> > > > + kfree(vdev->ctx[vector].name);
> > > > + eventfd_ctx_put(trigger);
> > > > +
> > > > + cmd = vfio_pci_memory_lock_and_enable(vdev);
> > > > + free_irq(irq, trigger);
> > > > + vfio_pci_memory_unlock_and_restore(vdev, cmd);
> > > > +
> > > > + vdev->ctx[vector].trigger = NULL;
> > > > + }
Re: [PATCH] irqbypass: fix error handle when irq_bypass_register_producer() return fails
On Fri, 9 Oct 2020 12:30:04 +0800
gchen chen wrote:
> Alex Williamson 于2020年9月30日周三 下午10:09写道：
> >
> >
> > Please version your postings so we know which one to consider as the
> > current proposal.
> >
> > On Wed, 30 Sep 2020 20:54:39 +0800
> > guomin_c...@sina.com wrote:
> >
> > > From: guomin chen
> > >
> > > When the producer object registration fails,In the future, due to
> > > incorrect matching when unregistering, list_del(>node)
> > > may still be called, then trigger a BUG:
> > >
> > > vfio-pci :db:00.0: irq bypass producer (token 60c8cda5)
> > > registration fails: -16
> > > vfio-pci :db:00.0: irq bypass producer (token 60c8cda5)
> > > registration fails: -16
> > > vfio-pci :db:00.0: irq bypass producer (token 60c8cda5)
> > > registration fails: -16
> > > ...
> > > list_del corruption, 8f7fb8ba0828->next is LIST_POISON1
> > > (dead0100)
> > > [ cut here ]
> > > kernel BUG at lib/list_debug.c:47!
> > > invalid opcode: [#1] SMP NOPTI
> > > CPU: 29 PID: 3914 Comm: qemu-kvm Kdump: loaded Tainted: G E
> > > - -4.18.0-193.6.3.el8.x86_64 #1
> > > Hardware name: Lenovo ThinkSystem SR650 -[7X06CTO1WW]-/-[7X06CTO1WW]-,
> > > BIOS -[IVE636Z-2.13]- 07/18/2019
> > > RIP: 0010:__list_del_entry_valid.cold.1+0x12/0x4c
> > > Code: ce ff 0f 0b 48 89 c1 4c 89 c6 48 c7 c7 40 85 4d 88 e8 8c bc
> > > ce ff 0f 0b 48 89 fe 48 89 c2 48 c7 c7 d0 85 4d 88 e8 78 bc
> > > ce ff <0f> 0b 48 c7 c7 80 86 4d 88 e8 6a bc ce ff 0f 0b 48
> > > 89 f2 48 89 fe
> > > RSP: 0018:aa9d60197d20 EFLAGS: 00010246
> > > RAX: 004e RBX: 8f7fb8ba0828 RCX:
> > > RDX: RSI: 8f7fbf4d6a08 RDI: 8f7fbf4d6a08
> > > RBP: R08: 084b R09: 005d
> > > R10: R11: aa9d60197bd0 R12: 8f4fbe863000
> > > R13: 00c2 R14: R15:
> > > FS: 7f7cb97fa700() GS:8f7fbf4c()
> > > knlGS:
> > > CS: 0010 DS: ES: CR0: 80050033
> > > CR2: 7fcf31da4000 CR3: 005f6d404001 CR4: 007626e0
> > > DR0: DR1: DR2:
> > > DR3: DR6: fffe0ff0 DR7: 0400
> > > PKRU: 5554
> > > Call Trace:
> > > irq_bypass_unregister_producer+0x9b/0xf0 [irqbypass]
> > > vfio_msi_set_vector_signal+0x8c/0x290 [vfio_pci]
> > > ? load_fixmap_gdt+0x22/0x30
> > > vfio_msi_set_block+0x6e/0xd0 [vfio_pci]
> > > vfio_pci_ioctl+0x218/0xbe0 [vfio_pci]
> > > ? kvm_vcpu_ioctl+0xf2/0x5f0 [kvm]
> > > do_vfs_ioctl+0xa4/0x630
> > > ? syscall_trace_enter+0x1d3/0x2c0
> > > ksys_ioctl+0x60/0x90
> > > __x64_sys_ioctl+0x16/0x20
> > > do_syscall_64+0x5b/0x1a0
> > > entry_SYSCALL_64_after_hwframe+0x65/0xca
> > >
> > > Cc: Alex Williamson
> > > Cc: Cornelia Huck
> > > Cc: Jiang Yi
> > > Cc: Marc Zyngier
> > > Cc: Peter Xu
> > > Cc: Eric Auger
> > > Cc: "Michael S. Tsirkin"
> > > Cc: Jason Wang
> > > Cc: k...@vger.kernel.org
> > > Cc: linux-kernel@vger.kernel.org
> > > Signed-off-by: guomin chen
> > > ---
> > > drivers/vfio/pci/vfio_pci_intrs.c | 13 +++--
> > > drivers/vhost/vdpa.c | 7 +++
> > > 2 files changed, 18 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/vfio/pci/vfio_pci_intrs.c
> > > b/drivers/vfio/pci/vfio_pci_intrs.c
> > > index 1d9fb25..c371943 100644
> > > --- a/drivers/vfio/pci/vfio_pci_intrs.c
> > > +++ b/drivers/vfio/pci/vfio_pci_intrs.c
> > > @@ -352,12 +352,21 @@ static int vfio_msi_set_vector_signal(struct
> > > vfio_pci_device *vdev,
> > > vdev->ctx[vector].producer.token = trigger;
> > > vdev->ctx[vector].producer.irq = irq;
> > > ret = irq_bypass_register_producer(>ctx[vector].producer);
> > > - if (unlikely(ret))
> > > + if (unlikely(ret)) {
> > > dev_info(>dev,
> > > "irq bypass producer (token %p) registration fails: %d\n",
> > > vdev->ctx[vector].producer.token, ret);
> > >
> > > - vdev->ctx[vector].trigger = trigger;
> > > + kfree(vdev->ctx[vector].name);
> > > + eventfd_ctx_put(trigger);
> > > +
> > > + cmd = vfio_pci_memory_lock_and_enable(vdev);
> > > + free_irq(irq, trigger);
> > > + vfio_pci_memory_unlock_and_restore(vdev, cmd);
> > > +
> > > + vdev->ctx[vector].trigger = NULL;
> > > + } else
> > > + vdev->ctx[vector].trigger = trigger;
> > >
> > > return 0;
> > > }
> >
> > Once again, the irq bypass registration cannot cause the vector setup
> > to fail, either by returning an error code or failing to configure the
> > vector
Re: [PATCH] irqbypass: fix error handle when irq_bypass_register_producer() return fails
Alex Williamson 于2020年9月30日周三 下午10:09写道：
>
>
> Please version your postings so we know which one to consider as the
> current proposal.
>
> On Wed, 30 Sep 2020 20:54:39 +0800
> guomin_c...@sina.com wrote:
>
> > From: guomin chen
> >
> > When the producer object registration fails,In the future, due to
> > incorrect matching when unregistering, list_del(>node)
> > may still be called, then trigger a BUG:
> >
> > vfio-pci :db:00.0: irq bypass producer (token 60c8cda5)
> > registration fails: -16
> > vfio-pci :db:00.0: irq bypass producer (token 60c8cda5)
> > registration fails: -16
> > vfio-pci :db:00.0: irq bypass producer (token 60c8cda5)
> > registration fails: -16
> > ...
> > list_del corruption, 8f7fb8ba0828->next is LIST_POISON1
> > (dead0100)
> > [ cut here ]
> > kernel BUG at lib/list_debug.c:47!
> > invalid opcode: [#1] SMP NOPTI
> > CPU: 29 PID: 3914 Comm: qemu-kvm Kdump: loaded Tainted: G E
> > - -4.18.0-193.6.3.el8.x86_64 #1
> > Hardware name: Lenovo ThinkSystem SR650 -[7X06CTO1WW]-/-[7X06CTO1WW]-,
> > BIOS -[IVE636Z-2.13]- 07/18/2019
> > RIP: 0010:__list_del_entry_valid.cold.1+0x12/0x4c
> > Code: ce ff 0f 0b 48 89 c1 4c 89 c6 48 c7 c7 40 85 4d 88 e8 8c bc
> > ce ff 0f 0b 48 89 fe 48 89 c2 48 c7 c7 d0 85 4d 88 e8 78 bc
> > ce ff <0f> 0b 48 c7 c7 80 86 4d 88 e8 6a bc ce ff 0f 0b 48
> > 89 f2 48 89 fe
> > RSP: 0018:aa9d60197d20 EFLAGS: 00010246
> > RAX: 004e RBX: 8f7fb8ba0828 RCX:
> > RDX: RSI: 8f7fbf4d6a08 RDI: 8f7fbf4d6a08
> > RBP: R08: 084b R09: 005d
> > R10: R11: aa9d60197bd0 R12: 8f4fbe863000
> > R13: 00c2 R14: R15:
> > FS: 7f7cb97fa700() GS:8f7fbf4c()
> > knlGS:
> > CS: 0010 DS: ES: CR0: 80050033
> > CR2: 7fcf31da4000 CR3: 005f6d404001 CR4: 007626e0
> > DR0: DR1: DR2:
> > DR3: DR6: fffe0ff0 DR7: 0400
> > PKRU: 5554
> > Call Trace:
> > irq_bypass_unregister_producer+0x9b/0xf0 [irqbypass]
> > vfio_msi_set_vector_signal+0x8c/0x290 [vfio_pci]
> > ? load_fixmap_gdt+0x22/0x30
> > vfio_msi_set_block+0x6e/0xd0 [vfio_pci]
> > vfio_pci_ioctl+0x218/0xbe0 [vfio_pci]
> > ? kvm_vcpu_ioctl+0xf2/0x5f0 [kvm]
> > do_vfs_ioctl+0xa4/0x630
> > ? syscall_trace_enter+0x1d3/0x2c0
> > ksys_ioctl+0x60/0x90
> > __x64_sys_ioctl+0x16/0x20
> > do_syscall_64+0x5b/0x1a0
> > entry_SYSCALL_64_after_hwframe+0x65/0xca
> >
> > Cc: Alex Williamson
> > Cc: Cornelia Huck
> > Cc: Jiang Yi
> > Cc: Marc Zyngier
> > Cc: Peter Xu
> > Cc: Eric Auger
> > Cc: "Michael S. Tsirkin"
> > Cc: Jason Wang
> > Cc: k...@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: guomin chen
> > ---
> > drivers/vfio/pci/vfio_pci_intrs.c | 13 +++--
> > drivers/vhost/vdpa.c | 7 +++
> > 2 files changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/vfio/pci/vfio_pci_intrs.c
> > b/drivers/vfio/pci/vfio_pci_intrs.c
> > index 1d9fb25..c371943 100644
> > --- a/drivers/vfio/pci/vfio_pci_intrs.c
> > +++ b/drivers/vfio/pci/vfio_pci_intrs.c
> > @@ -352,12 +352,21 @@ static int vfio_msi_set_vector_signal(struct
> > vfio_pci_device *vdev,
> > vdev->ctx[vector].producer.token = trigger;
> > vdev->ctx[vector].producer.irq = irq;
> > ret = irq_bypass_register_producer(>ctx[vector].producer);
> > - if (unlikely(ret))
> > + if (unlikely(ret)) {
> > dev_info(>dev,
> > "irq bypass producer (token %p) registration fails: %d\n",
> > vdev->ctx[vector].producer.token, ret);
> >
> > - vdev->ctx[vector].trigger = trigger;
> > + kfree(vdev->ctx[vector].name);
> > + eventfd_ctx_put(trigger);
> > +
> > + cmd = vfio_pci_memory_lock_and_enable(vdev);
> > + free_irq(irq, trigger);
> > + vfio_pci_memory_unlock_and_restore(vdev, cmd);
> > +
> > + vdev->ctx[vector].trigger = NULL;
> > + } else
> > + vdev->ctx[vector].trigger = trigger;
> >
> > return 0;
> > }
>
> Once again, the irq bypass registration cannot cause the vector setup
> to fail, either by returning an error code or failing to configure the
> vector while returning success. It's my assertion that we simply need
> to set the producer.token to NULL on failure such that unregistering
> the producer will not generate a match, as you've done below. The
> vector still works even if this registration fails.
>
Yes, the irq bypass
Re: [PATCH] irqbypass: fix error handle when irq_bypass_register_producer() return fails
Please version your postings so we know which one to consider as the
current proposal.
On Wed, 30 Sep 2020 20:54:39 +0800
guomin_c...@sina.com wrote:
> From: guomin chen
>
> When the producer object registration fails,In the future, due to
> incorrect matching when unregistering, list_del(>node)
> may still be called, then trigger a BUG:
>
> vfio-pci :db:00.0: irq bypass producer (token 60c8cda5)
> registration fails: -16
> vfio-pci :db:00.0: irq bypass producer (token 60c8cda5)
> registration fails: -16
> vfio-pci :db:00.0: irq bypass producer (token 60c8cda5)
> registration fails: -16
> ...
> list_del corruption, 8f7fb8ba0828->next is LIST_POISON1
> (dead0100)
> [ cut here ]
> kernel BUG at lib/list_debug.c:47!
> invalid opcode: [#1] SMP NOPTI
> CPU: 29 PID: 3914 Comm: qemu-kvm Kdump: loaded Tainted: G E
> - -4.18.0-193.6.3.el8.x86_64 #1
> Hardware name: Lenovo ThinkSystem SR650 -[7X06CTO1WW]-/-[7X06CTO1WW]-,
> BIOS -[IVE636Z-2.13]- 07/18/2019
> RIP: 0010:__list_del_entry_valid.cold.1+0x12/0x4c
> Code: ce ff 0f 0b 48 89 c1 4c 89 c6 48 c7 c7 40 85 4d 88 e8 8c bc
> ce ff 0f 0b 48 89 fe 48 89 c2 48 c7 c7 d0 85 4d 88 e8 78 bc
> ce ff <0f> 0b 48 c7 c7 80 86 4d 88 e8 6a bc ce ff 0f 0b 48
> 89 f2 48 89 fe
> RSP: 0018:aa9d60197d20 EFLAGS: 00010246
> RAX: 004e RBX: 8f7fb8ba0828 RCX:
> RDX: RSI: 8f7fbf4d6a08 RDI: 8f7fbf4d6a08
> RBP: R08: 084b R09: 005d
> R10: R11: aa9d60197bd0 R12: 8f4fbe863000
> R13: 00c2 R14: R15:
> FS: 7f7cb97fa700() GS:8f7fbf4c()
> knlGS:
> CS: 0010 DS: ES: CR0: 80050033
> CR2: 7fcf31da4000 CR3: 005f6d404001 CR4: 007626e0
> DR0: DR1: DR2:
> DR3: DR6: fffe0ff0 DR7: 0400
> PKRU: 5554
> Call Trace:
> irq_bypass_unregister_producer+0x9b/0xf0 [irqbypass]
> vfio_msi_set_vector_signal+0x8c/0x290 [vfio_pci]
> ? load_fixmap_gdt+0x22/0x30
> vfio_msi_set_block+0x6e/0xd0 [vfio_pci]
> vfio_pci_ioctl+0x218/0xbe0 [vfio_pci]
> ? kvm_vcpu_ioctl+0xf2/0x5f0 [kvm]
> do_vfs_ioctl+0xa4/0x630
> ? syscall_trace_enter+0x1d3/0x2c0
> ksys_ioctl+0x60/0x90
> __x64_sys_ioctl+0x16/0x20
> do_syscall_64+0x5b/0x1a0
> entry_SYSCALL_64_after_hwframe+0x65/0xca
>
> Cc: Alex Williamson
> Cc: Cornelia Huck
> Cc: Jiang Yi
> Cc: Marc Zyngier
> Cc: Peter Xu
> Cc: Eric Auger
> Cc: "Michael S. Tsirkin"
> Cc: Jason Wang
> Cc: k...@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: guomin chen
> ---
> drivers/vfio/pci/vfio_pci_intrs.c | 13 +++--
> drivers/vhost/vdpa.c | 7 +++
> 2 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vfio/pci/vfio_pci_intrs.c
> b/drivers/vfio/pci/vfio_pci_intrs.c
> index 1d9fb25..c371943 100644
> --- a/drivers/vfio/pci/vfio_pci_intrs.c
> +++ b/drivers/vfio/pci/vfio_pci_intrs.c
> @@ -352,12 +352,21 @@ static int vfio_msi_set_vector_signal(struct
> vfio_pci_device *vdev,
> vdev->ctx[vector].producer.token = trigger;
> vdev->ctx[vector].producer.irq = irq;
> ret = irq_bypass_register_producer(>ctx[vector].producer);
> - if (unlikely(ret))
> + if (unlikely(ret)) {
> dev_info(>dev,
> "irq bypass producer (token %p) registration fails: %d\n",
> vdev->ctx[vector].producer.token, ret);
>
> - vdev->ctx[vector].trigger = trigger;
> + kfree(vdev->ctx[vector].name);
> + eventfd_ctx_put(trigger);
> +
> + cmd = vfio_pci_memory_lock_and_enable(vdev);
> + free_irq(irq, trigger);
> + vfio_pci_memory_unlock_and_restore(vdev, cmd);
> +
> + vdev->ctx[vector].trigger = NULL;
> + } else
> + vdev->ctx[vector].trigger = trigger;
>
> return 0;
> }
Once again, the irq bypass registration cannot cause the vector setup
to fail, either by returning an error code or failing to configure the
vector while returning success. It's my assertion that we simply need
to set the producer.token to NULL on failure such that unregistering
the producer will not generate a match, as you've done below. The
vector still works even if this registration fails.
> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> index 796fe97..4e082b8 100644
> --- a/drivers/vhost/vdpa.c
> +++ b/drivers/vhost/vdpa.c
> @@ -107,6 +107,13 @@ static void vhost_vdpa_setup_vq_irq(struct vhost_vdpa
> *v, u16 qid)
> vq->call_ctx.producer.token =
