[Intel-gfx] [PATCH] drm/vgem: Allow root to inject hanging fences onto dmabufs
On Fri, Nov 18, 2016 at 08:49:37AM +, 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 > > 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] [PATCH] drm/vgem: Allow root to inject hanging fences onto dmabufs
On Fri, Nov 18, 2016 at 10:40:02AM +0100, Daniel Vetter wrote: > On Fri, Nov 18, 2016 at 08:49:37AM +, 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 > > > > 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
[PATCH] drm/vgem: Allow root to inject hanging fences onto dmabufs
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 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
[PATCH] drm/vgem: Allow root to inject hanging fences onto dmabufs
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
[PATCH] drm/vgem: Allow root to inject hanging fences onto dmabufs
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
---
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(>timer, vgem_fence_timeout, (unsigned long)fence);
/* We force the fence to expire within 10s to prevent driver hangs */
- mod_timer(>timer, jiffies + VGEM_FENCE_TIMEOUT);
+ if ((flags & VGEM_FENCE_NOTIMEOUT) == 0)
+ mod_timer(>timer, jiffies + VGEM_FENCE_TIMEOUT);
return >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
[PATCH] drm/vgem: Allow root to inject hanging fences onto dmabufs
Why is this useful if only root can use it?
Kristian
On Wed, Jul 20, 2016 at 12:39 PM, 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
> ---
> 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(>timer, vgem_fence_timeout, (unsigned long)fence);
>
> /* We force the fence to expire within 10s to prevent driver hangs */
> - mod_timer(>timer, jiffies + VGEM_FENCE_TIMEOUT);
> + if ((flags & VGEM_FENCE_NOTIMEOUT) == 0)
> + mod_timer(>timer, jiffies + VGEM_FENCE_TIMEOUT);
>
> return >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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
