Re: [PATCH v2] drm/i915: Avoid writing relocs with addresses in non-canonical form

[Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>]

On Fri, Dec 04, 2015 at 04:20:43PM +0100, Michał Winiarski wrote:
> According to bspec, some parts of HW expect 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.
> 
> v2: Whitespace fixup, gen8_canonical_addr description (Chris, Ville)
> 
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Michel Thierry <michel.thierry@xxxxxxxxx>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Signed-off-by: Michał Winiarski <michal.winiarski@xxxxxxxxx>

There is a hole! Inside the actual relocation fixup you need

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index a4c243cec4aa..6d1a2d20337b 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -249,6 +249,12 @@ static inline int use_cpu_reloc(struct drm_i915_gem_object *obj)
                obj->cache_level != I915_CACHE_NONE);
 }
 
+static uint64_t relocation_target(struct drm_i915_gem_relocation_entry *reloc,
+                                 uint64_t target_offset)
+{
+       return gen8_cannonical_address((int)reloc->delta + target_offset);
+}
+
 static int
 relocate_entry_cpu(struct drm_i915_gem_object *obj,
                   struct drm_i915_gem_relocation_entry *reloc,
@@ -256,7 +262,7 @@ relocate_entry_cpu(struct drm_i915_gem_object *obj,
 {
        struct drm_device *dev = obj->base.dev;
        uint32_t page_offset = offset_in_page(reloc->offset);
-       uint64_t delta = reloc->delta + target_offset;
+       uint64_t delta = relocation_target(reloc, target_get);
        char *vaddr;
        int ret;
 
@@ -292,7 +298,7 @@ relocate_entry_gtt(struct drm_i915_gem_object *obj,
 {
        struct drm_device *dev = obj->base.dev;
        struct drm_i915_private *dev_priv = dev->dev_private;
-       uint64_t delta = reloc->delta + target_offset;
+       uint64_t delta = relocation_target(reloc, target_get);
        uint64_t offset;
        void __iomem *reloc_page;
        int ret;
@@ -347,7 +353,7 @@ relocate_entry_clflush(struct drm_i915_gem_object *obj,
 {
        struct drm_device *dev = obj->base.dev;
        uint32_t page_offset = offset_in_page(reloc->offset);
-       uint64_t delta = (int)reloc->delta + target_offset;
+       uint64_t delta = relocation_target(reloc, target_get);
        char *vaddr;
        int ret;
 
Otherwise we may cross the 1<<47 boundary without swapping into cannonical
form.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx