[Intel-gfx] [PATCH 1/3] drm/i915: Added a new 'I915_CPU_MAP_NOT_NEEDED' flag to gem_create ioctl.

[Gupta, Sourab]

Hi Chris,

For the issue mentioned by you ( regarding botching up ioctls), we understand that this is related to the 
compatibility between the older/newer versions of driver/userspace.
In our old implementation, the 'pad' field was replaced with 'flags' in the ioctl structure. This would have 
led to the erroneous behavior when the new userspace is communicating with old driver.
 
So, we are planning to mitigate it using following approach:
1) Extend the ioctl structure with 'flags' field at the end of existing fields
2) Add a driver feature flag, so that newer userspace can figure out whether kernel supports the new 
   version of ioctl. Add checks in libdrm corresponding to the same. 
3) In the driver, check the passed in size for the ioctl structure and zero-extend any mismatch( which
     happens in case of older userspace calling newer kernel).
4) In case of newer userspace calling older driver (wherein feature is not supported), we have a few choices:
        a) The userspace will fallback to using the old ioctl structure. Driver will have the new version of the ioctl structure.
             Since the libdrm shared header files will also now have new ioctl structure definition, we'll have to keep an old 
             definition of the ioctl structure privately in order to send it in the ioctl.
        b) Or, we may keep the old ioctl structure as it is in the driver; and define a new structure in driver which will be used now.
              Then, libdrm will also use this new structure (if feature is supported) or old structure (feature not supported)
       c) Or, we may fail the call altogether, if the newer userspace is trying to communicate with older driver.
             We don't need to keep the two different version of ioctl structures in this case. But this may not be the desired behaviour

Can you provide your opinion regarding the above approaches.

Regards,
Sourab


-----Original Message-----
From: Gupta, Sourab 
Sent: Friday, March 07, 2014 4:26 PM
To: Chris Wilson
Cc: intel-gfx at lists.freedesktop.org; Wilson, Chris; Goel, Akash; daniel at ffwll.ch
Subject: RE: [PATCH 1/3] drm/i915: Added a new 'I915_CPU_MAP_NOT_NEEDED' flag to gem_create ioctl.

Hi Chris,

For the issue of clearing of stolen pages, we have a patch which does memset of stolen objs  after allocation, before handing the objs back over to userspace. We can apply this patch and send it in next version.

Regards,
Sourab

-----Original Message-----
From: Chris Wilson [mailto:chris at chris-wilson.co.uk]
Sent: Friday, March 07, 2014 3:36 PM
To: Gupta, Sourab
Cc: intel-gfx at lists.freedesktop.org; Vetter, Daniel; Wilson, Chris; Goel, Akash
Subject: Re: [PATCH 1/3] drm/i915: Added a new 'I915_CPU_MAP_NOT_NEEDED' flag to gem_create ioctl.

On Fri, Mar 07, 2014 at 03:29:25PM +0530, sourab.gupta at intel.com wrote:
> From: Sourab Gupta <sourab.gupta at intel.com>
> 
> Adding the flag 'I915_CPU_MAP_NOT_NEEDED' to gem_create ioctl.
> This is to indicate the driver that direct cpu access to this buffer 
> object is not needed and hence Driver can opt to use Stolen area as a 
> backing store for it. When this flag is set, try to allocate the 
> objects from Stolen area. Fallback to shmem in case of allocation 
> failure.

Besides the issue of
http://blog.ffwll.ch/2013/11/botching-up-ioctls.html, this has an information leak that we are not clearing the stolen pages. Danny got extremely fussy over that when I sent this code almost 9 months ago.
-Chris

--
Chris Wilson, Intel Open Source Technology Centre