diff mbox

aubdump: Handle 48-bit relocations properly

Message ID 1479498686-8710-1-git-send-email-jason.ekstrand@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jason Ekstrand Nov. 18, 2016, 7:51 p.m. UTC
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(-)

Comments

Lionel Landwerlin Nov. 22, 2016, 4:34 p.m. UTC | #1
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;
diff mbox

Patch

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;