| Message ID | 1436282103-5854-15-git-send-email-michel.thierry@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jul 07, 2015 at 04:14:59PM +0100, Michel Thierry wrote: > In a 48b world, users can try to allocate buffers bigger than 4GB; in > these cases it is important that size is a 64b variable. > > Also added a warning for illegal bind with size = 0. > > Signed-off-by: Michel Thierry <michel.thierry@intel.com> > --- > drivers/gpu/drm/i915/i915_gem.c | 5 +++-- > drivers/gpu/drm/i915/i915_gem_gtt.c | 3 +++ > 2 files changed, 6 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index a0bff41..ebfb789 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -3718,7 +3718,8 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, > { > struct drm_device *dev = obj->base.dev; > struct drm_i915_private *dev_priv = dev->dev_private; > - u32 size, fence_size, fence_alignment, unfenced_alignment; > + u32 fence_alignment, unfenced_alignment; > + u64 size, fence_size; > u64 start = > flags & PIN_OFFSET_BIAS ? flags & PIN_OFFSET_MASK : 0; > u64 end = > @@ -3777,7 +3778,7 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, > * attempt to find space. > */ > if (size > end) { > - DRM_DEBUG("Attempting to bind an object (view type=%u) larger than the aperture: size=%u > %s aperture=%llu\n", > + DRM_DEBUG("Attempting to bind an object (view type=%u) larger than the aperture: size=%llu > %s aperture=%llu\n", > ggtt_view ? ggtt_view->type : 0, > size, > flags & PIN_MAPPABLE ? "mappable" : "total", > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > index 449a245..900bce6 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -3312,6 +3312,9 @@ int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level, > if (WARN_ON(flags == 0)) > return -EINVAL; > > + if (WARN_ON(vma->node.size == 0)) > + return -EINVAL; This is superfluous. We don't allow size=0 object creation, and the test is better (if at all) at vma_create, but what you mean here is WARN_ON(!drm_mm_node_allocated()) which seems sensisble. And both of these would be better as ENODEV so we don't confuse the user when they get propagated back to userspace. -Chris
On 7/7/2015 4:27 PM, Chris Wilson wrote: > On Tue, Jul 07, 2015 at 04:14:59PM +0100, Michel Thierry wrote: >> In a 48b world, users can try to allocate buffers bigger than 4GB; in >> these cases it is important that size is a 64b variable. >> >> Also added a warning for illegal bind with size = 0. >> >> Signed-off-by: Michel Thierry <michel.thierry@intel.com> >> --- >> drivers/gpu/drm/i915/i915_gem.c | 5 +++-- >> drivers/gpu/drm/i915/i915_gem_gtt.c | 3 +++ >> 2 files changed, 6 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c >> index a0bff41..ebfb789 100644 >> --- a/drivers/gpu/drm/i915/i915_gem.c >> +++ b/drivers/gpu/drm/i915/i915_gem.c >> @@ -3718,7 +3718,8 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, >> { >> struct drm_device *dev = obj->base.dev; >> struct drm_i915_private *dev_priv = dev->dev_private; >> - u32 size, fence_size, fence_alignment, unfenced_alignment; >> + u32 fence_alignment, unfenced_alignment; >> + u64 size, fence_size; >> u64 start = >> flags & PIN_OFFSET_BIAS ? flags & PIN_OFFSET_MASK : 0; >> u64 end = >> @@ -3777,7 +3778,7 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, >> * attempt to find space. >> */ >> if (size > end) { >> - DRM_DEBUG("Attempting to bind an object (view type=%u) larger than the aperture: size=%u > %s aperture=%llu\n", >> + DRM_DEBUG("Attempting to bind an object (view type=%u) larger than the aperture: size=%llu > %s aperture=%llu\n", >> ggtt_view ? ggtt_view->type : 0, >> size, >> flags & PIN_MAPPABLE ? "mappable" : "total", >> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c >> index 449a245..900bce6 100644 >> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c >> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c >> @@ -3312,6 +3312,9 @@ int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level, >> if (WARN_ON(flags == 0)) >> return -EINVAL; >> >> + if (WARN_ON(vma->node.size == 0)) >> + return -EINVAL; > > This is superfluous. We don't allow size=0 object creation, and the test > is better (if at all) at vma_create, but what you mean here is > WARN_ON(!drm_mm_node_allocated()) which seems sensisble. And both of > these would be better as ENODEV so we don't confuse the user when they > get propagated back to userspace. > -Chris > My idea was to catch the node.size overflow if the variable is inadvertently changed back to u32 (which has already happen in other places). I'll change it to use drm_mm_node_allocated and return ENODEV in both places.
On Tue, Jul 07, 2015 at 04:44:30PM +0100, Michel Thierry wrote: > On 7/7/2015 4:27 PM, Chris Wilson wrote: > >On Tue, Jul 07, 2015 at 04:14:59PM +0100, Michel Thierry wrote: > >>In a 48b world, users can try to allocate buffers bigger than 4GB; in > >>these cases it is important that size is a 64b variable. > >> > >>Also added a warning for illegal bind with size = 0. > >> > >>Signed-off-by: Michel Thierry <michel.thierry@intel.com> > >>--- > >> drivers/gpu/drm/i915/i915_gem.c | 5 +++-- > >> drivers/gpu/drm/i915/i915_gem_gtt.c | 3 +++ > >> 2 files changed, 6 insertions(+), 2 deletions(-) > >> > >>diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > >>index a0bff41..ebfb789 100644 > >>--- a/drivers/gpu/drm/i915/i915_gem.c > >>+++ b/drivers/gpu/drm/i915/i915_gem.c > >>@@ -3718,7 +3718,8 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, > >> { > >> struct drm_device *dev = obj->base.dev; > >> struct drm_i915_private *dev_priv = dev->dev_private; > >>- u32 size, fence_size, fence_alignment, unfenced_alignment; > >>+ u32 fence_alignment, unfenced_alignment; > >>+ u64 size, fence_size; > >> u64 start = > >> flags & PIN_OFFSET_BIAS ? flags & PIN_OFFSET_MASK : 0; > >> u64 end = > >>@@ -3777,7 +3778,7 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, > >> * attempt to find space. > >> */ > >> if (size > end) { > >>- DRM_DEBUG("Attempting to bind an object (view type=%u) larger than the aperture: size=%u > %s aperture=%llu\n", > >>+ DRM_DEBUG("Attempting to bind an object (view type=%u) larger than the aperture: size=%llu > %s aperture=%llu\n", > >> ggtt_view ? ggtt_view->type : 0, > >> size, > >> flags & PIN_MAPPABLE ? "mappable" : "total", > >>diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > >>index 449a245..900bce6 100644 > >>--- a/drivers/gpu/drm/i915/i915_gem_gtt.c > >>+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > >>@@ -3312,6 +3312,9 @@ int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level, > >> if (WARN_ON(flags == 0)) > >> return -EINVAL; > >> > >>+ if (WARN_ON(vma->node.size == 0)) > >>+ return -EINVAL; > > > >This is superfluous. We don't allow size=0 object creation, and the test > >is better (if at all) at vma_create, but what you mean here is > >WARN_ON(!drm_mm_node_allocated()) which seems sensisble. And both of > >these would be better as ENODEV so we don't confuse the user when they > >get propagated back to userspace. > >-Chris > > > My idea was to catch the node.size overflow if the variable is > inadvertently changed back to u32 (which has already happen in other > places). Ok, that didn't come across when I just read node.size == 0 (what are chances that node.size was exactly 2^32 and then truncated?) vma->node should be fairly opaque, and if possible we want the checks in drm_mm.c - if we can think of good tests for that layer. Certainly drm_mm_reserve_node() probably wants a few sanity checks. Though most of those should fall out when it can't do the reservation the user requests. -Chris
On 7/7/2015 9:08 PM, Chris Wilson wrote: > On Tue, Jul 07, 2015 at 04:44:30PM +0100, Michel Thierry wrote: >> On 7/7/2015 4:27 PM, Chris Wilson wrote: >>> On Tue, Jul 07, 2015 at 04:14:59PM +0100, Michel Thierry wrote: >>>> In a 48b world, users can try to allocate buffers bigger than 4GB; in >>>> these cases it is important that size is a 64b variable. >>>> >>>> Also added a warning for illegal bind with size = 0. >>>> >>>> Signed-off-by: Michel Thierry <michel.thierry@intel.com> >>>> --- >>>> drivers/gpu/drm/i915/i915_gem.c | 5 +++-- >>>> drivers/gpu/drm/i915/i915_gem_gtt.c | 3 +++ >>>> 2 files changed, 6 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c >>>> index a0bff41..ebfb789 100644 >>>> --- a/drivers/gpu/drm/i915/i915_gem.c >>>> +++ b/drivers/gpu/drm/i915/i915_gem.c >>>> @@ -3718,7 +3718,8 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, >>>> { >>>> struct drm_device *dev = obj->base.dev; >>>> struct drm_i915_private *dev_priv = dev->dev_private; >>>> - u32 size, fence_size, fence_alignment, unfenced_alignment; >>>> + u32 fence_alignment, unfenced_alignment; >>>> + u64 size, fence_size; >>>> u64 start = >>>> flags & PIN_OFFSET_BIAS ? flags & PIN_OFFSET_MASK : 0; >>>> u64 end = >>>> @@ -3777,7 +3778,7 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, >>>> * attempt to find space. >>>> */ >>>> if (size > end) { >>>> - DRM_DEBUG("Attempting to bind an object (view type=%u) larger than the aperture: size=%u > %s aperture=%llu\n", >>>> + DRM_DEBUG("Attempting to bind an object (view type=%u) larger than the aperture: size=%llu > %s aperture=%llu\n", >>>> ggtt_view ? ggtt_view->type : 0, >>>> size, >>>> flags & PIN_MAPPABLE ? "mappable" : "total", >>>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c >>>> index 449a245..900bce6 100644 >>>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c >>>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c >>>> @@ -3312,6 +3312,9 @@ int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level, >>>> if (WARN_ON(flags == 0)) >>>> return -EINVAL; >>>> >>>> + if (WARN_ON(vma->node.size == 0)) >>>> + return -EINVAL; >>> >>> This is superfluous. We don't allow size=0 object creation, and the test >>> is better (if at all) at vma_create, but what you mean here is >>> WARN_ON(!drm_mm_node_allocated()) which seems sensisble. And both of >>> these would be better as ENODEV so we don't confuse the user when they >>> get propagated back to userspace. >>> -Chris >>> >> My idea was to catch the node.size overflow if the variable is >> inadvertently changed back to u32 (which has already happen in other >> places). > > Ok, that didn't come across when I just read node.size == 0 (what are > chances that node.size was exactly 2^32 and then truncated?) > Only a test explicitly looking for this kind of issues (I guess). In that test, objects bigger than 2^32 were truncated, while objects exactly 2^32 were hitting a WARN in the driver; alloc_pages wouldn't do anything because node.size == 0, and then insert would complain no pages existed. > vma->node should be fairly opaque, and if possible we want the checks in > drm_mm.c - if we can think of good tests for that layer. > > Certainly drm_mm_reserve_node() probably wants a few sanity checks. > Though most of those should fall out when it can't do the reservation > the user requests. Or change drm_mm_insert_node_in_range_generic() to warn when size==0?
On Wed, Jul 08, 2015 at 12:22:58PM +0100, Michel Thierry wrote: > On 7/7/2015 9:08 PM, Chris Wilson wrote: > >On Tue, Jul 07, 2015 at 04:44:30PM +0100, Michel Thierry wrote: > >>On 7/7/2015 4:27 PM, Chris Wilson wrote: > >>>On Tue, Jul 07, 2015 at 04:14:59PM +0100, Michel Thierry wrote: > >>>>In a 48b world, users can try to allocate buffers bigger than 4GB; in > >>>>these cases it is important that size is a 64b variable. > >>>> > >>>>Also added a warning for illegal bind with size = 0. > >>>> > >>>>Signed-off-by: Michel Thierry <michel.thierry@intel.com> > >>>>--- > >>>> drivers/gpu/drm/i915/i915_gem.c | 5 +++-- > >>>> drivers/gpu/drm/i915/i915_gem_gtt.c | 3 +++ > >>>> 2 files changed, 6 insertions(+), 2 deletions(-) > >>>> > >>>>diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > >>>>index a0bff41..ebfb789 100644 > >>>>--- a/drivers/gpu/drm/i915/i915_gem.c > >>>>+++ b/drivers/gpu/drm/i915/i915_gem.c > >>>>@@ -3718,7 +3718,8 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, > >>>> { > >>>> struct drm_device *dev = obj->base.dev; > >>>> struct drm_i915_private *dev_priv = dev->dev_private; > >>>>- u32 size, fence_size, fence_alignment, unfenced_alignment; > >>>>+ u32 fence_alignment, unfenced_alignment; > >>>>+ u64 size, fence_size; > >>>> u64 start = > >>>> flags & PIN_OFFSET_BIAS ? flags & PIN_OFFSET_MASK : 0; > >>>> u64 end = > >>>>@@ -3777,7 +3778,7 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, > >>>> * attempt to find space. > >>>> */ > >>>> if (size > end) { > >>>>- DRM_DEBUG("Attempting to bind an object (view type=%u) larger than the aperture: size=%u > %s aperture=%llu\n", > >>>>+ DRM_DEBUG("Attempting to bind an object (view type=%u) larger than the aperture: size=%llu > %s aperture=%llu\n", > >>>> ggtt_view ? ggtt_view->type : 0, > >>>> size, > >>>> flags & PIN_MAPPABLE ? "mappable" : "total", > >>>>diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > >>>>index 449a245..900bce6 100644 > >>>>--- a/drivers/gpu/drm/i915/i915_gem_gtt.c > >>>>+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > >>>>@@ -3312,6 +3312,9 @@ int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level, > >>>> if (WARN_ON(flags == 0)) > >>>> return -EINVAL; > >>>> > >>>>+ if (WARN_ON(vma->node.size == 0)) > >>>>+ return -EINVAL; > >>> > >>>This is superfluous. We don't allow size=0 object creation, and the test > >>>is better (if at all) at vma_create, but what you mean here is > >>>WARN_ON(!drm_mm_node_allocated()) which seems sensisble. And both of > >>>these would be better as ENODEV so we don't confuse the user when they > >>>get propagated back to userspace. > >>>-Chris > >>> > >>My idea was to catch the node.size overflow if the variable is > >>inadvertently changed back to u32 (which has already happen in other > >>places). > > > >Ok, that didn't come across when I just read node.size == 0 (what are > >chances that node.size was exactly 2^32 and then truncated?) > > > Only a test explicitly looking for this kind of issues (I guess). In that > test, objects bigger than 2^32 were truncated, while objects exactly 2^32 > were hitting a WARN in the driver; alloc_pages wouldn't do anything because > node.size == 0, and then insert would complain no pages existed. > > >vma->node should be fairly opaque, and if possible we want the checks in > >drm_mm.c - if we can think of good tests for that layer. > > > >Certainly drm_mm_reserve_node() probably wants a few sanity checks. > >Though most of those should fall out when it can't do the reservation > >the user requests. > Or change drm_mm_insert_node_in_range_generic() to warn when size==0? WARN_ON(vma->node.size != obj->base.size) ? Feel free to get the casting right - I suck at implicit C integer conversion rules ... -Daniel
On 7/8/2015 4:22 PM, Daniel Vetter wrote: > On Wed, Jul 08, 2015 at 12:22:58PM +0100, Michel Thierry wrote: >> On 7/7/2015 9:08 PM, Chris Wilson wrote: >>> On Tue, Jul 07, 2015 at 04:44:30PM +0100, Michel Thierry wrote: >>>> On 7/7/2015 4:27 PM, Chris Wilson wrote: >>>>> On Tue, Jul 07, 2015 at 04:14:59PM +0100, Michel Thierry wrote: >>>>>> In a 48b world, users can try to allocate buffers bigger than 4GB; in >>>>>> these cases it is important that size is a 64b variable. >>>>>> >>>>>> Also added a warning for illegal bind with size = 0. >>>>>> >>>>>> Signed-off-by: Michel Thierry <michel.thierry@intel.com> >>>>>> --- >>>>>> drivers/gpu/drm/i915/i915_gem.c | 5 +++-- >>>>>> drivers/gpu/drm/i915/i915_gem_gtt.c | 3 +++ >>>>>> 2 files changed, 6 insertions(+), 2 deletions(-) >>>>>> >>>>>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c >>>>>> index a0bff41..ebfb789 100644 >>>>>> --- a/drivers/gpu/drm/i915/i915_gem.c >>>>>> +++ b/drivers/gpu/drm/i915/i915_gem.c >>>>>> @@ -3718,7 +3718,8 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, >>>>>> { >>>>>> struct drm_device *dev = obj->base.dev; >>>>>> struct drm_i915_private *dev_priv = dev->dev_private; >>>>>> - u32 size, fence_size, fence_alignment, unfenced_alignment; >>>>>> + u32 fence_alignment, unfenced_alignment; >>>>>> + u64 size, fence_size; >>>>>> u64 start = >>>>>> flags & PIN_OFFSET_BIAS ? flags & PIN_OFFSET_MASK : 0; >>>>>> u64 end = >>>>>> @@ -3777,7 +3778,7 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, >>>>>> * attempt to find space. >>>>>> */ >>>>>> if (size > end) { >>>>>> - DRM_DEBUG("Attempting to bind an object (view type=%u) larger than the aperture: size=%u > %s aperture=%llu\n", >>>>>> + DRM_DEBUG("Attempting to bind an object (view type=%u) larger than the aperture: size=%llu > %s aperture=%llu\n", >>>>>> ggtt_view ? ggtt_view->type : 0, >>>>>> size, >>>>>> flags & PIN_MAPPABLE ? "mappable" : "total", >>>>>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c >>>>>> index 449a245..900bce6 100644 >>>>>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c >>>>>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c >>>>>> @@ -3312,6 +3312,9 @@ int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level, >>>>>> if (WARN_ON(flags == 0)) >>>>>> return -EINVAL; >>>>>> >>>>>> + if (WARN_ON(vma->node.size == 0)) >>>>>> + return -EINVAL; >>>>> >>>>> This is superfluous. We don't allow size=0 object creation, and the test >>>>> is better (if at all) at vma_create, but what you mean here is >>>>> WARN_ON(!drm_mm_node_allocated()) which seems sensisble. And both of >>>>> these would be better as ENODEV so we don't confuse the user when they >>>>> get propagated back to userspace. >>>>> -Chris >>>>> >>>> My idea was to catch the node.size overflow if the variable is >>>> inadvertently changed back to u32 (which has already happen in other >>>> places). >>> >>> Ok, that didn't come across when I just read node.size == 0 (what are >>> chances that node.size was exactly 2^32 and then truncated?) >>> >> Only a test explicitly looking for this kind of issues (I guess). In that >> test, objects bigger than 2^32 were truncated, while objects exactly 2^32 >> were hitting a WARN in the driver; alloc_pages wouldn't do anything because >> node.size == 0, and then insert would complain no pages existed. >> >>> vma->node should be fairly opaque, and if possible we want the checks in >>> drm_mm.c - if we can think of good tests for that layer. >>> >>> Certainly drm_mm_reserve_node() probably wants a few sanity checks. >>> Though most of those should fall out when it can't do the reservation >>> the user requests. >> Or change drm_mm_insert_node_in_range_generic() to warn when size==0? > > WARN_ON(vma->node.size != obj->base.size) ? Feel free to get the casting > right - I suck at implicit C integer conversion rules ... > -Daniel > Thanks, if there's no objections, I'll change it to: if (WARN_ON(vma->node.size != (u64)vma->obj->base.size)) return -ENODEV;
On Wed, Jul 08, 2015 at 05:42:17PM +0100, Michel Thierry wrote: > >WARN_ON(vma->node.size != obj->base.size) ? Feel free to get the casting > >right - I suck at implicit C integer conversion rules ... > >-Daniel > > > Thanks, if there's no objections, I'll change it to: > > if (WARN_ON(vma->node.size != (u64)vma->obj->base.size)) > return -ENODEV; Are we still talking about i915_vma_bind()? Then vma->node.size != vma->obj.base.size anyway. -Chris
On 7/8/2015 6:03 PM, Chris Wilson wrote: > On Wed, Jul 08, 2015 at 05:42:17PM +0100, Michel Thierry wrote: >>> WARN_ON(vma->node.size != obj->base.size) ? Feel free to get the casting >>> right - I suck at implicit C integer conversion rules ... >>> -Daniel >>> >> Thanks, if there's no objections, I'll change it to: >> >> if (WARN_ON(vma->node.size != (u64)vma->obj->base.size)) >> return -ENODEV; > > Are we still talking about i915_vma_bind()? Then vma->node.size != > vma->obj.base.size anyway. > -Chris > Right, it can be either obj->base.size, fence_size or view_size...
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index a0bff41..ebfb789 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3718,7 +3718,8 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, { struct drm_device *dev = obj->base.dev; struct drm_i915_private *dev_priv = dev->dev_private; - u32 size, fence_size, fence_alignment, unfenced_alignment; + u32 fence_alignment, unfenced_alignment; + u64 size, fence_size; u64 start = flags & PIN_OFFSET_BIAS ? flags & PIN_OFFSET_MASK : 0; u64 end = @@ -3777,7 +3778,7 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, * attempt to find space. */ if (size > end) { - DRM_DEBUG("Attempting to bind an object (view type=%u) larger than the aperture: size=%u > %s aperture=%llu\n", + DRM_DEBUG("Attempting to bind an object (view type=%u) larger than the aperture: size=%llu > %s aperture=%llu\n", ggtt_view ? ggtt_view->type : 0, size, flags & PIN_MAPPABLE ? "mappable" : "total", diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 449a245..900bce6 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -3312,6 +3312,9 @@ int i915_vma_bind(struct i915_vma *vma, enum i915_cache_level cache_level, if (WARN_ON(flags == 0)) return -EINVAL; + if (WARN_ON(vma->node.size == 0)) + return -EINVAL; + bind_flags = 0; if (flags & PIN_GLOBAL) bind_flags |= GLOBAL_BIND;
In a 48b world, users can try to allocate buffers bigger than 4GB; in these cases it is important that size is a 64b variable. Also added a warning for illegal bind with size = 0. Signed-off-by: Michel Thierry <michel.thierry@intel.com> --- drivers/gpu/drm/i915/i915_gem.c | 5 +++-- drivers/gpu/drm/i915/i915_gem_gtt.c | 3 +++ 2 files changed, 6 insertions(+), 2 deletions(-)