Re: [PATCH] igt/gem_create_stolen: Verifying extended gem_create ioctl

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, May 11, 2015 at 12:17:09AM +0530, Ankitprasad Sharma wrote:
> On Fri, 2015-05-08 at 09:16 +0200, Daniel Vetter wrote:
> > On Fri, May 08, 2015 at 10:54:26AM +0530, Ankitprasad Sharma wrote:
> > > On Thu, 2015-05-07 at 08:52 +0200, Daniel Vetter wrote:
> > > > On Wed, May 06, 2015 at 03:51:52PM +0530, ankitprasad.r.sharma@intel.com wrote:
> > > > > From: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> > > > > 
> > > > > This patch adds the testcases for verifying the new extended
> > > > > gem_create ioctl. By means of this extended ioctl, memory
> > > > > placement of the GEM object can be specified, i.e. either
> > > > > shmem or stolen memory.
> > > > > These testcases include functional tests and interface tests for
> > > > > testing the gem_create ioctl call for stolen memory placement
> > > > > 
> > > > > v2: Testing pread/pwrite functionality for stolen backed objects,
> > > > > added local struct for extended gem_create and gem_get_aperture,
> > > > > until headers catch up (Chris)
> > > > > 
> > > > > v3: Removed get_aperture related functions, extended gem_pread
> > > > > to compare speeds for user pages with and without page faults,
> > > > > unexposed local_gem_create struct, changed gem_create_stolen
> > > > > usage (Chris)
> > > > > 
> > > > > Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
> > > > 
> > > > An igt to check for invalid arguments of the gem create ioctl (especially
> > > > the newly added flags parameters) seems to be missing.
> > > > -Daniel
> > > 
> > > \Wwe already have a test to check invalid arguments for the newly added
> > > flags parameter in the current set of tests.
> > > 
> > > static void invalid_flag_test(int fd)
> > 
> > Oh right I totally missed that. Especially for future extension I think
> > Chris' idea to split up tests sounds really good, i.e.
> > 
> > gem_create/invalid-flags (the one testcase I didn't spot)
> > gem_stolen/<all the other tests>
> > 
> > Otherwise the next person extending gem_create will miss your
> > invalid-flags test for it.
> > Thanks, Daniel
> 
> As I see, to validate the parameters for gem_create ioctl there needs to
> be verification for size too, other than the newly added parameter
> flags. 
> I will create new igt for validating the parameters of gem_create ioctl.
> (i.e. invalid size and flags)
> Can you please suggest any other tests that can be added to the
> gem_create igt?

Besides testing a few invalid sizes (0, not page-aligned) and ensuring
invalid flags get rejected I don't think we need more gem_create ioctl
input validation tests.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx