[PATCH 2/2] drm/amdgpu: Synchronize after mapping into a compute VM

[Felix Kuehling]

On 2023-02-27 04:42, Christian König wrote:
> Am 25.02.23 um 00:36 schrieb Felix Kuehling:
>> Compute VMs use user mode queues for command submission. They cannot use
>> a CS ioctl to synchronize with pending PTE updates and flush TLBs. Do
>> this synchronization in amdgpu_gem_va_ioctl for compute VMs.
>>
>> Signed-off-by: Felix Kuehling <Felix.Kuehling at amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 16 ++++++++++++++--
>>   1 file changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> index 6936cd63df42..7de5057c40ec 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
>> @@ -601,7 +601,7 @@ int amdgpu_gem_metadata_ioctl(struct drm_device 
>> *dev, void *data,
>>   static void amdgpu_gem_va_update_vm(struct amdgpu_device *adev,
>>                       struct amdgpu_vm *vm,
>>                       struct amdgpu_bo_va *bo_va,
>> -                    uint32_t operation)
>> +                    uint32_t operation, uint32_t flags)
>>   {
>>       int r;
>>   @@ -620,6 +620,18 @@ static void amdgpu_gem_va_update_vm(struct 
>> amdgpu_device *adev,
>>       }
>>         r = amdgpu_vm_update_pdes(adev, vm, false);
>> +    if (r)
>> +        goto error;
>> +
>> +    if (vm->is_compute_context) {
>> +        if (bo_va->last_pt_update)
>> +            r = dma_fence_wait(bo_va->last_pt_update, true);
>> +        if (!r && vm->last_update)
>> +            r = dma_fence_wait(vm->last_update, true);
>> +        if (!r)
>> +            r = amdgpu_amdkfd_flush_tlb(adev, vm,
>> +                TLB_FLUSH_LEGACY);
>> +    }
>
> Mhm, that's not really something we can do here. The GEM VA IOCTL is 
> supposed to be async and never block.
>
> Can we do that on the KFD side in some IOCTL instead?

Not really. There is no existing KFD ioctl I would call after GEM_VA. 
The whole point was to use GEM ioctls to manages virtual address 
mappings for KFD and avoid adding more KFD ioctls that duplicate similar 
functionality.

If you wanted to tie it into the existing amdgpu memory manager, I guess 
we'd need a dummy command submission that does the synchronization and 
TLB flush and the wait for the fence from that.

Regards,
   Felix


>
> Regards,
> Christian.
>
>>     error:
>>       if (r && r != -ERESTARTSYS)
>> @@ -789,7 +801,7 @@ int amdgpu_gem_va_ioctl(struct drm_device *dev, 
>> void *data,
>>       }
>>       if (!r && !(args->flags & AMDGPU_VM_DELAY_UPDATE) && 
>> !amdgpu_vm_debug)
>>           amdgpu_gem_va_update_vm(adev, &fpriv->vm, bo_va,
>> -                    args->operation);
>> +                    args->operation, args->flags);
>>     error_backoff:
>>       ttm_eu_backoff_reservation(&ticket, &list);
>