Re: [Intel-gfx] [PATCH] drm/i915: Sanity check incoming ioctl data for a NULL pointer

[Ben Widawsky]

On Thu, Mar 14, 2013 at 12:59:57PM +, Chris Wilson wrote:
> In order to prevent a potential NULL deference with hostile userspace,
> we need to check whether the ioctl was passed an invalid args pointer.
> 
> Reported-by: Tommi Rantala 
> Link: 
> http://lkml.kernel.org/r/ca+ydwtpubvbwxbt-tdgpuvj1eu7itmcho_2b3w13hkd5+jw...@mail.gmail.com
> Signed-off-by: Chris Wilson 
> ---
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |   11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
> b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 365e41a..9f5602e 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1103,7 +1103,11 @@ i915_gem_execbuffer(struct drm_device *dev, void *data,
>   struct drm_i915_gem_exec_object2 *exec2_list = NULL;
>   int ret, i;
>  
> - if (args->buffer_count < 1) {
> + if (args == NULL)
> + return -EINVAL;
> +
> + if (args->buffer_count < 1 ||
> + args->buffer_count > INT_MAX / sizeof(*exec2_list)) {
>   DRM_DEBUG("execbuf with %d buffers\n", args->buffer_count);
>   return -EINVAL;
>   }
> @@ -1182,8 +1186,11 @@ i915_gem_execbuffer2(struct drm_device *dev, void 
> *data,
>   struct drm_i915_gem_exec_object2 *exec2_list = NULL;
>   int ret;
>  
> + if (args == NULL)
> + return -EINVAL;
> +
>   if (args->buffer_count < 1 ||
> - args->buffer_count > UINT_MAX / sizeof(*exec2_list)) {
> + args->buffer_count > INT_MAX / sizeof(*exec2_list)) {
>   DRM_DEBUG("execbuf2 with %d buffers\n", args->buffer_count);
>   return -EINVAL;
>   }

Why did you change UINT_MAX to INT_MAX? TBH, I'm confused what we're
trying to achieve, and why we need anything other than:
if (!args->buffer_count)

I'm also not seeing how the NULL checks are needed since at least it
seems to be for execbuffer (IOW) we could never have NULL args.

if (cmd & (IOC_IN | IOC_OUT)) {
if (asize <= sizeof(stack_kdata)) {
kdata = stack_kdata;
} else {
kdata = kmalloc(asize, GFP_KERNEL);
if (!kdata) {
retcode = -ENOMEM;
goto err_i1;
}
}
if (asize > usize)
memset(kdata + usize, 0, asize - usize);
}

Sorry if these are stupid questions.

-- 
Ben Widawsky, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915: Sanity check incoming ioctl data for a NULL pointer

[Ben Widawsky]

On Thu, Mar 14, 2013 at 12:59:57PM +, Chris Wilson wrote:
> In order to prevent a potential NULL deference with hostile userspace,
> we need to check whether the ioctl was passed an invalid args pointer.
> 
> Reported-by: Tommi Rantala 
> Link: 
> http://lkml.kernel.org/r/ca+ydwtpubvbwxbt-tdgpuvj1eu7itmcho_2b3w13hkd5+jw...@mail.gmail.com
> Signed-off-by: Chris Wilson 
> ---
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |   11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
> b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index 365e41a..9f5602e 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1103,7 +1103,11 @@ i915_gem_execbuffer(struct drm_device *dev, void *data,
>   struct drm_i915_gem_exec_object2 *exec2_list = NULL;
>   int ret, i;
>  
> - if (args->buffer_count < 1) {
> + if (args == NULL)
> + return -EINVAL;
> +
> + if (args->buffer_count < 1 ||
> + args->buffer_count > INT_MAX / sizeof(*exec2_list)) {
>   DRM_DEBUG("execbuf with %d buffers\n", args->buffer_count);
>   return -EINVAL;
>   }
> @@ -1182,8 +1186,11 @@ i915_gem_execbuffer2(struct drm_device *dev, void 
> *data,
>   struct drm_i915_gem_exec_object2 *exec2_list = NULL;
>   int ret;
>  
> + if (args == NULL)
> + return -EINVAL;
> +
>   if (args->buffer_count < 1 ||
> - args->buffer_count > UINT_MAX / sizeof(*exec2_list)) {
> + args->buffer_count > INT_MAX / sizeof(*exec2_list)) {
>   DRM_DEBUG("execbuf2 with %d buffers\n", args->buffer_count);
>   return -EINVAL;
>   }

Why did you change UINT_MAX to INT_MAX? TBH, I'm confused what we're
trying to achieve, and why we need anything other than:
if (!args->buffer_count)

I'm also not seeing how the NULL checks are needed since at least it
seems to be for execbuffer (IOW) we could never have NULL args.

if (cmd & (IOC_IN | IOC_OUT)) {
if (asize <= sizeof(stack_kdata)) {
kdata = stack_kdata;
} else {
kdata = kmalloc(asize, GFP_KERNEL);
if (!kdata) {
retcode = -ENOMEM;
goto err_i1;
}
}
if (asize > usize)
memset(kdata + usize, 0, asize - usize);
}

Sorry if these are stupid questions.

-- 
Ben Widawsky, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 09/10] drm/i915: Introduce IVB_FEATURES for device definition

[Ben Widawsky]

On Wed, 13 Mar 2013 14:08:28 -0700
Ben Widawsky  wrote:

> Recommended by Chris.
> 
> Cc: Chris Wilson 
> Signed-off-by: Ben Widawsky 
> ---
>  drivers/gpu/drm/i915/i915_drv.c | 24 +++-
>  1 file changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c
> b/drivers/gpu/drm/i915/i915_drv.c index a63abd7..2ee89c2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -248,23 +248,21 @@ static const struct intel_device_info
> intel_sandybridge_m_info = { .has_force_wake = 1,
>  };
>  
> +#define IVB_FEATURES  \
> + .is_ivybridge = 1, .gen = 7, .num_pipes = 3, \
> + .need_gfx_hws = 1, .has_hotplug = 1, \
> + .has_bsd_ring = 1, \
> + .has_blt_ring = 1, \
> + .has_llc = 1, \
> + .has_force_wake = 1
> +
>  static const struct intel_device_info intel_ivybridge_d_info = {
> - .is_ivybridge = 1, .gen = 7, .num_pipes = 3,
> - .need_gfx_hws = 1, .has_hotplug = 1,
> - .has_bsd_ring = 1,
> - .has_blt_ring = 1,
> - .has_llc = 1,
> - .has_force_wake = 1,
> + IVB_FEATURES,
>  };
>  
>  static const struct intel_device_info intel_ivybridge_m_info = {
> - .is_ivybridge = 1, .gen = 7, .is_mobile = 1, .num_pipes = 3,
> - .need_gfx_hws = 1, .has_hotplug = 1,
> - .has_fbc = 0,   /* FBC is not enabled on Ivybridge
> mobile yet */
> - .has_bsd_ring = 1,
> - .has_blt_ring = 1,
> - .has_llc = 1,
> - .has_force_wake = 1,
> + IVB_FEATURES,
> + .is_mobile = 1,
>  };
>  
>  static const struct intel_device_info intel_valleyview_m_info = {

I noticed I can use this for VLV, and HSW as well. I've modified it
locally.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [QA 03/15] Testing report for `drm-intel-testing` (was: Updated -next)

[Sun, Yi]

Summary

We finished a new round of kernel testing. Generally, in this circle, 4 new 
bugs are filed, 21 bugs are still opened, 3 bugs are closed.

3 bugs out of 4 new ones are related to eDP display.



Test Environment

Kernel: (drm-intel-testing) d08a6eb2690b1ac6f0582feb41c2ccbea945285f

Author: Daniel Vetter mailto:daniel.vet...@ffwll.ch>>

Date:   Thu Mar 7 22:54:25 2013 +0100



Merge remote-tracking branch 'danvet/drm-intel-fixes' into drm-intel-testing



Hardware

We covered the platform: Haswell, IvyBridge, SandyBridge, IronLake, G45



Finding Bugs



New Bugs:

Bug 62275 - [HSW 3.8.2 
regression] After running testdisplay with only eDP, screen turn to black and 
can't light up again

Bug 61929 - [HSW eDP 
regression 3.8] eDP blank until xrandr is re-run

Bug 61542 - [HSW bisected] 
Desktop-EDP can not work

Bug 55121 - Limited color 
range on screen that is connected via HDMI [SandyBridge] 
(https://bugzilla.kernel.org/show_bug.cgi?id=55121)



Closed Bugs:

Bug 60140 - 
[SNB]igt/kms_flip/flip-vs-modeset-vs-hang causes system hung

Bug 47034 - [G45] mode set 
failure with testdisplay, low resolution modes fail and display doesn't come 
back

Bug 60307 - [SNB ILK 
regression] Call trace appears while doing fbset



Opened Bugs:

Bug 61041 - 
[Pineview]I-G-T/testdisplay && VGA 1600x1200 65Hz messing the screen

Bug 61158 - [ILK] System 
hang while running testdisplay to mode 1280x800

Bug 60002 - 
[PNV]I-G-T/kms_flip subtest：'blocking-absolute-wf_vblank' fail

Bug 6 - [IVB]kms_flip 
error: inter-vblank ts jitter

Bug 5 - 
[ILK,IVB]kms_flip error: inter-flip ts jitter

Bug 59834 - 
[ILK]I-G-T/kms_flip subtest: 'wf_vblank-vs-modeset' fail

Bug 59836 - 
[PNV]igt/kms_flip/absolute-wf_vblank fail

Bug 58897 - [HSW 
CRW]*ERROR* Unknown unclaimed register

Bug 59229 - I-G-T/ 
prime_self_import fail

Bug 59339 - [ILK] 
I-G-T/kms_flip subtest: 'flip-vs-panning' fail

Bug 36997 - [G45 SDVO] TV can't display -- SDVO xfer fails due to Pending.

Bug 42194 - [IVB/SNB] coldplug new monitors for fbcon on lastclose()

Bug 51975 - [IVB]can't find the HDMI audio device

Bug 54111 - [IVB]I-G-T/module_reload fail with *ERROR* “Memory manager not 
clean. Delaying takedown”

Bug 57441 - 
[HSW]I-G-T/sysfs_l3_parity fail

Bug 58497 - [IVB HSW] 
I-G-T/testdisplay, there's error in dmesg

Bug 58695 - I-G-T/prime_udl 
fail

Bug 58701 - [SNB 
DP]I-G-T/testdisplay && 1920x1080i showing not full

Bug 54687 - [ilk] pipe off 
timeout
Bug 52424 - [Bisected SNB 
Regression] glxgears causes GPU hung
Bug 45729 - [bisected regression] Incorrect Mode Timing on DP Display, with 
3.3-rc (due to interlaced CEA modes) (blocked by bug #61158)


Thanks
--Sun, Yi

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] i915 black screen introduced by ACPI changes

[Chris Li]

On Mon, Mar 11, 2013 at 6:16 AM, Jani Nikula
 wrote:
> Interesting snippets from your dmesgs:
>
> 1) good
>
> [0.00] Linux version 3.6.0-rc6+ (chr...@ideapad.lan) (gcc version 
> 4.7.2 20121109 (Red Hat 4.7.2-8) (GCC) ) #25 SMP Wed Feb 20 12:55:06 PST 2013
> ...
> [5.341431] [drm:intel_panel_get_max_backlight], max backlight PWM = 4648
> [5.341442] [drm:intel_panel_actually_set_backlight], set backlight PWM = 
> 4648
> [5.342572] [drm:intel_panel_get_max_backlight], max backlight PWM = 4648
> [5.342578] [drm:intel_panel_actually_set_backlight], set backlight PWM = 
> 4648
>
> 2) bad
>
> [0.00] Linux version 3.8.0-rc7+ (chr...@ideapad.lan) (gcc version 
> 4.7.2 20121109 (Red Hat 4.7.2-8) (GCC) ) #23 SMP Tue Feb 19 19:24:57 PST 2013
> ...
> [5.692853] [drm:asle_set_backlight], bclp = 0x80ff
> [5.692865] [drm:intel_panel_get_max_backlight], max backlight PWM = 4648
> [5.692870] [drm:intel_panel_actually_set_backlight], set backlight PWM = 
> 4648
> [5.693401] [drm:asle_set_backlight], bclp = 0x8000
> [5.693408] [drm:intel_panel_get_max_backlight], max backlight PWM = 4648
> [5.693413] [drm:intel_panel_actually_set_backlight], set backlight PWM = 0
>
> (We've added another debug print to asle_set_backlight.)
>
> For some reason we get two asle requests in a row. In the good kernel
> it's the same request twice, in the bad kernel the second requests is
> for 0 backlight. The register dumps seem to confirm this.
>
> Please try a recent kernel, with and without the the bisected bad commit

Hi, I did try the tip of Linus git tree. However I can't make the kernel
boot regardless bad commit or not. The kernel just hang there after grub2
load the kernel and initram image. It looks like a different problem than the
intel black screen because it don't even get to the point show any console
print out.

I will try the intel branch instead.

Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3] drm/i915: bounds check execbuffer relocation count

[Kees Cook]

On Thu, Mar 14, 2013 at 9:57 AM, Daniel Vetter  wrote:
> On Wed, Mar 13, 2013 at 9:28 PM, Daniel Vetter  wrote:
>> On Tue, Mar 12, 2013 at 09:07:46AM +, Chris Wilson wrote:
>>> On Mon, Mar 11, 2013 at 05:31:45PM -0700, Kees Cook wrote:
>>> > It is possible to wrap the counter used to allocate the buffer for
>>> > relocation copies. This could lead to heap writing overflows.
>>> >
>>> > CVE-2013-0913
>>> >
>>> > v3: collapse test, improve comment
>>> > v2: move check into validate_exec_list
>>> >
>>> > Signed-off-by: Kees Cook 
>>> > Reported-by: Pinkie Pie
>>> > Cc: sta...@vger.kernel.org
>>>
>>> Looks good to me. The only bikeshed that remains is whether we should
>>> just collapse the two variables into one, but the current 'max - count'
>>> is more idiomatic and so preferrable.
>>> Reviewed-by: Chris Wilson 
>>
>> Picked up for -fixes, thanks for the patch.
>
> I've forgotten to dump my wishlist: Can I have an i-g-t for this? For
> this bug here specifically an execbuf with just one buffer with too
> many relocs plus another execbuf with two buffers with relocation so
> that the 2nd relocation list will overflow should be sufficient.

Sure thing. Where do these live? (Or what docs should I read for
this?) I'm assuming i-g-t means "intel graphics test"? :)

-Kees

-- 
Kees Cook
Chrome OS Security
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH v3] drm/i915: bounds check execbuffer relocation count

[Daniel Vetter]

On Wed, Mar 13, 2013 at 9:28 PM, Daniel Vetter  wrote:
> On Tue, Mar 12, 2013 at 09:07:46AM +, Chris Wilson wrote:
>> On Mon, Mar 11, 2013 at 05:31:45PM -0700, Kees Cook wrote:
>> > It is possible to wrap the counter used to allocate the buffer for
>> > relocation copies. This could lead to heap writing overflows.
>> >
>> > CVE-2013-0913
>> >
>> > v3: collapse test, improve comment
>> > v2: move check into validate_exec_list
>> >
>> > Signed-off-by: Kees Cook 
>> > Reported-by: Pinkie Pie
>> > Cc: sta...@vger.kernel.org
>>
>> Looks good to me. The only bikeshed that remains is whether we should
>> just collapse the two variables into one, but the current 'max - count'
>> is more idiomatic and so preferrable.
>> Reviewed-by: Chris Wilson 
>
> Picked up for -fixes, thanks for the patch.

I've forgotten to dump my wishlist: Can I have an i-g-t for this? For
this bug here specifically an execbuf with just one buffer with too
many relocs plus another execbuf with two buffers with relocation so
that the 2nd relocation list will overflow should be sufficient.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/4] drm/i915: Remove unused file arg from execbuf

[Ben Widawsky]

On Thu, Mar 14, 2013 at 08:56:41AM +, Chris Wilson wrote:
> I was hoping for something a little more magical. I couldn't spot any
> mistakes - the only thing missing is a hint as to why these are useful.
> 
> Acked-by: Chris Wilson 
> -Chris

I'm pretty sure you asked for magic the last time sent this patch.
Anyway, I can probably talk a little bit without disclosing the secretz.

Primarily the goal is to be able to easily queue up the work. Packing
more into the eb struct gives a nice compact /thing/ which I can put
into a list for later execution.

Now, the deferred execution has it's own pitfalls, and I'd prefer not to
get too far into that since I think these cleanups are decent on their
own.

> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Ben Widawsky, Intel Open Source Technology Center
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 04/10] [v2] drm/i915: Don't touch South Display when PCH_NOP

[Ben Widawsky]

Interrupts, clock gating, and GMBUS are all within the, "this will hang
the CPU" range when we have PCH_NOP.

There is a bit of a hack in init clock gating. We want to do most of the
clock gating, but the part we skip will hang the system. It could
probably be abstracted a bit better, but I don't feel it's too
unsightly.

v2: Use inverse HAS_PCH_NOP check (Jani)

Signed-off-by: Ben Widawsky 
---
 drivers/gpu/drm/i915/i915_irq.c   | 20 ++--
 drivers/gpu/drm/i915/intel_bios.c |  3 +++
 drivers/gpu/drm/i915/intel_i2c.c  |  4 +++-
 drivers/gpu/drm/i915/intel_pm.c   |  3 ++-
 4 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index b860f0b..15b4668 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -738,14 +738,16 @@ static irqreturn_t ivybridge_irq_handler(int irq, void 
*arg)
}
}
 
-   /* check event from PCH */
-   if (de_iir & DE_PCH_EVENT_IVB) {
-   u32 pch_iir = I915_READ(SDEIIR);
+   if (!HAS_PCH_NOP(dev)) {
+   /* check event from PCH */
+   if (de_iir & DE_PCH_EVENT_IVB) {
+   u32 pch_iir = I915_READ(SDEIIR);
 
-   cpt_irq_handler(dev, pch_iir);
+   cpt_irq_handler(dev, pch_iir);
 
-   /* clear PCH hotplug event before clear CPU irq */
-   I915_WRITE(SDEIIR, pch_iir);
+   /* clear PCH hotplug event before CPU irq */
+   I915_WRITE(SDEIIR, pch_iir);
+   }
}
 
I915_WRITE(DEIIR, de_iir);
@@ -1910,6 +1912,9 @@ static void ironlake_irq_preinstall(struct drm_device 
*dev)
I915_WRITE(GTIER, 0x0);
POSTING_READ(GTIER);
 
+   if (HAS_PCH_NOP(dev))
+   return;
+
/* south display irq */
I915_WRITE(SDEIMR, 0x);
I915_WRITE(SDEIER, 0x0);
@@ -1982,6 +1987,9 @@ static void ibx_irq_postinstall(struct drm_device *dev)
   SDE_GMBUS_CPT |
   SDE_AUX_MASK_CPT;
 
+   if (HAS_PCH_NOP(dev))
+   return;
+
I915_WRITE(SDEIIR, I915_READ(SDEIIR));
I915_WRITE(SDEIMR, ~mask);
I915_WRITE(SDEIER, mask);
diff --git a/drivers/gpu/drm/i915/intel_bios.c 
b/drivers/gpu/drm/i915/intel_bios.c
index 55ffba1..194df27 100644
--- a/drivers/gpu/drm/i915/intel_bios.c
+++ b/drivers/gpu/drm/i915/intel_bios.c
@@ -692,6 +692,9 @@ intel_parse_bios(struct drm_device *dev)
struct bdb_header *bdb = NULL;
u8 __iomem *bios = NULL;
 
+   if (HAS_PCH_NOP(dev))
+   return -ENODEV;
+
init_vbt_defaults(dev_priv);
 
/* XXX Should this validation be moved to intel_opregion.c? */
diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index acf8aec..fc19e49 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -513,7 +513,9 @@ int intel_setup_gmbus(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev->dev_private;
int ret, i;
 
-   if (HAS_PCH_SPLIT(dev))
+   if (HAS_PCH_NOP(dev))
+   return 0;
+   else if (HAS_PCH_SPLIT(dev))
dev_priv->gpio_mmio_base = PCH_GPIOA - GPIOA;
else if (IS_VALLEYVIEW(dev))
dev_priv->gpio_mmio_base = VLV_DISPLAY_BASE;
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 5479363..52203fd 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3874,7 +3874,8 @@ static void ivybridge_init_clock_gating(struct drm_device 
*dev)
snpcr |= GEN6_MBC_SNPCR_MED;
I915_WRITE(GEN6_MBCUNIT_SNPCR, snpcr);
 
-   cpt_init_clock_gating(dev);
+   if (HAS_PCH_NOP(dev))
+   cpt_init_clock_gating(dev);
 
gen6_check_mch_setup(dev);
 }
-- 
1.8.1.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 15/16] drm/i915: refuse to submit more batchbuffers from guilty context

[Mika Kuoppala]

If context has recently submitted a faulty batchbuffers guilty of
gpu hang and decides to keep submitting more crap, ban it permanently.

Signed-off-by: Mika Kuoppala 
---
 drivers/gpu/drm/i915/i915_drv.c|   23 ++-
 drivers/gpu/drm/i915/i915_drv.h|1 +
 drivers/gpu/drm/i915/i915_gem.c|1 +
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   11 +++
 4 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 1ebed96..69c9856 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -817,6 +817,8 @@ int intel_gpu_reset(struct drm_device *dev)
 int i915_reset(struct drm_device *dev)
 {
drm_i915_private_t *dev_priv = dev->dev_private;
+   struct ctx_reset_state *gstate;
+   bool do_wedge = true;
int ret;
 
if (!i915_try_reset)
@@ -824,10 +826,29 @@ int i915_reset(struct drm_device *dev)
 
mutex_lock(&dev->struct_mutex);
 
+   /* i915_gem_reset will set this if it finds guilty context */
+   dev_priv->gpu_error.guilty_state = NULL;
+
i915_gem_reset(dev);
 
+   gstate = dev_priv->gpu_error.guilty_state;
+
+   if (gstate) {
+   if (gstate->guilty == 1) {
+   do_wedge = false;
+   } else if (!gstate->banned &&
+  get_seconds() - gstate->last_guilty_reset < 5) {
+   gstate->banned = true;
+   do_wedge = false;
+   }
+
+   gstate->last_guilty_reset = get_seconds();
+   }
+
+   dev_priv->gpu_error.guilty_state = NULL;
+
ret = -ENODEV;
-   if (get_seconds() - dev_priv->gpu_error.last_reset < 5)
+   if (do_wedge && get_seconds() - dev_priv->gpu_error.last_reset < 5)
DRM_ERROR("GPU hanging too fast, declaring wedged!\n");
else
ret = intel_gpu_reset(dev);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1a4cba0..3e11acf 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -818,6 +818,7 @@ struct i915_gpu_error {
struct work_struct work;
 
unsigned long last_reset;
+   struct ctx_reset_state *guilty_state;
 
unsigned long guilty_cnt;
/**
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 6d3916a..de7403f 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2175,6 +2175,7 @@ static void i915_set_reset_status(struct 
intel_ring_buffer *ring,
if (guilty) {
rs->guilty++;
dev_priv->gpu_error.guilty_cnt++;
+   dev_priv->gpu_error.guilty_state = rs;
} else {
rs->innocent++;
}
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 2f23013..97d3887 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -841,6 +841,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
struct drm_clip_rect *cliprects = NULL;
struct intel_ring_buffer *ring;
struct i915_hw_context *ctx;
+   struct ctx_reset_state *rs;
u32 ctx_id = i915_execbuffer2_get_context_id(*args);
u32 exec_start, exec_len;
u32 mask, flags;
@@ -1023,6 +1024,16 @@ i915_gem_do_execbuffer(struct drm_device *dev, void 
*data,
if (ret)
goto err;
 
+   ret = i915_gem_context_get_reset_state(&dev_priv->ring[RCS],
+  file, ctx_id, &rs);
+   if (ret)
+   goto err;
+
+   if (rs->banned) {
+   ret = -EIO;
+   goto err;
+   }
+
ctx = i915_switch_context(ring, file, ctx_id);
if (IS_ERR(ctx)) {
ret = PTR_ERR(ctx);
-- 
1.7.9.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 06/16] drm/i915: track ring progression using seqnos

[Mika Kuoppala]

Instead of relying in acthd, track ring seqno progression
to detect if ring has hung.

Signed-off-by: Mika Kuoppala 
---
 drivers/gpu/drm/i915/i915_drv.h |2 --
 drivers/gpu/drm/i915/i915_irq.c |   30 +-
 drivers/gpu/drm/i915/intel_ringbuffer.h |2 ++
 3 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 79cd27f..534e673 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -797,8 +797,6 @@ struct i915_gpu_error {
 #define DRM_I915_HANGCHECK_JIFFIES msecs_to_jiffies(DRM_I915_HANGCHECK_PERIOD)
struct timer_list hangcheck_timer;
int hangcheck_count;
-   uint32_t last_acthd[I915_NUM_RINGS];
-   uint32_t prev_instdone[I915_NUM_INSTDONE_REG];
 
/* For reset and error_state handling. */
spinlock_t lock;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index ed8ac1f..6594c97 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1880,22 +1880,19 @@ void i915_hangcheck_elapsed(unsigned long data)
 {
struct drm_device *dev = (struct drm_device *)data;
drm_i915_private_t *dev_priv = dev->dev_private;
-   uint32_t acthd[I915_NUM_RINGS], instdone[I915_NUM_INSTDONE_REG];
struct intel_ring_buffer *ring;
bool err = false, idle;
int i;
+   u32 seqno[I915_NUM_RINGS];
+   bool work_done;
 
if (!i915_enable_hangcheck)
return;
 
-   memset(acthd, 0, sizeof(acthd));
idle = true;
for_each_ring(ring, dev_priv, i) {
-   u32 seqno;
-
-   seqno = ring->get_seqno(ring, false);
-   idle &= i915_hangcheck_ring_idle(ring, seqno, &err);
-   acthd[i] = intel_ring_get_active_head(ring);
+   seqno[i] = ring->get_seqno(ring, false);
+   idle &= i915_hangcheck_ring_idle(ring, seqno[i], &err);
}
 
/* If all work is done then ACTHD clearly hasn't advanced. */
@@ -1911,20 +1908,19 @@ void i915_hangcheck_elapsed(unsigned long data)
return;
}
 
-   i915_get_extra_instdone(dev, instdone);
-   if (memcmp(dev_priv->gpu_error.last_acthd, acthd,
-  sizeof(acthd)) == 0 &&
-   memcmp(dev_priv->gpu_error.prev_instdone, instdone,
-  sizeof(instdone)) == 0) {
+   work_done = false;
+   for_each_ring(ring, dev_priv, i) {
+   if (ring->hangcheck_seqno != seqno[i]) {
+   work_done = true;
+   ring->hangcheck_seqno = seqno[i];
+   }
+   }
+
+   if (!work_done) {
if (i915_hangcheck_hung(dev))
return;
} else {
dev_priv->gpu_error.hangcheck_count = 0;
-
-   memcpy(dev_priv->gpu_error.last_acthd, acthd,
-  sizeof(acthd));
-   memcpy(dev_priv->gpu_error.prev_instdone, instdone,
-  sizeof(instdone));
}
 
 repeat:
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h 
b/drivers/gpu/drm/i915/intel_ringbuffer.h
index d66208c..9599c56 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -137,6 +137,8 @@ struct  intel_ring_buffer {
struct i915_hw_context *default_context;
struct drm_i915_gem_object *last_context_obj;
 
+   u32 hangcheck_seqno;
+
void *private;
 };
 
-- 
1.7.9.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 12/16] drm/i915: add batch object and context to i915_add_request()

[Mika Kuoppala]

In order to track down a batch buffer and context which
caused the ring to hang, store reference to bo and context
into the request struct. Request can also cause gpu to hang
after the batch in the flush section in the ring. To detect this
add start of the flush portion offset into the request.

Signed-off-by: Mika Kuoppala 
---
 drivers/gpu/drm/i915/i915_drv.h|   13 ++---
 drivers/gpu/drm/i915/i915_gem.c|8 ++--
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |7 ---
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 393f6a2..1a4cba0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1249,9 +1249,15 @@ struct drm_i915_gem_request {
/** GEM sequence number associated with this request. */
uint32_t seqno;
 
-   /** Postion in the ringbuffer of the end of the request */
+   /** Position in the ringbuffer of the start of the request */
+   u32 head;
+
+   /** Position in the ringbuffer of the end of the request */
u32 tail;
 
+   /** Batch buffer related to this request if any */
+   struct drm_i915_gem_object *batch_obj;
+
/** Context related to this request */
struct i915_hw_context *ctx;
 
@@ -1647,9 +1653,10 @@ int __must_check i915_gem_idle(struct drm_device *dev);
 int i915_do_add_request(struct intel_ring_buffer *ring,
u32 *seqno,
struct drm_file *file,
-   struct i915_hw_context *ctx);
+   struct i915_hw_context *ctx,
+   struct drm_i915_gem_object *batch_obj);
 #define i915_add_request(ring, seqno) \
-   i915_do_add_request(ring, seqno, NULL, NULL)
+   i915_do_add_request(ring, seqno, NULL, NULL, NULL)
 int __must_check i915_wait_seqno(struct intel_ring_buffer *ring,
 uint32_t seqno);
 int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e905086..fa7f446 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1998,14 +1998,16 @@ int
 i915_do_add_request(struct intel_ring_buffer *ring,
u32 *out_seqno,
struct drm_file *file,
-   struct i915_hw_context *ctx)
+   struct i915_hw_context *ctx,
+   struct drm_i915_gem_object *obj)
 {
drm_i915_private_t *dev_priv = ring->dev->dev_private;
struct drm_i915_gem_request *request;
-   u32 request_ring_position;
+   u32 request_ring_position, request_start;
int was_empty;
int ret;
 
+   request_start = intel_ring_get_tail(ring);
/*
 * Emit any outstanding flushes - execbuf can fail to emit the flush
 * after having emitted the batchbuffer command. Hence we need to fix
@@ -2037,7 +2039,9 @@ i915_do_add_request(struct intel_ring_buffer *ring,
 
request->seqno = intel_ring_get_seqno(ring);
request->ring = ring;
+   request->head = request_start;
request->tail = request_ring_position;
+   request->batch_obj = obj;
request->ctx = ctx;
 
if (request->ctx)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 1b524e7..2f23013 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -794,13 +794,14 @@ static void
 i915_gem_execbuffer_retire_commands(struct drm_device *dev,
struct drm_file *file,
struct intel_ring_buffer *ring,
-   struct i915_hw_context *ctx)
+   struct i915_hw_context *ctx,
+   struct drm_i915_gem_object *obj)
 {
/* Unconditionally force add_request to emit a full flush. */
ring->gpu_caches_dirty = true;
 
/* Add a breadcrumb for the completion of the batch buffer */
-   (void)i915_do_add_request(ring, NULL, file, ctx);
+   (void)i915_do_add_request(ring, NULL, file, ctx, obj);
 }
 
 static int
@@ -1075,7 +1076,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
trace_i915_gem_ring_dispatch(ring, intel_ring_get_seqno(ring), flags);
 
i915_gem_execbuffer_move_to_active(&eb->objects, ring);
-   i915_gem_execbuffer_retire_commands(dev, file, ring, ctx);
+   i915_gem_execbuffer_retire_commands(dev, file, ring, ctx, batch_obj);
 
 err:
eb_destroy(eb);
-- 
1.7.9.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 04/16] drm/i915: Resurrect ring kicking for semaphores, selectively

[Mika Kuoppala]

From: Chris Wilson 

Once we thought we got semaphores working, we disabled kicking the ring
if hangcheck fired whilst waiting upon a ring as it was doing more harm
than good:

commit 4e0e90dcb8a7df1229c69e30abebb59b0b3c2a1f
Author: Daniel Vetter 
Date:   Wed Dec 14 13:56:58 2011 +0100

drm/i915: kicking rings stuck on semaphores considered harmful

However, life is never that easy and semaphores are still causing
problems whereby the value written by one ring (bcs) is not being
propagated to the waiter (rcs). Thus the waiter never wakes up and we
declare the GPU hung, which often has unfortunate consequences, even if
we successfully reset the GPU.

But the GPU is idle as it has completed the work, just didn't notify its
clients. So we can detect the incomplete wait during hang check and
probe the target ring to see if has indeed emitted the breadcrumb seqno
following the work and then and only then kick the waiter.

Based on a suggestion by Ben Widawsky.

v2: cross-check wait with iphdr. fix signaller calculation.

References: https://bugs.freedesktop.org/show_bug.cgi?id=54226
Signed-off-by: Chris Wilson 
Signed-off-by: Mika Kuoppala 
Cc: Daniel Vetter 
Cc: Ben Widawsky 
Acked-by: Ben Widawsky 
---
 drivers/gpu/drm/i915/i915_irq.c |   40 +++
 1 file changed, 40 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 2139714..ce7efa8 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1787,6 +1787,37 @@ static bool i915_hangcheck_ring_idle(struct 
intel_ring_buffer *ring, bool *err)
return false;
 }
 
+static bool semaphore_passed(struct intel_ring_buffer *ring)
+{
+   struct drm_i915_private *dev_priv = ring->dev->dev_private;
+   u32 acthd = intel_ring_get_active_head(ring) & HEAD_ADDR;
+   struct intel_ring_buffer *signaller;
+   u32 cmd, ipehr, acthd_min;
+
+   ipehr = I915_READ(RING_IPEHR(ring->mmio_base));
+   if ((ipehr & ~(0x3 << 16)) !=
+   (MI_SEMAPHORE_MBOX | MI_SEMAPHORE_COMPARE | MI_SEMAPHORE_REGISTER))
+   return false;
+
+   /* ACTHD is likely pointing to the dword after the actual command,
+* so scan backwards until we find the MBOX.
+*/
+   acthd_min = max((int)acthd - 3 * 4, 0);
+   do {
+   cmd = ioread32(ring->virtual_start + acthd);
+   if (cmd == ipehr)
+   break;
+
+   acthd -= 4;
+   if (acthd < acthd_min)
+   return false;
+   } while (1);
+
+   signaller = &dev_priv->ring[(ring->id + (((ipehr >> 17) & 1) + 1)) % 3];
+   return i915_seqno_passed(signaller->get_seqno(signaller, false),
+ioread32(ring->virtual_start+acthd+4)+1);
+}
+
 static bool kick_ring(struct intel_ring_buffer *ring)
 {
struct drm_device *dev = ring->dev;
@@ -1798,6 +1829,15 @@ static bool kick_ring(struct intel_ring_buffer *ring)
I915_WRITE_CTL(ring, tmp);
return true;
}
+
+   if (INTEL_INFO(dev)->gen >= 6 &&
+   tmp & RING_WAIT_SEMAPHORE &&
+   semaphore_passed(ring)) {
+   DRM_ERROR("Kicking stuck semaphore on %s\n",
+ ring->name);
+   I915_WRITE_CTL(ring, tmp);
+   return true;
+   }
return false;
 }
 
-- 
1.7.9.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 02/16] drm/i915: cleanup i915_add_request

[Mika Kuoppala]

Only execbuffer needs all the parameters. Cleanup everything
else behind macro.

Signed-off-by: Mika Kuoppala 
---
 drivers/gpu/drm/i915/i915_drv.h|8 +---
 drivers/gpu/drm/i915/i915_gem.c|   10 +-
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |2 +-
 drivers/gpu/drm/i915/intel_overlay.c   |4 ++--
 drivers/gpu/drm/i915/intel_ringbuffer.c|2 +-
 5 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ccac8ea..930b0ac 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1626,9 +1626,11 @@ void i915_gem_init_ppgtt(struct drm_device *dev);
 void i915_gem_cleanup_ringbuffer(struct drm_device *dev);
 int __must_check i915_gpu_idle(struct drm_device *dev);
 int __must_check i915_gem_idle(struct drm_device *dev);
-int i915_add_request(struct intel_ring_buffer *ring,
-struct drm_file *file,
-u32 *seqno);
+int i915_do_add_request(struct intel_ring_buffer *ring,
+   u32 *seqno,
+   struct drm_file *file);
+#define i915_add_request(ring, seqno) \
+   i915_do_add_request(ring, seqno, NULL)
 int __must_check i915_wait_seqno(struct intel_ring_buffer *ring,
 uint32_t seqno);
 int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 93b8e05..105ad4b 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -967,7 +967,7 @@ i915_gem_check_olr(struct intel_ring_buffer *ring, u32 
seqno)
 
ret = 0;
if (seqno == ring->outstanding_lazy_request)
-   ret = i915_add_request(ring, NULL, NULL);
+   ret = i915_add_request(ring, NULL);
 
return ret;
 }
@@ -1995,9 +1995,9 @@ i915_gem_get_seqno(struct drm_device *dev, u32 *seqno)
 }
 
 int
-i915_add_request(struct intel_ring_buffer *ring,
-struct drm_file *file,
-u32 *out_seqno)
+i915_do_add_request(struct intel_ring_buffer *ring,
+   u32 *out_seqno,
+   struct drm_file *file)
 {
drm_i915_private_t *dev_priv = ring->dev->dev_private;
struct drm_i915_gem_request *request;
@@ -2262,7 +2262,7 @@ i915_gem_retire_work_handler(struct work_struct *work)
idle = true;
for_each_ring(ring, dev_priv, i) {
if (ring->gpu_caches_dirty)
-   i915_add_request(ring, NULL, NULL);
+   i915_add_request(ring, NULL);
 
idle &= list_empty(&ring->request_list);
}
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index ad7029e..013aca6 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -799,7 +799,7 @@ i915_gem_execbuffer_retire_commands(struct drm_device *dev,
ring->gpu_caches_dirty = true;
 
/* Add a breadcrumb for the completion of the batch buffer */
-   (void)i915_add_request(ring, file, NULL);
+   (void)i915_do_add_request(ring, NULL, file);
 }
 
 static int
diff --git a/drivers/gpu/drm/i915/intel_overlay.c 
b/drivers/gpu/drm/i915/intel_overlay.c
index 67a2501..40e509c 100644
--- a/drivers/gpu/drm/i915/intel_overlay.c
+++ b/drivers/gpu/drm/i915/intel_overlay.c
@@ -217,7 +217,7 @@ static int intel_overlay_do_wait_request(struct 
intel_overlay *overlay,
int ret;
 
BUG_ON(overlay->last_flip_req);
-   ret = i915_add_request(ring, NULL, &overlay->last_flip_req);
+   ret = i915_add_request(ring, &overlay->last_flip_req);
if (ret)
return ret;
 
@@ -286,7 +286,7 @@ static int intel_overlay_continue(struct intel_overlay 
*overlay,
intel_ring_emit(ring, flip_addr);
intel_ring_advance(ring);
 
-   return i915_add_request(ring, NULL, &overlay->last_flip_req);
+   return i915_add_request(ring, &overlay->last_flip_req);
 }
 
 static void intel_overlay_release_old_vid_tail(struct intel_overlay *overlay)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c 
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 1d5d613..9584bc1 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1423,7 +1423,7 @@ int intel_ring_idle(struct intel_ring_buffer *ring)
 
/* We need to add any requests required to flush the objects and ring */
if (ring->outstanding_lazy_request) {
-   ret = i915_add_request(ring, NULL, NULL);
+   ret = i915_add_request(ring, NULL);
if (ret)
return ret;
}
-- 
1.7.9.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 11/16] drm/i915: add reset_state for hw_contexts

[Mika Kuoppala]

For arb-robustness, every context needs to have it's own
reset state tracking. Default context will be handled in a identical
way as the no-context case in further down in the patch set.
For no-context case, the reset state will be stored in
the file_priv part.

v2: handle default context inside get_reset_state

Signed-off-by: Mika Kuoppala 
---
 drivers/gpu/drm/i915/i915_drv.h |4 
 drivers/gpu/drm/i915/i915_gem_context.c |   34 +++
 2 files changed, 38 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d004548..393f6a2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1696,6 +1696,10 @@ i915_switch_context(struct intel_ring_buffer *ring,
 void i915_gem_context_free(struct kref *ctx_ref);
 void i915_gem_context_init_reset_state(struct drm_device *dev,
   struct ctx_reset_state *rs);
+int i915_gem_context_get_reset_state(struct intel_ring_buffer *ring,
+struct drm_file *file,
+u32 id,
+struct ctx_reset_state **rs);
 int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
  struct drm_file *file);
 int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index dbd14b8..c3faaa3 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -315,6 +315,40 @@ static int context_idr_cleanup(int id, void *p, void *data)
return 0;
 }
 
+int i915_gem_context_get_reset_state(struct intel_ring_buffer *ring,
+struct drm_file *file,
+u32 id,
+struct ctx_reset_state **rs)
+{
+   struct drm_i915_private *dev_priv = ring->dev->dev_private;
+   struct drm_i915_file_private *file_priv = file->driver_priv;
+   struct i915_hw_context *to;
+
+   if (dev_priv->hw_contexts_disabled)
+   return -ENOENT;
+
+   if (ring->id != RCS)
+   return -EINVAL;
+
+   if (rs == NULL)
+   return -EINVAL;
+
+   if (file == NULL)
+   return -EINVAL;
+
+   if (id == DEFAULT_CONTEXT_ID) {
+   *rs = &file_priv->reset_state;
+   } else {
+   to = i915_gem_context_get(file->driver_priv, id);
+   if (to == NULL)
+   return -ENOENT;
+
+   *rs = &to->reset_state;
+   }
+
+   return 0;
+}
+
 void i915_gem_context_close(struct drm_device *dev, struct drm_file *file)
 {
struct drm_i915_file_private *file_priv = file->driver_priv;
-- 
1.7.9.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 05/16] drm/i915: pass seqno to i915_hangcheck_ring_idle

[Mika Kuoppala]

In preparation for next commit, pass seqno as a parameter
to i915_hangcheck_ring_idle as it will be used inside
i915_hangcheck_elapsed.

Signed-off-by: Mika Kuoppala 
---
 drivers/gpu/drm/i915/i915_irq.c |   11 +++
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index ce7efa8..ed8ac1f 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1770,11 +1770,11 @@ ring_last_seqno(struct intel_ring_buffer *ring)
  struct drm_i915_gem_request, list)->seqno;
 }
 
-static bool i915_hangcheck_ring_idle(struct intel_ring_buffer *ring, bool *err)
+static bool i915_hangcheck_ring_idle(struct intel_ring_buffer *ring,
+u32 ring_seqno, bool *err)
 {
if (list_empty(&ring->request_list) ||
-   i915_seqno_passed(ring->get_seqno(ring, false),
- ring_last_seqno(ring))) {
+   i915_seqno_passed(ring_seqno, ring_last_seqno(ring))) {
/* Issue a wake-up to catch stuck h/w. */
if (waitqueue_active(&ring->irq_queue)) {
DRM_ERROR("Hangcheck timer elapsed... %s idle\n",
@@ -1891,7 +1891,10 @@ void i915_hangcheck_elapsed(unsigned long data)
memset(acthd, 0, sizeof(acthd));
idle = true;
for_each_ring(ring, dev_priv, i) {
-   idle &= i915_hangcheck_ring_idle(ring, &err);
+   u32 seqno;
+
+   seqno = ring->get_seqno(ring, false);
+   idle &= i915_hangcheck_ring_idle(ring, seqno, &err);
acthd[i] = intel_ring_get_active_head(ring);
}
 
-- 
1.7.9.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 14/16] drm/i915: find guilty batch buffer on ring resets

[Mika Kuoppala]

After hang check timer has declared gpu to be hang,
rings are reset. In ring reset, when clearing
request list, do post mortem analysis to find out
the guilty batch buffer.

Select requests for further analysis by inspecting
the completed sequence number which has been updated
into the HWS page. If request was completed, it can't
be related to the hang.

For noncompleted requests mark the batch as guilty
if the ring was not waiting and the ring head was
stuck inside the buffer object or in the flush region
right after the batch. For everything else, mark
them as innocents.

v2: Fixed a typo in commit message (Ville Syrjälä)

Signed-off-by: Mika Kuoppala 
---
 drivers/gpu/drm/i915/i915_gem.c |   91 +++
 1 file changed, 91 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index fa7f446..6d3916a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2099,9 +2099,97 @@ i915_gem_request_remove_from_client(struct 
drm_i915_gem_request *request)
spin_unlock(&file_priv->mm.lock);
 }
 
+static bool i915_head_inside_object(u32 acthd, struct drm_i915_gem_object *obj)
+{
+   if (acthd >= obj->gtt_offset &&
+   acthd < obj->gtt_offset + obj->base.size)
+   return true;
+
+   return false;
+}
+
+static bool i915_head_inside_request(u32 acthd, u32 rs, u32 re)
+{
+   if (rs < re) {
+   if (acthd >= rs && acthd < re)
+   return true;
+   } else if (rs > re) {
+   if (acthd >= rs || acthd < re)
+   return true;
+   }
+
+   return false;
+}
+
+static bool i915_request_guilty(struct drm_i915_gem_request *request,
+   const u32 acthd, bool *inside)
+{
+   if (request->batch_obj) {
+   if (i915_head_inside_object(acthd, request->batch_obj)) {
+   *inside = true;
+   return true;
+   }
+   }
+
+   if (i915_head_inside_request(acthd, request->head, request->tail)) {
+   *inside = false;
+   return true;
+   }
+
+   return false;
+}
+
+static void i915_set_reset_status(struct intel_ring_buffer *ring,
+ struct drm_i915_gem_request *request,
+ u32 acthd)
+{
+   struct drm_i915_private *dev_priv = ring->dev->dev_private;
+   struct ctx_reset_state *rs = NULL;
+   bool inside, guilty;
+
+   /* Innocent until proven guilty */
+   guilty = false;
+
+   if (!ring->hangcheck_was_waiting &&
+   i915_request_guilty(request, acthd, &inside)) {
+   DRM_ERROR("%s hung %s bo (0x%x ctx %d) at 0x%x\n",
+ ring->name,
+ inside ? "inside" : "flushing",
+ request->batch_obj ?
+ request->batch_obj->gtt_offset : 0,
+ request->ctx ? request->ctx->id : 0,
+ acthd);
+
+   guilty = true;
+   }
+
+   /* If contexts are disabled or this is the default context, use
+* file_priv->reset_state
+*/
+   if (request->ctx && request->ctx->id != DEFAULT_CONTEXT_ID)
+   rs = &request->ctx->reset_state;
+   else if (request->file_priv)
+   rs = &request->file_priv->reset_state;
+
+   if (rs) {
+   if (guilty) {
+   rs->guilty++;
+   dev_priv->gpu_error.guilty_cnt++;
+   } else {
+   rs->innocent++;
+   }
+   }
+}
+
 static void i915_gem_reset_ring_lists(struct drm_i915_private *dev_priv,
  struct intel_ring_buffer *ring)
 {
+   u32 completed_seqno;
+   u32 acthd;
+
+   acthd = intel_ring_get_active_head(ring);
+   completed_seqno = ring->get_seqno(ring, false);
+
while (!list_empty(&ring->request_list)) {
struct drm_i915_gem_request *request;
 
@@ -2109,6 +2197,9 @@ static void i915_gem_reset_ring_lists(struct 
drm_i915_private *dev_priv,
   struct drm_i915_gem_request,
   list);
 
+   if (request->seqno > completed_seqno)
+   i915_set_reset_status(ring, request, acthd);
+
list_del(&request->list);
i915_gem_request_remove_from_client(request);
 
-- 
1.7.9.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 00/16] arb robustness enablers v2

[Mika Kuoppala]

Hi,

Reworked patchset for guilty context detection.
Main changes since the last posting:
- i915_add_request cleanup
- i915_switch_context returns ERR_PTR
- 3 seconds to declare hang regardless if guilty was found
- semaphore kicking for hung rings (from Chris)
- test case for the new interface
- interface is now included to have common base for discussions.

Thank you all who provided feedback.

The tree can be found here:
https://github.com/mkuoppal/linux/commits/arb-robustness

And testcase for i-g-t here:
https://github.com/mkuoppal/intel-gpu-tools/tree/arb-robustness

-Mika

Chris Wilson (1):
  drm/i915: Resurrect ring kicking for semaphores, selectively

Mika Kuoppala (15):
  drm/i915: return context from i915_switch_context()
  drm/i915: cleanup i915_add_request
  drm/i915: reference count for i915_hw_contexts
  drm/i915: pass seqno to i915_hangcheck_ring_idle
  drm/i915: track ring progression using seqnos
  drm/i915: introduce i915_hangcheck_ring_hung
  drm/i915: detect hang using per ring hangcheck_score
  drm/i915: remove i915_hangcheck_hung
  drm/i915: add struct ctx_reset_state
  drm/i915: add reset_state for hw_contexts
  drm/i915: add batch object and context to i915_add_request()
  drm/i915: mark rings which were waiting when hang happened
  drm/i915: find guilty batch buffer on ring resets
  drm/i915: refuse to submit more batchbuffers from guilty context
  drm/i915: add i915_gem_context_get_reset_status_ioctl

 drivers/gpu/drm/i915/i915_dma.c|5 +-
 drivers/gpu/drm/i915/i915_drv.c|   84 +++-
 drivers/gpu/drm/i915/i915_drv.h|   57 +--
 drivers/gpu/drm/i915/i915_gem.c|  130 ++--
 drivers/gpu/drm/i915/i915_gem_context.c|   97 +++---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   24 -
 drivers/gpu/drm/i915/i915_irq.c|  151 
 drivers/gpu/drm/i915/intel_overlay.c   |4 +-
 drivers/gpu/drm/i915/intel_ringbuffer.c|2 +-
 drivers/gpu/drm/i915/intel_ringbuffer.h|4 +
 include/uapi/drm/i915_drm.h|   28 ++
 11 files changed, 481 insertions(+), 105 deletions(-)

-- 
1.7.9.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 16/16] drm/i915: add i915_gem_context_get_reset_status_ioctl

[Mika Kuoppala]

This ioctl returns context reset status for specified context.

Signed-off-by: Mika Kuoppala 
CC: i...@freedesktop.org
---
 drivers/gpu/drm/i915/i915_dma.c |1 +
 drivers/gpu/drm/i915/i915_drv.c |   61 +++
 drivers/gpu/drm/i915/i915_drv.h |2 ++
 include/uapi/drm/i915_drm.h |   28 ++
 4 files changed, 92 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 7902d97..c919832 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1903,6 +1903,7 @@ struct drm_ioctl_desc i915_ioctls[] = {
DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_CREATE, 
i915_gem_context_create_ioctl, DRM_UNLOCKED),
DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_DESTROY, 
i915_gem_context_destroy_ioctl, DRM_UNLOCKED),
DRM_IOCTL_DEF_DRV(I915_REG_READ, i915_reg_read_ioctl, DRM_UNLOCKED),
+   DRM_IOCTL_DEF_DRV(I915_GET_RESET_STATUS, 
i915_gem_context_get_reset_status_ioctl, DRM_UNLOCKED),
 };
 
 int i915_max_ioctl = DRM_ARRAY_SIZE(i915_ioctls);
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 69c9856..a4d06f2 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1267,3 +1267,64 @@ int i915_reg_read_ioctl(struct drm_device *dev,
 
return 0;
 }
+
+int i915_gem_context_get_reset_status_ioctl(struct drm_device *dev,
+   void *data, struct drm_file *file)
+{
+   struct drm_i915_private *dev_priv = dev->dev_private;
+   struct intel_ring_buffer *ring;
+   struct drm_i915_reset_status *args = data;
+   struct ctx_reset_state *rs = NULL;
+   unsigned long reset_cnt;
+   u32 reset_status = I915_RESET_UNKNOWN;
+   int ret;
+
+   ret = mutex_lock_interruptible(&dev->struct_mutex);
+   if (ret)
+   return ret;
+
+   ring = &dev_priv->ring[RCS];
+
+   ret = i915_gem_context_get_reset_state(ring,
+  file,
+  args->ctx_id,
+  &rs);
+   if (ret)
+   goto out;
+
+   BUG_ON(!rs);
+
+   reset_cnt = atomic_read(&dev_priv->gpu_error.reset_counter);
+
+   if (reset_cnt & I915_RESET_IN_PROGRESS_FLAG ||
+   reset_cnt == I915_WEDGED) {
+   goto out;
+   }
+
+   /* Set guilty/innocent status if only one reset was
+* observed and if only one guilty was found
+*/
+   if ((rs->reset_cnt + 2) == reset_cnt &&
+   (rs->guilty_cnt + 1) == dev_priv->gpu_error.guilty_cnt) {
+   reset_status = 0;
+
+   if (rs->guilty)
+   reset_status |= I915_RESET_BATCH_ACTIVE;
+
+   if (rs->innocent)
+   reset_status |= I915_RESET_BATCH_PENDING;
+
+   if (reset_status == 0)
+   reset_status = I915_RESET_UNKNOWN;
+   } else if (rs->reset_cnt == reset_cnt) {
+   reset_status = I915_RESET_NO_ERROR;
+   }
+
+out:
+   if (!ret)
+   args->reset_status = reset_status;
+
+   mutex_unlock(&dev->struct_mutex);
+
+   return ret ? -EINVAL : 0;
+}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3e11acf..2e5e8e7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1712,6 +1712,8 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, 
void *data,
  struct drm_file *file);
 int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
   struct drm_file *file);
+int i915_gem_context_get_reset_status_ioctl(struct drm_device *dev,
+   void *data, struct drm_file *file);
 
 /* i915_gem_gtt.c */
 void i915_gem_cleanup_aliasing_ppgtt(struct drm_device *dev);
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 07d5941..a195e0e 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -198,6 +198,7 @@ typedef struct _drm_i915_sarea {
 #define DRM_I915_GEM_SET_CACHING   0x2f
 #define DRM_I915_GEM_GET_CACHING   0x30
 #define DRM_I915_REG_READ  0x31
+#define DRM_I915_GET_RESET_STATUS  0x32
 
 #define DRM_IOCTL_I915_INITDRM_IOW( DRM_COMMAND_BASE + 
DRM_I915_INIT, drm_i915_init_t)
 #define DRM_IOCTL_I915_FLUSH   DRM_IO ( DRM_COMMAND_BASE + 
DRM_I915_FLUSH)
@@ -247,6 +248,7 @@ typedef struct _drm_i915_sarea {
 #define DRM_IOCTL_I915_GEM_CONTEXT_CREATE  DRM_IOWR (DRM_COMMAND_BASE + 
DRM_I915_GEM_CONTEXT_CREATE, struct drm_i915_gem_context_create)
 #define DRM_IOCTL_I915_GEM_CONTEXT_DESTROY DRM_IOW (DRM_COMMAND_BASE + 
DRM_I915_GEM_CONTEXT_DESTROY, struct drm_i915_gem_context_destroy)
 #define DRM_IOCTL_I915_REG_READDRM_IOWR 
(DRM_COMMAND_BASE 

[Intel-gfx] [PATCH v2 13/16] drm/i915: mark rings which were waiting when hang happened

[Mika Kuoppala]

For guilty batchbuffer analysis later on on ring resets,
mark all waiting rings so that we can skip them when
trying to find a true culprit for the gpu hang.

Signed-off-by: Mika Kuoppala 
---
 drivers/gpu/drm/i915/i915_irq.c |3 ++-
 drivers/gpu/drm/i915/intel_ringbuffer.h |1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 5e91e94..7d13259 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1889,7 +1889,8 @@ void i915_hangcheck_elapsed(unsigned long data)
ring->hangcheck_score++;
 
/* Kick ring */
-   i915_hangcheck_ring_hung(ring);
+   ring->hangcheck_was_waiting =
+   !i915_hangcheck_ring_hung(ring);
} else {
ring->hangcheck_score = 0;
}
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h 
b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 97b8f37..573b0ef 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -139,6 +139,7 @@ struct  intel_ring_buffer {
 
u32 hangcheck_seqno;
int hangcheck_score;
+   bool hangcheck_was_waiting;
 
void *private;
 };
-- 
1.7.9.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 10/16] drm/i915: add struct ctx_reset_state

[Mika Kuoppala]

To count context losses, add struct ctx_reset_state for
both i915_hw_context and drm_i915_file_private.
drm_i915_file_private is used when there is no context.

Signed-off-by: Mika Kuoppala 
---
 drivers/gpu/drm/i915/i915_dma.c |4 +++-
 drivers/gpu/drm/i915/i915_drv.h |   19 +++
 drivers/gpu/drm/i915/i915_gem_context.c |   11 +++
 3 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index e16099b..7902d97 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1792,7 +1792,7 @@ int i915_driver_open(struct drm_device *dev, struct 
drm_file *file)
struct drm_i915_file_private *file_priv;
 
DRM_DEBUG_DRIVER("\n");
-   file_priv = kmalloc(sizeof(*file_priv), GFP_KERNEL);
+   file_priv = kzalloc(sizeof(*file_priv), GFP_KERNEL);
if (!file_priv)
return -ENOMEM;
 
@@ -1801,6 +1801,8 @@ int i915_driver_open(struct drm_device *dev, struct 
drm_file *file)
spin_lock_init(&file_priv->mm.lock);
INIT_LIST_HEAD(&file_priv->mm.request_list);
 
+   i915_gem_context_init_reset_state(dev, &file_priv->reset_state);
+
idr_init(&file_priv->context_idr);
 
return 0;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a54c507..d004548 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -433,6 +433,19 @@ struct i915_hw_ppgtt {
void (*cleanup)(struct i915_hw_ppgtt *ppgtt);
 };
 
+struct ctx_reset_state {
+   /* guilty and reset counts when context initialized */
+   unsigned long guilty_cnt;
+   unsigned long reset_cnt;
+
+   unsigned innocent;
+   unsigned guilty;
+
+   unsigned long last_guilty_reset;
+
+   /* banned to submit more work */
+   bool banned;
+};
 
 /* This must match up with the value previously used for execbuf2.rsvd1. */
 #define DEFAULT_CONTEXT_ID 0
@@ -443,6 +456,7 @@ struct i915_hw_context {
struct drm_i915_file_private *file_priv;
struct intel_ring_buffer *ring;
struct drm_i915_gem_object *obj;
+   struct ctx_reset_state reset_state;
 };
 
 enum no_fbc_reason {
@@ -805,6 +819,7 @@ struct i915_gpu_error {
 
unsigned long last_reset;
 
+   unsigned long guilty_cnt;
/**
 * State variable and reset counter controlling the reset flow
 *
@@ -1257,6 +1272,8 @@ struct drm_i915_file_private {
struct list_head request_list;
} mm;
struct idr context_idr;
+
+   struct ctx_reset_state reset_state;
 };
 
 #define INTEL_INFO(dev)(((struct drm_i915_private *) 
(dev)->dev_private)->info)
@@ -1677,6 +1694,8 @@ struct i915_hw_context * __must_check
 i915_switch_context(struct intel_ring_buffer *ring,
struct drm_file *file, int to_id);
 void i915_gem_context_free(struct kref *ctx_ref);
+void i915_gem_context_init_reset_state(struct drm_device *dev,
+  struct ctx_reset_state *rs);
 int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
  struct drm_file *file);
 int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index 8fb4d3c..dbd14b8 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -145,6 +145,15 @@ static void do_destroy(struct i915_hw_context *ctx)
kfree(ctx);
 }
 
+void i915_gem_context_init_reset_state(struct drm_device *dev,
+  struct ctx_reset_state *rs)
+{
+   struct drm_i915_private *dev_priv = dev->dev_private;
+
+   rs->reset_cnt = atomic_read(&dev_priv->gpu_error.reset_counter);
+   rs->guilty_cnt = dev_priv->gpu_error.guilty_cnt;
+}
+
 static struct i915_hw_context *
 create_hw_context(struct drm_device *dev,
  struct drm_i915_file_private *file_priv)
@@ -177,6 +186,8 @@ create_hw_context(struct drm_device *dev,
 
ctx->file_priv = file_priv;
 
+   i915_gem_context_init_reset_state(dev, &ctx->reset_state);
+
 again:
if (idr_pre_get(&file_priv->context_idr, GFP_KERNEL) == 0) {
ret = -ENOMEM;
-- 
1.7.9.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 08/16] drm/i915: detect hang using per ring hangcheck_score

[Mika Kuoppala]

Add per ring score of possible culprit for gpu hang. If
ring is busy and not waiting, it will get the highest score
across calls to i915_hangcheck_elapsed. This way we are
most likely to find the ring that caused the hang among
the waiting ones.

Signed-off-by: Mika Kuoppala 
---
 drivers/gpu/drm/i915/i915_irq.c |   63 ---
 drivers/gpu/drm/i915/intel_ringbuffer.h |1 +
 2 files changed, 34 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 5ba7f19..dee4557 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -360,7 +360,6 @@ static void notify_ring(struct drm_device *dev,
 
wake_up_all(&ring->irq_queue);
if (i915_enable_hangcheck) {
-   dev_priv->gpu_error.hangcheck_count = 0;
mod_timer(&dev_priv->gpu_error.hangcheck_timer,
  round_jiffies_up(jiffies + 
DRM_I915_HANGCHECK_JIFFIES));
}
@@ -1886,52 +1885,56 @@ void i915_hangcheck_elapsed(unsigned long data)
struct drm_device *dev = (struct drm_device *)data;
drm_i915_private_t *dev_priv = dev->dev_private;
struct intel_ring_buffer *ring;
-   bool err = false, idle;
int i;
-   u32 seqno[I915_NUM_RINGS];
-   bool work_done;
+   int busy_count = 0, rings_hung = 0;
 
if (!i915_enable_hangcheck)
return;
 
-   idle = true;
for_each_ring(ring, dev_priv, i) {
-   seqno[i] = ring->get_seqno(ring, false);
-   idle &= i915_hangcheck_ring_idle(ring, seqno[i], &err);
-   }
+   u32 seqno;
+   bool idle, err = false;
+
+   seqno = ring->get_seqno(ring, false);
+   idle = i915_hangcheck_ring_idle(ring, seqno, &err);
 
-   /* If all work is done then ACTHD clearly hasn't advanced. */
-   if (idle) {
-   if (err) {
-   if (i915_hangcheck_hung(dev))
-   return;
+   if (idle) {
+   if (err)
+   ring->hangcheck_score++;
+   else
+   ring->hangcheck_score = 0;
+   } else {
+   busy_count++;
 
-   goto repeat;
+   if (ring->hangcheck_seqno == seqno) {
+   ring->hangcheck_score++;
+
+   /* Kick ring */
+   i915_hangcheck_ring_hung(ring);
+   } else {
+   ring->hangcheck_score = 0;
+   }
}
 
-   dev_priv->gpu_error.hangcheck_count = 0;
-   return;
+   ring->hangcheck_seqno = seqno;
}
 
-   work_done = false;
for_each_ring(ring, dev_priv, i) {
-   if (ring->hangcheck_seqno != seqno[i]) {
-   work_done = true;
-   ring->hangcheck_seqno = seqno[i];
+   if (ring->hangcheck_score > 1) {
+   rings_hung++;
+   DRM_ERROR("%s seems hung\n", ring->name);
}
}
 
-   if (!work_done) {
-   if (i915_hangcheck_hung(dev))
-   return;
-   } else {
-   dev_priv->gpu_error.hangcheck_count = 0;
-   }
+   if (rings_hung)
+   return i915_handle_error(dev, true);
 
-repeat:
-   /* Reset timer case chip hangs without another request being added */
-   mod_timer(&dev_priv->gpu_error.hangcheck_timer,
- round_jiffies_up(jiffies + DRM_I915_HANGCHECK_JIFFIES));
+   if (busy_count)
+   /* Reset timer case chip hangs without another request
+* being added */
+   mod_timer(&dev_priv->gpu_error.hangcheck_timer,
+ round_jiffies_up(jiffies +
+  DRM_I915_HANGCHECK_JIFFIES));
 }
 
 /* drm_dma.h hooks
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h 
b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 9599c56..97b8f37 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -138,6 +138,7 @@ struct  intel_ring_buffer {
struct drm_i915_gem_object *last_context_obj;
 
u32 hangcheck_seqno;
+   int hangcheck_score;
 
void *private;
 };
-- 
1.7.9.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 09/16] drm/i915: remove i915_hangcheck_hung

[Mika Kuoppala]

Rework of per ring hangcheck made this obsolete.

Signed-off-by: Mika Kuoppala 
---
 drivers/gpu/drm/i915/i915_drv.h |1 -
 drivers/gpu/drm/i915/i915_irq.c |   21 -
 2 files changed, 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 534e673..a54c507 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -796,7 +796,6 @@ struct i915_gpu_error {
 #define DRM_I915_HANGCHECK_PERIOD 1500 /* in ms */
 #define DRM_I915_HANGCHECK_JIFFIES msecs_to_jiffies(DRM_I915_HANGCHECK_PERIOD)
struct timer_list hangcheck_timer;
-   int hangcheck_count;
 
/* For reset and error_state handling. */
spinlock_t lock;
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index dee4557..5e91e94 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1853,27 +1853,6 @@ static bool i915_hangcheck_ring_hung(struct 
intel_ring_buffer *ring)
return !kick_ring(ring);
 }
 
-static bool i915_hangcheck_hung(struct drm_device *dev)
-{
-   drm_i915_private_t *dev_priv = dev->dev_private;
-
-   if (dev_priv->gpu_error.hangcheck_count++ > 1) {
-   bool hung = true;
-   struct intel_ring_buffer *ring;
-   int i;
-
-   DRM_ERROR("Hangcheck timer elapsed... GPU hung\n");
-   i915_handle_error(dev, true);
-
-   for_each_ring(ring, dev_priv, i)
-   hung &= i915_hangcheck_ring_hung(ring);
-
-   return hung;
-   }
-
-   return false;
-}
-
 /**
  * This is called when the chip hasn't reported back with completed
  * batchbuffers in a long time. The first time this is called we simply record
-- 
1.7.9.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 07/16] drm/i915: introduce i915_hangcheck_ring_hung

[Mika Kuoppala]

In preparation to track per ring progress in hangcheck,
add i915_hangcheck_ring_hung.

v2: omit dev parameter (Ben Widawsky)

Signed-off-by: Mika Kuoppala 
---
 drivers/gpu/drm/i915/i915_irq.c |   29 +
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 6594c97..5ba7f19 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1841,28 +1841,33 @@ static bool kick_ring(struct intel_ring_buffer *ring)
return false;
 }
 
+static bool i915_hangcheck_ring_hung(struct intel_ring_buffer *ring)
+{
+   if (IS_GEN2(ring->dev))
+   return false;
+
+   /* Is the chip hanging on a WAIT_FOR_EVENT?
+* If so we can simply poke the RB_WAIT bit
+* and break the hang. This should work on
+* all but the second generation chipsets.
+*/
+   return !kick_ring(ring);
+}
+
 static bool i915_hangcheck_hung(struct drm_device *dev)
 {
drm_i915_private_t *dev_priv = dev->dev_private;
 
if (dev_priv->gpu_error.hangcheck_count++ > 1) {
bool hung = true;
+   struct intel_ring_buffer *ring;
+   int i;
 
DRM_ERROR("Hangcheck timer elapsed... GPU hung\n");
i915_handle_error(dev, true);
 
-   if (!IS_GEN2(dev)) {
-   struct intel_ring_buffer *ring;
-   int i;
-
-   /* Is the chip hanging on a WAIT_FOR_EVENT?
-* If so we can simply poke the RB_WAIT bit
-* and break the hang. This should work on
-* all but the second generation chipsets.
-*/
-   for_each_ring(ring, dev_priv, i)
-   hung &= !kick_ring(ring);
-   }
+   for_each_ring(ring, dev_priv, i)
+   hung &= i915_hangcheck_ring_hung(ring);
 
return hung;
}
-- 
1.7.9.5

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH v2 01/16] drm/i915: return context from i915_switch_context()

[Mika Kuoppala]

In preparation for the next commit, return context that
was switched to from i915_switch_context().

v2: context in return value instead of param. (Ben Widawsky)

Signed-off-by: Mika Kuoppala 
---
 drivers/gpu/drm/i915/i915_drv.h|5 +++--
 drivers/gpu/drm/i915/i915_gem.c|8 ---
 drivers/gpu/drm/i915/i915_gem_context.c|   32 +---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |7 --
 4 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ca6b215..ccac8ea 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1669,8 +1669,9 @@ struct dma_buf *i915_gem_prime_export(struct drm_device 
*dev,
 void i915_gem_context_init(struct drm_device *dev);
 void i915_gem_context_fini(struct drm_device *dev);
 void i915_gem_context_close(struct drm_device *dev, struct drm_file *file);
-int i915_switch_context(struct intel_ring_buffer *ring,
-   struct drm_file *file, int to_id);
+struct i915_hw_context * __must_check
+i915_switch_context(struct intel_ring_buffer *ring,
+   struct drm_file *file, int to_id);
 int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
  struct drm_file *file);
 int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1417fc6..93b8e05 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2517,9 +2517,11 @@ int i915_gpu_idle(struct drm_device *dev)
 
/* Flush everything onto the inactive list. */
for_each_ring(ring, dev_priv, i) {
-   ret = i915_switch_context(ring, NULL, DEFAULT_CONTEXT_ID);
-   if (ret)
-   return ret;
+   struct i915_hw_context *ctx;
+
+   ctx = i915_switch_context(ring, NULL, DEFAULT_CONTEXT_ID);
+   if (IS_ERR(ctx))
+   return PTR_ERR(ctx);
 
ret = intel_ring_idle(ring);
if (ret)
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index 21177d9..258e5e0 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -428,41 +428,47 @@ static int do_switch(struct i915_hw_context *to)
 /**
  * i915_switch_context() - perform a GPU context switch.
  * @ring: ring for which we'll execute the context switch
- * @file_priv: file_priv associated with the context, may be NULL
- * @id: context id number
- * @seqno: sequence number by which the new context will be switched to
- * @flags:
+ * @file: file associated with the context, may be NULL
+ * @to_id: context id number
+ * @return: context that was switched to or ERR_PTR on error
  *
  * The context life cycle is simple. The context refcount is incremented and
  * decremented by 1 and create and destroy. If the context is in use by the 
GPU,
  * it will have a refoucnt > 1. This allows us to destroy the context abstract
  * object while letting the normal object tracking destroy the backing BO.
  */
-int i915_switch_context(struct intel_ring_buffer *ring,
-   struct drm_file *file,
-   int to_id)
+
+struct i915_hw_context *
+i915_switch_context(struct intel_ring_buffer *ring,
+   struct drm_file *file,
+   int to_id)
 {
struct drm_i915_private *dev_priv = ring->dev->dev_private;
-   struct i915_hw_context *to;
+   struct i915_hw_context *to = NULL;
+   int ret = 0;
 
if (dev_priv->hw_contexts_disabled)
-   return 0;
+   return NULL;
 
if (ring != &dev_priv->ring[RCS])
-   return 0;
+   return NULL;
 
if (to_id == DEFAULT_CONTEXT_ID) {
to = ring->default_context;
} else {
if (file == NULL)
-   return -EINVAL;
+   return ERR_PTR(-EINVAL);
 
to = i915_gem_context_get(file->driver_priv, to_id);
if (to == NULL)
-   return -ENOENT;
+   return ERR_PTR(-ENOENT);
}
 
-   return do_switch(to);
+   ret = do_switch(to);
+   if (ret)
+   return ERR_PTR(ret);
+
+   return to;
 }
 
 int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index ea963c3..ad7029e 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -838,6 +838,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
struct drm_i915_gem_object *batch_obj;
struct drm_clip_rect *cliprects = NULL;
struct intel_ring_buffer *ring;
+  

[Intel-gfx] [PATCH v2 03/16] drm/i915: reference count for i915_hw_contexts

[Mika Kuoppala]

In preparation to do analysis of which context was
guilty of gpu hung, store kreffed context pointer
into request struct.

This allows us to inspect contexts when gpu is reset
even if those contexts would already be released
by userspace.

v2: track i915_hw_context pointers instead of using ctx_ids
(from Chris Wilson)

Signed-off-by: Mika Kuoppala 
---
 drivers/gpu/drm/i915/i915_drv.h|   10 --
 drivers/gpu/drm/i915/i915_gem.c|   16 +++-
 drivers/gpu/drm/i915/i915_gem_context.c|   22 ++
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |7 ---
 4 files changed, 45 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 930b0ac..79cd27f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -437,6 +437,7 @@ struct i915_hw_ppgtt {
 /* This must match up with the value previously used for execbuf2.rsvd1. */
 #define DEFAULT_CONTEXT_ID 0
 struct i915_hw_context {
+   struct kref ref;
int id;
bool is_initialized;
struct drm_i915_file_private *file_priv;
@@ -1239,6 +1240,9 @@ struct drm_i915_gem_request {
/** Postion in the ringbuffer of the end of the request */
u32 tail;
 
+   /** Context related to this request */
+   struct i915_hw_context *ctx;
+
/** Time at which this request was emitted, in jiffies. */
unsigned long emitted_jiffies;
 
@@ -1628,9 +1632,10 @@ int __must_check i915_gpu_idle(struct drm_device *dev);
 int __must_check i915_gem_idle(struct drm_device *dev);
 int i915_do_add_request(struct intel_ring_buffer *ring,
u32 *seqno,
-   struct drm_file *file);
+   struct drm_file *file,
+   struct i915_hw_context *ctx);
 #define i915_add_request(ring, seqno) \
-   i915_do_add_request(ring, seqno, NULL)
+   i915_do_add_request(ring, seqno, NULL, NULL)
 int __must_check i915_wait_seqno(struct intel_ring_buffer *ring,
 uint32_t seqno);
 int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf);
@@ -1674,6 +1679,7 @@ void i915_gem_context_close(struct drm_device *dev, 
struct drm_file *file);
 struct i915_hw_context * __must_check
 i915_switch_context(struct intel_ring_buffer *ring,
struct drm_file *file, int to_id);
+void i915_gem_context_free(struct kref *ctx_ref);
 int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
  struct drm_file *file);
 int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 105ad4b..e905086 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1997,7 +1997,8 @@ i915_gem_get_seqno(struct drm_device *dev, u32 *seqno)
 int
 i915_do_add_request(struct intel_ring_buffer *ring,
u32 *out_seqno,
-   struct drm_file *file)
+   struct drm_file *file,
+   struct i915_hw_context *ctx)
 {
drm_i915_private_t *dev_priv = ring->dev->dev_private;
struct drm_i915_gem_request *request;
@@ -2037,6 +2038,11 @@ i915_do_add_request(struct intel_ring_buffer *ring,
request->seqno = intel_ring_get_seqno(ring);
request->ring = ring;
request->tail = request_ring_position;
+   request->ctx = ctx;
+
+   if (request->ctx)
+   kref_get(&request->ctx->ref);
+
request->emitted_jiffies = jiffies;
was_empty = list_empty(&ring->request_list);
list_add_tail(&request->list, &ring->request_list);
@@ -2101,6 +2107,10 @@ static void i915_gem_reset_ring_lists(struct 
drm_i915_private *dev_priv,
 
list_del(&request->list);
i915_gem_request_remove_from_client(request);
+
+   if (request->ctx)
+   kref_put(&request->ctx->ref, i915_gem_context_free);
+
kfree(request);
}
 
@@ -2195,6 +2205,10 @@ i915_gem_retire_requests_ring(struct intel_ring_buffer 
*ring)
 
list_del(&request->list);
i915_gem_request_remove_from_client(request);
+
+   if (request->ctx)
+   kref_put(&request->ctx->ref, i915_gem_context_free);
+
kfree(request);
}
 
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c 
b/drivers/gpu/drm/i915/i915_gem_context.c
index 258e5e0..8fb4d3c 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -124,12 +124,24 @@ static int get_context_size(struct drm_device *dev)
return ret;
 }
 
-static void do_destroy(struct i915_hw_context *ctx)
+void i915_gem_context_free(struct kref *ctx_ref)
+{
+   struct i915_hw_context *ctx = container_of(ctx_ref,
+ 

[Intel-gfx] Regression: HSW eDP broken on 3.9-rc[12]

[Takashi Iwai]

Hi,

I noticed that a Haswell desktop machine with eDP gets only the blank
screen on the latest Linus tree.  It works fine with 3.8, so it's a
regression since 3.9-rc1.  Actually, it's not a regression.  It's
three regressions in a shot! (I had to do painful bisections three
times...)

c464b2a17c59adedbdc02cc54341d630354edc3
drm/i915: set TRANSCODER_EDP even earlier

d6dd9eb1d96d2b7345fe4664066c2b7ed86da898
drm/i915: dynamic Haswell display power well support

commit cf0a6584aa6d382f802f2c3cacac23ccbccde0cd
drm/i915: write backlight harder


The first commit affects no matter whether power well is on or off.
It brings the eDP output always blank.

The next one has to be reverted completely, too.  The power well
doesn't seem working correctly after all.

At this point, you'll get a blank screen only after the boot until X
starts, at least.

The blank screen on fb after boot is due to the third commit, which is
actually my patch.  The patch has been never tested on the machines
where the original fix was targeted, so this is a clearly a wrong
regression fix :-<  Judging from increasing number of bug reports, we
should revert this one again and fix properly for the Dell laptop.
At least, many HP and Lenovo machines suffer from this commit.  The
details are found at:
https://bugzilla.kernel.org/show_bug.cgi?id=47941

Instead of reverting, a patch like below fixes the issue, too, but I
guess this will break Dell machine again, if I understand correctly.

A good news is that now I have a machine for a while so that I can
test any patch for the third, broken backlight, problem.


thanks,

Takashi

---
diff --git a/drivers/gpu/drm/i915/intel_panel.c 
b/drivers/gpu/drm/i915/intel_panel.c
index a3730e0..99ef9a3 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -317,10 +317,13 @@ void intel_panel_enable_backlight(struct drm_device *dev,
  enum pipe pipe)
 {
struct drm_i915_private *dev_priv = dev->dev_private;
+   int prev_level;
 
if (dev_priv->backlight_level == 0)
dev_priv->backlight_level = intel_panel_get_max_backlight(dev);
 
+   prev_level = intel_panel_get_backlight(dev);
+
dev_priv->backlight_enabled = true;
intel_panel_actually_set_backlight(dev, dev_priv->backlight_level);
 
@@ -363,7 +366,7 @@ set_level:
 * On some machines, BLC_PWM_CPU_CTL is cleared to zero automatically
 * when BLC_PWM_CPU_CTL2 and BLC_PWM_PCH_CTL1 are written.
 */
-   if (!intel_panel_get_backlight(dev))
+   if (!prev_level && dev_priv->backlight_level)
intel_panel_actually_set_backlight(dev, 
dev_priv->backlight_level);
 }
 
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH] drm/i915: Sanity check incoming ioctl data for a NULL pointer

[Chris Wilson]

In order to prevent a potential NULL deference with hostile userspace,
we need to check whether the ioctl was passed an invalid args pointer.

Reported-by: Tommi Rantala 
Link: 
http://lkml.kernel.org/r/ca+ydwtpubvbwxbt-tdgpuvj1eu7itmcho_2b3w13hkd5+jw...@mail.gmail.com
Signed-off-by: Chris Wilson 
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c 
b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 365e41a..9f5602e 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1103,7 +1103,11 @@ i915_gem_execbuffer(struct drm_device *dev, void *data,
struct drm_i915_gem_exec_object2 *exec2_list = NULL;
int ret, i;
 
-   if (args->buffer_count < 1) {
+   if (args == NULL)
+   return -EINVAL;
+
+   if (args->buffer_count < 1 ||
+   args->buffer_count > INT_MAX / sizeof(*exec2_list)) {
DRM_DEBUG("execbuf with %d buffers\n", args->buffer_count);
return -EINVAL;
}
@@ -1182,8 +1186,11 @@ i915_gem_execbuffer2(struct drm_device *dev, void *data,
struct drm_i915_gem_exec_object2 *exec2_list = NULL;
int ret;
 
+   if (args == NULL)
+   return -EINVAL;
+
if (args->buffer_count < 1 ||
-   args->buffer_count > UINT_MAX / sizeof(*exec2_list)) {
+   args->buffer_count > INT_MAX / sizeof(*exec2_list)) {
DRM_DEBUG("execbuf2 with %d buffers\n", args->buffer_count);
return -EINVAL;
}
-- 
1.7.10.4

___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 4/9] drm/i915: Don't touch South display when PCH_NOP

[Jani Nikula]

On Wed, 13 Mar 2013, Ben Widawsky  wrote:
> Interrupts, clock gating, and gmbus are all within the, "this will hang
> the CPU" range when we have PCH_NOP.
>
> There is a bit of a hack in init clock gating. We want to do most of the
> block gating, but the part we skip will hang the system. It could
> probably be abstracted a bit better, but I don't feel it's too
> unsightly.
>
> Signed-off-by: Ben Widawsky 
> ---
>  drivers/gpu/drm/i915/i915_irq.c   | 20 ++--
>  drivers/gpu/drm/i915/intel_bios.c |  3 +++
>  drivers/gpu/drm/i915/intel_i2c.c  |  4 +++-
>  drivers/gpu/drm/i915/intel_pm.c   |  3 ++-
>  4 files changed, 22 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index b860f0b..ba8ada3 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -738,14 +738,16 @@ static irqreturn_t ivybridge_irq_handler(int irq, void 
> *arg)
>   }
>   }
>  
> - /* check event from PCH */
> - if (de_iir & DE_PCH_EVENT_IVB) {
> - u32 pch_iir = I915_READ(SDEIIR);
> + if (HAS_PCH_NOP(dev)) {

!HAS_PCH_NOP() ?

BR,
Jani.

> + /* check event from PCH */
> + if (de_iir & DE_PCH_EVENT_IVB) {
> + u32 pch_iir = I915_READ(SDEIIR);
>  
> - cpt_irq_handler(dev, pch_iir);
> + cpt_irq_handler(dev, pch_iir);
>  
> - /* clear PCH hotplug event before clear CPU irq */
> - I915_WRITE(SDEIIR, pch_iir);
> + /* clear PCH hotplug event before CPU irq */
> + I915_WRITE(SDEIIR, pch_iir);
> + }
>   }
>  
>   I915_WRITE(DEIIR, de_iir);
> @@ -1910,6 +1912,9 @@ static void ironlake_irq_preinstall(struct drm_device 
> *dev)
>   I915_WRITE(GTIER, 0x0);
>   POSTING_READ(GTIER);
>  
> + if (HAS_PCH_NOP(dev))
> + return;
> +
>   /* south display irq */
>   I915_WRITE(SDEIMR, 0x);
>   I915_WRITE(SDEIER, 0x0);
> @@ -1982,6 +1987,9 @@ static void ibx_irq_postinstall(struct drm_device *dev)
>  SDE_GMBUS_CPT |
>  SDE_AUX_MASK_CPT;
>  
> + if (HAS_PCH_NOP(dev))
> + return;
> +
>   I915_WRITE(SDEIIR, I915_READ(SDEIIR));
>   I915_WRITE(SDEIMR, ~mask);
>   I915_WRITE(SDEIER, mask);
> diff --git a/drivers/gpu/drm/i915/intel_bios.c 
> b/drivers/gpu/drm/i915/intel_bios.c
> index 55ffba1..194df27 100644
> --- a/drivers/gpu/drm/i915/intel_bios.c
> +++ b/drivers/gpu/drm/i915/intel_bios.c
> @@ -692,6 +692,9 @@ intel_parse_bios(struct drm_device *dev)
>   struct bdb_header *bdb = NULL;
>   u8 __iomem *bios = NULL;
>  
> + if (HAS_PCH_NOP(dev))
> + return -ENODEV;
> +
>   init_vbt_defaults(dev_priv);
>  
>   /* XXX Should this validation be moved to intel_opregion.c? */
> diff --git a/drivers/gpu/drm/i915/intel_i2c.c 
> b/drivers/gpu/drm/i915/intel_i2c.c
> index acf8aec..fc19e49 100644
> --- a/drivers/gpu/drm/i915/intel_i2c.c
> +++ b/drivers/gpu/drm/i915/intel_i2c.c
> @@ -513,7 +513,9 @@ int intel_setup_gmbus(struct drm_device *dev)
>   struct drm_i915_private *dev_priv = dev->dev_private;
>   int ret, i;
>  
> - if (HAS_PCH_SPLIT(dev))
> + if (HAS_PCH_NOP(dev))
> + return 0;
> + else if (HAS_PCH_SPLIT(dev))
>   dev_priv->gpio_mmio_base = PCH_GPIOA - GPIOA;
>   else if (IS_VALLEYVIEW(dev))
>   dev_priv->gpio_mmio_base = VLV_DISPLAY_BASE;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 5479363..52203fd 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3874,7 +3874,8 @@ static void ivybridge_init_clock_gating(struct 
> drm_device *dev)
>   snpcr |= GEN6_MBC_SNPCR_MED;
>   I915_WRITE(GEN6_MBCUNIT_SNPCR, snpcr);
>  
> - cpt_init_clock_gating(dev);
> + if (HAS_PCH_NOP(dev))
> + cpt_init_clock_gating(dev);
>  
>   gen6_check_mch_setup(dev);
>  }
> -- 
> 1.8.1.5
>
> ___
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/4] drm/i915: Remove unused file arg from execbuf

[Chris Wilson]

I was hoping for something a little more magical. I couldn't spot any
mistakes - the only thing missing is a hint as to why these are useful.

Acked-by: Chris Wilson 
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx