[PATCH 2/2] drm/amdgpu: skip vm entries checking while in sriov vf

[Huang, Ray]

> -----Original Message-----
> From: amd-gfx [mailto:amd-gfx-bounces at lists.freedesktop.org] On Behalf
> Of Felix Kuehling
> Sent: Friday, October 12, 2018 3:46 AM
> To: Koenig, Christian <Christian.Koenig at amd.com>; Min, Frank
> <Frank.Min at amd.com>; amd-gfx at lists.freedesktop.org
> Subject: Re: [PATCH 2/2] drm/amdgpu: skip vm entries checking while in
> sriov vf
> 
> On 2018-10-11 08:32 AM, Christian König wrote:
> > Am 11.10.2018 um 14:09 schrieb Frank.Min:
> >> vm page table would be allocated while in amdgpu_vm_init by
> >> amdgpu_allocate_static_csa for sriov, so the checking here would be
> >> skipped.
> >
> > NAK, that checking is actually correct and points out that this
> > doesn't work correctly.
> >
> > Fix the function to be able to handle the case when there are already
> > mappings in the VM.
> 
> KFD and the Thunk assume that they own the entire address space. If
> something is already mapped, KFD and the Thunk would not be aware of it.
> 
> amdgpu_allocate_static_csa runs during driver initialization, before there are
> any VMs. Looks like amdgpu_map_static_csa is the more important function
> here, which currently runs in amdgpu_driver_open_kms.
> 
> Could amdgpu_map_static_csa be delayed to the first time user mode maps
> memory into the VM through GEM APIs? That way it would allow turning a
> VM into a compute VM that hasn't been used for any GEM memory
> mappings.
>

Yes. Frank and I found, while we do acquire_process_vm(), the vm->va RB trees are always not empty under SRIOV, because amdgpu_map_static_csa() will be called while vm inits under SRIOV, that will actually map the CSA BO to the VM and insert the node into vm->va before it acquires process VM in KFD. 
 
Thanks,
Ray

> Regards,
>   Felix
> 
> >
> > Christian.
> >
> >>
> >> Change-Id: Id30b86ad15ae509aeed9ed8ab60c259c88af3df5
> >> Signed-off-by: Frank.Min <Frank.Min at amd.com>
> >> ---
> >>   drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 9 +++++----
> >>   1 file changed, 5 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> >> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> >> index 6904d79..6066f87 100644
> >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
> >> @@ -3100,11 +3100,12 @@ int amdgpu_vm_make_compute(struct
> >> amdgpu_device *adev, struct amdgpu_vm *vm, uns
> >>           return r;
> >>         /* Sanity checks */
> >> -    if (!RB_EMPTY_ROOT(&vm->va.rb_root) || vm->root.entries) {
> >> -        r = -EINVAL;
> >> -        goto unreserve_bo;
> >> +    if (!amdgpu_sriov_vf(adev)) {
> >> +        if (!RB_EMPTY_ROOT(&vm->va.rb_root) || vm->root.entries) {
> >> +            r = -EINVAL;
> >> +            goto unreserve_bo;
> >> +        }
> >>       }
> >> -
> >>       if (pasid) {
> >>           unsigned long flags;
> >>
> >
> > _______________________________________________
> > amd-gfx mailing list
> > amd-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> 
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx