[PATCH] drm/vgem: Allow root to inject hanging fences onto dmabufs

[PATCH] drm/vgem: Allow root to inject hanging fences onto dmabufs

[Chris Wilson]

When performing driver testing, one factor we want to test is how we
handle a foreign fence that is never signaled. We can wait on that fence
indefinitely, in which case the driver appears hung, or we can take some
remedial action (with risks regarding the state of any shared content).
Whatever the action choosen by the driver, in order to perform the test
we want to disable the safety feature of vgem fence (which is then used
to test implicit dma-buf fencing). This is regarded as a highly
dangerous feature and so hidden behind an expert config option and only
available to root when enabled.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/Kconfig           | 13 +++++++++++++
 drivers/gpu/drm/vgem/vgem_fence.c | 14 ++++++++++++--
 include/uapi/drm/vgem_drm.h       |  1 +
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index fc357319de35..2b25bc38fad2 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -228,6 +228,19 @@ config DRM_VGEM
 	  as used by Mesa's software renderer for enhanced performance.
 	  If M is selected the module will be called vgem.
 
+config DRM_VGEM_FENCE_HANG
+	bool "Enable fence hang testing (DANGEROUS)"
+	depends on DRM_VGEM
+	depends on EXPERT
+	depends on !COMPILE_TEST
+	default n
+	help
+	  Enables support for root to use indefinite fences.
+	  Do not enable this unless you are testing DRM drivers.
+
+	  Recommended for driver developers only.
+
+	  If in doubt, say "N".
 
 source "drivers/gpu/drm/exynos/Kconfig"
 
diff --git a/drivers/gpu/drm/vgem/vgem_fence.c b/drivers/gpu/drm/vgem/vgem_fence.c
index 5c57c1ffa1f9..91d3d75fc9c5 100644
--- a/drivers/gpu/drm/vgem/vgem_fence.c
+++ b/drivers/gpu/drm/vgem/vgem_fence.c
@@ -107,7 +107,8 @@ static struct fence *vgem_fence_create(struct vgem_file *vfile,
 	setup_timer(&fence->timer, vgem_fence_timeout, (unsigned long)fence);
 
 	/* We force the fence to expire within 10s to prevent driver hangs */
-	mod_timer(&fence->timer, jiffies + VGEM_FENCE_TIMEOUT);
+	if ((flags & VGEM_FENCE_NOTIMEOUT) == 0)
+		mod_timer(&fence->timer, jiffies + VGEM_FENCE_TIMEOUT);
 
 	return &fence->base;
 }
@@ -160,9 +161,18 @@ int vgem_fence_attach_ioctl(struct drm_device *dev,
 	struct fence *fence;
 	int ret;
 
-	if (arg->flags & ~VGEM_FENCE_WRITE)
+	if (arg->flags & ~(VGEM_FENCE_WRITE | VGEM_FENCE_NOTIMEOUT))
 		return -EINVAL;
 
+	if (arg->flags & VGEM_FENCE_NOTIMEOUT) {
+		if (config_enabled(CONFIG_DRM_VGEM_FENCE_HANG)) {
+			if (!capable(CAP_SYS_ADMIN))
+				return -EPERM;
+		} else {
+			return -EINVAL;
+		}
+	}
+
 	if (arg->pad)
 		return -EINVAL;
 
diff --git a/include/uapi/drm/vgem_drm.h b/include/uapi/drm/vgem_drm.h
index bf66f5db6da8..55fd08750773 100644
--- a/include/uapi/drm/vgem_drm.h
+++ b/include/uapi/drm/vgem_drm.h
@@ -46,6 +46,7 @@ struct drm_vgem_fence_attach {
 	__u32 handle;
 	__u32 flags;
 #define VGEM_FENCE_WRITE	0x1
+#define VGEM_FENCE_NOTIMEOUT	0x2
 	__u32 out_fence;
 	__u32 pad;
 };
-- 
2.8.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/vgem: Allow root to inject hanging fences onto dmabufs

[Kristian Høgsberg]

Why is this useful if only root can use it?

Kristian

On Wed, Jul 20, 2016 at 12:39 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> When performing driver testing, one factor we want to test is how we
> handle a foreign fence that is never signaled. We can wait on that fence
> indefinitely, in which case the driver appears hung, or we can take some
> remedial action (with risks regarding the state of any shared content).
> Whatever the action choosen by the driver, in order to perform the test
> we want to disable the safety feature of vgem fence (which is then used
> to test implicit dma-buf fencing). This is regarded as a highly
> dangerous feature and so hidden behind an expert config option and only
> available to root when enabled.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/Kconfig           | 13 +++++++++++++
>  drivers/gpu/drm/vgem/vgem_fence.c | 14 ++++++++++++--
>  include/uapi/drm/vgem_drm.h       |  1 +
>  3 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index fc357319de35..2b25bc38fad2 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -228,6 +228,19 @@ config DRM_VGEM
>           as used by Mesa's software renderer for enhanced performance.
>           If M is selected the module will be called vgem.
>
> +config DRM_VGEM_FENCE_HANG
> +       bool "Enable fence hang testing (DANGEROUS)"
> +       depends on DRM_VGEM
> +       depends on EXPERT
> +       depends on !COMPILE_TEST
> +       default n
> +       help
> +         Enables support for root to use indefinite fences.
> +         Do not enable this unless you are testing DRM drivers.
> +
> +         Recommended for driver developers only.
> +
> +         If in doubt, say "N".
>
>  source "drivers/gpu/drm/exynos/Kconfig"
>
> diff --git a/drivers/gpu/drm/vgem/vgem_fence.c b/drivers/gpu/drm/vgem/vgem_fence.c
> index 5c57c1ffa1f9..91d3d75fc9c5 100644
> --- a/drivers/gpu/drm/vgem/vgem_fence.c
> +++ b/drivers/gpu/drm/vgem/vgem_fence.c
> @@ -107,7 +107,8 @@ static struct fence *vgem_fence_create(struct vgem_file *vfile,
>         setup_timer(&fence->timer, vgem_fence_timeout, (unsigned long)fence);
>
>         /* We force the fence to expire within 10s to prevent driver hangs */
> -       mod_timer(&fence->timer, jiffies + VGEM_FENCE_TIMEOUT);
> +       if ((flags & VGEM_FENCE_NOTIMEOUT) == 0)
> +               mod_timer(&fence->timer, jiffies + VGEM_FENCE_TIMEOUT);
>
>         return &fence->base;
>  }
> @@ -160,9 +161,18 @@ int vgem_fence_attach_ioctl(struct drm_device *dev,
>         struct fence *fence;
>         int ret;
>
> -       if (arg->flags & ~VGEM_FENCE_WRITE)
> +       if (arg->flags & ~(VGEM_FENCE_WRITE | VGEM_FENCE_NOTIMEOUT))
>                 return -EINVAL;
>
> +       if (arg->flags & VGEM_FENCE_NOTIMEOUT) {
> +               if (config_enabled(CONFIG_DRM_VGEM_FENCE_HANG)) {
> +                       if (!capable(CAP_SYS_ADMIN))
> +                               return -EPERM;
> +               } else {
> +                       return -EINVAL;
> +               }
> +       }
> +
>         if (arg->pad)
>                 return -EINVAL;
>
> diff --git a/include/uapi/drm/vgem_drm.h b/include/uapi/drm/vgem_drm.h
> index bf66f5db6da8..55fd08750773 100644
> --- a/include/uapi/drm/vgem_drm.h
> +++ b/include/uapi/drm/vgem_drm.h
> @@ -46,6 +46,7 @@ struct drm_vgem_fence_attach {
>         __u32 handle;
>         __u32 flags;
>  #define VGEM_FENCE_WRITE       0x1
> +#define VGEM_FENCE_NOTIMEOUT   0x2
>         __u32 out_fence;
>         __u32 pad;
>  };
> --
> 2.8.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/vgem: Allow root to inject hanging fences onto dmabufs

[Chris Wilson]

On Wed, Jul 20, 2016 at 04:28:40PM -0700, Kristian Høgsberg wrote:
> Why is this useful if only root can use it?

> > When performing driver testing, one factor we want to test is how we
> > handle a foreign fence that is never signaled. We can wait on that fence
> > indefinitely, in which case the driver appears hung, or we can take some
> > remedial action (with risks regarding the state of any shared content).
> > Whatever the action choosen by the driver, in order to perform the test
> > we want to disable the safety feature of vgem fence (which is then used
> > to test implicit dma-buf fencing). This is regarded as a highly
> > dangerous feature and so hidden behind an expert config option and only
> > available to root when enabled.

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

✗ Ro.CI.BAT: failure for drm/vgem: Allow root to inject hanging fences onto dmabufs

[Patchwork]

== Series Details ==

Series: drm/vgem: Allow root to inject hanging fences onto dmabufs
URL   : https://patchwork.freedesktop.org/series/10107/
State : failure

== Summary ==

Series 10107v1 drm/vgem: Allow root to inject hanging fences onto dmabufs
http://patchwork.freedesktop.org/api/1.0/series/10107/revisions/1/mbox

Test gem_exec_suspend:
        Subgroup basic-s3:
                pass       -> INCOMPLETE (fi-skl-i7-6700k)

fi-hsw-i7-4770k  total:244  pass:216  dwarn:0   dfail:0   fail:8   skip:20 
fi-kbl-qkkr      total:244  pass:180  dwarn:29  dfail:0   fail:8   skip:27 
fi-skl-i5-6260u  total:244  pass:224  dwarn:0   dfail:0   fail:8   skip:12 
fi-skl-i7-6700k  total:107  pass:84   dwarn:0   dfail:0   fail:0   skip:22 
ro-bdw-i5-5250u  total:244  pass:219  dwarn:4   dfail:0   fail:8   skip:13 
ro-bdw-i7-5600u  total:244  pass:204  dwarn:0   dfail:0   fail:8   skip:32 
ro-bsw-n3050     total:218  pass:173  dwarn:0   dfail:0   fail:2   skip:42 
ro-byt-n2820     total:244  pass:197  dwarn:0   dfail:0   fail:9   skip:38 
ro-hsw-i3-4010u  total:244  pass:212  dwarn:0   dfail:0   fail:8   skip:24 
ro-hsw-i7-4770r  total:244  pass:212  dwarn:0   dfail:0   fail:8   skip:24 
ro-ilk-i7-620lm  total:244  pass:172  dwarn:0   dfail:0   fail:9   skip:63 
ro-ilk1-i5-650   total:239  pass:172  dwarn:0   dfail:0   fail:9   skip:58 
ro-ivb-i7-3770   total:244  pass:203  dwarn:0   dfail:0   fail:8   skip:33 
ro-skl3-i5-6260u total:244  pass:224  dwarn:0   dfail:0   fail:8   skip:12 
ro-snb-i7-2620M  total:244  pass:193  dwarn:0   dfail:0   fail:9   skip:42 
ro-bdw-i7-5557U failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1553/

621e9dc drm-intel-nightly: 2016y-07m-20d-19h-19m-37s UTC integration manifest
6ae4173 drm/vgem: Allow root to inject hanging fences onto dmabufs

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/vgem: Allow root to inject hanging fences onto dmabufs

[Chris Wilson]

On Wed, Jul 20, 2016 at 08:39:43PM +0100, Chris Wilson wrote:
> When performing driver testing, one factor we want to test is how we
> handle a foreign fence that is never signaled. We can wait on that fence
> indefinitely, in which case the driver appears hung, or we can take some
> remedial action (with risks regarding the state of any shared content).
> Whatever the action choosen by the driver, in order to perform the test
> we want to disable the safety feature of vgem fence (which is then used
> to test implicit dma-buf fencing). This is regarded as a highly
> dangerous feature and so hidden behind an expert config option and only
> available to root when enabled.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Ping. This is invaluable for my testing (some tests are impossible
without it).

The only comment was by Kristian asking why this should be root-only
which nicely contrasts the earlier *demand* that this should be root-only
and removed from the default set of vgem capabilities.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm/vgem: Allow root to inject hanging fences onto dmabufs

[Daniel Vetter]

On Fri, Nov 18, 2016 at 08:49:37AM +0000, Chris Wilson wrote:
> On Wed, Jul 20, 2016 at 08:39:43PM +0100, Chris Wilson wrote:
> > When performing driver testing, one factor we want to test is how we
> > handle a foreign fence that is never signaled. We can wait on that fence
> > indefinitely, in which case the driver appears hung, or we can take some
> > remedial action (with risks regarding the state of any shared content).
> > Whatever the action choosen by the driver, in order to perform the test
> > we want to disable the safety feature of vgem fence (which is then used
> > to test implicit dma-buf fencing). This is regarded as a highly
> > dangerous feature and so hidden behind an expert config option and only
> > available to root when enabled.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Ping. This is invaluable for my testing (some tests are impossible
> without it).

Hm, I thought I voted for a module option (so that tests can frob at
runtime) that auto-taints the kernel. And then maybe always allow it. I
just don't like when Kconfig settings can result in testcase skips, makes
it uncertain whether QA did QA or not.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/vgem: Allow root to inject hanging fences onto dmabufs

[Chris Wilson]

On Fri, Nov 18, 2016 at 10:40:02AM +0100, Daniel Vetter wrote:
> On Fri, Nov 18, 2016 at 08:49:37AM +0000, Chris Wilson wrote:
> > On Wed, Jul 20, 2016 at 08:39:43PM +0100, Chris Wilson wrote:
> > > When performing driver testing, one factor we want to test is how we
> > > handle a foreign fence that is never signaled. We can wait on that fence
> > > indefinitely, in which case the driver appears hung, or we can take some
> > > remedial action (with risks regarding the state of any shared content).
> > > Whatever the action choosen by the driver, in order to perform the test
> > > we want to disable the safety feature of vgem fence (which is then used
> > > to test implicit dma-buf fencing). This is regarded as a highly
> > > dangerous feature and so hidden behind an expert config option and only
> > > available to root when enabled.
> > > 
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > 
> > Ping. This is invaluable for my testing (some tests are impossible
> > without it).
> 
> Hm, I thought I voted for a module option (so that tests can frob at
> runtime) that auto-taints the kernel. And then maybe always allow it. I
> just don't like when Kconfig settings can result in testcase skips, makes
> it uncertain whether QA did QA or not.

We know when it is disabled, and so when it skips. We also control the
kconfig set by QA. There was very strong objections to having yet
another dangling fence that I thought making it available in default
builds was not desired.
-Chris

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