[Intel-gfx] [PATCH 1/6] drm/i915: Eliminate vmap overhead for cmd parser

[Chris Wilson]

On Thu, Oct 01, 2015 at 04:29:48PM +0300, Ville Syrjälä wrote:
> On Thu, Oct 01, 2015 at 02:24:53PM +0100, Chris Wilson wrote:
> > On Thu, Oct 01, 2015 at 03:37:21PM +0300, Ville Syrjälä wrote:
> > > On Thu, Oct 01, 2015 at 12:57:10PM +0100, Chris Wilson wrote:
> > > > -	while (cmd < batch_end) {
> > > > -		const struct drm_i915_cmd_descriptor *desc;
> > > > -		u32 length;
> > > > +			k = i;
> > > > +			if (k > PAGE_SIZE - out)
> > > > +				k = PAGE_SIZE - out;
> > > > +			if (k == PAGE_SIZE)
> > > > +				copy_page(dst, src);
> > > > +			else
> > > > +				memcpy(dst + out, src + j, k);
> > > > +
> > > > +			out += k;
> > > > +			j += k;
> > > > +			i -= k;
> > > > +		} while (i);
> > > > +
> > > > +		cmd = src + in;
> > > 
> > > So you're now checking the src batch? What prevents userspace from
> > > overwriting it with eg. NOPS between the time you copied it and the
> > > time you check it?
> > 
> > Zilch. I picked src as it was already in the CPU cache, whereas dst will
> > be WC later. To satisfy you and byt, I need to stage the copy into a
> > temporary page, scan it, then copy into dst.
> > 
> > The silver lining is that it does remove some lines of code. I'm pretty
> > confident that the double copy should not be noticed if I remember about
> > the cache-trashing of kunmap and carefully manage that.
> 
> Yeah, I was thinking that optimally we'd do the copy+scan in cacheline
> units, but maybe that's too small to make it actually efficient?

Yes, the issue there becomes that we need up to 256 bytes (though I
think the largest command is actually ~130bytes) in order to pass a
complete copy of a command to the validator. If we make the temporary
too small we waste time building up partial commands, and the maximum
sane size is one page. __get_free_page() seems a reasonable first
choice, though a kmalloc() from a fresh slab tends to be faster to
allocate.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre