On 2018-02-12 11:57 AM, Felix Kuehling wrote:
>>> I just thought of a slightly different approach I would consider more
>>> realistic, without having thought through all the details: Adding
>>> KFD-specific memory management ioctls to the amdgpu device. Basically
>>> call amdgpu_amdkfd_gpuvm functions from amdgpu ioctl functions instead
>>> of KFD ioctl functions. But we'd still have KFD ioctls for other things,
>>> and the new amdgpu ioctls would be KFD-specific and not useful for
>>> graphics applications. It's probably still several weeks of work, but
>>> shouldn't have major teething issues because the internal logic and
>>> functionality would be basically unchanged. It would just move the
>>> ioctls from one device to another.
>> My thinking went into a similar direction. But instead of exposing the
>> KFD IOCTLs through the DRM FD, I would let the KFD import a DRM FD.
>>
>> And then use the DRM FD in the KFD for things like the buffer provider
>> of a device, e.g. no separate IDR for BOs in the KFD but rather a
>> reference to the DRM FD.
> I'm not sure I understand this. With "DRM FD" I think you mean the
> device file descriptor. Then have KFD call file operation on the FD to
> call AMDGPU ioctls? Is there any precedent for this that I can look at
> for an example?
OK, I think this could work for finding a VM from a DRM file descriptor:
struct file *filp = fget(fd);
struct drm_file *drm_priv = filp->private_data;
struct amdgpu_fpriv *drv_priv = file_priv->driver_priv;
struct amdgpu_vm *vm = &drv_priv->vm;
That would let us use the DRM VM instead of creating our own and would
avoid wasting a perfectly good VM that gets created by opening the DRM
device. We'd need a new ioctl to import VMs into KFD. But that's about
as far as I would take it.
We'd still need to add KFD-specific data to the VM. If we use an
existing VM, we can't wrap it in our own structure any more. We'd need
to come up with a different solution or extend struct amdgpu_vm with
what we need.
And we still need our own BO creation ioctl. Importing DMABufs adds
extra overhead (more system calls, file descriptors, GEM objects) and
doesn't cover some of our use cases (userptr). We also need our own idr
for managing buffer handles that are used with all our memory and VM
management functions.
Regards,
 Felix
>
> Or do you mean a DMABuf FD exported by GEM? After importing the buffer
> and closing the FD, we still need a way to refer to the buffer for
> managing the GPU mappings and for releasing the reference. So we still
> need handles that are currently provided by our own IDR. I think we'll
> still need that.
>
> Finally, the kfd_ioctl_alloc_memory_of_gpu will later also be used for
> creating userptr BOs. Those can't be exported as DMABufs, so we still
> need our own ioctl for creating that type of BO. That was going to be my
> next patch series.
>
> Regards,
> Â Felix
>
