Nouveau - Sep 2023 - [PATCH drm-misc-next v3 6/7] drm/gpuvm: generalize dma_resv/extobj handling and GEM validation

Hi, Danilo

Some additional minor comments as xe conversion progresses.

On 9/9/23 17:31, Danilo Krummrich wrote:
> So far the DRM GPUVA manager offers common infrastructure to track GPU VA
> allocations and mappings, generically connect GPU VA mappings to their
> backing buffers and perform more complex mapping operations on the GPU VA
> space.
>
> However, there are more design patterns commonly used by drivers, which
> can potentially be generalized in order to make the DRM GPUVA manager
> represent a basic GPU-VM implementation. In this context, this patch aims
> at generalizing the following elements.
>
> 1) Provide a common dma-resv for GEM objects not being used outside of
>     this GPU-VM.
>
> 2) Provide tracking of external GEM objects (GEM objects which are
>     shared with other GPU-VMs).
>
> 3) Provide functions to efficiently lock all GEM objects dma-resv the
>     GPU-VM contains mappings of.
>
> 4) Provide tracking of evicted GEM objects the GPU-VM contains mappings
>     of, such that validation of evicted GEM objects is accelerated.
>
> 5) Provide some convinience functions for common patterns.
>
> Rather than being designed as a "framework", the target is to
make all
> features appear as a collection of optional helper functions, such that
> drivers are free to make use of the DRM GPUVA managers basic
> functionality and opt-in for other features without setting any feature
> flags, just by making use of the corresponding functions.
>
> Big kudos to Boris Brezillon for his help to figure out locking for drivers
> updating the GPU VA space within the fence signalling path.
>
> Suggested-by: Matthew Brost <matthew.brost at intel.com>
> Signed-off-by: Danilo Krummrich <dakr at redhat.com>
> ---
>
> +/**
> + * drm_gpuvm_bo_evict() - add / remove a &drm_gem_object to / from a
> + * &drm_gpuvms evicted list
> + * @obj: the &drm_gem_object to add or remove
> + * @evict: indicates whether the object is evicted
> + *
> + * Adds a &drm_gem_object to or removes it from all &drm_gpuvms
evicted
> + * list containing a mapping of this &drm_gem_object.
> + */
> +void
> +drm_gpuvm_bo_evict(struct drm_gem_object *obj, bool evict)
> +{
> +	struct drm_gpuvm_bo *vm_bo;
> +
> +	drm_gem_for_each_gpuvm_bo(vm_bo, obj) {
> +		if (evict)
> +			drm_gpuvm_bo_list_add(vm_bo, evict);
> +		else
> +			drm_gpuvm_bo_list_del(vm_bo, evict);
> +	}
> +}
> +EXPORT_SYMBOL_GPL(drm_gpuvm_bo_evict);
> +
We need a drm_gpuvm_bo_evict(struct drm_gpuvm_bo *vm_bo, ...) that puts 
a single gpuvm_bo on the list, the above function could perhaps be 
renamed as drm_gpuvm_gem_obj_evict(obj, ....).

Reason is some vm's are faulting vms which don't have an evict list, but
validate from the pagefault handler. Also evict == false is dangerous 
because if called from within an exec, it might remove the obj from 
other vm's evict list before they've had a chance to rebind their VMAs.
>   static int
>   __drm_gpuva_insert(struct drm_gpuvm *gpuvm,
>   		   struct drm_gpuva *va)
> diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
> index afa50b9059a2..834bb6d6617e 100644
> --- a/include/drm/drm_gpuvm.h
> +++ b/include/drm/drm_gpuvm.h
> @@ -26,10 +26,12 @@
>    */
>   
>   #include <linux/list.h>
> +#include <linux/dma-resv.h>
>   #include <linux/rbtree.h>
>   #include <linux/types.h>
>   
>   #include <drm/drm_gem.h>
> +#include <drm/drm_exec.h>
>   
>   struct drm_gpuvm;
>   struct drm_gpuvm_bo;
> @@ -259,6 +261,38 @@ struct drm_gpuvm {
>   	 * space
>   	 */
>   	struct dma_resv *resv;
> +
> +	/**
> +	 * @extobj: structure holding the extobj list
> +	 */
> +	struct {
> +		/**
> +		 * @list: &list_head storing &drm_gpuvm_bos serving as
> +		 * external object
> +		 */
> +		struct list_head list;
> +
> +		/**
> +		 * @lock: spinlock to protect the extobj list
> +		 */
> +		spinlock_t lock;
> +	} extobj;
> +
> +	/**
> +	 * @evict: structure holding the evict list and evict list lock
> +	 */
> +	struct {
> +		/**
> +		 * @list: &list_head storing &drm_gpuvm_bos currently being
> +		 * evicted
> +		 */
> +		struct list_head list;
> +
> +		/**
> +		 * @lock: spinlock to protect the evict list
> +		 */
> +		spinlock_t lock;
> +	} evict;
>   };
>   
>   void drm_gpuvm_init(struct drm_gpuvm *gpuvm, struct drm_device *drm,
> @@ -268,6 +302,21 @@ void drm_gpuvm_init(struct drm_gpuvm *gpuvm, struct
drm_device *drm,
>   		    const struct drm_gpuvm_ops *ops);
>   void drm_gpuvm_destroy(struct drm_gpuvm *gpuvm);
>   
> +/**
> + * drm_gpuvm_is_extobj() - indicates whether the given &drm_gem_object
is an
> + * external object
> + * @gpuvm: the &drm_gpuvm to check
> + * @obj: the &drm_gem_object to check
> + *
> + * Returns: true if the &drm_gem_object &dma_resv differs from the
> + * &drm_gpuvms &dma_resv, false otherwise
> + */
> +static inline bool drm_gpuvm_is_extobj(struct drm_gpuvm *gpuvm,
> +				       struct drm_gem_object *obj)
> +{
> +	return obj && obj->resv != gpuvm->resv;
> +}
> +
>   static inline struct drm_gpuva *
>   __drm_gpuva_next(struct drm_gpuva *va)
>   {
> @@ -346,6 +395,128 @@ __drm_gpuva_next(struct drm_gpuva *va)
>   #define drm_gpuvm_for_each_va_safe(va__, next__, gpuvm__) \
>   	list_for_each_entry_safe(va__, next__, &(gpuvm__)->rb.list,
rb.entry)
>   
> +/**
> + * struct drm_gpuvm_exec - &drm_gpuvm abstraction of &drm_exec
> + *
> + * This structure should be created on the stack as &drm_exec should
be.
> + *
> + * Optionally, @extra can be set in order to lock additional
&drm_gem_objects.
> + */
> +struct drm_gpuvm_exec {
> +	/**
> +	 * @exec: the &drm_exec structure
> +	 */
> +	struct drm_exec exec;
> +
> +	/**
> +	 * @vm: the &drm_gpuvm to lock its DMA reservations
> +	 */
> +	struct drm_gpuvm *vm;
> +
> +	/**
> +	 * @extra: Callback and corresponding private data for the driver to
> +	 * lock arbitrary additional &drm_gem_objects.
> +	 */
> +	struct {
> +		/**
> +		 * @fn: The driver callback to lock additional &drm_gem_objects.
> +		 */
> +		int (*fn)(struct drm_gpuvm_exec *vm_exec,
> +			  unsigned int num_fences);
> +
> +		/**
> +		 * @priv: driver private data for the @fn callback
> +		 */
> +		void *priv;
> +	} extra;
> +};
> +
> +/**
> + * drm_gpuvm_prepare_vm() - prepare the GPUVMs common dma-resv
> + * @gpuvm: the &drm_gpuvm
> + * @exec: the &drm_exec context
> + * @num_fences: the amount of &dma_fences to reserve
> + *
> + * Calls drm_exec_prepare_obj() for the GPUVMs dummy &drm_gem_object.
> + *
> + * Using this function directly, it is the drivers responsibility to call
> + * drm_exec_init() and drm_exec_fini() accordingly.
> + *
> + * Returns: 0 on success, negative error code on failure.
> + */
> +static inline int
> +drm_gpuvm_prepare_vm(struct drm_gpuvm *gpuvm,
> +		     struct drm_exec *exec,
> +		     unsigned int num_fences)
> +{
> +	return drm_exec_prepare_obj(exec, &gpuvm->d_obj, num_fences);
> +}
> +
> +int drm_gpuvm_prepare_objects(struct drm_gpuvm *gpuvm,
> +			      struct drm_exec *exec,
> +			      unsigned int num_fences);
> +
> +int drm_gpuvm_prepare_range(struct drm_gpuvm *gpuvm,
> +			    struct drm_exec *exec,
> +			    u64 addr, u64 range,
> +			    unsigned int num_fences);
> +
> +int drm_gpuvm_exec_lock(struct drm_gpuvm_exec *vm_exec,
> +			unsigned int num_fences,
> +			bool interruptible);
> +
> +int drm_gpuvm_exec_lock_array(struct drm_gpuvm_exec *vm_exec,
> +			      struct drm_gem_object **objs,
> +			      unsigned int num_objs,
> +			      unsigned int num_fences,
> +			      bool interruptible);
> +
> +int drm_gpuvm_exec_lock_range(struct drm_gpuvm_exec *vm_exec,
> +			      u64 addr, u64 range,
> +			      unsigned int num_fences,
> +			      bool interruptible);
> +
> +/**
> + * drm_gpuvm_lock() - lock all dma-resv of all assoiciated BOs
> + * @gpuvm: the &drm_gpuvm
> + *
> + * Releases all dma-resv locks of all &drm_gem_objects previously
acquired
> + * through drm_gpuvm_lock() or its variants.
> + *
> + * Returns: 0 on success, negative error code on failure.
> + */
> +static inline void
> +drm_gpuvm_exec_unlock(struct drm_gpuvm_exec *vm_exec)
> +{
> +	drm_exec_fini(&vm_exec->exec);
> +}
> +
> +int drm_gpuvm_validate(struct drm_gpuvm *gpuvm);
> +void drm_gpuvm_resv_add_fence(struct drm_gpuvm *gpuvm,
> +			      struct drm_exec *exec,
> +			      struct dma_fence *fence,
> +			      enum dma_resv_usage private_usage,
> +			      enum dma_resv_usage extobj_usage);
> +
> +/**
> + * drm_gpuvm_exec_resv_add_fence()
> + * @vm_exec: the &drm_gpuvm_exec abstraction
> + * @fence: fence to add
> + * @private_usage: private dma-resv usage
> + * @extobj_usage: extobj dma-resv usage
> + *
> + * See drm_gpuvm_resv_add_fence().
> + */
> +static inline void
> +drm_gpuvm_exec_resv_add_fence(struct drm_gpuvm_exec *vm_exec,
> +			      struct dma_fence *fence,
> +			      enum dma_resv_usage private_usage,
> +			      enum dma_resv_usage extobj_usage)
> +{
> +	drm_gpuvm_resv_add_fence(vm_exec->vm, &vm_exec->exec, fence,
> +				 private_usage, extobj_usage);
> +}
> +
>   /**
>    * struct drm_gpuvm_bo - structure representing a &drm_gpuvm and
>    * &drm_gem_object combination
> @@ -398,6 +569,18 @@ struct drm_gpuvm_bo {
>   			 * gpuva list.
>   			 */
>   			struct list_head gem;
> +
> +			/**
> +			 * @evict: List entry to attach to the &drm_gpuvms
> +			 * extobj list.
> +			 */
> +			struct list_head extobj;
> +
> +			/**
> +			 * @evict: List entry to attach to the &drm_gpuvms evict
> +			 * list.
> +			 */
> +			struct list_head evict;
>   		} entry;
>   	} list;
>   };
> @@ -432,6 +615,9 @@ struct drm_gpuvm_bo *
>   drm_gpuvm_bo_find(struct drm_gpuvm *gpuvm,
>   		  struct drm_gem_object *obj);
>   
> +void drm_gpuvm_bo_evict(struct drm_gem_object *obj, bool evict);
> +void drm_gpuvm_bo_extobj_add(struct drm_gpuvm_bo *vm_bo);
> +
>   /**
>    * drm_gpuvm_bo_for_each_va() - iterator to walk over a list of
&drm_gpuva
>    * @va__: &drm_gpuva structure to assign to in each iteration step
> @@ -837,6 +1023,17 @@ struct drm_gpuvm_ops {
>   	 * used.
>   	 */
>   	int (*sm_step_unmap)(struct drm_gpuva_op *op, void *priv);
> +
> +	/**
> +	 * @bo_validate: called from drm_gpuvm_validate()
> +	 *
> +	 * Drivers receive this callback for every evicted &drm_gem_object
being
> +	 * mapped in the corresponding &drm_gpuvm.
> +	 *
> +	 * Typically, drivers would call their driver specific variant of
> +	 * ttm_bo_validate() from within this callback.
> +	 */
> +	int (*bo_validate)(struct drm_gem_object *obj);
Same here. Could we have a vm_bo as an argument instead, so that the 
callback knows what gpuvm we're targeting and can mark all its gpu_vas 
for revalidation? Or is that intended to be done elsewhere?
>   };
>   
>   int drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, void *priv,
Thanks,

Thomas

On 9/14/23 15:48, Thomas Hellstr?m wrote:
> Hi, Danilo
> 
> Some additional minor comments as xe conversion progresses.
> 
> On 9/9/23 17:31, Danilo Krummrich wrote:
>> So far the DRM GPUVA manager offers common infrastructure to track GPU
VA
>> allocations and mappings, generically connect GPU VA mappings to their
>> backing buffers and perform more complex mapping operations on the GPU
VA
>> space.
>>
>> However, there are more design patterns commonly used by drivers, which
>> can potentially be generalized in order to make the DRM GPUVA manager
>> represent a basic GPU-VM implementation. In this context, this patch
aims
>> at generalizing the following elements.
>>
>> 1) Provide a common dma-resv for GEM objects not being used outside of
>> ??? this GPU-VM.
>>
>> 2) Provide tracking of external GEM objects (GEM objects which are
>> ??? shared with other GPU-VMs).
>>
>> 3) Provide functions to efficiently lock all GEM objects dma-resv the
>> ??? GPU-VM contains mappings of.
>>
>> 4) Provide tracking of evicted GEM objects the GPU-VM contains mappings
>> ??? of, such that validation of evicted GEM objects is accelerated.
>>
>> 5) Provide some convinience functions for common patterns.
>>
>> Rather than being designed as a "framework", the target is to
make all
>> features appear as a collection of optional helper functions, such that
>> drivers are free to make use of the DRM GPUVA managers basic
>> functionality and opt-in for other features without setting any feature
>> flags, just by making use of the corresponding functions.
>>
>> Big kudos to Boris Brezillon for his help to figure out locking for
drivers
>> updating the GPU VA space within the fence signalling path.
>>
>> Suggested-by: Matthew Brost <matthew.brost at intel.com>
>> Signed-off-by: Danilo Krummrich <dakr at redhat.com>
>> ---
>>
>> +/**
>> + * drm_gpuvm_bo_evict() - add / remove a &drm_gem_object to / from
a
>> + * &drm_gpuvms evicted list
>> + * @obj: the &drm_gem_object to add or remove
>> + * @evict: indicates whether the object is evicted
>> + *
>> + * Adds a &drm_gem_object to or removes it from all
&drm_gpuvms evicted
>> + * list containing a mapping of this &drm_gem_object.
>> + */
>> +void
>> +drm_gpuvm_bo_evict(struct drm_gem_object *obj, bool evict)
>> +{
>> +??? struct drm_gpuvm_bo *vm_bo;
>> +
>> +??? drm_gem_for_each_gpuvm_bo(vm_bo, obj) {
>> +??????? if (evict)
>> +??????????? drm_gpuvm_bo_list_add(vm_bo, evict);
>> +??????? else
>> +??????????? drm_gpuvm_bo_list_del(vm_bo, evict);
>> +??? }
>> +}
>> +EXPORT_SYMBOL_GPL(drm_gpuvm_bo_evict);
>> +
> 
> We need a drm_gpuvm_bo_evict(struct drm_gpuvm_bo *vm_bo, ...) that puts a
single gpuvm_bo on the list, the above function could perhaps be renamed as
drm_gpuvm_gem_obj_evict(obj, ....).
Makes sense - gonna change that.
> 
> Reason is some vm's are faulting vms which don't have an evict
list, but validate from the pagefault handler. Also evict == false is dangerous
because if called from within an exec, it might remove the obj from other
vm's evict list before they've had a chance to rebind their VMAs.
> 
>> ? static int
>> ? __drm_gpuva_insert(struct drm_gpuvm *gpuvm,
>> ???????????? struct drm_gpuva *va)
>> diff --git a/include/drm/drm_gpuvm.h b/include/drm/drm_gpuvm.h
>> index afa50b9059a2..834bb6d6617e 100644
>> --- a/include/drm/drm_gpuvm.h
>> +++ b/include/drm/drm_gpuvm.h
>> @@ -26,10 +26,12 @@
>> ?? */
>> ? #include <linux/list.h>
>> +#include <linux/dma-resv.h>
>> ? #include <linux/rbtree.h>
>> ? #include <linux/types.h>
>> ? #include <drm/drm_gem.h>
>> +#include <drm/drm_exec.h>
>> ? struct drm_gpuvm;
>> ? struct drm_gpuvm_bo;
>> @@ -259,6 +261,38 @@ struct drm_gpuvm {
>> ?????? * space
>> ?????? */
>> ????? struct dma_resv *resv;
>> +
>> +??? /**
>> +???? * @extobj: structure holding the extobj list
>> +???? */
>> +??? struct {
>> +??????? /**
>> +???????? * @list: &list_head storing &drm_gpuvm_bos serving as
>> +???????? * external object
>> +???????? */
>> +??????? struct list_head list;
>> +
>> +??????? /**
>> +???????? * @lock: spinlock to protect the extobj list
>> +???????? */
>> +??????? spinlock_t lock;
>> +??? } extobj;
>> +
>> +??? /**
>> +???? * @evict: structure holding the evict list and evict list lock
>> +???? */
>> +??? struct {
>> +??????? /**
>> +???????? * @list: &list_head storing &drm_gpuvm_bos currently
being
>> +???????? * evicted
>> +???????? */
>> +??????? struct list_head list;
>> +
>> +??????? /**
>> +???????? * @lock: spinlock to protect the evict list
>> +???????? */
>> +??????? spinlock_t lock;
>> +??? } evict;
>> ? };
>> ? void drm_gpuvm_init(struct drm_gpuvm *gpuvm, struct drm_device *drm,
>> @@ -268,6 +302,21 @@ void drm_gpuvm_init(struct drm_gpuvm *gpuvm,
struct drm_device *drm,
>> ????????????? const struct drm_gpuvm_ops *ops);
>> ? void drm_gpuvm_destroy(struct drm_gpuvm *gpuvm);
>> +/**
>> + * drm_gpuvm_is_extobj() - indicates whether the given
&drm_gem_object is an
>> + * external object
>> + * @gpuvm: the &drm_gpuvm to check
>> + * @obj: the &drm_gem_object to check
>> + *
>> + * Returns: true if the &drm_gem_object &dma_resv differs from
the
>> + * &drm_gpuvms &dma_resv, false otherwise
>> + */
>> +static inline bool drm_gpuvm_is_extobj(struct drm_gpuvm *gpuvm,
>> +?????????????????????? struct drm_gem_object *obj)
>> +{
>> +??? return obj && obj->resv != gpuvm->resv;
>> +}
>> +
>> ? static inline struct drm_gpuva *
>> ? __drm_gpuva_next(struct drm_gpuva *va)
>> ? {
>> @@ -346,6 +395,128 @@ __drm_gpuva_next(struct drm_gpuva *va)
>> ? #define drm_gpuvm_for_each_va_safe(va__, next__, gpuvm__) \
>> ????? list_for_each_entry_safe(va__, next__,
&(gpuvm__)->rb.list, rb.entry)
>> +/**
>> + * struct drm_gpuvm_exec - &drm_gpuvm abstraction of &drm_exec
>> + *
>> + * This structure should be created on the stack as &drm_exec
should be.
>> + *
>> + * Optionally, @extra can be set in order to lock additional
&drm_gem_objects.
>> + */
>> +struct drm_gpuvm_exec {
>> +??? /**
>> +???? * @exec: the &drm_exec structure
>> +???? */
>> +??? struct drm_exec exec;
>> +
>> +??? /**
>> +???? * @vm: the &drm_gpuvm to lock its DMA reservations
>> +???? */
>> +??? struct drm_gpuvm *vm;
>> +
>> +??? /**
>> +???? * @extra: Callback and corresponding private data for the driver
to
>> +???? * lock arbitrary additional &drm_gem_objects.
>> +???? */
>> +??? struct {
>> +??????? /**
>> +???????? * @fn: The driver callback to lock additional
&drm_gem_objects.
>> +???????? */
>> +??????? int (*fn)(struct drm_gpuvm_exec *vm_exec,
>> +????????????? unsigned int num_fences);
>> +
>> +??????? /**
>> +???????? * @priv: driver private data for the @fn callback
>> +???????? */
>> +??????? void *priv;
>> +??? } extra;
>> +};
>> +
>> +/**
>> + * drm_gpuvm_prepare_vm() - prepare the GPUVMs common dma-resv
>> + * @gpuvm: the &drm_gpuvm
>> + * @exec: the &drm_exec context
>> + * @num_fences: the amount of &dma_fences to reserve
>> + *
>> + * Calls drm_exec_prepare_obj() for the GPUVMs dummy
&drm_gem_object.
>> + *
>> + * Using this function directly, it is the drivers responsibility to
call
>> + * drm_exec_init() and drm_exec_fini() accordingly.
>> + *
>> + * Returns: 0 on success, negative error code on failure.
>> + */
>> +static inline int
>> +drm_gpuvm_prepare_vm(struct drm_gpuvm *gpuvm,
>> +???????????? struct drm_exec *exec,
>> +???????????? unsigned int num_fences)
>> +{
>> +??? return drm_exec_prepare_obj(exec, &gpuvm->d_obj,
num_fences);
>> +}
>> +
>> +int drm_gpuvm_prepare_objects(struct drm_gpuvm *gpuvm,
>> +????????????????? struct drm_exec *exec,
>> +????????????????? unsigned int num_fences);
>> +
>> +int drm_gpuvm_prepare_range(struct drm_gpuvm *gpuvm,
>> +??????????????? struct drm_exec *exec,
>> +??????????????? u64 addr, u64 range,
>> +??????????????? unsigned int num_fences);
>> +
>> +int drm_gpuvm_exec_lock(struct drm_gpuvm_exec *vm_exec,
>> +??????????? unsigned int num_fences,
>> +??????????? bool interruptible);
>> +
>> +int drm_gpuvm_exec_lock_array(struct drm_gpuvm_exec *vm_exec,
>> +????????????????? struct drm_gem_object **objs,
>> +????????????????? unsigned int num_objs,
>> +????????????????? unsigned int num_fences,
>> +????????????????? bool interruptible);
>> +
>> +int drm_gpuvm_exec_lock_range(struct drm_gpuvm_exec *vm_exec,
>> +????????????????? u64 addr, u64 range,
>> +????????????????? unsigned int num_fences,
>> +????????????????? bool interruptible);
>> +
>> +/**
>> + * drm_gpuvm_lock() - lock all dma-resv of all assoiciated BOs
>> + * @gpuvm: the &drm_gpuvm
>> + *
>> + * Releases all dma-resv locks of all &drm_gem_objects previously
acquired
>> + * through drm_gpuvm_lock() or its variants.
>> + *
>> + * Returns: 0 on success, negative error code on failure.
>> + */
>> +static inline void
>> +drm_gpuvm_exec_unlock(struct drm_gpuvm_exec *vm_exec)
>> +{
>> +??? drm_exec_fini(&vm_exec->exec);
>> +}
>> +
>> +int drm_gpuvm_validate(struct drm_gpuvm *gpuvm);
>> +void drm_gpuvm_resv_add_fence(struct drm_gpuvm *gpuvm,
>> +????????????????? struct drm_exec *exec,
>> +????????????????? struct dma_fence *fence,
>> +????????????????? enum dma_resv_usage private_usage,
>> +????????????????? enum dma_resv_usage extobj_usage);
>> +
>> +/**
>> + * drm_gpuvm_exec_resv_add_fence()
>> + * @vm_exec: the &drm_gpuvm_exec abstraction
>> + * @fence: fence to add
>> + * @private_usage: private dma-resv usage
>> + * @extobj_usage: extobj dma-resv usage
>> + *
>> + * See drm_gpuvm_resv_add_fence().
>> + */
>> +static inline void
>> +drm_gpuvm_exec_resv_add_fence(struct drm_gpuvm_exec *vm_exec,
>> +????????????????? struct dma_fence *fence,
>> +????????????????? enum dma_resv_usage private_usage,
>> +????????????????? enum dma_resv_usage extobj_usage)
>> +{
>> +??? drm_gpuvm_resv_add_fence(vm_exec->vm, &vm_exec->exec,
fence,
>> +???????????????? private_usage, extobj_usage);
>> +}
>> +
>> ? /**
>> ?? * struct drm_gpuvm_bo - structure representing a &drm_gpuvm and
>> ?? * &drm_gem_object combination
>> @@ -398,6 +569,18 @@ struct drm_gpuvm_bo {
>> ?????????????? * gpuva list.
>> ?????????????? */
>> ????????????? struct list_head gem;
>> +
>> +??????????? /**
>> +???????????? * @evict: List entry to attach to the &drm_gpuvms
>> +???????????? * extobj list.
>> +???????????? */
>> +??????????? struct list_head extobj;
>> +
>> +??????????? /**
>> +???????????? * @evict: List entry to attach to the &drm_gpuvms
evict
>> +???????????? * list.
>> +???????????? */
>> +??????????? struct list_head evict;
>> ????????? } entry;
>> ????? } list;
>> ? };
>> @@ -432,6 +615,9 @@ struct drm_gpuvm_bo *
>> ? drm_gpuvm_bo_find(struct drm_gpuvm *gpuvm,
>> ??????????? struct drm_gem_object *obj);
>> +void drm_gpuvm_bo_evict(struct drm_gem_object *obj, bool evict);
>> +void drm_gpuvm_bo_extobj_add(struct drm_gpuvm_bo *vm_bo);
>> +
>> ? /**
>> ?? * drm_gpuvm_bo_for_each_va() - iterator to walk over a list of
&drm_gpuva
>> ?? * @va__: &drm_gpuva structure to assign to in each iteration
step
>> @@ -837,6 +1023,17 @@ struct drm_gpuvm_ops {
>> ?????? * used.
>> ?????? */
>> ????? int (*sm_step_unmap)(struct drm_gpuva_op *op, void *priv);
>> +
>> +??? /**
>> +???? * @bo_validate: called from drm_gpuvm_validate()
>> +???? *
>> +???? * Drivers receive this callback for every evicted
&drm_gem_object being
>> +???? * mapped in the corresponding &drm_gpuvm.
>> +???? *
>> +???? * Typically, drivers would call their driver specific variant of
>> +???? * ttm_bo_validate() from within this callback.
>> +???? */
>> +??? int (*bo_validate)(struct drm_gem_object *obj);
> 
> Same here. Could we have a vm_bo as an argument instead, so that the
callback knows what gpuvm we're targeting and can mark all its gpu_vas for
revalidation? Or is that intended to be done elsewhere?
Makes sense as well. I'll change that too.
> 
>> ? };
>> ? int drm_gpuvm_sm_map(struct drm_gpuvm *gpuvm, void *priv,
> 
> Thanks,
> 
> Thomas
> 
>