drm/i915: Use batch length instead of object size in command parser
Previously we couldn't trust the user-supplied batch length because it came directly from userspace (i.e. untrusted code). It would have affected what commands software parsed without regard to what hardware would actually execute, leaving a potential hole. With the parser now copying the user supplied batch buffer and writing MI_NOP commands to any space after the copied region, we can safely use the batch length input. This should be a performance win as the actual batch length is frequently much smaller than the allocated object size. v2: Fix handling of non-zero batch_start_offset Issue: VIZ-4719 Signed-off-by: Brad Volkin <bradley.d.volkin@intel.com> Reviewed-By: Jon Bloomfield <jon.bloomfield@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
This commit is contained in:
parent
78a423772d
commit
b9ffd80ed6
|
@ -850,11 +850,19 @@ finish:
|
||||||
|
|
||||||
/* Returns a vmap'd pointer to dest_obj, which the caller must unmap */
|
/* Returns a vmap'd pointer to dest_obj, which the caller must unmap */
|
||||||
static u32 *copy_batch(struct drm_i915_gem_object *dest_obj,
|
static u32 *copy_batch(struct drm_i915_gem_object *dest_obj,
|
||||||
struct drm_i915_gem_object *src_obj)
|
struct drm_i915_gem_object *src_obj,
|
||||||
|
u32 batch_start_offset,
|
||||||
|
u32 batch_len)
|
||||||
{
|
{
|
||||||
int ret = 0;
|
int ret = 0;
|
||||||
int needs_clflush = 0;
|
int needs_clflush = 0;
|
||||||
u32 *src_addr, *dest_addr = NULL;
|
u32 *src_base, *dest_base = NULL;
|
||||||
|
u32 *src_addr, *dest_addr;
|
||||||
|
u32 offset = batch_start_offset / sizeof(*dest_addr);
|
||||||
|
u32 end = batch_start_offset + batch_len;
|
||||||
|
|
||||||
|
if (end > dest_obj->base.size || end > src_obj->base.size)
|
||||||
|
return ERR_PTR(-E2BIG);
|
||||||
|
|
||||||
ret = i915_gem_obj_prepare_shmem_read(src_obj, &needs_clflush);
|
ret = i915_gem_obj_prepare_shmem_read(src_obj, &needs_clflush);
|
||||||
if (ret) {
|
if (ret) {
|
||||||
|
@ -862,15 +870,17 @@ static u32 *copy_batch(struct drm_i915_gem_object *dest_obj,
|
||||||
return ERR_PTR(ret);
|
return ERR_PTR(ret);
|
||||||
}
|
}
|
||||||
|
|
||||||
src_addr = vmap_batch(src_obj);
|
src_base = vmap_batch(src_obj);
|
||||||
if (!src_addr) {
|
if (!src_base) {
|
||||||
DRM_DEBUG_DRIVER("CMD: Failed to vmap batch\n");
|
DRM_DEBUG_DRIVER("CMD: Failed to vmap batch\n");
|
||||||
ret = -ENOMEM;
|
ret = -ENOMEM;
|
||||||
goto unpin_src;
|
goto unpin_src;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
src_addr = src_base + offset;
|
||||||
|
|
||||||
if (needs_clflush)
|
if (needs_clflush)
|
||||||
drm_clflush_virt_range((char *)src_addr, src_obj->base.size);
|
drm_clflush_virt_range((char *)src_addr, batch_len);
|
||||||
|
|
||||||
ret = i915_gem_object_set_to_cpu_domain(dest_obj, true);
|
ret = i915_gem_object_set_to_cpu_domain(dest_obj, true);
|
||||||
if (ret) {
|
if (ret) {
|
||||||
|
@ -878,24 +888,27 @@ static u32 *copy_batch(struct drm_i915_gem_object *dest_obj,
|
||||||
goto unmap_src;
|
goto unmap_src;
|
||||||
}
|
}
|
||||||
|
|
||||||
dest_addr = vmap_batch(dest_obj);
|
dest_base = vmap_batch(dest_obj);
|
||||||
if (!dest_addr) {
|
if (!dest_base) {
|
||||||
DRM_DEBUG_DRIVER("CMD: Failed to vmap shadow batch\n");
|
DRM_DEBUG_DRIVER("CMD: Failed to vmap shadow batch\n");
|
||||||
ret = -ENOMEM;
|
ret = -ENOMEM;
|
||||||
goto unmap_src;
|
goto unmap_src;
|
||||||
}
|
}
|
||||||
|
|
||||||
memcpy(dest_addr, src_addr, src_obj->base.size);
|
dest_addr = dest_base + offset;
|
||||||
if (dest_obj->base.size > src_obj->base.size)
|
|
||||||
memset((u8 *)dest_addr + src_obj->base.size, 0,
|
if (batch_start_offset != 0)
|
||||||
dest_obj->base.size - src_obj->base.size);
|
memset((u8 *)dest_base, 0, batch_start_offset);
|
||||||
|
|
||||||
|
memcpy(dest_addr, src_addr, batch_len);
|
||||||
|
memset((u8 *)dest_addr + batch_len, 0, dest_obj->base.size - end);
|
||||||
|
|
||||||
unmap_src:
|
unmap_src:
|
||||||
vunmap(src_addr);
|
vunmap(src_base);
|
||||||
unpin_src:
|
unpin_src:
|
||||||
i915_gem_object_unpin_pages(src_obj);
|
i915_gem_object_unpin_pages(src_obj);
|
||||||
|
|
||||||
return ret ? ERR_PTR(ret) : dest_addr;
|
return ret ? ERR_PTR(ret) : dest_base;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -1016,6 +1029,7 @@ static bool check_cmd(const struct intel_engine_cs *ring,
|
||||||
* @batch_obj: the batch buffer in question
|
* @batch_obj: the batch buffer in question
|
||||||
* @shadow_batch_obj: copy of the batch buffer in question
|
* @shadow_batch_obj: copy of the batch buffer in question
|
||||||
* @batch_start_offset: byte offset in the batch at which execution starts
|
* @batch_start_offset: byte offset in the batch at which execution starts
|
||||||
|
* @batch_len: length of the commands in batch_obj
|
||||||
* @is_master: is the submitting process the drm master?
|
* @is_master: is the submitting process the drm master?
|
||||||
*
|
*
|
||||||
* Parses the specified batch buffer looking for privilege violations as
|
* Parses the specified batch buffer looking for privilege violations as
|
||||||
|
@ -1028,6 +1042,7 @@ int i915_parse_cmds(struct intel_engine_cs *ring,
|
||||||
struct drm_i915_gem_object *batch_obj,
|
struct drm_i915_gem_object *batch_obj,
|
||||||
struct drm_i915_gem_object *shadow_batch_obj,
|
struct drm_i915_gem_object *shadow_batch_obj,
|
||||||
u32 batch_start_offset,
|
u32 batch_start_offset,
|
||||||
|
u32 batch_len,
|
||||||
bool is_master)
|
bool is_master)
|
||||||
{
|
{
|
||||||
int ret = 0;
|
int ret = 0;
|
||||||
|
@ -1035,7 +1050,8 @@ int i915_parse_cmds(struct intel_engine_cs *ring,
|
||||||
struct drm_i915_cmd_descriptor default_desc = { 0 };
|
struct drm_i915_cmd_descriptor default_desc = { 0 };
|
||||||
bool oacontrol_set = false; /* OACONTROL tracking. See check_cmd() */
|
bool oacontrol_set = false; /* OACONTROL tracking. See check_cmd() */
|
||||||
|
|
||||||
batch_base = copy_batch(shadow_batch_obj, batch_obj);
|
batch_base = copy_batch(shadow_batch_obj, batch_obj,
|
||||||
|
batch_start_offset, batch_len);
|
||||||
if (IS_ERR(batch_base)) {
|
if (IS_ERR(batch_base)) {
|
||||||
DRM_DEBUG_DRIVER("CMD: Failed to copy batch\n");
|
DRM_DEBUG_DRIVER("CMD: Failed to copy batch\n");
|
||||||
return PTR_ERR(batch_base);
|
return PTR_ERR(batch_base);
|
||||||
|
@ -1044,11 +1060,11 @@ int i915_parse_cmds(struct intel_engine_cs *ring,
|
||||||
cmd = batch_base + (batch_start_offset / sizeof(*cmd));
|
cmd = batch_base + (batch_start_offset / sizeof(*cmd));
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* We use the source object's size because the shadow object is as
|
* We use the batch length as size because the shadow object is as
|
||||||
* large or larger and copy_batch() will write MI_NOPs to the extra
|
* large or larger and copy_batch() will write MI_NOPs to the extra
|
||||||
* space. Parsing should be faster in some cases this way.
|
* space. Parsing should be faster in some cases this way.
|
||||||
*/
|
*/
|
||||||
batch_end = cmd + (batch_obj->base.size / sizeof(*batch_end));
|
batch_end = cmd + (batch_len / sizeof(*batch_end));
|
||||||
|
|
||||||
while (cmd < batch_end) {
|
while (cmd < batch_end) {
|
||||||
const struct drm_i915_cmd_descriptor *desc;
|
const struct drm_i915_cmd_descriptor *desc;
|
||||||
|
|
|
@ -2965,6 +2965,7 @@ int i915_parse_cmds(struct intel_engine_cs *ring,
|
||||||
struct drm_i915_gem_object *batch_obj,
|
struct drm_i915_gem_object *batch_obj,
|
||||||
struct drm_i915_gem_object *shadow_batch_obj,
|
struct drm_i915_gem_object *shadow_batch_obj,
|
||||||
u32 batch_start_offset,
|
u32 batch_start_offset,
|
||||||
|
u32 batch_len,
|
||||||
bool is_master);
|
bool is_master);
|
||||||
|
|
||||||
/* i915_suspend.c */
|
/* i915_suspend.c */
|
||||||
|
|
|
@ -1421,6 +1421,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
|
||||||
batch_obj,
|
batch_obj,
|
||||||
shadow_batch_obj,
|
shadow_batch_obj,
|
||||||
args->batch_start_offset,
|
args->batch_start_offset,
|
||||||
|
args->batch_len,
|
||||||
file->is_master);
|
file->is_master);
|
||||||
i915_gem_object_ggtt_unpin(shadow_batch_obj);
|
i915_gem_object_ggtt_unpin(shadow_batch_obj);
|
||||||
|
|
||||||
|
|
Loading…
Reference in New Issue