[PATCH 3/3] drm/amdkfd: use existing VM helper for PD and PT validation in SVM

[Felix Kuehling]

Am 2022-06-07 um 22:21 schrieb Lang Yu:
> On 06/07/ , Felix Kuehling wrote:
>> Am 2022-06-07 um 05:59 schrieb Lang Yu:
>>> This will remove some redundant codes.
>>>
>>> Signed-off-by: Lang Yu <Lang.Yu at amd.com>
>> The redundancy is quite small, and amdgpu_amdkfd_gpuvm_validate_pt_pd_bos
>> and amdgpu_amdkfd_bo_validate are quite a bit more complex and handle more
>> different cases. Someone changing those functions in the future may not
>> realize the effect that may have on the SVM code.
>>
>> I'd prefer to keep the svm_range_bo_validate function in kfd_svm.c to make
>> the code easier to understand and maintain. If anything, I'd move it closer
>> to where its used, because it's only used in one place.
> Thanks for your comments. I got it. By the way,
> is it necessary to update vm->pd_phys_addr here?
> I noticed that vm->pd_phys_addr is updated in
> vm_validate_pt_pd_bos()? Thanks!

It's not needed here. The only time we need to update this is, when we 
re-enable queues after a PD eviction. This is handled by the PD 
validation in amdgpu_amdkfd_gpuvm_restore_process_bos -> 
process_validate_vms -> vm_validate_pt_pd_bos.

Regards,
   Felix


>
> Regards,
> Lang
>
>> Regards,
>>    Felix
>>
>>
>>> ---
>>>    drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 13 +------------
>>>    1 file changed, 1 insertion(+), 12 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
>>> index d6fc00d51c8c..03e07d1d1d1a 100644
>>> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
>>> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
>>> @@ -625,15 +625,6 @@ svm_range_get_pdd_by_adev(struct svm_range *prange, struct amdgpu_device *adev)
>>>    	return kfd_process_device_from_gpuidx(p, gpu_idx);
>>>    }
>>> -static int svm_range_bo_validate(void *param, struct amdgpu_bo *bo)
>>> -{
>>> -	struct ttm_operation_ctx ctx = { false, false };
>>> -
>>> -	amdgpu_bo_placement_from_domain(bo, AMDGPU_GEM_DOMAIN_VRAM);
>>> -
>>> -	return ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
>>> -}
>>> -
>>>    static int
>>>    svm_range_check_attr(struct kfd_process *p,
>>>    		     uint32_t nattr, struct kfd_ioctl_svm_attribute *attrs)
>>> @@ -1428,9 +1419,7 @@ static int svm_range_reserve_bos(struct svm_validate_context *ctx)
>>>    			goto unreserve_out;
>>>    		}
>>> -		r = amdgpu_vm_validate_pt_bos(pdd->dev->adev,
>>> -					      drm_priv_to_vm(pdd->drm_priv),
>>> -					      svm_range_bo_validate, NULL);
>>> +		r = amdgpu_amdkfd_gpuvm_validate_pt_pd_bos(drm_priv_to_vm(pdd->drm_priv));
>>>    		if (r) {
>>>    			pr_debug("failed %d validate pt bos\n", r);
>>>    			goto unreserve_out;