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

["Belgaumkar, Vinay" <vinay.belgaumkar@intel.com>]

The stress test will need to be modified to ensure a canonical address(currently uses starting address of 0x8000000000000). The invalid_vma test takes care of the non-canonical scenario in an indirect way. Do we still need a separate test for this change then?

Thanks,
Vinay. 

-----Original Message-----
From: Tvrtko Ursulin [mailto:tvrtko.ursulin@linux.intel.com] 
Sent: Wednesday, December 9, 2015 5:36 AM
To: Thierry, Michel; Chris Wilson; Daniel, Thomas; intel-gfx@lists.freedesktop.org; Belgaumkar, Vinay; Goel, Akash; Kristian Høgsberg
Subject: Re: [Intel-gfx] [PATCH v7] drm/i915: Add soft-pinning API for execbuffer


On 09/12/15 13:33, Michel Thierry wrote:
> 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@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@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@chris-wilson.co.uk>
>>>>>> Cc: Akash Goel <akash.goel@intel.com>
>>>>>> Cc: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>>>>>> Cc: Michal Winiarski <michal.winiarski@intel.com>
>>>>>> Cc: Zou Nanhai <nanhai.zou@intel.com>
>>>>>> Cc: Kristian Høgsberg <hoegsberg@gmail.com>
>>>>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
>>>>>> Signed-off-by: Thomas Daniel <thomas.daniel@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@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?

I think so, since they are not valid userspace virtual addresses.

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx