[Intel-gfx] [PATCH 05/11] drm/i915: Support for creating Stolen memory backed objects

[Tvrtko Ursulin]

On 14/01/16 11:14, Chris Wilson wrote:
> On Thu, Jan 14, 2016 at 10:46:39AM +0000, Tvrtko Ursulin wrote:
>>
>> On 14/01/16 10:24, Chris Wilson wrote:
>>> On Thu, Jan 14, 2016 at 11:46:41AM +0530, ankitprasad.r.sharma at intel.com wrote:
>>>> From: Ankitprasad Sharma <ankitprasad.r.sharma at intel.com>
>>>>
>>>> Extend the drm_i915_gem_create structure to add support for
>>>> creating Stolen memory backed objects. Added a new flag through
>>>> which user can specify the preference to allocate the object from
>>>> stolen memory, which if set, an attempt will be made to allocate
>>>> the object from stolen memory subject to the availability of
>>>> free space in the stolen region.
>>>>
>>>> v2: Rebased to the latest drm-intel-nightly (Ankit)
>>>>
>>>> v3: Changed versioning of GEM_CREATE param, added new comments (Tvrtko)
>>>>
>>>> v4: Changed size from 32b to 64b to prevent userspace overflow (Tvrtko)
>>>> Corrected function arguments ordering (Chris)
>>>>
>>>> v5: Corrected function name (Chris)
>>>>
>>>> v6: Updated datatype for flags to keep sizeof(drm_i915_gem_create) u64
>>>> aligned (Chris)
>>>>
>>>> v7: Use first 8 bits of gem_create flags for placement (Chris), Add helper
>>>> function for object allocation from stolen region (Ankit)
>>>>
>>>> Testcase: igt/gem_stolen
>>>>
>>>> Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma at intel.com>
>>>
>>> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>
>>>
>>>> +	/**
>>>> +	 * Requested flags (currently used for placement
>>>> +	 * (which memory domain))
>>>> +	 *
>>>> +	 * You can request that the object be created from special memory
>>>> +	 * rather than regular system pages using this parameter. Such
>>>> +	 * irregular objects may have certain restrictions (such as CPU
>>>> +	 * access to a stolen object is verboten).
>>>> +	 *
>>>> +	 * This can be used in the future for other purposes too
>>>> +	 * e.g. specifying tiling/caching/madvise
>>>> +	 */
>>>> +	__u64 flags;
>>>> +#define I915_CREATE_PLACEMENT_NORMAL 	0 /* standard swappable bo  */
>>>> +#define I915_CREATE_PLACEMENT_STOLEN 	1 /* Cannot use CPU mmaps */
>>>
>>> /* Allocate the object from memory reserved for the igfx (stolen).
>>>   *
>>>   * Objects allocated from stolen are restricted in the API they can use,
>>>   * as direct CPU access to stolen memory is prohibited by the system.
>>>   * This means that you cannot use a regular CPU mmap (either using WB
>>>   * or with the WC extension). You can still use a GTT mmap, pwrite,
>>>   * pread and pass it around for use by execbuffer and between processes
>>>   * like normal.
>>
>> What happens when a stolen batch buffer gets to relocate_entry_cpu ?
>
> The kernel goes bang. It is easy to fix. To cover all corner cases, such
> as the stolen object being larger than the mappable aperture, requires
> the insert-page routines though.

I know, I keep pointing it out. :/

>>>   * Stolen memory is a very limited resource and certain functions of the
>>>   * hardware can only work from within stolen memory. Userspace's
>>>   * allocations may be evicted from stolen and moved to normal memory as
>>>   * required. If the allocation is marked as purgeable (using madvise),
>>>   * the allocation will be dropped and further access to the object's
>>>   * backing storage will result in -EFAULT. Stolen objects will also be
>>>   * migrated to normal memory across suspend and resume, as the stolen
>>>   * memory is not preserved.
>>>   *
>>>   * Stolen memory is regarded as a resource placement hint, most suitable
>>>   * for medium-sized buffers that are only accessed by the GPU and can be
>>>   * discarded.
>>>   */
>>
>> Would it be better if the placement hint did not specifically
>> mention stolen memory but specific limitations? Like,
>
> It is a nice idea, but it doesn't really fit with the other placement
> domains I have sketched out (they are all fully featured, or nearly so,
> but exist in different pools of memory with different characteristics
> and reasons for choice).

Perhaps these pools have some characteristics which could be described 
abstractly? I am of course coming back to why would userspace have to 
know about stolen, why would we fail object creation if there is no 
space, and then move objects out of stolen on hibernate and never put 
them back. Sounds like a bad and inflexible interface, and a lying 
kernel to me.

Regards,

Tvrtko