[PATCH 0/1] Upstreaming the KFD IPC API

[PATCH 0/1] Upstreaming the KFD IPC API

[Felix Kuehling]

This API is used by MPI/UCX for efficiently sharing VRAM between MPI
ranks on the same node. It has been part of the ROCm DKMS branch for a
long time. This code is refactored to be less invasive for upstreaming.
As a result struct kfd_bo and the associated interval tree is not needed
upstream.

The introduction of this API upstream bumps the KFD API version to 1.4.

The corresponding Thunk change that goes with the proposed upstreaming of
this API can be found here:
https://github.com/RadeonOpenCompute/ROCT-Thunk-Interface/commit/682922bd405caaab64887e8e702b7d996ba9e2a8

Felix Kuehling (1):
  drm/amdkfd: Add IPC API

 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h    |   5 +
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  56 +++-
 drivers/gpu/drm/amd/amdkfd/Makefile           |   3 +-
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c      |  74 ++---
 drivers/gpu/drm/amd/amdkfd/kfd_ipc.c          | 263 ++++++++++++++++++
 drivers/gpu/drm/amd/amdkfd/kfd_ipc.h          |  55 ++++
 drivers/gpu/drm/amd/amdkfd/kfd_module.c       |   5 +
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h         |   5 +
 include/uapi/linux/kfd_ioctl.h                |  62 ++++-
 9 files changed, 488 insertions(+), 40 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/amdkfd/kfd_ipc.c
 create mode 100644 drivers/gpu/drm/amd/amdkfd/kfd_ipc.h

-- 
2.27.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/1] drm/amdkfd: Add IPC API

[Felix Kuehling]

This allows exporting and importing buffers. The API generates handles
that can be used with the HIP IPC API, i.e. big numbers rather than
file descriptors.

Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h    |   5 +
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  56 +++-
 drivers/gpu/drm/amd/amdkfd/Makefile           |   3 +-
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c      |  74 ++---
 drivers/gpu/drm/amd/amdkfd/kfd_ipc.c          | 263 ++++++++++++++++++
 drivers/gpu/drm/amd/amdkfd/kfd_ipc.h          |  55 ++++
 drivers/gpu/drm/amd/amdkfd/kfd_module.c       |   5 +
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h         |   5 +
 include/uapi/linux/kfd_ioctl.h                |  62 ++++-
 9 files changed, 488 insertions(+), 40 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/amdkfd/kfd_ipc.c
 create mode 100644 drivers/gpu/drm/amd/amdkfd/kfd_ipc.h

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index 3f2b695cf19e..0f8dc4c4f924 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -49,6 +49,7 @@ struct kfd_bo_va_list {
 struct kgd_mem {
 	struct mutex lock;
 	struct amdgpu_bo *bo;
+	struct kfd_ipc_obj *ipc_obj;
 	struct list_head bo_va_list;
 	/* protected by amdkfd_process_info.lock */
 	struct ttm_validate_buffer validate_list;
@@ -240,9 +241,13 @@ int amdgpu_amdkfd_gpuvm_get_vm_fault_info(struct kgd_dev *kgd,
 
 int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev *kgd,
 				      struct dma_buf *dmabuf,
+				      struct kfd_ipc_obj *ipc_obj,
 				      uint64_t va, void *vm,
 				      struct kgd_mem **mem, uint64_t *size,
 				      uint64_t *mmap_offset);
+int amdgpu_amdkfd_gpuvm_export_ipc_obj(struct kgd_dev *kgd, void *vm,
+				       struct kgd_mem *mem,
+				       struct kfd_ipc_obj **ipc_obj);
 
 void amdgpu_amdkfd_gpuvm_init_mem_limits(void);
 void amdgpu_amdkfd_unreserve_memory_limit(struct amdgpu_bo *bo);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index c408936e8f98..cd5f23c0c2ca 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -29,6 +29,7 @@
 #include "amdgpu_vm.h"
 #include "amdgpu_amdkfd.h"
 #include "amdgpu_dma_buf.h"
+#include "kfd_ipc.h"
 #include <uapi/linux/kfd_ioctl.h>
 
 /* BO flag to indicate a KFD userptr BO */
@@ -1353,6 +1354,9 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
 			*size = 0;
 	}
 
+	/* Unreference the ipc_obj if applicable */
+	kfd_ipc_obj_put(&mem->ipc_obj);
+
 	/* Free the BO*/
 	drm_gem_object_put_unlocked(&mem->bo->tbo.base);
 	mutex_destroy(&mem->lock);
@@ -1656,6 +1660,7 @@ int amdgpu_amdkfd_gpuvm_get_vm_fault_info(struct kgd_dev *kgd,
 
 int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev *kgd,
 				      struct dma_buf *dma_buf,
+				      struct kfd_ipc_obj *ipc_obj,
 				      uint64_t va, void *vm,
 				      struct kgd_mem **mem, uint64_t *size,
 				      uint64_t *mmap_offset)
@@ -1692,15 +1697,18 @@ int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev *kgd,
 
 	INIT_LIST_HEAD(&(*mem)->bo_va_list);
 	mutex_init(&(*mem)->lock);
-	
-	(*mem)->alloc_flags =
-		((bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM) ?
-		KFD_IOC_ALLOC_MEM_FLAGS_VRAM : KFD_IOC_ALLOC_MEM_FLAGS_GTT)
-		| KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE
-		| KFD_IOC_ALLOC_MEM_FLAGS_EXECUTABLE;
+	if (bo->kfd_bo)
+		(*mem)->alloc_flags = bo->kfd_bo->alloc_flags;
+	else
+		(*mem)->alloc_flags =
+			((bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM) ?
+			KFD_IOC_ALLOC_MEM_FLAGS_VRAM : KFD_IOC_ALLOC_MEM_FLAGS_GTT)
+			| KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE
+			| KFD_IOC_ALLOC_MEM_FLAGS_EXECUTABLE;
 
 	drm_gem_object_get(&bo->tbo.base);
 	(*mem)->bo = bo;
+	(*mem)->ipc_obj = ipc_obj;
 	(*mem)->va = va;
 	(*mem)->domain = (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM) ?
 		AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT;
@@ -1713,6 +1721,42 @@ int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev *kgd,
 	return 0;
 }
 
+int amdgpu_amdkfd_gpuvm_export_ipc_obj(struct kgd_dev *kgd, void *vm,
+				       struct kgd_mem *mem,
+				       struct kfd_ipc_obj **ipc_obj)
+{
+	struct amdgpu_device *adev = NULL;
+	struct dma_buf *dmabuf;
+	int r = 0;
+
+	if (!kgd || !vm || !mem)
+		return -EINVAL;
+
+	adev = get_amdgpu_device(kgd);
+	mutex_lock(&mem->lock);
+
+	if (mem->ipc_obj) {
+		*ipc_obj = mem->ipc_obj;
+		goto unlock_out;
+	}
+
+	dmabuf = amdgpu_gem_prime_export(&mem->bo->tbo.base, 0);
+	if (IS_ERR(dmabuf)) {
+		r = PTR_ERR(dmabuf);
+		goto unlock_out;
+	}
+
+	r = kfd_ipc_store_insert(dmabuf, &mem->ipc_obj);
+	if (r)
+		dma_buf_put(dmabuf);
+	else
+		*ipc_obj = mem->ipc_obj;
+
+unlock_out:
+	mutex_unlock(&mem->lock);
+	return r;
+}
+
 /* Evict a userptr BO by stopping the queues if necessary
  *
  * Runs in MMU notifier, may be in RECLAIM_FS context. This means it
diff --git a/drivers/gpu/drm/amd/amdkfd/Makefile b/drivers/gpu/drm/amd/amdkfd/Makefile
index e1e4115dcf78..815d9a9e7a3c 100644
--- a/drivers/gpu/drm/amd/amdkfd/Makefile
+++ b/drivers/gpu/drm/amd/amdkfd/Makefile
@@ -54,7 +54,8 @@ AMDKFD_FILES	:= $(AMDKFD_PATH)/kfd_module.o \
 		$(AMDKFD_PATH)/kfd_dbgdev.o \
 		$(AMDKFD_PATH)/kfd_dbgmgr.o \
 		$(AMDKFD_PATH)/kfd_smi_events.o \
-		$(AMDKFD_PATH)/kfd_crat.o
+		$(AMDKFD_PATH)/kfd_crat.o \
+		$(AMDKFD_PATH)/kfd_ipc.o
 
 ifneq ($(CONFIG_AMD_IOMMU_V2),)
 AMDKFD_FILES += $(AMDKFD_PATH)/kfd_iommu.o
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index e9b96ad3d9a5..e7d15fa02b5e 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -38,6 +38,7 @@
 #include "kfd_priv.h"
 #include "kfd_device_queue_manager.h"
 #include "kfd_dbgmgr.h"
+#include "kfd_ipc.h"
 #include "amdgpu_amdkfd.h"
 #include "kfd_smi_events.h"
 
@@ -1691,53 +1692,58 @@ static int kfd_ioctl_import_dmabuf(struct file *filep,
 				   struct kfd_process *p, void *data)
 {
 	struct kfd_ioctl_import_dmabuf_args *args = data;
-	struct kfd_process_device *pdd;
-	struct dma_buf *dmabuf;
 	struct kfd_dev *dev;
-	int idr_handle;
-	uint64_t size;
-	void *mem;
 	int r;
 
 	dev = kfd_device_by_id(args->gpu_id);
 	if (!dev)
 		return -EINVAL;
 
-	dmabuf = dma_buf_get(args->dmabuf_fd);
-	if (IS_ERR(dmabuf))
-		return PTR_ERR(dmabuf);
+	r = kfd_ipc_import_dmabuf(dev, p, args->dmabuf_fd,
+				  args->va_addr, &args->handle, NULL);
+	if (r)
+		pr_err("Failed to import dmabuf\n");
 
-	mutex_lock(&p->mutex);
+	return r;
+}
 
-	pdd = kfd_bind_process_to_device(dev, p);
-	if (IS_ERR(pdd)) {
-		r = PTR_ERR(pdd);
-		goto err_unlock;
-	}
+static int kfd_ioctl_ipc_export_handle(struct file *filep,
+				       struct kfd_process *p,
+				       void *data)
+{
+	struct kfd_ioctl_ipc_export_handle_args *args = data;
+	struct kfd_dev *dev;
+	int r;
+
+	dev = kfd_device_by_id(args->gpu_id);
+	if (!dev)
+		return -EINVAL;
 
-	r = amdgpu_amdkfd_gpuvm_import_dmabuf(dev->kgd, dmabuf,
-					      args->va_addr, pdd->vm,
-					      (struct kgd_mem **)&mem, &size,
-					      NULL);
+	r = kfd_ipc_export_as_handle(dev, p, args->handle, args->share_handle);
 	if (r)
-		goto err_unlock;
+		pr_err("Failed to export IPC handle\n");
 
-	idr_handle = kfd_process_device_create_obj_handle(pdd, mem);
-	if (idr_handle < 0) {
-		r = -EFAULT;
-		goto err_free;
-	}
+	return r;
+}
 
-	mutex_unlock(&p->mutex);
+static int kfd_ioctl_ipc_import_handle(struct file *filep,
+				       struct kfd_process *p,
+				       void *data)
+{
+	struct kfd_ioctl_ipc_import_handle_args *args = data;
+	struct kfd_dev *dev = NULL;
+	int r;
 
-	args->handle = MAKE_HANDLE(args->gpu_id, idr_handle);
+	dev = kfd_device_by_id(args->gpu_id);
+	if (!dev)
+		return -EINVAL;
 
-	return 0;
+	r = kfd_ipc_import_handle(p, args->share_handle, args->va_addr,
+				  &args->handle, &args->gpu_id,
+				  &args->mmap_offset);
+	if (r)
+		pr_err("Failed to import IPC handle\n");
 
-err_free:
-	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, (struct kgd_mem *)mem, NULL);
-err_unlock:
-	mutex_unlock(&p->mutex);
 	return r;
 }
 
@@ -1853,6 +1859,12 @@ static const struct amdkfd_ioctl_desc amdkfd_ioctls[] = {
 
 	AMDKFD_IOCTL_DEF(AMDKFD_IOC_SMI_EVENTS,
 			kfd_ioctl_smi_events, 0),
+
+	AMDKFD_IOCTL_DEF(AMDKFD_IOC_IPC_IMPORT_HANDLE,
+				kfd_ioctl_ipc_import_handle, 0),
+
+	AMDKFD_IOCTL_DEF(AMDKFD_IOC_IPC_EXPORT_HANDLE,
+				kfd_ioctl_ipc_export_handle, 0),
 };
 
 #define AMDKFD_CORE_IOCTL_COUNT	ARRAY_SIZE(amdkfd_ioctls)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_ipc.c b/drivers/gpu/drm/amd/amdkfd/kfd_ipc.c
new file mode 100644
index 000000000000..3de8d7826f07
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_ipc.c
@@ -0,0 +1,263 @@
+/*
+ * Copyright 2014 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#include <linux/slab.h>
+#include <linux/random.h>
+
+#include "kfd_ipc.h"
+#include "kfd_priv.h"
+#include "amdgpu_amdkfd.h"
+
+#define KFD_IPC_HASH_TABLE_SIZE_SHIFT 4
+#define KFD_IPC_HASH_TABLE_SIZE_MASK ((1 << KFD_IPC_HASH_TABLE_SIZE_SHIFT) - 1)
+
+static struct kfd_ipc_handles {
+	DECLARE_HASHTABLE(handles, KFD_IPC_HASH_TABLE_SIZE_SHIFT);
+	struct mutex lock;
+} kfd_ipc_handles;
+
+/* Since, handles are random numbers, it can be used directly as hashing key.
+ * The least 4 bits of the handle are used as key. However, during import all
+ * 128 bits of the handle are checked to prevent handle snooping.
+ */
+#define HANDLE_TO_KEY(sh) ((*(uint64_t *)sh) & KFD_IPC_HASH_TABLE_SIZE_MASK)
+
+int kfd_ipc_store_insert(struct dma_buf *dmabuf, struct kfd_ipc_obj **ipc_obj)
+{
+	struct kfd_ipc_obj *obj;
+
+	obj = kmalloc(sizeof(*obj), GFP_KERNEL);
+	if (!obj)
+		return -ENOMEM;
+
+	/* The initial ref belongs to the allocator process.
+	 * The IPC object store itself does not hold a ref since
+	 * there is no specific moment in time where that ref should
+	 * be dropped, except "when there are no more userspace processes
+	 * holding a ref to the object". Therefore the removal from IPC
+	 * storage happens at ipc_obj release time.
+	 */
+	kref_init(&obj->ref);
+	obj->dmabuf = dmabuf;
+	get_random_bytes(obj->share_handle, sizeof(obj->share_handle));
+
+	mutex_lock(&kfd_ipc_handles.lock);
+	hlist_add_head(&obj->node,
+		&kfd_ipc_handles.handles[HANDLE_TO_KEY(obj->share_handle)]);
+	mutex_unlock(&kfd_ipc_handles.lock);
+
+	if (ipc_obj)
+		*ipc_obj = obj;
+
+	return 0;
+}
+
+static void ipc_obj_release(struct kref *r)
+{
+	struct kfd_ipc_obj *obj;
+
+	obj = container_of(r, struct kfd_ipc_obj, ref);
+
+	mutex_lock(&kfd_ipc_handles.lock);
+	hash_del(&obj->node);
+	mutex_unlock(&kfd_ipc_handles.lock);
+
+	dma_buf_put(obj->dmabuf);
+	kfree(obj);
+}
+
+static struct kfd_ipc_obj *ipc_obj_get(struct kfd_ipc_obj *obj)
+{
+	if (kref_get_unless_zero(&obj->ref))
+		return obj;
+	return NULL;
+}
+
+void kfd_ipc_obj_put(struct kfd_ipc_obj **obj)
+{
+	if (*obj) {
+		kref_put(&(*obj)->ref, ipc_obj_release);
+		*obj = NULL;
+	}
+}
+
+int kfd_ipc_init(void)
+{
+	mutex_init(&kfd_ipc_handles.lock);
+	hash_init(kfd_ipc_handles.handles);
+	return 0;
+}
+
+static int kfd_import_dmabuf_create_kfd_bo(struct kfd_dev *dev,
+					   struct kfd_process *p,
+					   struct dma_buf *dmabuf,
+					   struct kfd_ipc_obj *ipc_obj,
+					   uint64_t va_addr, uint64_t *handle,
+					   uint64_t *mmap_offset)
+{
+	int r;
+	void *mem;
+	uint64_t size;
+	int idr_handle;
+	struct kfd_process_device *pdd = NULL;
+
+	if (!handle)
+		return -EINVAL;
+
+	if (!dev)
+		return -EINVAL;
+
+	mutex_lock(&p->mutex);
+
+	pdd = kfd_bind_process_to_device(dev, p);
+	if (IS_ERR(pdd)) {
+		r = PTR_ERR(pdd);
+		goto err_unlock;
+	}
+
+	r = amdgpu_amdkfd_gpuvm_import_dmabuf(dev->kgd, dmabuf, ipc_obj,
+					va_addr, pdd->vm,
+					(struct kgd_mem **)&mem, &size,
+					mmap_offset);
+	if (r)
+		goto err_unlock;
+
+	idr_handle = kfd_process_device_create_obj_handle(pdd, mem);
+	if (idr_handle < 0) {
+		r = -EFAULT;
+		goto err_free;
+	}
+
+	mutex_unlock(&p->mutex);
+
+	*handle = MAKE_HANDLE(dev->id, idr_handle);
+
+	return 0;
+
+err_free:
+	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, (struct kgd_mem *)mem,
+					       NULL);
+err_unlock:
+	mutex_unlock(&p->mutex);
+	return r;
+}
+
+int kfd_ipc_import_dmabuf(struct kfd_dev *dev, struct kfd_process *p,
+			  int dmabuf_fd, uint64_t va_addr,
+			  uint64_t *handle, uint64_t *mmap_offset)
+{
+	int r;
+	struct dma_buf *dmabuf = dma_buf_get(dmabuf_fd);
+
+	if (!dmabuf)
+		return -EINVAL;
+
+	r = kfd_import_dmabuf_create_kfd_bo(dev, p, dmabuf, NULL,
+					    va_addr, handle, mmap_offset);
+	dma_buf_put(dmabuf);
+	return r;
+}
+
+int kfd_ipc_import_handle(struct kfd_process *p,
+			  uint32_t *share_handle, uint64_t va_addr,
+			  uint64_t *handle, uint32_t *gpu_id,
+			  uint64_t *mmap_offset)
+{
+	struct kfd_dev *dev;
+	int r;
+	struct kfd_ipc_obj *entry, *found = NULL;
+
+	mutex_lock(&kfd_ipc_handles.lock);
+	/* Convert the user provided handle to hash key and search only in that
+	 * bucket
+	 */
+	hlist_for_each_entry(entry,
+		&kfd_ipc_handles.handles[HANDLE_TO_KEY(share_handle)], node) {
+		if (!memcmp(entry->share_handle, share_handle,
+			    sizeof(entry->share_handle))) {
+			found = ipc_obj_get(entry);
+			break;
+		}
+	}
+	mutex_unlock(&kfd_ipc_handles.lock);
+
+	if (!found)
+		return -EINVAL;
+
+	pr_debug("Found ipc_dma_buf: %p\n", found->dmabuf);
+
+	dev = kfd_device_by_id(found->gpu_id);
+	if (!dev)
+		return -ENODEV;
+
+	r = kfd_import_dmabuf_create_kfd_bo(dev, p, found->dmabuf, found,
+					    va_addr, handle, mmap_offset);
+	if (r)
+		goto error_unref;
+
+	*gpu_id = found->gpu_id;
+
+	return r;
+
+error_unref:
+	kfd_ipc_obj_put(&found);
+	return r;
+}
+
+int kfd_ipc_export_as_handle(struct kfd_dev *dev, struct kfd_process *p,
+			     uint64_t handle, uint32_t *ipc_handle)
+{
+	struct kfd_process_device *pdd = NULL;
+	struct kfd_ipc_obj *ipc_obj;
+	struct kgd_mem *mem;
+	int r;
+
+	if (!dev || !ipc_handle)
+		return -EINVAL;
+
+	mutex_lock(&p->mutex);
+	pdd = kfd_bind_process_to_device(dev, p);
+	if (IS_ERR(pdd)) {
+		mutex_unlock(&p->mutex);
+		pr_err("Failed to get pdd\n");
+		return PTR_ERR(pdd);
+	}
+
+	mem = kfd_process_device_translate_handle(pdd, GET_IDR_HANDLE(handle));
+	mutex_unlock(&p->mutex);
+
+	if (!mem) {
+		pr_err("Failed to get bo");
+		return -EINVAL;
+	}
+
+	r = amdgpu_amdkfd_gpuvm_export_ipc_obj(dev->kgd, pdd->vm, mem,
+					       &ipc_obj);
+	if (r)
+		return r;
+
+	ipc_obj->gpu_id = dev->id;
+	memcpy(ipc_handle, ipc_obj->share_handle,
+	       sizeof(ipc_obj->share_handle));
+
+	return r;
+}
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_ipc.h b/drivers/gpu/drm/amd/amdkfd/kfd_ipc.h
new file mode 100644
index 000000000000..9450a667918e
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_ipc.h
@@ -0,0 +1,55 @@
+/*
+ * Copyright 2014 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+#ifndef KFD_IPC_H_
+#define KFD_IPC_H_
+
+#include <linux/types.h>
+#include <linux/dma-buf.h>
+
+/* avoid including kfd_priv.h */
+struct kfd_dev;
+struct kfd_process;
+
+struct kfd_ipc_obj {
+	struct hlist_node node;
+	struct kref ref;
+	struct dma_buf *dmabuf;
+	uint32_t share_handle[4];
+	uint32_t gpu_id;
+};
+
+int kfd_ipc_import_handle(struct kfd_process *p,
+			  uint32_t *share_handle, uint64_t va_addr,
+			  uint64_t *handle, uint32_t *gpu_id,
+			  uint64_t *mmap_offset);
+int kfd_ipc_import_dmabuf(struct kfd_dev *kfd, struct kfd_process *p,
+			  int dmabuf_fd, uint64_t va_addr,
+			  uint64_t *handle, uint64_t *mmap_offset);
+int kfd_ipc_export_as_handle(struct kfd_dev *dev, struct kfd_process *p,
+			     uint64_t handle, uint32_t *ipc_handle);
+
+int kfd_ipc_store_insert(struct dma_buf *dmabuf, struct kfd_ipc_obj **ipc_obj);
+void kfd_ipc_obj_put(struct kfd_ipc_obj **obj);
+
+#endif /* KFD_IPC_H_ */
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_module.c b/drivers/gpu/drm/amd/amdkfd/kfd_module.c
index f4b7f7e6c40e..0946d5692692 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_module.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_module.c
@@ -52,6 +52,10 @@ static int kfd_init(void)
 	if (err < 0)
 		goto err_topology;
 
+	err = kfd_ipc_init();
+	if (err < 0)
+		goto err_ipc;
+
 	err = kfd_process_create_wq();
 	if (err < 0)
 		goto err_create_wq;
@@ -66,6 +70,7 @@ static int kfd_init(void)
 	return 0;
 
 err_create_wq:
+err_ipc:
 	kfd_topology_shutdown();
 err_topology:
 	kfd_chardev_exit();
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 51ba2020732e..1588b2b45a32 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -321,6 +321,8 @@ struct kfd_dev {
 	spinlock_t smi_lock;
 };
 
+struct kfd_ipc_obj;
+
 enum kfd_mempool {
 	KFD_MEMPOOL_SYSTEM_CACHEABLE = 1,
 	KFD_MEMPOOL_SYSTEM_WRITECOMBINE = 2,
@@ -1087,6 +1089,9 @@ int dbgdev_wave_reset_wavefronts(struct kfd_dev *dev, struct kfd_process *p);
 
 bool kfd_is_locked(void);
 
+/* IPC Support */
+int kfd_ipc_init(void);
+
 /* Compute profile */
 void kfd_inc_compute_active(struct kfd_dev *dev);
 void kfd_dec_compute_active(struct kfd_dev *dev);
diff --git a/include/uapi/linux/kfd_ioctl.h b/include/uapi/linux/kfd_ioctl.h
index f738c3b53f4e..90ff334e2b5b 100644
--- a/include/uapi/linux/kfd_ioctl.h
+++ b/include/uapi/linux/kfd_ioctl.h
@@ -29,9 +29,10 @@
 /*
  * - 1.1 - initial version
  * - 1.3 - Add SMI events support
+ * - 1.4 - Add IPC export/import
  */
 #define KFD_IOCTL_MAJOR_VERSION 1
-#define KFD_IOCTL_MINOR_VERSION 3
+#define KFD_IOCTL_MINOR_VERSION 4
 
 struct kfd_ioctl_get_version_args {
 	__u32 major_version;	/* from KFD */
@@ -464,6 +465,57 @@ enum kfd_mmio_remap {
 	KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL = 4,
 };
 
+/* Export IPC handle
+ *
+ * @handle[in]:         buffer handle of the BO to export
+ * @gpu_id[in]:         GPU ID where @handle was allocated
+ * @share_handle[out]:  share handle that can be used with
+ *                      @kfd_ioctl_ipc_import_handle_args
+ *
+ * @share_handle is a 128 bit random number generated with
+ * @get_random_bytes. This number should be very hard to guess.
+ * Knowledge of the @share_handle implies authorization to access
+ * the shared memory. User mode should treat it like a secret key.
+ * It can be used to import this BO in a different process context
+ * for IPC buffer sharing. The handle will be valid as long as the
+ * underlying BO exists. If the same BO is exported multiple times,
+ * the same handle will be returned.
+ *
+ * Return 0 on success, negative errno on errors.
+ */
+struct kfd_ioctl_ipc_export_handle_args {
+	__u64 handle;		/* to KFD */
+	__u32 share_handle[4];	/* from KFD */
+	__u32 gpu_id;		/* to KFD */
+	__u32 pad;
+};
+
+/* Import IPC handle
+ *
+ * @share_handle[in]:  share handle from @kfd_ioctl_ipc_export_handle_args
+ * @va_addr[in]:       virtual address at which to import the BO
+ * @handle[out]:       buffer handle of the imported BO
+ * @gpu_id[out]:       device in which the shared BO was created
+ * @mmap_offset[out]:  mmap offset for CPU-mapping the BO
+ *
+ * @handle represents a new reference to the shared BO. This reference
+ * must be released with kfd_ioctl_free_memory_of_gpu_args.
+ *
+ * The BO can be mapped for GPU access with @kfd_ioctl_map_memory_to_gpu_args.
+ *
+ * It can be mapped for CPU access using the @mmap_offset.
+ *
+ * Return 0 on success, negative errno on errors.
+ */
+struct kfd_ioctl_ipc_import_handle_args {
+	__u64 handle;		/* from KFD */
+	__u64 va_addr;		/* to KFD */
+	__u64 mmap_offset;	/* from KFD */
+	__u32 share_handle[4];	/* to KFD */
+	__u32 gpu_id;		/* from KFD */
+	__u32 pad;
+};
+
 #define AMDKFD_IOCTL_BASE 'K'
 #define AMDKFD_IO(nr)			_IO(AMDKFD_IOCTL_BASE, nr)
 #define AMDKFD_IOR(nr, type)		_IOR(AMDKFD_IOCTL_BASE, nr, type)
@@ -564,7 +616,13 @@ enum kfd_mmio_remap {
 #define AMDKFD_IOC_SMI_EVENTS			\
 		AMDKFD_IOWR(0x1F, struct kfd_ioctl_smi_events_args)
 
+#define AMDKFD_IOC_IPC_IMPORT_HANDLE                                    \
+		AMDKFD_IOWR(0x20, struct kfd_ioctl_ipc_import_handle_args)
+
+#define AMDKFD_IOC_IPC_EXPORT_HANDLE		\
+		AMDKFD_IOWR(0x21, struct kfd_ioctl_ipc_export_handle_args)
+
 #define AMDKFD_COMMAND_START		0x01
-#define AMDKFD_COMMAND_END		0x20
+#define AMDKFD_COMMAND_END		0x22
 
 #endif
-- 
2.27.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] drm/amdkfd: Add IPC API

[Dave Airlie]

On Tue, 14 Jul 2020 at 13:14, Felix Kuehling <Felix.Kuehling@amd.com> wrote:
>
> This allows exporting and importing buffers. The API generates handles
> that can be used with the HIP IPC API, i.e. big numbers rather than
> file descriptors.

First up why? I get the how.

> + * @share_handle is a 128 bit random number generated with
> + * @get_random_bytes. This number should be very hard to guess.
> + * Knowledge of the @share_handle implies authorization to access
> + * the shared memory. User mode should treat it like a secret key.
> + * It can be used to import this BO in a different process context
> + * for IPC buffer sharing. The handle will be valid as long as the
> + * underlying BO exists. If the same BO is exported multiple times,

Do we have any examples of any APIs in the kernel that operate like
this? That don't at least layer some sort of file permissions  and
access control on top?

The reason fd's are good is that combined with unix sockets, you can't
sniff it, you can't ptrace a process and find it, you can't write it
out in a coredump and have someone access it later.

To me this isn't secure design, it's obscure design, now I get to find
you've likely shipped this in "downstream" ROCm already, and have
customers running it.

Maybe someone who knows security can ack merging this sort of uAPI
design, I'm not confident in what it's doing is in any ways a good
idea. I might have to ask some people to take a closer look.

Dave.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] drm/amdkfd: Add IPC API

[Felix Kuehling]

Am 2020-07-13 um 11:28 p.m. schrieb Dave Airlie:
> On Tue, 14 Jul 2020 at 13:14, Felix Kuehling <Felix.Kuehling@amd.com> wrote:
>> This allows exporting and importing buffers. The API generates handles
>> that can be used with the HIP IPC API, i.e. big numbers rather than
>> file descriptors.
> First up why? I get the how.

The "why" is compatibility with HIP code ported from CUDA. The
equivalent CUDA IPC API works with handles that can be communicated
through e.g. a pipe or shared memory. You can't do that with file
descriptors.

https://docs.nvidia.com/cuda/cuda-runtime-api/group__CUDART__DEVICE.html#group__CUDART__DEVICE_1g8a37f7dfafaca652391d0758b3667539

https://docs.nvidia.com/cuda/cuda-runtime-api/group__CUDART__DEVICE.html#group__CUDART__DEVICE_1g01050a29fefde385b1042081ada4cde9


>
>> + * @share_handle is a 128 bit random number generated with
>> + * @get_random_bytes. This number should be very hard to guess.
>> + * Knowledge of the @share_handle implies authorization to access
>> + * the shared memory. User mode should treat it like a secret key.
>> + * It can be used to import this BO in a different process context
>> + * for IPC buffer sharing. The handle will be valid as long as the
>> + * underlying BO exists. If the same BO is exported multiple times,
> Do we have any examples of any APIs in the kernel that operate like
> this? That don't at least layer some sort of file permissions  and
> access control on top?

SystemV shared memory APIs (shmget, shmat) work similarly. There are
some permissions that can be specified by the exporter in shmget.
However, the handles are just numbers and much easier to guess (they are
32-bit integers). The most restrictive permissions would allow only the
exporting UID to attach to the shared memory segment.

I think DRM flink works similarly as well, with a global name IDR used
for looking up GEM objects using global object names.


>
> The reason fd's are good is that combined with unix sockets, you can't
> sniff it, you can't ptrace a process and find it, you can't write it
> out in a coredump and have someone access it later.

Arguably ptrace and core dumps give you access to all the memory
contents already. So you don't need the shared memory handle to access
memory in that case.


>
> To me this isn't secure design, it's obscure design, now I get to find
> you've likely shipped this in "downstream" ROCm already, and have
> customers running it.
>
> Maybe someone who knows security can ack merging this sort of uAPI
> design, I'm not confident in what it's doing is in any ways a good
> idea. I might have to ask some people to take a closer look.

Please do. We have tried to make this API as secure as possible within
the constraints of the user mode API we needed to implement.

Thanks,
  Felix


>
> Dave.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] drm/amdkfd: Add IPC API

[Dave Airlie]

On Tue, 14 Jul 2020 at 14:09, Felix Kuehling <felix.kuehling@amd.com> wrote:
>
> Am 2020-07-13 um 11:28 p.m. schrieb Dave Airlie:
> > On Tue, 14 Jul 2020 at 13:14, Felix Kuehling <Felix.Kuehling@amd.com> wrote:
> >> This allows exporting and importing buffers. The API generates handles
> >> that can be used with the HIP IPC API, i.e. big numbers rather than
> >> file descriptors.
> > First up why? I get the how.
>
> The "why" is compatibility with HIP code ported from CUDA. The
> equivalent CUDA IPC API works with handles that can be communicated
> through e.g. a pipe or shared memory. You can't do that with file
> descriptors.

Okay that sort of useful information should definitely be in the patch
description.

>
> https://docs.nvidia.com/cuda/cuda-runtime-api/group__CUDART__DEVICE.html#group__CUDART__DEVICE_1g8a37f7dfafaca652391d0758b3667539
>
> https://docs.nvidia.com/cuda/cuda-runtime-api/group__CUDART__DEVICE.html#group__CUDART__DEVICE_1g01050a29fefde385b1042081ada4cde9
>
> >
> >> + * @share_handle is a 128 bit random number generated with
> >> + * @get_random_bytes. This number should be very hard to guess.
> >> + * Knowledge of the @share_handle implies authorization to access
> >> + * the shared memory. User mode should treat it like a secret key.
> >> + * It can be used to import this BO in a different process context
> >> + * for IPC buffer sharing. The handle will be valid as long as the
> >> + * underlying BO exists. If the same BO is exported multiple times,
> > Do we have any examples of any APIs in the kernel that operate like
> > this? That don't at least layer some sort of file permissions  and
> > access control on top?
>
> SystemV shared memory APIs (shmget, shmat) work similarly. There are
> some permissions that can be specified by the exporter in shmget.
> However, the handles are just numbers and much easier to guess (they are
> 32-bit integers). The most restrictive permissions would allow only the
> exporting UID to attach to the shared memory segment.
>
> I think DRM flink works similarly as well, with a global name IDR used
> for looking up GEM objects using global object names.

flink is why I asked, because flink was a mistake and not one I'd care
to make again.
shm is horrible also, but at least has some permissions on what users
can attack it.

> > The reason fd's are good is that combined with unix sockets, you can't
> > sniff it, you can't ptrace a process and find it, you can't write it
> > out in a coredump and have someone access it later.
>
> Arguably ptrace and core dumps give you access to all the memory
> contents already. So you don't need the shared memory handle to access
> memory in that case.

core dumps might not dump this memory though, but yeah ptrace would
likely already mean you have access.

> > Maybe someone who knows security can ack merging this sort of uAPI
> > design, I'm not confident in what it's doing is in any ways a good
> > idea. I might have to ask some people to take a closer look.
>
> Please do. We have tried to make this API as secure as possible within
> the constraints of the user mode API we needed to implement.

I'll see if I hear back, but also if danvet has any input like I
suppose it's UUID based buffer access, so maybe 128-bit is enough and
you have enough entropy not to create anything insanely predictable.

Dave.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] drm/amdkfd: Add IPC API

[Daniel Vetter]

On Tue, Jul 14, 2020 at 02:26:36PM +1000, Dave Airlie wrote:
> On Tue, 14 Jul 2020 at 14:09, Felix Kuehling <felix.kuehling@amd.com> wrote:
> >
> > Am 2020-07-13 um 11:28 p.m. schrieb Dave Airlie:
> > > On Tue, 14 Jul 2020 at 13:14, Felix Kuehling <Felix.Kuehling@amd.com> wrote:
> > >> This allows exporting and importing buffers. The API generates handles
> > >> that can be used with the HIP IPC API, i.e. big numbers rather than
> > >> file descriptors.
> > > First up why? I get the how.
> >
> > The "why" is compatibility with HIP code ported from CUDA. The
> > equivalent CUDA IPC API works with handles that can be communicated
> > through e.g. a pipe or shared memory. You can't do that with file
> > descriptors.
> 
> Okay that sort of useful information should definitely be in the patch
> description.
> 
> >
> > https://docs.nvidia.com/cuda/cuda-runtime-api/group__CUDART__DEVICE.html#group__CUDART__DEVICE_1g8a37f7dfafaca652391d0758b3667539
> >
> > https://docs.nvidia.com/cuda/cuda-runtime-api/group__CUDART__DEVICE.html#group__CUDART__DEVICE_1g01050a29fefde385b1042081ada4cde9
> >
> > >
> > >> + * @share_handle is a 128 bit random number generated with
> > >> + * @get_random_bytes. This number should be very hard to guess.
> > >> + * Knowledge of the @share_handle implies authorization to access
> > >> + * the shared memory. User mode should treat it like a secret key.
> > >> + * It can be used to import this BO in a different process context
> > >> + * for IPC buffer sharing. The handle will be valid as long as the
> > >> + * underlying BO exists. If the same BO is exported multiple times,
> > > Do we have any examples of any APIs in the kernel that operate like
> > > this? That don't at least layer some sort of file permissions  and
> > > access control on top?
> >
> > SystemV shared memory APIs (shmget, shmat) work similarly. There are
> > some permissions that can be specified by the exporter in shmget.
> > However, the handles are just numbers and much easier to guess (they are
> > 32-bit integers). The most restrictive permissions would allow only the
> > exporting UID to attach to the shared memory segment.
> >
> > I think DRM flink works similarly as well, with a global name IDR used
> > for looking up GEM objects using global object names.
> 
> flink is why I asked, because flink was a mistake and not one I'd care
> to make again.
> shm is horrible also, but at least has some permissions on what users
> can attack it.

Yeah this smells way too much like flink. I had the same reaction, and
kinda sad that we have to do this because nvidia defines how this works
with 0 input from anyone else. Oh well, the world sucks.

> > > The reason fd's are good is that combined with unix sockets, you can't
> > > sniff it, you can't ptrace a process and find it, you can't write it
> > > out in a coredump and have someone access it later.
> >
> > Arguably ptrace and core dumps give you access to all the memory
> > contents already. So you don't need the shared memory handle to access
> > memory in that case.
> 
> core dumps might not dump this memory though, but yeah ptrace would
> likely already mean you have access.
> 
> > > Maybe someone who knows security can ack merging this sort of uAPI
> > > design, I'm not confident in what it's doing is in any ways a good
> > > idea. I might have to ask some people to take a closer look.
> >
> > Please do. We have tried to make this API as secure as possible within
> > the constraints of the user mode API we needed to implement.
> 
> I'll see if I hear back, but also if danvet has any input like I
> suppose it's UUID based buffer access, so maybe 128-bit is enough and
> you have enough entropy not to create anything insanely predictable.

So one idea that crossed my mind is if we don't want to do this as a
generic dma-buf handle converter.

Something like /dev/dri/cuda_is_nasty (maybe slightly nicer name) which
provides a generic dma-buf <-> cuda uuid converter. With separate access
restrictions, so admins can decide whether they want to allow this
silliness, or not. Anyone else who wants to reimplement cuda will need
this too, so that's another reason for splitting this out.

Wrt security: I think assuming that there's none and the lookup has a
side-channel you can use to efficiently scan the entire range is probably
the safe approach here. This is way out of my league, but I think people
who know how to do this won't have a much harder time scanning this than
the flink space.

Also, if we have one common uuid->dma-buf converter, we might actually
have a chance to proof the "it's not secure" assumption wrong. Also, we
might be able to tie this into cgroups or namespaces or similar that way.

Just some thoughts to give my initial "eek, why this" reaction a bit more
substance :-) No idea whether this would work or make more sense.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] drm/amdkfd: Add IPC API

[Christian König]

Am 14.07.20 um 10:58 schrieb Daniel Vetter:
> On Tue, Jul 14, 2020 at 02:26:36PM +1000, Dave Airlie wrote:
>> On Tue, 14 Jul 2020 at 14:09, Felix Kuehling <felix.kuehling@amd.com> wrote:
>>> Am 2020-07-13 um 11:28 p.m. schrieb Dave Airlie:
>>>> On Tue, 14 Jul 2020 at 13:14, Felix Kuehling <Felix.Kuehling@amd.com> wrote:
>>>>> This allows exporting and importing buffers. The API generates handles
>>>>> that can be used with the HIP IPC API, i.e. big numbers rather than
>>>>> file descriptors.
>>>> First up why? I get the how.
>>> The "why" is compatibility with HIP code ported from CUDA. The
>>> equivalent CUDA IPC API works with handles that can be communicated
>>> through e.g. a pipe or shared memory. You can't do that with file
>>> descriptors.
>> Okay that sort of useful information should definitely be in the patch
>> description.
>>
>>> https://docs.nvidia.com/cuda/cuda-runtime-api/group__CUDART__DEVICE.html#group__CUDART__DEVICE_1g8a37f7dfafaca652391d0758b3667539
>>>
>>> https://docs.nvidia.com/cuda/cuda-runtime-api/group__CUDART__DEVICE.html#group__CUDART__DEVICE_1g01050a29fefde385b1042081ada4cde9
>>>
>>>>> + * @share_handle is a 128 bit random number generated with
>>>>> + * @get_random_bytes. This number should be very hard to guess.
>>>>> + * Knowledge of the @share_handle implies authorization to access
>>>>> + * the shared memory. User mode should treat it like a secret key.
>>>>> + * It can be used to import this BO in a different process context
>>>>> + * for IPC buffer sharing. The handle will be valid as long as the
>>>>> + * underlying BO exists. If the same BO is exported multiple times,
>>>> Do we have any examples of any APIs in the kernel that operate like
>>>> this? That don't at least layer some sort of file permissions  and
>>>> access control on top?
>>> SystemV shared memory APIs (shmget, shmat) work similarly. There are
>>> some permissions that can be specified by the exporter in shmget.
>>> However, the handles are just numbers and much easier to guess (they are
>>> 32-bit integers). The most restrictive permissions would allow only the
>>> exporting UID to attach to the shared memory segment.
>>>
>>> I think DRM flink works similarly as well, with a global name IDR used
>>> for looking up GEM objects using global object names.
>> flink is why I asked, because flink was a mistake and not one I'd care
>> to make again.
>> shm is horrible also, but at least has some permissions on what users
>> can attack it.
> Yeah this smells way too much like flink. I had the same reaction, and
> kinda sad that we have to do this because nvidia defines how this works
> with 0 input from anyone else. Oh well, the world sucks.
>
>>>> The reason fd's are good is that combined with unix sockets, you can't
>>>> sniff it, you can't ptrace a process and find it, you can't write it
>>>> out in a coredump and have someone access it later.
>>> Arguably ptrace and core dumps give you access to all the memory
>>> contents already. So you don't need the shared memory handle to access
>>> memory in that case.
>> core dumps might not dump this memory though, but yeah ptrace would
>> likely already mean you have access.
>>
>>>> Maybe someone who knows security can ack merging this sort of uAPI
>>>> design, I'm not confident in what it's doing is in any ways a good
>>>> idea. I might have to ask some people to take a closer look.
>>> Please do. We have tried to make this API as secure as possible within
>>> the constraints of the user mode API we needed to implement.
>> I'll see if I hear back, but also if danvet has any input like I
>> suppose it's UUID based buffer access, so maybe 128-bit is enough and
>> you have enough entropy not to create anything insanely predictable.
> So one idea that crossed my mind is if we don't want to do this as a
> generic dma-buf handle converter.
>
> Something like /dev/dri/cuda_is_nasty (maybe slightly nicer name) which
> provides a generic dma-buf <-> cuda uuid converter. With separate access
> restrictions, so admins can decide whether they want to allow this
> silliness, or not. Anyone else who wants to reimplement cuda will need
> this too, so that's another reason for splitting this out.
>
> Wrt security: I think assuming that there's none and the lookup has a
> side-channel you can use to efficiently scan the entire range is probably
> the safe approach here. This is way out of my league, but I think people
> who know how to do this won't have a much harder time scanning this than
> the flink space.
>
> Also, if we have one common uuid->dma-buf converter, we might actually
> have a chance to proof the "it's not secure" assumption wrong. Also, we
> might be able to tie this into cgroups or namespaces or similar that way.
>
> Just some thoughts to give my initial "eek, why this" reaction a bit more
> substance :-) No idea whether this would work or make more sense.

Yeah, my initial reaction was the same. On the pro side is that we use 
more than the 32bits flink did as identifier.


What we could maybe do to improve this is to link DMA-buf file 
descriptors into the file system from userspace. And then we could just 
do something like:

open("/tmp/dma-buf-0x0123-4567-89AB-CDEF-0123-4567-89AB-CDEF", "rw");

But to be honest I don't really know the fs code that well enough to 
judge if this is possible or not.


Or we let DMA-bufs appear under some directory of /sys by their name so 
that applications can open and use them.

Regards,
Christian.

>
> Cheers, Daniel

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] drm/amdkfd: Add IPC API

[Daniel Vetter]

On Tue, Jul 14, 2020 at 11:28:12AM +0200, Christian König wrote:
> Am 14.07.20 um 10:58 schrieb Daniel Vetter:
> > On Tue, Jul 14, 2020 at 02:26:36PM +1000, Dave Airlie wrote:
> > > On Tue, 14 Jul 2020 at 14:09, Felix Kuehling <felix.kuehling@amd.com> wrote:
> > > > Am 2020-07-13 um 11:28 p.m. schrieb Dave Airlie:
> > > > > On Tue, 14 Jul 2020 at 13:14, Felix Kuehling <Felix.Kuehling@amd.com> wrote:
> > > > > > This allows exporting and importing buffers. The API generates handles
> > > > > > that can be used with the HIP IPC API, i.e. big numbers rather than
> > > > > > file descriptors.
> > > > > First up why? I get the how.
> > > > The "why" is compatibility with HIP code ported from CUDA. The
> > > > equivalent CUDA IPC API works with handles that can be communicated
> > > > through e.g. a pipe or shared memory. You can't do that with file
> > > > descriptors.
> > > Okay that sort of useful information should definitely be in the patch
> > > description.
> > > 
> > > > https://docs.nvidia.com/cuda/cuda-runtime-api/group__CUDART__DEVICE.html#group__CUDART__DEVICE_1g8a37f7dfafaca652391d0758b3667539
> > > > 
> > > > https://docs.nvidia.com/cuda/cuda-runtime-api/group__CUDART__DEVICE.html#group__CUDART__DEVICE_1g01050a29fefde385b1042081ada4cde9
> > > > 
> > > > > > + * @share_handle is a 128 bit random number generated with
> > > > > > + * @get_random_bytes. This number should be very hard to guess.
> > > > > > + * Knowledge of the @share_handle implies authorization to access
> > > > > > + * the shared memory. User mode should treat it like a secret key.
> > > > > > + * It can be used to import this BO in a different process context
> > > > > > + * for IPC buffer sharing. The handle will be valid as long as the
> > > > > > + * underlying BO exists. If the same BO is exported multiple times,
> > > > > Do we have any examples of any APIs in the kernel that operate like
> > > > > this? That don't at least layer some sort of file permissions  and
> > > > > access control on top?
> > > > SystemV shared memory APIs (shmget, shmat) work similarly. There are
> > > > some permissions that can be specified by the exporter in shmget.
> > > > However, the handles are just numbers and much easier to guess (they are
> > > > 32-bit integers). The most restrictive permissions would allow only the
> > > > exporting UID to attach to the shared memory segment.
> > > > 
> > > > I think DRM flink works similarly as well, with a global name IDR used
> > > > for looking up GEM objects using global object names.
> > > flink is why I asked, because flink was a mistake and not one I'd care
> > > to make again.
> > > shm is horrible also, but at least has some permissions on what users
> > > can attack it.
> > Yeah this smells way too much like flink. I had the same reaction, and
> > kinda sad that we have to do this because nvidia defines how this works
> > with 0 input from anyone else. Oh well, the world sucks.
> > 
> > > > > The reason fd's are good is that combined with unix sockets, you can't
> > > > > sniff it, you can't ptrace a process and find it, you can't write it
> > > > > out in a coredump and have someone access it later.
> > > > Arguably ptrace and core dumps give you access to all the memory
> > > > contents already. So you don't need the shared memory handle to access
> > > > memory in that case.
> > > core dumps might not dump this memory though, but yeah ptrace would
> > > likely already mean you have access.
> > > 
> > > > > Maybe someone who knows security can ack merging this sort of uAPI
> > > > > design, I'm not confident in what it's doing is in any ways a good
> > > > > idea. I might have to ask some people to take a closer look.
> > > > Please do. We have tried to make this API as secure as possible within
> > > > the constraints of the user mode API we needed to implement.
> > > I'll see if I hear back, but also if danvet has any input like I
> > > suppose it's UUID based buffer access, so maybe 128-bit is enough and
> > > you have enough entropy not to create anything insanely predictable.
> > So one idea that crossed my mind is if we don't want to do this as a
> > generic dma-buf handle converter.
> > 
> > Something like /dev/dri/cuda_is_nasty (maybe slightly nicer name) which
> > provides a generic dma-buf <-> cuda uuid converter. With separate access
> > restrictions, so admins can decide whether they want to allow this
> > silliness, or not. Anyone else who wants to reimplement cuda will need
> > this too, so that's another reason for splitting this out.
> > 
> > Wrt security: I think assuming that there's none and the lookup has a
> > side-channel you can use to efficiently scan the entire range is probably
> > the safe approach here. This is way out of my league, but I think people
> > who know how to do this won't have a much harder time scanning this than
> > the flink space.
> > 
> > Also, if we have one common uuid->dma-buf converter, we might actually
> > have a chance to proof the "it's not secure" assumption wrong. Also, we
> > might be able to tie this into cgroups or namespaces or similar that way.
> > 
> > Just some thoughts to give my initial "eek, why this" reaction a bit more
> > substance :-) No idea whether this would work or make more sense.
> 
> Yeah, my initial reaction was the same. On the pro side is that we use more
> than the 32bits flink did as identifier.

flink started at 0, so in practice it was trivial to enumerate. Not even
randomized.

But the thing is if your uuid lookup isn't guaranteed to be const and
side-channel free, then you can use that to guess where ids are. Doesn't
need to be much until you can brute-force the remaining bits. Engineering
an implementation (not just the theory) that relies on the assumption that
full brute-force is the fastes option is very hard engineering. And I
think here in the gpu world we just don't have any of that expertise.

> What we could maybe do to improve this is to link DMA-buf file descriptors
> into the file system from userspace. And then we could just do something
> like:
> 
> open("/tmp/dma-buf-0x0123-4567-89AB-CDEF-0123-4567-89AB-CDEF", "rw");
> 
> But to be honest I don't really know the fs code that well enough to judge
> if this is possible or not.
> 
> 
> Or we let DMA-bufs appear under some directory of /sys by their name so that
> applications can open and use them.

Yeah dmabuffs might be another option, but not sure how that would work,
since we want a creat() that takes in a dma-buf fd and then freezes those
two together forever. Maybe something like devpts, but I think that's also
somewhat considered a design mistake (because namespace or something else,
I dunno). Since if we link all dma-buf by default into the fs, we again
have the flink "free for everyone" security issues.

Hm I guess one option could be such a dmabufs, but still with an explicit
export step. And the export amounts to a creat() in dmabufs, using the
uid/gid and entire security context of the process that has done the
dmabuf2uuid export.

That would also give us namespacing for free, using fs namespaces. All
we'd need is multiple instances of this dmabuffs. Plus I guess we'd need a
canonical mount point for this thing, and then a special ioctl on the root
node for creating a file from a dma-buf fd.

Feels mildly overengineered, but I think this would at least make the cuda
uuid stuff fit reasonably well into the overall linux architecture we
have. Only bikeshed left would be to figure out where to mount this fs.
Maybe /dev/dma-buf-uuids or something like that as the canonical thing.
/sys feels misplaced, and we alread have /dev/pts for similar stuff.

Wrt typing up an entire fs, I thought with all the libfs work that
shouldn't be too hard to pull off.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] drm/amdkfd: Add IPC API

[Christian König]

Am 14.07.20 um 11:53 schrieb Daniel Vetter:
> On Tue, Jul 14, 2020 at 11:28:12AM +0200, Christian König wrote:
>> Am 14.07.20 um 10:58 schrieb Daniel Vetter:
>>> On Tue, Jul 14, 2020 at 02:26:36PM +1000, Dave Airlie wrote:
>>>> On Tue, 14 Jul 2020 at 14:09, Felix Kuehling <felix.kuehling@amd.com> wrote:
>>>>> Am 2020-07-13 um 11:28 p.m. schrieb Dave Airlie:
>>>>>> On Tue, 14 Jul 2020 at 13:14, Felix Kuehling <Felix.Kuehling@amd.com> wrote:
>>>>>>> This allows exporting and importing buffers. The API generates handles
>>>>>>> that can be used with the HIP IPC API, i.e. big numbers rather than
>>>>>>> file descriptors.
>>>>>> First up why? I get the how.
>>>>> The "why" is compatibility with HIP code ported from CUDA. The
>>>>> equivalent CUDA IPC API works with handles that can be communicated
>>>>> through e.g. a pipe or shared memory. You can't do that with file
>>>>> descriptors.
>>>> Okay that sort of useful information should definitely be in the patch
>>>> description.
>>>>
>>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs.nvidia.com%2Fcuda%2Fcuda-runtime-api%2Fgroup__CUDART__DEVICE.html%23group__CUDART__DEVICE_1g8a37f7dfafaca652391d0758b3667539&amp;data=02%7C01%7Cchristian.koenig%40amd.com%7C2e0b8a6d2aac49e0f6c908d827dbcb46%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637303172250574336&amp;sdata=mDWVEDsP%2BKTvvhqYp%2BSstczPtEV9l7n%2B%2BuNj30de0sQ%3D&amp;reserved=0
>>>>>
>>>>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs.nvidia.com%2Fcuda%2Fcuda-runtime-api%2Fgroup__CUDART__DEVICE.html%23group__CUDART__DEVICE_1g01050a29fefde385b1042081ada4cde9&amp;data=02%7C01%7Cchristian.koenig%40amd.com%7C2e0b8a6d2aac49e0f6c908d827dbcb46%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637303172250574336&amp;sdata=yb80LA1csqkctuF1rAXBpYgJT%2BkS0Nmfilnd8%2BjQSW4%3D&amp;reserved=0
>>>>>
>>>>>>> + * @share_handle is a 128 bit random number generated with
>>>>>>> + * @get_random_bytes. This number should be very hard to guess.
>>>>>>> + * Knowledge of the @share_handle implies authorization to access
>>>>>>> + * the shared memory. User mode should treat it like a secret key.
>>>>>>> + * It can be used to import this BO in a different process context
>>>>>>> + * for IPC buffer sharing. The handle will be valid as long as the
>>>>>>> + * underlying BO exists. If the same BO is exported multiple times,
>>>>>> Do we have any examples of any APIs in the kernel that operate like
>>>>>> this? That don't at least layer some sort of file permissions  and
>>>>>> access control on top?
>>>>> SystemV shared memory APIs (shmget, shmat) work similarly. There are
>>>>> some permissions that can be specified by the exporter in shmget.
>>>>> However, the handles are just numbers and much easier to guess (they are
>>>>> 32-bit integers). The most restrictive permissions would allow only the
>>>>> exporting UID to attach to the shared memory segment.
>>>>>
>>>>> I think DRM flink works similarly as well, with a global name IDR used
>>>>> for looking up GEM objects using global object names.
>>>> flink is why I asked, because flink was a mistake and not one I'd care
>>>> to make again.
>>>> shm is horrible also, but at least has some permissions on what users
>>>> can attack it.
>>> Yeah this smells way too much like flink. I had the same reaction, and
>>> kinda sad that we have to do this because nvidia defines how this works
>>> with 0 input from anyone else. Oh well, the world sucks.
>>>
>>>>>> The reason fd's are good is that combined with unix sockets, you can't
>>>>>> sniff it, you can't ptrace a process and find it, you can't write it
>>>>>> out in a coredump and have someone access it later.
>>>>> Arguably ptrace and core dumps give you access to all the memory
>>>>> contents already. So you don't need the shared memory handle to access
>>>>> memory in that case.
>>>> core dumps might not dump this memory though, but yeah ptrace would
>>>> likely already mean you have access.
>>>>
>>>>>> Maybe someone who knows security can ack merging this sort of uAPI
>>>>>> design, I'm not confident in what it's doing is in any ways a good
>>>>>> idea. I might have to ask some people to take a closer look.
>>>>> Please do. We have tried to make this API as secure as possible within
>>>>> the constraints of the user mode API we needed to implement.
>>>> I'll see if I hear back, but also if danvet has any input like I
>>>> suppose it's UUID based buffer access, so maybe 128-bit is enough and
>>>> you have enough entropy not to create anything insanely predictable.
>>> So one idea that crossed my mind is if we don't want to do this as a
>>> generic dma-buf handle converter.
>>>
>>> Something like /dev/dri/cuda_is_nasty (maybe slightly nicer name) which
>>> provides a generic dma-buf <-> cuda uuid converter. With separate access
>>> restrictions, so admins can decide whether they want to allow this
>>> silliness, or not. Anyone else who wants to reimplement cuda will need
>>> this too, so that's another reason for splitting this out.
>>>
>>> Wrt security: I think assuming that there's none and the lookup has a
>>> side-channel you can use to efficiently scan the entire range is probably
>>> the safe approach here. This is way out of my league, but I think people
>>> who know how to do this won't have a much harder time scanning this than
>>> the flink space.
>>>
>>> Also, if we have one common uuid->dma-buf converter, we might actually
>>> have a chance to proof the "it's not secure" assumption wrong. Also, we
>>> might be able to tie this into cgroups or namespaces or similar that way.
>>>
>>> Just some thoughts to give my initial "eek, why this" reaction a bit more
>>> substance :-) No idea whether this would work or make more sense.
>> Yeah, my initial reaction was the same. On the pro side is that we use more
>> than the 32bits flink did as identifier.
> flink started at 0, so in practice it was trivial to enumerate. Not even
> randomized.
>
> But the thing is if your uuid lookup isn't guaranteed to be const and
> side-channel free, then you can use that to guess where ids are. Doesn't
> need to be much until you can brute-force the remaining bits. Engineering
> an implementation (not just the theory) that relies on the assumption that
> full brute-force is the fastes option is very hard engineering. And I
> think here in the gpu world we just don't have any of that expertise.

Well being able to look up up ids is not necessary a problem. See that 
root can see all buffers for debugging purposes is most likely not even 
a bad idea.

When we manage it as an fs we can just add/remove the executable bit 
from the directory to control enumeration.

And we can still have normal r/w permissions on the individual buffers 
as well.

>> What we could maybe do to improve this is to link DMA-buf file descriptors
>> into the file system from userspace. And then we could just do something
>> like:
>>
>> open("/tmp/dma-buf-0x0123-4567-89AB-CDEF-0123-4567-89AB-CDEF", "rw");
>>
>> But to be honest I don't really know the fs code that well enough to judge
>> if this is possible or not.
>>
>>
>> Or we let DMA-bufs appear under some directory of /sys by their name so that
>> applications can open and use them.
> Yeah dmabuffs might be another option, but not sure how that would work,
> since we want a creat() that takes in a dma-buf fd and then freezes those
> two together forever. Maybe something like devpts, but I think that's also
> somewhat considered a design mistake (because namespace or something else,
> I dunno). Since if we link all dma-buf by default into the fs, we again
> have the flink "free for everyone" security issues.
>
> Hm I guess one option could be such a dmabufs, but still with an explicit
> export step. And the export amounts to a creat() in dmabufs, using the
> uid/gid and entire security context of the process that has done the
> dmabuf2uuid export.
>
> That would also give us namespacing for free, using fs namespaces. All
> we'd need is multiple instances of this dmabuffs. Plus I guess we'd need a
> canonical mount point for this thing, and then a special ioctl on the root
> node for creating a file from a dma-buf fd.
>
> Feels mildly overengineered, but I think this would at least make the cuda
> uuid stuff fit reasonably well into the overall linux architecture we
> have. Only bikeshed left would be to figure out where to mount this fs.
> Maybe /dev/dma-buf-uuids or something like that as the canonical thing.
> /sys feels misplaced, and we alread have /dev/pts for similar stuff.

Yeah a dmabuffs sounds overengineered to me as well.

It's a pity that we can't just open anonymous inodes under 
/proc/$pid/fd/$fd.

This way we would just need to encode the pid and fd number in the id 
transmitted to the other process and could solve it this way.

That an application changes the permissions of its own file descriptors 
because it needs to implement some questionable API design is not a 
problem of the kernel.

Regards,
Christian.

>
> Wrt typing up an entire fs, I thought with all the libfs work that
> shouldn't be too hard to pull off.
> -Daniel

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] drm/amdkfd: Add IPC API

[Daniel Vetter]

On Tue, Jul 14, 2020 at 01:36:41PM +0200, Christian König wrote:
> Am 14.07.20 um 11:53 schrieb Daniel Vetter:
> > On Tue, Jul 14, 2020 at 11:28:12AM +0200, Christian König wrote:
> > > Am 14.07.20 um 10:58 schrieb Daniel Vetter:
> > > > On Tue, Jul 14, 2020 at 02:26:36PM +1000, Dave Airlie wrote:
> > > > > On Tue, 14 Jul 2020 at 14:09, Felix Kuehling <felix.kuehling@amd.com> wrote:
> > > > > > Am 2020-07-13 um 11:28 p.m. schrieb Dave Airlie:
> > > > > > > On Tue, 14 Jul 2020 at 13:14, Felix Kuehling <Felix.Kuehling@amd.com> wrote:
> > > > > > > > This allows exporting and importing buffers. The API generates handles
> > > > > > > > that can be used with the HIP IPC API, i.e. big numbers rather than
> > > > > > > > file descriptors.
> > > > > > > First up why? I get the how.
> > > > > > The "why" is compatibility with HIP code ported from CUDA. The
> > > > > > equivalent CUDA IPC API works with handles that can be communicated
> > > > > > through e.g. a pipe or shared memory. You can't do that with file
> > > > > > descriptors.
> > > > > Okay that sort of useful information should definitely be in the patch
> > > > > description.
> > > > > 
> > > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs.nvidia.com%2Fcuda%2Fcuda-runtime-api%2Fgroup__CUDART__DEVICE.html%23group__CUDART__DEVICE_1g8a37f7dfafaca652391d0758b3667539&amp;data=02%7C01%7Cchristian.koenig%40amd.com%7C2e0b8a6d2aac49e0f6c908d827dbcb46%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637303172250574336&amp;sdata=mDWVEDsP%2BKTvvhqYp%2BSstczPtEV9l7n%2B%2BuNj30de0sQ%3D&amp;reserved=0
> > > > > > 
> > > > > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs.nvidia.com%2Fcuda%2Fcuda-runtime-api%2Fgroup__CUDART__DEVICE.html%23group__CUDART__DEVICE_1g01050a29fefde385b1042081ada4cde9&amp;data=02%7C01%7Cchristian.koenig%40amd.com%7C2e0b8a6d2aac49e0f6c908d827dbcb46%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637303172250574336&amp;sdata=yb80LA1csqkctuF1rAXBpYgJT%2BkS0Nmfilnd8%2BjQSW4%3D&amp;reserved=0
> > > > > > 
> > > > > > > > + * @share_handle is a 128 bit random number generated with
> > > > > > > > + * @get_random_bytes. This number should be very hard to guess.
> > > > > > > > + * Knowledge of the @share_handle implies authorization to access
> > > > > > > > + * the shared memory. User mode should treat it like a secret key.
> > > > > > > > + * It can be used to import this BO in a different process context
> > > > > > > > + * for IPC buffer sharing. The handle will be valid as long as the
> > > > > > > > + * underlying BO exists. If the same BO is exported multiple times,
> > > > > > > Do we have any examples of any APIs in the kernel that operate like
> > > > > > > this? That don't at least layer some sort of file permissions  and
> > > > > > > access control on top?
> > > > > > SystemV shared memory APIs (shmget, shmat) work similarly. There are
> > > > > > some permissions that can be specified by the exporter in shmget.
> > > > > > However, the handles are just numbers and much easier to guess (they are
> > > > > > 32-bit integers). The most restrictive permissions would allow only the
> > > > > > exporting UID to attach to the shared memory segment.
> > > > > > 
> > > > > > I think DRM flink works similarly as well, with a global name IDR used
> > > > > > for looking up GEM objects using global object names.
> > > > > flink is why I asked, because flink was a mistake and not one I'd care
> > > > > to make again.
> > > > > shm is horrible also, but at least has some permissions on what users
> > > > > can attack it.
> > > > Yeah this smells way too much like flink. I had the same reaction, and
> > > > kinda sad that we have to do this because nvidia defines how this works
> > > > with 0 input from anyone else. Oh well, the world sucks.
> > > > 
> > > > > > > The reason fd's are good is that combined with unix sockets, you can't
> > > > > > > sniff it, you can't ptrace a process and find it, you can't write it
> > > > > > > out in a coredump and have someone access it later.
> > > > > > Arguably ptrace and core dumps give you access to all the memory
> > > > > > contents already. So you don't need the shared memory handle to access
> > > > > > memory in that case.
> > > > > core dumps might not dump this memory though, but yeah ptrace would
> > > > > likely already mean you have access.
> > > > > 
> > > > > > > Maybe someone who knows security can ack merging this sort of uAPI
> > > > > > > design, I'm not confident in what it's doing is in any ways a good
> > > > > > > idea. I might have to ask some people to take a closer look.
> > > > > > Please do. We have tried to make this API as secure as possible within
> > > > > > the constraints of the user mode API we needed to implement.
> > > > > I'll see if I hear back, but also if danvet has any input like I
> > > > > suppose it's UUID based buffer access, so maybe 128-bit is enough and
> > > > > you have enough entropy not to create anything insanely predictable.
> > > > So one idea that crossed my mind is if we don't want to do this as a
> > > > generic dma-buf handle converter.
> > > > 
> > > > Something like /dev/dri/cuda_is_nasty (maybe slightly nicer name) which
> > > > provides a generic dma-buf <-> cuda uuid converter. With separate access
> > > > restrictions, so admins can decide whether they want to allow this
> > > > silliness, or not. Anyone else who wants to reimplement cuda will need
> > > > this too, so that's another reason for splitting this out.
> > > > 
> > > > Wrt security: I think assuming that there's none and the lookup has a
> > > > side-channel you can use to efficiently scan the entire range is probably
> > > > the safe approach here. This is way out of my league, but I think people
> > > > who know how to do this won't have a much harder time scanning this than
> > > > the flink space.
> > > > 
> > > > Also, if we have one common uuid->dma-buf converter, we might actually
> > > > have a chance to proof the "it's not secure" assumption wrong. Also, we
> > > > might be able to tie this into cgroups or namespaces or similar that way.
> > > > 
> > > > Just some thoughts to give my initial "eek, why this" reaction a bit more
> > > > substance :-) No idea whether this would work or make more sense.
> > > Yeah, my initial reaction was the same. On the pro side is that we use more
> > > than the 32bits flink did as identifier.
> > flink started at 0, so in practice it was trivial to enumerate. Not even
> > randomized.
> > 
> > But the thing is if your uuid lookup isn't guaranteed to be const and
> > side-channel free, then you can use that to guess where ids are. Doesn't
> > need to be much until you can brute-force the remaining bits. Engineering
> > an implementation (not just the theory) that relies on the assumption that
> > full brute-force is the fastes option is very hard engineering. And I
> > think here in the gpu world we just don't have any of that expertise.
> 
> Well being able to look up up ids is not necessary a problem. See that root
> can see all buffers for debugging purposes is most likely not even a bad
> idea.
> 
> When we manage it as an fs we can just add/remove the executable bit from
> the directory to control enumeration.
> 
> And we can still have normal r/w permissions on the individual buffers as
> well.

Yeah id look-up isn't a problem, it's that the uuid comes with nothing
else like a fd or inode attached which would allows people to add some
security checks and namespacing to it. Which means that the entire
security boils down to "you can't guess the uuid", which means the
hashtable lookup must be sidechannel proof. Which we're not going to be
able to pull off I think :-)

> > > What we could maybe do to improve this is to link DMA-buf file descriptors
> > > into the file system from userspace. And then we could just do something
> > > like:
> > > 
> > > open("/tmp/dma-buf-0x0123-4567-89AB-CDEF-0123-4567-89AB-CDEF", "rw");
> > > 
> > > But to be honest I don't really know the fs code that well enough to judge
> > > if this is possible or not.
> > > 
> > > 
> > > Or we let DMA-bufs appear under some directory of /sys by their name so that
> > > applications can open and use them.
> > Yeah dmabuffs might be another option, but not sure how that would work,
> > since we want a creat() that takes in a dma-buf fd and then freezes those
> > two together forever. Maybe something like devpts, but I think that's also
> > somewhat considered a design mistake (because namespace or something else,
> > I dunno). Since if we link all dma-buf by default into the fs, we again
> > have the flink "free for everyone" security issues.
> > 
> > Hm I guess one option could be such a dmabufs, but still with an explicit
> > export step. And the export amounts to a creat() in dmabufs, using the
> > uid/gid and entire security context of the process that has done the
> > dmabuf2uuid export.
> > 
> > That would also give us namespacing for free, using fs namespaces. All
> > we'd need is multiple instances of this dmabuffs. Plus I guess we'd need a
> > canonical mount point for this thing, and then a special ioctl on the root
> > node for creating a file from a dma-buf fd.
> > 
> > Feels mildly overengineered, but I think this would at least make the cuda
> > uuid stuff fit reasonably well into the overall linux architecture we
> > have. Only bikeshed left would be to figure out where to mount this fs.
> > Maybe /dev/dma-buf-uuids or something like that as the canonical thing.
> > /sys feels misplaced, and we alread have /dev/pts for similar stuff.
> 
> Yeah a dmabuffs sounds overengineered to me as well.
> 
> It's a pity that we can't just open anonymous inodes under
> /proc/$pid/fd/$fd.
> 
> This way we would just need to encode the pid and fd number in the id
> transmitted to the other process and could solve it this way.
> 
> That an application changes the permissions of its own file descriptors
> because it needs to implement some questionable API design is not a problem
> of the kernel.

Hm could we fix this by making the inode non-anon? Maybe also quite a pile
of overkill ...

Another classic issue is lifetime of these names, I guess these are
supposed to be weak references, i.e. the uuid disappears with the last
buffer reference (like flink) and doesn't hold a reference of its own
(like dma-buf fd). Implementing that in a dmabuffs would be quite a pile
of fun.

So maybe it's a lot more nasty in the implementation, but dmabuffs feels a
pile cleaner for all this. Also gives us a clear thing to throw at the
next cuda reimplementation.
-Daniel

> 
> Regards,
> Christian.
> 
> > 
> > Wrt typing up an entire fs, I thought with all the libfs work that
> > shouldn't be too hard to pull off.
> > -Daniel
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

RE: [PATCH 1/1] drm/amdkfd: Add IPC API

[Li, Dennis]

[AMD Official Use Only - Internal Distribution Only]

Hi, Felix,
      amdgpu_gem_prime_export has different define in the old driver. I added some comment in the below codes. 

Best Regards
Dennis Li
-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Felix Kuehling
Sent: Tuesday, July 14, 2020 11:15 AM
To: amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; daniel.vetter@ffwll.ch; airlied@gmail.com
Subject: [PATCH 1/1] drm/amdkfd: Add IPC API

This allows exporting and importing buffers. The API generates handles that can be used with the HIP IPC API, i.e. big numbers rather than file descriptors.

Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h    |   5 +
 .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  56 +++-
 drivers/gpu/drm/amd/amdkfd/Makefile           |   3 +-
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c      |  74 ++---
 drivers/gpu/drm/amd/amdkfd/kfd_ipc.c          | 263 ++++++++++++++++++
 drivers/gpu/drm/amd/amdkfd/kfd_ipc.h          |  55 ++++
 drivers/gpu/drm/amd/amdkfd/kfd_module.c       |   5 +
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h         |   5 +
 include/uapi/linux/kfd_ioctl.h                |  62 ++++-
 9 files changed, 488 insertions(+), 40 deletions(-)  create mode 100644 drivers/gpu/drm/amd/amdkfd/kfd_ipc.c
 create mode 100644 drivers/gpu/drm/amd/amdkfd/kfd_ipc.h

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
index 3f2b695cf19e..0f8dc4c4f924 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
@@ -49,6 +49,7 @@ struct kfd_bo_va_list {  struct kgd_mem {
 	struct mutex lock;
 	struct amdgpu_bo *bo;
+	struct kfd_ipc_obj *ipc_obj;
 	struct list_head bo_va_list;
 	/* protected by amdkfd_process_info.lock */
 	struct ttm_validate_buffer validate_list; @@ -240,9 +241,13 @@ int amdgpu_amdkfd_gpuvm_get_vm_fault_info(struct kgd_dev *kgd,
 
 int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev *kgd,
 				      struct dma_buf *dmabuf,
+				      struct kfd_ipc_obj *ipc_obj,
 				      uint64_t va, void *vm,
 				      struct kgd_mem **mem, uint64_t *size,
 				      uint64_t *mmap_offset);
+int amdgpu_amdkfd_gpuvm_export_ipc_obj(struct kgd_dev *kgd, void *vm,
+				       struct kgd_mem *mem,
+				       struct kfd_ipc_obj **ipc_obj);
 
 void amdgpu_amdkfd_gpuvm_init_mem_limits(void);
 void amdgpu_amdkfd_unreserve_memory_limit(struct amdgpu_bo *bo); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
index c408936e8f98..cd5f23c0c2ca 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
@@ -29,6 +29,7 @@
 #include "amdgpu_vm.h"
 #include "amdgpu_amdkfd.h"
 #include "amdgpu_dma_buf.h"
+#include "kfd_ipc.h"
 #include <uapi/linux/kfd_ioctl.h>
 
 /* BO flag to indicate a KFD userptr BO */ @@ -1353,6 +1354,9 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
 			*size = 0;
 	}
 
+	/* Unreference the ipc_obj if applicable */
+	kfd_ipc_obj_put(&mem->ipc_obj);
+
 	/* Free the BO*/
 	drm_gem_object_put_unlocked(&mem->bo->tbo.base);
 	mutex_destroy(&mem->lock);
@@ -1656,6 +1660,7 @@ int amdgpu_amdkfd_gpuvm_get_vm_fault_info(struct kgd_dev *kgd,
 
 int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev *kgd,
 				      struct dma_buf *dma_buf,
+				      struct kfd_ipc_obj *ipc_obj,
 				      uint64_t va, void *vm,
 				      struct kgd_mem **mem, uint64_t *size,
 				      uint64_t *mmap_offset)
@@ -1692,15 +1697,18 @@ int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev *kgd,
 
 	INIT_LIST_HEAD(&(*mem)->bo_va_list);
 	mutex_init(&(*mem)->lock);
-	
-	(*mem)->alloc_flags =
-		((bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM) ?
-		KFD_IOC_ALLOC_MEM_FLAGS_VRAM : KFD_IOC_ALLOC_MEM_FLAGS_GTT)
-		| KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE
-		| KFD_IOC_ALLOC_MEM_FLAGS_EXECUTABLE;
+	if (bo->kfd_bo)
+		(*mem)->alloc_flags = bo->kfd_bo->alloc_flags;
+	else
+		(*mem)->alloc_flags =
+			((bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM) ?
+			KFD_IOC_ALLOC_MEM_FLAGS_VRAM : KFD_IOC_ALLOC_MEM_FLAGS_GTT)
+			| KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE
+			| KFD_IOC_ALLOC_MEM_FLAGS_EXECUTABLE;
 
 	drm_gem_object_get(&bo->tbo.base);
 	(*mem)->bo = bo;
+	(*mem)->ipc_obj = ipc_obj;
 	(*mem)->va = va;
 	(*mem)->domain = (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM) ?
 		AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT; @@ -1713,6 +1721,42 @@ int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev *kgd,
 	return 0;
 }
 
+int amdgpu_amdkfd_gpuvm_export_ipc_obj(struct kgd_dev *kgd, void *vm,
+				       struct kgd_mem *mem,
+				       struct kfd_ipc_obj **ipc_obj) {
+	struct amdgpu_device *adev = NULL;
+	struct dma_buf *dmabuf;
+	int r = 0;
+
+	if (!kgd || !vm || !mem)
+		return -EINVAL;
+
+	adev = get_amdgpu_device(kgd);
+	mutex_lock(&mem->lock);
+
+	if (mem->ipc_obj) {
+		*ipc_obj = mem->ipc_obj;
+		goto unlock_out;
+	}
+
+	dmabuf = amdgpu_gem_prime_export(&mem->bo->tbo.base, 0);

[Dennis Li] It may be changed as the following to make sure it also work well in old kernel. 
#ifdef HAVE_DRM_DRV_GEM_PRIME_EXPORT_PI
	dmabuf = amdgpu_gem_prime_export(&mem->bo->tbo.base, 0);
#else
	dmabuf = amdgpu_gem_prime_export(adev->ddev , &mem->bo->tbo.base, 0);
#endif

+	if (IS_ERR(dmabuf)) {
+		r = PTR_ERR(dmabuf);
+		goto unlock_out;
+	}
+
+	r = kfd_ipc_store_insert(dmabuf, &mem->ipc_obj);
+	if (r)
+		dma_buf_put(dmabuf);
+	else
+		*ipc_obj = mem->ipc_obj;
+
+unlock_out:
+	mutex_unlock(&mem->lock);
+	return r;
+}
+
 /* Evict a userptr BO by stopping the queues if necessary
  *
  * Runs in MMU notifier, may be in RECLAIM_FS context. This means it diff --git a/drivers/gpu/drm/amd/amdkfd/Makefile b/drivers/gpu/drm/amd/amdkfd/Makefile
index e1e4115dcf78..815d9a9e7a3c 100644
--- a/drivers/gpu/drm/amd/amdkfd/Makefile
+++ b/drivers/gpu/drm/amd/amdkfd/Makefile
@@ -54,7 +54,8 @@ AMDKFD_FILES	:= $(AMDKFD_PATH)/kfd_module.o \
 		$(AMDKFD_PATH)/kfd_dbgdev.o \
 		$(AMDKFD_PATH)/kfd_dbgmgr.o \
 		$(AMDKFD_PATH)/kfd_smi_events.o \
-		$(AMDKFD_PATH)/kfd_crat.o
+		$(AMDKFD_PATH)/kfd_crat.o \
+		$(AMDKFD_PATH)/kfd_ipc.o
 
 ifneq ($(CONFIG_AMD_IOMMU_V2),)
 AMDKFD_FILES += $(AMDKFD_PATH)/kfd_iommu.o diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index e9b96ad3d9a5..e7d15fa02b5e 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -38,6 +38,7 @@
 #include "kfd_priv.h"
 #include "kfd_device_queue_manager.h"
 #include "kfd_dbgmgr.h"
+#include "kfd_ipc.h"
 #include "amdgpu_amdkfd.h"
 #include "kfd_smi_events.h"
 
@@ -1691,53 +1692,58 @@ static int kfd_ioctl_import_dmabuf(struct file *filep,
 				   struct kfd_process *p, void *data)  {
 	struct kfd_ioctl_import_dmabuf_args *args = data;
-	struct kfd_process_device *pdd;
-	struct dma_buf *dmabuf;
 	struct kfd_dev *dev;
-	int idr_handle;
-	uint64_t size;
-	void *mem;
 	int r;
 
 	dev = kfd_device_by_id(args->gpu_id);
 	if (!dev)
 		return -EINVAL;
 
-	dmabuf = dma_buf_get(args->dmabuf_fd);
-	if (IS_ERR(dmabuf))
-		return PTR_ERR(dmabuf);
+	r = kfd_ipc_import_dmabuf(dev, p, args->dmabuf_fd,
+				  args->va_addr, &args->handle, NULL);
+	if (r)
+		pr_err("Failed to import dmabuf\n");
 
-	mutex_lock(&p->mutex);
+	return r;
+}
 
-	pdd = kfd_bind_process_to_device(dev, p);
-	if (IS_ERR(pdd)) {
-		r = PTR_ERR(pdd);
-		goto err_unlock;
-	}
+static int kfd_ioctl_ipc_export_handle(struct file *filep,
+				       struct kfd_process *p,
+				       void *data)
+{
+	struct kfd_ioctl_ipc_export_handle_args *args = data;
+	struct kfd_dev *dev;
+	int r;
+
+	dev = kfd_device_by_id(args->gpu_id);
+	if (!dev)
+		return -EINVAL;
 
-	r = amdgpu_amdkfd_gpuvm_import_dmabuf(dev->kgd, dmabuf,
-					      args->va_addr, pdd->vm,
-					      (struct kgd_mem **)&mem, &size,
-					      NULL);
+	r = kfd_ipc_export_as_handle(dev, p, args->handle, 
+args->share_handle);
 	if (r)
-		goto err_unlock;
+		pr_err("Failed to export IPC handle\n");
 
-	idr_handle = kfd_process_device_create_obj_handle(pdd, mem);
-	if (idr_handle < 0) {
-		r = -EFAULT;
-		goto err_free;
-	}
+	return r;
+}
 
-	mutex_unlock(&p->mutex);
+static int kfd_ioctl_ipc_import_handle(struct file *filep,
+				       struct kfd_process *p,
+				       void *data)
+{
+	struct kfd_ioctl_ipc_import_handle_args *args = data;
+	struct kfd_dev *dev = NULL;
+	int r;
 
-	args->handle = MAKE_HANDLE(args->gpu_id, idr_handle);
+	dev = kfd_device_by_id(args->gpu_id);
+	if (!dev)
+		return -EINVAL;
 
-	return 0;
+	r = kfd_ipc_import_handle(p, args->share_handle, args->va_addr,
+				  &args->handle, &args->gpu_id,
+				  &args->mmap_offset);
+	if (r)
+		pr_err("Failed to import IPC handle\n");
 
-err_free:
-	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, (struct kgd_mem *)mem, NULL);
-err_unlock:
-	mutex_unlock(&p->mutex);
 	return r;
 }
 
@@ -1853,6 +1859,12 @@ static const struct amdkfd_ioctl_desc amdkfd_ioctls[] = {
 
 	AMDKFD_IOCTL_DEF(AMDKFD_IOC_SMI_EVENTS,
 			kfd_ioctl_smi_events, 0),
+
+	AMDKFD_IOCTL_DEF(AMDKFD_IOC_IPC_IMPORT_HANDLE,
+				kfd_ioctl_ipc_import_handle, 0),
+
+	AMDKFD_IOCTL_DEF(AMDKFD_IOC_IPC_EXPORT_HANDLE,
+				kfd_ioctl_ipc_export_handle, 0),
 };
 
 #define AMDKFD_CORE_IOCTL_COUNT	ARRAY_SIZE(amdkfd_ioctls)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_ipc.c b/drivers/gpu/drm/amd/amdkfd/kfd_ipc.c
new file mode 100644
index 000000000000..3de8d7826f07
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_ipc.c
@@ -0,0 +1,263 @@
+/*
+ * Copyright 2014 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person 
+obtaining a
+ * copy of this software and associated documentation files (the 
+"Software"),
+ * to deal in the Software without restriction, including without 
+limitation
+ * the rights to use, copy, modify, merge, publish, distribute, 
+sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom 
+the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be 
+included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
+EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
+MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT 
+SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, 
+DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR 
+OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE 
+OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+#include <linux/slab.h>
+#include <linux/random.h>
+
+#include "kfd_ipc.h"
+#include "kfd_priv.h"
+#include "amdgpu_amdkfd.h"
+
+#define KFD_IPC_HASH_TABLE_SIZE_SHIFT 4 #define 
+KFD_IPC_HASH_TABLE_SIZE_MASK ((1 << KFD_IPC_HASH_TABLE_SIZE_SHIFT) - 1)
+
+static struct kfd_ipc_handles {
+	DECLARE_HASHTABLE(handles, KFD_IPC_HASH_TABLE_SIZE_SHIFT);
+	struct mutex lock;
+} kfd_ipc_handles;
+
+/* Since, handles are random numbers, it can be used directly as hashing key.
+ * The least 4 bits of the handle are used as key. However, during 
+import all
+ * 128 bits of the handle are checked to prevent handle snooping.
+ */
+#define HANDLE_TO_KEY(sh) ((*(uint64_t *)sh) & 
+KFD_IPC_HASH_TABLE_SIZE_MASK)
+
+int kfd_ipc_store_insert(struct dma_buf *dmabuf, struct kfd_ipc_obj 
+**ipc_obj) {
+	struct kfd_ipc_obj *obj;
+
+	obj = kmalloc(sizeof(*obj), GFP_KERNEL);
+	if (!obj)
+		return -ENOMEM;
+
+	/* The initial ref belongs to the allocator process.
+	 * The IPC object store itself does not hold a ref since
+	 * there is no specific moment in time where that ref should
+	 * be dropped, except "when there are no more userspace processes
+	 * holding a ref to the object". Therefore the removal from IPC
+	 * storage happens at ipc_obj release time.
+	 */
+	kref_init(&obj->ref);
+	obj->dmabuf = dmabuf;
+	get_random_bytes(obj->share_handle, sizeof(obj->share_handle));
+
+	mutex_lock(&kfd_ipc_handles.lock);
+	hlist_add_head(&obj->node,
+		&kfd_ipc_handles.handles[HANDLE_TO_KEY(obj->share_handle)]);
+	mutex_unlock(&kfd_ipc_handles.lock);
+
+	if (ipc_obj)
+		*ipc_obj = obj;
+
+	return 0;
+}
+
+static void ipc_obj_release(struct kref *r) {
+	struct kfd_ipc_obj *obj;
+
+	obj = container_of(r, struct kfd_ipc_obj, ref);
+
+	mutex_lock(&kfd_ipc_handles.lock);
+	hash_del(&obj->node);
+	mutex_unlock(&kfd_ipc_handles.lock);
+
+	dma_buf_put(obj->dmabuf);
+	kfree(obj);
+}
+
+static struct kfd_ipc_obj *ipc_obj_get(struct kfd_ipc_obj *obj) {
+	if (kref_get_unless_zero(&obj->ref))
+		return obj;
+	return NULL;
+}
+
+void kfd_ipc_obj_put(struct kfd_ipc_obj **obj) {
+	if (*obj) {
+		kref_put(&(*obj)->ref, ipc_obj_release);
+		*obj = NULL;
+	}
+}
+
+int kfd_ipc_init(void)
+{
+	mutex_init(&kfd_ipc_handles.lock);
+	hash_init(kfd_ipc_handles.handles);
+	return 0;
+}
+
+static int kfd_import_dmabuf_create_kfd_bo(struct kfd_dev *dev,
+					   struct kfd_process *p,
+					   struct dma_buf *dmabuf,
+					   struct kfd_ipc_obj *ipc_obj,
+					   uint64_t va_addr, uint64_t *handle,
+					   uint64_t *mmap_offset)
+{
+	int r;
+	void *mem;
+	uint64_t size;
+	int idr_handle;
+	struct kfd_process_device *pdd = NULL;
+
+	if (!handle)
+		return -EINVAL;
+
+	if (!dev)
+		return -EINVAL;
+
+	mutex_lock(&p->mutex);
+
+	pdd = kfd_bind_process_to_device(dev, p);
+	if (IS_ERR(pdd)) {
+		r = PTR_ERR(pdd);
+		goto err_unlock;
+	}
+
+	r = amdgpu_amdkfd_gpuvm_import_dmabuf(dev->kgd, dmabuf, ipc_obj,
+					va_addr, pdd->vm,
+					(struct kgd_mem **)&mem, &size,
+					mmap_offset);
+	if (r)
+		goto err_unlock;
+
+	idr_handle = kfd_process_device_create_obj_handle(pdd, mem);
+	if (idr_handle < 0) {
+		r = -EFAULT;
+		goto err_free;
+	}
+
+	mutex_unlock(&p->mutex);
+
+	*handle = MAKE_HANDLE(dev->id, idr_handle);
+
+	return 0;
+
+err_free:
+	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, (struct kgd_mem *)mem,
+					       NULL);
+err_unlock:
+	mutex_unlock(&p->mutex);
+	return r;
+}
+
+int kfd_ipc_import_dmabuf(struct kfd_dev *dev, struct kfd_process *p,
+			  int dmabuf_fd, uint64_t va_addr,
+			  uint64_t *handle, uint64_t *mmap_offset) {
+	int r;
+	struct dma_buf *dmabuf = dma_buf_get(dmabuf_fd);
+
+	if (!dmabuf)
+		return -EINVAL;
+
+	r = kfd_import_dmabuf_create_kfd_bo(dev, p, dmabuf, NULL,
+					    va_addr, handle, mmap_offset);
+	dma_buf_put(dmabuf);
+	return r;
+}
+
+int kfd_ipc_import_handle(struct kfd_process *p,
+			  uint32_t *share_handle, uint64_t va_addr,
+			  uint64_t *handle, uint32_t *gpu_id,
+			  uint64_t *mmap_offset)
+{
+	struct kfd_dev *dev;
+	int r;
+	struct kfd_ipc_obj *entry, *found = NULL;
+
+	mutex_lock(&kfd_ipc_handles.lock);
+	/* Convert the user provided handle to hash key and search only in that
+	 * bucket
+	 */
+	hlist_for_each_entry(entry,
+		&kfd_ipc_handles.handles[HANDLE_TO_KEY(share_handle)], node) {
+		if (!memcmp(entry->share_handle, share_handle,
+			    sizeof(entry->share_handle))) {
+			found = ipc_obj_get(entry);
+			break;
+		}
+	}
+	mutex_unlock(&kfd_ipc_handles.lock);
+
+	if (!found)
+		return -EINVAL;
+
+	pr_debug("Found ipc_dma_buf: %p\n", found->dmabuf);
+
+	dev = kfd_device_by_id(found->gpu_id);
+	if (!dev)
+		return -ENODEV;
+
+	r = kfd_import_dmabuf_create_kfd_bo(dev, p, found->dmabuf, found,
+					    va_addr, handle, mmap_offset);
+	if (r)
+		goto error_unref;
+
+	*gpu_id = found->gpu_id;
+
+	return r;
+
+error_unref:
+	kfd_ipc_obj_put(&found);
+	return r;
+}
+
+int kfd_ipc_export_as_handle(struct kfd_dev *dev, struct kfd_process *p,
+			     uint64_t handle, uint32_t *ipc_handle) {
+	struct kfd_process_device *pdd = NULL;
+	struct kfd_ipc_obj *ipc_obj;
+	struct kgd_mem *mem;
+	int r;
+
+	if (!dev || !ipc_handle)
+		return -EINVAL;
+
+	mutex_lock(&p->mutex);
+	pdd = kfd_bind_process_to_device(dev, p);
+	if (IS_ERR(pdd)) {
+		mutex_unlock(&p->mutex);
+		pr_err("Failed to get pdd\n");
+		return PTR_ERR(pdd);
+	}
+
+	mem = kfd_process_device_translate_handle(pdd, GET_IDR_HANDLE(handle));
+	mutex_unlock(&p->mutex);
+
+	if (!mem) {
+		pr_err("Failed to get bo");
+		return -EINVAL;
+	}
+
+	r = amdgpu_amdkfd_gpuvm_export_ipc_obj(dev->kgd, pdd->vm, mem,
+					       &ipc_obj);
+	if (r)
+		return r;
+
+	ipc_obj->gpu_id = dev->id;
+	memcpy(ipc_handle, ipc_obj->share_handle,
+	       sizeof(ipc_obj->share_handle));
+
+	return r;
+}
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_ipc.h b/drivers/gpu/drm/amd/amdkfd/kfd_ipc.h
new file mode 100644
index 000000000000..9450a667918e
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_ipc.h
@@ -0,0 +1,55 @@
+/*
+ * Copyright 2014 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person 
+obtaining a
+ * copy of this software and associated documentation files (the 
+"Software"),
+ * to deal in the Software without restriction, including without 
+limitation
+ * the rights to use, copy, modify, merge, publish, distribute, 
+sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom 
+the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be 
+included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
+EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
+MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT 
+SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, 
+DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR 
+OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE 
+OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+#ifndef KFD_IPC_H_
+#define KFD_IPC_H_
+
+#include <linux/types.h>
+#include <linux/dma-buf.h>
+
+/* avoid including kfd_priv.h */
+struct kfd_dev;
+struct kfd_process;
+
+struct kfd_ipc_obj {
+	struct hlist_node node;
+	struct kref ref;
+	struct dma_buf *dmabuf;
+	uint32_t share_handle[4];
+	uint32_t gpu_id;
+};
+
+int kfd_ipc_import_handle(struct kfd_process *p,
+			  uint32_t *share_handle, uint64_t va_addr,
+			  uint64_t *handle, uint32_t *gpu_id,
+			  uint64_t *mmap_offset);
+int kfd_ipc_import_dmabuf(struct kfd_dev *kfd, struct kfd_process *p,
+			  int dmabuf_fd, uint64_t va_addr,
+			  uint64_t *handle, uint64_t *mmap_offset); int 
+kfd_ipc_export_as_handle(struct kfd_dev *dev, struct kfd_process *p,
+			     uint64_t handle, uint32_t *ipc_handle);
+
+int kfd_ipc_store_insert(struct dma_buf *dmabuf, struct kfd_ipc_obj 
+**ipc_obj); void kfd_ipc_obj_put(struct kfd_ipc_obj **obj);
+
+#endif /* KFD_IPC_H_ */
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_module.c b/drivers/gpu/drm/amd/amdkfd/kfd_module.c
index f4b7f7e6c40e..0946d5692692 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_module.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_module.c
@@ -52,6 +52,10 @@ static int kfd_init(void)
 	if (err < 0)
 		goto err_topology;
 
+	err = kfd_ipc_init();
+	if (err < 0)
+		goto err_ipc;
+
 	err = kfd_process_create_wq();
 	if (err < 0)
 		goto err_create_wq;
@@ -66,6 +70,7 @@ static int kfd_init(void)
 	return 0;
 
 err_create_wq:
+err_ipc:
 	kfd_topology_shutdown();
 err_topology:
 	kfd_chardev_exit();
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 51ba2020732e..1588b2b45a32 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -321,6 +321,8 @@ struct kfd_dev {
 	spinlock_t smi_lock;
 };
 
+struct kfd_ipc_obj;
+
 enum kfd_mempool {
 	KFD_MEMPOOL_SYSTEM_CACHEABLE = 1,
 	KFD_MEMPOOL_SYSTEM_WRITECOMBINE = 2,
@@ -1087,6 +1089,9 @@ int dbgdev_wave_reset_wavefronts(struct kfd_dev *dev, struct kfd_process *p);
 
 bool kfd_is_locked(void);
 
+/* IPC Support */
+int kfd_ipc_init(void);
+
 /* Compute profile */
 void kfd_inc_compute_active(struct kfd_dev *dev);  void kfd_dec_compute_active(struct kfd_dev *dev); diff --git a/include/uapi/linux/kfd_ioctl.h b/include/uapi/linux/kfd_ioctl.h index f738c3b53f4e..90ff334e2b5b 100644
--- a/include/uapi/linux/kfd_ioctl.h
+++ b/include/uapi/linux/kfd_ioctl.h
@@ -29,9 +29,10 @@
 /*
  * - 1.1 - initial version
  * - 1.3 - Add SMI events support
+ * - 1.4 - Add IPC export/import
  */
 #define KFD_IOCTL_MAJOR_VERSION 1
-#define KFD_IOCTL_MINOR_VERSION 3
+#define KFD_IOCTL_MINOR_VERSION 4
 
 struct kfd_ioctl_get_version_args {
 	__u32 major_version;	/* from KFD */
@@ -464,6 +465,57 @@ enum kfd_mmio_remap {
 	KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL = 4,  };
 
+/* Export IPC handle
+ *
+ * @handle[in]:         buffer handle of the BO to export
+ * @gpu_id[in]:         GPU ID where @handle was allocated
+ * @share_handle[out]:  share handle that can be used with
+ *                      @kfd_ioctl_ipc_import_handle_args
+ *
+ * @share_handle is a 128 bit random number generated with
+ * @get_random_bytes. This number should be very hard to guess.
+ * Knowledge of the @share_handle implies authorization to access
+ * the shared memory. User mode should treat it like a secret key.
+ * It can be used to import this BO in a different process context
+ * for IPC buffer sharing. The handle will be valid as long as the
+ * underlying BO exists. If the same BO is exported multiple times,
+ * the same handle will be returned.
+ *
+ * Return 0 on success, negative errno on errors.
+ */
+struct kfd_ioctl_ipc_export_handle_args {
+	__u64 handle;		/* to KFD */
+	__u32 share_handle[4];	/* from KFD */
+	__u32 gpu_id;		/* to KFD */
+	__u32 pad;
+};
+
+/* Import IPC handle
+ *
+ * @share_handle[in]:  share handle from @kfd_ioctl_ipc_export_handle_args
+ * @va_addr[in]:       virtual address at which to import the BO
+ * @handle[out]:       buffer handle of the imported BO
+ * @gpu_id[out]:       device in which the shared BO was created
+ * @mmap_offset[out]:  mmap offset for CPU-mapping the BO
+ *
+ * @handle represents a new reference to the shared BO. This reference
+ * must be released with kfd_ioctl_free_memory_of_gpu_args.
+ *
+ * The BO can be mapped for GPU access with @kfd_ioctl_map_memory_to_gpu_args.
+ *
+ * It can be mapped for CPU access using the @mmap_offset.
+ *
+ * Return 0 on success, negative errno on errors.
+ */
+struct kfd_ioctl_ipc_import_handle_args {
+	__u64 handle;		/* from KFD */
+	__u64 va_addr;		/* to KFD */
+	__u64 mmap_offset;	/* from KFD */
+	__u32 share_handle[4];	/* to KFD */
+	__u32 gpu_id;		/* from KFD */
+	__u32 pad;
+};
+
 #define AMDKFD_IOCTL_BASE 'K'
 #define AMDKFD_IO(nr)			_IO(AMDKFD_IOCTL_BASE, nr)
 #define AMDKFD_IOR(nr, type)		_IOR(AMDKFD_IOCTL_BASE, nr, type)
@@ -564,7 +616,13 @@ enum kfd_mmio_remap {
 #define AMDKFD_IOC_SMI_EVENTS			\
 		AMDKFD_IOWR(0x1F, struct kfd_ioctl_smi_events_args)
 
+#define AMDKFD_IOC_IPC_IMPORT_HANDLE                                    \
+		AMDKFD_IOWR(0x20, struct kfd_ioctl_ipc_import_handle_args)
+
+#define AMDKFD_IOC_IPC_EXPORT_HANDLE		\
+		AMDKFD_IOWR(0x21, struct kfd_ioctl_ipc_export_handle_args)
+
 #define AMDKFD_COMMAND_START		0x01
-#define AMDKFD_COMMAND_END		0x20
+#define AMDKFD_COMMAND_END		0x22
 
 #endif
--
2.27.0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7CDennis.Li%40amd.com%7C3c0d831fd7404f9ae71508d827a4191b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637302933043451790&amp;sdata=FxF5FhJK8DZ3RNOmKHMXAFfUNb%2BvQUs3V0LZAwQkC24%3D&amp;reserved=0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/1] drm/amdkfd: Add IPC API

[Felix Kuehling]

Am 2020-07-14 um 1:05 a.m. schrieb Li, Dennis:
> [AMD Official Use Only - Internal Distribution Only]
>
> Hi, Felix,
>       amdgpu_gem_prime_export has different define in the old driver. I added some comment in the below codes. 
>
> Best Regards
> Dennis Li
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Felix Kuehling
> Sent: Tuesday, July 14, 2020 11:15 AM
> To: amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org
> Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; daniel.vetter@ffwll.ch; airlied@gmail.com
> Subject: [PATCH 1/1] drm/amdkfd: Add IPC API
>
> This allows exporting and importing buffers. The API generates handles that can be used with the HIP IPC API, i.e. big numbers rather than file descriptors.
>
> Signed-off-by: Felix Kuehling <Felix.Kuehling@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h    |   5 +
>  .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c  |  56 +++-
>  drivers/gpu/drm/amd/amdkfd/Makefile           |   3 +-
>  drivers/gpu/drm/amd/amdkfd/kfd_chardev.c      |  74 ++---
>  drivers/gpu/drm/amd/amdkfd/kfd_ipc.c          | 263 ++++++++++++++++++
>  drivers/gpu/drm/amd/amdkfd/kfd_ipc.h          |  55 ++++
>  drivers/gpu/drm/amd/amdkfd/kfd_module.c       |   5 +
>  drivers/gpu/drm/amd/amdkfd/kfd_priv.h         |   5 +
>  include/uapi/linux/kfd_ioctl.h                |  62 ++++-
>  9 files changed, 488 insertions(+), 40 deletions(-)  create mode 100644 drivers/gpu/drm/amd/amdkfd/kfd_ipc.c
>  create mode 100644 drivers/gpu/drm/amd/amdkfd/kfd_ipc.h
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> index 3f2b695cf19e..0f8dc4c4f924 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> @@ -49,6 +49,7 @@ struct kfd_bo_va_list {  struct kgd_mem {
>  	struct mutex lock;
>  	struct amdgpu_bo *bo;
> +	struct kfd_ipc_obj *ipc_obj;
>  	struct list_head bo_va_list;
>  	/* protected by amdkfd_process_info.lock */
>  	struct ttm_validate_buffer validate_list; @@ -240,9 +241,13 @@ int amdgpu_amdkfd_gpuvm_get_vm_fault_info(struct kgd_dev *kgd,
>  
>  int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev *kgd,
>  				      struct dma_buf *dmabuf,
> +				      struct kfd_ipc_obj *ipc_obj,
>  				      uint64_t va, void *vm,
>  				      struct kgd_mem **mem, uint64_t *size,
>  				      uint64_t *mmap_offset);
> +int amdgpu_amdkfd_gpuvm_export_ipc_obj(struct kgd_dev *kgd, void *vm,
> +				       struct kgd_mem *mem,
> +				       struct kfd_ipc_obj **ipc_obj);
>  
>  void amdgpu_amdkfd_gpuvm_init_mem_limits(void);
>  void amdgpu_amdkfd_unreserve_memory_limit(struct amdgpu_bo *bo); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index c408936e8f98..cd5f23c0c2ca 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -29,6 +29,7 @@
>  #include "amdgpu_vm.h"
>  #include "amdgpu_amdkfd.h"
>  #include "amdgpu_dma_buf.h"
> +#include "kfd_ipc.h"
>  #include <uapi/linux/kfd_ioctl.h>
>  
>  /* BO flag to indicate a KFD userptr BO */ @@ -1353,6 +1354,9 @@ int amdgpu_amdkfd_gpuvm_free_memory_of_gpu(
>  			*size = 0;
>  	}
>  
> +	/* Unreference the ipc_obj if applicable */
> +	kfd_ipc_obj_put(&mem->ipc_obj);
> +
>  	/* Free the BO*/
>  	drm_gem_object_put_unlocked(&mem->bo->tbo.base);
>  	mutex_destroy(&mem->lock);
> @@ -1656,6 +1660,7 @@ int amdgpu_amdkfd_gpuvm_get_vm_fault_info(struct kgd_dev *kgd,
>  
>  int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev *kgd,
>  				      struct dma_buf *dma_buf,
> +				      struct kfd_ipc_obj *ipc_obj,
>  				      uint64_t va, void *vm,
>  				      struct kgd_mem **mem, uint64_t *size,
>  				      uint64_t *mmap_offset)
> @@ -1692,15 +1697,18 @@ int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev *kgd,
>  
>  	INIT_LIST_HEAD(&(*mem)->bo_va_list);
>  	mutex_init(&(*mem)->lock);
> -	
> -	(*mem)->alloc_flags =
> -		((bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM) ?
> -		KFD_IOC_ALLOC_MEM_FLAGS_VRAM : KFD_IOC_ALLOC_MEM_FLAGS_GTT)
> -		| KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE
> -		| KFD_IOC_ALLOC_MEM_FLAGS_EXECUTABLE;
> +	if (bo->kfd_bo)
> +		(*mem)->alloc_flags = bo->kfd_bo->alloc_flags;
> +	else
> +		(*mem)->alloc_flags =
> +			((bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM) ?
> +			KFD_IOC_ALLOC_MEM_FLAGS_VRAM : KFD_IOC_ALLOC_MEM_FLAGS_GTT)
> +			| KFD_IOC_ALLOC_MEM_FLAGS_WRITABLE
> +			| KFD_IOC_ALLOC_MEM_FLAGS_EXECUTABLE;
>  
>  	drm_gem_object_get(&bo->tbo.base);
>  	(*mem)->bo = bo;
> +	(*mem)->ipc_obj = ipc_obj;
>  	(*mem)->va = va;
>  	(*mem)->domain = (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM) ?
>  		AMDGPU_GEM_DOMAIN_VRAM : AMDGPU_GEM_DOMAIN_GTT; @@ -1713,6 +1721,42 @@ int amdgpu_amdkfd_gpuvm_import_dmabuf(struct kgd_dev *kgd,
>  	return 0;
>  }
>  
> +int amdgpu_amdkfd_gpuvm_export_ipc_obj(struct kgd_dev *kgd, void *vm,
> +				       struct kgd_mem *mem,
> +				       struct kfd_ipc_obj **ipc_obj) {
> +	struct amdgpu_device *adev = NULL;
> +	struct dma_buf *dmabuf;
> +	int r = 0;
> +
> +	if (!kgd || !vm || !mem)
> +		return -EINVAL;
> +
> +	adev = get_amdgpu_device(kgd);
> +	mutex_lock(&mem->lock);
> +
> +	if (mem->ipc_obj) {
> +		*ipc_obj = mem->ipc_obj;
> +		goto unlock_out;
> +	}
> +
> +	dmabuf = amdgpu_gem_prime_export(&mem->bo->tbo.base, 0);
>
> [Dennis Li] It may be changed as the following to make sure it also work well in old kernel. 
> #ifdef HAVE_DRM_DRV_GEM_PRIME_EXPORT_PI
> 	dmabuf = amdgpu_gem_prime_export(&mem->bo->tbo.base, 0);
> #else
> 	dmabuf = amdgpu_gem_prime_export(adev->ddev , &mem->bo->tbo.base, 0);
> #endif

I know, this is what this code looks like on the DKMS branch that needs
to build against old kernel versions. But this change is meant for
upstream. It doesn't need to work with old kernels, only with the
current one.

Regards,
  Felix


>
> +	if (IS_ERR(dmabuf)) {
> +		r = PTR_ERR(dmabuf);
> +		goto unlock_out;
> +	}
> +
> +	r = kfd_ipc_store_insert(dmabuf, &mem->ipc_obj);
> +	if (r)
> +		dma_buf_put(dmabuf);
> +	else
> +		*ipc_obj = mem->ipc_obj;
> +
> +unlock_out:
> +	mutex_unlock(&mem->lock);
> +	return r;
> +}
> +
>  /* Evict a userptr BO by stopping the queues if necessary
>   *
>   * Runs in MMU notifier, may be in RECLAIM_FS context. This means it diff --git a/drivers/gpu/drm/amd/amdkfd/Makefile b/drivers/gpu/drm/amd/amdkfd/Makefile
> index e1e4115dcf78..815d9a9e7a3c 100644
> --- a/drivers/gpu/drm/amd/amdkfd/Makefile
> +++ b/drivers/gpu/drm/amd/amdkfd/Makefile
> @@ -54,7 +54,8 @@ AMDKFD_FILES	:= $(AMDKFD_PATH)/kfd_module.o \
>  		$(AMDKFD_PATH)/kfd_dbgdev.o \
>  		$(AMDKFD_PATH)/kfd_dbgmgr.o \
>  		$(AMDKFD_PATH)/kfd_smi_events.o \
> -		$(AMDKFD_PATH)/kfd_crat.o
> +		$(AMDKFD_PATH)/kfd_crat.o \
> +		$(AMDKFD_PATH)/kfd_ipc.o
>  
>  ifneq ($(CONFIG_AMD_IOMMU_V2),)
>  AMDKFD_FILES += $(AMDKFD_PATH)/kfd_iommu.o diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index e9b96ad3d9a5..e7d15fa02b5e 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -38,6 +38,7 @@
>  #include "kfd_priv.h"
>  #include "kfd_device_queue_manager.h"
>  #include "kfd_dbgmgr.h"
> +#include "kfd_ipc.h"
>  #include "amdgpu_amdkfd.h"
>  #include "kfd_smi_events.h"
>  
> @@ -1691,53 +1692,58 @@ static int kfd_ioctl_import_dmabuf(struct file *filep,
>  				   struct kfd_process *p, void *data)  {
>  	struct kfd_ioctl_import_dmabuf_args *args = data;
> -	struct kfd_process_device *pdd;
> -	struct dma_buf *dmabuf;
>  	struct kfd_dev *dev;
> -	int idr_handle;
> -	uint64_t size;
> -	void *mem;
>  	int r;
>  
>  	dev = kfd_device_by_id(args->gpu_id);
>  	if (!dev)
>  		return -EINVAL;
>  
> -	dmabuf = dma_buf_get(args->dmabuf_fd);
> -	if (IS_ERR(dmabuf))
> -		return PTR_ERR(dmabuf);
> +	r = kfd_ipc_import_dmabuf(dev, p, args->dmabuf_fd,
> +				  args->va_addr, &args->handle, NULL);
> +	if (r)
> +		pr_err("Failed to import dmabuf\n");
>  
> -	mutex_lock(&p->mutex);
> +	return r;
> +}
>  
> -	pdd = kfd_bind_process_to_device(dev, p);
> -	if (IS_ERR(pdd)) {
> -		r = PTR_ERR(pdd);
> -		goto err_unlock;
> -	}
> +static int kfd_ioctl_ipc_export_handle(struct file *filep,
> +				       struct kfd_process *p,
> +				       void *data)
> +{
> +	struct kfd_ioctl_ipc_export_handle_args *args = data;
> +	struct kfd_dev *dev;
> +	int r;
> +
> +	dev = kfd_device_by_id(args->gpu_id);
> +	if (!dev)
> +		return -EINVAL;
>  
> -	r = amdgpu_amdkfd_gpuvm_import_dmabuf(dev->kgd, dmabuf,
> -					      args->va_addr, pdd->vm,
> -					      (struct kgd_mem **)&mem, &size,
> -					      NULL);
> +	r = kfd_ipc_export_as_handle(dev, p, args->handle, 
> +args->share_handle);
>  	if (r)
> -		goto err_unlock;
> +		pr_err("Failed to export IPC handle\n");
>  
> -	idr_handle = kfd_process_device_create_obj_handle(pdd, mem);
> -	if (idr_handle < 0) {
> -		r = -EFAULT;
> -		goto err_free;
> -	}
> +	return r;
> +}
>  
> -	mutex_unlock(&p->mutex);
> +static int kfd_ioctl_ipc_import_handle(struct file *filep,
> +				       struct kfd_process *p,
> +				       void *data)
> +{
> +	struct kfd_ioctl_ipc_import_handle_args *args = data;
> +	struct kfd_dev *dev = NULL;
> +	int r;
>  
> -	args->handle = MAKE_HANDLE(args->gpu_id, idr_handle);
> +	dev = kfd_device_by_id(args->gpu_id);
> +	if (!dev)
> +		return -EINVAL;
>  
> -	return 0;
> +	r = kfd_ipc_import_handle(p, args->share_handle, args->va_addr,
> +				  &args->handle, &args->gpu_id,
> +				  &args->mmap_offset);
> +	if (r)
> +		pr_err("Failed to import IPC handle\n");
>  
> -err_free:
> -	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, (struct kgd_mem *)mem, NULL);
> -err_unlock:
> -	mutex_unlock(&p->mutex);
>  	return r;
>  }
>  
> @@ -1853,6 +1859,12 @@ static const struct amdkfd_ioctl_desc amdkfd_ioctls[] = {
>  
>  	AMDKFD_IOCTL_DEF(AMDKFD_IOC_SMI_EVENTS,
>  			kfd_ioctl_smi_events, 0),
> +
> +	AMDKFD_IOCTL_DEF(AMDKFD_IOC_IPC_IMPORT_HANDLE,
> +				kfd_ioctl_ipc_import_handle, 0),
> +
> +	AMDKFD_IOCTL_DEF(AMDKFD_IOC_IPC_EXPORT_HANDLE,
> +				kfd_ioctl_ipc_export_handle, 0),
>  };
>  
>  #define AMDKFD_CORE_IOCTL_COUNT	ARRAY_SIZE(amdkfd_ioctls)
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_ipc.c b/drivers/gpu/drm/amd/amdkfd/kfd_ipc.c
> new file mode 100644
> index 000000000000..3de8d7826f07
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_ipc.c
> @@ -0,0 +1,263 @@
> +/*
> + * Copyright 2014 Advanced Micro Devices, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person 
> +obtaining a
> + * copy of this software and associated documentation files (the 
> +"Software"),
> + * to deal in the Software without restriction, including without 
> +limitation
> + * the rights to use, copy, modify, merge, publish, distribute, 
> +sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom 
> +the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be 
> +included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
> +EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
> +MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT 
> +SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, 
> +DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR 
> +OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE 
> +OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +#include <linux/slab.h>
> +#include <linux/random.h>
> +
> +#include "kfd_ipc.h"
> +#include "kfd_priv.h"
> +#include "amdgpu_amdkfd.h"
> +
> +#define KFD_IPC_HASH_TABLE_SIZE_SHIFT 4 #define 
> +KFD_IPC_HASH_TABLE_SIZE_MASK ((1 << KFD_IPC_HASH_TABLE_SIZE_SHIFT) - 1)
> +
> +static struct kfd_ipc_handles {
> +	DECLARE_HASHTABLE(handles, KFD_IPC_HASH_TABLE_SIZE_SHIFT);
> +	struct mutex lock;
> +} kfd_ipc_handles;
> +
> +/* Since, handles are random numbers, it can be used directly as hashing key.
> + * The least 4 bits of the handle are used as key. However, during 
> +import all
> + * 128 bits of the handle are checked to prevent handle snooping.
> + */
> +#define HANDLE_TO_KEY(sh) ((*(uint64_t *)sh) & 
> +KFD_IPC_HASH_TABLE_SIZE_MASK)
> +
> +int kfd_ipc_store_insert(struct dma_buf *dmabuf, struct kfd_ipc_obj 
> +**ipc_obj) {
> +	struct kfd_ipc_obj *obj;
> +
> +	obj = kmalloc(sizeof(*obj), GFP_KERNEL);
> +	if (!obj)
> +		return -ENOMEM;
> +
> +	/* The initial ref belongs to the allocator process.
> +	 * The IPC object store itself does not hold a ref since
> +	 * there is no specific moment in time where that ref should
> +	 * be dropped, except "when there are no more userspace processes
> +	 * holding a ref to the object". Therefore the removal from IPC
> +	 * storage happens at ipc_obj release time.
> +	 */
> +	kref_init(&obj->ref);
> +	obj->dmabuf = dmabuf;
> +	get_random_bytes(obj->share_handle, sizeof(obj->share_handle));
> +
> +	mutex_lock(&kfd_ipc_handles.lock);
> +	hlist_add_head(&obj->node,
> +		&kfd_ipc_handles.handles[HANDLE_TO_KEY(obj->share_handle)]);
> +	mutex_unlock(&kfd_ipc_handles.lock);
> +
> +	if (ipc_obj)
> +		*ipc_obj = obj;
> +
> +	return 0;
> +}
> +
> +static void ipc_obj_release(struct kref *r) {
> +	struct kfd_ipc_obj *obj;
> +
> +	obj = container_of(r, struct kfd_ipc_obj, ref);
> +
> +	mutex_lock(&kfd_ipc_handles.lock);
> +	hash_del(&obj->node);
> +	mutex_unlock(&kfd_ipc_handles.lock);
> +
> +	dma_buf_put(obj->dmabuf);
> +	kfree(obj);
> +}
> +
> +static struct kfd_ipc_obj *ipc_obj_get(struct kfd_ipc_obj *obj) {
> +	if (kref_get_unless_zero(&obj->ref))
> +		return obj;
> +	return NULL;
> +}
> +
> +void kfd_ipc_obj_put(struct kfd_ipc_obj **obj) {
> +	if (*obj) {
> +		kref_put(&(*obj)->ref, ipc_obj_release);
> +		*obj = NULL;
> +	}
> +}
> +
> +int kfd_ipc_init(void)
> +{
> +	mutex_init(&kfd_ipc_handles.lock);
> +	hash_init(kfd_ipc_handles.handles);
> +	return 0;
> +}
> +
> +static int kfd_import_dmabuf_create_kfd_bo(struct kfd_dev *dev,
> +					   struct kfd_process *p,
> +					   struct dma_buf *dmabuf,
> +					   struct kfd_ipc_obj *ipc_obj,
> +					   uint64_t va_addr, uint64_t *handle,
> +					   uint64_t *mmap_offset)
> +{
> +	int r;
> +	void *mem;
> +	uint64_t size;
> +	int idr_handle;
> +	struct kfd_process_device *pdd = NULL;
> +
> +	if (!handle)
> +		return -EINVAL;
> +
> +	if (!dev)
> +		return -EINVAL;
> +
> +	mutex_lock(&p->mutex);
> +
> +	pdd = kfd_bind_process_to_device(dev, p);
> +	if (IS_ERR(pdd)) {
> +		r = PTR_ERR(pdd);
> +		goto err_unlock;
> +	}
> +
> +	r = amdgpu_amdkfd_gpuvm_import_dmabuf(dev->kgd, dmabuf, ipc_obj,
> +					va_addr, pdd->vm,
> +					(struct kgd_mem **)&mem, &size,
> +					mmap_offset);
> +	if (r)
> +		goto err_unlock;
> +
> +	idr_handle = kfd_process_device_create_obj_handle(pdd, mem);
> +	if (idr_handle < 0) {
> +		r = -EFAULT;
> +		goto err_free;
> +	}
> +
> +	mutex_unlock(&p->mutex);
> +
> +	*handle = MAKE_HANDLE(dev->id, idr_handle);
> +
> +	return 0;
> +
> +err_free:
> +	amdgpu_amdkfd_gpuvm_free_memory_of_gpu(dev->kgd, (struct kgd_mem *)mem,
> +					       NULL);
> +err_unlock:
> +	mutex_unlock(&p->mutex);
> +	return r;
> +}
> +
> +int kfd_ipc_import_dmabuf(struct kfd_dev *dev, struct kfd_process *p,
> +			  int dmabuf_fd, uint64_t va_addr,
> +			  uint64_t *handle, uint64_t *mmap_offset) {
> +	int r;
> +	struct dma_buf *dmabuf = dma_buf_get(dmabuf_fd);
> +
> +	if (!dmabuf)
> +		return -EINVAL;
> +
> +	r = kfd_import_dmabuf_create_kfd_bo(dev, p, dmabuf, NULL,
> +					    va_addr, handle, mmap_offset);
> +	dma_buf_put(dmabuf);
> +	return r;
> +}
> +
> +int kfd_ipc_import_handle(struct kfd_process *p,
> +			  uint32_t *share_handle, uint64_t va_addr,
> +			  uint64_t *handle, uint32_t *gpu_id,
> +			  uint64_t *mmap_offset)
> +{
> +	struct kfd_dev *dev;
> +	int r;
> +	struct kfd_ipc_obj *entry, *found = NULL;
> +
> +	mutex_lock(&kfd_ipc_handles.lock);
> +	/* Convert the user provided handle to hash key and search only in that
> +	 * bucket
> +	 */
> +	hlist_for_each_entry(entry,
> +		&kfd_ipc_handles.handles[HANDLE_TO_KEY(share_handle)], node) {
> +		if (!memcmp(entry->share_handle, share_handle,
> +			    sizeof(entry->share_handle))) {
> +			found = ipc_obj_get(entry);
> +			break;
> +		}
> +	}
> +	mutex_unlock(&kfd_ipc_handles.lock);
> +
> +	if (!found)
> +		return -EINVAL;
> +
> +	pr_debug("Found ipc_dma_buf: %p\n", found->dmabuf);
> +
> +	dev = kfd_device_by_id(found->gpu_id);
> +	if (!dev)
> +		return -ENODEV;
> +
> +	r = kfd_import_dmabuf_create_kfd_bo(dev, p, found->dmabuf, found,
> +					    va_addr, handle, mmap_offset);
> +	if (r)
> +		goto error_unref;
> +
> +	*gpu_id = found->gpu_id;
> +
> +	return r;
> +
> +error_unref:
> +	kfd_ipc_obj_put(&found);
> +	return r;
> +}
> +
> +int kfd_ipc_export_as_handle(struct kfd_dev *dev, struct kfd_process *p,
> +			     uint64_t handle, uint32_t *ipc_handle) {
> +	struct kfd_process_device *pdd = NULL;
> +	struct kfd_ipc_obj *ipc_obj;
> +	struct kgd_mem *mem;
> +	int r;
> +
> +	if (!dev || !ipc_handle)
> +		return -EINVAL;
> +
> +	mutex_lock(&p->mutex);
> +	pdd = kfd_bind_process_to_device(dev, p);
> +	if (IS_ERR(pdd)) {
> +		mutex_unlock(&p->mutex);
> +		pr_err("Failed to get pdd\n");
> +		return PTR_ERR(pdd);
> +	}
> +
> +	mem = kfd_process_device_translate_handle(pdd, GET_IDR_HANDLE(handle));
> +	mutex_unlock(&p->mutex);
> +
> +	if (!mem) {
> +		pr_err("Failed to get bo");
> +		return -EINVAL;
> +	}
> +
> +	r = amdgpu_amdkfd_gpuvm_export_ipc_obj(dev->kgd, pdd->vm, mem,
> +					       &ipc_obj);
> +	if (r)
> +		return r;
> +
> +	ipc_obj->gpu_id = dev->id;
> +	memcpy(ipc_handle, ipc_obj->share_handle,
> +	       sizeof(ipc_obj->share_handle));
> +
> +	return r;
> +}
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_ipc.h b/drivers/gpu/drm/amd/amdkfd/kfd_ipc.h
> new file mode 100644
> index 000000000000..9450a667918e
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_ipc.h
> @@ -0,0 +1,55 @@
> +/*
> + * Copyright 2014 Advanced Micro Devices, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person 
> +obtaining a
> + * copy of this software and associated documentation files (the 
> +"Software"),
> + * to deal in the Software without restriction, including without 
> +limitation
> + * the rights to use, copy, modify, merge, publish, distribute, 
> +sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom 
> +the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be 
> +included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
> +EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
> +MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT 
> +SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, 
> +DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR 
> +OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE 
> +OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + *
> + */
> +
> +#ifndef KFD_IPC_H_
> +#define KFD_IPC_H_
> +
> +#include <linux/types.h>
> +#include <linux/dma-buf.h>
> +
> +/* avoid including kfd_priv.h */
> +struct kfd_dev;
> +struct kfd_process;
> +
> +struct kfd_ipc_obj {
> +	struct hlist_node node;
> +	struct kref ref;
> +	struct dma_buf *dmabuf;
> +	uint32_t share_handle[4];
> +	uint32_t gpu_id;
> +};
> +
> +int kfd_ipc_import_handle(struct kfd_process *p,
> +			  uint32_t *share_handle, uint64_t va_addr,
> +			  uint64_t *handle, uint32_t *gpu_id,
> +			  uint64_t *mmap_offset);
> +int kfd_ipc_import_dmabuf(struct kfd_dev *kfd, struct kfd_process *p,
> +			  int dmabuf_fd, uint64_t va_addr,
> +			  uint64_t *handle, uint64_t *mmap_offset); int 
> +kfd_ipc_export_as_handle(struct kfd_dev *dev, struct kfd_process *p,
> +			     uint64_t handle, uint32_t *ipc_handle);
> +
> +int kfd_ipc_store_insert(struct dma_buf *dmabuf, struct kfd_ipc_obj 
> +**ipc_obj); void kfd_ipc_obj_put(struct kfd_ipc_obj **obj);
> +
> +#endif /* KFD_IPC_H_ */
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_module.c b/drivers/gpu/drm/amd/amdkfd/kfd_module.c
> index f4b7f7e6c40e..0946d5692692 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_module.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_module.c
> @@ -52,6 +52,10 @@ static int kfd_init(void)
>  	if (err < 0)
>  		goto err_topology;
>  
> +	err = kfd_ipc_init();
> +	if (err < 0)
> +		goto err_ipc;
> +
>  	err = kfd_process_create_wq();
>  	if (err < 0)
>  		goto err_create_wq;
> @@ -66,6 +70,7 @@ static int kfd_init(void)
>  	return 0;
>  
>  err_create_wq:
> +err_ipc:
>  	kfd_topology_shutdown();
>  err_topology:
>  	kfd_chardev_exit();
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index 51ba2020732e..1588b2b45a32 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -321,6 +321,8 @@ struct kfd_dev {
>  	spinlock_t smi_lock;
>  };
>  
> +struct kfd_ipc_obj;
> +
>  enum kfd_mempool {
>  	KFD_MEMPOOL_SYSTEM_CACHEABLE = 1,
>  	KFD_MEMPOOL_SYSTEM_WRITECOMBINE = 2,
> @@ -1087,6 +1089,9 @@ int dbgdev_wave_reset_wavefronts(struct kfd_dev *dev, struct kfd_process *p);
>  
>  bool kfd_is_locked(void);
>  
> +/* IPC Support */
> +int kfd_ipc_init(void);
> +
>  /* Compute profile */
>  void kfd_inc_compute_active(struct kfd_dev *dev);  void kfd_dec_compute_active(struct kfd_dev *dev); diff --git a/include/uapi/linux/kfd_ioctl.h b/include/uapi/linux/kfd_ioctl.h index f738c3b53f4e..90ff334e2b5b 100644
> --- a/include/uapi/linux/kfd_ioctl.h
> +++ b/include/uapi/linux/kfd_ioctl.h
> @@ -29,9 +29,10 @@
>  /*
>   * - 1.1 - initial version
>   * - 1.3 - Add SMI events support
> + * - 1.4 - Add IPC export/import
>   */
>  #define KFD_IOCTL_MAJOR_VERSION 1
> -#define KFD_IOCTL_MINOR_VERSION 3
> +#define KFD_IOCTL_MINOR_VERSION 4
>  
>  struct kfd_ioctl_get_version_args {
>  	__u32 major_version;	/* from KFD */
> @@ -464,6 +465,57 @@ enum kfd_mmio_remap {
>  	KFD_MMIO_REMAP_HDP_REG_FLUSH_CNTL = 4,  };
>  
> +/* Export IPC handle
> + *
> + * @handle[in]:         buffer handle of the BO to export
> + * @gpu_id[in]:         GPU ID where @handle was allocated
> + * @share_handle[out]:  share handle that can be used with
> + *                      @kfd_ioctl_ipc_import_handle_args
> + *
> + * @share_handle is a 128 bit random number generated with
> + * @get_random_bytes. This number should be very hard to guess.
> + * Knowledge of the @share_handle implies authorization to access
> + * the shared memory. User mode should treat it like a secret key.
> + * It can be used to import this BO in a different process context
> + * for IPC buffer sharing. The handle will be valid as long as the
> + * underlying BO exists. If the same BO is exported multiple times,
> + * the same handle will be returned.
> + *
> + * Return 0 on success, negative errno on errors.
> + */
> +struct kfd_ioctl_ipc_export_handle_args {
> +	__u64 handle;		/* to KFD */
> +	__u32 share_handle[4];	/* from KFD */
> +	__u32 gpu_id;		/* to KFD */
> +	__u32 pad;
> +};
> +
> +/* Import IPC handle
> + *
> + * @share_handle[in]:  share handle from @kfd_ioctl_ipc_export_handle_args
> + * @va_addr[in]:       virtual address at which to import the BO
> + * @handle[out]:       buffer handle of the imported BO
> + * @gpu_id[out]:       device in which the shared BO was created
> + * @mmap_offset[out]:  mmap offset for CPU-mapping the BO
> + *
> + * @handle represents a new reference to the shared BO. This reference
> + * must be released with kfd_ioctl_free_memory_of_gpu_args.
> + *
> + * The BO can be mapped for GPU access with @kfd_ioctl_map_memory_to_gpu_args.
> + *
> + * It can be mapped for CPU access using the @mmap_offset.
> + *
> + * Return 0 on success, negative errno on errors.
> + */
> +struct kfd_ioctl_ipc_import_handle_args {
> +	__u64 handle;		/* from KFD */
> +	__u64 va_addr;		/* to KFD */
> +	__u64 mmap_offset;	/* from KFD */
> +	__u32 share_handle[4];	/* to KFD */
> +	__u32 gpu_id;		/* from KFD */
> +	__u32 pad;
> +};
> +
>  #define AMDKFD_IOCTL_BASE 'K'
>  #define AMDKFD_IO(nr)			_IO(AMDKFD_IOCTL_BASE, nr)
>  #define AMDKFD_IOR(nr, type)		_IOR(AMDKFD_IOCTL_BASE, nr, type)
> @@ -564,7 +616,13 @@ enum kfd_mmio_remap {
>  #define AMDKFD_IOC_SMI_EVENTS			\
>  		AMDKFD_IOWR(0x1F, struct kfd_ioctl_smi_events_args)
>  
> +#define AMDKFD_IOC_IPC_IMPORT_HANDLE                                    \
> +		AMDKFD_IOWR(0x20, struct kfd_ioctl_ipc_import_handle_args)
> +
> +#define AMDKFD_IOC_IPC_EXPORT_HANDLE		\
> +		AMDKFD_IOWR(0x21, struct kfd_ioctl_ipc_export_handle_args)
> +
>  #define AMDKFD_COMMAND_START		0x01
> -#define AMDKFD_COMMAND_END		0x20
> +#define AMDKFD_COMMAND_END		0x22
>  
>  #endif
> --
> 2.27.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel