[Intel-gfx] [PATCH v7] drm/i915: Add soft-pinning API for execbuffer

[Michel Thierry]

On 12/9/2015 12:34 PM, Tvrtko Ursulin wrote:
>
> On 09/12/15 10:51, Chris Wilson wrote:
>> On Wed, Dec 09, 2015 at 10:30:41AM +0000, Tvrtko Ursulin wrote:
>>>
>>> On 08/12/15 18:49, Michel Thierry wrote:
>>>> On 12/8/2015 11:55 AM, Thomas Daniel wrote:
>>>>> From: Chris Wilson <chris at chris-wilson.co.uk>
>>>>>
>>>>> Userspace can pass in an offset that it presumes the object is located
>>>>> at. The kernel will then do its utmost to fit the object into that
>>>>> location. The assumption is that userspace is handling its own object
>>>>> locations (for example along with full-ppgtt) and that the kernel will
>>>>> rarely have to make space for the user's requests.
>>>>>
>>>>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>>>>>
>>>>> v2: Fixed incorrect eviction found by Michal Winiarski - fix suggested
>>>>> by Chris
>>>>> Wilson.  Fixed incorrect error paths causing crash found by Michal
>>>>> Winiarski.
>>>>> (Not published externally)
>>>>>
>>>>> v3: Rebased because of trivial conflict in object_bind_to_vm.  Fixed
>>>>> eviction
>>>>> to allow eviction of soft-pinned objects when another soft-pinned
>>>>> object used
>>>>> by a subsequent execbuffer overlaps reported by Michal Winiarski.
>>>>> (Not published externally)
>>>>>
>>>>> v4: Moved soft-pinned objects to the front of ordered_vmas so that
>>>>> they are
>>>>> pinned first after an address conflict happens to avoid repeated
>>>>> conflicts in
>>>>> rare cases (Suggested by Chris Wilson).  Expanded comment on
>>>>> drm_i915_gem_exec_object2.offset to cover this new API.
>>>>>
>>>>> v5: Added I915_PARAM_HAS_EXEC_SOFTPIN parameter for detecting this
>>>>> capability
>>>>> (Kristian). Added check for multiple pinnings on eviction (Akash).
>>>>> Made sure
>>>>> buffers are not considered misplaced without the user specifying
>>>>> EXEC_OBJECT_SUPPORTS_48B_ADDRESS.  User must assume responsibility for
>>>>> any
>>>>> addressing workarounds.  Updated object2.offset field comment again to
>>>>> clarify
>>>>> NO_RELOC case (Chris).  checkpatch cleanup.
>>>>>
>>>>> v6: Trivial rebase on latest drm-intel-nightly
>>>>>
>>>>> v7: Catch attempts to pin above the max virtual address size and
>>>>> return
>>>>> EINVAL (Tvrtko). Decouple EXEC_OBJECT_SUPPORTS_48B_ADDRESS and
>>>>> EXEC_OBJECT_PINNED flags, user must pass both flags in any attempt
>>>>> to pin
>>>>> something at an offset above 4GB (Chris, Daniel Vetter).
>>>>>
>>>>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>>>>> Cc: Akash Goel <akash.goel at intel.com>
>>>>> Cc: Vinay Belgaumkar <vinay.belgaumkar at intel.com>
>>>>> Cc: Michal Winiarski <michal.winiarski at intel.com>
>>>>> Cc: Zou Nanhai <nanhai.zou at intel.com>
>>>>> Cc: Kristian Høgsberg <hoegsberg at gmail.com>
>>>>> Cc: Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com>
>>>>> Signed-off-by: Thomas Daniel <thomas.daniel at intel.com>
>>>>> ---
>>>>>   drivers/gpu/drm/i915/i915_dma.c            |  3 ++
>>>>>   drivers/gpu/drm/i915/i915_drv.h            |  2 +
>>>>>   drivers/gpu/drm/i915/i915_gem.c            | 64
>>>>> ++++++++++++++++++++----------
>>>>>   drivers/gpu/drm/i915/i915_gem_evict.c      | 39 ++++++++++++++++++
>>>>>   drivers/gpu/drm/i915/i915_gem_execbuffer.c | 16 +++++++-
>>>>>   include/uapi/drm/i915_drm.h                | 12 ++++--
>>>>>   6 files changed, 111 insertions(+), 25 deletions(-)
>>>>>
>>>>
>>>> Extra support from the other patch aside, v6 already had rb from Akash
>>>> and this one,
>>>> Reviewed-by: Michel Thierry <michel.thierry at intel.com>
>>>
>>> This patch was acked by the PDT so I merged it to drm-intel-next-queued.
>>
>> Please revert immediately. We need to fix the ABI for canonical
>> addressing before proceeding. Then please work on the better patch.
>
> Sounds like this is a valid comment, guys please check the thread with
> subject "[PATCH v2] drm/i915: Avoid writing relocs with addresses in
> non-canonical form".
>
> amd64 ABI mandates rules on virtual addresess -
> https://en.wikipedia.org/wiki/X86-64#Canonical_form_addresses.
>
> Regards,
>
> Tvrtko

And if the someone tries to use softpin with a virtual address in 
non-canonical form, reject with EINVAL?