On 11/1/2025 4:56 AM, Kuehling, Felix wrote:

> On 2025-10-30 23:32, Zhu Lingshan wrote:
>> This commit removes pasid under kfd sysfs folders
>> because pasid should only be used in kernel internally,
>> should not be exposed to user space, and current
>> pasid under kfd sysfs is buggy hard-coded 0.
>
> NAK. We left this file with a dummy value deliberately, to prevent
> breaking existing ROCm SMI versions. We can never remove this as long
> as such versions of ROCm SMI may be in circulation.

I am not sure 0 is a good dummy value, because 0 is meaningful, usually stands 
for "the first one" or "success".
ROCm SMI may treat pasid == 0 as a dummy, but there can be other user space 
tools, even non-opensource customer tools
may see this 0 as the first pasid.

pasid ==0 is misleading here, so I suggest we show pasid == -1 which is 
opposite to the concept of pasid.

Thanks
Lingshan 

>  
>
> Regards,
>   Felix
>
>
>>
>> Signed-off-by: Zhu Lingshan <[email protected]>
>> ---
>>   drivers/gpu/drm/amd/amdkfd/kfd_priv.h    | 7 -------
>>   drivers/gpu/drm/amd/amdkfd/kfd_process.c | 8 +-------
>>   2 files changed, 1 insertion(+), 14 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> index 70ef051511bb..d69079cab1e6 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
>> @@ -983,7 +983,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 +1099,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..f45780502f06 100644
>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
>> @@ -328,9 +328,7 @@ 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, "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));
>> @@ -888,9 +886,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 +1099,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;

Reply via email to