From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Kirill A. Shutemov" Subject: Re: [PATCH] vgaarb: fix signal handling in vga_get() Date: Mon, 14 Dec 2015 10:19:56 +0200 Message-ID: <20151214081956.GA29540@node.shutemov.name> References: <1448849851-26061-1-git-send-email-kirill@shutemov.name> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail-wm0-f65.google.com (mail-wm0-f65.google.com [74.125.82.65]) by gabe.freedesktop.org (Postfix) with ESMTPS id 9C40F6E059 for ; Mon, 14 Dec 2015 00:20:00 -0800 (PST) Received: by wmll124 with SMTP id l124so7343411wml.0 for ; Mon, 14 Dec 2015 00:19:58 -0800 (PST) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: David Herrmann Cc: kvm@vger.kernel.org, Daniel Vetter , linux-kernel , "dri-devel@lists.freedesktop.org" , Alex Williamson , vfio-users@redhat.com List-Id: dri-devel@lists.freedesktop.org T24gVGh1LCBEZWMgMTAsIDIwMTUgYXQgMTE6Mjg6NThBTSArMDEwMCwgRGF2aWQgSGVycm1hbm4g d3JvdGU6Cj4gSGkKPiAKPiBPbiBNb24sIE5vdiAzMCwgMjAxNSBhdCAzOjE3IEFNLCBLaXJpbGwg QS4gU2h1dGVtb3YKPiA8a2lyaWxsQHNodXRlbW92Lm5hbWU+IHdyb3RlOgo+ID4gVGhlcmUgYXJl IGZldyBkZWZlY3RzIGluIHZnYV9nZXQoKSByZWxhdGVkIHRvIHNpZ25hbCBoYWRuaW5nOgo+ID4K PiA+ICAgLSB3ZSBzaG91bGRuJ3QgY2hlY2sgZm9yIHBlbmRpbmcgc2lnbmFscyBmb3IgVEFTS19V TklOVEVSUlVQVElCTEUKPiA+ICAgICBjYXNlOwo+ID4KPiA+ICAgLSBpZiB3ZSBmb3VuZCBwZW5k aW5nIHNpZ25hbCB3ZSBtdXN0IHJlbW92ZSBvdXJzZWxmIGZyb20gd2FpdCBxdWV1ZQo+ID4gICAg IGFuZCBjaGFuZ2UgdGFzayBzdGF0ZSBiYWNrIHRvIHJ1bm5pbmc7Cj4gPgo+ID4gICAtIC1FUkVT VEFSVFNZUyBpcyBtb3JlIGFwcHJvcHJpYXRlLCBJIGd1ZXNzLgo+ID4KPiA+IFNpZ25lZC1vZmYt Ynk6IEtpcmlsbCBBLiBTaHV0ZW1vdiA8a2lyaWxsQHNodXRlbW92Lm5hbWU+Cj4gPiAtLS0KPiA+ Cj4gPiBBbGV4LCBJIHRyeSB0byBnZXQgS1ZNIHdpdGggVkdBIHBhc3N0aHJvdWdoIHdvcmtpbmcg cHJvcGVybHkuIEkgaGF2ZSBpOTE1Cj4gPiAoSEQgNDYwMCkgb24gdGhlIGhvc3QgYW5kIEdUWCA1 ODAgZm9yIHRoZSBndWVzdC4gVGhlIGd1ZXN0IEdQVSBpcyBub3QKPiA+IGNhcGFiYWxlIG9mIEVG SSwgc28gSSBoYXZlIHRvIHVzZSB4LXZnYT1vbi4gSXQncyBraW5kYSB3b3JrIHdpdGggeW91cgo+ ID4gcGF0Y2ggZm9yIGk5MTUuZW5hYmxlX2hkX3ZnYWFyYj0xLiBCdXQgZ3Vlc3QgcmVmdXNlIHRv IGluaXRpYWxpemUgdGhlIEdQVQo+ID4gYWZ0ZXIgS1ZNIHdhcyBub3Qgc2h1dCBkb3duIGNvcnJl Y3RseSwgcmVzdWx0aW5nIGluIGhvc3QgY3Jhc2ggbGlrZSB0aGlzOgo+ID4KPiA+IEJVRzogdW5h YmxlIHRvIGhhbmRsZSBrZXJuZWwgcGFnaW5nIHJlcXVlc3QgYXQgZmZmZjg4MDg3MDE4N2VkOAo+ ID4gSVA6IFs8ZmZmZjg4MDg3MDE4N2VkOD5dIDB4ZmZmZjg4MDg3MDE4N2VkOAo+ID4gUEdEIDIx MjkwNjcgUFVEIDgwMDAwMDA4NDAwMDAxZTMKPiA+IE9vcHM6IDAwMTEgWyMxXSBQUkVFTVBUIFNN UAo+ID4gTW9kdWxlcyBsaW5rZWQgaW46IGl3bG12bSBpd2x3aWZpCj4gPiBDUFU6IDYgUElEOiAz OTgzIENvbW06IHFlbXUtc3lzdGVtLXg4NiBOb3QgdGFpbnRlZCA0LjMuMC1nZW50b28gIzYKPiA+ IEhhcmR3YXJlIG5hbWU6IEdpZ2FieXRlIFRlY2hub2xvZ3kgQ28uLCBMdGQuIFo4N1gtVUQ3IFRI L1o4N1gtVUQ3IFRILUNGLCBCSU9TIEY1YSAwNi8xMi8yMDE0Cj4gPiB0YXNrOiBmZmZmODgwODdh OTEwMDAwIHRpOiBmZmZmODgwODYzMmMwMDAwIHRhc2sudGk6IGZmZmY4ODA4NjMyYzAwMDAKPiA+ IFJJUDogMDAxMDpbPGZmZmY4ODA4NzAxODdlZDg+XSAgWzxmZmZmODgwODcwMTg3ZWQ4Pl0gMHhm ZmZmODgwODcwMTg3ZWQ4Cj4gPiBSU1A6IDAwMTg6ZmZmZjg4MDg2MzJjM2QwOCAgRUZMQUdTOiAw MDAxMDAwNgo+ID4gUkFYOiBmZmZmODgwODcwMTg3ZGIwIFJCWDogMDAwMDAwMDA3MDE4N2Y1OCBS Q1g6IDAwMDAwMDAwMDAwMDAwMDAKPiA+IFJEWDogMDAwMDAwMDAwMDAwMDAwMCBSU0k6IDAwMDAw MDAwMDAwMDAwMDMgUkRJOiBmZmZmODgwODcwMTg3ZGIwCj4gPiBSQlA6IGZmZmY4ODA4NjMyYzNk NDggUjA4OiAwMDAwMDAwMDAwMDAwMDAwIFIwOTogMDAwMDAwMDAwMDAwMDAwMAo+ID4gUjEwOiAw MDAwMDAwMDAwMDEwM2MwIFIxMTogMDAwMDAwMDAwMDAwMDI5MyBSMTI6IGZmZmZmZmZmODFlYTAz YzgKPiA+IFIxMzogZmZmZmZmZmY4MTA0YzdjYiBSMTQ6IDAwMDAwMDAwMDAwMDAwMDAgUjE1OiAw MDAwMDAwMDAwMDAwMDAzCj4gPiBGUzogIDAwMDA3Zjk4NGY5YjI3MDAoMDAwMCkgR1M6ZmZmZjg4 MDg5ZjM4MDAwMCgwMDAwKSBrbmxHUzowMDAwMDAwMDAwMDAwMDAwCj4gPiBDUzogIDAwMTAgRFM6 IDAwMDAgRVM6IDAwMDAgQ1IwOiAwMDAwMDAwMDgwMDUwMDMzCj4gPiBDUjI6IGZmZmY4ODA4NzAx ODdlZDggQ1IzOiAwMDAwMDAwODY0NWY4MDAwIENSNDogMDAwMDAwMDAwMDE0MjZlMAo+ID4gU3Rh Y2s6Cj4gPiAgZmZmZmZmZmY4MTBjYzgzZCAwMDAwMDAwMDYzMmMzZDI4IDAwMDAwMDAwMDAwMDAw MDAgZmZmZmZmZmY4MWVhMDNjMAo+ID4gIDAwMDAwMDAwMDAwMDAwNDYgMDAwMDAwMDAwMDAwMDAw MyAwMDAwMDAwMDAwMDAwMDAwIDAwMDAwMDAwMDAwMDAwMDAKPiA+ICBmZmZmODgwODYzMmMzZDgw IGZmZmZmZmZmODEwY2NhNDQgZmZmZjg4MDg3YWY2MzgwMCAwMDAwMDAwMDAwMDAwMjg2Cj4gPiBD YWxsIFRyYWNlOgo+ID4gIFs8ZmZmZmZmZmY4MTBjYzgzZD5dID8gX193YWtlX3VwX2NvbW1vbisw eDRkLzB4ODAKPiA+ICBbPGZmZmZmZmZmODEwY2NhNDQ+XSBfX3dha2VfdXArMHgzNC8weDUwCj4g PiAgWzxmZmZmZmZmZjgxNWQ5OWUzPl0gX192Z2FfcHV0KzB4NzMvMHhkMAo+ID4gIFs8ZmZmZmZm ZmY4MTVkOWRiND5dIHZnYV9wdXQrMHg1NC8weDgwCj4gPiAgWzxmZmZmZmZmZjgxNjlkMDQyPl0g dmZpb19wY2lfdmdhX3J3KzB4MWQyLzB4MjIwCj4gPiAgWzxmZmZmZmZmZjgxNjlhN2YzPl0gdmZp b19wY2lfcncrMHgzMy8weDYwCj4gPiAgWzxmZmZmZmZmZjgxNjlhYmY3Pl0gdmZpb19wY2lfd3Jp dGUrMHgxNy8weDIwCj4gPiAgWzxmZmZmZmZmZjgxNjk2NmE2Pl0gdmZpb19kZXZpY2VfZm9wc193 cml0ZSsweDI2LzB4MzAKPiA+ICBbPGZmZmZmZmZmODExYTRiMjM+XSBfX3Zmc193cml0ZSsweDIz LzB4ZTAKPiA+ICBbPGZmZmZmZmZmODExYTRhNTM+XSA/IF9fdmZzX3JlYWQrMHgyMy8weGQwCj4g PiAgWzxmZmZmZmZmZjgxMWI2ZTM1Pl0gPyBkb192ZnNfaW9jdGwrMHgyYjUvMHg0OTAKPiA+ICBb PGZmZmZmZmZmODExYTUxOTQ+XSB2ZnNfd3JpdGUrMHhhNC8weDE5MAo+ID4gIFs8ZmZmZmZmZmY4 MTFhNWZhNj5dIFN5U19wd3JpdGU2NCsweDY2LzB4YTAKPiA+ICBbPGZmZmZmZmZmODE5YTE3ZDc+ XSBlbnRyeV9TWVNDQUxMXzY0X2Zhc3RwYXRoKzB4MTIvMHg2YQo+ID4gQ29kZTogODggZmYgZmYg ZTAgN2UgMTggNzAgMDggODggZmYgZmYgMDAgOGMgNTcgNzYgMDggODggZmYgZmYgMjAgN2YgMTgg NzAgMDggODggZmYgZmYgMDggN2YgMTggNzAgMDggODggZmYgZmYgOTQgNTEgMWEgODEgZmYgZmYg ZmYgZmYgPDA5PiAwMCAwMCAwMCAwMCAwMCAwMCAwMCAwMSA4YyA1NyA3NiAwOCA4OCBmZiBmZiAw MCA4YyA1NyA3NiAwOAo+ID4gUklQICBbPGZmZmY4ODA4NzAxODdlZDg+XSAweGZmZmY4ODA4NzAx ODdlZDgKPiA+ICBSU1AgPGZmZmY4ODA4NjMyYzNkMDg+Cj4gPiBDUjI6IGZmZmY4ODA4NzAxODdl ZDgKPiA+Cj4gPiBUaGUgcGF0Y2ggZml4ZXMgdGhlIGNyYXNoLCBidXQgZG9lc24ndCBoZWxwIHdp dGggZ2V0dGluZyBHUFUgaW4gZ3Vlc3QKPiA+IHdvcmtpbmcgYWdhaW4uCj4gPgo+ID4gQW55IGlk ZWFzPwo+ID4KPiA+IC0tLQo+ID4gIGRyaXZlcnMvZ3B1L3ZnYS92Z2FhcmIuYyB8IDYgKysrKy0t Cj4gPiAgMSBmaWxlIGNoYW5nZWQsIDQgaW5zZXJ0aW9ucygrKSwgMiBkZWxldGlvbnMoLSkKPiA+ Cj4gPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvdmdhL3ZnYWFyYi5jIGIvZHJpdmVycy9ncHUv dmdhL3ZnYWFyYi5jCj4gPiBpbmRleCAzMTY2ZTRiYzRlYjYuLjlhYmNhYTUzYmQyNSAxMDA2NDQK PiA+IC0tLSBhL2RyaXZlcnMvZ3B1L3ZnYS92Z2FhcmIuYwo+ID4gKysrIGIvZHJpdmVycy9ncHUv dmdhL3ZnYWFyYi5jCj4gPiBAQCAtMzk1LDggKzM5NSwxMCBAQCBpbnQgdmdhX2dldChzdHJ1Y3Qg cGNpX2RldiAqcGRldiwgdW5zaWduZWQgaW50IHJzcmMsIGludCBpbnRlcnJ1cHRpYmxlKQo+ID4g ICAgICAgICAgICAgICAgIHNldF9jdXJyZW50X3N0YXRlKGludGVycnVwdGlibGUgPwo+ID4gICAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIFRBU0tfSU5URVJSVVBUSUJMRSA6Cj4gPiAg ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgVEFTS19VTklOVEVSUlVQVElCTEUpOwo+ ID4gLSAgICAgICAgICAgICAgIGlmIChzaWduYWxfcGVuZGluZyhjdXJyZW50KSkgewo+ID4gLSAg ICAgICAgICAgICAgICAgICAgICAgcmMgPSAtRUlOVFI7Cj4gPiArICAgICAgICAgICAgICAgaWYg KGludGVycnVwdGlibGUgJiYgc2lnbmFsX3BlbmRpbmcoY3VycmVudCkpIHsKPiA+ICsgICAgICAg ICAgICAgICAgICAgICAgIF9fc2V0X2N1cnJlbnRfc3RhdGUoVEFTS19SVU5OSU5HKTsKPiA+ICsg ICAgICAgICAgICAgICAgICAgICAgIHJlbW92ZV93YWl0X3F1ZXVlKCZ2Z2Ffd2FpdF9xdWV1ZSwg JndhaXQpOwo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgcmMgPSAtRVJFU1RBUlRTWVM7Cj4g PiAgICAgICAgICAgICAgICAgICAgICAgICBicmVhazsKPiAKPiBBbGwgMyBwb2ludHMgYXJlIHZh bGlkLCBhbmQgdGhlIHBhdGNoIGxvb2tzIGdvb2QgdG8gbWU6Cj4gCj4gUmV2aWV3ZWQtYnk6IERh dmlkIEhlcnJtYW5uIDxkaC5oZXJybWFubkBnbWFpbC5jb20+Cj4gCj4gSG93ZXZlciwgdGhlcmUg c2VlbXMgdG8gYmUgYSByYWNlIGJldHdlZW4gdmdhX2xvY2sgYW5kIHB1dHRpbmcgdGhlCj4gdGhy ZWFkIGFzbGVlcC4KCkknbSBub3Qgc3VyZSBJIHVuZGVyc3RhbmQgdGhlIHJhY2UgeW91J3JlIHRh bGtpbmcgYWJvdXQuCkNvdWxkIHlvdSBlbGFib3JhdGU/CgotLSAKIEtpcmlsbCBBLiBTaHV0ZW1v dgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2 ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cDovL2xp c3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753089AbbLNIUE (ORCPT ); Mon, 14 Dec 2015 03:20:04 -0500 Received: from mail-wm0-f66.google.com ([74.125.82.66]:35264 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752670AbbLNIUA (ORCPT ); Mon, 14 Dec 2015 03:20:00 -0500 Date: Mon, 14 Dec 2015 10:19:56 +0200 From: "Kirill A. Shutemov" To: David Herrmann Cc: Dave Airlie , Daniel Vetter , Alex Williamson , David Airlie , kvm@vger.kernel.org, "dri-devel@lists.freedesktop.org" , linux-kernel , vfio-users@redhat.com Subject: Re: [PATCH] vgaarb: fix signal handling in vga_get() Message-ID: <20151214081956.GA29540@node.shutemov.name> References: <1448849851-26061-1-git-send-email-kirill@shutemov.name> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23.1 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Dec 10, 2015 at 11:28:58AM +0100, David Herrmann wrote: > Hi > > On Mon, Nov 30, 2015 at 3:17 AM, 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 > > --- > > > > 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: [] 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:[] [] 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: > > [] ? __wake_up_common+0x4d/0x80 > > [] __wake_up+0x34/0x50 > > [] __vga_put+0x73/0xd0 > > [] vga_put+0x54/0x80 > > [] vfio_pci_vga_rw+0x1d2/0x220 > > [] vfio_pci_rw+0x33/0x60 > > [] vfio_pci_write+0x17/0x20 > > [] vfio_device_fops_write+0x26/0x30 > > [] __vfs_write+0x23/0xe0 > > [] ? __vfs_read+0x23/0xd0 > > [] ? do_vfs_ioctl+0x2b5/0x490 > > [] vfs_write+0xa4/0x190 > > [] SyS_pwrite64+0x66/0xa0 > > [] 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 [] 0xffff880870187ed8 > > RSP > > 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; > > All 3 points are valid, and the patch looks good to me: > > Reviewed-by: David Herrmann > > However, there seems to be a race between vga_lock and putting the > thread asleep. I'm not sure I understand the race you're talking about. Could you elaborate? -- Kirill A. Shutemov