From d270ae34eb77c58dea60e5b1e300a698d2ce39ac Mon Sep 17 00:00:00 2001 From: Jesse Barnes Date: Mon, 27 Sep 2010 10:35:44 -0700 Subject: [PATCH 1/7] drm/i915: fix GMCH power reporting The IPS driver needs to know the current power consumption of the GMCH in order to make decisions about when to increase or decrease the CPU and/or GPU power envelope. So fix up the divisions to save the results so the numbers are actually correct (contrary to some earlier comments and code, these functions do not modify the first argument and use it for the result). Signed-off-by: Jesse Barnes Signed-off-by: Chris Wilson Cc: stable@kernel.org --- drivers/gpu/drm/i915/i915_dma.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 9d67b4853030..c74e4e8006d4 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1787,9 +1787,9 @@ unsigned long i915_chipset_val(struct drm_i915_private *dev_priv) } } - div_u64(diff, diff1); + diff = div_u64(diff, diff1); ret = ((m * diff) + c); - div_u64(ret, 10); + ret = div_u64(ret, 10); dev_priv->last_count1 = total_count; dev_priv->last_time1 = now; @@ -1858,7 +1858,7 @@ void i915_update_gfx_val(struct drm_i915_private *dev_priv) /* More magic constants... */ diff = diff * 1181; - div_u64(diff, diffms * 10); + diff = div_u64(diff, diffms * 10); dev_priv->gfx_power = diff; } From e39a01501b228e1be2037d5bddccae2a820af902 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 29 Sep 2010 22:23:05 +0100 Subject: [PATCH 2/7] drm/i915: Fix refleak during eviction. Now that we hold onto a reference whilst evicting objects, we need to be sure that we drop all the references taken -- even on the error paths. Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_gem_evict.c | 49 ++++++++++++--------------- 1 file changed, 22 insertions(+), 27 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c index e85246ef691c..5c428fa3e0b3 100644 --- a/drivers/gpu/drm/i915/i915_gem_evict.c +++ b/drivers/gpu/drm/i915/i915_gem_evict.c @@ -93,7 +93,7 @@ i915_gem_evict_something(struct drm_device *dev, int min_size, unsigned alignmen { drm_i915_private_t *dev_priv = dev->dev_private; struct list_head eviction_list, unwind_list; - struct drm_i915_gem_object *obj_priv, *tmp_obj_priv; + struct drm_i915_gem_object *obj_priv; struct list_head *render_iter, *bsd_iter; int ret = 0; @@ -175,39 +175,34 @@ i915_gem_evict_something(struct drm_device *dev, int min_size, unsigned alignmen return -ENOSPC; found: + /* drm_mm doesn't allow any other other operations while + * scanning, therefore store to be evicted objects on a + * temporary list. */ INIT_LIST_HEAD(&eviction_list); - list_for_each_entry_safe(obj_priv, tmp_obj_priv, - &unwind_list, evict_list) { + while (!list_empty(&unwind_list)) { + obj_priv = list_first_entry(&unwind_list, + struct drm_i915_gem_object, + evict_list); if (drm_mm_scan_remove_block(obj_priv->gtt_space)) { - /* drm_mm doesn't allow any other other operations while - * scanning, therefore store to be evicted objects on a - * temporary list. */ list_move(&obj_priv->evict_list, &eviction_list); - } else - drm_gem_object_unreference(&obj_priv->base); - } - - /* Unbinding will emit any required flushes */ - list_for_each_entry_safe(obj_priv, tmp_obj_priv, - &eviction_list, evict_list) { -#if WATCH_LRU - DRM_INFO("%s: evicting %p\n", __func__, &obj_priv->base); -#endif - ret = i915_gem_object_unbind(&obj_priv->base); - if (ret) - return ret; - + continue; + } + list_del(&obj_priv->evict_list); drm_gem_object_unreference(&obj_priv->base); } - /* The just created free hole should be on the top of the free stack - * maintained by drm_mm, so this BUG_ON actually executes in O(1). - * Furthermore all accessed data has just recently been used, so it - * should be really fast, too. */ - BUG_ON(!drm_mm_search_free(&dev_priv->mm.gtt_space, min_size, - alignment, 0)); + /* Unbinding will emit any required flushes */ + while (!list_empty(&eviction_list)) { + obj_priv = list_first_entry(&eviction_list, + struct drm_i915_gem_object, + evict_list); + if (ret == 0) + ret = i915_gem_object_unbind(&obj_priv->base); + list_del(&obj_priv->evict_list); + drm_gem_object_unreference(&obj_priv->base); + } - return 0; + return ret; } int From 929f49bf225b1b6cd04d0a7b9c0f7377d9131220 Mon Sep 17 00:00:00 2001 From: Julia Lawall Date: Sat, 2 Oct 2010 15:59:17 +0200 Subject: [PATCH 3/7] drivers/gpu/drm/i915/i915_gem.c: Add missing error handling code Extend the error handling code with operations found in other nearby error handling code A simplified version of the sematic match that finds this problem is as follows: (http://coccinelle.lip6.fr/) // @r exists@ @r@ statement S1,S2,S3; constant C1,C2,C3; @@ *if (...) {... S1 return -C1;} ... *if (...) {... when != S1 return -C2;} ... *if (...) {... S1 return -C3;} // Signed-off-by: Julia Lawall Signed-off-by: Chris Wilson Cc: stable@kernel.org --- drivers/gpu/drm/i915/i915_gem.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index bced9b25c71e..cfe597865f5d 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3258,6 +3258,8 @@ i915_gem_object_pin_and_relocate(struct drm_gem_object *obj, (int) reloc->offset, reloc->read_domains, reloc->write_domain); + drm_gem_object_unreference(target_obj); + i915_gem_object_unpin(obj); return -EINVAL; } if (reloc->write_domain & I915_GEM_DOMAIN_CPU || From b99a9d9bb62a984bdfcb6c973dfe180bd776abbe Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Sun, 3 Oct 2010 00:33:05 -0700 Subject: [PATCH 4/7] drm/i915: vblank status not valid while training display port While the display port is in training mode, vblank interrupts don't occur. Because we have to wait for the display port output to turn on before starting the training sequence, enable the output in 'normal' mode so that we can tell when a vblank has occurred, then start the training sequence. Signed-off-by: Keith Packard Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/intel_dp.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 1a51ee07de3e..9ab8708ac6ba 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1138,18 +1138,14 @@ static bool intel_dp_set_link_train(struct intel_dp *intel_dp, uint32_t dp_reg_value, uint8_t dp_train_pat, - uint8_t train_set[4], - bool first) + uint8_t train_set[4]) { struct drm_device *dev = intel_dp->base.enc.dev; struct drm_i915_private *dev_priv = dev->dev_private; - struct intel_crtc *intel_crtc = to_intel_crtc(intel_dp->base.enc.crtc); int ret; I915_WRITE(intel_dp->output_reg, dp_reg_value); POSTING_READ(intel_dp->output_reg); - if (first) - intel_wait_for_vblank(dev, intel_crtc->pipe); intel_dp_aux_native_write_1(intel_dp, DP_TRAINING_PATTERN_SET, @@ -1174,10 +1170,15 @@ intel_dp_link_train(struct intel_dp *intel_dp) uint8_t voltage; bool clock_recovery = false; bool channel_eq = false; - bool first = true; int tries; u32 reg; uint32_t DP = intel_dp->DP; + struct intel_crtc *intel_crtc = to_intel_crtc(intel_dp->base.enc.crtc); + + /* Enable output, wait for it to become active */ + I915_WRITE(intel_dp->output_reg, intel_dp->DP); + POSTING_READ(intel_dp->output_reg); + intel_wait_for_vblank(dev, intel_crtc->pipe); /* Write the link configuration data */ intel_dp_aux_native_write(intel_dp, DP_LINK_BW_SET, @@ -1210,9 +1211,8 @@ intel_dp_link_train(struct intel_dp *intel_dp) reg = DP | DP_LINK_TRAIN_PAT_1; if (!intel_dp_set_link_train(intel_dp, reg, - DP_TRAINING_PATTERN_1, train_set, first)) + DP_TRAINING_PATTERN_1, train_set)) break; - first = false; /* Set training pattern 1 */ udelay(100); @@ -1266,8 +1266,7 @@ intel_dp_link_train(struct intel_dp *intel_dp) /* channel eq pattern */ if (!intel_dp_set_link_train(intel_dp, reg, - DP_TRAINING_PATTERN_2, train_set, - false)) + DP_TRAINING_PATTERN_2, train_set)) break; udelay(400); From ab7ad7f6451580aa7eccc0ba62807c872088a8f9 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Sun, 3 Oct 2010 00:33:06 -0700 Subject: [PATCH 5/7] drm/i915: Use pipe state to tell when pipe is off Instead of waiting for the display line value to settle, we can simply wait for the pipe configuration register 'state' bit to turn off. Contrarywise, disabling the plane will not cause the display line value to stop changing, so instead we wait for the vblank interrupt bit to get set. And, we only do this when we're not about to wait for the pipe to turn off. Signed-off-by: Keith Packard Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/intel_display.c | 58 +++++++++++++++++----------- drivers/gpu/drm/i915/intel_drv.h | 1 - 2 files changed, 36 insertions(+), 23 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index b5bf51a4502d..979228594599 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1013,8 +1013,8 @@ void intel_wait_for_vblank(struct drm_device *dev, int pipe) DRM_DEBUG_KMS("vblank wait timed out\n"); } -/** - * intel_wait_for_vblank_off - wait for vblank after disabling a pipe +/* + * intel_wait_for_pipe_off - wait for pipe to turn off * @dev: drm device * @pipe: pipe to wait for * @@ -1022,25 +1022,39 @@ void intel_wait_for_vblank(struct drm_device *dev, int pipe) * spinning on the vblank interrupt status bit, since we won't actually * see an interrupt when the pipe is disabled. * - * So this function waits for the display line value to settle (it - * usually ends up stopping at the start of the next frame). + * On Gen4 and above: + * wait for the pipe register state bit to turn off + * + * Otherwise: + * wait for the display line value to settle (it usually + * ends up stopping at the start of the next frame). + * */ -void intel_wait_for_vblank_off(struct drm_device *dev, int pipe) +static void intel_wait_for_pipe_off(struct drm_device *dev, int pipe) { struct drm_i915_private *dev_priv = dev->dev_private; - int pipedsl_reg = (pipe == 0 ? PIPEADSL : PIPEBDSL); - unsigned long timeout = jiffies + msecs_to_jiffies(100); - u32 last_line; - /* Wait for the display line to settle */ - do { - last_line = I915_READ(pipedsl_reg) & DSL_LINEMASK; - mdelay(5); - } while (((I915_READ(pipedsl_reg) & DSL_LINEMASK) != last_line) && - time_after(timeout, jiffies)); + if (INTEL_INFO(dev)->gen >= 4) { + int pipeconf_reg = (pipe == 0 ? PIPEACONF : PIPEBCONF); - if (time_after(jiffies, timeout)) - DRM_DEBUG_KMS("vblank wait timed out\n"); + /* Wait for the Pipe State to go off */ + if (wait_for((I915_READ(pipeconf_reg) & I965_PIPECONF_ACTIVE) == 0, + 100, 0)) + DRM_DEBUG_KMS("pipe_off wait timed out\n"); + } else { + u32 last_line; + int pipedsl_reg = (pipe == 0 ? PIPEADSL : PIPEBDSL); + unsigned long timeout = jiffies + msecs_to_jiffies(100); + + /* Wait for the display line to settle */ + do { + last_line = I915_READ(pipedsl_reg) & DSL_LINEMASK; + mdelay(5); + } while (((I915_READ(pipedsl_reg) & DSL_LINEMASK) != last_line) && + time_after(timeout, jiffies)); + if (time_after(jiffies, timeout)) + DRM_DEBUG_KMS("pipe_off wait timed out\n"); + } } /* Parameters have changed, update FBC info */ @@ -2328,13 +2342,13 @@ static void i9xx_crtc_dpms(struct drm_crtc *crtc, int mode) I915_READ(dspbase_reg); } - /* Wait for vblank for the disable to take effect */ - intel_wait_for_vblank_off(dev, pipe); - /* Don't disable pipe A or pipe A PLLs if needed */ if (pipeconf_reg == PIPEACONF && - (dev_priv->quirks & QUIRK_PIPEA_FORCE)) + (dev_priv->quirks & QUIRK_PIPEA_FORCE)) { + /* Wait for vblank for the disable to take effect */ + intel_wait_for_vblank(dev, pipe); goto skip_pipe_off; + } /* Next, disable display pipes */ temp = I915_READ(pipeconf_reg); @@ -2343,8 +2357,8 @@ static void i9xx_crtc_dpms(struct drm_crtc *crtc, int mode) I915_READ(pipeconf_reg); } - /* Wait for vblank for the disable to take effect. */ - intel_wait_for_vblank_off(dev, pipe); + /* Wait for the pipe to turn off */ + intel_wait_for_pipe_off(dev, pipe); temp = I915_READ(dpll_reg); if ((temp & DPLL_VCO_ENABLE) != 0) { diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index ad312ca6b3e5..8828b3ac6414 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -229,7 +229,6 @@ extern struct drm_display_mode *intel_crtc_mode_get(struct drm_device *dev, struct drm_crtc *crtc); int intel_get_pipe_from_crtc_id(struct drm_device *dev, void *data, struct drm_file *file_priv); -extern void intel_wait_for_vblank_off(struct drm_device *dev, int pipe); extern void intel_wait_for_vblank(struct drm_device *dev, int pipe); extern struct drm_crtc *intel_get_crtc_from_pipe(struct drm_device *dev, int pipe); extern struct drm_crtc *intel_get_load_detect_pipe(struct intel_encoder *intel_encoder, From ce9d419dbecc292cc3e06e8b1d6d123d3fa813a4 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Sun, 26 Sep 2010 20:50:05 +0100 Subject: [PATCH 6/7] drm/i915: Sanity check pread/pwrite Move the access control up from the fast paths, which are no longer universally taken first, up into the caller. This then duplicates some sanity checking along the slow paths, but is much simpler. Tracked as CVE-2010-2962. Reported-by: Kees Cook Signed-off-by: Chris Wilson Cc: stable@kernel.org --- drivers/gpu/drm/i915/i915_gem.c | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index cfe597865f5d..7749e78a7300 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -477,8 +477,15 @@ i915_gem_pread_ioctl(struct drm_device *dev, void *data, */ if (args->offset > obj->size || args->size > obj->size || args->offset + args->size > obj->size) { - drm_gem_object_unreference_unlocked(obj); - return -EINVAL; + ret = -EINVAL; + goto err; + } + + if (!access_ok(VERIFY_WRITE, + (char __user *)(uintptr_t)args->data_ptr, + args->size)) { + ret = -EFAULT; + goto err; } if (i915_gem_object_needs_bit17_swizzle(obj)) { @@ -490,8 +497,8 @@ i915_gem_pread_ioctl(struct drm_device *dev, void *data, file_priv); } +err: drm_gem_object_unreference_unlocked(obj); - return ret; } @@ -580,8 +587,6 @@ i915_gem_gtt_pwrite_fast(struct drm_device *dev, struct drm_gem_object *obj, user_data = (char __user *) (uintptr_t) args->data_ptr; remain = args->size; - if (!access_ok(VERIFY_READ, user_data, remain)) - return -EFAULT; mutex_lock(&dev->struct_mutex); @@ -940,8 +945,15 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data, */ if (args->offset > obj->size || args->size > obj->size || args->offset + args->size > obj->size) { - drm_gem_object_unreference_unlocked(obj); - return -EINVAL; + ret = -EINVAL; + goto err; + } + + if (!access_ok(VERIFY_READ, + (char __user *)(uintptr_t)args->data_ptr, + args->size)) { + ret = -EFAULT; + goto err; } /* We can only do the GTT pwrite on untiled buffers, as otherwise @@ -975,8 +987,8 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data, DRM_INFO("pwrite failed %d\n", ret); #endif +err: drm_gem_object_unreference_unlocked(obj); - return ret; } From 7dcd2499deab8f10011713c40bc2f309c9b65077 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Sun, 26 Sep 2010 20:21:44 +0100 Subject: [PATCH 7/7] drm/i915: Rephrase pwrite bounds checking to avoid any potential overflow ... and do the same for pread. Signed-off-by: Chris Wilson Cc: stable@kernel.org --- drivers/gpu/drm/i915/i915_gem.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 7749e78a7300..aa959c4a1836 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -471,12 +471,8 @@ i915_gem_pread_ioctl(struct drm_device *dev, void *data, return -ENOENT; obj_priv = to_intel_bo(obj); - /* Bounds check source. - * - * XXX: This could use review for overflow issues... - */ - if (args->offset > obj->size || args->size > obj->size || - args->offset + args->size > obj->size) { + /* Bounds check source. */ + if (args->offset > obj->size || args->size > obj->size - args->offset) { ret = -EINVAL; goto err; } @@ -939,12 +935,8 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data, return -ENOENT; obj_priv = to_intel_bo(obj); - /* Bounds check destination. - * - * XXX: This could use review for overflow issues... - */ - if (args->offset > obj->size || args->size > obj->size || - args->offset + args->size > obj->size) { + /* Bounds check destination. */ + if (args->offset > obj->size || args->size > obj->size - args->offset) { ret = -EINVAL; goto err; }