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

[Lang Yu <Lang.Yu@amd.com>]

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@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!

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;