On Fri, 2024-08-02 at 11:29 +0800, Jason Wang wrote: > On Thu, Aug 1, 2024 at 11:38 PM Dragos Tatulea <dtatulea@xxxxxxxxxx> wrote: > > > > The following workflow triggers the crash referenced below: > > > > 1) vhost_vdpa_unsetup_vq_irq() unregisters the irq bypass producer > > but the producer->token is still valid. > > 2) vq context gets released and reassigned to another vq. > > Just to make sure I understand here, which structure is referred to as > "vq context" here? I guess it's not call_ctx as it is a part of the vq > itself. > > > 3) That other vq registers it's producer with the same vq context > > pointer as token in vhost_vdpa_setup_vq_irq(). > > Or did you mean when a single eventfd is shared among different vqs? > Yes, that's what I mean: vq->call_ctx.ctx which is a eventfd_ctx. But I don't think it's shared in this case, only that the old eventfd_ctx value is lingering in producer->token. And this old eventfd_ctx is assigned now to another vq. > > 4) The original vq tries to unregister it's producer which it has > > already unlinked in step 1. irq_bypass_unregister_producer() will go > > ahead and unlink the producer once again. That happens because: > > a) The producer has a token. > > b) An element with that token is found. But that element comes > > from step 3. > > > > I see 3 ways to fix this: > > 1) Fix the vhost-vdpa part. What this patch does. vfio has a different > > workflow. > > 2) Set the token to NULL directly in irq_bypass_unregister_producer() > > after unlinking the producer. But that makes the API asymmetrical. > > 3) Make irq_bypass_unregister_producer() also compare the pointer > > elements not just the tokens and do the unlink only on match. > > > > Any thoughts? > > > > Oops: general protection fault, probably for non-canonical address 0xdead000000000108: 0000 [#1] SMP > > CPU: 8 PID: 5190 Comm: qemu-system-x86 Not tainted 6.10.0-rc7+ #6 > > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014 > > RIP: 0010:irq_bypass_unregister_producer+0xa5/0xd0 > > RSP: 0018:ffffc900034d7e50 EFLAGS: 00010246 > > RAX: dead000000000122 RBX: ffff888353d12718 RCX: ffff88810336a000 > > RDX: dead000000000100 RSI: ffffffff829243a0 RDI: 0000000000000000 > > RBP: ffff888353c42000 R08: ffff888104882738 R09: ffff88810336a000 > > R10: ffff888448ab2050 R11: 0000000000000000 R12: ffff888353d126a0 > > R13: 0000000000000004 R14: 0000000000000055 R15: 0000000000000004 > > FS: 00007f9df9403c80(0000) GS:ffff88852cc00000(0000) knlGS:0000000000000000 > > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > > CR2: 0000562dffc6b568 CR3: 000000012efbb006 CR4: 0000000000772ef0 > > PKRU: 55555554 > > Call Trace: > > <TASK> > > ? die_addr+0x36/0x90 > > ? exc_general_protection+0x1a8/0x390 > > ? asm_exc_general_protection+0x26/0x30 > > ? irq_bypass_unregister_producer+0xa5/0xd0 > > vhost_vdpa_setup_vq_irq+0x5a/0xc0 [vhost_vdpa] > > vhost_vdpa_unlocked_ioctl+0xdcd/0xe00 [vhost_vdpa] > > ? vhost_vdpa_config_cb+0x30/0x30 [vhost_vdpa] > > __x64_sys_ioctl+0x90/0xc0 > > do_syscall_64+0x4f/0x110 > > entry_SYSCALL_64_after_hwframe+0x4b/0x53 > > RIP: 0033:0x7f9df930774f > > RSP: 002b:00007ffc55013080 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 > > RAX: ffffffffffffffda RBX: 0000562dfe134d20 RCX: 00007f9df930774f > > RDX: 00007ffc55013200 RSI: 000000004008af21 RDI: 0000000000000011 > > RBP: 00007ffc55013200 R08: 0000000000000002 R09: 0000000000000000 > > R10: 0000000000000000 R11: 0000000000000246 R12: 0000562dfe134360 > > R13: 0000562dfe134d20 R14: 0000000000000000 R15: 00007f9df801e190 > > > > Signed-off-by: Dragos Tatulea <dtatulea@xxxxxxxxxx> > > --- > > drivers/vhost/vdpa.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c > > index 478cd46a49ed..d4a7a3918d86 100644 > > --- a/drivers/vhost/vdpa.c > > +++ b/drivers/vhost/vdpa.c > > @@ -226,6 +226,7 @@ static void vhost_vdpa_unsetup_vq_irq(struct vhost_vdpa *v, u16 qid) > > struct vhost_virtqueue *vq = &v->vqs[qid]; > > > > irq_bypass_unregister_producer(&vq->call_ctx.producer); > > + vq->call_ctx.producer.token = NULL; > > } > > > > static int _compat_vdpa_reset(struct vhost_vdpa *v) > > -- > > 2.45.2 > > > Thanks
