[PATCH 35/36] drm/i915: Make command parser dma-fence signaling safe
Maarten Lankhorst
maarten.lankhorst at linux.intel.com
Fri Sep 18 10:34:04 UTC 2020
Allocate jump_whitelist in the execbuffer, and add annotations around
intel_engine_cmd_parser(), to ensure we only call the command parser
without allocating any memory, or taking any locks we're not supposed to.
Because i915_gem_object_get_page() may also allocate memory, add a
path to i915_gem_object_get_sg() that prevents memory allocations,
and walk the sg list manually. It should be similarly fast.
Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
---
.../gpu/drm/i915/gem/i915_gem_execbuffer.c | 30 ++++++++---
drivers/gpu/drm/i915/gem/i915_gem_object.h | 3 +-
drivers/gpu/drm/i915/gem/i915_gem_pages.c | 20 +++++--
drivers/gpu/drm/i915/gt/intel_ggtt.c | 2 +-
drivers/gpu/drm/i915/i915_cmd_parser.c | 52 +++++++++++--------
drivers/gpu/drm/i915/i915_drv.h | 5 +-
6 files changed, 74 insertions(+), 38 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
index fd1f0da9862b..6fc2efe82cdd 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c
@@ -2362,6 +2362,7 @@ struct eb_parse_work {
struct i915_vma *trampoline;
unsigned int batch_offset;
unsigned int batch_length;
+ unsigned long *jump_whitelist;
const void *batch_map;
void *shadow_map;
};
@@ -2369,21 +2370,30 @@ struct eb_parse_work {
static int __eb_parse(struct dma_fence_work *work)
{
struct eb_parse_work *pw = container_of(work, typeof(*pw), base);
+ int ret;
+ bool cookie;
+
+ cookie = dma_fence_begin_signalling();
+ ret = intel_engine_cmd_parser(pw->engine,
+ pw->batch,
+ pw->batch_offset,
+ pw->batch_length,
+ pw->shadow,
+ pw->jump_whitelist,
+ pw->shadow_map,
+ pw->batch_map);
+ dma_fence_end_signalling(cookie);
- return intel_engine_cmd_parser(pw->engine,
- pw->batch,
- pw->batch_offset,
- pw->batch_length,
- pw->shadow,
- pw->trampoline,
- pw->shadow_map,
- pw->batch_map);
+ return ret;
}
static void __eb_parse_release(struct dma_fence_work *work)
{
struct eb_parse_work *pw = container_of(work, typeof(*pw), base);
+ if (!IS_ERR_OR_NULL(pw->jump_whitelist))
+ kfree(pw->jump_whitelist);
+
if (pw->batch_map)
i915_gem_object_unpin_map(pw->batch->obj);
else
@@ -2482,6 +2492,10 @@ static int eb_parse_pipeline(struct i915_execbuffer *eb,
pw->batch_map = NULL;
}
+ pw->jump_whitelist =
+ intel_engine_cmd_parser_alloc_jump_whitelist(eb->batch_len,
+ trampoline);
+
dma_fence_work_init(&pw->base, &eb_parse_ops);
pw->engine = eb->engine;
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.h b/drivers/gpu/drm/i915/gem/i915_gem_object.h
index a855fe6cb417..9b9812d02e79 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/gem/i915_gem_object.h
@@ -299,7 +299,8 @@ int i915_gem_object_set_tiling(struct drm_i915_gem_object *obj,
struct scatterlist *
i915_gem_object_get_sg(struct drm_i915_gem_object *obj,
- unsigned int n, unsigned int *offset);
+ unsigned int n, unsigned int *offset,
+ bool allow_alloc);
struct page *
i915_gem_object_get_page(struct drm_i915_gem_object *obj,
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pages.c b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
index 82c0d4704113..0969619cb63a 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_pages.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_pages.c
@@ -479,7 +479,8 @@ void __i915_gem_object_release_map(struct drm_i915_gem_object *obj)
struct scatterlist *
i915_gem_object_get_sg(struct drm_i915_gem_object *obj,
unsigned int n,
- unsigned int *offset)
+ unsigned int *offset,
+ bool allow_alloc)
{
struct i915_gem_object_page_iter *iter = &obj->mm.get_page;
struct scatterlist *sg;
@@ -501,6 +502,9 @@ i915_gem_object_get_sg(struct drm_i915_gem_object *obj,
if (n < READ_ONCE(iter->sg_idx))
goto lookup;
+ if (!allow_alloc)
+ goto manual_lookup;
+
mutex_lock(&iter->lock);
/* We prefer to reuse the last sg so that repeated lookup of this
@@ -550,7 +554,15 @@ i915_gem_object_get_sg(struct drm_i915_gem_object *obj,
if (unlikely(n < idx)) /* insertion completed by another thread */
goto lookup;
- /* In case we failed to insert the entry into the radixtree, we need
+ goto manual_walk;
+
+manual_lookup:
+ idx = 0;
+ sg = obj->mm.pages->sgl;
+
+manual_walk:
+ /*
+ * In case we failed to insert the entry into the radixtree, we need
* to look beyond the current sg.
*/
while (idx + count <= n) {
@@ -597,7 +609,7 @@ i915_gem_object_get_page(struct drm_i915_gem_object *obj, unsigned int n)
GEM_BUG_ON(!i915_gem_object_has_struct_page(obj));
- sg = i915_gem_object_get_sg(obj, n, &offset);
+ sg = i915_gem_object_get_sg(obj, n, &offset, true);
return nth_page(sg_page(sg), offset);
}
@@ -623,7 +635,7 @@ i915_gem_object_get_dma_address_len(struct drm_i915_gem_object *obj,
struct scatterlist *sg;
unsigned int offset;
- sg = i915_gem_object_get_sg(obj, n, &offset);
+ sg = i915_gem_object_get_sg(obj, n, &offset, true);
if (len)
*len = sg_dma_len(sg) - (offset << PAGE_SHIFT);
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index d6334f5b64ad..451e5cde2fec 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -1389,7 +1389,7 @@ intel_partial_pages(const struct i915_ggtt_view *view,
if (ret)
goto err_sg_alloc;
- iter = i915_gem_object_get_sg(obj, view->partial.offset, &offset);
+ iter = i915_gem_object_get_sg(obj, view->partial.offset, &offset, true);
GEM_BUG_ON(!iter);
sg = st->sgl;
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
index e48c8b631abe..a6be1795614e 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -1145,8 +1145,9 @@ static u32 *copy_batch(struct drm_i915_gem_object *dst_obj,
GEM_BUG_ON(!needs_clflush);
i915_unaligned_memcpy_from_wc(dst, src + offset, length);
} else {
+ struct scatterlist *sg;
void *ptr;
- int x, n;
+ u32 x, sg_ofs;
/*
* We can avoid clflushing partial cachelines before the write
@@ -1162,18 +1163,26 @@ static u32 *copy_batch(struct drm_i915_gem_object *dst_obj,
ptr = dst;
x = offset_in_page(offset);
- for (n = offset >> PAGE_SHIFT; length; n++) {
- int len = min_t(int, length, PAGE_SIZE - x);
- void *map = kmap_atomic(i915_gem_object_get_page(src_obj, n));
-
- if (needs_clflush)
- drm_clflush_virt_range(map + x, len);
- memcpy(ptr, map + x, len);
- kunmap_atomic(map);
-
- ptr += len;
- length -= len;
- x = 0;
+
+ sg = i915_gem_object_get_sg(src_obj, offset >> PAGE_SHIFT, &sg_ofs, false);
+
+ for (; length; sg_ofs = 0, sg = sg_next(sg)) {
+ u32 sg_max = sg->length >> PAGE_SHIFT;
+
+ for (; length && sg_ofs < sg_max; sg_ofs++) {
+ u32 len = min_t(u32, length, PAGE_SIZE - x);
+ void *map;
+
+ map = kmap_atomic(nth_page(sg_page(sg), sg_ofs));
+ if (needs_clflush)
+ drm_clflush_virt_range(map + x, len);
+ memcpy(ptr, map + x, len);
+ kunmap_atomic(map);
+
+ ptr += len;
+ length -= len;
+ x = 0;
+ }
}
}
@@ -1348,10 +1357,14 @@ static int check_bbstart(u32 *cmd, u32 offset, u32 length,
return 0;
}
-static unsigned long *alloc_whitelist(u32 batch_length)
+unsigned long *intel_engine_cmd_parser_alloc_jump_whitelist(u32 batch_length,
+ bool trampoline)
{
unsigned long *jmp;
+ if (trampoline)
+ return NULL;
+
/*
* We expect batch_length to be less than 256KiB for known users,
* i.e. we need at most an 8KiB bitmap allocation which should be
@@ -1394,16 +1407,16 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine,
u32 batch_offset,
u32 batch_length,
struct i915_vma *shadow,
- bool trampoline,
+ unsigned long *jump_whitelist,
void *shadow_map,
const void *batch_map)
{
u32 *cmd, *batch_end, offset = 0;
struct drm_i915_cmd_descriptor default_desc = noop_desc;
const struct drm_i915_cmd_descriptor *desc = &default_desc;
- unsigned long *jump_whitelist;
u64 batch_addr, shadow_addr;
int ret = 0;
+ bool trampoline = !jump_whitelist;
GEM_BUG_ON(!IS_ALIGNED(batch_offset, sizeof(*cmd)));
GEM_BUG_ON(!IS_ALIGNED(batch_length, sizeof(*cmd)));
@@ -1418,11 +1431,6 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine,
return PTR_ERR(cmd);
}
- jump_whitelist = NULL;
- if (!trampoline)
- /* Defer failure until attempted use */
- jump_whitelist = alloc_whitelist(batch_length);
-
shadow_addr = gen8_canonical_addr(shadow->node.start);
batch_addr = gen8_canonical_addr(batch->node.start + batch_offset);
@@ -1530,8 +1538,6 @@ int intel_engine_cmd_parser(struct intel_engine_cs *engine,
drm_clflush_virt_range(ptr, (void *)(cmd + 1) - ptr);
}
- if (!IS_ERR_OR_NULL(jump_whitelist))
- kfree(jump_whitelist);
return ret;
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b4a80fa4f8e2..24b82e32240b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1946,12 +1946,15 @@ const char *i915_cache_level_str(struct drm_i915_private *i915, int type);
int i915_cmd_parser_get_version(struct drm_i915_private *dev_priv);
void intel_engine_init_cmd_parser(struct intel_engine_cs *engine);
void intel_engine_cleanup_cmd_parser(struct intel_engine_cs *engine);
+unsigned long *intel_engine_cmd_parser_alloc_jump_whitelist(u32 batch_length,
+ bool trampoline);
+
int intel_engine_cmd_parser(struct intel_engine_cs *engine,
struct i915_vma *batch,
u32 batch_offset,
u32 batch_length,
struct i915_vma *shadow,
- bool trampoline,
+ unsigned long *jump_whitelist,
void *shadow_map,
const void *batch_map);
#define I915_CMD_PARSER_TRAMPOLINE_SIZE 8
--
2.28.0
More information about the Intel-gfx-trybot
mailing list