All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Kirill A. Shutemov" <kirill@shutemov.name>
To: Alex Williamson <alex.williamson@redhat.com>,
	David Airlie <airlied@linux.ie>
Cc: vfio-users@redhat.com, dri-devel@lists.freedesktop.org,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] vgaarb: fix signal handling in vga_get()
Date: Thu, 10 Dec 2015 05:53:03 +0200	[thread overview]
Message-ID: <20151210035303.GA27821@node.shutemov.name> (raw)
In-Reply-To: <1448849851-26061-1-git-send-email-kirill@shutemov.name>

On Mon, Nov 30, 2015 at 04:17:31AM +0200, Kirill A. Shutemov wrote:
> There are few defects in vga_get() related to signal hadning:
> 
>   - we shouldn't check for pending signals for TASK_UNINTERRUPTIBLE
>     case;
> 
>   - if we found pending signal we must remove ourself from wait queue
>     and change task state back to running;
> 
>   - -ERESTARTSYS is more appropriate, I guess.
> 
> Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>

Ping?

David, this patch fixes crash. It would be nice to get it into 4.4.

> ---
> 
> Alex, I try to get KVM with VGA passthrough working properly. I have i915
> (HD 4600) on the host and GTX 580 for the guest. The guest GPU is not
> capabale of EFI, so I have to use x-vga=on. It's kinda work with your
> patch for i915.enable_hd_vgaarb=1. But guest refuse to initialize the GPU
> after KVM was not shut down correctly, resulting in host crash like this:
> 
> BUG: unable to handle kernel paging request at ffff880870187ed8
> IP: [<ffff880870187ed8>] 0xffff880870187ed8
> PGD 2129067 PUD 80000008400001e3
> Oops: 0011 [#1] PREEMPT SMP
> Modules linked in: iwlmvm iwlwifi
> CPU: 6 PID: 3983 Comm: qemu-system-x86 Not tainted 4.3.0-gentoo #6
> Hardware name: Gigabyte Technology Co., Ltd. Z87X-UD7 TH/Z87X-UD7 TH-CF, BIOS F5a 06/12/2014
> task: ffff88087a910000 ti: ffff8808632c0000 task.ti: ffff8808632c0000
> RIP: 0010:[<ffff880870187ed8>]  [<ffff880870187ed8>] 0xffff880870187ed8
> RSP: 0018:ffff8808632c3d08  EFLAGS: 00010006
> RAX: ffff880870187db0 RBX: 0000000070187f58 RCX: 0000000000000000
> RDX: 0000000000000000 RSI: 0000000000000003 RDI: ffff880870187db0
> RBP: ffff8808632c3d48 R08: 0000000000000000 R09: 0000000000000000
> R10: 00000000000103c0 R11: 0000000000000293 R12: ffffffff81ea03c8
> R13: ffffffff8104c7cb R14: 0000000000000000 R15: 0000000000000003
> FS:  00007f984f9b2700(0000) GS:ffff88089f380000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: ffff880870187ed8 CR3: 00000008645f8000 CR4: 00000000001426e0
> Stack:
>  ffffffff810cc83d 00000000632c3d28 0000000000000000 ffffffff81ea03c0
>  0000000000000046 0000000000000003 0000000000000000 0000000000000000
>  ffff8808632c3d80 ffffffff810cca44 ffff88087af63800 0000000000000286
> Call Trace:
>  [<ffffffff810cc83d>] ? __wake_up_common+0x4d/0x80
>  [<ffffffff810cca44>] __wake_up+0x34/0x50
>  [<ffffffff815d99e3>] __vga_put+0x73/0xd0
>  [<ffffffff815d9db4>] vga_put+0x54/0x80
>  [<ffffffff8169d042>] vfio_pci_vga_rw+0x1d2/0x220
>  [<ffffffff8169a7f3>] vfio_pci_rw+0x33/0x60
>  [<ffffffff8169abf7>] vfio_pci_write+0x17/0x20
>  [<ffffffff816966a6>] vfio_device_fops_write+0x26/0x30
>  [<ffffffff811a4b23>] __vfs_write+0x23/0xe0
>  [<ffffffff811a4a53>] ? __vfs_read+0x23/0xd0
>  [<ffffffff811b6e35>] ? do_vfs_ioctl+0x2b5/0x490
>  [<ffffffff811a5194>] vfs_write+0xa4/0x190
>  [<ffffffff811a5fa6>] SyS_pwrite64+0x66/0xa0
>  [<ffffffff819a17d7>] entry_SYSCALL_64_fastpath+0x12/0x6a
> Code: 88 ff ff e0 7e 18 70 08 88 ff ff 00 8c 57 76 08 88 ff ff 20 7f 18 70 08 88 ff ff 08 7f 18 70 08 88 ff ff 94 51 1a 81 ff ff ff ff <09> 00 00 00 00 00 00 00 01 8c 57 76 08 88 ff ff 00 8c 57 76 08
> RIP  [<ffff880870187ed8>] 0xffff880870187ed8
>  RSP <ffff8808632c3d08>
> CR2: ffff880870187ed8
> 
> The patch fixes the crash, but doesn't help with getting GPU in guest
> working again.
> 
> Any ideas?
> 
> ---
>  drivers/gpu/vga/vgaarb.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
> index 3166e4bc4eb6..9abcaa53bd25 100644
> --- a/drivers/gpu/vga/vgaarb.c
> +++ b/drivers/gpu/vga/vgaarb.c
> @@ -395,8 +395,10 @@ int vga_get(struct pci_dev *pdev, unsigned int rsrc, int interruptible)
>  		set_current_state(interruptible ?
>  				  TASK_INTERRUPTIBLE :
>  				  TASK_UNINTERRUPTIBLE);
> -		if (signal_pending(current)) {
> -			rc = -EINTR;
> +		if (interruptible && signal_pending(current)) {
> +			__set_current_state(TASK_RUNNING);
> +			remove_wait_queue(&vga_wait_queue, &wait);
> +			rc = -ERESTARTSYS;
>  			break;
>  		}
>  		schedule();
> -- 
> 2.6.3
> 

-- 
 Kirill A. Shutemov
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

WARNING: multiple messages have this Message-ID (diff)
From: "Kirill A. Shutemov" <kirill@shutemov.name>
To: Alex Williamson <alex.williamson@redhat.com>,
	David Airlie <airlied@linux.ie>
Cc: kvm@vger.kernel.org, dri-devel@lists.freedesktop.org,
	linux-kernel@vger.kernel.org, vfio-users@redhat.com
Subject: Re: [PATCH] vgaarb: fix signal handling in vga_get()
Date: Thu, 10 Dec 2015 05:53:03 +0200	[thread overview]
Message-ID: <20151210035303.GA27821@node.shutemov.name> (raw)
In-Reply-To: <1448849851-26061-1-git-send-email-kirill@shutemov.name>

On Mon, Nov 30, 2015 at 04:17:31AM +0200, Kirill A. Shutemov wrote:
> There are few defects in vga_get() related to signal hadning:
> 
>   - we shouldn't check for pending signals for TASK_UNINTERRUPTIBLE
>     case;
> 
>   - if we found pending signal we must remove ourself from wait queue
>     and change task state back to running;
> 
>   - -ERESTARTSYS is more appropriate, I guess.
> 
> Signed-off-by: Kirill A. Shutemov <kirill@shutemov.name>

Ping?

David, this patch fixes crash. It would be nice to get it into 4.4.

> ---
> 
> Alex, I try to get KVM with VGA passthrough working properly. I have i915
> (HD 4600) on the host and GTX 580 for the guest. The guest GPU is not
> capabale of EFI, so I have to use x-vga=on. It's kinda work with your
> patch for i915.enable_hd_vgaarb=1. But guest refuse to initialize the GPU
> after KVM was not shut down correctly, resulting in host crash like this:
> 
> BUG: unable to handle kernel paging request at ffff880870187ed8
> IP: [<ffff880870187ed8>] 0xffff880870187ed8
> PGD 2129067 PUD 80000008400001e3
> Oops: 0011 [#1] PREEMPT SMP
> Modules linked in: iwlmvm iwlwifi
> CPU: 6 PID: 3983 Comm: qemu-system-x86 Not tainted 4.3.0-gentoo #6
> Hardware name: Gigabyte Technology Co., Ltd. Z87X-UD7 TH/Z87X-UD7 TH-CF, BIOS F5a 06/12/2014
> task: ffff88087a910000 ti: ffff8808632c0000 task.ti: ffff8808632c0000
> RIP: 0010:[<ffff880870187ed8>]  [<ffff880870187ed8>] 0xffff880870187ed8
> RSP: 0018:ffff8808632c3d08  EFLAGS: 00010006
> RAX: ffff880870187db0 RBX: 0000000070187f58 RCX: 0000000000000000
> RDX: 0000000000000000 RSI: 0000000000000003 RDI: ffff880870187db0
> RBP: ffff8808632c3d48 R08: 0000000000000000 R09: 0000000000000000
> R10: 00000000000103c0 R11: 0000000000000293 R12: ffffffff81ea03c8
> R13: ffffffff8104c7cb R14: 0000000000000000 R15: 0000000000000003
> FS:  00007f984f9b2700(0000) GS:ffff88089f380000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: ffff880870187ed8 CR3: 00000008645f8000 CR4: 00000000001426e0
> Stack:
>  ffffffff810cc83d 00000000632c3d28 0000000000000000 ffffffff81ea03c0
>  0000000000000046 0000000000000003 0000000000000000 0000000000000000
>  ffff8808632c3d80 ffffffff810cca44 ffff88087af63800 0000000000000286
> Call Trace:
>  [<ffffffff810cc83d>] ? __wake_up_common+0x4d/0x80
>  [<ffffffff810cca44>] __wake_up+0x34/0x50
>  [<ffffffff815d99e3>] __vga_put+0x73/0xd0
>  [<ffffffff815d9db4>] vga_put+0x54/0x80
>  [<ffffffff8169d042>] vfio_pci_vga_rw+0x1d2/0x220
>  [<ffffffff8169a7f3>] vfio_pci_rw+0x33/0x60
>  [<ffffffff8169abf7>] vfio_pci_write+0x17/0x20
>  [<ffffffff816966a6>] vfio_device_fops_write+0x26/0x30
>  [<ffffffff811a4b23>] __vfs_write+0x23/0xe0
>  [<ffffffff811a4a53>] ? __vfs_read+0x23/0xd0
>  [<ffffffff811b6e35>] ? do_vfs_ioctl+0x2b5/0x490
>  [<ffffffff811a5194>] vfs_write+0xa4/0x190
>  [<ffffffff811a5fa6>] SyS_pwrite64+0x66/0xa0
>  [<ffffffff819a17d7>] entry_SYSCALL_64_fastpath+0x12/0x6a
> Code: 88 ff ff e0 7e 18 70 08 88 ff ff 00 8c 57 76 08 88 ff ff 20 7f 18 70 08 88 ff ff 08 7f 18 70 08 88 ff ff 94 51 1a 81 ff ff ff ff <09> 00 00 00 00 00 00 00 01 8c 57 76 08 88 ff ff 00 8c 57 76 08
> RIP  [<ffff880870187ed8>] 0xffff880870187ed8
>  RSP <ffff8808632c3d08>
> CR2: ffff880870187ed8
> 
> The patch fixes the crash, but doesn't help with getting GPU in guest
> working again.
> 
> Any ideas?
> 
> ---
>  drivers/gpu/vga/vgaarb.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/vga/vgaarb.c b/drivers/gpu/vga/vgaarb.c
> index 3166e4bc4eb6..9abcaa53bd25 100644
> --- a/drivers/gpu/vga/vgaarb.c
> +++ b/drivers/gpu/vga/vgaarb.c
> @@ -395,8 +395,10 @@ int vga_get(struct pci_dev *pdev, unsigned int rsrc, int interruptible)
>  		set_current_state(interruptible ?
>  				  TASK_INTERRUPTIBLE :
>  				  TASK_UNINTERRUPTIBLE);
> -		if (signal_pending(current)) {
> -			rc = -EINTR;
> +		if (interruptible && signal_pending(current)) {
> +			__set_current_state(TASK_RUNNING);
> +			remove_wait_queue(&vga_wait_queue, &wait);
> +			rc = -ERESTARTSYS;
>  			break;
>  		}
>  		schedule();
> -- 
> 2.6.3
> 

-- 
 Kirill A. Shutemov

  reply	other threads:[~2015-12-10  3:53 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-30  2:17 [PATCH] vgaarb: fix signal handling in vga_get() Kirill A. Shutemov
2015-11-30  2:17 ` Kirill A. Shutemov
2015-12-10  3:53 ` Kirill A. Shutemov [this message]
2015-12-10  3:53   ` Kirill A. Shutemov
2015-12-10 10:28 ` David Herrmann
2015-12-10 10:28   ` David Herrmann
2015-12-14  8:19   ` Kirill A. Shutemov
2015-12-14  8:19     ` Kirill A. Shutemov
2015-12-14 10:20     ` David Herrmann
2015-12-14 10:20       ` David Herrmann
2015-12-14 12:13       ` Kirill A. Shutemov
2015-12-14 12:13         ` Kirill A. Shutemov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151210035303.GA27821@node.shutemov.name \
    --to=kirill@shutemov.name \
    --cc=airlied@linux.ie \
    --cc=alex.williamson@redhat.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=vfio-users@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.