From 62d02fd1f807bf5a259a242c483c9fb98a242630 Mon Sep 17 00:00:00 2001 From: Chuanxiao Dong Date: Mon, 26 Jun 2017 15:20:49 +0800 Subject: drm/i915/gvt: Fix possible recursive locking issue vfio_unpin_pages will hold a read semaphore however it is already hold in the same thread by vfio ioctl. It will cause below warning: [ 5102.127454] ============================================ [ 5102.133379] WARNING: possible recursive locking detected [ 5102.139304] 4.12.0-rc4+ #3 Not tainted [ 5102.143483] -------------------------------------------- [ 5102.149407] qemu-system-x86/1620 is trying to acquire lock: [ 5102.155624] (&container->group_lock){++++++}, at: [] vfio_unpin_pages+0x96/0xf0 [ 5102.165626] but task is already holding lock: [ 5102.172134] (&container->group_lock){++++++}, at: [] vfio_fops_unl_ioctl+0x5f/0x280 [ 5102.182522] other info that might help us debug this: [ 5102.189806] Possible unsafe locking scenario: [ 5102.196411] CPU0 [ 5102.199136] ---- [ 5102.201861] lock(&container->group_lock); [ 5102.206527] lock(&container->group_lock); [ 5102.211191] *** DEADLOCK *** [ 5102.217796] May be due to missing lock nesting notation [ 5102.225370] 3 locks held by qemu-system-x86/1620: [ 5102.230618] #0: (&container->group_lock){++++++}, at: [] vfio_fops_unl_ioctl+0x5f/0x280 [ 5102.241482] #1: (&(&iommu->notifier)->rwsem){++++..}, at: [] __blocking_notifier_call_chain+0x35/0x70 [ 5102.253713] #2: (&vgpu->vdev.cache_lock){+.+...}, at: [] intel_vgpu_iommu_notifier+0x77/0x120 [ 5102.265163] stack backtrace: [ 5102.270022] CPU: 5 PID: 1620 Comm: qemu-system-x86 Not tainted 4.12.0-rc4+ #3 [ 5102.277991] Hardware name: Intel Corporation S1200RP/S1200RP, BIOS S1200RP.86B.03.01.APER.061220151418 06/12/2015 [ 5102.289445] Call Trace: [ 5102.292175] dump_stack+0x85/0xc7 [ 5102.295871] validate_chain.isra.21+0x9da/0xaf0 [ 5102.300925] __lock_acquire+0x405/0x820 [ 5102.305202] lock_acquire+0xc7/0x220 [ 5102.309191] ? vfio_unpin_pages+0x96/0xf0 [ 5102.313666] down_read+0x2b/0x50 [ 5102.317259] ? vfio_unpin_pages+0x96/0xf0 [ 5102.321732] vfio_unpin_pages+0x96/0xf0 [ 5102.326024] intel_vgpu_iommu_notifier+0xe5/0x120 [ 5102.331283] notifier_call_chain+0x4a/0x70 [ 5102.335851] __blocking_notifier_call_chain+0x4d/0x70 [ 5102.341490] blocking_notifier_call_chain+0x16/0x20 [ 5102.346935] vfio_iommu_type1_ioctl+0x87b/0x920 [ 5102.351994] vfio_fops_unl_ioctl+0x81/0x280 [ 5102.356660] ? __fget+0xf0/0x210 [ 5102.360261] do_vfs_ioctl+0x93/0x6a0 [ 5102.364247] ? __fget+0x111/0x210 [ 5102.367942] SyS_ioctl+0x41/0x70 [ 5102.371542] entry_SYSCALL_64_fastpath+0x1f/0xbe put the vfio_unpin_pages in a workqueue can fix this. v2: - use for style instead of do{}while(1). (Zhenyu) v3: - rename gvt_cache_mark to gvt_cache_mark_remove. (Zhenyu) Fixes: 659643f7d814 ("drm/i915/gvt/kvmgt: add vfio/mdev support to KVMGT") Signed-off-by: Chuanxiao Dong Cc: Zhenyu Wang Cc: stable@vger.kernel.org # v4.10+ Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/gvt.h | 3 +++ drivers/gpu/drm/i915/gvt/kvmgt.c | 55 ++++++++++++++++++++++++++++++++-------- 2 files changed, 48 insertions(+), 10 deletions(-) (limited to 'drivers') diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h index 930732e5c780..8b5876d63624 100644 --- a/drivers/gpu/drm/i915/gvt/gvt.h +++ b/drivers/gpu/drm/i915/gvt/gvt.h @@ -183,6 +183,9 @@ struct intel_vgpu { struct kvm *kvm; struct work_struct release_work; atomic_t released; + struct work_struct unpin_work; + spinlock_t unpin_lock; /* To protect unpin_list */ + struct list_head unpin_list; } vdev; #endif }; diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c index 1ae0b4083ce1..56a8d6a7d9c6 100644 --- a/drivers/gpu/drm/i915/gvt/kvmgt.c +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c @@ -78,6 +78,7 @@ struct gvt_dma { struct rb_node node; gfn_t gfn; unsigned long iova; + struct list_head list; }; static inline bool handle_valid(unsigned long handle) @@ -166,6 +167,7 @@ static void gvt_cache_add(struct intel_vgpu *vgpu, gfn_t gfn, new->gfn = gfn; new->iova = iova; + INIT_LIST_HEAD(&new->list); mutex_lock(&vgpu->vdev.cache_lock); while (*link) { @@ -197,26 +199,52 @@ static void __gvt_cache_remove_entry(struct intel_vgpu *vgpu, kfree(entry); } -static void gvt_cache_remove(struct intel_vgpu *vgpu, gfn_t gfn) +static void intel_vgpu_unpin_work(struct work_struct *work) { + struct intel_vgpu *vgpu = container_of(work, struct intel_vgpu, + vdev.unpin_work); struct device *dev = mdev_dev(vgpu->vdev.mdev); struct gvt_dma *this; - unsigned long g1; - int rc; + unsigned long gfn; + + for (;;) { + spin_lock(&vgpu->vdev.unpin_lock); + if (list_empty(&vgpu->vdev.unpin_list)) { + spin_unlock(&vgpu->vdev.unpin_lock); + break; + } + this = list_first_entry(&vgpu->vdev.unpin_list, + struct gvt_dma, list); + list_del(&this->list); + spin_unlock(&vgpu->vdev.unpin_lock); + + gfn = this->gfn; + vfio_unpin_pages(dev, &gfn, 1); + kfree(this); + } +} + +static bool gvt_cache_mark_remove(struct intel_vgpu *vgpu, gfn_t gfn) +{ + struct gvt_dma *this; mutex_lock(&vgpu->vdev.cache_lock); this = __gvt_cache_find(vgpu, gfn); if (!this) { mutex_unlock(&vgpu->vdev.cache_lock); - return; + return false; } - - g1 = gfn; gvt_dma_unmap_iova(vgpu, this->iova); - rc = vfio_unpin_pages(dev, &g1, 1); - WARN_ON(rc != 1); - __gvt_cache_remove_entry(vgpu, this); + /* remove this from rb tree */ + rb_erase(&this->node, &vgpu->vdev.cache); mutex_unlock(&vgpu->vdev.cache_lock); + + /* put this to the unpin_list */ + spin_lock(&vgpu->vdev.unpin_lock); + list_move_tail(&this->list, &vgpu->vdev.unpin_list); + spin_unlock(&vgpu->vdev.unpin_lock); + + return true; } static void gvt_cache_init(struct intel_vgpu *vgpu) @@ -453,6 +481,9 @@ static int intel_vgpu_create(struct kobject *kobj, struct mdev_device *mdev) } INIT_WORK(&vgpu->vdev.release_work, intel_vgpu_release_work); + INIT_WORK(&vgpu->vdev.unpin_work, intel_vgpu_unpin_work); + spin_lock_init(&vgpu->vdev.unpin_lock); + INIT_LIST_HEAD(&vgpu->vdev.unpin_list); vgpu->vdev.mdev = mdev; mdev_set_drvdata(mdev, vgpu); @@ -482,6 +513,7 @@ static int intel_vgpu_iommu_notifier(struct notifier_block *nb, struct intel_vgpu *vgpu = container_of(nb, struct intel_vgpu, vdev.iommu_notifier); + bool sched_unmap = false; if (action == VFIO_IOMMU_NOTIFY_DMA_UNMAP) { struct vfio_iommu_type1_dma_unmap *unmap = data; @@ -491,7 +523,10 @@ static int intel_vgpu_iommu_notifier(struct notifier_block *nb, end_gfn = gfn + unmap->size / PAGE_SIZE; while (gfn < end_gfn) - gvt_cache_remove(vgpu, gfn++); + sched_unmap |= gvt_cache_mark_remove(vgpu, gfn++); + + if (sched_unmap) + schedule_work(&vgpu->vdev.unpin_work); } return NOTIFY_OK; -- cgit v1.2.3 From f16bd3dda2c8bf6699e808cd9cc540cfab10e60e Mon Sep 17 00:00:00 2001 From: Chuanxiao Dong Date: Mon, 26 Jun 2017 15:20:50 +0800 Subject: drm/i915/gvt: Fix inconsistent locks holding sequence There are two kinds of locking sequence. One is in the thread which is started by vfio ioctl to do the iommu unmapping. The locking sequence is: down_read(&group_lock) ----> mutex_lock(&cached_lock) The other is in the vfio release thread which will unpin all the cached pages. The lock sequence is: mutex_lock(&cached_lock) ---> down_read(&group_lock) And, the cache_lock is used to protect the rb tree of the cache node and doing vfio unpin doesn't require this lock. Move the vfio unpin out of the cache_lock protected region. v2: - use for style instead of do{}while(1). (Zhenyu) Fixes: f30437c5e7bf ("drm/i915/gvt: add KVMGT support") Signed-off-by: Chuanxiao Dong Cc: Zhenyu Wang Cc: stable@vger.kernel.org # v4.10+ Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/kvmgt.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) (limited to 'drivers') diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c index 56a8d6a7d9c6..75a6e1d8af0d 100644 --- a/drivers/gpu/drm/i915/gvt/kvmgt.c +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c @@ -260,16 +260,20 @@ static void gvt_cache_destroy(struct intel_vgpu *vgpu) struct device *dev = mdev_dev(vgpu->vdev.mdev); unsigned long gfn; - mutex_lock(&vgpu->vdev.cache_lock); - while ((node = rb_first(&vgpu->vdev.cache))) { + for (;;) { + mutex_lock(&vgpu->vdev.cache_lock); + node = rb_first(&vgpu->vdev.cache); + if (!node) { + mutex_unlock(&vgpu->vdev.cache_lock); + break; + } dma = rb_entry(node, struct gvt_dma, node); gvt_dma_unmap_iova(vgpu, dma->iova); gfn = dma->gfn; - - vfio_unpin_pages(dev, &gfn, 1); __gvt_cache_remove_entry(vgpu, dma); + mutex_unlock(&vgpu->vdev.cache_lock); + vfio_unpin_pages(dev, &gfn, 1); } - mutex_unlock(&vgpu->vdev.cache_lock); } static struct intel_vgpu_type *intel_gvt_find_vgpu_type(struct intel_gvt *gvt, -- cgit v1.2.3 From 295a0d0b55269fce2290a7aebfcc8c2525866c33 Mon Sep 17 00:00:00 2001 From: Xiong Zhang Date: Tue, 20 Jun 2017 11:37:22 +0800 Subject: drm/i915/gvt: Set initial PORT_CLK_SEL vreg for BDW On BDW, when host physical screen and guest virtual screen aren't on the same DDI port, guest i915 driver prints the following error and stop running. [ 6.775873] BUG: unable to handle kernel NULL pointer dereference at 0000000000000068 [ 6.775928] IP: intel_ddi_clock_get+0x81/0x430 [i915] [ 6.776206] Call Trace: [ 6.776233] ? vgpu_read32+0x4f/0x100 [i915] [ 6.776264] intel_ddi_get_config+0x11c/0x230 [i915] [ 6.776298] intel_modeset_setup_hw_state+0x313/0xd40 [i915] [ 6.776334] intel_modeset_init+0xe49/0x18d0 [i915] [ 6.776368] ? vgpu_write32+0x53/0x100 [i915] [ 6.776731] ? intel_i2c_reset+0x42/0x50 [i915] [ 6.777085] ? intel_setup_gmbus+0x32a/0x350 [i915] [ 6.777427] i915_driver_load+0xabc/0x14d0 [i915] [ 6.777768] i915_pci_probe+0x4f/0x70 [i915] The null pointer is guest intel_crtc_state->shared_dpll which is setted in haswell_get_ddi_pll(). When guest and host screen are on different DDI port, host driver won't set PORT_CLK_SET(guest_port), so haswell_get_ddi_pll() will return null and don't set pipe_config->shared_dpll, once the following program refernce this structure, it will print the above error. This patch set the initial val of guest PORT_CLK_SEL(guest_port) to LCPLL_810. And guest i915 driver will reset this value according to guest screen mode. Signed-off-by: Xiong Zhang Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/display.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'drivers') diff --git a/drivers/gpu/drm/i915/gvt/display.c b/drivers/gpu/drm/i915/gvt/display.c index e0261fcc5b50..818e94907c3b 100644 --- a/drivers/gpu/drm/i915/gvt/display.c +++ b/drivers/gpu/drm/i915/gvt/display.c @@ -197,6 +197,12 @@ static void emulate_monitor_status_change(struct intel_vgpu *vgpu) (TRANS_DDI_BPC_8 | TRANS_DDI_MODE_SELECT_DP_SST | (PORT_B << TRANS_DDI_PORT_SHIFT) | TRANS_DDI_FUNC_ENABLE); + if (IS_BROADWELL(dev_priv)) { + vgpu_vreg(vgpu, PORT_CLK_SEL(PORT_B)) &= + ~PORT_CLK_SEL_MASK; + vgpu_vreg(vgpu, PORT_CLK_SEL(PORT_B)) |= + PORT_CLK_SEL_LCPLL_810; + } vgpu_vreg(vgpu, DDI_BUF_CTL(PORT_B)) |= DDI_BUF_CTL_ENABLE; vgpu_vreg(vgpu, DDI_BUF_CTL(PORT_B)) &= ~DDI_BUF_IS_IDLE; vgpu_vreg(vgpu, SDEISR) |= SDE_PORTB_HOTPLUG_CPT; @@ -211,6 +217,12 @@ static void emulate_monitor_status_change(struct intel_vgpu *vgpu) (TRANS_DDI_BPC_8 | TRANS_DDI_MODE_SELECT_DP_SST | (PORT_C << TRANS_DDI_PORT_SHIFT) | TRANS_DDI_FUNC_ENABLE); + if (IS_BROADWELL(dev_priv)) { + vgpu_vreg(vgpu, PORT_CLK_SEL(PORT_C)) &= + ~PORT_CLK_SEL_MASK; + vgpu_vreg(vgpu, PORT_CLK_SEL(PORT_C)) |= + PORT_CLK_SEL_LCPLL_810; + } vgpu_vreg(vgpu, DDI_BUF_CTL(PORT_C)) |= DDI_BUF_CTL_ENABLE; vgpu_vreg(vgpu, DDI_BUF_CTL(PORT_C)) &= ~DDI_BUF_IS_IDLE; vgpu_vreg(vgpu, SFUSE_STRAP) |= SFUSE_STRAP_DDIC_DETECTED; @@ -225,6 +237,12 @@ static void emulate_monitor_status_change(struct intel_vgpu *vgpu) (TRANS_DDI_BPC_8 | TRANS_DDI_MODE_SELECT_DP_SST | (PORT_D << TRANS_DDI_PORT_SHIFT) | TRANS_DDI_FUNC_ENABLE); + if (IS_BROADWELL(dev_priv)) { + vgpu_vreg(vgpu, PORT_CLK_SEL(PORT_D)) &= + ~PORT_CLK_SEL_MASK; + vgpu_vreg(vgpu, PORT_CLK_SEL(PORT_D)) |= + PORT_CLK_SEL_LCPLL_810; + } vgpu_vreg(vgpu, DDI_BUF_CTL(PORT_D)) |= DDI_BUF_CTL_ENABLE; vgpu_vreg(vgpu, DDI_BUF_CTL(PORT_D)) &= ~DDI_BUF_IS_IDLE; vgpu_vreg(vgpu, SFUSE_STRAP) |= SFUSE_STRAP_DDID_DETECTED; -- cgit v1.2.3 From 75e64ff2c2f5ce1ae5b47b2f372fe5b9dc99f5a9 Mon Sep 17 00:00:00 2001 From: Xiong Zhang Date: Wed, 28 Jun 2017 02:03:16 +0800 Subject: drm/i915/gvt: Don't read ADPA_CRT_HOTPLUG_MONITOR from host When host connects a crt screen, linux guest will detect two screens: crt and dp. This is wrong as linux guest has only one dp. In order to avoid guest get host crt screen, we should set ADPA_CRT_HOTPLUG_MONITOR to none. But MMIO_RO(PCH_ADPA) prevent from that. So MMIO_DH should be used instead of MMIO_RO. v2: Clear its staus to none at initialize, so guest don't get host crt.(Zhangyu) v3: SKL doesn't have this register, limit it to pre_skl.(xiong) Signed-off-by: Xiong Zhang Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/display.c | 4 ++++ drivers/gpu/drm/i915/gvt/handlers.c | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/gpu/drm/i915/gvt/display.c b/drivers/gpu/drm/i915/gvt/display.c index 818e94907c3b..2deb05f618fb 100644 --- a/drivers/gpu/drm/i915/gvt/display.c +++ b/drivers/gpu/drm/i915/gvt/display.c @@ -262,6 +262,10 @@ static void emulate_monitor_status_change(struct intel_vgpu *vgpu) vgpu_vreg(vgpu, DDI_BUF_CTL(PORT_A)) |= DDI_INIT_DISPLAY_DETECTED; } + + /* Clear host CRT status, so guest couldn't detect this host CRT. */ + if (IS_BROADWELL(dev_priv)) + vgpu_vreg(vgpu, PCH_ADPA) &= ~ADPA_CRT_HOTPLUG_MONITOR_MASK; } static void clean_virtual_dp_monitor(struct intel_vgpu *vgpu, int port_num) diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c index 0ffd69654592..437a35f41b62 100644 --- a/drivers/gpu/drm/i915/gvt/handlers.c +++ b/drivers/gpu/drm/i915/gvt/handlers.c @@ -1905,7 +1905,7 @@ static int init_generic_mmio_info(struct intel_gvt *gvt) MMIO_F(_PCH_DPD_AUX_CH_CTL, 6 * 4, 0, 0, 0, D_PRE_SKL, NULL, dp_aux_ch_ctl_mmio_write); - MMIO_RO(PCH_ADPA, D_ALL, 0, ADPA_CRT_HOTPLUG_MONITOR_MASK, NULL, pch_adpa_mmio_write); + MMIO_DH(PCH_ADPA, D_PRE_SKL, NULL, pch_adpa_mmio_write); MMIO_DH(_PCH_TRANSACONF, D_ALL, NULL, transconf_mmio_write); MMIO_DH(_PCH_TRANSBCONF, D_ALL, NULL, transconf_mmio_write); -- cgit v1.2.3 From 5cd82b757795228516bf60a0552d1a40fa8adeb2 Mon Sep 17 00:00:00 2001 From: Changbin Du Date: Tue, 13 Jun 2017 10:15:26 +0800 Subject: drm/i915/gvt: Make function dpy_reg_mmio_readx safe The dpy_reg_mmio_read_x functions directly copy 4 bytes data to the target address with considering the length. If may cause the target memory corrupted if the requested length less than 4 bytes. Fix it for safety even we already have some checking to avoid this happen. And for convince, the 3 functions are merged. Signed-off-by: Changbin Du Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/handlers.c | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) (limited to 'drivers') diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c index 437a35f41b62..a9114aa1eddb 100644 --- a/drivers/gpu/drm/i915/gvt/handlers.c +++ b/drivers/gpu/drm/i915/gvt/handlers.c @@ -366,21 +366,24 @@ static int lcpll_ctl_mmio_write(struct intel_vgpu *vgpu, unsigned int offset, static int dpy_reg_mmio_read(struct intel_vgpu *vgpu, unsigned int offset, void *p_data, unsigned int bytes) { - *(u32 *)p_data = (1 << 17); - return 0; -} - -static int dpy_reg_mmio_read_2(struct intel_vgpu *vgpu, unsigned int offset, - void *p_data, unsigned int bytes) -{ - *(u32 *)p_data = 3; - return 0; -} + switch (offset) { + case 0xe651c: + case 0xe661c: + case 0xe671c: + case 0xe681c: + vgpu_vreg(vgpu, offset) = 1 << 17; + break; + case 0xe6c04: + vgpu_vreg(vgpu, offset) = 0x3; + break; + case 0xe6e1c: + vgpu_vreg(vgpu, offset) = 0x2f << 16; + break; + default: + return -EINVAL; + } -static int dpy_reg_mmio_read_3(struct intel_vgpu *vgpu, unsigned int offset, - void *p_data, unsigned int bytes) -{ - *(u32 *)p_data = (0x2f << 16); + read_vreg(vgpu, offset, p_data, bytes); return 0; } @@ -1991,8 +1994,8 @@ static int init_generic_mmio_info(struct intel_gvt *gvt) MMIO_DH(0xe661c, D_ALL, dpy_reg_mmio_read, NULL); MMIO_DH(0xe671c, D_ALL, dpy_reg_mmio_read, NULL); MMIO_DH(0xe681c, D_ALL, dpy_reg_mmio_read, NULL); - MMIO_DH(0xe6c04, D_ALL, dpy_reg_mmio_read_2, NULL); - MMIO_DH(0xe6e1c, D_ALL, dpy_reg_mmio_read_3, NULL); + MMIO_DH(0xe6c04, D_ALL, dpy_reg_mmio_read, NULL); + MMIO_DH(0xe6e1c, D_ALL, dpy_reg_mmio_read, NULL); MMIO_RO(PCH_PORT_HOTPLUG, D_ALL, 0, PORTA_HOTPLUG_STATUS_MASK -- cgit v1.2.3 From ce3f7163e4ce8fd583dcb36b6ee6b81fd1b419ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Mon, 26 Jun 2017 23:30:51 +0300 Subject: drm/i915: Disable MSI for all pre-gen5 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We have pretty clear evidence that MSIs are getting lost on g4x and somehow the interrupt logic doesn't seem to recover from that state even if we try hard to clear the IIR. Disabling IER around the normal IIR clearing in the irq handler isn't sufficient to avoid this, so the problem really seems to be further up the interrupt chain. This should guarantee that there's always an edge if any IIR bits are set after the interrupt handler is done, which should normally guarantee that the CPU interrupt is generated. That approach seems to work perfectly on VLV/CHV, but apparently not on g4x. MSI is documented to be broken on 965gm at least. The chipset spec says MSI is defeatured because interrupts can be delayed or lost, which fits well with what we're seeing on g4x. Previously we've already disabled GMBUS interrupts on g4x because somehow GMBUS manages to raise legacy interrupts even when MSI is enabled. Since there's such widespread MSI breakahge all over in the pre-gen5 land let's just give up on MSI on these platforms. Seqno reporting might be negatively affected by this since the legcy interrupts aren't guaranteed to be ordered with the seqno writes, whereas MSI interrupts may be? But an occasioanlly missed seqno seems like a small price to pay for generally working interrupts. Cc: stable@vger.kernel.org Cc: Diego Viola Tested-by: Diego Viola Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101261 Signed-off-by: Ville Syrjälä Link: http://patchwork.freedesktop.org/patch/msgid/20170626203051.28480-1-ville.syrjala@linux.intel.com Reviewed-by: Daniel Vetter (cherry picked from commit e38c2da01f76cca82b59ca612529b81df82a7cc7) Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/i915_drv.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index ee2325b180e7..fc307e03943c 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1132,10 +1132,12 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv) * and the registers being closely associated. * * According to chipset errata, on the 965GM, MSI interrupts may - * be lost or delayed, but we use them anyways to avoid - * stuck interrupts on some machines. + * be lost or delayed, and was defeatured. MSI interrupts seem to + * get lost on g4x as well, and interrupt delivery seems to stay + * properly dead afterwards. So we'll just disable them for all + * pre-gen5 chipsets. */ - if (!IS_I945G(dev_priv) && !IS_I945GM(dev_priv)) { + if (INTEL_GEN(dev_priv) >= 5) { if (pci_enable_msi(pdev) < 0) DRM_DEBUG_DRIVER("can't enable MSI"); } -- cgit v1.2.3 From 1a13a2ec3eb360c80e1ea88b9b7616fa64d4e278 Mon Sep 17 00:00:00 2001 From: Christophe JAILLET Date: Tue, 27 Jun 2017 07:38:54 +0200 Subject: drm/i915: Fix an error checking test 'dma_buf_vmap' returns NULL on error, not an error pointer. Fixes: 6cca22ede8a4 ("drm/i915: Add some mock tests for dmabuf interop") Signed-off-by: Christophe JAILLET Link: http://patchwork.freedesktop.org/patch/msgid/20170627053854.21152-1-christophe.jaillet@wanadoo.fr Reviewed-by: Chris Wilson Signed-off-by: Chris Wilson (cherry picked from commit 7c3f5317b8c2828ab10e8cf87c8ab5232d1966d0) Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/selftests/i915_gem_dmabuf.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers') diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/selftests/i915_gem_dmabuf.c index d15cc9d3a5cd..89dc25a5a53b 100644 --- a/drivers/gpu/drm/i915/selftests/i915_gem_dmabuf.c +++ b/drivers/gpu/drm/i915/selftests/i915_gem_dmabuf.c @@ -246,9 +246,9 @@ static int igt_dmabuf_export_vmap(void *arg) i915_gem_object_put(obj); ptr = dma_buf_vmap(dmabuf); - if (IS_ERR(ptr)) { - err = PTR_ERR(ptr); - pr_err("dma_buf_vmap failed with err=%d\n", err); + if (!ptr) { + pr_err("dma_buf_vmap failed\n"); + err = -ENOMEM; goto out; } -- cgit v1.2.3 From 9c75b185274b7766fe69c2e73607c1ed780b284b Mon Sep 17 00:00:00 2001 From: Gabriel Krisman Bertazi Date: Wed, 28 Jun 2017 18:06:05 -0300 Subject: drm/i915: reintroduce VLV/CHV PFI programming power domain workaround MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are still cases on these platforms where an attempt is made to configure the CDCLK while the power domain is off, like when coming back from a suspend. So the workaround below is still needed. This effectively reverts commit 63ff30442519 ("drm/i915: Nuke the VLV/CHV PFI programming power domain workaround"). Cc: stable@vger.kernel.org Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101517 Suggested-by: Ville Syrjälä Signed-off-by: Gabriel Krisman Bertazi Link: http://patchwork.freedesktop.org/patch/msgid/20170628210605.4994-1-krisman@collabora.co.uk Reviewed-by: Ville Syrjälä Signed-off-by: Ville Syrjälä (cherry picked from commit 886015a0ad43c7fc034b23ea4614ba39162f9ddd) Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_cdclk.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) (limited to 'drivers') diff --git a/drivers/gpu/drm/i915/intel_cdclk.c b/drivers/gpu/drm/i915/intel_cdclk.c index b8914db7d2e1..1241e5891b29 100644 --- a/drivers/gpu/drm/i915/intel_cdclk.c +++ b/drivers/gpu/drm/i915/intel_cdclk.c @@ -491,6 +491,14 @@ static void vlv_set_cdclk(struct drm_i915_private *dev_priv, int cdclk = cdclk_state->cdclk; u32 val, cmd; + /* There are cases where we can end up here with power domains + * off and a CDCLK frequency other than the minimum, like when + * issuing a modeset without actually changing any display after + * a system suspend. So grab the PIPE-A domain, which covers + * the HW blocks needed for the following programming. + */ + intel_display_power_get(dev_priv, POWER_DOMAIN_PIPE_A); + if (cdclk >= 320000) /* jump to highest voltage for 400MHz too */ cmd = 2; else if (cdclk == 266667) @@ -549,6 +557,8 @@ static void vlv_set_cdclk(struct drm_i915_private *dev_priv, intel_update_cdclk(dev_priv); vlv_program_pfi_credits(dev_priv); + + intel_display_power_put(dev_priv, POWER_DOMAIN_PIPE_A); } static void chv_set_cdclk(struct drm_i915_private *dev_priv, @@ -568,6 +578,14 @@ static void chv_set_cdclk(struct drm_i915_private *dev_priv, return; } + /* There are cases where we can end up here with power domains + * off and a CDCLK frequency other than the minimum, like when + * issuing a modeset without actually changing any display after + * a system suspend. So grab the PIPE-A domain, which covers + * the HW blocks needed for the following programming. + */ + intel_display_power_get(dev_priv, POWER_DOMAIN_PIPE_A); + /* * Specs are full of misinformation, but testing on actual * hardware has shown that we just need to write the desired @@ -590,6 +608,8 @@ static void chv_set_cdclk(struct drm_i915_private *dev_priv, intel_update_cdclk(dev_priv); vlv_program_pfi_credits(dev_priv); + + intel_display_power_put(dev_priv, POWER_DOMAIN_PIPE_A); } static int bdw_calc_cdclk(int max_pixclk) -- cgit v1.2.3 From 4ec654bf3a63d503e3c5336eade5c369ae17db56 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Thu, 29 Jun 2017 16:04:25 +0100 Subject: drm/i915: Avoid undefined behaviour of "u32 >> 32" When computing a hash for looking up relocation target handles in an execbuf, we start with a large size for the hashtable and proceed to halve it until the allocation succeeds. The final attempt is with an order of 0 (i.e. a single element). This means that we then pass bits=0 to hash_32() which then computes "hash >> (32 - 0)" to lookup the single element. Right shifting a value by the width of the operand is undefined, so limit the smallest hash table we use to order 1. v2: Keep the retry allocation flag for the final pass Fixes: 4ff4b44cbb70 ("drm/i915: Store a direct lookup from object handle to vma") Signed-off-by: Chris Wilson Cc: Joonas Lahtinen Cc: Tvrtko Ursulin Reviewed-by: Tvrtko Ursulin Link: http://patchwork.freedesktop.org/patch/msgid/20170629150425.27508-1-chris@chris-wilson.co.uk (cherry picked from commit 4d470f7359c4bf22518baa30700ad45649371a22) Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 38 +++++++++++++++++++----------- 1 file changed, 24 insertions(+), 14 deletions(-) (limited to 'drivers') diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 9337446f1068..054b2e54cdaf 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -288,20 +288,26 @@ static int eb_create(struct i915_execbuffer *eb) * direct lookup. */ do { + unsigned int flags; + + /* While we can still reduce the allocation size, don't + * raise a warning and allow the allocation to fail. + * On the last pass though, we want to try as hard + * as possible to perform the allocation and warn + * if it fails. + */ + flags = GFP_TEMPORARY; + if (size > 1) + flags |= __GFP_NORETRY | __GFP_NOWARN; + eb->buckets = kzalloc(sizeof(struct hlist_head) << size, - GFP_TEMPORARY | - __GFP_NORETRY | - __GFP_NOWARN); + flags); if (eb->buckets) break; } while (--size); - if (unlikely(!eb->buckets)) { - eb->buckets = kzalloc(sizeof(struct hlist_head), - GFP_TEMPORARY); - if (unlikely(!eb->buckets)) - return -ENOMEM; - } + if (unlikely(!size)) + return -ENOMEM; eb->lut_size = size; } else { @@ -452,7 +458,7 @@ eb_add_vma(struct i915_execbuffer *eb, return err; } - if (eb->lut_size >= 0) { + if (eb->lut_size > 0) { vma->exec_handle = entry->handle; hlist_add_head(&vma->exec_node, &eb->buckets[hash_32(entry->handle, @@ -894,7 +900,7 @@ static void eb_release_vmas(const struct i915_execbuffer *eb) static void eb_reset_vmas(const struct i915_execbuffer *eb) { eb_release_vmas(eb); - if (eb->lut_size >= 0) + if (eb->lut_size > 0) memset(eb->buckets, 0, sizeof(struct hlist_head) << eb->lut_size); } @@ -903,7 +909,7 @@ static void eb_destroy(const struct i915_execbuffer *eb) { GEM_BUG_ON(eb->reloc_cache.rq); - if (eb->lut_size >= 0) + if (eb->lut_size > 0) kfree(eb->buckets); } @@ -2180,8 +2186,11 @@ i915_gem_do_execbuffer(struct drm_device *dev, } } - if (eb_create(&eb)) - return -ENOMEM; + err = eb_create(&eb); + if (err) + goto err_out_fence; + + GEM_BUG_ON(!eb.lut_size); /* * Take a local wakeref for preparing to dispatch the execbuf as @@ -2340,6 +2349,7 @@ err_unlock: err_rpm: intel_runtime_pm_put(eb.i915); eb_destroy(&eb); +err_out_fence: if (out_fence_fd != -1) put_unused_fd(out_fence_fd); err_in_fence: -- cgit v1.2.3 From 2347728934b501b07bea8b1b6e8fa27a56dc098f Mon Sep 17 00:00:00 2001 From: Rodrigo Vivi Date: Mon, 19 Jun 2017 14:21:47 -0700 Subject: drm/i915/cfl: Fix Workarounds. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit During the review of Coffee Lake workarounds Mika pointed out that WaDisableKillLogic and GEN9_DISABLE_OCL_OOB_SUPPRESS_LOGIC should be removed from CFL and with that I should carry the rv-b. However when doing the v2 I removed another Workaround that should remain because although not mentioned by spec the history of hangs around it advocates on its favor. On some follow-up patches I continued operating on the wrong workardound, but Ville noticed that, so here is the fix for the current CFL code that is upstream already. Fixes: 46c26662d2f ("drm/i915/cfl: Introduce Coffee Lake workarounds.") Cc: Ville Syrjälä Cc: Dhinakaran Pandiyan Cc: Mika Kuoppala Signed-off-by: Rodrigo Vivi Reviewed-by: Dhinakaran Pandiyan (cherry picked from commit 98eed3d1ade53596e1c8785e049f03da4480a820) Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_engine_cs.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'drivers') diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index a4487c5b7e37..5b4de719bec3 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -821,9 +821,10 @@ static int gen9_init_workarounds(struct intel_engine_cs *engine) I915_WRITE(BDW_SCRATCH1, I915_READ(BDW_SCRATCH1) | GEN9_LBS_SLA_RETRY_TIMER_DECREMENT_ENABLE); - /* WaDisableKillLogic:bxt,skl,kbl,cfl */ - I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | - ECOCHK_DIS_TLB); + /* WaDisableKillLogic:bxt,skl,kbl */ + if (!IS_COFFEELAKE(dev_priv)) + I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | + ECOCHK_DIS_TLB); /* WaClearFlowControlGpgpuContextSave:skl,bxt,kbl,glk,cfl */ /* WaDisablePartialInstShootdown:skl,bxt,kbl,glk,cfl */ @@ -894,10 +895,9 @@ static int gen9_init_workarounds(struct intel_engine_cs *engine) WA_SET_BIT_MASKED(HDC_CHICKEN0, HDC_FORCE_NON_COHERENT); - /* WaDisableHDCInvalidation:skl,bxt,kbl */ - if (!IS_COFFEELAKE(dev_priv)) - I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | - BDW_DISABLE_HDC_INVALIDATION); + /* WaDisableHDCInvalidation:skl,bxt,kbl,cfl */ + I915_WRITE(GAM_ECOCHK, I915_READ(GAM_ECOCHK) | + BDW_DISABLE_HDC_INVALIDATION); /* WaDisableSamplerPowerBypassForSOPingPong:skl,bxt,kbl,cfl */ if (IS_SKYLAKE(dev_priv) || -- cgit v1.2.3 From c379b897ba1a7f786419e8c36a3438ce856f016a Mon Sep 17 00:00:00 2001 From: "Navare, Manasi D" Date: Thu, 29 Jun 2017 18:14:01 -0700 Subject: drm/i915/cnl: Fix the CURSOR_COEFF_MASK used in DDI Vswing Programming The Cursor Coeff is lower 6 bits in the PORT_TX_DW4 register and hence the CURSOR_COEFF_MASK should be (0x3F << 0) Fixes: 04416108ccea ("drm/i915/cnl: Add registers related to voltage swing sequences.") Signed-off-by: Manasi Navare Reviewed-by: Rodrigo Vivi Signed-off-by: Rodrigo Vivi Link: http://patchwork.freedesktop.org/patch/msgid/1498785241-21138-1-git-send-email-manasi.d.navare@intel.com (cherry picked from commit fcace3b9b727e25ffa3f7ad2c96e76b8584a9f3e) Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/i915_reg.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index c8647cfa81ba..64cc674b652a 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -1802,7 +1802,7 @@ enum skl_disp_power_wells { #define POST_CURSOR_2(x) ((x) << 6) #define POST_CURSOR_2_MASK (0x3F << 6) #define CURSOR_COEFF(x) ((x) << 0) -#define CURSOR_COEFF_MASK (0x3F << 6) +#define CURSOR_COEFF_MASK (0x3F << 0) #define _CNL_PORT_TX_DW5_GRP_AE 0x162354 #define _CNL_PORT_TX_DW5_GRP_B 0x1623D4 -- cgit v1.2.3 From 04941829b0049d2446c7042ab9686dd057d809a6 Mon Sep 17 00:00:00 2001 From: "sagar.a.kamble@intel.com" Date: Tue, 27 Jun 2017 23:09:41 +0530 Subject: drm/i915: Hold RPM wakelock while initializing OA buffer OA buffer initialization involves access to HW registers to set the OA base, head and tail. Ensure device is awake while setting these. With this, all oa.ops are covered under RPM and forcewake wakelock. Cc: Lionel Landwerlin Signed-off-by: Sagar Arun Kamble Reviewed-by: Lionel Landwerlin Signed-off-by: Maarten Lankhorst Link: http://patchwork.freedesktop.org/patch/msgid/1498585181-23048-1-git-send-email-sagar.a.kamble@intel.com Fixes: d79651522e89c ("drm/i915: Enable i915 perf stream for Haswell OA unit") Cc: # v4.11+ (cherry picked from commit 987f8c444aa2c33d98e7030d0c5f0a5325cc84ea) Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/i915_perf.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'drivers') diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index 38c44407bafc..9cd22f83b0cf 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -2067,10 +2067,6 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream, return ret; } - ret = alloc_oa_buffer(dev_priv); - if (ret) - goto err_oa_buf_alloc; - /* PRM - observability performance counters: * * OACONTROL, performance counter enable, note: @@ -2086,6 +2082,10 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream, intel_runtime_pm_get(dev_priv); intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL); + ret = alloc_oa_buffer(dev_priv); + if (ret) + goto err_oa_buf_alloc; + ret = dev_priv->perf.oa.ops.enable_metric_set(dev_priv); if (ret) goto err_enable; @@ -2097,11 +2097,11 @@ static int i915_oa_stream_init(struct i915_perf_stream *stream, return 0; err_enable: - intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); - intel_runtime_pm_put(dev_priv); free_oa_buffer(dev_priv); err_oa_buf_alloc: + intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL); + intel_runtime_pm_put(dev_priv); if (stream->ctx) oa_put_render_ctx_id(stream); -- cgit v1.2.3 From 7581d5ca2bb269cfc2ce2d0cb489aac513167f6b Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Thu, 22 Jun 2017 17:02:11 +0100 Subject: drm/i915/fbdev: Check for existence of ifbdev->vma before operations Commit fabef825626d ("drm/i915: Drop struct_mutex around frontbuffer flushes") adds a dependency to ifbdev->vma when flushing the framebufer, but the checks are only against the existence of the ifbdev->fb and not against ifbdev->vma. This leaves a window of opportunity where we may try to operate on the fbdev prior to it being probed (thanks to asynchronous booting). Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101534 Fixes: fabef825626d ("drm/i915: Drop struct_mutex around frontbuffer flushes") Signed-off-by: Chris Wilson Cc: Joonas Lahtinen Cc: Daniel Vetter Link: http://patchwork.freedesktop.org/patch/msgid/20170622160211.783-1-chris@chris-wilson.co.uk Reviewed-by: Tvrtko Ursulin Cc: stable@vger.kernel.org (cherry picked from commit 15727ed0d944ce1dec8b9e1082dd3df29a0fdf44) Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/intel_fbdev.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'drivers') diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c index 03347c6ae599..0c4cde6b2e6f 100644 --- a/drivers/gpu/drm/i915/intel_fbdev.c +++ b/drivers/gpu/drm/i915/intel_fbdev.c @@ -535,13 +535,14 @@ static void intel_fbdev_destroy(struct intel_fbdev *ifbdev) drm_fb_helper_fini(&ifbdev->helper); - if (ifbdev->fb) { + if (ifbdev->vma) { mutex_lock(&ifbdev->helper.dev->struct_mutex); intel_unpin_fb_vma(ifbdev->vma); mutex_unlock(&ifbdev->helper.dev->struct_mutex); + } + if (ifbdev->fb) drm_framebuffer_remove(&ifbdev->fb->base); - } kfree(ifbdev); } @@ -765,7 +766,7 @@ void intel_fbdev_set_suspend(struct drm_device *dev, int state, bool synchronous struct intel_fbdev *ifbdev = dev_priv->fbdev; struct fb_info *info; - if (!ifbdev || !ifbdev->fb) + if (!ifbdev || !ifbdev->vma) return; info = ifbdev->helper.fbdev; @@ -812,7 +813,7 @@ void intel_fbdev_output_poll_changed(struct drm_device *dev) { struct intel_fbdev *ifbdev = to_i915(dev)->fbdev; - if (ifbdev && ifbdev->fb) + if (ifbdev && ifbdev->vma) drm_fb_helper_hotplug_event(&ifbdev->helper); } @@ -824,7 +825,7 @@ void intel_fbdev_restore_mode(struct drm_device *dev) return; intel_fbdev_sync(ifbdev); - if (!ifbdev->fb) + if (!ifbdev->vma) return; if (drm_fb_helper_restore_fbdev_mode_unlocked(&ifbdev->helper) == 0) -- cgit v1.2.3 From 0de98709896d9c02ce3121ec3afb524253a5853f Mon Sep 17 00:00:00 2001 From: "Zhou, Wenjia" Date: Tue, 4 Jul 2017 15:47:00 +0800 Subject: drm/i915/gvt: Fix a memory leak in intel_gvt_init_gtt() It will causes memory leak, if the function setup_spt_oos() fail, in the function intel_gvt_init_gtt(), which allocated by get_zeroed_page() and mapped by dma_map_page(). Unmap and free the page, after STP oos initialize fail, it will fix this issue. Signed-off-by: Zhou, Wenjia Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/gtt.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers') diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c index 66374dba3b1a..6166e34d892b 100644 --- a/drivers/gpu/drm/i915/gvt/gtt.c +++ b/drivers/gpu/drm/i915/gvt/gtt.c @@ -2259,6 +2259,8 @@ int intel_gvt_init_gtt(struct intel_gvt *gvt) ret = setup_spt_oos(gvt); if (ret) { gvt_err("fail to initialize SPT oos\n"); + dma_unmap_page(dev, daddr, 4096, PCI_DMA_BIDIRECTIONAL); + __free_page(gvt->gtt.scratch_ggtt_page); return ret; } } -- cgit v1.2.3 From 3364bf5fd00f0391ad090f547932a5c4b2068dbc Mon Sep 17 00:00:00 2001 From: Ping Gao Date: Tue, 4 Jul 2017 16:09:58 +0800 Subject: drm/i915/gvt: Audit the command buffer address The command buffer address in context like ring buffer base address and wa_ctx address need to be audit to make sure they are in the valid GGTT range. Signed-off-by: Ping Gao Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/cmd_parser.c | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'drivers') diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c b/drivers/gpu/drm/i915/gvt/cmd_parser.c index 51241de5e7a7..713848c36349 100644 --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c @@ -2536,6 +2536,11 @@ static int scan_workload(struct intel_vgpu_workload *workload) gma_head == gma_tail) return 0; + if (!intel_gvt_ggtt_validate_range(s.vgpu, s.ring_start, s.ring_size)) { + ret = -EINVAL; + goto out; + } + ret = ip_gma_set(&s, gma_head); if (ret) goto out; @@ -2579,6 +2584,11 @@ static int scan_wa_ctx(struct intel_shadow_wa_ctx *wa_ctx) s.rb_va = wa_ctx->indirect_ctx.shadow_va; s.workload = workload; + if (!intel_gvt_ggtt_validate_range(s.vgpu, s.ring_start, s.ring_size)) { + ret = -EINVAL; + goto out; + } + ret = ip_gma_set(&s, gma_head); if (ret) goto out; -- cgit v1.2.3 From 08673c3e27aa4407899e4fbb4738dac25370f706 Mon Sep 17 00:00:00 2001 From: Chuanxiao Dong Date: Fri, 7 Jul 2017 13:21:52 +0800 Subject: drm/i915/gvt: Revert "drm/i915/gvt: Fix possible recursive locking issue" This reverts commit 62d02fd1f807bf5a259a242c483c9fb98a242630. The rwsem recursive trace should not be fixed from kvmgt side by using a workqueue and it is an issue should be fixed in VFIO. So this one should be reverted. Signed-off-by: Chuanxiao Dong Cc: Zhenyu Wang Cc: stable@vger.kernel.org # v4.10+ Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/gvt.h | 3 --- drivers/gpu/drm/i915/gvt/kvmgt.c | 55 ++++++++-------------------------------- 2 files changed, 10 insertions(+), 48 deletions(-) (limited to 'drivers') diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h index 28d817e96e58..3a74e79eac2f 100644 --- a/drivers/gpu/drm/i915/gvt/gvt.h +++ b/drivers/gpu/drm/i915/gvt/gvt.h @@ -182,9 +182,6 @@ struct intel_vgpu { struct kvm *kvm; struct work_struct release_work; atomic_t released; - struct work_struct unpin_work; - spinlock_t unpin_lock; /* To protect unpin_list */ - struct list_head unpin_list; } vdev; #endif }; diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c index 75a6e1d8af0d..fd0c85f9ef3c 100644 --- a/drivers/gpu/drm/i915/gvt/kvmgt.c +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c @@ -78,7 +78,6 @@ struct gvt_dma { struct rb_node node; gfn_t gfn; unsigned long iova; - struct list_head list; }; static inline bool handle_valid(unsigned long handle) @@ -167,7 +166,6 @@ static void gvt_cache_add(struct intel_vgpu *vgpu, gfn_t gfn, new->gfn = gfn; new->iova = iova; - INIT_LIST_HEAD(&new->list); mutex_lock(&vgpu->vdev.cache_lock); while (*link) { @@ -199,52 +197,26 @@ static void __gvt_cache_remove_entry(struct intel_vgpu *vgpu, kfree(entry); } -static void intel_vgpu_unpin_work(struct work_struct *work) +static void gvt_cache_remove(struct intel_vgpu *vgpu, gfn_t gfn) { - struct intel_vgpu *vgpu = container_of(work, struct intel_vgpu, - vdev.unpin_work); struct device *dev = mdev_dev(vgpu->vdev.mdev); struct gvt_dma *this; - unsigned long gfn; - - for (;;) { - spin_lock(&vgpu->vdev.unpin_lock); - if (list_empty(&vgpu->vdev.unpin_list)) { - spin_unlock(&vgpu->vdev.unpin_lock); - break; - } - this = list_first_entry(&vgpu->vdev.unpin_list, - struct gvt_dma, list); - list_del(&this->list); - spin_unlock(&vgpu->vdev.unpin_lock); - - gfn = this->gfn; - vfio_unpin_pages(dev, &gfn, 1); - kfree(this); - } -} - -static bool gvt_cache_mark_remove(struct intel_vgpu *vgpu, gfn_t gfn) -{ - struct gvt_dma *this; + unsigned long g1; + int rc; mutex_lock(&vgpu->vdev.cache_lock); this = __gvt_cache_find(vgpu, gfn); if (!this) { mutex_unlock(&vgpu->vdev.cache_lock); - return false; + return; } + + g1 = gfn; gvt_dma_unmap_iova(vgpu, this->iova); - /* remove this from rb tree */ - rb_erase(&this->node, &vgpu->vdev.cache); + rc = vfio_unpin_pages(dev, &g1, 1); + WARN_ON(rc != 1); + __gvt_cache_remove_entry(vgpu, this); mutex_unlock(&vgpu->vdev.cache_lock); - - /* put this to the unpin_list */ - spin_lock(&vgpu->vdev.unpin_lock); - list_move_tail(&this->list, &vgpu->vdev.unpin_list); - spin_unlock(&vgpu->vdev.unpin_lock); - - return true; } static void gvt_cache_init(struct intel_vgpu *vgpu) @@ -485,9 +457,6 @@ static int intel_vgpu_create(struct kobject *kobj, struct mdev_device *mdev) } INIT_WORK(&vgpu->vdev.release_work, intel_vgpu_release_work); - INIT_WORK(&vgpu->vdev.unpin_work, intel_vgpu_unpin_work); - spin_lock_init(&vgpu->vdev.unpin_lock); - INIT_LIST_HEAD(&vgpu->vdev.unpin_list); vgpu->vdev.mdev = mdev; mdev_set_drvdata(mdev, vgpu); @@ -517,7 +486,6 @@ static int intel_vgpu_iommu_notifier(struct notifier_block *nb, struct intel_vgpu *vgpu = container_of(nb, struct intel_vgpu, vdev.iommu_notifier); - bool sched_unmap = false; if (action == VFIO_IOMMU_NOTIFY_DMA_UNMAP) { struct vfio_iommu_type1_dma_unmap *unmap = data; @@ -527,10 +495,7 @@ static int intel_vgpu_iommu_notifier(struct notifier_block *nb, end_gfn = gfn + unmap->size / PAGE_SIZE; while (gfn < end_gfn) - sched_unmap |= gvt_cache_mark_remove(vgpu, gfn++); - - if (sched_unmap) - schedule_work(&vgpu->vdev.unpin_work); + gvt_cache_remove(vgpu, gfn++); } return NOTIFY_OK; -- cgit v1.2.3 From 4cc74389a551dc95fce72d58c11e55a93b6ecd19 Mon Sep 17 00:00:00 2001 From: Weinan Li Date: Mon, 19 Jun 2017 08:49:17 +0800 Subject: drm/i915/gvt: remove scheduler_mutex in per-engine workload_thread For the vGPU workloads, now GVT-g use per vGPU scheduler, the per-ring work_thread only pick workload belongs to the current vGPU. And with time slice based scheduler, it waits all the engines become idle before do vGPU switch. So we can run free dispatch in per-ring work_thread, different ring running in different 'vGPU' won't happen. For the workloads between vGPU and Host, this scheduler_mutex can't block host to dispatch workload into other ring engines. Here remove this mutex since it impacts the performance when applications use more than 1 ring engines in 1 vgpu. ring0 running in vGPU1, ring1 running in Host. Will happen. ring0 running in vGPU1, ring1 running in vGPU2. Won't happen. Signed-off-by: Weinan Li Signed-off-by: Ping Gao Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/scheduler.c | 7 ------- 1 file changed, 7 deletions(-) (limited to 'drivers') diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c index 488fdea348a9..5aeba13a5de4 100644 --- a/drivers/gpu/drm/i915/gvt/scheduler.c +++ b/drivers/gpu/drm/i915/gvt/scheduler.c @@ -464,8 +464,6 @@ struct workload_thread_param { int ring_id; }; -static DEFINE_MUTEX(scheduler_mutex); - static int workload_thread(void *priv) { struct workload_thread_param *p = (struct workload_thread_param *)priv; @@ -497,8 +495,6 @@ static int workload_thread(void *priv) if (!workload) break; - mutex_lock(&scheduler_mutex); - gvt_dbg_sched("ring id %d next workload %p vgpu %d\n", workload->ring_id, workload, workload->vgpu->id); @@ -537,9 +533,6 @@ complete: FORCEWAKE_ALL); intel_runtime_pm_put(gvt->dev_priv); - - mutex_unlock(&scheduler_mutex); - } return 0; } -- cgit v1.2.3 From 0cf5ec41839d82ee7f8fbb47f137b7afc562b9f1 Mon Sep 17 00:00:00 2001 From: Chuanxiao Dong Date: Fri, 23 Jun 2017 13:01:11 +0800 Subject: drm/i915/gvt: Use fence error from GVT request for workload status The req->fence.error will be set if this request caused GPU hang so we can use this value to workload->status to indicate whether this GVT request caused any problem. If it caused GPU hang, we shouldn't trigger any context switch back to the guest. v2: - only take -EIO from fence->error. (Zhenyu) Fixes: 8f1117abb408 (drm/i915/gvt: handle workload lifecycle properly) Signed-off-by: Chuanxiao Dong Cc: Zhenyu Wang Signed-off-by: Zhenyu Wang --- drivers/gpu/drm/i915/gvt/scheduler.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) (limited to 'drivers') diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c index 5aeba13a5de4..4f7057d62d88 100644 --- a/drivers/gpu/drm/i915/gvt/scheduler.c +++ b/drivers/gpu/drm/i915/gvt/scheduler.c @@ -174,15 +174,6 @@ static int shadow_context_status_change(struct notifier_block *nb, atomic_set(&workload->shadow_ctx_active, 1); break; case INTEL_CONTEXT_SCHEDULE_OUT: - /* If the status is -EINPROGRESS means this workload - * doesn't meet any issue during dispatching so when - * get the SCHEDULE_OUT set the status to be zero for - * good. If the status is NOT -EINPROGRESS means there - * is something wrong happened during dispatching and - * the status should not be set to zero - */ - if (workload->status == -EINPROGRESS) - workload->status = 0; atomic_set(&workload->shadow_ctx_active, 0); break; default: @@ -427,6 +418,18 @@ static void complete_current_workload(struct intel_gvt *gvt, int ring_id) wait_event(workload->shadow_ctx_status_wq, !atomic_read(&workload->shadow_ctx_active)); + /* If this request caused GPU hang, req->fence.error will + * be set to -EIO. Use -EIO to set workload status so + * that when this request caused GPU hang, didn't trigger + * context switch interrupt to guest. + */ + if (likely(workload->status == -EINPROGRESS)) { + if (workload->req->fence.error == -EIO) + workload->status = -EIO; + else + workload->status = 0; + } + i915_gem_request_put(fetch_and_zero(&workload->req)); if (!workload->status && !vgpu->resetting) { -- cgit v1.2.3 From 50740024bc393b608f7e391ac35e70f33938dd24 Mon Sep 17 00:00:00 2001 From: Maarten Lankhorst Date: Mon, 26 Jun 2017 10:33:49 +0200 Subject: drm/i915: Make DP-MST connector info work Commit 9a148a96fc3a ("drm/i915/debugfs: add dp mst info") adds support for DP-MST to intel_connector_info, but forgot to remove the early return for DP-MST. Remove it, and print out MST connectors directly. Fixes: 9a148a96fc3a ("drm/i915/debugfs: add dp mst info") Cc: # v4.11+ Cc: Dhinakaran Pandiyan Cc: Libin Yang Signed-off-by: Maarten Lankhorst Link: http://patchwork.freedesktop.org/patch/msgid/20170626083349.24389-1-maarten.lankhorst@linux.intel.com Reviewed-by: Dhinakaran Pandiyan (cherry picked from commit 77d1f615c78a73a04254fa2bff07ee9fa27145d9) Signed-off-by: Jani Nikula --- drivers/gpu/drm/i915/i915_debugfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 4577b0af6886..23cd865a3da6 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -3083,7 +3083,7 @@ static void intel_connector_info(struct seq_file *m, connector->display_info.cea_rev); } - if (!intel_encoder || intel_encoder->type == INTEL_OUTPUT_DP_MST) + if (!intel_encoder) return; switch (connector->connector_type) { -- cgit v1.2.3