Re: [PATCH] drm/amdgpu: Raise dma resv usage for created TLB fence

[Christian König <christian.koenig@xxxxxxx>]

    Well that explanation doesn't seem to make much sense either.
    
    What do you mean with TLB flush is occurring prematurely?
    
    Regards,
    Christian.

Am 05.09.24 um 14:38 schrieb Andjelkovic, Dejan:

[AMD Official Use Only - AMD Internal Distribution Only]

Hi there. We're running into a page fault issue that's very easily reproducible on a SRIOV environment when using SDMA for page table updates. Going through mapping logs and trace files, it seems TLB flush is occurring prematurely. Changing the usage to KERNEL completely stops the page fault from occurring with no performance impact, which was confirmed with extensive testing. Looking through amdgpu_vm_sdma.c, namely within amdgpu_vm_sdma_update when waiting for PD/PT moves to be completed, fences are iterated with KERNEL usage specified which are then added as a job dependency.

Best regards,

Dejan

---

From: Koenig, Christian <Christian.Koenig@xxxxxxx>
Sent: Thursday, September 5, 2024 1:17 PM
To: Andjelkovic, Dejan <Dejan.Andjelkovic@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>
Cc: Prica, Nikola <Nikola.Prica@xxxxxxx>; Kuehling, Felix <Felix.Kuehling@xxxxxxx>; Deng, Emily <Emily.Deng@xxxxxxx>
Subject: Re: [PATCH] drm/amdgpu: Raise dma resv usage for created TLB fence

 

Am 05.09.24 um 10:58 schrieb Dejan Andjelkovic:
> When using SDMA for PT updates, a TLB fence hooked to a buffer
> objects dma resv object with usage declared as BOOKKEEP leaves a
> chance for TLB flush to occur prematurely. This will lead to a page
> fault. Raising the usage from BOOKKEEP to KERNEL removes this
> possibility.

Well that's complete nonsense. The usage model is for implicit syncing
and not even remotely related to TLB flushing.

What exactly is the problem you run into?

Regards,
Christian.

>
> Signed-off-by: Dejan Andjelkovic <Dejan.Andjelkovic@xxxxxxx>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> index f93804902fd3..8efc2cf9bbb0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> @@ -928,7 +928,7 @@ amdgpu_vm_tlb_flush(struct amdgpu_vm_update_params *params,
>  
>                /* Makes sure no PD/PT is freed before the flush */
>                dma_resv_add_fence(vm->root.bo->tbo.base.resv, *fence,
> -                                DMA_RESV_USAGE_BOOKKEEP);
> +                                DMA_RESV_USAGE_KERNEL);
>        }
>   }
>