Hi Tvrtko,
[...]
> > @@ -768,8 +773,17 @@ i915_vma_insert(struct i915_vma *vma, struct i915_gem_ww_ctx *ww,
> > GEM_BUG_ON(!IS_ALIGNED(alignment, I915_GTT_MIN_ALIGNMENT));
> > GEM_BUG_ON(!is_power_of_2(alignment));
> > + guard = vma->guard; /* retain guard across rebinds */
> > + if (flags & PIN_OFFSET_GUARD) {
> > + GEM_BUG_ON(overflows_type(flags & PIN_OFFSET_MASK, u32));
> > + guard = max_t(u32, guard, flags & PIN_OFFSET_MASK);
> > + }
> > + roundup(guard, BIT(vma->vm->scratch_order + PAGE_SHIFT));
>
> roundup = ?
ehehe... yes, please ignore, that's some copy/paste error during
the rebase...
> Lets have a comment here as well.
>
> /*
> * Be efficient with PTE use by using the native size for the guard.
> */
>
> Would that be accurate?
and I also forgot the update of my previous comment... yours is
quite accurate.
> > +
> > start = flags & PIN_OFFSET_BIAS ? flags & PIN_OFFSET_MASK : 0;
> > GEM_BUG_ON(!IS_ALIGNED(start, I915_GTT_PAGE_SIZE));
> > + /* We need to be sure we do not ecceed the va area */
> > + GEM_BUG_ON(2 * guard > end);
>
> "exceed" but haven't we said this is not needed?
I wrote it in the cover letter. I had an offline chat with Chris
and he was keen to have this check not only for overflow
protection but also for a documentation purpose so that the
reader knows better about the size and usage of the guard.
Does it make sense?
Thanks a lot!
Andi
