[igt-dev] [PATCH i-g-t] tests/api_intel_bb: Verify delta is properly added in emit relocation

[Imre Deak]

On Tue, Nov 10, 2020 at 09:06:59AM +0100, Zbigniew Kempczyński wrote:
> When delta is added to buffer address and sum exceeds 32bits verify
> emit relocation contains properly set low + high address dwords.
> 
> References: https://patchwork.freedesktop.org/series/83644/
> 
> Signed-off-by: Zbigniew Kempczyński <zbigniew.kempczynski at intel.com>
> Reported-by: Imre Deak <imre.deak at intel.com>
> Cc: Imre Deak <imre.deak at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>

Thanks,
Acked-by: Imre Deak <imre.deak at intel.com>

I was wondering if there is a reaspm why not just set addr.offset and
pin, but I guess this works just as well.

Also would it make sense to also do an actual write/read-back to/from
the relocated offset as an extra check?

> ---
>  tests/i915/api_intel_bb.c | 84 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 84 insertions(+)
> 
> diff --git a/tests/i915/api_intel_bb.c b/tests/i915/api_intel_bb.c
> index 965a7dde..7678fbce 100644
> --- a/tests/i915/api_intel_bb.c
> +++ b/tests/i915/api_intel_bb.c
> @@ -860,6 +860,87 @@ static void offset_control(struct buf_ops *bops)
>  	intel_bb_destroy(ibb);
>  }
>  
> +/*
> + * Idea of the test is to verify delta is properly added to address
> + * when emit_reloc() is called.
> + *
> + * We create 3 buffers aligned to 0x4000_0000, last one has to be
> + * at 0xc000_0000. Then we add 0x7000_0000 to get 0x1_3000_0000.
> + * We cannot use less buffers because delta is s32 (even if it is passed as
> + * u32) so maximum positive value is 0x7fff_ffff.
> + */
> +#define DELTA_BUFFERS 3
> +static void delta_check(struct buf_ops *bops)
> +{
> +	int i915 = buf_ops_get_fd(bops), i;
> +	struct intel_bb *ibb;
> +	struct intel_buf *buf[DELTA_BUFFERS], *target;
> +	uint64_t default_alignment, offset;
> +	uint32_t *ptr, hi, lo;
> +	bool supports_48bit;
> +
> +	ibb = intel_bb_create_with_relocs(i915, PAGE_SIZE);
> +	supports_48bit = ibb->supports_48b_address;
> +	if (!supports_48bit)
> +		intel_bb_destroy(ibb);
> +	igt_require_f(supports_48bit, "We need 48bit ppgtt for testing\n");
> +
> +	if (debug_bb)
> +		intel_bb_set_debug(ibb, true);
> +
> +	/* Alignment has to be power of 2, we need to do 3 buffers */
> +	default_alignment = intel_bb_set_default_object_alignment(ibb, 0x40000000);
> +	for (i = 0; i < ARRAY_SIZE(buf); i++) {
> +		buf[i] = create_buf(bops, WIDTH, HEIGHT, COLOR_CC);
> +		intel_bb_add_object(ibb, buf[i]->handle, intel_buf_bo_size(buf[i]),
> +				    buf[i]->addr.offset, false);
> +	}
> +	intel_bb_set_default_object_alignment(ibb, default_alignment);
> +
> +	intel_bb_out(ibb, MI_BATCH_BUFFER_END);
> +	intel_bb_ptr_align(ibb, 8);
> +
> +	intel_bb_exec(ibb, intel_bb_offset(ibb), I915_EXEC_DEFAULT, false);
> +
> +	target = buf[DELTA_BUFFERS - 1];
> +	/* Last buffer should be @ 0xc000_0000 */
> +	offset = intel_bb_get_object_offset(ibb, target->handle);
> +	igt_assert(offset == 0xc0000000);
> +
> +	intel_bb_reset(ibb, false);
> +
> +	/*
> +	 * We need to do this ugly hack to enforce using passed presumed_offsets
> +	 * in add_object() otherwise -1 is used and relocation is performed.
> +	 */
> +	ibb->enforce_relocs = false;
> +
> +	intel_bb_add_object(ibb, target->handle, intel_buf_bo_size(target),
> +			    offset, false);
> +	intel_bb_out(ibb, MI_BATCH_BUFFER_END);
> +	intel_bb_ptr_align(ibb, 8);
> +
> +	intel_bb_emit_reloc(ibb, target->handle,
> +			    I915_GEM_DOMAIN_RENDER,
> +			    I915_GEM_DOMAIN_RENDER,
> +			    0x70000000, offset);
> +
> +	intel_bb_exec(ibb, intel_bb_offset(ibb), I915_EXEC_DEFAULT, false);
> +
> +	ptr = gem_mmap__device_coherent(i915, ibb->handle, 0, ibb->size, PROT_READ);
> +	lo = ptr[2];
> +	hi = ptr[3];
> +	gem_munmap(ptr, ibb->size);
> +
> +	for (i = 0; i < ARRAY_SIZE(buf); i++)
> +		intel_buf_destroy(buf[i]);
> +	intel_bb_destroy(ibb);
> +
> +	/* Assert after all resources are freed */
> +	igt_assert_f(lo == 0x30000000 && hi == 0x1,
> +		     "intel-bb doesn't properly handle delta in emit relocation\n");
> +}
> +
>  static void full_batch(struct buf_ops *bops)
>  {
>  	int i915 = buf_ops_get_fd(bops);
> @@ -1150,6 +1231,9 @@ igt_main_args("dpib", NULL, help_str, opt_handler, NULL)
>  	igt_subtest("offset-control")
>  		offset_control(bops);
>  
> +	igt_subtest("delta-check")
> +		delta_check(bops);
> +
>  	igt_subtest("full-batch")
>  		full_batch(bops);
>  
> -- 
> 2.26.0
>