Re: [PATCH 3/6] drm/i915: Add support for stealing purgable stolen pages

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Apr 10, 2014 at 10:12:39AM +0000, Gupta, Sourab wrote:
> On Wed, 2014-04-09 at 13:06 +0000, Daniel Vetter wrote:
> > On Tue, Apr 08, 2014 at 06:53:03AM +0000, Gupta, Sourab wrote:
> > > On Tue, 2014-04-08 at 06:45 +0000, Chris Wilson wrote:
> > > > On Tue, Apr 08, 2014 at 04:32:02AM +0000, Gupta, Sourab wrote:
> > > > > Hi Rodrigo,
> > > > > In this patch, while freeing the purgeable stolen object, the memory
> > > > > node also has to be freed, so as to make space for new object. We need
> > > > > to call drm_mm_remove_node while freeing obj.
> > > > > 
> > > > > The below modification patch was floated earlier for this purpose:
> > > > > http://lists.freedesktop.org/archives/intel-gfx/2014-March/041282.html
> > > > 
> > > > Right, I have a v2 locally with the fix you identified.
> > > > -Chris
> > > > 
> > > Ok, Thanks Chris.
> > 
> > I'd really prefer if someone would pick up all the
> > stolen/create2_ioctl/whatever patches, pack them up into a polished
> > series, add the testcases and submit this all for review and merging.
> > 
> > Otherwise this will linger forever and we'll get nowhere. Chris seems
> > swamped with other stuff, so Sourab could you please take a look at this?
> > 
> > Please check with your manager that you have sufficient bandwidth to pull
> > this through.
> > 
> 
> I'll be on vacation from next week, so I'll be able to gauge this better
> after coming back.
> Nevertheless, I have some questions regarding the expectation of
> userspace code changes required for these patches (i.e. libdrm changes
> and igt testcases)
> 
> 1) For libdrm , I am assuming, a counterpart of
> drm_intel_gem_bo_alloc_tiled() function would call the create2 ioctl and
> take in the parameters needed. 
> Should the caching of objects from libdrm need to take care of both the
> placement domains seperately (as in different sets of bo buckets)?
> Should libdrm be transparent to all the combinations of different
> parameters being passed by user or should the prohibited combinations be
> disallowed from libdrm side?

I'm not sure whether we need a cache implemented in libdrm. Since stolen
objects are fairly special it's probably easier to just have a simple
linear cache tailor-made in the respective UMD. So just exposing
create2_ioctl should be good enough.

> 2) For the igt, since we have a lot of parameters exposed to user, the
> number of subtests required may be huge and still they may not test out
> everything. 
> So, Is the expectation here to have exhaustive test cases for all the
> parameters (tiling/cache/domain/madvise/offset etc.) going in as input
> to the create2 ioctl?
> For eg. let us say we are going to check the render copy operation of
> src and dest bo's. Do we need to provide all possible combinations of
> different (create2 ioctl) input parameters to these src and dest bo's
> and then run the render copy test for all these combinations.
> Any guiding pointers from your side as to how we may go about the igt
> testcases?

At a high-level there's two parts for igt tests:
- First is functional tests, where we try to make sure that the feature
  actually works. I.e. allocate some stolen memory and then do something
  with it, making sure that data access for the gpu and similar things
  work. For this we just want some reasonable base coverage so that when
  we hit a bug somewhere it's easy to extend the testcase to cover that
  bug with a specific subtest.

- Then there's interface testing. kernel/userspace is a trust barrier, so
  we need to make sure that evil userspace can't make the kernel crash
  with some crazy invalid combination of flags or operations (like create
  a stolen object and then try to mmap it). Since this is security
  relevant and also since we can't ever change established userspace ABI I
  want full coverage of all cases. But this is just about detecting abuse
  correctly, no functional tests here.

For details see my two blog posts on the topic:

http://blog.ffwll.ch/2013/11/botching-up-ioctls.html

http://blog.ffwll.ch/2013/11/testing-requirements-for-drmi915.html

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch