Re: [PATCH] amdkfd: implement per process/device pasid in sysfs

[Kuehling, Felix]

On 2025-10-30 22:46, Zhu, Lingshan wrote:
On 10/30/2025 11:05 PM, Kuehling, Felix wrote:
On 2025-10-29 23:45, Zhu Lingshan wrote:
The pasid is a per-process-per-device attribute,
therefore this commit implements per
struct kfd_process_device->pasid in sysfs

Does anyone in user mode actually need this PASID? When we changed 
the PASID allocation to be per-process-device, we changed a bunch of 
our dmesg logging (and I think debugfs files, too) to report PIDs 
instead of PASIDs. So there should be no good reason to know PASIDs 
in user mode.
Hello Felix,

This patch is to fix current buggy pasid in sysfs which is hard-coded 0,
if we don't need to expose pasid to user space, I think we should remove it 
from sysfs.

It's not buggy, it's there to avoid breaking old user mode tools that 
expected to find a PASID per process. Take a look at this commit for the 
history: commit 8544374c0f82edb285779f21b149826fe2c2977c

Author: Xiaogang Chen <xiaogang.c...@amd.com>
Date:   Mon Jan 13 17:35:59 2025 -0600

    drm/amdkfd: Have kfd driver use same PASID values from graphic driver

We used to have a PASID per process and reported it to user mode. We had 
to change that to fix some issues related to multiple GPU partitions. 
That broke reporting of PASIDs to user mode. We kept reporting a 0 PASID 
to avoid breaking existing user mode tools that expect to read a PASID. 
But because that PASID was never that useful to begin with, reporting 0 
doesn't do any harm here.

Adding another interface to report per-process/device PASIDs to user 
mode is pointless if there is no user mode client that needs that 
information. And we don't want user mode to use PASIDs to refer to 
processes or VM address spaces.

Regards,
  Felix



Thanks
Lingshan

Regards,
  Felix



Signed-off-by: Zhu Lingshan <lingshan....@amd.com>
---
  drivers/gpu/drm/amd/amdkfd/kfd_priv.h    |  9 ++-------
  drivers/gpu/drm/amd/amdkfd/kfd_process.c | 18 +++++++++++-------
  2 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h 
b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
index 70ef051511bb..6a3cfeccacd8 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
@@ -864,6 +864,8 @@ struct kfd_process_device {
      bool has_reset_queue;
        u32 pasid;
+    char pasid_filename[MAX_SYSFS_FILENAME_LEN];
+    struct attribute attr_pasid;
  };
    #define qpd_to_pdd(x) container_of(x, struct kfd_process_device, 
qpd)
@@ -983,7 +985,6 @@ struct kfd_process {
      /* Kobj for our procfs */
      struct kobject *kobj;
      struct kobject *kobj_queues;
-    struct attribute attr_pasid;
        /* Keep track cwsr init */
      bool has_cwsr;
@@ -1100,12 +1101,6 @@ void 
kfd_process_device_remove_obj_handle(struct kfd_process_device *pdd,
                      int handle);
  struct kfd_process *kfd_lookup_process_by_pid(struct pid *pid);
  -/* PASIDs */
-int kfd_pasid_init(void);
-void kfd_pasid_exit(void);
-u32 kfd_pasid_alloc(void);
-void kfd_pasid_free(u32 pasid);
-
  /* Doorbells */
  size_t kfd_doorbell_process_slice(struct kfd_dev *kfd);
  int kfd_doorbell_init(struct kfd_dev *kfd);
diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c 
b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
index ddfe30c13e9d..24cf3b250b37 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
@@ -328,9 +328,11 @@ static int kfd_get_cu_occupancy(struct 
attribute *attr, char *buffer)
  static ssize_t kfd_procfs_show(struct kobject *kobj, struct 
attribute *attr,
                     char *buffer)
  {
-    if (strcmp(attr->name, "pasid") == 0)
-        return snprintf(buffer, PAGE_SIZE, "%d\n", 0);
-    else if (strncmp(attr->name, "vram_", 5) == 0) {
+    if (strncmp(attr->name, "pasid_", 6) == 0) {
+        struct kfd_process_device *pdd = container_of(attr, struct 
kfd_process_device,
+                                  attr_pasid);
+        return snprintf(buffer, PAGE_SIZE, "%u\n", pdd->pasid);
+    } else if (strncmp(attr->name, "vram_", 5) == 0) {
          struct kfd_process_device *pdd = container_of(attr, struct 
kfd_process_device,
                                    attr_vram);
          return snprintf(buffer, PAGE_SIZE, "%llu\n", 
atomic64_read(&pdd->vram_usage));
@@ -662,6 +664,7 @@ static void kfd_procfs_add_sysfs_files(struct 
kfd_process *p)
       * Create sysfs files for each GPU:
       * - proc/<pid>/vram_<gpuid>
       * - proc/<pid>/sdma_<gpuid>
+     * - proc/<pid>/pasid_<gpuid>
       */
      for (i = 0; i < p->n_pdds; i++) {
          struct kfd_process_device *pdd = p->pdds[i];
@@ -675,6 +678,10 @@ static void kfd_procfs_add_sysfs_files(struct 
kfd_process *p)
               pdd->dev->id);
          kfd_sysfs_create_file(p->kobj, &pdd->attr_sdma,
                          pdd->sdma_filename);
+
+        snprintf(pdd->pasid_filename, MAX_SYSFS_FILENAME_LEN, 
"pasid_%u",
+             pdd->dev->id);
+        kfd_sysfs_create_file(p->kobj, &pdd->attr_pasid, 
pdd->pasid_filename);
      }
  }
  @@ -888,9 +895,6 @@ struct kfd_process *kfd_create_process(struct 
task_struct *thread)
              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)
@@ -1104,7 +1108,6 @@ static void kfd_process_remove_sysfs(struct 
kfd_process *p)
      if (!p->kobj)
          return;
  -    sysfs_remove_file(p->kobj, &p->attr_pasid);
      kobject_del(p->kobj_queues);
      kobject_put(p->kobj_queues);
      p->kobj_queues = NULL;
@@ -1114,6 +1117,7 @@ static void kfd_process_remove_sysfs(struct 
kfd_process *p)
            sysfs_remove_file(p->kobj, &pdd->attr_vram);
          sysfs_remove_file(p->kobj, &pdd->attr_sdma);
+        sysfs_remove_file(p->kobj, &pdd->attr_pasid);
            sysfs_remove_file(pdd->kobj_stats, &pdd->attr_evict);
          if (pdd->dev->kfd2kgd->get_cu_occupancy)