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 <[email protected]> >>> --- >>> 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)
