Commit a5f0edf6 authored by Michał Winiarski's avatar Michał Winiarski Committed by Jani Nikula

drm/i915: Avoid writing relocs with addresses in non-canonical form

According to PRM, some parts of HW require the addresses to be in
a canonical form, where bits [63:48] == [47]. Let's convert addresses to
canonical form prior to relocating and return converted offsets to
userspace. We also need to make sure that userspace is using addresses
in canonical form in case of softpin.

v2: Whitespace fixup, gen8_canonical_addr description (Chris, Ville)
v3: Rebase on top of softpin, fix a hole in relocate_entry,
    s/expect/require (Chris)
v4: Handle softpin in validate_exec_list (Chris)
v5: Convert back to canonical form at copy_to_user time (Chris)
v6: Don't use struct exec_object2 in place of exec_object
v7: Use sign_extend64 for converting to canonical form (Joonas),
    reject non-canonical and non-page-aligned offset for softpin (Chris)
v8: Convert back to non-canonical form in a function,
    split the test for EXEC_OBJECT_PINNED (Chris)
v9: s/canonial/canonical, drop accidental double newline (Chris)

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: default avatarMichał Winiarski <michal.winiarski@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1451409892-13708-1-git-send-email-michal.winiarski@intel.com
Testcase: igt/gem_bad_reloc/negative-reloc-blt
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92699
Cc: drm-intel-fixes@lists.freedesktop.org
Reviewed-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
(cherry picked from commit 934acce3)
Signed-off-by: default avatarJani Nikula <jani.nikula@intel.com>
parent d5f384de
...@@ -249,6 +249,31 @@ static inline int use_cpu_reloc(struct drm_i915_gem_object *obj) ...@@ -249,6 +249,31 @@ static inline int use_cpu_reloc(struct drm_i915_gem_object *obj)
obj->cache_level != I915_CACHE_NONE); obj->cache_level != I915_CACHE_NONE);
} }
/* Used to convert any address to canonical form.
* Starting from gen8, some commands (e.g. STATE_BASE_ADDRESS,
* MI_LOAD_REGISTER_MEM and others, see Broadwell PRM Vol2a) require the
* addresses to be in a canonical form:
* "GraphicsAddress[63:48] are ignored by the HW and assumed to be in correct
* canonical form [63:48] == [47]."
*/
#define GEN8_HIGH_ADDRESS_BIT 47
static inline uint64_t gen8_canonical_addr(uint64_t address)
{
return sign_extend64(address, GEN8_HIGH_ADDRESS_BIT);
}
static inline uint64_t gen8_noncanonical_addr(uint64_t address)
{
return address & ((1ULL << (GEN8_HIGH_ADDRESS_BIT + 1)) - 1);
}
static inline uint64_t
relocation_target(struct drm_i915_gem_relocation_entry *reloc,
uint64_t target_offset)
{
return gen8_canonical_addr((int)reloc->delta + target_offset);
}
static int static int
relocate_entry_cpu(struct drm_i915_gem_object *obj, relocate_entry_cpu(struct drm_i915_gem_object *obj,
struct drm_i915_gem_relocation_entry *reloc, struct drm_i915_gem_relocation_entry *reloc,
...@@ -256,7 +281,7 @@ relocate_entry_cpu(struct drm_i915_gem_object *obj, ...@@ -256,7 +281,7 @@ relocate_entry_cpu(struct drm_i915_gem_object *obj,
{ {
struct drm_device *dev = obj->base.dev; struct drm_device *dev = obj->base.dev;
uint32_t page_offset = offset_in_page(reloc->offset); uint32_t page_offset = offset_in_page(reloc->offset);
uint64_t delta = reloc->delta + target_offset; uint64_t delta = relocation_target(reloc, target_offset);
char *vaddr; char *vaddr;
int ret; int ret;
...@@ -292,7 +317,7 @@ relocate_entry_gtt(struct drm_i915_gem_object *obj, ...@@ -292,7 +317,7 @@ relocate_entry_gtt(struct drm_i915_gem_object *obj,
{ {
struct drm_device *dev = obj->base.dev; struct drm_device *dev = obj->base.dev;
struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_private *dev_priv = dev->dev_private;
uint64_t delta = reloc->delta + target_offset; uint64_t delta = relocation_target(reloc, target_offset);
uint64_t offset; uint64_t offset;
void __iomem *reloc_page; void __iomem *reloc_page;
int ret; int ret;
...@@ -347,7 +372,7 @@ relocate_entry_clflush(struct drm_i915_gem_object *obj, ...@@ -347,7 +372,7 @@ relocate_entry_clflush(struct drm_i915_gem_object *obj,
{ {
struct drm_device *dev = obj->base.dev; struct drm_device *dev = obj->base.dev;
uint32_t page_offset = offset_in_page(reloc->offset); uint32_t page_offset = offset_in_page(reloc->offset);
uint64_t delta = (int)reloc->delta + target_offset; uint64_t delta = relocation_target(reloc, target_offset);
char *vaddr; char *vaddr;
int ret; int ret;
...@@ -395,7 +420,7 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj, ...@@ -395,7 +420,7 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj,
target_i915_obj = target_vma->obj; target_i915_obj = target_vma->obj;
target_obj = &target_vma->obj->base; target_obj = &target_vma->obj->base;
target_offset = target_vma->node.start; target_offset = gen8_canonical_addr(target_vma->node.start);
/* Sandybridge PPGTT errata: We need a global gtt mapping for MI and /* Sandybridge PPGTT errata: We need a global gtt mapping for MI and
* pipe_control writes because the gpu doesn't properly redirect them * pipe_control writes because the gpu doesn't properly redirect them
...@@ -994,6 +1019,21 @@ validate_exec_list(struct drm_device *dev, ...@@ -994,6 +1019,21 @@ validate_exec_list(struct drm_device *dev,
if (exec[i].flags & invalid_flags) if (exec[i].flags & invalid_flags)
return -EINVAL; return -EINVAL;
/* Offset can be used as input (EXEC_OBJECT_PINNED), reject
* any non-page-aligned or non-canonical addresses.
*/
if (exec[i].flags & EXEC_OBJECT_PINNED) {
if (exec[i].offset !=
gen8_canonical_addr(exec[i].offset & PAGE_MASK))
return -EINVAL;
/* From drm_mm perspective address space is continuous,
* so from this point we're always using non-canonical
* form internally.
*/
exec[i].offset = gen8_noncanonical_addr(exec[i].offset);
}
if (exec[i].alignment && !is_power_of_2(exec[i].alignment)) if (exec[i].alignment && !is_power_of_2(exec[i].alignment))
return -EINVAL; return -EINVAL;
...@@ -1687,6 +1727,8 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, ...@@ -1687,6 +1727,8 @@ i915_gem_execbuffer(struct drm_device *dev, void *data,
/* Copy the new buffer offsets back to the user's exec list. */ /* Copy the new buffer offsets back to the user's exec list. */
for (i = 0; i < args->buffer_count; i++) { for (i = 0; i < args->buffer_count; i++) {
exec2_list[i].offset =
gen8_canonical_addr(exec2_list[i].offset);
ret = __copy_to_user(&user_exec_list[i].offset, ret = __copy_to_user(&user_exec_list[i].offset,
&exec2_list[i].offset, &exec2_list[i].offset,
sizeof(user_exec_list[i].offset)); sizeof(user_exec_list[i].offset));
...@@ -1752,6 +1794,8 @@ i915_gem_execbuffer2(struct drm_device *dev, void *data, ...@@ -1752,6 +1794,8 @@ i915_gem_execbuffer2(struct drm_device *dev, void *data,
int i; int i;
for (i = 0; i < args->buffer_count; i++) { for (i = 0; i < args->buffer_count; i++) {
exec2_list[i].offset =
gen8_canonical_addr(exec2_list[i].offset);
ret = __copy_to_user(&user_exec_list[i].offset, ret = __copy_to_user(&user_exec_list[i].offset,
&exec2_list[i].offset, &exec2_list[i].offset,
sizeof(user_exec_list[i].offset)); sizeof(user_exec_list[i].offset));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment