From ffdf16edfbbe77f5f5c3c87fe8d7387ecd16241b Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Tue, 10 Apr 2018 13:33:12 +0100 Subject: [PATCH 1/4] drm/i915/gvt: fix memory leak of a cmd_entry struct on error exit path The error exit path when a duplicate is found does not kfree and cmd_entry struct and hence there is a small memory leak. Fix this by kfree'ing it. Detected by CoverityScan, CID#1370198 ("Resource Leak") Fixes: be1da7070aea ("drm/i915/gvt: vGPU command scanner") Signed-off-by: Colin Ian King Reviewed-by: Chris Wilson Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/cmd_parser.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c b/drivers/gpu/drm/i915/gvt/cmd_parser.c index db6b94dda5dfa..6bd56ecda31ce 100644 --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c @@ -2863,6 +2863,7 @@ static int init_cmd_table(struct intel_gvt *gvt) if (info) { gvt_err("%s %s duplicated\n", e->info->name, info->name); + kfree(e); return -EEXIST; } From 2f24636b4b12e4f1eca23e6e5dd86ba335fc8066 Mon Sep 17 00:00:00 2001 From: Changbin Du Date: Wed, 11 Apr 2018 16:39:22 +0800 Subject: [PATCH 2/4] drm/i915/gvt: Fix the validation on size field of dp aux header The assertion for len is wrong, so fix it. And for where to validate user input, we should not warn by call trace. [ 290.584739] WARNING: CPU: 0 PID: 1471 at drivers/gpu/drm/i915/gvt/handlers.c:969 dp_aux_ch_ctl_mmio_write+0x394/0x430 [i915] [ 290.586113] task: ffff880111fe8000 task.stack: ffffc90044a9c000 [ 290.586192] RIP: e030:dp_aux_ch_ctl_mmio_write+0x394/0x430 [i915] [ 290.586258] RSP: e02b:ffffc90044a9fd88 EFLAGS: 00010282 [ 290.586315] RAX: 0000000000000017 RBX: 0000000000000003 RCX: ffffffff82461148 [ 290.586391] RDX: 0000000000000000 RSI: 0000000000000001 RDI: 0000000000000201 [ 290.586468] RBP: ffffc90043ed1000 R08: 0000000000000248 R09: 00000000000003d8 [ 290.586544] R10: ffffc90044bdd314 R11: 0000000000000011 R12: 0000000000064310 [ 290.586621] R13: 00000000fe4003ff R14: ffffc900432d1008 R15: ffff88010fa7cb40 [ 290.586701] FS: 0000000000000000(0000) GS:ffff880123200000(0000) knlGS:0000000000000000 [ 290.586787] CS: e033 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 290.586849] CR2: 00007f67ea44e000 CR3: 0000000116078000 CR4: 0000000000042660 [ 290.586926] Call Trace: [ 290.586958] ? __switch_to_asm+0x40/0x70 [ 290.587017] intel_vgpu_mmio_reg_rw+0x1ec/0x3c0 [i915] [ 290.587087] intel_vgpu_emulate_mmio_write+0xa8/0x2c0 [i915] [ 290.587151] xengt_emulation_thread+0x501/0x7a0 [xengt] [ 290.587208] ? __schedule+0x3c6/0x890 [ 290.587250] ? wait_woken+0x80/0x80 [ 290.587290] kthread+0xfc/0x130 [ 290.587326] ? xengt_gpa_to_va+0x1f0/0x1f0 [xengt] [ 290.587378] ? kthread_create_on_node+0x70/0x70 [ 290.587429] ? do_group_exit+0x3a/0xa0 [ 290.587471] ret_from_fork+0x35/0x40 Fixes: 04d348a ("drm/i915/gvt: vGPU display virtualization") Signed-off-by: Changbin Du Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/display.h | 2 +- drivers/gpu/drm/i915/gvt/handlers.c | 13 +++++++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/gvt/display.h b/drivers/gpu/drm/i915/gvt/display.h index b46b86892d58f..ea7c1c525b8c3 100644 --- a/drivers/gpu/drm/i915/gvt/display.h +++ b/drivers/gpu/drm/i915/gvt/display.h @@ -67,7 +67,7 @@ #define AUX_NATIVE_REPLY_NAK (0x1 << 4) #define AUX_NATIVE_REPLY_DEFER (0x2 << 4) -#define AUX_BURST_SIZE 16 +#define AUX_BURST_SIZE 20 /* DPCD addresses */ #define DPCD_REV 0x000 diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c index 9be639aa3b554..dea751e76196c 100644 --- a/drivers/gpu/drm/i915/gvt/handlers.c +++ b/drivers/gpu/drm/i915/gvt/handlers.c @@ -898,11 +898,14 @@ static int dp_aux_ch_ctl_mmio_write(struct intel_vgpu *vgpu, } /* - * Write request format: (command + address) occupies - * 3 bytes, followed by (len + 1) bytes of data. + * Write request format: Headr (command + address + size) occupies + * 4 bytes, followed by (len + 1) bytes of data. See details at + * intel_dp_aux_transfer(). */ - if (WARN_ON((len + 4) > AUX_BURST_SIZE)) + if ((len + 1 + 4) > AUX_BURST_SIZE) { + gvt_vgpu_err("dp_aux_header: len %d is too large\n", len); return -EINVAL; + } /* unpack data from vreg to buf */ for (t = 0; t < 4; t++) { @@ -966,8 +969,10 @@ static int dp_aux_ch_ctl_mmio_write(struct intel_vgpu *vgpu, /* * Read reply format: ACK (1 byte) plus (len + 1) bytes of data. */ - if (WARN_ON((len + 2) > AUX_BURST_SIZE)) + if ((len + 2) > AUX_BURST_SIZE) { + gvt_vgpu_err("dp_aux_header: len %d is too large\n", len); return -EINVAL; + } /* read from virtual DPCD to vreg */ /* first 4 bytes: [ACK][addr][addr+1][addr+2] */ From d54e79340ff8d65b6c63ac278158add2fe211fd0 Mon Sep 17 00:00:00 2001 From: Xiong Zhang Date: Fri, 13 Apr 2018 10:26:16 +0800 Subject: [PATCH 3/4] drm/i915/gvt: Dereference msi eventfd_ctx when it isn't used anymore kvmgt get msi eventfd_ctx at qemu vfio set irq eventfd, then msi eventfd_ctx should be put at some point. The first point is kvmgt handle qemu vfio_disable_irqindex() call which has DATA_NONE and ACTION_TRIGGER in flags. If qemu doesn't call vfio_disable_irqindex(), the second point is vgpu release function. v2: Don't inject msi interrupt into guest if eventfd_ctx is dereferenced Signed-off-by: Xiong Zhang Reviewed-by: Zhenyu Wang Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/kvmgt.c | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c index 021f722e24816..a7487f4575080 100644 --- a/drivers/gpu/drm/i915/gvt/kvmgt.c +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c @@ -566,6 +566,17 @@ static int intel_vgpu_open(struct mdev_device *mdev) return ret; } +static void intel_vgpu_release_msi_eventfd_ctx(struct intel_vgpu *vgpu) +{ + struct eventfd_ctx *trigger; + + trigger = vgpu->vdev.msi_trigger; + if (trigger) { + eventfd_ctx_put(trigger); + vgpu->vdev.msi_trigger = NULL; + } +} + static void __intel_vgpu_release(struct intel_vgpu *vgpu) { struct kvmgt_guest_info *info; @@ -590,6 +601,8 @@ static void __intel_vgpu_release(struct intel_vgpu *vgpu) info = (struct kvmgt_guest_info *)vgpu->handle; kvmgt_guest_exit(info); + intel_vgpu_release_msi_eventfd_ctx(vgpu); + vgpu->vdev.kvm = NULL; vgpu->handle = 0; } @@ -970,7 +983,8 @@ static int intel_vgpu_set_msi_trigger(struct intel_vgpu *vgpu, return PTR_ERR(trigger); } vgpu->vdev.msi_trigger = trigger; - } + } else if ((flags & VFIO_IRQ_SET_DATA_NONE) && !count) + intel_vgpu_release_msi_eventfd_ctx(vgpu); return 0; } @@ -1566,6 +1580,18 @@ static int kvmgt_inject_msi(unsigned long handle, u32 addr, u16 data) info = (struct kvmgt_guest_info *)handle; vgpu = info->vgpu; + /* + * When guest is poweroff, msi_trigger is set to NULL, but vgpu's + * config and mmio register isn't restored to default during guest + * poweroff. If this vgpu is still used in next vm, this vgpu's pipe + * may be enabled, then once this vgpu is active, it will get inject + * vblank interrupt request. But msi_trigger is null until msi is + * enabled by guest. so if msi_trigger is null, success is still + * returned and don't inject interrupt into guest. + */ + if (vgpu->vdev.msi_trigger == NULL) + return 0; + if (eventfd_signal(vgpu->vdev.msi_trigger, 1) == 1) return 0; From 39b4cbadb9a95bf3f13ea102d6ec841940916ee2 Mon Sep 17 00:00:00 2001 From: Changbin Du Date: Fri, 30 Mar 2018 15:35:19 +0800 Subject: [PATCH 4/4] drm/i915/kvmgt: Check the pfn got from vfio_pin_pages This can fix below oops. The target pfn must be mem backed. [ 3639.109674] BUG: unable to handle kernel paging request at ffff8c44832a3000 [ 3639.109681] IP: memcpy_erms+0x6/0x10 [ 3639.109682] PGD 0 P4D 0 [ 3639.109685] Oops: 0000 1 SMP PTI [ 3639.109726] CPU: 2 PID: 1724 Comm: qemu-system-x86 Not tainted 4.16.0-rc5+ #1 [ 3639.109727] Hardware name: /NUC7i7BNB, BIOS BNKBL357.86A.0050.2017.0816.2002 08/16/2017 [ 3639.109729] RIP: 0010:memcpy_erms+0x6/0x10 [ 3639.109730] RSP: 0018:ffffb1b7c3fbbbf0 EFLAGS: 00010246 [ 3639.109731] RAX: ffff8a44b6460000 RBX: 0000000036460000 RCX: 0000000000001000 [ 3639.109732] RDX: 0000000000001000 RSI: ffff8c44832a3000 RDI: ffff8a44b6460000 [ 3639.109733] RBP: 000000000006c8c0 R08: ffff8a44b6460000 R09: 0000000000000000 [ 3639.109734] R10: ffffb1b7c3fbbcd0 R11: ffff8a4d102018c0 R12: 0000000000000000 [ 3639.109734] R13: 0000000000000002 R14: 0000000000200000 R15: 0000000000000000 [ 3639.109736] FS: 00007f37f6d09700(0000) GS:ffff8a4d36d00000(0000) knlGS:0000000000000000 [ 3639.109737] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 3639.109738] CR2: ffff8c44832a3000 CR3: 000000088b7b8004 CR4: 00000000003626e0 [ 3639.109739] Call Trace: [ 3639.109743] swiotlb_tbl_map_single+0x2bb/0x300 [ 3639.109746] map_single+0x30/0x80 [ 3639.109748] swiotlb_map_page+0x87/0x150 [ 3639.109751] kvmgt_dma_map_guest_page+0x329/0x3a0 [kvmgt] [ 3639.109764] ? kvm_write_guest_offset_cached+0x84/0xe0 [kvm] [ 3639.109789] intel_vgpu_emulate_ggtt_mmio_write+0x1f4/0x250 [i915] [ 3639.109808] intel_vgpu_emulate_mmio_write+0x162/0x230 [i915] [ 3639.109811] intel_vgpu_rw+0x1fc/0x240 [kvmgt] [ 3639.109813] intel_vgpu_write+0x164/0x1f0 [kvmgt] [ 3639.109816] __vfs_write+0x33/0x170 [ 3639.109818] ? do_vfs_ioctl+0x9f/0x5f0 [ 3639.109820] vfs_write+0xb3/0x1a0 [ 3639.109822] SyS_pwrite64+0x90/0xb0 [ 3639.109825] do_syscall_64+0x68/0x120 [ 3639.109827] entry_SYSCALL_64_after_hwframe+0x3d/0xa2 [ 3639.109829] RIP: 0033:0x7f3802b2d873 [ 3639.109830] RSP: 002b:00007f37f6d08670 EFLAGS: 00000293 ORIG_RAX: 0000000000000012 [ 3639.109831] RAX: ffffffffffffffda RBX: 0000000000000001 RCX: 00007f3802b2d873 [ 3639.109832] RDX: 0000000000000008 RSI: 00007f37f6d086a0 RDI: 000000000000001a [ 3639.109833] RBP: 00007f37f6d086c0 R08: 0000000000000008 R09: ffffffffffffffff [ 3639.109834] R10: 00000000008041c8 R11: 0000000000000293 R12: 00007ffd8bbf92ae [ 3639.109835] R13: 00007ffd8bbf92af R14: 00007f37f6d09700 R15: 00007f37f6d099c0 v2: add Fixes tag. Signed-off-by: Changbin Du Fixes: cf4ee73 ("drm/i915/gvt: Fix guest vGPU hang caused by very high dma setup overhead") Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/kvmgt.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c index a2a59ff782c61..29ec05b08b86d 100644 --- a/drivers/gpu/drm/i915/gvt/kvmgt.c +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c @@ -123,6 +123,12 @@ static int gvt_dma_map_page(struct intel_vgpu *vgpu, unsigned long gfn, return -EINVAL; } + if (!pfn_valid(pfn)) { + gvt_vgpu_err("pfn 0x%lx is not mem backed\n", pfn); + vfio_unpin_pages(mdev_dev(vgpu->vdev.mdev), &gfn, 1); + return -EINVAL; + } + /* Setup DMA mapping. */ page = pfn_to_page(pfn); *dma_addr = dma_map_page(dev, page, 0, PAGE_SIZE,