public inbox for linux-kernel@vger.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: 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: 6+ 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-12-10  3:53 ` Kirill A. Shutemov [this message]
2015-12-10 10:28 ` David Herrmann
2015-12-14  8:19   ` Kirill A. Shutemov
2015-12-14 10:20     ` David Herrmann
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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox