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@xxxxxxxxxxxxxxx] Sent: Wednesday, December 9, 2015 5:36 AM To: Thierry, Michel; Chris Wilson; Daniel, Thomas; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Belgaumkar, Vinay; Goel, Akash; Kristian Høgsberg Subject: Re: [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@xxxxxxxxxxxxxxxxxx> >>>>>> >>>>>> 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@xxxxxxxxxxxxxxxxxx> >>>>>> >>>>>> 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@xxxxxxxxxxxxxxxxxx> >>>>>> Cc: Akash Goel <akash.goel@xxxxxxxxx> >>>>>> Cc: Vinay Belgaumkar <vinay.belgaumkar@xxxxxxxxx> >>>>>> Cc: Michal Winiarski <michal.winiarski@xxxxxxxxx> >>>>>> Cc: Zou Nanhai <nanhai.zou@xxxxxxxxx> >>>>>> Cc: Kristian Høgsberg <hoegsberg@xxxxxxxxx> >>>>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx> >>>>>> Signed-off-by: Thomas Daniel <thomas.daniel@xxxxxxxxx> >>>>>> --- >>>>>> 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@xxxxxxxxx> >>>> >>>> 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@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
