* Re: [PATCH] amdkfd: implement per process/device pasid in sysfs
2025-10-30 3:45 [PATCH] amdkfd: implement per process/device pasid in sysfs Zhu Lingshan
@ 2025-10-30 15:05 ` Kuehling, Felix
2025-10-31 2:46 ` Zhu, Lingshan
2025-10-30 15:53 ` Chen, Xiaogang
2025-10-30 18:32 ` Eric Huang
2 siblings, 1 reply; 10+ messages in thread
From: Kuehling, Felix @ 2025-10-30 15:05 UTC (permalink / raw)
To: Zhu Lingshan, alexander.deucher; +Cc: ray.huang, amd-gfx
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.
Regards,
Felix
>
> Signed-off-by: Zhu Lingshan <lingshan.zhu@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)
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] amdkfd: implement per process/device pasid in sysfs
2025-10-30 15:05 ` Kuehling, Felix
@ 2025-10-31 2:46 ` Zhu, Lingshan
2025-10-31 13:44 ` Kuehling, Felix
0 siblings, 1 reply; 10+ messages in thread
From: Zhu, Lingshan @ 2025-10-31 2:46 UTC (permalink / raw)
To: Kuehling, Felix, alexander.deucher; +Cc: ray.huang, amd-gfx
[-- Attachment #1: Type: text/plain, Size: 5884 bytes --]
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.
Thanks
Lingshan
>
> Regards,
> Felix
>
>
>>
>> Signed-off-by: Zhu Lingshan <lingshan.zhu@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)
[-- Attachment #2: Type: text/html, Size: 10844 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] amdkfd: implement per process/device pasid in sysfs
2025-10-31 2:46 ` Zhu, Lingshan
@ 2025-10-31 13:44 ` Kuehling, Felix
0 siblings, 0 replies; 10+ messages in thread
From: Kuehling, Felix @ 2025-10-31 13:44 UTC (permalink / raw)
To: Zhu, Lingshan, alexander.deucher; +Cc: ray.huang, amd-gfx
[-- Attachment #1: Type: text/plain, Size: 7105 bytes --]
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.chen@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.zhu@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)
[-- Attachment #2: Type: text/html, Size: 12036 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] amdkfd: implement per process/device pasid in sysfs
2025-10-30 3:45 [PATCH] amdkfd: implement per process/device pasid in sysfs Zhu Lingshan
2025-10-30 15:05 ` Kuehling, Felix
@ 2025-10-30 15:53 ` Chen, Xiaogang
2025-10-30 16:15 ` Christian König
2025-10-31 2:48 ` Zhu, Lingshan
2025-10-30 18:32 ` Eric Huang
2 siblings, 2 replies; 10+ messages in thread
From: Chen, Xiaogang @ 2025-10-30 15:53 UTC (permalink / raw)
To: Zhu Lingshan, felix.kuehling, alexander.deucher; +Cc: ray.huang, amd-gfx
On 10/29/2025 10:45 PM, Zhu Lingshan wrote:
> The pasid is a per-process-per-device attribute,
> therefore this commit implements per
> struct kfd_process_device->pasid in sysfs
This per device pasid is used internally in kfd, not used at user space.
So no need to exposing it.
>
> Signed-off-by: Zhu Lingshan <lingshan.zhu@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;
We keep it to have use space tools(ex rocm-smi) work as the tools still
read it before they change.
>
> /* 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);
This part is right, these declarations were forgotten to remove.
> -
> /* 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);
Same as above we keep it to have compatibility with current tools.
Regards
Xiaogang
> - 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)
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] amdkfd: implement per process/device pasid in sysfs
2025-10-30 15:53 ` Chen, Xiaogang
@ 2025-10-30 16:15 ` Christian König
2025-10-31 2:53 ` Zhu, Lingshan
2025-10-31 2:48 ` Zhu, Lingshan
1 sibling, 1 reply; 10+ messages in thread
From: Christian König @ 2025-10-30 16:15 UTC (permalink / raw)
To: Chen, Xiaogang, Zhu Lingshan, felix.kuehling, alexander.deucher
Cc: ray.huang, amd-gfx
On 10/30/25 16:53, Chen, Xiaogang wrote:
>
> On 10/29/2025 10:45 PM, Zhu Lingshan wrote:
>> The pasid is a per-process-per-device attribute,
>> therefore this commit implements per
>> struct kfd_process_device->pasid in sysfs
> This per device pasid is used internally in kfd, not used at user space. So no need to exposing it.
Agree completely, the PASID is a technical attribute we use internally in the kernel and should not expose to userspace at all.
Maybe in debugfs to narrow down problems, but certainly not in sysfs. That would make the internal handling an uAPI and so not changeable any more.
Regards,
Christian.
>>
>> Signed-off-by: Zhu Lingshan <lingshan.zhu@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;
> We keep it to have use space tools(ex rocm-smi) work as the tools still read it before they change.
>> /* 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);
> This part is right, these declarations were forgotten to remove.
>> -
>> /* 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);
>
> Same as above we keep it to have compatibility with current tools.
>
> Regards
>
> Xiaogang
>
>> - 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)
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] amdkfd: implement per process/device pasid in sysfs
2025-10-30 16:15 ` Christian König
@ 2025-10-31 2:53 ` Zhu, Lingshan
0 siblings, 0 replies; 10+ messages in thread
From: Zhu, Lingshan @ 2025-10-31 2:53 UTC (permalink / raw)
To: Christian König, Chen, Xiaogang, felix.kuehling,
alexander.deucher
Cc: ray.huang, amd-gfx
[-- Attachment #1: Type: text/plain, Size: 6349 bytes --]
On 10/31/2025 12:15 AM, Christian König wrote:
> On 10/30/25 16:53, Chen, Xiaogang wrote:
>> On 10/29/2025 10:45 PM, Zhu Lingshan wrote:
>>> The pasid is a per-process-per-device attribute,
>>> therefore this commit implements per
>>> struct kfd_process_device->pasid in sysfs
>> This per device pasid is used internally in kfd, not used at user space. So no need to exposing it.
> Agree completely, the PASID is a technical attribute we use internally in the kernel and should not expose to userspace at all.
current kfd sysfs reports pasid to user space, but buggy hard-coded zero, this patch tries fixing this issue.
Or we should remove sysfs/pasid if user space does not need it at all.
Thanks
Lingshan
>
> Maybe in debugfs to narrow down problems, but certainly not in sysfs. That would make the internal handling an uAPI and so not changeable any more.
>
> Regards,
> Christian.
>
>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@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;
>> We keep it to have use space tools(ex rocm-smi) work as the tools still read it before they change.
>>> /* 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);
>> This part is right, these declarations were forgotten to remove.
>>> -
>>> /* 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);
>> Same as above we keep it to have compatibility with current tools.
>>
>> Regards
>>
>> Xiaogang
>>
>>> - 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)
[-- Attachment #2: Type: text/html, Size: 9844 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] amdkfd: implement per process/device pasid in sysfs
2025-10-30 15:53 ` Chen, Xiaogang
2025-10-30 16:15 ` Christian König
@ 2025-10-31 2:48 ` Zhu, Lingshan
2025-10-31 14:40 ` Chen, Xiaogang
1 sibling, 1 reply; 10+ messages in thread
From: Zhu, Lingshan @ 2025-10-31 2:48 UTC (permalink / raw)
To: Chen, Xiaogang, felix.kuehling, alexander.deucher; +Cc: ray.huang, amd-gfx
[-- Attachment #1: Type: text/plain, Size: 5883 bytes --]
On 10/30/2025 11:53 PM, Chen, Xiaogang wrote:
>
> On 10/29/2025 10:45 PM, Zhu Lingshan wrote:
>> The pasid is a per-process-per-device attribute,
>> therefore this commit implements per
>> struct kfd_process_device->pasid in sysfs
> This per device pasid is used internally in kfd, not used at user
> space. So no need to exposing it.
current sysfs implementation exposing PASID to user space, but buggy value 0, this commit
intends to fix this issue.
Thanks
Lingshan
>>
>> Signed-off-by: Zhu Lingshan <lingshan.zhu@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;
> We keep it to have use space tools(ex rocm-smi) work as the tools
> still read it before they change.
>> /* 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);
> This part is right, these declarations were forgotten to remove.
>> -
>> /* 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);
>
> Same as above we keep it to have compatibility with current tools.
>
> Regards
>
> Xiaogang
>
>> - 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)
[-- Attachment #2: Type: text/html, Size: 11029 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH] amdkfd: implement per process/device pasid in sysfs
2025-10-31 2:48 ` Zhu, Lingshan
@ 2025-10-31 14:40 ` Chen, Xiaogang
0 siblings, 0 replies; 10+ messages in thread
From: Chen, Xiaogang @ 2025-10-31 14:40 UTC (permalink / raw)
To: Zhu, Lingshan, felix.kuehling, alexander.deucher; +Cc: ray.huang, amd-gfx
[-- Attachment #1: Type: text/plain, Size: 6294 bytes --]
On 10/30/2025 9:48 PM, Zhu, Lingshan wrote:
> On 10/30/2025 11:53 PM, Chen, Xiaogang wrote:
>>
>> On 10/29/2025 10:45 PM, Zhu Lingshan wrote:
>>> The pasid is a per-process-per-device attribute,
>>> therefore this commit implements per
>>> struct kfd_process_device->pasid in sysfs
>> This per device pasid is used internally in kfd, not used at user
>> space. So no need to exposing it.
> current sysfs implementation exposing PASID to user space, but buggy value 0, this commit
> intends to fix this issue.
It is just for keeping current tools working. The PASID value has no
meaning for use space that it would not get useful info about driver
through it or uses PASID to change driver behavior.
Regards
Xiaogang
>
> Thanks
> Lingshan
>>>
>>> Signed-off-by: Zhu Lingshan <lingshan.zhu@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;
>> We keep it to have use space tools(ex rocm-smi) work as the tools
>> still read it before they change.
>>> /* 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);
>> This part is right, these declarations were forgotten to remove.
>>> -
>>> /* 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);
>>
>> Same as above we keep it to have compatibility with current tools.
>>
>> Regards
>>
>> Xiaogang
>>
>>> - 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)
[-- Attachment #2: Type: text/html, Size: 11180 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] amdkfd: implement per process/device pasid in sysfs
2025-10-30 3:45 [PATCH] amdkfd: implement per process/device pasid in sysfs Zhu Lingshan
2025-10-30 15:05 ` Kuehling, Felix
2025-10-30 15:53 ` Chen, Xiaogang
@ 2025-10-30 18:32 ` Eric Huang
2 siblings, 0 replies; 10+ messages in thread
From: Eric Huang @ 2025-10-30 18:32 UTC (permalink / raw)
To: amd-gfx
We already have pasid in debugfs under
/sys/kernel/debug/kfd/proc/<pid>/pasid_<gpuid> with my patch
"drm/amdkfd: add pasid debugfs entries", so what is the point to add it
in sysfs?
Regards,
Eric
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
>
> Signed-off-by: Zhu Lingshan <lingshan.zhu@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)
^ permalink raw reply [flat|nested] 10+ messages in thread