summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorThomas Hellstrom <thomas-at-tungstengraphics-dot-com>2008-02-26 00:01:09 +0100
committerThomas Hellstrom <thomas-at-tungstengraphics-dot-com>2008-02-26 00:05:26 +0100
commit56bb29cf37c27b283efcd1a32d3583393e5208ea (patch)
tree35cb47aed684fa66861e161f7361d339d6a46363
parentd6098db1409e8ee45052920d3acdd3b6f2cb80aa (diff)
Make the execbuffer code reasonably safe against errors.
In particular -EAGAINs, which should be common during Xserver operation. Also handle the fence creation failure case.
-rw-r--r--shared-core/i915_dma.c244
1 files changed, 180 insertions, 64 deletions
diff --git a/shared-core/i915_dma.c b/shared-core/i915_dma.c
index 7da8d55c..4f41a688 100644
--- a/shared-core/i915_dma.c
+++ b/shared-core/i915_dma.c
@@ -809,7 +809,10 @@ struct i915_relocatee_info {
struct drm_i915_validate_buffer {
struct drm_buffer_object *buffer;
+ struct drm_bo_info_rep rep;
int presumed_offset_correct;
+ void __user *data;
+ int ret;
};
static void i915_dereference_buffers_locked(struct drm_i915_validate_buffer *buffers,
@@ -1012,6 +1015,40 @@ out_err:
return ret;
}
+static int i915_check_presumed(struct drm_i915_op_arg *arg,
+ struct drm_buffer_object *bo,
+ uint32_t __user *data,
+ int *presumed_ok)
+{
+ struct drm_bo_op_req *req = &arg->d.req;
+ uint32_t hint_offset;
+ uint32_t hint = req->bo_req.hint;
+
+ *presumed_ok = 0;
+
+ if (!(hint & DRM_BO_HINT_PRESUMED_OFFSET))
+ return 0;
+ if (bo->offset == req->bo_req.presumed_offset) {
+ *presumed_ok = 1;
+ return 0;
+ }
+
+ /*
+ * We need to turn off the HINT_PRESUMED_OFFSET for this buffer in
+ * the user-space IOCTL argument list, since the buffer has moved,
+ * we're about to apply relocations and we might subsequently
+ * hit an -EAGAIN. In that case the argument list will be reused by
+ * user-space, but the presumed offset is no longer valid.
+ *
+ * Needless to say, this is a bit ugly.
+ */
+
+ hint_offset = (uint32_t *)&req->bo_req.hint - (uint32_t *)arg;
+ hint &= ~DRM_BO_HINT_PRESUMED_OFFSET;
+ return __put_user(hint, data + hint_offset);
+}
+
+
/*
* Validate, add fence and relocate a block of bos from a userspace list
*/
@@ -1022,13 +1059,11 @@ int i915_validate_buffer_list(struct drm_file *file_priv,
{
struct drm_i915_op_arg arg;
struct drm_bo_op_req *req = &arg.d.req;
- struct drm_bo_arg_rep rep;
- unsigned long next = 0;
int ret = 0;
unsigned buf_count = 0;
- struct drm_device *dev = file_priv->head->dev;
uint32_t buf_handle;
uint32_t __user *reloc_user_ptr;
+ struct drm_i915_validate_buffer *item = buffers;
do {
if (buf_count >= *num_buffers) {
@@ -1036,31 +1071,26 @@ int i915_validate_buffer_list(struct drm_file *file_priv,
ret = -EINVAL;
goto out_err;
}
+ item = buffers + buf_count;
+ item->buffer = NULL;
+ item->presumed_offset_correct = 0;
buffers[buf_count].buffer = NULL;
- buffers[buf_count].presumed_offset_correct = 0;
if (copy_from_user(&arg, (void __user *)(unsigned long)data, sizeof(arg))) {
ret = -EFAULT;
goto out_err;
}
- if (arg.handled) {
- data = arg.next;
- mutex_lock(&dev->struct_mutex);
- buffers[buf_count].buffer = drm_lookup_buffer_object(file_priv, req->arg_handle, 1);
- mutex_unlock(&dev->struct_mutex);
- buf_count++;
- continue;
- }
-
- rep.ret = 0;
+ ret = 0;
if (req->op != drm_bo_validate) {
DRM_ERROR
("Buffer object operation wasn't \"validate\".\n");
- rep.ret = -EINVAL;
+ ret = -EINVAL;
goto out_err;
}
+ item->ret = 0;
+ item->data = (void __user *) (unsigned long) data;
buf_handle = req->bo_req.handle;
reloc_user_ptr = (uint32_t *)(unsigned long)arg.reloc_ptr;
@@ -1072,48 +1102,149 @@ int i915_validate_buffer_list(struct drm_file *file_priv,
DRM_MEMORYBARRIER();
}
- rep.ret = drm_bo_handle_validate(file_priv, req->bo_req.handle,
- req->bo_req.flags, req->bo_req.mask,
- req->bo_req.hint,
- req->bo_req.fence_class, 0,
- &rep.bo_info,
- &buffers[buf_count].buffer);
+ ret = drm_bo_handle_validate(file_priv, req->bo_req.handle,
+ req->bo_req.flags, req->bo_req.mask,
+ req->bo_req.hint,
+ req->bo_req.fence_class, 0,
+ &item->rep,
+ &item->buffer);
+
+ if (ret) {
+ DRM_ERROR("error on handle validate %d\n", ret);
+ goto out_err;
+ }
+
+ buf_count++;
- if (rep.ret) {
- DRM_ERROR("error on handle validate %d\n", rep.ret);
+ ret = i915_check_presumed(&arg, item->buffer,
+ (uint32_t __user *)
+ (unsigned long) data,
+ &item->presumed_offset_correct);
+ if (ret)
goto out_err;
+
+ data = arg.next;
+ } while (data != 0);
+out_err:
+ *num_buffers = buf_count;
+ item->ret = (ret != -EAGAIN) ? ret : 0;
+ return ret;
+}
+
+
+/*
+ * Remove all buffers from the unfenced list.
+ * If the execbuffer operation was aborted, for example due to a signal,
+ * this also make sure that buffers retain their original state and
+ * fence pointers.
+ * Copy back buffer information to user-space unless we were interrupted
+ * by a signal. In which case the IOCTL must be rerun.
+ */
+
+static int i915_handle_copyback(struct drm_device *dev,
+ struct drm_i915_validate_buffer *buffers,
+ unsigned int num_buffers, int ret)
+{
+ int err = ret;
+ int i;
+ struct drm_i915_op_arg arg;
+
+ if (ret)
+ drm_putback_buffer_objects(dev);
+
+ if (ret != -EAGAIN) {
+ for (i = 0; i < num_buffers; ++i) {
+ arg.handled = 1;
+ arg.d.rep.ret = buffers->ret;
+ arg.d.rep.bo_info = buffers->rep;
+ if (__copy_to_user(buffers->data, &arg, sizeof(arg)))
+ err = -EFAULT;
+ buffers++;
}
+ }
+
+ return err;
+}
+
+/*
+ * Create a fence object, and if that fails, pretend that everything is
+ * OK and just idle the GPU.
+ */
+
+void i915_fence_or_sync(struct drm_file *file_priv,
+ uint32_t fence_flags,
+ struct drm_fence_arg *fence_arg,
+ struct drm_fence_object **fence_p)
+{
+ struct drm_device *dev = file_priv->head->dev;
+ int ret;
+ struct drm_fence_object *fence;
+
+ ret = drm_fence_buffer_objects(dev, NULL, fence_flags,
+ NULL, &fence);
+
+ if (ret) {
+
/*
- * If the user provided a presumed offset hint, check whether
- * the buffer is in the same place, if so, relocations relative to
- * this buffer need not be performed
+ * Fence creation failed.
+ * Fall back to synchronous operation and idle the engine.
*/
- if ((req->bo_req.hint & DRM_BO_HINT_PRESUMED_OFFSET) &&
- buffers[buf_count].buffer->offset == req->bo_req.presumed_offset) {
- buffers[buf_count].presumed_offset_correct = 1;
- }
- next = arg.next;
- arg.handled = 1;
- arg.d.rep = rep;
+ (void) i915_quiescent(dev);
- if (copy_to_user((void __user *)(unsigned long)data, &arg, sizeof(arg)))
- return -EFAULT;
+ /*
+ * FIXME: Might need a sync flush here.
+ */
- data = next;
- buf_count++;
+ if (!(fence_flags & DRM_FENCE_FLAG_NO_USER)) {
- } while (next != 0);
- *num_buffers = buf_count;
- return 0;
-out_err:
- mutex_lock(&dev->struct_mutex);
- i915_dereference_buffers_locked(buffers, buf_count);
- mutex_unlock(&dev->struct_mutex);
- *num_buffers = 0;
- return (ret) ? ret : rep.ret;
+ /*
+ * Communicate to user-space that
+ * fence creation has failed and that
+ * the engine is idle.
+ */
+
+ fence_arg->handle = ~0;
+ fence_arg->error = ret;
+ }
+
+ drm_putback_buffer_objects(dev);
+ if (fence_p)
+ *fence_p = NULL;
+ return;
+ }
+
+ if (!(fence_flags & DRM_FENCE_FLAG_NO_USER)) {
+
+ ret = drm_fence_add_user_object(file_priv, fence,
+ fence_flags &
+ DRM_FENCE_FLAG_SHAREABLE);
+ if (!ret)
+ drm_fence_fill_arg(fence, fence_arg);
+ else {
+ /*
+ * Fence user object creation failed.
+ * We must idle the engine here as well, as user-
+ * space expects a fence object to wait on. Since we
+ * have a fence object we wait for it to signal
+ * to indicate engine "sufficiently" idle.
+ */
+
+ (void) drm_fence_object_wait(fence, 0, 1,
+ fence->type);
+ drm_fence_usage_deref_unlocked(&fence);
+ fence_arg->handle = ~0;
+ fence_arg->error = ret;
+ }
+ }
+
+ if (fence_p)
+ *fence_p = fence;
+ else if (fence)
+ drm_fence_usage_deref_unlocked(&fence);
}
+
static int i915_execbuffer(struct drm_device *dev, void *data,
struct drm_file *file_priv)
{
@@ -1126,7 +1257,6 @@ static int i915_execbuffer(struct drm_device *dev, void *data,
int num_buffers;
int ret;
struct drm_i915_validate_buffer *buffers;
- struct drm_fence_object *fence;
if (!dev_priv->allow_batchbuffer) {
DRM_ERROR("Batchbuffer ioctl disabled\n");
@@ -1171,7 +1301,7 @@ static int i915_execbuffer(struct drm_device *dev, void *data,
ret = i915_validate_buffer_list(file_priv, 0, exec_buf->ops_list,
buffers, &num_buffers);
if (ret)
- goto out_free;
+ goto out_err0;
/* make sure all previous memory operations have passed */
DRM_MEMORYBARRIER();
@@ -1190,30 +1320,16 @@ static int i915_execbuffer(struct drm_device *dev, void *data,
if (sarea_priv)
sarea_priv->last_dispatch = READ_BREADCRUMB(dev_priv);
- /* fence */
- ret = drm_fence_buffer_objects(dev, NULL, fence_arg->flags,
- NULL, &fence);
- if (ret)
- goto out_err0;
+ i915_fence_or_sync(file_priv, fence_arg->flags, fence_arg, NULL);
- if (!(fence_arg->flags & DRM_FENCE_FLAG_NO_USER)) {
- ret = drm_fence_add_user_object(file_priv, fence, fence_arg->flags & DRM_FENCE_FLAG_SHAREABLE);
- if (!ret) {
- fence_arg->handle = fence->base.hash.key;
- fence_arg->fence_class = fence->fence_class;
- fence_arg->type = fence->type;
- fence_arg->signaled = fence->signaled_types;
- }
- }
- drm_fence_usage_deref_unlocked(&fence);
out_err0:
/* handle errors */
+ ret = i915_handle_copyback(dev, buffers, num_buffers, ret);
mutex_lock(&dev->struct_mutex);
i915_dereference_buffers_locked(buffers, num_buffers);
mutex_unlock(&dev->struct_mutex);
-out_free:
drm_free(buffers, (exec_buf->num_buffers * sizeof(struct drm_buffer_object *)), DRM_MEM_DRIVER);
mutex_unlock(&dev_priv->cmdbuf_mutex);