Am 28.08.2018 um 19:34 schrieb Oak Zeng:
> For compute vm acquired from amdgpu, vm.pasid is managed
> by kfd. Decouple pasid from such vm on process destroy
> to avoid duplicate pasid release.
>
> Signed-off-by: Oak Zeng <Oak.Zeng at amd.com>
One minor nit pick below, apart from that Acked-by: Christian König
<christian.koenig at amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h | 1 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c | 1 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c | 1 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c | 1 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 19 +++++++++++++++++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 20 ++++++++++++++++++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 1 +
> drivers/gpu/drm/amd/amdkfd/kfd_process.c | 4 +++-
> drivers/gpu/drm/amd/include/kgd_kfd_interface.h | 1 +
> 9 files changed, 48 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> index e1f6454..b414889 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h
> @@ -165,6 +165,7 @@ int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct kgd_dev *kgd,
> void amdgpu_amdkfd_gpuvm_destroy_cb(struct amdgpu_device *adev,
> struct amdgpu_vm *vm);
> void amdgpu_amdkfd_gpuvm_destroy_process_vm(struct kgd_dev *kgd, void *vm);
> +void amdgpu_amdkfd_gpuvm_release_process_vm(struct kgd_dev *kgd, void *vm);
> uint32_t amdgpu_amdkfd_gpuvm_get_process_page_dir(void *vm);
> int amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu(
> struct kgd_dev *kgd, uint64_t va, uint64_t size,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c
> index ea79908..03a83d3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c
> @@ -204,6 +204,7 @@ static const struct kfd2kgd_calls kfd2kgd = {
> .create_process_vm = amdgpu_amdkfd_gpuvm_create_process_vm,
> .acquire_process_vm = amdgpu_amdkfd_gpuvm_acquire_process_vm,
> .destroy_process_vm = amdgpu_amdkfd_gpuvm_destroy_process_vm,
> + .release_process_vm = amdgpu_amdkfd_gpuvm_release_process_vm,
> .get_process_page_dir = amdgpu_amdkfd_gpuvm_get_process_page_dir,
> .set_vm_context_page_table_base = set_vm_context_page_table_base,
> .alloc_memory_of_gpu = amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c
> index 19dd665..b2b9c5f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c
> @@ -164,6 +164,7 @@ static const struct kfd2kgd_calls kfd2kgd = {
> .create_process_vm = amdgpu_amdkfd_gpuvm_create_process_vm,
> .acquire_process_vm = amdgpu_amdkfd_gpuvm_acquire_process_vm,
> .destroy_process_vm = amdgpu_amdkfd_gpuvm_destroy_process_vm,
> + .release_process_vm = amdgpu_amdkfd_gpuvm_release_process_vm,
> .get_process_page_dir = amdgpu_amdkfd_gpuvm_get_process_page_dir,
> .set_vm_context_page_table_base = set_vm_context_page_table_base,
> .alloc_memory_of_gpu = amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
> index 1db60aa..3722bbd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c
> @@ -201,6 +201,7 @@ static const struct kfd2kgd_calls kfd2kgd = {
> .create_process_vm = amdgpu_amdkfd_gpuvm_create_process_vm,
> .acquire_process_vm = amdgpu_amdkfd_gpuvm_acquire_process_vm,
> .destroy_process_vm = amdgpu_amdkfd_gpuvm_destroy_process_vm,
> + .release_process_vm = amdgpu_amdkfd_gpuvm_release_process_vm,
> .get_process_page_dir = amdgpu_amdkfd_gpuvm_get_process_page_dir,
> .set_vm_context_page_table_base = set_vm_context_page_table_base,
> .alloc_memory_of_gpu = amdgpu_amdkfd_gpuvm_alloc_memory_of_gpu,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> index 0980a1f..bc4413f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c
> @@ -1117,6 +1117,25 @@ void amdgpu_amdkfd_gpuvm_destroy_process_vm(struct kgd_dev *kgd, void *vm)
> kfree(vm);
> }
>
> +void amdgpu_amdkfd_gpuvm_release_process_vm(struct kgd_dev *kgd, void *vm)
> +{
> + struct amdgpu_device *adev = get_amdgpu_device(kgd);
> + struct amdgpu_vm *avm = (struct amdgpu_vm *)vm;
> +
> + if (WARN_ON(!kgd || !vm))
> + return;
> +
> + pr_debug("Releasing process vm %p\n", vm);
> +
> + /* The original pasid of amdgpu vm has already been
> + * released during making a amdgpu vm to a compute vm
> + * The current pasid is managed by kfd and will be
> + * released on kfd process destroy. Set amdgpu pasid
> + * to 0 to avoid duplicate release.
> + */
> + amdgpu_vm_release_compute(adev, avm);
> +}
> +
> uint32_t amdgpu_amdkfd_gpuvm_get_process_page_dir(void *vm)
> {
> struct amdgpu_vm *avm = (struct amdgpu_vm *)vm;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index d8a99f4..d93f2f8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -2766,6 +2766,26 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm, uns
> }
>
> /**
> + * amdgpu_vm_release_compute - release a compute vm
> + * @adev: amdgpu_device pointer
> + * @vm: a vm turned into compute vm by calling amdgpu_vm_make_compute
> + *
> + * This is a correspondant of amdgpu_vm_make_compute. It decouples compute
> + * pasid from vm. Compute should stop use of vm after this call.
> + */
> +void amdgpu_vm_release_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm)
> +{
> + if (vm->pasid) {
> + unsigned long flags;
> +
> + spin_lock_irqsave(&adev->vm_manager.pasid_lock, flags);
> + idr_remove(&adev->vm_manager.pasid_idr, vm->pasid);
> + spin_unlock_irqrestore(&adev->vm_manager.pasid_lock, flags);
> + }
> + vm->pasid = 0;
> +}
> +
> +/**
> * amdgpu_vm_free_levels - free PD/PT levels
> *
> * @adev: amdgpu device structure
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> index c68945d..bd55516 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h
> @@ -295,6 +295,7 @@ void amdgpu_vm_manager_fini(struct amdgpu_device *adev);
> int amdgpu_vm_init(struct amdgpu_device *adev, struct amdgpu_vm *vm,
> int vm_context, unsigned int pasid);
> int amdgpu_vm_make_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm, unsigned int pasid);
> +void amdgpu_vm_release_compute(struct amdgpu_device *adev, struct amdgpu_vm *vm);
> void amdgpu_vm_fini(struct amdgpu_device *adev, struct amdgpu_vm *vm);
> bool amdgpu_vm_pasid_fault_credit(struct amdgpu_device *adev,
> unsigned int pasid);
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_process.c b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> index 5f559c5..7afeaa1 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_process.c
> @@ -320,8 +320,10 @@ static void kfd_process_destroy_pdds(struct kfd_process *p)
> pr_debug("Releasing pdd (topology id %d) for process (pasid %d)\n",
> pdd->dev->id, p->pasid);
>
> - if (pdd->drm_file)
> + if (pdd->drm_file) {
> + pdd->dev->kfd2kgd->release_process_vm(pdd->dev->kgd, pdd->vm);
> fput(pdd->drm_file);
> + }
> else if (pdd->vm)
Coding style says when one branch of an if uses {} all other branches
should do as well. In other words use "} else if (..) {" here.
Christian.
> pdd->dev->kfd2kgd->destroy_process_vm(
> pdd->dev->kgd, pdd->vm);
> diff --git a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> index 1632869..8fdd032 100644
> --- a/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> +++ b/drivers/gpu/drm/amd/include/kgd_kfd_interface.h
> @@ -352,6 +352,7 @@ struct kfd2kgd_calls {
> unsigned int pasid, void **vm, void **process_info,
> struct dma_fence **ef);
> void (*destroy_process_vm)(struct kgd_dev *kgd, void *vm);
> + void (*release_process_vm)(struct kgd_dev *kgd, void *vm);
> uint32_t (*get_process_page_dir)(void *vm);
> void (*set_vm_context_page_table_base)(struct kgd_dev *kgd,
> uint32_t vmid, uint32_t page_table_base);
