Ah, in this case please separate the amdgpu_vm_bo_rmv() from setting
csa_addr to NULL.
Cause amdgpu_vm_bo_rmv() should come before amdgpu_vm_fini() and that in
turn should become before waiting for the scheduler so that the MM knows
that the memory is about to be freed.
Regards,
Christian.
Am 13.01.2017 um 10:56 schrieb Liu, Monk:
only with amdgpu_vm_bo_rmv() won't has such bug, but in another branch
for sriov, we not only call vm_bo_rmv(), and we also set csa_addr to
NULL after it, so the NULL address is inserted in RB, and when
preemption occured, CP backup snapshot to NULL address.
although in staging-4.9 we didn't set csa_addr to NULL (because as you
suggested we always use HARDCODE/MACRO for CSA address), but logically
we'd better put CSA unmapping stuffs behind "sched_entity_fini", which
is more reasonable ...
BR Monk
*发件人:* amd-gfx 代表 Christian
König
*发送时间:* 2017年1月13日 17:25:09
*收件人:* Liu, Monk; amd-gfx@lists.freedesktop.org
*主题:* Re: [PATCH] drm/amdgpu:put CSA unmap after sched_entity_fini
Am 13.01.2017 um 05:11 schrieb Monk Liu:
> otherwise CSA may unmapped before gpu_scheduler scheduling
> jobs and trigger VM fault on CSA address
>
> Change-Id: Ib2e25ededf89bca44c764477dd2f9127024ca78c
> Signed-off-by: Monk Liu
Did you really run into an issue because of that?
Calling amdgpu_vm_bo_rmv() shouldn't affect the page tables nor already
submitted command submissions in any way.
Regards,
Christian.
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 8
> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 8
> 2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 45484c0..e13cdde 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -694,14 +694,6 @@ void amdgpu_driver_postclose_kms(struct
drm_device *dev,
>amdgpu_uvd_free_handles(adev, file_priv);
>amdgpu_vce_free_handles(adev, file_priv);
>
> - if (amdgpu_sriov_vf(adev)) {
> - /* TODO: how to handle reserve failure */
> - BUG_ON(amdgpu_bo_reserve(adev->virt.csa_obj, false));
> - amdgpu_vm_bo_rmv(adev, fpriv->vm.csa_bo_va);
> - fpriv->vm.csa_bo_va = NULL;
> - amdgpu_bo_unreserve(adev->virt.csa_obj);
> - }
> -
>amdgpu_vm_fini(adev, &fpriv->vm);
>
> idr_for_each_entry(&fpriv->bo_list_handles, list, handle)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index d05546e..94098bc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -1608,6 +1608,14 @@ void amdgpu_vm_fini(struct amdgpu_device
*adev, struct amdgpu_vm *vm)
>
>amd_sched_entity_fini(vm->entity.sched, &vm->entity);
>
> + if (amdgpu_sriov_vf(adev)) {
> + /* TODO: how to handle reserve failure */
> + BUG_ON(amdgpu_bo_reserve(adev->virt.csa_obj, false));
> + amdgpu_vm_bo_rmv(adev, vm->csa_bo_va);
> + vm->csa_bo_va = NULL;
> + amdgpu_bo_unreserve(adev->virt.csa_obj);
> + }
> +
>if (!RB_EMPTY_ROOT(&vm->va)) {
>dev_err(adev->dev, "still active bo inside vm\n");
>}
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
