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

[Dave Gordon]

On 11/12/15 12:19, Tvrtko Ursulin wrote:
>
> On 11/12/15 11:22, Ankitprasad Sharma wrote:
>> On Wed, 2015-12-09 at 14:06 +0000, Tvrtko Ursulin wrote:
>>> Hi,
>>>
>>> On 09/12/15 12:46, ankitprasad.r.sharma at intel.com wrote:
>>>> From: Ankitprasad Sharma <ankitprasad.r.sharma at intel.com>
>>>>
[snip!]
>>>> +    /**
>>>> +     * 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
>>>> +     */
>>>> +    __u32 flags;
>>>> +#define I915_CREATE_PLACEMENT_STOLEN     (1<<0) /* Cannot use CPU
>>>> mmaps */
>>>> +#define __I915_CREATE_UNKNOWN_FLAGS
>>>> -(I915_CREATE_PLACEMENT_STOLEN << 1)
>>>
>>> I've asked in another reply, now that userspace can create a stolen
>>> object, what happens if it tries to use it for a batch buffer?
>>>
>>> Can it end up in the relocate_entry_cpu with a batch buffer allocated
>>> from stolen, which would then call i915_gem_object_get_page and crash?
>> Thanks for pointing it out.
>> Yes, this is definitely a possibility, if we allocate batchbuffers from
>> the stolen region. I have started working on that, to do
>> relocate_entry_stolen() if the object is allocated from stolen.
>
> Or perhaps it would be OK to just fail the execbuf?
>
> Just thinking to simplify things. Is it required (or expected) that
> users will need or want to create batch buffers from stolen?
>
> Regards,
> Tvrtko

Let's NOT have batchbuffers in stolen. Or anywhere else exotic, just in 
regular shmfs-backed GEM objects (no phys, userptr, or dma_buf either).
And I'd rather contexts and ringbuffers weren't placed there either, 
because the CPU needs to write those all the time. All special-purpose 
GEM objects should be usable ONLY as data buffers for the GPU, or for 
CPU access with pread/pwrite. The objects that the kernel needs to 
understand and manipulate (contexts, ringbuffers, and batches) should 
always be default (shmfs-backed) GEM objects, so that we don't have to 
propagate the understanding of all the exceptional cases into a 
multitude of different kernel functions.

Oh, and I'd suggest that once we have more than two GEM object types, 
the pread/pwrite operations should be extracted and turned into vfuncs 
rather than adding complexity to the common ioctl/shmfs path.

.Dave.