[PATCH] aubdump: Handle 48-bit relocations properly

[PATCH] aubdump: Handle 48-bit relocations properly

[Jason Ekstrand]

aubdump was only writing 32-bits regardless of platform.  This is fine if
the client being dumped leaves the top 32 bits zero since the aubdump GTT
is fairly small.  However, if the client does store something in the upper
32 bits, this results in an invalid relocation.

Cc: Kristian Høgsberg <krh@bitplanet.net>
---
 tools/aubdump.c | 31 ++++++++++++++++++++++++++++---
 1 file changed, 28 insertions(+), 3 deletions(-)

diff --git a/tools/aubdump.c b/tools/aubdump.c
index 4e0d0a8..0cf993d 100644
--- a/tools/aubdump.c
+++ b/tools/aubdump.c
@@ -284,6 +284,32 @@ aub_dump_ringbuffer(uint64_t batch_offset, uint64_t offset, int ring_flag)
 	data_out(ringbuffer, ring_count * 4);
 }
 
+static void
+write_reloc(void *p, uint64_t v)
+{
+	if (gen >= 8) {
+		/* From the Broadwell PRM Vol. 2a,
+		 * MI_LOAD_REGISTER_MEM::MemoryAddress:
+		 *
+		 *	"This field specifies the address of the memory
+		 *	location where the register value specified in the
+		 *	DWord above will read from.  The address specifies
+		 *	the DWord location of the data. Range =
+		 *	GraphicsVirtualAddress[63:2] for a DWord register
+		 *	GraphicsAddress [63:48] are ignored by the HW and
+		 *	assumed to be in correct canonical form [63:48] ==
+		 *	[47]."
+		 *
+		 * In practice, this will always mean the top bits are zero
+		 * because of the GTT size limitation of the aubdump tool.
+		 */
+		const int shift = 63 - 47;
+		*(uint64_t *)p = (((int64_t)v) << shift) >> shift;
+	} else {
+		*(uint32_t *)p = v;
+	}
+}
+
 static void *
 relocate_bo(struct bo *bo, const struct drm_i915_gem_execbuffer2 *execbuffer2,
 	    const struct drm_i915_gem_exec_object2 *obj)
@@ -293,7 +319,6 @@ relocate_bo(struct bo *bo, const struct drm_i915_gem_execbuffer2 *execbuffer2,
 	const struct drm_i915_gem_relocation_entry *relocs =
 		(const struct drm_i915_gem_relocation_entry *) (uintptr_t) obj->relocs_ptr;
 	void *relocated;
-	uint32_t *dw;
 	int handle;
 
 	relocated = malloc(bo->size);
@@ -307,8 +332,8 @@ relocate_bo(struct bo *bo, const struct drm_i915_gem_execbuffer2 *execbuffer2,
 		else
 			handle = relocs[i].target_handle;
 
-		dw = (uint32_t*)(((char *) relocated) + relocs[i].offset);
-		*dw = get_bo(handle)->offset + relocs[i].delta;
+		write_reloc(((char *)relocated) + relocs[i].offset,
+			    get_bo(handle)->offset + relocs[i].delta);
 	}
 
 	return relocated;
-- 
2.5.0.400.gff86faf

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] aubdump: Handle 48-bit relocations properly

[Lionel Landwerlin]

Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>



On 18/11/16 19:51, Jason Ekstrand wrote:
> aubdump was only writing 32-bits regardless of platform.  This is fine if
> the client being dumped leaves the top 32 bits zero since the aubdump GTT
> is fairly small.  However, if the client does store something in the upper
> 32 bits, this results in an invalid relocation.
>
> Cc: Kristian Høgsberg <krh@bitplanet.net>
> ---
>   tools/aubdump.c | 31 ++++++++++++++++++++++++++++---
>   1 file changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/tools/aubdump.c b/tools/aubdump.c
> index 4e0d0a8..0cf993d 100644
> --- a/tools/aubdump.c
> +++ b/tools/aubdump.c
> @@ -284,6 +284,32 @@ aub_dump_ringbuffer(uint64_t batch_offset, uint64_t offset, int ring_flag)
>   	data_out(ringbuffer, ring_count * 4);
>   }
>   
> +static void
> +write_reloc(void *p, uint64_t v)
> +{
> +	if (gen >= 8) {
> +		/* From the Broadwell PRM Vol. 2a,
> +		 * MI_LOAD_REGISTER_MEM::MemoryAddress:
> +		 *
> +		 *	"This field specifies the address of the memory
> +		 *	location where the register value specified in the
> +		 *	DWord above will read from.  The address specifies
> +		 *	the DWord location of the data. Range =
> +		 *	GraphicsVirtualAddress[63:2] for a DWord register
> +		 *	GraphicsAddress [63:48] are ignored by the HW and
> +		 *	assumed to be in correct canonical form [63:48] ==
> +		 *	[47]."
> +		 *
> +		 * In practice, this will always mean the top bits are zero
> +		 * because of the GTT size limitation of the aubdump tool.
> +		 */
> +		const int shift = 63 - 47;
> +		*(uint64_t *)p = (((int64_t)v) << shift) >> shift;
> +	} else {
> +		*(uint32_t *)p = v;
> +	}
> +}
> +
>   static void *
>   relocate_bo(struct bo *bo, const struct drm_i915_gem_execbuffer2 *execbuffer2,
>   	    const struct drm_i915_gem_exec_object2 *obj)
> @@ -293,7 +319,6 @@ relocate_bo(struct bo *bo, const struct drm_i915_gem_execbuffer2 *execbuffer2,
>   	const struct drm_i915_gem_relocation_entry *relocs =
>   		(const struct drm_i915_gem_relocation_entry *) (uintptr_t) obj->relocs_ptr;
>   	void *relocated;
> -	uint32_t *dw;
>   	int handle;
>   
>   	relocated = malloc(bo->size);
> @@ -307,8 +332,8 @@ relocate_bo(struct bo *bo, const struct drm_i915_gem_execbuffer2 *execbuffer2,
>   		else
>   			handle = relocs[i].target_handle;
>   
> -		dw = (uint32_t*)(((char *) relocated) + relocs[i].offset);
> -		*dw = get_bo(handle)->offset + relocs[i].delta;
> +		write_reloc(((char *)relocated) + relocs[i].offset,
> +			    get_bo(handle)->offset + relocs[i].delta);
>   	}
>   
>   	return relocated;


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx