From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tvrtko Ursulin Subject: Re: [PATCH] tests/gem_userptr_blits: Expanded userptr test cases Date: Thu, 30 Jan 2014 09:20:38 +0000 Message-ID: <52EA1966.4080300@linux.intel.com> References: <1391002254-28826-1-git-send-email-tvrtko.ursulin@linux.intel.com> <20140129201133.GB11705@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id B9E89FA7DC for ; Thu, 30 Jan 2014 01:20:40 -0800 (PST) In-Reply-To: <20140129201133.GB11705@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Daniel Vetter Cc: Intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org Hi, On 01/29/2014 08:11 PM, Daniel Vetter wrote: > On Wed, Jan 29, 2014 at 01:30:54PM +0000, Tvrtko Ursulin wrote: >> From: Tvrtko Ursulin >> >> A set of userptr test cases to support the new feature. [snip] > Minor thing on patch style: I'd split this up into 3 parts: > - Extraction of the helpers - always useful to shine a bit light onto new > helper stuff so other people also notice them. > - The new testcase. > - Removal of the old vmap testcase. I was afraid someone will say that, but was hoping for a lower bar since, to quote what yourself say later in this mail, "is a bit evil, but this is a testuite ;-)". :) Okay, I'll split it up. > The other patch style thing are the helpers - the forking_eviction stuff > doesn't really sound like a bit of helper code. igt_exchange_uint32_t > certainly is, but the other stuff I'd just put into a common source file > which is included by both tests. Yeah #include "common.c" is a bit evil, > but this is a testsuite ;-) Or just copy&paste the code into the userptr > test, which is the approach I'd have done. It would be too much c&p for my liking so I chose this route. > Now for test coverage it sounds like this testcases here has been more > than good enough to shake down the userptr implementation, so I think > we're covered. > > But there is also basic interface coverage for sanity-checking and > defending against evil userspace. For this case here I think we need: > > - Tests with un-aligned ptr/size. > - Tests with invalid flags. Above are already there as test_usage_restrictions and test_input_checking. > - Basic nastiness of handing in an invalid pointer. You mean trying to map something which doesn't exist in user address space? Any idea how to obtain such a pointer? Or just use zero? > Then there's all the interactions with other gem interfaces: > - pwrite/pread/set_tiling: Should probably all fail with -EINVAL or > something like that. Ok. > - dma-buf export/gem flink: should succeed. > - But: dma-buf mapping for a foreign device should fail. This will be a > pain to test on Android since we don't have anything else really. I can > take that and do a test like the pile of prime_nv tests we have. Flink is there in forking_evictions. Dmabuf is something I know nothing at the moment so I'll have to look into it. > - gtt mmaps: Theoretically works, but dunno whether it makes sense. According to Chris not on all architectures, I have to find relevant documentation for that. > - Anything esle I've forgotten? Don't know, but thanks for your comments! Regards, Tvrtko