* [PATCH] drm/i915: Enforce execobject.alignment to be a power-of-two
@ 2015-06-19 12:59 Chris Wilson
2015-06-22 11:03 ` Damien Lespiau
0 siblings, 1 reply; 7+ messages in thread
From: Chris Wilson @ 2015-06-19 12:59 UTC (permalink / raw
To: intel-gfx
Internal requirement for the alignment is that it must be a
power-of-two, so enforce rejection at the user interface to execbuffer
(which allows the caller to specify a stricter-than-expected alignment
criterion).
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index f4fb2bd33753..b14733908d8d 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1189,6 +1189,9 @@ validate_exec_list(const struct drm_i915_gem_execbuffer2 *args,
if (exec[i].flags & invalid_flags)
return -EINVAL;
+ if (exec[i].alignment && !is_power_of_2(exec[i].alignment))
+ return -EINVAL;
+
/* pad_to_size was once a reserved field, so sanitize it */
if (exec[i].flags & EXEC_OBJECT_PAD_TO_SIZE) {
if (offset_in_page(exec[i].pad_to_size))
--
2.1.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Enforce execobject.alignment to be a power-of-two
2015-06-19 12:59 [PATCH] drm/i915: Enforce execobject.alignment to be a power-of-two Chris Wilson
@ 2015-06-22 11:03 ` Damien Lespiau
2015-06-22 14:08 ` Daniel Vetter
0 siblings, 1 reply; 7+ messages in thread
From: Damien Lespiau @ 2015-06-22 11:03 UTC (permalink / raw
To: Chris Wilson; +Cc: intel-gfx
On Fri, Jun 19, 2015 at 01:59:46PM +0100, Chris Wilson wrote:
> Internal requirement for the alignment is that it must be a
> power-of-two, so enforce rejection at the user interface to execbuffer
> (which allows the caller to specify a stricter-than-expected alignment
> criterion).
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
That sounds right in general and I could find at least one instance
where we rely on alignment being a power of two (eb_vma_misplaced() and
vma->node.start & (entry->alignment - 1))
so:
Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
--
Damien
> ---
> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index f4fb2bd33753..b14733908d8d 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1189,6 +1189,9 @@ validate_exec_list(const struct drm_i915_gem_execbuffer2 *args,
> if (exec[i].flags & invalid_flags)
> return -EINVAL;
>
> + if (exec[i].alignment && !is_power_of_2(exec[i].alignment))
> + return -EINVAL;
> +
> /* pad_to_size was once a reserved field, so sanitize it */
> if (exec[i].flags & EXEC_OBJECT_PAD_TO_SIZE) {
> if (offset_in_page(exec[i].pad_to_size))
> --
> 2.1.4
>
> _______________________________________________
> 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Enforce execobject.alignment to be a power-of-two
2015-06-22 11:03 ` Damien Lespiau
@ 2015-06-22 14:08 ` Daniel Vetter
2015-06-22 14:28 ` Chris Wilson
0 siblings, 1 reply; 7+ messages in thread
From: Daniel Vetter @ 2015-06-22 14:08 UTC (permalink / raw
To: Damien Lespiau; +Cc: intel-gfx
On Mon, Jun 22, 2015 at 12:03:08PM +0100, Damien Lespiau wrote:
> On Fri, Jun 19, 2015 at 01:59:46PM +0100, Chris Wilson wrote:
> > Internal requirement for the alignment is that it must be a
> > power-of-two, so enforce rejection at the user interface to execbuffer
> > (which allows the caller to specify a stricter-than-expected alignment
> > criterion).
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> That sounds right in general and I could find at least one instance
> where we rely on alignment being a power of two (eb_vma_misplaced() and
> vma->node.start & (entry->alignment - 1))
>
> so:
>
> Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
Needs a nasty igt I think ... Do we have? Applied meanwhile.
-Daniel
>
> --
> Damien
>
> > ---
> > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > index f4fb2bd33753..b14733908d8d 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> > @@ -1189,6 +1189,9 @@ validate_exec_list(const struct drm_i915_gem_execbuffer2 *args,
> > if (exec[i].flags & invalid_flags)
> > return -EINVAL;
> >
> > + if (exec[i].alignment && !is_power_of_2(exec[i].alignment))
> > + return -EINVAL;
> > +
> > /* pad_to_size was once a reserved field, so sanitize it */
> > if (exec[i].flags & EXEC_OBJECT_PAD_TO_SIZE) {
> > if (offset_in_page(exec[i].pad_to_size))
> > --
> > 2.1.4
> >
> > _______________________________________________
> > 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
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Enforce execobject.alignment to be a power-of-two
2015-06-22 14:08 ` Daniel Vetter
@ 2015-06-22 14:28 ` Chris Wilson
2015-06-22 14:56 ` Chris Wilson
2015-06-22 15:32 ` Daniel Vetter
0 siblings, 2 replies; 7+ messages in thread
From: Chris Wilson @ 2015-06-22 14:28 UTC (permalink / raw
To: Daniel Vetter; +Cc: intel-gfx
On Mon, Jun 22, 2015 at 04:08:20PM +0200, Daniel Vetter wrote:
> On Mon, Jun 22, 2015 at 12:03:08PM +0100, Damien Lespiau wrote:
> > On Fri, Jun 19, 2015 at 01:59:46PM +0100, Chris Wilson wrote:
> > > Internal requirement for the alignment is that it must be a
> > > power-of-two, so enforce rejection at the user interface to execbuffer
> > > (which allows the caller to specify a stricter-than-expected alignment
> > > criterion).
> > >
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >
> > That sounds right in general and I could find at least one instance
> > where we rely on alignment being a power of two (eb_vma_misplaced() and
> > vma->node.start & (entry->alignment - 1))
> >
> > so:
> >
> > Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
>
> Needs a nasty igt I think ... Do we have? Applied meanwhile.
Sure, we can demonstrate a bug in the current code that would not
realign an object to the arbitrary alignment requested by the user.
-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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Enforce execobject.alignment to be a power-of-two
2015-06-22 14:28 ` Chris Wilson
@ 2015-06-22 14:56 ` Chris Wilson
2015-06-22 15:32 ` Daniel Vetter
1 sibling, 0 replies; 7+ messages in thread
From: Chris Wilson @ 2015-06-22 14:56 UTC (permalink / raw
To: Daniel Vetter, Damien Lespiau, intel-gfx
On Mon, Jun 22, 2015 at 03:28:26PM +0100, Chris Wilson wrote:
> On Mon, Jun 22, 2015 at 04:08:20PM +0200, Daniel Vetter wrote:
> > On Mon, Jun 22, 2015 at 12:03:08PM +0100, Damien Lespiau wrote:
> > > On Fri, Jun 19, 2015 at 01:59:46PM +0100, Chris Wilson wrote:
> > > > Internal requirement for the alignment is that it must be a
> > > > power-of-two, so enforce rejection at the user interface to execbuffer
> > > > (which allows the caller to specify a stricter-than-expected alignment
> > > > criterion).
> > > >
> > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > >
> > > That sounds right in general and I could find at least one instance
> > > where we rely on alignment being a power of two (eb_vma_misplaced() and
> > > vma->node.start & (entry->alignment - 1))
> > >
> > > so:
> > >
> > > Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
> >
> > Needs a nasty igt I think ... Do we have? Applied meanwhile.
>
> Sure, we can demonstrate a bug in the current code that would not
> realign an object to the arbitrary alignment requested by the user.
igt/gem_exec_alignment exercises said bug.
-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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Enforce execobject.alignment to be a power-of-two
2015-06-22 14:28 ` Chris Wilson
2015-06-22 14:56 ` Chris Wilson
@ 2015-06-22 15:32 ` Daniel Vetter
2015-06-22 16:09 ` Chris Wilson
1 sibling, 1 reply; 7+ messages in thread
From: Daniel Vetter @ 2015-06-22 15:32 UTC (permalink / raw
To: Chris Wilson, Daniel Vetter, Damien Lespiau, intel-gfx
On Mon, Jun 22, 2015 at 03:28:26PM +0100, Chris Wilson wrote:
> On Mon, Jun 22, 2015 at 04:08:20PM +0200, Daniel Vetter wrote:
> > On Mon, Jun 22, 2015 at 12:03:08PM +0100, Damien Lespiau wrote:
> > > On Fri, Jun 19, 2015 at 01:59:46PM +0100, Chris Wilson wrote:
> > > > Internal requirement for the alignment is that it must be a
> > > > power-of-two, so enforce rejection at the user interface to execbuffer
> > > > (which allows the caller to specify a stricter-than-expected alignment
> > > > criterion).
> > > >
> > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > >
> > > That sounds right in general and I could find at least one instance
> > > where we rely on alignment being a power of two (eb_vma_misplaced() and
> > > vma->node.start & (entry->alignment - 1))
> > >
> > > so:
> > >
> > > Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
> >
> > Needs a nasty igt I think ... Do we have? Applied meanwhile.
>
> Sure, we can demonstrate a bug in the current code that would not
> realign an object to the arbitrary alignment requested by the user.
Just checking that the kernel rejects non-pot alignment should be good
enough. No need imo to write a _that_ nasty igt ;-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] drm/i915: Enforce execobject.alignment to be a power-of-two
2015-06-22 15:32 ` Daniel Vetter
@ 2015-06-22 16:09 ` Chris Wilson
0 siblings, 0 replies; 7+ messages in thread
From: Chris Wilson @ 2015-06-22 16:09 UTC (permalink / raw
To: Daniel Vetter; +Cc: intel-gfx
On Mon, Jun 22, 2015 at 05:32:49PM +0200, Daniel Vetter wrote:
> On Mon, Jun 22, 2015 at 03:28:26PM +0100, Chris Wilson wrote:
> > On Mon, Jun 22, 2015 at 04:08:20PM +0200, Daniel Vetter wrote:
> > > On Mon, Jun 22, 2015 at 12:03:08PM +0100, Damien Lespiau wrote:
> > > > On Fri, Jun 19, 2015 at 01:59:46PM +0100, Chris Wilson wrote:
> > > > > Internal requirement for the alignment is that it must be a
> > > > > power-of-two, so enforce rejection at the user interface to execbuffer
> > > > > (which allows the caller to specify a stricter-than-expected alignment
> > > > > criterion).
> > > > >
> > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > >
> > > > That sounds right in general and I could find at least one instance
> > > > where we rely on alignment being a power of two (eb_vma_misplaced() and
> > > > vma->node.start & (entry->alignment - 1))
> > > >
> > > > so:
> > > >
> > > > Reviewed-by: Damien Lespiau <damien.lespiau@intel.com>
> > >
> > > Needs a nasty igt I think ... Do we have? Applied meanwhile.
> >
> > Sure, we can demonstrate a bug in the current code that would not
> > realign an object to the arbitrary alignment requested by the user.
>
> Just checking that the kernel rejects non-pot alignment should be good
> enough. No need imo to write a _that_ nasty igt ;-)
Nah, we will probably fix our pot requirement (given a usecase), so just
that the kernel fails to adhere to the user's request without throwing
an error is the bug.
-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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-06-22 16:09 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-06-19 12:59 [PATCH] drm/i915: Enforce execobject.alignment to be a power-of-two Chris Wilson
2015-06-22 11:03 ` Damien Lespiau
2015-06-22 14:08 ` Daniel Vetter
2015-06-22 14:28 ` Chris Wilson
2015-06-22 14:56 ` Chris Wilson
2015-06-22 15:32 ` Daniel Vetter
2015-06-22 16:09 ` Chris Wilson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.