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

[Zbigniew Kempczyński]

On Tue, Nov 10, 2020 at 01:11:39PM +0200, Imre Deak wrote:
> 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.

Hmm, I haven't thought about it :) When intel-bb is not using relocations
random offset is proposed, I should likely just alter it.  
> 
> Also would it make sense to also do an actual write/read-back to/from
> the relocated offset as an extra check?

Right, will add this check too. I'll add it and send it in  v2.

--
Zbigniew

> 
> > ---
> >  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
> >