amd-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/9] amdkfd: Implement kfd multiple contexts
@ 2025-07-25  2:43 Zhu Lingshan
  2025-07-25  2:43 ` [PATCH 1/9] amdkfd: enlarge the hashtable of kfd_process Zhu Lingshan
                   ` (9 more replies)
  0 siblings, 10 replies; 15+ messages in thread
From: Zhu Lingshan @ 2025-07-25  2:43 UTC (permalink / raw)
  To: alexander.deucher, felix.kuehling; +Cc: ray.huang, amd-gfx, Zhu Lingshan

Currently kfd manages kfd_process in a one context (kfd_process)
per program manner, thus each user space program
only onws one kfd context (kfd_process).

This model works fine for most of the programs, but imperfect
for a hypervisor like QEMU. Because all programs in the guest
user space share the same only one kfd context, which is
problematic, including but not limited to:

As illustrated in Figure 1, all guest user space programs share the same fd of /dev/kfd
and the same kfd_process, and the same PASID leading to the same
GPU_VM address space. Therefore the IOVA range of each
guest user space programs are not isolated,
they can attack each other through GPU DMA.


 +----------------------------------------------------------------------------------+
 |                                                                                  |
 |  +-----------+      +-----------+      +------------+      +------------+        |
 |  |           |      |           |      |            |      |            |        |
 |  | Program 1 |      | Program 2 |      | Program 3  |      | Program N  |        |
 |  |           |      |           |      |            |      |            |        |
 |  +----+------+      +--------+--+      +--+---------+      +-----+------+        |
 |       |                      |            |                      |               |
 |       |                      |            |                      |        Guest  |
 |       |                      |            |                      |               |
 +-------+----------------------+------------+----------------------+---------------+
         |                      |            |                      |
         |                      |            |                      |
         |                      |            |                      |
         |                      |            |                      |
         |                   +--+------------+---+                  |
         |                   | file descriptor   |                  |
         +-------------------+ of /dev/kfd       +------------------+
                             | opened by QEMU    |
                             |                   |
                             +---------+---------+                   User Space
                                       |                             QEMU
                                       |
---------------------------------------+-----------------------------------------------------
                                       |                             Kernel Space
                                       |                             KFD Module
                                       |
                              +--------+--------+
                              |                 |
                              |   kfd_process   |<------------------The only one KFD context
                              |                 |
                              +--------+--------+
                                       |
                              +--------+--------+
                              |     PASID       |
                              +--------+--------+
                                       |
                              +--------+--------+
                              |      GPU_VM     |
                              +-----------------+

                                 Fiture 1


This series implements a multiple contexts solution:
- Allow each program to create and hold multiple contexts (kfd processes)
- Each context has its own fd of /dev/kfd and an exclusive kfd_process,
  which is a secondary kfd context. So that PASID/GPU VM isolates their IOVA address spaces.
  Therefore, they can not attack each other through GPU DMA.

The design is illustrated in Figure 2 below:

   +---------------------------------------------------------------------------------------------------------+                    
   |                                                                                                         |                    
   |                                                                                                         |                    
   |                                                                                                         |                    
   |       +----------------------------------------------------------------------------------+              |                    
   |       |                                                                                  |              |                    
   |       | +-----------+      +-----------+     +-----------+    +-----------+              |              |                    
   |       | |           |      |           |     |           |    |           |              |              |                    
   |       | | Program 1 |      | Program 2 |     | Program 3 |    | Program N |              |              |                    
   |       | |           |      |           |     |           |    |           |              |              |                    
   |       | +-----+-----+      +-----+-----+     +-----+-----+    +-----+-----+              |              |                    
   |       |       |                  |                 |                |                    |              |                    
   |       |       |                  |                 |                |        Guest       |              |                    
   |       |       |                  |                 |                |                    |              |                    
   |       +-------+------------------+-----------------+----------------+--------------------+              |                    
   |               |                  |                 |                |                            QEMU   |                    
   |               |                  |                 |                |                                   |                    
   +---------------+------------------+-----------------+----------------+--------------------------+--------+                    
                   |                  |                 |                |                          |                             
                   |                  |                 |                |                          |                             
                   |                  |                 |                |                          |                             
               +---+----+         +---+----+        +---+----+       +---+----+                 +---+-----+                       
               |        |         |        |        |        |       |        |                 | Primary |                       
               |  FD 1  |         |  FD 2  |        |  FD 3  |       |  FD 4  |                 |   FD    |                       
               |        |         |        |        |        |       |        |                 |         |                       
               +---+----+         +---+----+        +---+----+       +----+---+                 +----+----+                       
                   |                  |                 |                 |                          |             User Space     
                   |                  |                 |                 |                          |                            
-------------------+------------------+-----------------+-----------------+--------------------------+----------------------------
                   |                  |                 |                 |                          |             Kernel SPace   
                   |                  |                 |                 |                          |                            
                   |                  |                 |                 |                          |                            
   +--------------------------------------------------------------------------------------------------------------------------+   
   |        +------+------+    +------+------+   +------+------+   +------+------+            +------+------+                 |   
   |        | Secondary   |    | Secondary   |   | Secondary   |   | Secondary   |            |  Primary    |   KFD Module    |   
   |        |kfd_process 1|    |kfd_process 2|   |kfd_process 3|   |kfd_process 4|            | kfd_process |                 |   
   |        |             |    |             |   |             |   |             |            |             |                 |   
   |        +------+------+    +------+------+   +------+------+   +------+------+            +------+------+                 |   
   |               |                  |                 |                 |                          |                        |   
   |        +------+------+    +------+------+   +------+------+   +------+------+            +------+------+                 |   
   |        |   PASID     |    |   PASID     |   |   PASID     |   |   PASID     |            |   PASID     |                 |   
   |        +------+------+    +------+------+   +------+------+   +------+------+            +------+------+                 |   
   |               |                  |                 |                 |                          |                        |   
   |               |                  |                 |                 |                          |                        |   
   |        +------+------+    +------+------+   +------+------+   +------+------+            +------+------+                 |   
   |        |   GPU_VM    |    |   GPU_VM    |   |   GPU_VM    |   |   GPU_VM    |            |   GPU_VM    |                 |   
   |        +-------------+    +-------------+   +-------------+   +-------------+            +-------------+                 |   
   |                                                                                                                          |   
   +--------------------------------------------------------------------------------------------------------------------------+   
                                                                                                                                  
                                                  Figure 2  

Zhu Lingshan (9):
  amdkfd: enlarge the hashtable of kfd_process
  amdkfd: mark the first kfd_process as the primary one
  amdkfd: find_process_by_mm always return the primary context
  amdkfd: Introduce kfd_create_process_sysfs as a separate function
  amdkfd: destroy kfd secondary contexts through fd close
  amdkfd: process svm ioctl only on the primary kfd process
  amdkfd: process USERPTR allocation only on the primary kfd process
  amdkfd: identify a secondary kfd process by its id
  amdkfd: introduce new ioctl AMDKFD_IOC_CREATE_PROCESS

 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c |  62 ++++++-
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h    |  14 +-
 drivers/gpu/drm/amd/amdkfd/kfd_process.c | 204 +++++++++++++++++------
 include/uapi/linux/kfd_ioctl.h           |   8 +-
 4 files changed, 231 insertions(+), 57 deletions(-)

-- 
2.47.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH 1/9] amdkfd: enlarge the hashtable of kfd_process
  2025-07-25  2:43 [PATCH 0/9] amdkfd: Implement kfd multiple contexts Zhu Lingshan
@ 2025-07-25  2:43 ` Zhu Lingshan
  2025-07-25  2:43 ` [PATCH 2/9] amdkfd: mark the first kfd_process as the primary one Zhu Lingshan
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Zhu Lingshan @ 2025-07-25  2:43 UTC (permalink / raw)
  To: alexander.deucher, felix.kuehling; +Cc: ray.huang, amd-gfx, Zhu Lingshan

This commit enlarges the hashtable size of
kfd_process to 256, because of the multiple
contexts feature allowing each application
create multiple kfd_processes

Signed-off-by: Zhu Lingshan <lingshan.zhu@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index d221c58dccc3..38a20ba61e24 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -1008,7 +1008,7 @@ struct kfd_process {
 	bool gpu_page_fault;
 };
 
-#define KFD_PROCESS_TABLE_SIZE 5 /* bits: 32 entries */
+#define KFD_PROCESS_TABLE_SIZE 8 /* bits: 256 entries */
 extern DECLARE_HASHTABLE(kfd_processes_table, KFD_PROCESS_TABLE_SIZE);
 extern struct srcu_struct kfd_processes_srcu;
 
-- 
2.47.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/9] amdkfd: mark the first kfd_process as the primary one
  2025-07-25  2:43 [PATCH 0/9] amdkfd: Implement kfd multiple contexts Zhu Lingshan
  2025-07-25  2:43 ` [PATCH 1/9] amdkfd: enlarge the hashtable of kfd_process Zhu Lingshan
@ 2025-07-25  2:43 ` Zhu Lingshan
  2025-07-25  2:43 ` [PATCH 3/9] amdkfd: find_process_by_mm always return the primary context Zhu Lingshan
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Zhu Lingshan @ 2025-07-25  2:43 UTC (permalink / raw)
  To: alexander.deucher, felix.kuehling; +Cc: ray.huang, amd-gfx, Zhu Lingshan

The first kfd_process is created through open(),
this commit marks it as the primary kfd_process.

Only the primary process should register the mmu_notifier.

Signed-off-by: Zhu Lingshan <lingshan.zhu@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h    |  3 +++
 drivers/gpu/drm/amd/amdkfd/kfd_process.c | 20 ++++++++++++--------
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 38a20ba61e24..8149ce0639c0 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -1006,6 +1006,9 @@ struct kfd_process {
 
 	/* if gpu page fault sent to KFD */
 	bool gpu_page_fault;
+
+	/* indicating whether this is a primary kfd_process */
+	bool primary;
 };
 
 #define KFD_PROCESS_TABLE_SIZE 8 /* bits: 256 entries */
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 722ac1662bdc..955ca8725bc5 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -68,7 +68,7 @@ static struct workqueue_struct *kfd_restore_wq;
 static struct kfd_process *find_process(const struct task_struct *thread,
 					bool ref);
 static void kfd_process_ref_release(struct kref *ref);
-static struct kfd_process *create_process(const struct task_struct *thread);
+static struct kfd_process *create_process(const struct task_struct *thread, bool primary);
 
 static void evict_process_worker(struct work_struct *work);
 static void restore_process_worker(struct work_struct *work);
@@ -867,7 +867,7 @@ struct kfd_process *kfd_create_process(struct task_struct *thread)
 	if (process) {
 		pr_debug("Process already found\n");
 	} else {
-		process = create_process(thread);
+		process = create_process(thread, true);
 		if (IS_ERR(process))
 			goto out;
 
@@ -1510,7 +1510,7 @@ void kfd_process_set_trap_debug_flag(struct qcm_process_device *qpd,
  * On return the kfd_process is fully operational and will be freed when the
  * mm is released
  */
-static struct kfd_process *create_process(const struct task_struct *thread)
+static struct kfd_process *create_process(const struct task_struct *thread, bool primary)
 {
 	struct kfd_process *process;
 	struct mmu_notifier *mn;
@@ -1526,6 +1526,8 @@ static struct kfd_process *create_process(const struct task_struct *thread)
 	process->lead_thread = thread->group_leader;
 	process->n_pdds = 0;
 	process->queues_paused = false;
+	process->primary = primary;
+
 	INIT_DELAYED_WORK(&process->eviction_work, evict_process_worker);
 	INIT_DELAYED_WORK(&process->restore_work, restore_process_worker);
 	process->last_restore_timestamp = get_jiffies_64();
@@ -1569,12 +1571,14 @@ static struct kfd_process *create_process(const struct task_struct *thread)
 	 * After this point, mmu_notifier_put will trigger the cleanup by
 	 * dropping the last process reference in the free_notifier.
 	 */
-	mn = mmu_notifier_get(&kfd_process_mmu_notifier_ops, process->mm);
-	if (IS_ERR(mn)) {
-		err = PTR_ERR(mn);
-		goto err_register_notifier;
+	if (primary) {
+		mn = mmu_notifier_get(&kfd_process_mmu_notifier_ops, process->mm);
+		if (IS_ERR(mn)) {
+			err = PTR_ERR(mn);
+			goto err_register_notifier;
+		}
+		BUG_ON(mn != &process->mmu_notifier);
 	}
-	BUG_ON(mn != &process->mmu_notifier);
 
 	kfd_unref_process(process);
 	get_task_struct(process->lead_thread);
-- 
2.47.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/9] amdkfd: find_process_by_mm always return the primary context
  2025-07-25  2:43 [PATCH 0/9] amdkfd: Implement kfd multiple contexts Zhu Lingshan
  2025-07-25  2:43 ` [PATCH 1/9] amdkfd: enlarge the hashtable of kfd_process Zhu Lingshan
  2025-07-25  2:43 ` [PATCH 2/9] amdkfd: mark the first kfd_process as the primary one Zhu Lingshan
@ 2025-07-25  2:43 ` Zhu Lingshan
  2025-07-25  2:43 ` [PATCH 4/9] amdkfd: Introduce kfd_create_process_sysfs as a separate function Zhu Lingshan
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Zhu Lingshan @ 2025-07-25  2:43 UTC (permalink / raw)
  To: alexander.deucher, felix.kuehling; +Cc: ray.huang, amd-gfx, Zhu Lingshan

Up until this commit, the kfd multiple contexts feature has
not been fully implemented in mainline kernel yet.

For backawrd compatibility, not break existing use cases,
this commit changes function find_process_by_mm, let it
always return the primary kfd_process.

Signed-off-by: Zhu Lingshan <lingshan.zhu@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_process.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 955ca8725bc5..3f4ac9122203 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -935,7 +935,7 @@ static struct kfd_process *find_process_by_mm(const struct mm_struct *mm)
 
 	hash_for_each_possible_rcu(kfd_processes_table, process,
 					kfd_processes, (uintptr_t)mm)
-		if (process->mm == mm)
+		if (process->mm == mm && process->primary)
 			return process;
 
 	return NULL;
-- 
2.47.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 4/9] amdkfd: Introduce kfd_create_process_sysfs as a separate function
  2025-07-25  2:43 [PATCH 0/9] amdkfd: Implement kfd multiple contexts Zhu Lingshan
                   ` (2 preceding siblings ...)
  2025-07-25  2:43 ` [PATCH 3/9] amdkfd: find_process_by_mm always return the primary context Zhu Lingshan
@ 2025-07-25  2:43 ` Zhu Lingshan
  2025-07-25  2:43 ` [PATCH 5/9] amdkfd: destroy kfd secondary contexts through fd close Zhu Lingshan
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Zhu Lingshan @ 2025-07-25  2:43 UTC (permalink / raw)
  To: alexander.deucher, felix.kuehling; +Cc: ray.huang, amd-gfx, Zhu Lingshan

KFD creates sysfs entries for a kfd_process in
function kfd_create_process when creating it.

This commit extracts the code creating sysfs
entries to a separate function because it
would be invoked in other code path like
creating secondary kfd contexts (kfd_process).

Signed-off-by: Zhu Lingshan <lingshan.zhu@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h    |  1 +
 drivers/gpu/drm/amd/amdkfd/kfd_process.c | 66 +++++++++++++++---------
 2 files changed, 42 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 8149ce0639c0..bf4a8972e3bf 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -1040,6 +1040,7 @@ int kfd_process_create_wq(void);
 void kfd_process_destroy_wq(void);
 void kfd_cleanup_processes(void);
 struct kfd_process *kfd_create_process(struct task_struct *thread);
+int kfd_create_process_sysfs(struct kfd_process *process);
 struct kfd_process *kfd_get_process(const struct task_struct *task);
 struct kfd_process *kfd_lookup_process_by_pasid(u32 pasid,
 						 struct kfd_process_device **pdd);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 3f4ac9122203..d8535cd47850 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -825,6 +825,44 @@ static void kfd_process_device_destroy_ib_mem(struct kfd_process_device *pdd)
 	kfd_process_free_gpuvm(qpd->ib_mem, pdd, &qpd->ib_kaddr);
 }
 
+int kfd_create_process_sysfs(struct kfd_process *process)
+{
+	int ret;
+
+	if (process->kobj) {
+		pr_warn("kobject already exsists for the kfd_process\n");
+		return -EINVAL;
+	}
+
+	process->kobj = kfd_alloc_struct(process->kobj);
+	if (!process->kobj) {
+		pr_warn("Creating procfs kobject failed");
+		return -ENOMEM;
+	}
+	ret = kobject_init_and_add(process->kobj, &procfs_type,
+				   procfs.kobj, "%d",
+				   (int)process->lead_thread->pid);
+	if (ret) {
+		pr_warn("Creating procfs pid directory failed");
+		kobject_put(process->kobj);
+		return ret;
+	}
+
+	kfd_sysfs_create_file(process->kobj, &process->attr_pasid,
+			      "pasid");
+
+	process->kobj_queues = kobject_create_and_add("queues",
+						process->kobj);
+	if (!process->kobj_queues)
+		pr_warn("Creating KFD proc/queues folder failed");
+
+	kfd_procfs_add_sysfs_stats(process);
+	kfd_procfs_add_sysfs_files(process);
+	kfd_procfs_add_sysfs_counters(process);
+
+	return 0;
+}
+
 struct kfd_process *kfd_create_process(struct task_struct *thread)
 {
 	struct kfd_process *process;
@@ -874,31 +912,9 @@ struct kfd_process *kfd_create_process(struct task_struct *thread)
 		if (!procfs.kobj)
 			goto out;
 
-		process->kobj = kfd_alloc_struct(process->kobj);
-		if (!process->kobj) {
-			pr_warn("Creating procfs kobject failed");
-			goto out;
-		}
-		ret = kobject_init_and_add(process->kobj, &procfs_type,
-					   procfs.kobj, "%d",
-					   (int)process->lead_thread->pid);
-		if (ret) {
-			pr_warn("Creating procfs pid directory failed");
-			kobject_put(process->kobj);
-			goto out;
-		}
-
-		kfd_sysfs_create_file(process->kobj, &process->attr_pasid,
-				      "pasid");
-
-		process->kobj_queues = kobject_create_and_add("queues",
-							process->kobj);
-		if (!process->kobj_queues)
-			pr_warn("Creating KFD proc/queues folder failed");
-
-		kfd_procfs_add_sysfs_stats(process);
-		kfd_procfs_add_sysfs_files(process);
-		kfd_procfs_add_sysfs_counters(process);
+		ret = kfd_create_process_sysfs(process);
+		if (ret)
+			pr_warn("Failed to create sysfs entry for the kfd_process");
 
 		kfd_debugfs_add_process(process);
 
-- 
2.47.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 5/9] amdkfd: destroy kfd secondary contexts through fd close
  2025-07-25  2:43 [PATCH 0/9] amdkfd: Implement kfd multiple contexts Zhu Lingshan
                   ` (3 preceding siblings ...)
  2025-07-25  2:43 ` [PATCH 4/9] amdkfd: Introduce kfd_create_process_sysfs as a separate function Zhu Lingshan
@ 2025-07-25  2:43 ` Zhu Lingshan
  2025-07-25  2:43 ` [PATCH 6/9] amdkfd: process svm ioctl only on the primary kfd process Zhu Lingshan
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Zhu Lingshan @ 2025-07-25  2:43 UTC (permalink / raw)
  To: alexander.deucher, felix.kuehling; +Cc: ray.huang, amd-gfx, Zhu Lingshan

Life cycle of a KFD secondary context(kfd_process) is tied
to the opened file. Therefore this commit destroy a kfd
secondary context when close the fd it belonging to.

This commit extracts the code removing the kfd_process
from the kfd_process_table to a separate function and
call it in kfd_process_notifier_release_internal unconditionally.

Signed-off-by: Zhu Lingshan <lingshan.zhu@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c |  9 ++++--
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h    |  1 +
 drivers/gpu/drm/amd/amdkfd/kfd_process.c | 41 +++++++++++++-----------
 3 files changed, 31 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index a2149afa5803..ec9bc359d5be 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -165,8 +165,13 @@ static int kfd_release(struct inode *inode, struct file *filep)
 {
 	struct kfd_process *process = filep->private_data;
 
-	if (process)
-		kfd_unref_process(process);
+	if (!process)
+		return 0;
+
+	if (!process->primary)
+		kfd_process_notifier_release_internal(process);
+
+	kfd_unref_process(process);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index bf4a8972e3bf..de701d72aa5c 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -1082,6 +1082,7 @@ bool kfd_process_xnack_mode(struct kfd_process *p, bool supported);
 
 int kfd_reserved_mem_mmap(struct kfd_node *dev, struct kfd_process *process,
 			  struct vm_area_struct *vma);
+void kfd_process_notifier_release_internal(struct kfd_process *p);
 
 /* KFD process API for creating and translating handles */
 int kfd_process_device_create_obj_handle(struct kfd_process_device *pdd,
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index d8535cd47850..440fde75d1e4 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -1233,10 +1233,30 @@ static void kfd_process_free_notifier(struct mmu_notifier *mn)
 	kfd_unref_process(container_of(mn, struct kfd_process, mmu_notifier));
 }
 
-static void kfd_process_notifier_release_internal(struct kfd_process *p)
+static void kfd_process_table_remove(struct kfd_process *p)
+{
+	mutex_lock(&kfd_processes_mutex);
+	/*
+	 * Do early return if table is empty.
+	 *
+	 * This could potentially happen if this function is called concurrently
+	 * by mmu_notifier and by kfd_cleanup_pocesses.
+	 *
+	 */
+	if (hash_empty(kfd_processes_table)) {
+		mutex_unlock(&kfd_processes_mutex);
+		return;
+	}
+	hash_del_rcu(&p->kfd_processes);
+	mutex_unlock(&kfd_processes_mutex);
+	synchronize_srcu(&kfd_processes_srcu);
+}
+
+void kfd_process_notifier_release_internal(struct kfd_process *p)
 {
 	int i;
 
+	kfd_process_table_remove(p);
 	cancel_delayed_work_sync(&p->eviction_work);
 	cancel_delayed_work_sync(&p->restore_work);
 
@@ -1270,7 +1290,8 @@ static void kfd_process_notifier_release_internal(struct kfd_process *p)
 		srcu_read_unlock(&kfd_processes_srcu, idx);
 	}
 
-	mmu_notifier_put(&p->mmu_notifier);
+	if (p->primary)
+		mmu_notifier_put(&p->mmu_notifier);
 }
 
 static void kfd_process_notifier_release(struct mmu_notifier *mn,
@@ -1286,22 +1307,6 @@ static void kfd_process_notifier_release(struct mmu_notifier *mn,
 	if (WARN_ON(p->mm != mm))
 		return;
 
-	mutex_lock(&kfd_processes_mutex);
-	/*
-	 * Do early return if table is empty.
-	 *
-	 * This could potentially happen if this function is called concurrently
-	 * by mmu_notifier and by kfd_cleanup_pocesses.
-	 *
-	 */
-	if (hash_empty(kfd_processes_table)) {
-		mutex_unlock(&kfd_processes_mutex);
-		return;
-	}
-	hash_del_rcu(&p->kfd_processes);
-	mutex_unlock(&kfd_processes_mutex);
-	synchronize_srcu(&kfd_processes_srcu);
-
 	kfd_process_notifier_release_internal(p);
 }
 
-- 
2.47.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 6/9] amdkfd: process svm ioctl only on the primary kfd process
  2025-07-25  2:43 [PATCH 0/9] amdkfd: Implement kfd multiple contexts Zhu Lingshan
                   ` (4 preceding siblings ...)
  2025-07-25  2:43 ` [PATCH 5/9] amdkfd: destroy kfd secondary contexts through fd close Zhu Lingshan
@ 2025-07-25  2:43 ` Zhu Lingshan
  2025-07-25  2:43 ` [PATCH 7/9] amdkfd: process USERPTR allocation " Zhu Lingshan
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Zhu Lingshan @ 2025-07-25  2:43 UTC (permalink / raw)
  To: alexander.deucher, felix.kuehling; +Cc: ray.huang, amd-gfx, Zhu Lingshan

svm ioctl should only be processed on the primary
kfd process because only the lifecycle of the
primary kfd process is tied to the  user space
applicaiton.

Another reason is in virtualization the hypervisor owns
the primary kfd process as a privileged one.

Signed-off-by: Zhu Lingshan <lingshan.zhu@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index ec9bc359d5be..b1ab1565a4cd 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1720,6 +1720,12 @@ static int kfd_ioctl_svm(struct file *filep, struct kfd_process *p, void *data)
 	struct kfd_ioctl_svm_args *args = data;
 	int r = 0;
 
+	if (!p->primary) {
+		pr_debug("SVM ioctl not supported on non-primary kfd process\n");
+
+		return -EOPNOTSUPP;
+	}
+
 	pr_debug("start 0x%llx size 0x%llx op 0x%x nattr 0x%x\n",
 		 args->start_addr, args->size, args->op, args->nattr);
 
-- 
2.47.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 7/9] amdkfd: process USERPTR allocation only on the primary kfd process
  2025-07-25  2:43 [PATCH 0/9] amdkfd: Implement kfd multiple contexts Zhu Lingshan
                   ` (5 preceding siblings ...)
  2025-07-25  2:43 ` [PATCH 6/9] amdkfd: process svm ioctl only on the primary kfd process Zhu Lingshan
@ 2025-07-25  2:43 ` Zhu Lingshan
  2025-07-25  2:43 ` [PATCH 8/9] amdkfd: identify a secondary kfd process by its id Zhu Lingshan
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 15+ messages in thread
From: Zhu Lingshan @ 2025-07-25  2:43 UTC (permalink / raw)
  To: alexander.deucher, felix.kuehling; +Cc: ray.huang, amd-gfx, Zhu Lingshan

The lifecycle of the primary kfd process is tied to
the user space program, all secondary kfd process
would be destroyed when fd close. Thus only the primary
kfd process should process USERPTR memory allocation.

Signed-off-by: Zhu Lingshan <lingshan.zhu@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index b1ab1565a4cd..5b22e1c47b2e 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -1069,6 +1069,12 @@ static int kfd_ioctl_alloc_memory_of_gpu(struct file *filep,
 	if (args->size == 0)
 		return -EINVAL;
 
+	if (!p->primary && (flags & KFD_IOC_ALLOC_MEM_FLAGS_USERPTR)) {
+		pr_debug("USERPTR is not supported on non-primary kfd_process\n");
+
+		return -EOPNOTSUPP;
+	}
+
 #if IS_ENABLED(CONFIG_HSA_AMD_SVM)
 	/* Flush pending deferred work to avoid racing with deferred actions
 	 * from previous memory map changes (e.g. munmap).
-- 
2.47.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 8/9] amdkfd: identify a secondary kfd process by its id
  2025-07-25  2:43 [PATCH 0/9] amdkfd: Implement kfd multiple contexts Zhu Lingshan
                   ` (6 preceding siblings ...)
  2025-07-25  2:43 ` [PATCH 7/9] amdkfd: process USERPTR allocation " Zhu Lingshan
@ 2025-07-25  2:43 ` Zhu Lingshan
  2025-07-28 19:48   ` Felix Kuehling
  2025-07-25  2:43 ` [PATCH 9/9] amdkfd: introduce new ioctl AMDKFD_IOC_CREATE_PROCESS Zhu Lingshan
  2025-07-28 19:59 ` [PATCH 0/9] amdkfd: Implement kfd multiple contexts Felix Kuehling
  9 siblings, 1 reply; 15+ messages in thread
From: Zhu Lingshan @ 2025-07-25  2:43 UTC (permalink / raw)
  To: alexander.deucher, felix.kuehling; +Cc: ray.huang, amd-gfx, Zhu Lingshan

This commit introduces a new id field for
struct kfd process, which helps identify
a kfd process among multiple contexts that
all belong to a single user space program.

The sysfs entry of a secondary kfd process
is placed under the sysfs entry folder of
its primary kfd process.

The naming format of the sysfs entry of a secondary
kfd process is "context_%u" where %u is the process id.

Signed-off-by: Zhu Lingshan <lingshan.zhu@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h    |  6 ++
 drivers/gpu/drm/amd/amdkfd/kfd_process.c | 82 +++++++++++++++++++++++-
 2 files changed, 85 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index de701d72aa5c..a6e12c705734 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -995,6 +995,9 @@ struct kfd_process {
 	/* Tracks debug per-vmid request for debug flags */
 	u32 dbg_flags;
 
+	/* kfd process id */
+	u16 id;
+
 	atomic_t poison;
 	/* Queues are in paused stated because we are in the process of doing a CRIU checkpoint */
 	bool queues_paused;
@@ -1009,6 +1012,9 @@ struct kfd_process {
 
 	/* indicating whether this is a primary kfd_process */
 	bool primary;
+
+	/* The primary kfd_process allocating IDs for its secondary kfd_process, 0 for primary kfd_process */
+	struct ida id_table;
 };
 
 #define KFD_PROCESS_TABLE_SIZE 8 /* bits: 256 entries */
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index 440fde75d1e4..e1ba9015bb83 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -54,6 +54,9 @@ DEFINE_MUTEX(kfd_processes_mutex);
 
 DEFINE_SRCU(kfd_processes_srcu);
 
+#define KFD_PROCESS_ID_MIN 1
+#define KFD_PROCESS_ID_WIDTH 16
+
 /* For process termination handling */
 static struct workqueue_struct *kfd_process_wq;
 
@@ -827,6 +830,7 @@ static void kfd_process_device_destroy_ib_mem(struct kfd_process_device *pdd)
 
 int kfd_create_process_sysfs(struct kfd_process *process)
 {
+	struct kfd_process *primary_process;
 	int ret;
 
 	if (process->kobj) {
@@ -839,9 +843,22 @@ int kfd_create_process_sysfs(struct kfd_process *process)
 		pr_warn("Creating procfs kobject failed");
 		return -ENOMEM;
 	}
-	ret = kobject_init_and_add(process->kobj, &procfs_type,
-				   procfs.kobj, "%d",
-				   (int)process->lead_thread->pid);
+
+	if (process->primary)
+		ret = kobject_init_and_add(process->kobj, &procfs_type,
+					   procfs.kobj, "%d",
+					   (int)process->lead_thread->pid);
+	else {
+		primary_process = kfd_lookup_process_by_mm(process->lead_thread->mm);
+		if (!primary_process)
+			return -EFAULT;
+
+		ret = kobject_init_and_add(process->kobj, &procfs_type,
+					   primary_process->kobj, "context_%u",
+					   process->id);
+		kfd_unref_process(primary_process);
+	}
+
 	if (ret) {
 		pr_warn("Creating procfs pid directory failed");
 		kobject_put(process->kobj);
@@ -863,6 +880,51 @@ int kfd_create_process_sysfs(struct kfd_process *process)
 	return 0;
 }
 
+static int kfd_process_alloc_id(struct kfd_process *process)
+{
+	u16 ret;
+	struct kfd_process *primary_process;
+
+	if (process->primary) {
+		process->id = 0;
+
+		return 0;
+	}
+
+	primary_process = kfd_lookup_process_by_mm(process->lead_thread->mm);
+	if (!primary_process)
+		return -EFAULT;
+
+	ret = ida_alloc_range(&primary_process->id_table, KFD_PROCESS_ID_MIN,
+	     1 << (KFD_PROCESS_ID_WIDTH - 1), GFP_KERNEL);
+	if (ret < 0)
+		return ret;
+
+	process->id = ret;
+
+	kfd_unref_process(primary_process);
+
+	return 0;
+}
+
+static void kfd_process_free_id(struct kfd_process *process)
+{
+	struct kfd_process *primary_process;
+
+	if (process->primary)
+		return;
+
+	primary_process = kfd_lookup_process_by_mm(process->lead_thread->mm);
+	if (!primary_process)
+		return;
+
+	ida_free(&primary_process->id_table, process->id);
+
+	kfd_unref_process(primary_process);
+
+	return;
+}
+
 struct kfd_process *kfd_create_process(struct task_struct *thread)
 {
 	struct kfd_process *process;
@@ -1193,6 +1255,11 @@ static void kfd_process_wq_release(struct work_struct *work)
 	if (ef)
 		dma_fence_signal(ef);
 
+	if (!p->primary)
+		kfd_process_free_id(p);
+	else
+		ida_destroy(&p->id_table);
+
 	kfd_process_remove_sysfs(p);
 	kfd_debugfs_remove_process(p);
 
@@ -1549,6 +1616,12 @@ static struct kfd_process *create_process(const struct task_struct *thread, bool
 	process->queues_paused = false;
 	process->primary = primary;
 
+	err = kfd_process_alloc_id(process);
+	if (err) {
+		pr_err("Creating kfd process: failed to alloc an id\n");
+		goto err_alloc_process;
+	}
+
 	INIT_DELAYED_WORK(&process->eviction_work, evict_process_worker);
 	INIT_DELAYED_WORK(&process->restore_work, restore_process_worker);
 	process->last_restore_timestamp = get_jiffies_64();
@@ -1599,6 +1672,8 @@ static struct kfd_process *create_process(const struct task_struct *thread, bool
 			goto err_register_notifier;
 		}
 		BUG_ON(mn != &process->mmu_notifier);
+
+		ida_init(&process->id_table);
 	}
 
 	kfd_unref_process(process);
@@ -1619,6 +1694,7 @@ static struct kfd_process *create_process(const struct task_struct *thread, bool
 err_process_pqm_init:
 	kfd_event_free_process(process);
 err_event_init:
+	kfd_process_free_id(process);
 	mutex_destroy(&process->mutex);
 	kfree(process);
 err_alloc_process:
-- 
2.47.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 9/9] amdkfd: introduce new ioctl AMDKFD_IOC_CREATE_PROCESS
  2025-07-25  2:43 [PATCH 0/9] amdkfd: Implement kfd multiple contexts Zhu Lingshan
                   ` (7 preceding siblings ...)
  2025-07-25  2:43 ` [PATCH 8/9] amdkfd: identify a secondary kfd process by its id Zhu Lingshan
@ 2025-07-25  2:43 ` Zhu Lingshan
  2025-07-28 19:59   ` Felix Kuehling
  2025-07-28 19:59 ` [PATCH 0/9] amdkfd: Implement kfd multiple contexts Felix Kuehling
  9 siblings, 1 reply; 15+ messages in thread
From: Zhu Lingshan @ 2025-07-25  2:43 UTC (permalink / raw)
  To: alexander.deucher, felix.kuehling; +Cc: ray.huang, amd-gfx, Zhu Lingshan

This commit implemetns a new ioctl AMDKFD_IOC_CREATE_PROCESS
that creates a new secondary kfd_progress on the FD.

To keep backward compatibility, userspace programs need to invoke
this ioctl explicitly on a FD to create a secondary
kfd_process which replacing its primary kfd_process.

This commit bumps ioctl minor version.

Signed-off-by: Zhu Lingshan <lingshan.zhu@amd.com>
---
 drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 41 ++++++++++++++++++++++++
 drivers/gpu/drm/amd/amdkfd/kfd_priv.h    |  1 +
 drivers/gpu/drm/amd/amdkfd/kfd_process.c |  3 +-
 include/uapi/linux/kfd_ioctl.h           |  8 +++--
 4 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
index 5b22e1c47b2e..f9c43ff8a89f 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
@@ -3136,6 +3136,44 @@ static int kfd_ioctl_set_debug_trap(struct file *filep, struct kfd_process *p, v
 	return r;
 }
 
+/* userspace programs need to invoke this ioctl explicitly on a FD to
+ * create a secondary kfd_process which replacing its primary kfd_process
+ */
+static int kfd_ioctl_create_process(struct file *filep, struct kfd_process *p, void *data)
+{
+	struct kfd_process *process;
+	int ret;
+
+	if (!filep->private_data || !p)
+		return -EINVAL;
+
+	if (p != filep->private_data)
+		return -EINVAL;
+
+	/* Each FD owns only one kfd_process */
+	if (!p->primary)
+		return -EINVAL;
+
+	mutex_lock(&kfd_processes_mutex);
+	process = create_process(current, false);
+	mutex_unlock(&kfd_processes_mutex);
+
+	if (IS_ERR(process))
+		return PTR_ERR(process);
+
+	/* Each open() increases kref of the primary kfd_process,
+	 * so we need to reduce it here before we create a new secondary process replacing it
+	 */
+	kfd_unref_process(p);
+
+	filep->private_data = process;
+	ret = kfd_create_process_sysfs(process);
+	if (ret)
+		pr_warn("Failed to create sysfs entry for the kfd_process");
+
+	return 0;
+}
+
 #define AMDKFD_IOCTL_DEF(ioctl, _func, _flags) \
 	[_IOC_NR(ioctl)] = {.cmd = ioctl, .func = _func, .flags = _flags, \
 			    .cmd_drv = 0, .name = #ioctl}
@@ -3254,6 +3292,9 @@ static const struct amdkfd_ioctl_desc amdkfd_ioctls[] = {
 
 	AMDKFD_IOCTL_DEF(AMDKFD_IOC_DBG_TRAP,
 			kfd_ioctl_set_debug_trap, 0),
+
+	AMDKFD_IOCTL_DEF(AMDKFD_IOC_CREATE_PROCESS,
+			kfd_ioctl_create_process, 0),
 };
 
 #define AMDKFD_CORE_IOCTL_COUNT	ARRAY_SIZE(amdkfd_ioctls)
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index a6e12c705734..a2b5081fbfc0 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -1051,6 +1051,7 @@ struct kfd_process *kfd_get_process(const struct task_struct *task);
 struct kfd_process *kfd_lookup_process_by_pasid(u32 pasid,
 						 struct kfd_process_device **pdd);
 struct kfd_process *kfd_lookup_process_by_mm(const struct mm_struct *mm);
+struct kfd_process *create_process(const struct task_struct *thread, bool primary);
 
 int kfd_process_gpuidx_from_gpuid(struct kfd_process *p, uint32_t gpu_id);
 int kfd_process_gpuid_from_node(struct kfd_process *p, struct kfd_node *node,
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index e1ba9015bb83..15a8de2275f4 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -71,7 +71,6 @@ static struct workqueue_struct *kfd_restore_wq;
 static struct kfd_process *find_process(const struct task_struct *thread,
 					bool ref);
 static void kfd_process_ref_release(struct kref *ref);
-static struct kfd_process *create_process(const struct task_struct *thread, bool primary);
 
 static void evict_process_worker(struct work_struct *work);
 static void restore_process_worker(struct work_struct *work);
@@ -1598,7 +1597,7 @@ void kfd_process_set_trap_debug_flag(struct qcm_process_device *qpd,
  * On return the kfd_process is fully operational and will be freed when the
  * mm is released
  */
-static struct kfd_process *create_process(const struct task_struct *thread, bool primary)
+struct kfd_process *create_process(const struct task_struct *thread, bool primary)
 {
 	struct kfd_process *process;
 	struct mmu_notifier *mn;
diff --git a/include/uapi/linux/kfd_ioctl.h b/include/uapi/linux/kfd_ioctl.h
index 04c7d283dc7d..1d206ecc831e 100644
--- a/include/uapi/linux/kfd_ioctl.h
+++ b/include/uapi/linux/kfd_ioctl.h
@@ -44,9 +44,10 @@
  * - 1.16 - Add contiguous VRAM allocation flag
  * - 1.17 - Add SDMA queue creation with target SDMA engine ID
  * - 1.18 - Rename pad in set_memory_policy_args to misc_process_flag
+ * - 1.19 - Add a new ioctl to craete secondary kfd processes
  */
 #define KFD_IOCTL_MAJOR_VERSION 1
-#define KFD_IOCTL_MINOR_VERSION 18
+#define KFD_IOCTL_MINOR_VERSION 19
 
 struct kfd_ioctl_get_version_args {
 	__u32 major_version;	/* from KFD */
@@ -1671,7 +1672,10 @@ struct kfd_ioctl_dbg_trap_args {
 #define AMDKFD_IOC_DBG_TRAP			\
 		AMDKFD_IOWR(0x26, struct kfd_ioctl_dbg_trap_args)
 
+#define AMDKFD_IOC_CREATE_PROCESS		\
+		AMDKFD_IO(0x27)
+
 #define AMDKFD_COMMAND_START		0x01
-#define AMDKFD_COMMAND_END		0x27
+#define AMDKFD_COMMAND_END		0x28
 
 #endif
-- 
2.47.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 8/9] amdkfd: identify a secondary kfd process by its id
  2025-07-25  2:43 ` [PATCH 8/9] amdkfd: identify a secondary kfd process by its id Zhu Lingshan
@ 2025-07-28 19:48   ` Felix Kuehling
  2025-07-29  6:21     ` Zhu, Lingshan
  0 siblings, 1 reply; 15+ messages in thread
From: Felix Kuehling @ 2025-07-28 19:48 UTC (permalink / raw)
  To: Zhu Lingshan, alexander.deucher; +Cc: ray.huang, amd-gfx

On 2025-07-24 22:43, Zhu Lingshan wrote:
> This commit introduces a new id field for
> struct kfd process, which helps identify
> a kfd process among multiple contexts that
> all belong to a single user space program.
>
> The sysfs entry of a secondary kfd process
> is placed under the sysfs entry folder of
> its primary kfd process.
>
> The naming format of the sysfs entry of a secondary
> kfd process is "context_%u" where %u is the process id.
>
> Signed-off-by: Zhu Lingshan <lingshan.zhu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_priv.h    |  6 ++
>   drivers/gpu/drm/amd/amdkfd/kfd_process.c | 82 +++++++++++++++++++++++-
>   2 files changed, 85 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index de701d72aa5c..a6e12c705734 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -995,6 +995,9 @@ struct kfd_process {
>   	/* Tracks debug per-vmid request for debug flags */
>   	u32 dbg_flags;
>   
> +	/* kfd process id */
> +	u16 id;
> +
>   	atomic_t poison;
>   	/* Queues are in paused stated because we are in the process of doing a CRIU checkpoint */
>   	bool queues_paused;
> @@ -1009,6 +1012,9 @@ struct kfd_process {
>   
>   	/* indicating whether this is a primary kfd_process */
>   	bool primary;
> +
> +	/* The primary kfd_process allocating IDs for its secondary kfd_process, 0 for primary kfd_process */
> +	struct ida id_table;
>   };
>   
>   #define KFD_PROCESS_TABLE_SIZE 8 /* bits: 256 entries */
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index 440fde75d1e4..e1ba9015bb83 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -54,6 +54,9 @@ DEFINE_MUTEX(kfd_processes_mutex);
>   
>   DEFINE_SRCU(kfd_processes_srcu);
>   
> +#define KFD_PROCESS_ID_MIN 1
> +#define KFD_PROCESS_ID_WIDTH 16
> +
>   /* For process termination handling */
>   static struct workqueue_struct *kfd_process_wq;
>   
> @@ -827,6 +830,7 @@ static void kfd_process_device_destroy_ib_mem(struct kfd_process_device *pdd)
>   
>   int kfd_create_process_sysfs(struct kfd_process *process)
>   {
> +	struct kfd_process *primary_process;
>   	int ret;
>   
>   	if (process->kobj) {
> @@ -839,9 +843,22 @@ int kfd_create_process_sysfs(struct kfd_process *process)
>   		pr_warn("Creating procfs kobject failed");
>   		return -ENOMEM;
>   	}
> -	ret = kobject_init_and_add(process->kobj, &procfs_type,
> -				   procfs.kobj, "%d",
> -				   (int)process->lead_thread->pid);
> +
> +	if (process->primary)
> +		ret = kobject_init_and_add(process->kobj, &procfs_type,
> +					   procfs.kobj, "%d",
> +					   (int)process->lead_thread->pid);
> +	else {
> +		primary_process = kfd_lookup_process_by_mm(process->lead_thread->mm);
> +		if (!primary_process)
> +			return -EFAULT;

EFAULT means "Bad address". A better error code would be ESRCH "No such 
process".


> +
> +		ret = kobject_init_and_add(process->kobj, &procfs_type,
> +					   primary_process->kobj, "context_%u",
> +					   process->id);
> +		kfd_unref_process(primary_process);
> +	}
> +
>   	if (ret) {
>   		pr_warn("Creating procfs pid directory failed");
>   		kobject_put(process->kobj);
> @@ -863,6 +880,51 @@ int kfd_create_process_sysfs(struct kfd_process *process)
>   	return 0;
>   }
>   
> +static int kfd_process_alloc_id(struct kfd_process *process)
> +{
> +	u16 ret;
> +	struct kfd_process *primary_process;
> +
> +	if (process->primary) {
> +		process->id = 0;
> +
> +		return 0;
> +	}
> +
> +	primary_process = kfd_lookup_process_by_mm(process->lead_thread->mm);
> +	if (!primary_process)
> +		return -EFAULT;

ESRCH


> +
> +	ret = ida_alloc_range(&primary_process->id_table, KFD_PROCESS_ID_MIN,
> +	     1 << (KFD_PROCESS_ID_WIDTH - 1), GFP_KERNEL);

Did you mean (1 << KFD_PROCESS_ID_WIDTH) - 1? That would give you the 
range from 1 to 0xffff, which is what I'd expect with 16-bit wide ID.


> +	if (ret < 0)
> +		return ret;

You're leaking a process reference here.


> +
> +	process->id = ret;
> +
> +	kfd_unref_process(primary_process);
> +
> +	return 0;
> +}
> +
> +static void kfd_process_free_id(struct kfd_process *process)
> +{
> +	struct kfd_process *primary_process;
> +
> +	if (process->primary)
> +		return;
> +
> +	primary_process = kfd_lookup_process_by_mm(process->lead_thread->mm);
> +	if (!primary_process)
> +		return;
> +
> +	ida_free(&primary_process->id_table, process->id);
> +
> +	kfd_unref_process(primary_process);
> +
> +	return;

This return statement is unnecessary.


> +}
> +
>   struct kfd_process *kfd_create_process(struct task_struct *thread)
>   {
>   	struct kfd_process *process;
> @@ -1193,6 +1255,11 @@ static void kfd_process_wq_release(struct work_struct *work)
>   	if (ef)
>   		dma_fence_signal(ef);
>   
> +	if (!p->primary)
> +		kfd_process_free_id(p);
> +	else
> +		ida_destroy(&p->id_table);
> +
>   	kfd_process_remove_sysfs(p);
>   	kfd_debugfs_remove_process(p);
>   
> @@ -1549,6 +1616,12 @@ static struct kfd_process *create_process(const struct task_struct *thread, bool
>   	process->queues_paused = false;
>   	process->primary = primary;
>   
> +	err = kfd_process_alloc_id(process);
> +	if (err) {
> +		pr_err("Creating kfd process: failed to alloc an id\n");
> +		goto err_alloc_process;

That's the wrong label for cleanup. You'd end up leaking the process 
structure. You need to create a new label. See below.


> +	}
> +
>   	INIT_DELAYED_WORK(&process->eviction_work, evict_process_worker);
>   	INIT_DELAYED_WORK(&process->restore_work, restore_process_worker);
>   	process->last_restore_timestamp = get_jiffies_64();
> @@ -1599,6 +1672,8 @@ static struct kfd_process *create_process(const struct task_struct *thread, bool
>   			goto err_register_notifier;
>   		}
>   		BUG_ON(mn != &process->mmu_notifier);
> +
> +		ida_init(&process->id_table);
>   	}
>   
>   	kfd_unref_process(process);
> @@ -1619,6 +1694,7 @@ static struct kfd_process *create_process(const struct task_struct *thread, bool
>   err_process_pqm_init:
>   	kfd_event_free_process(process);
>   err_event_init:
> +	kfd_process_free_id(process);

You should add a new label here

err_alloc_id:

Regards,
   Felix


>   	mutex_destroy(&process->mutex);
>   	kfree(process);
>   err_alloc_process:

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 9/9] amdkfd: introduce new ioctl AMDKFD_IOC_CREATE_PROCESS
  2025-07-25  2:43 ` [PATCH 9/9] amdkfd: introduce new ioctl AMDKFD_IOC_CREATE_PROCESS Zhu Lingshan
@ 2025-07-28 19:59   ` Felix Kuehling
  2025-07-29  6:27     ` Zhu, Lingshan
  0 siblings, 1 reply; 15+ messages in thread
From: Felix Kuehling @ 2025-07-28 19:59 UTC (permalink / raw)
  To: Zhu Lingshan, alexander.deucher; +Cc: ray.huang, amd-gfx

On 2025-07-24 22:43, Zhu Lingshan wrote:
> This commit implemetns a new ioctl AMDKFD_IOC_CREATE_PROCESS
> that creates a new secondary kfd_progress on the FD.

To add a new ioctl upstream, do you have a link to the corresponding 
user mode changes?

Other than that, this patch looks good to me.

Regards,
   Felix


>
> To keep backward compatibility, userspace programs need to invoke
> this ioctl explicitly on a FD to create a secondary
> kfd_process which replacing its primary kfd_process.
>
> This commit bumps ioctl minor version.
>
> Signed-off-by: Zhu Lingshan <lingshan.zhu@amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 41 ++++++++++++++++++++++++
>   drivers/gpu/drm/amd/amdkfd/kfd_priv.h    |  1 +
>   drivers/gpu/drm/amd/amdkfd/kfd_process.c |  3 +-
>   include/uapi/linux/kfd_ioctl.h           |  8 +++--
>   4 files changed, 49 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index 5b22e1c47b2e..f9c43ff8a89f 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -3136,6 +3136,44 @@ static int kfd_ioctl_set_debug_trap(struct file *filep, struct kfd_process *p, v
>   	return r;
>   }
>   
> +/* userspace programs need to invoke this ioctl explicitly on a FD to
> + * create a secondary kfd_process which replacing its primary kfd_process
> + */
> +static int kfd_ioctl_create_process(struct file *filep, struct kfd_process *p, void *data)
> +{
> +	struct kfd_process *process;
> +	int ret;
> +
> +	if (!filep->private_data || !p)
> +		return -EINVAL;
> +
> +	if (p != filep->private_data)
> +		return -EINVAL;
> +
> +	/* Each FD owns only one kfd_process */
> +	if (!p->primary)
> +		return -EINVAL;
> +
> +	mutex_lock(&kfd_processes_mutex);
> +	process = create_process(current, false);
> +	mutex_unlock(&kfd_processes_mutex);
> +
> +	if (IS_ERR(process))
> +		return PTR_ERR(process);
> +
> +	/* Each open() increases kref of the primary kfd_process,
> +	 * so we need to reduce it here before we create a new secondary process replacing it
> +	 */
> +	kfd_unref_process(p);
> +
> +	filep->private_data = process;
> +	ret = kfd_create_process_sysfs(process);
> +	if (ret)
> +		pr_warn("Failed to create sysfs entry for the kfd_process");
> +
> +	return 0;
> +}
> +
>   #define AMDKFD_IOCTL_DEF(ioctl, _func, _flags) \
>   	[_IOC_NR(ioctl)] = {.cmd = ioctl, .func = _func, .flags = _flags, \
>   			    .cmd_drv = 0, .name = #ioctl}
> @@ -3254,6 +3292,9 @@ static const struct amdkfd_ioctl_desc amdkfd_ioctls[] = {
>   
>   	AMDKFD_IOCTL_DEF(AMDKFD_IOC_DBG_TRAP,
>   			kfd_ioctl_set_debug_trap, 0),
> +
> +	AMDKFD_IOCTL_DEF(AMDKFD_IOC_CREATE_PROCESS,
> +			kfd_ioctl_create_process, 0),
>   };
>   
>   #define AMDKFD_CORE_IOCTL_COUNT	ARRAY_SIZE(amdkfd_ioctls)
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index a6e12c705734..a2b5081fbfc0 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -1051,6 +1051,7 @@ struct kfd_process *kfd_get_process(const struct task_struct *task);
>   struct kfd_process *kfd_lookup_process_by_pasid(u32 pasid,
>   						 struct kfd_process_device **pdd);
>   struct kfd_process *kfd_lookup_process_by_mm(const struct mm_struct *mm);
> +struct kfd_process *create_process(const struct task_struct *thread, bool primary);
>   
>   int kfd_process_gpuidx_from_gpuid(struct kfd_process *p, uint32_t gpu_id);
>   int kfd_process_gpuid_from_node(struct kfd_process *p, struct kfd_node *node,
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index e1ba9015bb83..15a8de2275f4 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -71,7 +71,6 @@ static struct workqueue_struct *kfd_restore_wq;
>   static struct kfd_process *find_process(const struct task_struct *thread,
>   					bool ref);
>   static void kfd_process_ref_release(struct kref *ref);
> -static struct kfd_process *create_process(const struct task_struct *thread, bool primary);
>   
>   static void evict_process_worker(struct work_struct *work);
>   static void restore_process_worker(struct work_struct *work);
> @@ -1598,7 +1597,7 @@ void kfd_process_set_trap_debug_flag(struct qcm_process_device *qpd,
>    * On return the kfd_process is fully operational and will be freed when the
>    * mm is released
>    */
> -static struct kfd_process *create_process(const struct task_struct *thread, bool primary)
> +struct kfd_process *create_process(const struct task_struct *thread, bool primary)
>   {
>   	struct kfd_process *process;
>   	struct mmu_notifier *mn;
> diff --git a/include/uapi/linux/kfd_ioctl.h b/include/uapi/linux/kfd_ioctl.h
> index 04c7d283dc7d..1d206ecc831e 100644
> --- a/include/uapi/linux/kfd_ioctl.h
> +++ b/include/uapi/linux/kfd_ioctl.h
> @@ -44,9 +44,10 @@
>    * - 1.16 - Add contiguous VRAM allocation flag
>    * - 1.17 - Add SDMA queue creation with target SDMA engine ID
>    * - 1.18 - Rename pad in set_memory_policy_args to misc_process_flag
> + * - 1.19 - Add a new ioctl to craete secondary kfd processes
>    */
>   #define KFD_IOCTL_MAJOR_VERSION 1
> -#define KFD_IOCTL_MINOR_VERSION 18
> +#define KFD_IOCTL_MINOR_VERSION 19
>   
>   struct kfd_ioctl_get_version_args {
>   	__u32 major_version;	/* from KFD */
> @@ -1671,7 +1672,10 @@ struct kfd_ioctl_dbg_trap_args {
>   #define AMDKFD_IOC_DBG_TRAP			\
>   		AMDKFD_IOWR(0x26, struct kfd_ioctl_dbg_trap_args)
>   
> +#define AMDKFD_IOC_CREATE_PROCESS		\
> +		AMDKFD_IO(0x27)
> +
>   #define AMDKFD_COMMAND_START		0x01
> -#define AMDKFD_COMMAND_END		0x27
> +#define AMDKFD_COMMAND_END		0x28
>   
>   #endif

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 0/9] amdkfd: Implement kfd multiple contexts
  2025-07-25  2:43 [PATCH 0/9] amdkfd: Implement kfd multiple contexts Zhu Lingshan
                   ` (8 preceding siblings ...)
  2025-07-25  2:43 ` [PATCH 9/9] amdkfd: introduce new ioctl AMDKFD_IOC_CREATE_PROCESS Zhu Lingshan
@ 2025-07-28 19:59 ` Felix Kuehling
  9 siblings, 0 replies; 15+ messages in thread
From: Felix Kuehling @ 2025-07-28 19:59 UTC (permalink / raw)
  To: Zhu Lingshan, alexander.deucher; +Cc: ray.huang, amd-gfx

Patches 1-7 are

Reviewed-by: Felix Kuehling <felix.kuehling@amd.com>

See my separate comments on patches 8 and 9.

Regards,
   Felix


On 2025-07-24 22:43, Zhu Lingshan wrote:
> Currently kfd manages kfd_process in a one context (kfd_process)
> per program manner, thus each user space program
> only onws one kfd context (kfd_process).
>
> This model works fine for most of the programs, but imperfect
> for a hypervisor like QEMU. Because all programs in the guest
> user space share the same only one kfd context, which is
> problematic, including but not limited to:
>
> As illustrated in Figure 1, all guest user space programs share the same fd of /dev/kfd
> and the same kfd_process, and the same PASID leading to the same
> GPU_VM address space. Therefore the IOVA range of each
> guest user space programs are not isolated,
> they can attack each other through GPU DMA.
>
>
>   +----------------------------------------------------------------------------------+
>   |                                                                                  |
>   |  +-----------+      +-----------+      +------------+      +------------+        |
>   |  |           |      |           |      |            |      |            |        |
>   |  | Program 1 |      | Program 2 |      | Program 3  |      | Program N  |        |
>   |  |           |      |           |      |            |      |            |        |
>   |  +----+------+      +--------+--+      +--+---------+      +-----+------+        |
>   |       |                      |            |                      |               |
>   |       |                      |            |                      |        Guest  |
>   |       |                      |            |                      |               |
>   +-------+----------------------+------------+----------------------+---------------+
>           |                      |            |                      |
>           |                      |            |                      |
>           |                      |            |                      |
>           |                      |            |                      |
>           |                   +--+------------+---+                  |
>           |                   | file descriptor   |                  |
>           +-------------------+ of /dev/kfd       +------------------+
>                               | opened by QEMU    |
>                               |                   |
>                               +---------+---------+                   User Space
>                                         |                             QEMU
>                                         |
> ---------------------------------------+-----------------------------------------------------
>                                         |                             Kernel Space
>                                         |                             KFD Module
>                                         |
>                                +--------+--------+
>                                |                 |
>                                |   kfd_process   |<------------------The only one KFD context
>                                |                 |
>                                +--------+--------+
>                                         |
>                                +--------+--------+
>                                |     PASID       |
>                                +--------+--------+
>                                         |
>                                +--------+--------+
>                                |      GPU_VM     |
>                                +-----------------+
>
>                                   Fiture 1
>
>
> This series implements a multiple contexts solution:
> - Allow each program to create and hold multiple contexts (kfd processes)
> - Each context has its own fd of /dev/kfd and an exclusive kfd_process,
>    which is a secondary kfd context. So that PASID/GPU VM isolates their IOVA address spaces.
>    Therefore, they can not attack each other through GPU DMA.
>
> The design is illustrated in Figure 2 below:
>
>     +---------------------------------------------------------------------------------------------------------+
>     |                                                                                                         |
>     |                                                                                                         |
>     |                                                                                                         |
>     |       +----------------------------------------------------------------------------------+              |
>     |       |                                                                                  |              |
>     |       | +-----------+      +-----------+     +-----------+    +-----------+              |              |
>     |       | |           |      |           |     |           |    |           |              |              |
>     |       | | Program 1 |      | Program 2 |     | Program 3 |    | Program N |              |              |
>     |       | |           |      |           |     |           |    |           |              |              |
>     |       | +-----+-----+      +-----+-----+     +-----+-----+    +-----+-----+              |              |
>     |       |       |                  |                 |                |                    |              |
>     |       |       |                  |                 |                |        Guest       |              |
>     |       |       |                  |                 |                |                    |              |
>     |       +-------+------------------+-----------------+----------------+--------------------+              |
>     |               |                  |                 |                |                            QEMU   |
>     |               |                  |                 |                |                                   |
>     +---------------+------------------+-----------------+----------------+--------------------------+--------+
>                     |                  |                 |                |                          |
>                     |                  |                 |                |                          |
>                     |                  |                 |                |                          |
>                 +---+----+         +---+----+        +---+----+       +---+----+                 +---+-----+
>                 |        |         |        |        |        |       |        |                 | Primary |
>                 |  FD 1  |         |  FD 2  |        |  FD 3  |       |  FD 4  |                 |   FD    |
>                 |        |         |        |        |        |       |        |                 |         |
>                 +---+----+         +---+----+        +---+----+       +----+---+                 +----+----+
>                     |                  |                 |                 |                          |             User Space
>                     |                  |                 |                 |                          |
> -------------------+------------------+-----------------+-----------------+--------------------------+----------------------------
>                     |                  |                 |                 |                          |             Kernel SPace
>                     |                  |                 |                 |                          |
>                     |                  |                 |                 |                          |
>     +--------------------------------------------------------------------------------------------------------------------------+
>     |        +------+------+    +------+------+   +------+------+   +------+------+            +------+------+                 |
>     |        | Secondary   |    | Secondary   |   | Secondary   |   | Secondary   |            |  Primary    |   KFD Module    |
>     |        |kfd_process 1|    |kfd_process 2|   |kfd_process 3|   |kfd_process 4|            | kfd_process |                 |
>     |        |             |    |             |   |             |   |             |            |             |                 |
>     |        +------+------+    +------+------+   +------+------+   +------+------+            +------+------+                 |
>     |               |                  |                 |                 |                          |                        |
>     |        +------+------+    +------+------+   +------+------+   +------+------+            +------+------+                 |
>     |        |   PASID     |    |   PASID     |   |   PASID     |   |   PASID     |            |   PASID     |                 |
>     |        +------+------+    +------+------+   +------+------+   +------+------+            +------+------+                 |
>     |               |                  |                 |                 |                          |                        |
>     |               |                  |                 |                 |                          |                        |
>     |        +------+------+    +------+------+   +------+------+   +------+------+            +------+------+                 |
>     |        |   GPU_VM    |    |   GPU_VM    |   |   GPU_VM    |   |   GPU_VM    |            |   GPU_VM    |                 |
>     |        +-------------+    +-------------+   +-------------+   +-------------+            +-------------+                 |
>     |                                                                                                                          |
>     +--------------------------------------------------------------------------------------------------------------------------+
>                                                                                                                                    
>                                                    Figure 2
>
> Zhu Lingshan (9):
>    amdkfd: enlarge the hashtable of kfd_process
>    amdkfd: mark the first kfd_process as the primary one
>    amdkfd: find_process_by_mm always return the primary context
>    amdkfd: Introduce kfd_create_process_sysfs as a separate function
>    amdkfd: destroy kfd secondary contexts through fd close
>    amdkfd: process svm ioctl only on the primary kfd process
>    amdkfd: process USERPTR allocation only on the primary kfd process
>    amdkfd: identify a secondary kfd process by its id
>    amdkfd: introduce new ioctl AMDKFD_IOC_CREATE_PROCESS
>
>   drivers/gpu/drm/amd/amdkfd/kfd_chardev.c |  62 ++++++-
>   drivers/gpu/drm/amd/amdkfd/kfd_priv.h    |  14 +-
>   drivers/gpu/drm/amd/amdkfd/kfd_process.c | 204 +++++++++++++++++------
>   include/uapi/linux/kfd_ioctl.h           |   8 +-
>   4 files changed, 231 insertions(+), 57 deletions(-)
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 8/9] amdkfd: identify a secondary kfd process by its id
  2025-07-28 19:48   ` Felix Kuehling
@ 2025-07-29  6:21     ` Zhu, Lingshan
  0 siblings, 0 replies; 15+ messages in thread
From: Zhu, Lingshan @ 2025-07-29  6:21 UTC (permalink / raw)
  To: Felix Kuehling, alexander.deucher; +Cc: ray.huang, amd-gfx

[-- Attachment #1: Type: text/plain, Size: 7743 bytes --]

Thanks, Felix. All issues have been fixed, and I will send out V2 soon.

Thanks
Lingshan

On 7/29/2025 3:48 AM, Felix Kuehling wrote:
> On 2025-07-24 22:43, Zhu Lingshan wrote:
>> This commit introduces a new id field for
>> struct kfd process, which helps identify
>> a kfd process among multiple contexts that
>> all belong to a single user space program.
>>
>> The sysfs entry of a secondary kfd process
>> is placed under the sysfs entry folder of
>> its primary kfd process.
>>
>> The naming format of the sysfs entry of a secondary
>> kfd process is "context_%u" where %u is the process id.
>>
>> Signed-off-by: Zhu Lingshan <lingshan.zhu@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdkfd/kfd_priv.h    |  6 ++
>>   drivers/gpu/drm/amd/amdkfd/kfd_process.c | 82 +++++++++++++++++++++++-
>>   2 files changed, 85 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> index de701d72aa5c..a6e12c705734 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> @@ -995,6 +995,9 @@ struct kfd_process {
>>       /* Tracks debug per-vmid request for debug flags */
>>       u32 dbg_flags;
>>   +    /* kfd process id */
>> +    u16 id;
>> +
>>       atomic_t poison;
>>       /* Queues are in paused stated because we are in the process of
>> doing a CRIU checkpoint */
>>       bool queues_paused;
>> @@ -1009,6 +1012,9 @@ struct kfd_process {
>>         /* indicating whether this is a primary kfd_process */
>>       bool primary;
>> +
>> +    /* The primary kfd_process allocating IDs for its secondary
>> kfd_process, 0 for primary kfd_process */
>> +    struct ida id_table;
>>   };
>>     #define KFD_PROCESS_TABLE_SIZE 8 /* bits: 256 entries */
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>> b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>> index 440fde75d1e4..e1ba9015bb83 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>> @@ -54,6 +54,9 @@ DEFINE_MUTEX(kfd_processes_mutex);
>>     DEFINE_SRCU(kfd_processes_srcu);
>>   +#define KFD_PROCESS_ID_MIN 1
>> +#define KFD_PROCESS_ID_WIDTH 16
>> +
>>   /* For process termination handling */
>>   static struct workqueue_struct *kfd_process_wq;
>>   @@ -827,6 +830,7 @@ static void
>> kfd_process_device_destroy_ib_mem(struct kfd_process_device *pdd)
>>     int kfd_create_process_sysfs(struct kfd_process *process)
>>   {
>> +    struct kfd_process *primary_process;
>>       int ret;
>>         if (process->kobj) {
>> @@ -839,9 +843,22 @@ int kfd_create_process_sysfs(struct kfd_process
>> *process)
>>           pr_warn("Creating procfs kobject failed");
>>           return -ENOMEM;
>>       }
>> -    ret = kobject_init_and_add(process->kobj, &procfs_type,
>> -                   procfs.kobj, "%d",
>> -                   (int)process->lead_thread->pid);
>> +
>> +    if (process->primary)
>> +        ret = kobject_init_and_add(process->kobj, &procfs_type,
>> +                       procfs.kobj, "%d",
>> +                       (int)process->lead_thread->pid);
>> +    else {
>> +        primary_process =
>> kfd_lookup_process_by_mm(process->lead_thread->mm);
>> +        if (!primary_process)
>> +            return -EFAULT;
>
> EFAULT means "Bad address". A better error code would be ESRCH "No
> such process".
>
>
>> +
>> +        ret = kobject_init_and_add(process->kobj, &procfs_type,
>> +                       primary_process->kobj, "context_%u",
>> +                       process->id);
>> +        kfd_unref_process(primary_process);
>> +    }
>> +
>>       if (ret) {
>>           pr_warn("Creating procfs pid directory failed");
>>           kobject_put(process->kobj);
>> @@ -863,6 +880,51 @@ int kfd_create_process_sysfs(struct kfd_process
>> *process)
>>       return 0;
>>   }
>>   +static int kfd_process_alloc_id(struct kfd_process *process)
>> +{
>> +    u16 ret;
>> +    struct kfd_process *primary_process;
>> +
>> +    if (process->primary) {
>> +        process->id = 0;
>> +
>> +        return 0;
>> +    }
>> +
>> +    primary_process =
>> kfd_lookup_process_by_mm(process->lead_thread->mm);
>> +    if (!primary_process)
>> +        return -EFAULT;
>
> ESRCH
>
>
>> +
>> +    ret = ida_alloc_range(&primary_process->id_table,
>> KFD_PROCESS_ID_MIN,
>> +         1 << (KFD_PROCESS_ID_WIDTH - 1), GFP_KERNEL);
>
> Did you mean (1 << KFD_PROCESS_ID_WIDTH) - 1? That would give you the
> range from 1 to 0xffff, which is what I'd expect with 16-bit wide ID.
>
>
>> +    if (ret < 0)
>> +        return ret;
>
> You're leaking a process reference here.
>
>
>> +
>> +    process->id = ret;
>> +
>> +    kfd_unref_process(primary_process);
>> +
>> +    return 0;
>> +}
>> +
>> +static void kfd_process_free_id(struct kfd_process *process)
>> +{
>> +    struct kfd_process *primary_process;
>> +
>> +    if (process->primary)
>> +        return;
>> +
>> +    primary_process =
>> kfd_lookup_process_by_mm(process->lead_thread->mm);
>> +    if (!primary_process)
>> +        return;
>> +
>> +    ida_free(&primary_process->id_table, process->id);
>> +
>> +    kfd_unref_process(primary_process);
>> +
>> +    return;
>
> This return statement is unnecessary.
>
>
>> +}
>> +
>>   struct kfd_process *kfd_create_process(struct task_struct *thread)
>>   {
>>       struct kfd_process *process;
>> @@ -1193,6 +1255,11 @@ static void kfd_process_wq_release(struct
>> work_struct *work)
>>       if (ef)
>>           dma_fence_signal(ef);
>>   +    if (!p->primary)
>> +        kfd_process_free_id(p);
>> +    else
>> +        ida_destroy(&p->id_table);
>> +
>>       kfd_process_remove_sysfs(p);
>>       kfd_debugfs_remove_process(p);
>>   @@ -1549,6 +1616,12 @@ static struct kfd_process
>> *create_process(const struct task_struct *thread, bool
>>       process->queues_paused = false;
>>       process->primary = primary;
>>   +    err = kfd_process_alloc_id(process);
>> +    if (err) {
>> +        pr_err("Creating kfd process: failed to alloc an id\n");
>> +        goto err_alloc_process;
>
> That's the wrong label for cleanup. You'd end up leaking the process
> structure. You need to create a new label. See below.
>
>
>> +    }
>> +
>>       INIT_DELAYED_WORK(&process->eviction_work, evict_process_worker);
>>       INIT_DELAYED_WORK(&process->restore_work, restore_process_worker);
>>       process->last_restore_timestamp = get_jiffies_64();
>> @@ -1599,6 +1672,8 @@ static struct kfd_process *create_process(const
>> struct task_struct *thread, bool
>>               goto err_register_notifier;
>>           }
>>           BUG_ON(mn != &process->mmu_notifier);
>> +
>> +        ida_init(&process->id_table);
>>       }
>>         kfd_unref_process(process);
>> @@ -1619,6 +1694,7 @@ static struct kfd_process *create_process(const
>> struct task_struct *thread, bool
>>   err_process_pqm_init:
>>       kfd_event_free_process(process);
>>   err_event_init:
>> +    kfd_process_free_id(process);
>
> You should add a new label here
>
> err_alloc_id:
>
> Regards,
>   Felix
>
>
>>       mutex_destroy(&process->mutex);
>>       kfree(process);
>>   err_alloc_process:

[-- Attachment #2: Type: text/html, Size: 14690 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 9/9] amdkfd: introduce new ioctl AMDKFD_IOC_CREATE_PROCESS
  2025-07-28 19:59   ` Felix Kuehling
@ 2025-07-29  6:27     ` Zhu, Lingshan
  0 siblings, 0 replies; 15+ messages in thread
From: Zhu, Lingshan @ 2025-07-29  6:27 UTC (permalink / raw)
  To: Felix Kuehling, alexander.deucher; +Cc: ray.huang, amd-gfx

[-- Attachment #1: Type: text/plain, Size: 6783 bytes --]

On 7/29/2025 3:59 AM, Felix Kuehling wrote:

> On 2025-07-24 22:43, Zhu Lingshan wrote:
>> This commit implemetns a new ioctl AMDKFD_IOC_CREATE_PROCESS
>> that creates a new secondary kfd_progress on the FD.
>
> To add a new ioctl upstream, do you have a link to the corresponding
> user mode changes? 

I have tested the code locally and we are developing kfdtest for multiple contexts.
This new ioctl will not break any current use cases.

I will post my local test program for this ioctl in the cover letter.

Thanks
Lingshan

>
> Other than that, this patch looks good to me.
>
> Regards,
>   Felix
>
>
>>
>> To keep backward compatibility, userspace programs need to invoke
>> this ioctl explicitly on a FD to create a secondary
>> kfd_process which replacing its primary kfd_process.
>>
>> This commit bumps ioctl minor version.
>>
>> Signed-off-by: Zhu Lingshan <lingshan.zhu@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 41 ++++++++++++++++++++++++
>>   drivers/gpu/drm/amd/amdkfd/kfd_priv.h    |  1 +
>>   drivers/gpu/drm/amd/amdkfd/kfd_process.c |  3 +-
>>   include/uapi/linux/kfd_ioctl.h           |  8 +++--
>>   4 files changed, 49 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>> b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>> index 5b22e1c47b2e..f9c43ff8a89f 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
>> @@ -3136,6 +3136,44 @@ static int kfd_ioctl_set_debug_trap(struct
>> file *filep, struct kfd_process *p, v
>>       return r;
>>   }
>>   +/* userspace programs need to invoke this ioctl explicitly on a FD to
>> + * create a secondary kfd_process which replacing its primary
>> kfd_process
>> + */
>> +static int kfd_ioctl_create_process(struct file *filep, struct
>> kfd_process *p, void *data)
>> +{
>> +    struct kfd_process *process;
>> +    int ret;
>> +
>> +    if (!filep->private_data || !p)
>> +        return -EINVAL;
>> +
>> +    if (p != filep->private_data)
>> +        return -EINVAL;
>> +
>> +    /* Each FD owns only one kfd_process */
>> +    if (!p->primary)
>> +        return -EINVAL;
>> +
>> +    mutex_lock(&kfd_processes_mutex);
>> +    process = create_process(current, false);
>> +    mutex_unlock(&kfd_processes_mutex);
>> +
>> +    if (IS_ERR(process))
>> +        return PTR_ERR(process);
>> +
>> +    /* Each open() increases kref of the primary kfd_process,
>> +     * so we need to reduce it here before we create a new secondary
>> process replacing it
>> +     */
>> +    kfd_unref_process(p);
>> +
>> +    filep->private_data = process;
>> +    ret = kfd_create_process_sysfs(process);
>> +    if (ret)
>> +        pr_warn("Failed to create sysfs entry for the kfd_process");
>> +
>> +    return 0;
>> +}
>> +
>>   #define AMDKFD_IOCTL_DEF(ioctl, _func, _flags) \
>>       [_IOC_NR(ioctl)] = {.cmd = ioctl, .func = _func, .flags =
>> _flags, \
>>                   .cmd_drv = 0, .name = #ioctl}
>> @@ -3254,6 +3292,9 @@ static const struct amdkfd_ioctl_desc
>> amdkfd_ioctls[] = {
>>         AMDKFD_IOCTL_DEF(AMDKFD_IOC_DBG_TRAP,
>>               kfd_ioctl_set_debug_trap, 0),
>> +
>> +    AMDKFD_IOCTL_DEF(AMDKFD_IOC_CREATE_PROCESS,
>> +            kfd_ioctl_create_process, 0),
>>   };
>>     #define AMDKFD_CORE_IOCTL_COUNT    ARRAY_SIZE(amdkfd_ioctls)
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> index a6e12c705734..a2b5081fbfc0 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> @@ -1051,6 +1051,7 @@ struct kfd_process *kfd_get_process(const
>> struct task_struct *task);
>>   struct kfd_process *kfd_lookup_process_by_pasid(u32 pasid,
>>                            struct kfd_process_device **pdd);
>>   struct kfd_process *kfd_lookup_process_by_mm(const struct mm_struct
>> *mm);
>> +struct kfd_process *create_process(const struct task_struct *thread,
>> bool primary);
>>     int kfd_process_gpuidx_from_gpuid(struct kfd_process *p, uint32_t
>> gpu_id);
>>   int kfd_process_gpuid_from_node(struct kfd_process *p, struct
>> kfd_node *node,
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>> b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>> index e1ba9015bb83..15a8de2275f4 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>> @@ -71,7 +71,6 @@ static struct workqueue_struct *kfd_restore_wq;
>>   static struct kfd_process *find_process(const struct task_struct
>> *thread,
>>                       bool ref);
>>   static void kfd_process_ref_release(struct kref *ref);
>> -static struct kfd_process *create_process(const struct task_struct
>> *thread, bool primary);
>>     static void evict_process_worker(struct work_struct *work);
>>   static void restore_process_worker(struct work_struct *work);
>> @@ -1598,7 +1597,7 @@ void kfd_process_set_trap_debug_flag(struct
>> qcm_process_device *qpd,
>>    * On return the kfd_process is fully operational and will be freed
>> when the
>>    * mm is released
>>    */
>> -static struct kfd_process *create_process(const struct task_struct
>> *thread, bool primary)
>> +struct kfd_process *create_process(const struct task_struct *thread,
>> bool primary)
>>   {
>>       struct kfd_process *process;
>>       struct mmu_notifier *mn;
>> diff --git a/include/uapi/linux/kfd_ioctl.h
>> b/include/uapi/linux/kfd_ioctl.h
>> index 04c7d283dc7d..1d206ecc831e 100644
>> --- a/include/uapi/linux/kfd_ioctl.h
>> +++ b/include/uapi/linux/kfd_ioctl.h
>> @@ -44,9 +44,10 @@
>>    * - 1.16 - Add contiguous VRAM allocation flag
>>    * - 1.17 - Add SDMA queue creation with target SDMA engine ID
>>    * - 1.18 - Rename pad in set_memory_policy_args to misc_process_flag
>> + * - 1.19 - Add a new ioctl to craete secondary kfd processes
>>    */
>>   #define KFD_IOCTL_MAJOR_VERSION 1
>> -#define KFD_IOCTL_MINOR_VERSION 18
>> +#define KFD_IOCTL_MINOR_VERSION 19
>>     struct kfd_ioctl_get_version_args {
>>       __u32 major_version;    /* from KFD */
>> @@ -1671,7 +1672,10 @@ struct kfd_ioctl_dbg_trap_args {
>>   #define AMDKFD_IOC_DBG_TRAP            \
>>           AMDKFD_IOWR(0x26, struct kfd_ioctl_dbg_trap_args)
>>   +#define AMDKFD_IOC_CREATE_PROCESS        \
>> +        AMDKFD_IO(0x27)
>> +
>>   #define AMDKFD_COMMAND_START        0x01
>> -#define AMDKFD_COMMAND_END        0x27
>> +#define AMDKFD_COMMAND_END        0x28
>>     #endif

[-- Attachment #2: Type: text/html, Size: 11263 bytes --]

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2025-07-29  6:27 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-25  2:43 [PATCH 0/9] amdkfd: Implement kfd multiple contexts Zhu Lingshan
2025-07-25  2:43 ` [PATCH 1/9] amdkfd: enlarge the hashtable of kfd_process Zhu Lingshan
2025-07-25  2:43 ` [PATCH 2/9] amdkfd: mark the first kfd_process as the primary one Zhu Lingshan
2025-07-25  2:43 ` [PATCH 3/9] amdkfd: find_process_by_mm always return the primary context Zhu Lingshan
2025-07-25  2:43 ` [PATCH 4/9] amdkfd: Introduce kfd_create_process_sysfs as a separate function Zhu Lingshan
2025-07-25  2:43 ` [PATCH 5/9] amdkfd: destroy kfd secondary contexts through fd close Zhu Lingshan
2025-07-25  2:43 ` [PATCH 6/9] amdkfd: process svm ioctl only on the primary kfd process Zhu Lingshan
2025-07-25  2:43 ` [PATCH 7/9] amdkfd: process USERPTR allocation " Zhu Lingshan
2025-07-25  2:43 ` [PATCH 8/9] amdkfd: identify a secondary kfd process by its id Zhu Lingshan
2025-07-28 19:48   ` Felix Kuehling
2025-07-29  6:21     ` Zhu, Lingshan
2025-07-25  2:43 ` [PATCH 9/9] amdkfd: introduce new ioctl AMDKFD_IOC_CREATE_PROCESS Zhu Lingshan
2025-07-28 19:59   ` Felix Kuehling
2025-07-29  6:27     ` Zhu, Lingshan
2025-07-28 19:59 ` [PATCH 0/9] amdkfd: Implement kfd multiple contexts Felix Kuehling

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).