Re: [Intel-xe] [PATCH] drm/xe: Extend drm_xe_vm_bind_op

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Thu, Sep 07, 2023 at 04:51:21PM +0300, Mika Kuoppala wrote:
> Rodrigo Vivi <rodrigo.vivi@intel.com> writes:
> 
> > On Mon, Sep 04, 2023 at 05:46:44PM +0300, Mika Kuoppala wrote:
> >> The bind api is extensible but for a single bind op, there
> >> is not a mechanism to extend. Add extensions field to
> >> struct drm_xe_vm_bind_op.
> >
> > But why would you want to extend the operation?
> > Except for the destroy ones, every ioctl itself is extensible.
> >
> > So, DRM_IOCTL_XE_VM_BIND is extensible. Why would we need to get
> > prepared to extend the operations themselves? And if we extend
> > the operation, what to do with the extension at the ioctl level?
> > which one has precedence? how to organize that?
> >
> 
> The intent is to pass debugger metadata as part of particular
> vm bind operation. For example on MAP, we could associate
> ELF/ISA (relevant parts) as metadata for this bind range.
> 
> So in vector of binds, we want to tag specific one map (in between)
> with debugger metadata.
> 
> With extending the XE_VM_BIND itself, this could be possible too
> but would then need to deliver index into the vector instead
> of carrying the metadata as part of per operation.
> As atleast in this example, the extension is heavily tied
> into particular OP (map).
> 
> I take that you mean precedence of VM_BIND vs bind op?
> Excellent question and I dont know all the use cases the
> vm_bind have to cater. So I can only refer to example above,
> VM_BIND extensions would be only in scope to all operations
> and vm_bind_op extensions would be tightly coupled to per
> operation only.

So, maybe we should do the same union that we do with the
ops themselves and make the extension also an array of num_binds ?

> 
> Thanks for feedback!
> -Mika
> 
> >> 
> >> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> >> Cc: Matthew Brost <matthew.brost@intel.com>
> >> Cc: Lucas De Marchi <lucas.demarchi@intel.com>
> >> Cc: Francois Dugast <francois.dugast@intel.com>
> >> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> >> Cc: Dominik Grzegorzek <dominik.grzegorzek@intel.com>
> >> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> >> ---
> >>  include/uapi/drm/xe_drm.h | 3 +++
> >>  1 file changed, 3 insertions(+)
> >> 
> >> diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h
> >> index 86f16d50e9cc..5c6c86f5e5fc 100644
> >> --- a/include/uapi/drm/xe_drm.h
> >> +++ b/include/uapi/drm/xe_drm.h
> >> @@ -552,6 +552,9 @@ struct drm_xe_vm_destroy {
> >>  };
> >>  
> >>  struct drm_xe_vm_bind_op {
> >> +	/** @extensions: Pointer to the first extension struct, if any */
> >> +	__u64 extensions;
> >> +
> >>  	/**
> >>  	 * @obj: GEM object to operate on, MBZ for MAP_USERPTR, MBZ for UNMAP
> >>  	 */
> >> -- 
> >> 2.34.1
> >>