[Intel-gfx] [PATCH] aubdump: Handle 48-bit relocations properly

[Lionel Landwerlin]

Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin at 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 at 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;