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@xxxxxxxxx>
---
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;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
